On Fri, Dec 01, 2023 at 10:34:50AM +0200, Dmitry Baryshkov wrote:
> On Fri, 1 Dec 2023 at 05:47, Bjorn Andersson <quic_bjora...@quicinc.com> 
> 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

Reply via email to