Re: [PATCH 10/14] drm/i915: Populate possible_crtcs correctly

Ville Syrjälä
2018-06-25T11:10:18Z

On Thu, Jun 21, 2018 at 06:26:04PM -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-06-15 at 19:49 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Don't advertize non-exisiting crtcs in the encoder possible_crtcs
> > bitmask.
> >
> How do we end up advertising non-existing CRTCs? encoder->crtc_mask
> seems to be populated in the encoder init functions based on possible
> pipes. Do you mean pipe and crtc->index can potentially differ?

No. Just that we may sometimes set BIT(PIPE_C) in crtc_mask when
there are only two pipes on the device.

>
>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index b095899d68a9..3fa9da714403 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13959,6 +13959,20 @@ static int intel_encoder_clones(struct
> > intel_encoder *encoder)
> >   return index_mask;
> >  }
> >
> > +static int intel_encoder_crtcs(struct intel_encoder *encoder)
> > +{
> > + struct drm_device *dev = encoder->base.dev;
> > + struct intel_crtc *crtc;
> > + int index_mask = 0;
> > +
> > + for_each_intel_crtc(dev, crtc) {
> > + if (encoder->crtc_mask & BIT(crtc->pipe))
> > + index_mask |= drm_crtc_mask(&crtc->base);
> > + }
> > +
> > + return index_mask;
> > +}
> > +
> >  static bool has_edp_a(struct drm_i915_private *dev_priv)
> >  {
> >   if (!IS_MOBILE(dev_priv))
> > @@ -14211,7 +14225,8 @@ static void intel_setup_outputs(struct
> > drm_i915_private *dev_priv)
> >   intel_psr_init(dev_priv);
> >
> >   for_each_intel_encoder(&dev_priv->drm, encoder) {
> > - encoder->base.possible_crtcs = encoder->crtc_mask;
> > + encoder->base.possible_crtcs =
> > + intel_encoder_crtcs(encoder);
> >   encoder->base.possible_clones =
> >   intel_encoder_clones(encoder);
> >   }

--
Ville Syrjälä
Intel