| Message ID | 20250107175703.1667934-1-ravi.kumar.vodapalli@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] drm/i915/display: Don't update DBUF_TRACKER_STATE_SERVICE | expand |
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@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 >
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. */
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 Bspec: 49213 Signed-off-by: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@intel.com> --- drivers/gpu/drm/i915/display/intel_display_power.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)