Re: [Intel-gfx] [PATCH v1] drm/i915: Ensure no conflicts with BIOS when updating Dbuf

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 12, 2020 at 05:36:40PM +0200, Jani Nikula wrote:
> On Wed, 12 Feb 2020, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> > TGL BIOS seems to enable both DBuf slices ocasionally, depending
> > how many displays are connected, while i915 according to BSpec
> > was powering on S1 DBuf slice, until a modeset was done.
> >
> > This was causing a brief flash during the boot as we were
> > disabling slice, previously used by BIOS with that.
> >
> > To prevent this, now we are ensuring tht we are enabling
> > _at least_ one slice, but if there are more, let's not
> > power them off.
> >
> > Fixes: ff2cd8635e41 ("drm/i915: Correctly map DBUF slices to pipes")
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_power.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 53056def5414..b9a9cbad8a03 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -4470,11 +4470,13 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
> >  
> >  static void icl_dbuf_enable(struct drm_i915_private *dev_priv)
> >  {
> > +	skl_ddb_get_hw_state(dev_priv);
> >  	/*
> > -	 * Just power up 1 slice, we will
> > +	 * Just power up at least 1 slice, we will
> >  	 * figure out later which slices we have and what we need.
> >  	 */
> > -	icl_dbuf_slices_update(dev_priv, BIT(DBUF_S1));
> > +	icl_dbuf_slices_update(dev_priv, dev_priv->enabled_dbuf_slices_mask |
> > +			       BIT(DBUF_S1));
> 
> I don't know anything about this, but it seems obvious to me the
> enabling code should not do hardware readout; they should be separated
> from a much higher level.

This is part of display core init, which is more or less half readout
vs. half initialization anyway. So can't think of a better place for it
really.

What I don't like is that it's now only in the icl+ function but not in
the pre-icl stuff. In fact I don't see why we even have this pre-icl vs.
icl split anymore. The same code should work for both. So as a followup
I'd like to see a patch to nuke the redundant code.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx