RE: [PATCH v3] drm/i915/display: WA for Re-initialize dispcnlunitt1 xosc clock

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 20 Jun 2024, "Garg, Nemesa" <nemesa.garg@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Mitul
>> Golani
>> Sent: Wednesday, June 19, 2024 4:08 PM
>> To: intel-gfx@lists.freedesktop.org
>> Subject: [PATCH v3] drm/i915/display: WA for Re-initialize dispcnlunitt1 xosc clock
>> 
>> The dispcnlunit1_cp_xosc_clk should be de-asserted in display off and only
>> asserted in display on. But during observation it found clk remains active in display
>> OFF. As workaround, Display driver shall execute set-reset sequence at the end of
>> the Initialize Sequence.
>> 
>> Wa_15013987218
>> 
>> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> Reviewed-by: Nemesa Garg <nemesa.garg@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display_power.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
>> b/drivers/gpu/drm/i915/display/intel_display_power.c
>> index e288a1b21d7e..aef54c1a2ba9 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
>> @@ -1704,6 +1704,14 @@ static void icl_display_core_init(struct
>> drm_i915_private *dev_priv,
>>  	/* Wa_14011503030:xelpd */
>>  	if (DISPLAY_VER(dev_priv) == 13)
>>  		intel_de_write(dev_priv, XELPD_DISPLAY_ERR_FATAL_MASK,
>> ~0);
>> +
>> +	/* Wa_15013987218 */
>> +	if (DISPLAY_VER(dev_priv) == 20) {
>> +		intel_de_write(dev_priv, SOUTH_DSPCLK_GATE_D,
>> +			       PCH_GMBUSUNIT_CLOCK_GATE_DISABLE);
>
> Nit:  we can replace the above statement with this intel_de_rmw(dev_priv, SOUTH_DSPCLK_GATE_D, 0, PCH_GMBUSUNIT_CLOCK_GATE_DISABLE) so that code consistency can be maintained in the code block.
> otherwise LGTM.

It's not just about consistency. Are you even sure you can erase
everything else in the register here?

BR,
Jani.

>
>> +		intel_de_rmw(dev_priv, SOUTH_DSPCLK_GATE_D,
>> +			     PCH_GMBUSUNIT_CLOCK_GATE_DISABLE, 0);
>> +	}
>>  }
>> 
>>  static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
>> --
>> 2.45.2
>

-- 
Jani Nikula, Intel