On Tue, 29 Jun 2021, "Souza, Jose" <jose.so...@intel.com> wrote:
> 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.prad...@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.

In general, I've put a bunch of effort into moving most PPS stuff and
PP_CONTROL reg access into intel_pps.c, not least because you must hold
appropriate locks and power domain references to poke at this. You can't
just mess with it nilly willy. I don't want these abstractions bypassed.

BR,
Jani.

>
> 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

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to