[Intel-gfx] [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume.

[Jani Nikula]

On Thu, 04 Feb 2016, Matt Roper <matthew.d.roper at intel.com> wrote:
> On Thu, Feb 04, 2016 at 07:17:08AM +0530, Thulasimani, Sivakumar wrote:
>> 
>> 
>> On 2/4/2016 6:19 AM, Matt Roper wrote:
>> >From: Bob Paauwe <bob.j.paauwe at intel.com>
>> >
>> >Broxton has some additional bits in the HOTPLUG_CTL register that
>> >indicate whether the HPD sense lines need to be inverted or not for the
>> >current platform.  The BIOS sets these bits to an appropriate value at
>> >boot time, but the value is lost across suspend/resume.  We need to save
>> >and restore the register so that hotplug and display detect works on
>> >resume.
>> i have a patch that is about to be upstreamed that will read and write these
>> values based on vbt. Shuhangi did the basic testing last week so will
>> ask her to send to mail list today.  i would prefer that patch where
>> we know how and when to set these bits rather than just save
>> & restore.
>
> Sure, sounds good.  Can your patch handle cases where there is no VBT by
> falling back to a save/restore?  Quite often in the embedded world, we
> have very specialized boot firmware that doesn't resemble vbios/gop and
> doesn't have any VBT info.

Please no blind save/restore of registers anywhere.

We can (and in many places do) read the BIOS set values for defaults at
driver load, but preferably should also survive without. If there's
anything you really need enabled, it should be stored in the appropriate
driver data structures, and the registers should be fully initialized
when needed.

Looks like there's still plenty of read-modify-write in
*_hpd_irq_setup(), but the right approach would be to ensure
bxt_hpd_irq_setup() does the right thing for you. Either based on sane
defaults or VBT.


BR,
Jani.


>
>
> Matt
>
>> 
>> regards,
>> Sivakumar
>> >Signed-off-by: Bob Paauwe <bob.j.paauwe at intel.com>
>> >[mattrope: Expand commit message]
>> >Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>> >---
>> >  drivers/gpu/drm/i915/i915_drv.h     | 1 +
>> >  drivers/gpu/drm/i915/i915_suspend.c | 7 +++++++
>> >  2 files changed, 8 insertions(+)
>> >
>> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> >index 77227a3..2278117 100644
>> >--- a/drivers/gpu/drm/i915/i915_drv.h
>> >+++ b/drivers/gpu/drm/i915/i915_drv.h
>> >@@ -1061,6 +1061,7 @@ struct i915_suspend_saved_registers {
>> >  	uint64_t saveFENCE[I915_MAX_NUM_FENCES];
>> >  	u32 savePCH_PORT_HOTPLUG;
>> >  	u16 saveGCDGMBUS;
>> >+	u32 saveHOTPLUG;
>> >  };
>> >  struct vlv_s0ix_state {
>> >diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
>> >index 34e061a..efe1e77 100644
>> >--- a/drivers/gpu/drm/i915/i915_suspend.c
>> >+++ b/drivers/gpu/drm/i915/i915_suspend.c
>> >@@ -59,6 +59,10 @@ static void i915_save_display(struct drm_device *dev)
>> >  	/* save FBC interval */
>> >  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev))
>> >  		dev_priv->regfile.saveFBC_CONTROL = I915_READ(FBC_CONTROL);
>> >+
>> >+	if (IS_BROXTON(dev))
>> >+		dev_priv->regfile.saveHOTPLUG = I915_READ(PCH_PORT_HOTPLUG);
>> >+
>> >  }
>> >  static void i915_restore_display(struct drm_device *dev)
>> >@@ -98,6 +102,9 @@ static void i915_restore_display(struct drm_device *dev)
>> >  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev))
>> >  		I915_WRITE(FBC_CONTROL, dev_priv->regfile.saveFBC_CONTROL);
>> >+	if (IS_BROXTON(dev))
>> >+		I915_WRITE(PCH_PORT_HOTPLUG, dev_priv->regfile.saveHOTPLUG);
>> >+
>> >  	i915_redisable_vga(dev);
>> >  }
>> 

-- 
Jani Nikula, Intel Open Source Technology Center