On Fri, Dec 01, 2023 at 10:34:50AM +0200, Dmitry Baryshkov wrote: > On Fri, 1 Dec 2023 at 05:47, Bjorn Andersson <quic_bjorande@xxxxxxxxxxx> wrote: > > On Thu, Nov 30, 2023 at 05:40:55PM -0800, Paloma Arellano wrote: [..] > > > @@ -2386,6 +2390,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, > > > dpu_enc->enabled = false; > > > mutex_init(&dpu_enc->enc_lock); > > > mutex_init(&dpu_enc->rc_lock); > > > + mutex_init(&dpu_enc->vblank_ctl_lock); > > > > Is this somehow propagated to multiple different dpu_encoder_phys > > instances, or why do you need to initialize it here and pass the pointer > > through 2 different intermediate structures before assigning it to > > phys_enc->vblank_ctl_lock below? > > Yes, there can be two phys_enc instances for a single encoder, so this > part is fine. > Thanks for the clarification, Dmitry. Sounds like it make sense then. But, if I read the code correctly the two instances will have separate vblank_refcount copies, and the dpu_core_irq_*() interface does mutual exclusion within. So why do we need shared mutual exclusion between the two? (This is where a proper description of the problem in the commit message would have been very helpful) Regards, Bjorn
