[Intel-gfx] [PATCH V2] drm/i915/jsl: Add W/A 1409054076 for JSL

[Jani Nikula]

On Thu, 13 May 2021, Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay at intel.com> wrote:
> When pipe A is disabled and MIPI DSI is enabled on pipe B,
> the AMT KVMR feature will incorrectly see pipe A as enabled.
> Set 0x42080 bit 23=1 before enabling DSI on pipe B and leave
> it set while DSI is enabled on pipe B. No impact to setting
> it all the time.
>
> Changes since V1:
> 	- ./dim checkpatch errors addressed
>
> Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay at intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c | 38 ++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h        |  1 +
>  2 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index ce544e20f35c..e5a6660861e8 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -40,6 +40,8 @@
>  #include "skl_scaler.h"
>  #include "skl_universal_plane.h"
>  
> +static bool gen11_dsi_get_hw_state(struct intel_encoder *encoder,
> +				   enum pipe *pipe);
>  static int header_credits_available(struct drm_i915_private *dev_priv,
>  				    enum transcoder dsi_trans)
>  {
> @@ -1036,9 +1038,26 @@ static void gen11_dsi_enable_transcoder(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
> +	enum pipe pipe;
>  	enum transcoder dsi_trans;
>  	u32 tmp;
>  
> +	/*
> +	 * WA 1409054076:JSL
> +	 * When pipe A is disabled and MIPI DSI is enabled on pipe B,
> +	 * the AMT KVMR feature will incorrectly see pipe A as enabled.
> +	 * Set 0x42080 bit 23=1 before enabling DSI on pipe B and leave
> +	 * it set while DSI is enabled on pipe B
> +	 */
> +	gen11_dsi_get_hw_state(encoder, &pipe);

That function is only for reading the state for taking over hardware
state at probe and hardware/software state verification after modeset.

It reads the state that is being set later in this function, so it's
never going to be correct here! Also, we try not to do stuff based on
the hardware state, but rather the software state.

> +	if (IS_PLATFORM(dev_priv, INTEL_JASPERLAKE) &&
> +	    pipe == PIPE_B &&
> +	    dev_priv->active_pipes != BIT(PIPE_A) &&
> +	    !(intel_de_read(dev_priv, CHICKEN_PAR1_1) &
> +			    IGNORE_KVMR_PIPE_A)) {
> +		intel_de_write(dev_priv, CHICKEN_PAR1_1,
> +			       intel_de_read(dev_priv, CHICKEN_PAR1_1) | IGNORE_KVMR_PIPE_A);
> +	}

As far as I understand the explanation, we can set this regardless of
whether pipe A is disabled or not, and we can just set it based on where
DSI is enabled.

It should probably also be IS_JSL_EHL().

With pipe from new_crtc_state:

	if (IS_JSL_EHL(dev_priv) && pipe == PIPE_B)
        	intel_de_rmw(dev_priv, CHICKEN_PAR1_1, 0, IGNORE_KVMR_PIPE_A);

To disable, with pipe from old_crtc_state:

	if (IS_JSL_EHL(dev_priv) && pipe == PIPE_B)
        	intel_de_rmw(dev_priv, CHICKEN_PAR1_1, IGNORE_KVMR_PIPE_A, 0);

At the right locations.

>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		dsi_trans = dsi_port_to_transcoder(port);
>  		tmp = intel_de_read(dev_priv, PIPECONF(dsi_trans));
> @@ -1245,6 +1264,7 @@ static void gen11_dsi_enable(struct intel_atomic_state *state,
>  
>  	drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
>  
> +
>  	/* step6d: enable dsi transcoder */
>  	gen11_dsi_enable_transcoder(encoder);
>  
> @@ -1260,9 +1280,27 @@ static void gen11_dsi_disable_transcoder(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
> +	enum pipe pipe;
>  	enum transcoder dsi_trans;
>  	u32 tmp;
>  
> +	/*
> +	 * WA 1409054076:JSL
> +	 * When pipe A is disabled and MIPI DSI is enabled on pipe B,
> +	 * the AMT KVMR feature will incorrectly see pipe A as enabled.
> +	 * Set 0x42080 bit 23=1 before enabling DSI on pipe B and leave
> +	 * it set while DSI is enabled on pipe B
> +	 */
> +	gen11_dsi_get_hw_state(encoder, &pipe);
> +	if (IS_PLATFORM(dev_priv, INTEL_JASPERLAKE) &&
> +	    pipe == PIPE_B &&
> +	    dev_priv->active_pipes != BIT(PIPE_A) &&
> +	    (intel_de_read(dev_priv, CHICKEN_PAR1_1) &
> +			   IGNORE_KVMR_PIPE_A)) {
> +		intel_de_write(dev_priv, CHICKEN_PAR1_1,
> +			       intel_de_read(dev_priv, CHICKEN_PAR1_1) &
> +						!IGNORE_KVMR_PIPE_A);
> +	}
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		dsi_trans = dsi_port_to_transcoder(port);
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 871d839dfcb8..8b67cd14ff7e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8039,6 +8039,7 @@ enum {
>  # define CHICKEN3_DGMG_DONE_FIX_DISABLE		(1 << 2)
>  
>  #define CHICKEN_PAR1_1			_MMIO(0x42080)
> +#define  IGNORE_KVMR_PIPE_A		BIT(23)

REG_BIT(), not BIT(). Please read the big comment near the top of the
file. Please observe the REG_BIT() on the very next line.

>  #define  KBL_ARB_FILL_SPARE_22		REG_BIT(22)
>  #define  DIS_RAM_BYPASS_PSR2_MAN_TRACK	(1 << 16)
>  #define  SKL_DE_COMPRESSED_HASH_MODE	(1 << 15)

-- 
Jani Nikula, Intel Open Source Graphics Center