Re: [PATCH] drm/i915/display: Add WA_14018221282

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

On Mon, 11 Nov 2024, Nemesa Garg <nemesa.garg@intel.com> wrote:
> It was observed that the first write to DKL PHY DP Mode
> register was not taking effect, hence rewrite this register.
>
> v2: Rename function [Mitul]
>
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> Signed-off-by: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 769bd1f26db2..f955d89951b8 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2104,10 +2104,21 @@ void intel_ddi_sanitize_encoder_pll_mapping(struct intel_encoder *encoder)
>  	encoder->disable_clock(encoder);
>  }
>  
> +static void
> +tgl_is_dp_mode_enabled(struct drm_i915_private *dev_priv,
> +		       enum tc_port tc_port, u32 ln0, u32 ln1)

I went ahead and asked ChatGPT what it thinks of the function name:

If a function's name is tgl_is_dp_mode_enabled(), do you expect it to
return a value, and what would you expect it to return?


It replied:

Yes, based on the function's name, I would expect
`tgl_is_dp_mode_enabled()` to return a value. The naming convention
suggests that it checks whether "DP mode" (perhaps "DisplayPort mode")
is enabled and returns a Boolean value (`true` or `false`).

In this context:

- `tgl_` might be a prefix denoting a specific category or module (e.g.,
  "toggle" or "Tiger Lake" if it's hardware-specific).

- `is_` implies a question, typical of Boolean-returning functions.

- `dp_mode_enabled` likely refers to a state or condition ("DisplayPort
  mode enabled" in this case).

So, I would expect `tgl_is_dp_mode_enabled()` to return `true` if DP
mode is currently enabled, and `false` otherwise.


> +{
> +	if (ln0 != intel_dkl_phy_read(dev_priv, DKL_DP_MODE(tc_port, 0)))
> +		intel_dkl_phy_write(dev_priv, DKL_DP_MODE(tc_port, 0), ln0);
> +	if (ln1 != intel_dkl_phy_read(dev_priv, DKL_DP_MODE(tc_port, 1)))
> +		intel_dkl_phy_write(dev_priv, DKL_DP_MODE(tc_port, 1), ln1);
> +}
> +
>  static void
>  icl_program_mg_dp_mode(struct intel_digital_port *dig_port,
>  		       const struct intel_crtc_state *crtc_state)
>  {
> +	struct intel_display *display = to_intel_display(crtc_state);
>  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>  	enum tc_port tc_port = intel_encoder_to_tc(&dig_port->base);
>  	u32 ln0, ln1, pin_assignment;
> @@ -2185,6 +2196,10 @@ icl_program_mg_dp_mode(struct intel_digital_port *dig_port,
>  	if (DISPLAY_VER(dev_priv) >= 12) {
>  		intel_dkl_phy_write(dev_priv, DKL_DP_MODE(tc_port, 0), ln0);
>  		intel_dkl_phy_write(dev_priv, DKL_DP_MODE(tc_port, 1), ln1);
> +		 /* WA_14018221282 */
> +		if (DISPLAY_VER(display) == 12)
> +			tgl_is_dp_mode_enabled(dev_priv, tc_port, ln0, ln1);
> +
>  	} else {
>  		intel_de_write(dev_priv, MG_DP_MODE(0, tc_port), ln0);
>  		intel_de_write(dev_priv, MG_DP_MODE(1, tc_port), ln1);

-- 
Jani Nikula, Intel