mbox series

[v3,0/2] Stabilize use of vblank_refcount

Message ID 20231201014101.15802-1-quic_parellan@quicinc.com
Headers show
Series Stabilize use of vblank_refcount | expand

Message

Paloma Arellano Dec. 1, 2023, 1:40 a.m. UTC
There is currently a race condition occuring when accessing
vblank_refcount. Therefore, vblank irq timeouts may occur.

Avoid any vblank irq timeouts by stablizing the use of vblank_refcount.

Changes from prior versions:
   v2: - Slightly changed wording of patch #2 commit message
   v3: - Mistakenly did not change wording of patch #2 in last version.
         It is done now.

Paloma Arellano (2):
  drm/msm/dpu: Modify vblank_refcount if error in callback
  drm/msm/dpu: Add mutex lock in control vblank irq

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c          |  6 ++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h     |  6 ++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 11 +++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 11 +++++++++--
 4 files changed, 30 insertions(+), 4 deletions(-)

Comments

Abhinav Kumar Dec. 1, 2023, 7:14 p.m. UTC | #1
On 11/30/2023 11:45 PM, Dmitry Baryshkov wrote:
> On Fri, 1 Dec 2023 at 03:41, Paloma Arellano <quic_parellan@quicinc.com> wrote:
>>
>> When the irq callback returns a value other than zero,
>> modify vblank_refcount by performing the inverse
>> operation of its corresponding if-else condition.
> 
> I think it might be better to follow Bjorn's suggestion: once we have
> the lock, we don't need atomics at all.
> Then you rearrange the code to set the new value after getting return
> code from dpu_core_irq_register_callback() /
> dpu_core_irq_unregister_callback().
> 

Even if we drop the atomics, we will have to replace it with a simple 
refcount. The refcount checks will be before calling 
dpu_core_irq_register_callback() / dpu_core_irq_unregister_callback().

So we will still need the corresponding inverse refcount when either of 
those calls fail but just that they wont be atomics.

Am I missing something here?

>>
>> Signed-off-by: Paloma Arellano <quic_parellan@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 9 +++++++--
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 +++++++--
>>   2 files changed, 14 insertions(+), 4 deletions(-)
>