[PATCH v2] drm/i915/display: Don't update DBUF_TRACKER_STATE_SERVICE

[Matt Roper]

On Tue, Jan 07, 2025 at 11:27:03PM +0530, Ravi Kumar Vodapalli wrote:
> DBUF_TRACKER_STATE_SERVICE is set only during initialization.
> We see that it was done for TGL (display version 12) and DG2,
> because the field didn't actually have 0x8 as default value,
> so the driver had to take care of it.
> 
> According to the BSpec.
> The most compeling reason why we should not program
> that field for other display versions that is not part of the
> official programming sequence in BSpec, and doing so could affect
> future platforms if the default is changed for some reason.
> 
> So we need to check for
> 1. display version 12
> 2. DG2.
> Other platforms unless stated should use their default value

At least to me, this commit message still sounds confusing.  I think
this could be written much more clearly as something along the lines of:

   The bspec only asks the driver to reprogram the DBUF_CTL's "tracker
   state service" field on DG2 and platforms with display version 12;
   All other platforms should avoid reprogramming this register at
   driver init.  Update and consolidate the code conditions so that
   these platforms are properly matched.

   Although we've been accidentally reprogramming DBUF_CTL on platforms
   where the spec does not ask us to, that mistake has been harmless so
   far because the value being programmed by the driver happened to
   match the hardware's default settings.


Matt

> 
> Bspec: 49213
> Signed-off-by: Ravi Kumar Vodapalli <ravi.kumar.vodapalli at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 34465d56def0..9c20459053fe 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1126,9 +1126,6 @@ static void gen12_dbuf_slices_config(struct intel_display *display)
>  {
>  	enum dbuf_slice slice;
>  
> -	if (display->platform.alderlake_p)
> -		return;
> -
>  	for_each_dbuf_slice(display, slice)
>  		intel_de_rmw(display, DBUF_CTL_S(slice),
>  			     DBUF_TRACKER_STATE_SERVICE_MASK,
> @@ -1681,7 +1678,7 @@ static void icl_display_core_init(struct intel_display *display,
>  	/* 4. Enable CDCLK. */
>  	intel_cdclk_init_hw(display);
>  
> -	if (DISPLAY_VER(display) >= 12)
> +	if (DISPLAY_VER(display) == 12 || display->platform.dg2)
>  		gen12_dbuf_slices_config(display);
>  
>  	/* 5. Enable DBUF. */
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation