Re: [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer
On 2018-11-26 13:53, Sean Paul wrote:
On Mon, Nov 19, 2018 at 12:03:53PM -0800, Jeykumar Sankaran wrote:
On 2018-11-16 13:14, Sean Paul wrote:
> On Fri, Nov 16, 2018 at 12:05:09PM -0800, Jeykumar Sankaran wrote:
> > On 2018-11-16 10:42, Sean Paul wrote:
> > > From: Sean Paul
> > >
> > > It's for legacy drivers, for atomic drivers
crtc->state->encoder_mask
> > > should be used to map encoder to crtc.
> > >
> > > Changes in v2:
> > > - None
> > >
> > > Signed-off-by: Sean Paul
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
>
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 19 +++---
> > > 2 files changed, 29 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > index 156f4c77ca44..a008a87a8113 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > @@ -284,9 +284,9 @@ enum dpu_intf_mode
dpu_crtc_get_intf_mode(struct
> > > drm_crtc *crtc)
> > > return INTF_MODE_NONE;
> > > }
> > >
> > > -drm_for_each_encoder(encoder, crtc->dev)
> > > -if (encoder->crtc == crtc)
> > > -return dpu_encoder_get_intf_mode(encoder);
> > > +/* TODO: Returns the first INTF_MODE, could there be
multiple
> > > values? */
> > > +drm_for_each_encoder_mask(encoder, crtc->dev,
> > > crtc->state->encoder_mask)
> > > +return dpu_encoder_get_intf_mode(encoder);
> > >
> > > return INTF_MODE_NONE;
> > > }
> > > @@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct
drm_crtc
> > > *crtc,
> > > spin_unlock_irqrestore(>event_lock, flags);
> > > }
> > >
> > > -list_for_each_entry(encoder,
>mode_config.encoder_list, head)
> > > {
> > > -if (encoder->crtc != crtc)
> > > -continue;
> > > -
> > > -/* encoder will trigger pending mask now */
> > > +/* encoder will trigger pending mask now */
> > > +drm_for_each_encoder_mask(encoder, crtc->dev,
> > > crtc->state->encoder_mask)
> > > dpu_encoder_trigger_kickoff_pending(encoder);
> > > -}
> > >
> > > /*
> > > * If no mixers have been allocated in
dpu_crtc_atomic_check(),
> > > @@ -704,7 +700,6 @@ static int
_dpu_crtc_wait_for_frame_done(struct
> > > drm_crtc *crtc)
> > > void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> > > {
> > > struct drm_encoder *encoder;
> > > -struct drm_device *dev = crtc->dev;
> > > struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> > > struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> > > struct dpu_crtc_state *cstate =
to_dpu_crtc_state(crtc->state);
> > > @@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > > *crtc)
> > >
> > > DPU_ATRACE_BEGIN("crtc_commit");
> > >
> > > -list_for_each_entry(encoder,
>mode_config.encoder_list, head)
> > > {
> > > +/*
> > > + * Encoder will flush/start now, unless it has a tx
pending. If
> > > so, it
> > > + * may delay and flush at an irq event (e.g. ppdone)
> > > + */
> > > +drm_for_each_encoder_mask(encoder, crtc->dev,
> > > + crtc->state->encoder_mask) {
> > > struct dpu_encoder_kickoff_params params = { 0 };
> > > -
> > > -if (encoder->crtc != crtc)
> > > -continue;
> > > -
> > > -/*
> > > - * Encoder will flush/start now, unless it has a
tx
> > > pending.
> > > - * If so, it may delay and flush at an irq event
(e.g.
> > > ppdone)
> > > - */
> > > dpu_encoder_prepare_for_kickoff(encoder, );
> > > }
> > >
> > > @@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> *crtc)
> > >
> > > dpu_vbif_clear_errors(dpu_kms);
> > >
> > > -list_for_each_entry(encoder,
>mode_config.encoder_list, head)
> > > {
> > > -if (encoder->crtc != crtc)
> > > -continue;
> > > -
> > > +drm_for_each_encoder_mask(encoder, crtc->dev,
> > > crtc->state->encoder_mask)
> > > dpu_encoder_kickoff(encoder);
> > > -}
> > We wont be holding the modeset locks here (and in crtc_atomic_begin)
> > in
> the
> > display thread. Is
> > it safe to iterate over encoder_mask?
>
> Hmm, I'm not sure I follow. AFAICT, there are 2 callsites for
> dpu_crtc_commit_kickoff():
>
> 1- dpu_kms_encoder_enable() which is called via the
> encoder->funcs->enable
> hook
> 2- dpu_kms_commit() which is called in the
> mode_config->funcs->atomic_commit_tail
atomic_commit_tail will be called from the WQ. Do we hold
the modeset locks in the WQ? I believe userspace thread will drop
the locks at the
Re: [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer
On Mon, Nov 19, 2018 at 12:03:53PM -0800, Jeykumar Sankaran wrote:
> On 2018-11-16 13:14, Sean Paul wrote:
> > On Fri, Nov 16, 2018 at 12:05:09PM -0800, Jeykumar Sankaran wrote:
> > > On 2018-11-16 10:42, Sean Paul wrote:
> > > > From: Sean Paul
> > > >
> > > > It's for legacy drivers, for atomic drivers crtc->state->encoder_mask
> > > > should be used to map encoder to crtc.
> > > >
> > > > Changes in v2:
> > > > - None
> > > >
> > > > Signed-off-by: Sean Paul
> > > > ---
> > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
> >
> > > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 19 +++---
> > > > 2 files changed, 29 insertions(+), 36 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > index 156f4c77ca44..a008a87a8113 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > @@ -284,9 +284,9 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
> > > > drm_crtc *crtc)
> > > > return INTF_MODE_NONE;
> > > > }
> > > >
> > > > - drm_for_each_encoder(encoder, crtc->dev)
> > > > - if (encoder->crtc == crtc)
> > > > - return dpu_encoder_get_intf_mode(encoder);
> > > > + /* TODO: Returns the first INTF_MODE, could there be multiple
> > > > values? */
> > > > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > > > crtc->state->encoder_mask)
> > > > + return dpu_encoder_get_intf_mode(encoder);
> > > >
> > > > return INTF_MODE_NONE;
> > > > }
> > > > @@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
> > > > *crtc,
> > > > spin_unlock_irqrestore(>event_lock, flags);
> > > > }
> > > >
> > > > - list_for_each_entry(encoder, >mode_config.encoder_list,
> > > > head)
> > > > {
> > > > - if (encoder->crtc != crtc)
> > > > - continue;
> > > > -
> > > > - /* encoder will trigger pending mask now */
> > > > + /* encoder will trigger pending mask now */
> > > > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > > > crtc->state->encoder_mask)
> > > > dpu_encoder_trigger_kickoff_pending(encoder);
> > > > - }
> > > >
> > > > /*
> > > > * If no mixers have been allocated in dpu_crtc_atomic_check(),
> > > > @@ -704,7 +700,6 @@ static int _dpu_crtc_wait_for_frame_done(struct
> > > > drm_crtc *crtc)
> > > > void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> > > > {
> > > > struct drm_encoder *encoder;
> > > > - struct drm_device *dev = crtc->dev;
> > > > struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> > > > struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> > > > struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> > > > @@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > > > *crtc)
> > > >
> > > > DPU_ATRACE_BEGIN("crtc_commit");
> > > >
> > > > - list_for_each_entry(encoder, >mode_config.encoder_list,
> > > > head)
> > > > {
> > > > + /*
> > > > +* Encoder will flush/start now, unless it has a tx pending. If
> > > > so, it
> > > > +* may delay and flush at an irq event (e.g. ppdone)
> > > > +*/
> > > > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > > > + crtc->state->encoder_mask) {
> > > > struct dpu_encoder_kickoff_params params = { 0 };
> > > > -
> > > > - if (encoder->crtc != crtc)
> > > > - continue;
> > > > -
> > > > - /*
> > > > -* Encoder will flush/start now, unless it has a tx
> > > > pending.
> > > > -* If so, it may delay and flush at an irq event (e.g.
> > > > ppdone)
> > > > -*/
> > > > dpu_encoder_prepare_for_kickoff(encoder, );
> > > > }
> > > >
> > > > @@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > *crtc)
> > > >
> > > > dpu_vbif_clear_errors(dpu_kms);
> > > >
> > > > - list_for_each_entry(encoder, >mode_config.encoder_list,
> > > > head)
> > > > {
> > > > - if (encoder->crtc != crtc)
> > > > - continue;
> > > > -
> > > > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > > > crtc->state->encoder_mask)
> > > > dpu_encoder_kickoff(encoder);
> > > > - }
> > > We wont be holding the modeset locks here (and in crtc_atomic_begin)
> > > in
> > the
> > > display thread. Is
> > > it safe to iterate over encoder_mask?
> >
> > Hmm, I'm not sure I follow. AFAICT, there are 2 callsites for
> > dpu_crtc_commit_kickoff():
> >
> > 1- dpu_kms_encoder_enable() which is called via the
> > encoder->funcs->enable
> > hook
> > 2- dpu_kms_commit() which is called in the
> >
Re: [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer
On 2018-11-16 13:14, Sean Paul wrote:
On Fri, Nov 16, 2018 at 12:05:09PM -0800, Jeykumar Sankaran wrote:
On 2018-11-16 10:42, Sean Paul wrote:
> From: Sean Paul
>
> It's for legacy drivers, for atomic drivers crtc->state->encoder_mask
> should be used to map encoder to crtc.
>
> Changes in v2:
> - None
>
> Signed-off-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 19 +++---
> 2 files changed, 29 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 156f4c77ca44..a008a87a8113 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -284,9 +284,9 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
> drm_crtc *crtc)
>return INTF_MODE_NONE;
>}
>
> - drm_for_each_encoder(encoder, crtc->dev)
> - if (encoder->crtc == crtc)
> - return dpu_encoder_get_intf_mode(encoder);
> + /* TODO: Returns the first INTF_MODE, could there be multiple
> values? */
> + drm_for_each_encoder_mask(encoder, crtc->dev,
> crtc->state->encoder_mask)
> + return dpu_encoder_get_intf_mode(encoder);
>
>return INTF_MODE_NONE;
> }
> @@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
> *crtc,
>spin_unlock_irqrestore(>event_lock, flags);
>}
>
> - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> {
> - if (encoder->crtc != crtc)
> - continue;
> -
> - /* encoder will trigger pending mask now */
> + /* encoder will trigger pending mask now */
> + drm_for_each_encoder_mask(encoder, crtc->dev,
> crtc->state->encoder_mask)
>dpu_encoder_trigger_kickoff_pending(encoder);
> - }
>
>/*
> * If no mixers have been allocated in dpu_crtc_atomic_check(),
> @@ -704,7 +700,6 @@ static int _dpu_crtc_wait_for_frame_done(struct
> drm_crtc *crtc)
> void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> {
>struct drm_encoder *encoder;
> - struct drm_device *dev = crtc->dev;
>struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
>struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
>struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> @@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> *crtc)
>
>DPU_ATRACE_BEGIN("crtc_commit");
>
> - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> {
> + /*
> + * Encoder will flush/start now, unless it has a tx pending. If
> so, it
> + * may delay and flush at an irq event (e.g. ppdone)
> + */
> + drm_for_each_encoder_mask(encoder, crtc->dev,
> +crtc->state->encoder_mask) {
>struct dpu_encoder_kickoff_params params = { 0 };
> -
> - if (encoder->crtc != crtc)
> - continue;
> -
> - /*
> - * Encoder will flush/start now, unless it has a tx
> pending.
> - * If so, it may delay and flush at an irq event (e.g.
> ppdone)
> - */
>dpu_encoder_prepare_for_kickoff(encoder, );
>}
>
> @@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
*crtc)
>
>dpu_vbif_clear_errors(dpu_kms);
>
> - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> {
> - if (encoder->crtc != crtc)
> - continue;
> -
> + drm_for_each_encoder_mask(encoder, crtc->dev,
> crtc->state->encoder_mask)
>dpu_encoder_kickoff(encoder);
> - }
We wont be holding the modeset locks here (and in crtc_atomic_begin)
in
the
display thread. Is
it safe to iterate over encoder_mask?
Hmm, I'm not sure I follow. AFAICT, there are 2 callsites for
dpu_crtc_commit_kickoff():
1- dpu_kms_encoder_enable() which is called via the
encoder->funcs->enable
hook
2- dpu_kms_commit() which is called in the
mode_config->funcs->atomic_commit_tail
atomic_commit_tail will be called from the WQ. Do we hold
the modeset locks in the WQ? I believe userspace thread will drop
the locks at the end of drm_mode_atomic_ioctl.
Thanks
Both of these callsites will hold the modeset locks.
Am I missing something?
Thanks for your review,
Sean
>
> end:
>reinit_completion(_crtc->frame_done_comp);
> @@ -883,11 +871,8 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
>
>dpu_core_perf_crtc_update(crtc, 0, true);
>
> - drm_for_each_encoder(encoder, crtc->dev) {
> - if (encoder->crtc != crtc)
> - continue;
> + drm_for_each_encoder_mask(encoder, crtc->dev,
> crtc->state->encoder_mask)
>dpu_encoder_register_frame_event_callback(encoder, NULL,
> NULL);
> - }
>
>memset(cstate->mixers, 0, sizeof(cstate->mixers));
>cstate->num_mixers = 0;
> @@ -922,12 +907,9 @@ static void dpu_crtc_enable(struct drm_crtc
*crtc,
>DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
>dpu_crtc = to_dpu_crtc(crtc);
>
> -
Re: [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer
On Fri, Nov 16, 2018 at 12:05:09PM -0800, Jeykumar Sankaran wrote:
> On 2018-11-16 10:42, Sean Paul wrote:
> > From: Sean Paul
> >
> > It's for legacy drivers, for atomic drivers crtc->state->encoder_mask
> > should be used to map encoder to crtc.
> >
> > Changes in v2:
> > - None
> >
> > Signed-off-by: Sean Paul
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 19 +++---
> > 2 files changed, 29 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index 156f4c77ca44..a008a87a8113 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -284,9 +284,9 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
> > drm_crtc *crtc)
> > return INTF_MODE_NONE;
> > }
> >
> > - drm_for_each_encoder(encoder, crtc->dev)
> > - if (encoder->crtc == crtc)
> > - return dpu_encoder_get_intf_mode(encoder);
> > + /* TODO: Returns the first INTF_MODE, could there be multiple
> > values? */
> > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > crtc->state->encoder_mask)
> > + return dpu_encoder_get_intf_mode(encoder);
> >
> > return INTF_MODE_NONE;
> > }
> > @@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
> > *crtc,
> > spin_unlock_irqrestore(>event_lock, flags);
> > }
> >
> > - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> > {
> > - if (encoder->crtc != crtc)
> > - continue;
> > -
> > - /* encoder will trigger pending mask now */
> > + /* encoder will trigger pending mask now */
> > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > crtc->state->encoder_mask)
> > dpu_encoder_trigger_kickoff_pending(encoder);
> > - }
> >
> > /*
> > * If no mixers have been allocated in dpu_crtc_atomic_check(),
> > @@ -704,7 +700,6 @@ static int _dpu_crtc_wait_for_frame_done(struct
> > drm_crtc *crtc)
> > void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> > {
> > struct drm_encoder *encoder;
> > - struct drm_device *dev = crtc->dev;
> > struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> > struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> > struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> > @@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > *crtc)
> >
> > DPU_ATRACE_BEGIN("crtc_commit");
> >
> > - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> > {
> > + /*
> > +* Encoder will flush/start now, unless it has a tx pending. If
> > so, it
> > +* may delay and flush at an irq event (e.g. ppdone)
> > +*/
> > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > + crtc->state->encoder_mask) {
> > struct dpu_encoder_kickoff_params params = { 0 };
> > -
> > - if (encoder->crtc != crtc)
> > - continue;
> > -
> > - /*
> > -* Encoder will flush/start now, unless it has a tx
> > pending.
> > -* If so, it may delay and flush at an irq event (e.g.
> > ppdone)
> > -*/
> > dpu_encoder_prepare_for_kickoff(encoder, );
> > }
> >
> > @@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> >
> > dpu_vbif_clear_errors(dpu_kms);
> >
> > - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> > {
> > - if (encoder->crtc != crtc)
> > - continue;
> > -
> > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > crtc->state->encoder_mask)
> > dpu_encoder_kickoff(encoder);
> > - }
> We wont be holding the modeset locks here (and in crtc_atomic_begin) in the
> display thread. Is
> it safe to iterate over encoder_mask?
Hmm, I'm not sure I follow. AFAICT, there are 2 callsites for
dpu_crtc_commit_kickoff():
1- dpu_kms_encoder_enable() which is called via the encoder->funcs->enable hook
2- dpu_kms_commit() which is called in the
mode_config->funcs->atomic_commit_tail
Both of these callsites will hold the modeset locks.
Am I missing something?
Thanks for your review,
Sean
> >
> > end:
> > reinit_completion(_crtc->frame_done_comp);
> > @@ -883,11 +871,8 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
> >
> > dpu_core_perf_crtc_update(crtc, 0, true);
> >
> > - drm_for_each_encoder(encoder, crtc->dev) {
> > - if (encoder->crtc != crtc)
> > - continue;
> > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > crtc->state->encoder_mask)
> > dpu_encoder_register_frame_event_callback(encoder, NULL,
> > NULL);
> > - }
> >
> > memset(cstate->mixers, 0, sizeof(cstate->mixers));
> > cstate->num_mixers = 0;
> > @@ -922,12 +907,9 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
> >
Re: [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer
On 2018-11-16 10:42, Sean Paul wrote:
From: Sean Paul
It's for legacy drivers, for atomic drivers crtc->state->encoder_mask
should be used to map encoder to crtc.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 19 +++---
2 files changed, 29 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 156f4c77ca44..a008a87a8113 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -284,9 +284,9 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
drm_crtc *crtc)
return INTF_MODE_NONE;
}
- drm_for_each_encoder(encoder, crtc->dev)
- if (encoder->crtc == crtc)
- return dpu_encoder_get_intf_mode(encoder);
+ /* TODO: Returns the first INTF_MODE, could there be multiple
values? */
+ drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
+ return dpu_encoder_get_intf_mode(encoder);
return INTF_MODE_NONE;
}
@@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
*crtc,
spin_unlock_irqrestore(>event_lock, flags);
}
- list_for_each_entry(encoder, >mode_config.encoder_list, head)
{
- if (encoder->crtc != crtc)
- continue;
-
- /* encoder will trigger pending mask now */
+ /* encoder will trigger pending mask now */
+ drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
dpu_encoder_trigger_kickoff_pending(encoder);
- }
/*
* If no mixers have been allocated in dpu_crtc_atomic_check(),
@@ -704,7 +700,6 @@ static int _dpu_crtc_wait_for_frame_done(struct
drm_crtc *crtc)
void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
{
struct drm_encoder *encoder;
- struct drm_device *dev = crtc->dev;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
@@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
*crtc)
DPU_ATRACE_BEGIN("crtc_commit");
- list_for_each_entry(encoder, >mode_config.encoder_list, head)
{
+ /*
+* Encoder will flush/start now, unless it has a tx pending. If
so, it
+* may delay and flush at an irq event (e.g. ppdone)
+*/
+ drm_for_each_encoder_mask(encoder, crtc->dev,
+ crtc->state->encoder_mask) {
struct dpu_encoder_kickoff_params params = { 0 };
-
- if (encoder->crtc != crtc)
- continue;
-
- /*
-* Encoder will flush/start now, unless it has a tx
pending.
-* If so, it may delay and flush at an irq event (e.g.
ppdone)
-*/
dpu_encoder_prepare_for_kickoff(encoder, );
}
@@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
*crtc)
dpu_vbif_clear_errors(dpu_kms);
- list_for_each_entry(encoder, >mode_config.encoder_list, head)
{
- if (encoder->crtc != crtc)
- continue;
-
+ drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
dpu_encoder_kickoff(encoder);
- }
We wont be holding the modeset locks here (and in crtc_atomic_begin) in
the display thread. Is
it safe to iterate over encoder_mask?
end:
reinit_completion(_crtc->frame_done_comp);
@@ -883,11 +871,8 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
dpu_core_perf_crtc_update(crtc, 0, true);
- drm_for_each_encoder(encoder, crtc->dev) {
- if (encoder->crtc != crtc)
- continue;
+ drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
dpu_encoder_register_frame_event_callback(encoder, NULL,
NULL);
- }
memset(cstate->mixers, 0, sizeof(cstate->mixers));
cstate->num_mixers = 0;
@@ -922,12 +907,9 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
dpu_crtc = to_dpu_crtc(crtc);
- drm_for_each_encoder(encoder, crtc->dev) {
- if (encoder->crtc != crtc)
- continue;
+ drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
dpu_encoder_register_frame_event_callback(encoder,
dpu_crtc_frame_event_cb, (void *)crtc);
- }
mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1bec4540f3e1..64134d619748 100644
---
