Re: [PATCH v3 1/2] drm/msm/dpu: Modify vblank_refcount if error in callback
On Fri, 1 Dec 2023 at 21:14, Abhinav Kumar wrote: > > > > On 11/30/2023 11:45 PM, Dmitry Baryshkov wrote: > > On Fri, 1 Dec 2023 at 03:41, Paloma Arellano > > 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. Within the critical section you can check the value before register_callback and increment it afterwards if registration succeeds. > > Am I missing something here? > > >> > >> Signed-off-by: Paloma Arellano > >> --- > >> 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(-) > > -- With best wishes Dmitry
Re: [PATCH v3 1/2] drm/msm/dpu: Modify vblank_refcount if error in callback
On 11/30/2023 11:45 PM, Dmitry Baryshkov wrote: On Fri, 1 Dec 2023 at 03:41, Paloma Arellano 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 --- 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(-)
Re: [PATCH v3 1/2] drm/msm/dpu: Modify vblank_refcount if error in callback
On Fri, 1 Dec 2023 at 03:41, Paloma Arellano 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(). > > Signed-off-by: Paloma Arellano > --- > 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(-) -- With best wishes Dmitry
