Re: [Intel-gfx] [PATCH] drm/i915/display/tgl: Implement Wa_14013120569

["Souza, Jose" <jose.souza@intel.com>]

On Mon, 2021-06-28 at 16:50 -0700, Madhumitha Tolakanahalli Pradeep wrote:
> PCH display HPD IRQ is not detected with default filter value.
> So, PP_CONTROL is manually reprogrammed.
> 
> Signed-off-by: Madhumitha Tolakanahalli Pradeep <madhumitha.tolakanahalli.pradeep@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_display_power.c   |  8 ++++++++
>  drivers/gpu/drm/i915/display/intel_hotplug.c     | 16 ++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 285380079aab..e44323cc76f5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -6385,8 +6385,16 @@ static void intel_power_domains_verify_state(struct drm_i915_private *i915)
>  
>  void intel_display_power_suspend_late(struct drm_i915_private *i915)
>  {
> +    struct drm_i915_private *dev_priv = i915;
> +    u32 val;
>  	if (DISPLAY_VER(i915) >= 11 || IS_GEMINILAKE(i915) ||
>  	    IS_BROXTON(i915)) {
> +		val = intel_de_read(dev_priv, PP_CONTROL(0));
> +		/* Wa_14013120569:tgl */
> +		if (IS_TIGERLAKE(i915)) {
> +			val &= ~PANEL_POWER_ON;
> +			intel_de_write(dev_priv, PP_CONTROL(0), val);
> +	}

Code style is all wrong, please fix it and run "dim checkpatch" to validate it before sending patches.
Also PP_CONTROL(0) don't point to the same register that the workaround is talking about, between generations register address change that might be
the case for this one.

This satisfy the "before going into sleep to allow CS entry" but it do not restore the workaround after waking up from suspend.
Also you could improve the code, you are reading the register even for platforms that don't need the wa, also check intel_de_rmw() it is better suited
to this case.

>  		bxt_enable_dc9(i915);
>  		/* Tweaked Wa_14010685332:icp,jsp,mcc */
>  		if (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) <= PCH_MCC)
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 47c85ac97c87..8e3f84100daf 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -26,6 +26,7 @@
>  #include "i915_drv.h"
>  #include "intel_display_types.h"
>  #include "intel_hotplug.h"
> +#include "intel_de.h"
>  
>  /**
>   * DOC: Hotplug
> @@ -266,7 +267,9 @@ intel_encoder_hotplug(struct intel_encoder *encoder,
>  		      struct intel_connector *connector)
>  {
>  	struct drm_device *dev = connector->base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
>  	enum drm_connector_status old_status;
> +	u32 val;
>  	u64 old_epoch_counter;
>  	bool ret = false;
>  
> @@ -288,6 +291,19 @@ intel_encoder_hotplug(struct intel_encoder *encoder,
>  			      drm_get_connector_status_name(connector->base.status),
>  			      old_epoch_counter,
>  			      connector->base.epoch_counter);
> +
> +		/* Wa_14013120569:tgl */
> +		if (IS_TIGERLAKE(dev_priv)) {
> +			val = intel_de_read(dev_priv, PP_CONTROL(0));
> +			if (connector->base.status == connector_status_connected) {
> +				val |= PANEL_POWER_ON;
> +				intel_de_write(dev_priv, PP_CONTROL(0), val);
> +			}
> +			else if (connector->base.status == connector_status_disconnected) {
> +				val &= ~PANEL_POWER_ON;
> +				intel_de_write(dev_priv, PP_CONTROL(0), val);
> +			}
> +		}

Not sure if this is the best place but anyways it is missing handle the case were tigerlake boots with the external display connected.
No hotplug will happen and workaround will never be enabled.

>  		return INTEL_HOTPLUG_CHANGED;
>  	}
>  	return INTEL_HOTPLUG_UNCHANGED;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx