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

From: Matt Roper <matthew.d.roper@intel.com>
To: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: jani.nikula@intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v1 1/3] drm/i915: Ensure no conflicts with BIOS when updating Dbuf
Date: Thu, 13 Feb 2020 12:25:33 -0800	[thread overview]
Message-ID: <20200213202533.GH2014153@mdroper-desk1.amr.corp.intel.com> (raw)
In-Reply-To: <20200213140412.32697-2-stanislav.lisovskiy@intel.com>

On Thu, Feb 13, 2020 at 04:04:10PM +0200, Stanislav Lisovskiy 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);

Might want to add a blank line here to make it visually easier to read.

>  	/*
> -	 * Just power up 1 slice, we will
> +	 * Just power up at least 1 slice, we will

You might want to say "at least slice S1" since if the BIOS were to
initialize only S2, that wouldn't be sufficient for us (according to the
bspec).

>  	 * 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));

We could potentially avoid some unnecessary work by checking whether
what we read back above was already good enough or not.

        if (!(dev_priv->enabled_dbuf_slices_mask & BIT(DBUF_S1)))
                icl_dbuf_slices_update(...)

But either way,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

>  }
>  
>  static void icl_dbuf_disable(struct drm_i915_private *dev_priv)
> -- 
> 2.24.1.485.gad05a3d8e5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx