* [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. @ 2016-02-04 0:49 Matt Roper 2016-02-04 1:23 ` Matt Roper ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Matt Roper @ 2016-02-04 0:49 UTC (permalink / raw) To: intel-gfx From: Bob Paauwe <bob.j.paauwe@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. Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> [mattrope: Expand commit message] Signed-off-by: Matt Roper <matthew.d.roper@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); } -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. 2016-02-04 0:49 [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume Matt Roper @ 2016-02-04 1:23 ` Matt Roper 2016-02-04 1:47 ` Thulasimani, Sivakumar 2016-02-04 8:15 ` ✗ Fi.CI.BAT: warning for " Patchwork 2 siblings, 0 replies; 9+ messages in thread From: Matt Roper @ 2016-02-04 1:23 UTC (permalink / raw) To: intel-gfx On Wed, Feb 03, 2016 at 04:49:07PM -0800, Matt Roper wrote: > From: Bob Paauwe <bob.j.paauwe@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. > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > [mattrope: Expand commit message] > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93915 > --- > 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); > } > > -- > 2.1.4 > -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. 2016-02-04 0:49 [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume Matt Roper 2016-02-04 1:23 ` Matt Roper @ 2016-02-04 1:47 ` Thulasimani, Sivakumar 2016-02-04 1:51 ` Matt Roper 2016-02-04 8:15 ` ✗ Fi.CI.BAT: warning for " Patchwork 2 siblings, 1 reply; 9+ messages in thread From: Thulasimani, Sivakumar @ 2016-02-04 1:47 UTC (permalink / raw) To: Matt Roper, intel-gfx On 2/4/2016 6:19 AM, Matt Roper wrote: > From: Bob Paauwe <bob.j.paauwe@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. regards, Sivakumar > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > [mattrope: Expand commit message] > Signed-off-by: Matt Roper <matthew.d.roper@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); > } > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. 2016-02-04 1:47 ` Thulasimani, Sivakumar @ 2016-02-04 1:51 ` Matt Roper 2016-02-04 1:59 ` Thulasimani, Sivakumar 2016-02-04 8:41 ` Jani Nikula 0 siblings, 2 replies; 9+ messages in thread From: Matt Roper @ 2016-02-04 1:51 UTC (permalink / raw) To: Thulasimani, Sivakumar; +Cc: intel-gfx 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@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. Matt > > regards, > Sivakumar > >Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > >[mattrope: Expand commit message] > >Signed-off-by: Matt Roper <matthew.d.roper@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); > > } > -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. 2016-02-04 1:51 ` Matt Roper @ 2016-02-04 1:59 ` Thulasimani, Sivakumar 2016-02-04 2:22 ` Thulasimani, Sivakumar 2016-02-04 8:41 ` Jani Nikula 1 sibling, 1 reply; 9+ messages in thread From: Thulasimani, Sivakumar @ 2016-02-04 1:59 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx On 2/4/2016 7:21 AM, Matt Roper 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@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. > hmm.. falling back will be required only when older VBTs are used. in which case we assume that invert bit is not needed. if you still need this patch then can you save and restore only the invert bits ? the code below does it for whole register and might enable interrupts too which is not intention of the patch. regards, Sivakumar > Matt > >> regards, >> Sivakumar >>> Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> >>> [mattrope: Expand commit message] >>> Signed-off-by: Matt Roper <matthew.d.roper@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); >>> } _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. 2016-02-04 1:59 ` Thulasimani, Sivakumar @ 2016-02-04 2:22 ` Thulasimani, Sivakumar 0 siblings, 0 replies; 9+ messages in thread From: Thulasimani, Sivakumar @ 2016-02-04 2:22 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx On 2/4/2016 7:29 AM, Thulasimani, Sivakumar wrote: > > > On 2/4/2016 7:21 AM, Matt Roper 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@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. >> > hmm.. falling back will be required only when older VBTs are used. in > which case we > assume that invert bit is not needed. if you still need this patch > then can you save > and restore only the invert bits ? the code below does it for whole > register > and might enable interrupts too which is not intention of the patch. > i am a bit more concerned on the scenario where there is no GOP/VBIOS and so no VBT. can you share how common it is not have VBIOS/GOP ? i am aware of coreboot but not sure if it has VBT or not (will check on this). VBT is currently used in nightly where we have no other options but whenever we dont have to depend on it we have some sort if WA. i feel this is not good since we cannot say if some scenario will work or not if VBT is not present. may be a new thread just to discuss that ? > regards, > Sivakumar > >> Matt >> >>> regards, >>> Sivakumar >>>> Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> >>>> [mattrope: Expand commit message] >>>> Signed-off-by: Matt Roper <matthew.d.roper@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); >>>> } > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. 2016-02-04 1:51 ` Matt Roper 2016-02-04 1:59 ` Thulasimani, Sivakumar @ 2016-02-04 8:41 ` Jani Nikula 2016-02-15 16:41 ` Daniel Vetter 1 sibling, 1 reply; 9+ messages in thread From: Jani Nikula @ 2016-02-04 8:41 UTC (permalink / raw) To: Matt Roper, Thulasimani, Sivakumar; +Cc: intel-gfx On Thu, 04 Feb 2016, Matt Roper <matthew.d.roper@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@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@intel.com> >> >[mattrope: Expand commit message] >> >Signed-off-by: Matt Roper <matthew.d.roper@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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. 2016-02-04 8:41 ` Jani Nikula @ 2016-02-15 16:41 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2016-02-15 16:41 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Thu, Feb 04, 2016 at 10:41:33AM +0200, Jani Nikula wrote: > On Thu, 04 Feb 2016, Matt Roper <matthew.d.roper@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@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. Fully concurred. The safe/restore stuff for byt/chv/bxt is really not a part I like, and if we already have code to set up hpd bits (hpd_setup et al), then please don't proliferate safe/restore hacks. They're way too fragile, it's pretty much impossible to get ordering right and as soon as they need to interact with other setup functions you're landing in rmw hell. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. 2016-02-04 0:49 [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume Matt Roper 2016-02-04 1:23 ` Matt Roper 2016-02-04 1:47 ` Thulasimani, Sivakumar @ 2016-02-04 8:15 ` Patchwork 2 siblings, 0 replies; 9+ messages in thread From: Patchwork @ 2016-02-04 8:15 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx == Summary == Series 3066v1 drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. http://patchwork.freedesktop.org/api/1.0/series/3066/revisions/1/mbox/ Test kms_flip: Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (bsw-nuc-2) bdw-nuci7 total:156 pass:147 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultra total:159 pass:147 dwarn:0 dfail:0 fail:0 skip:12 bsw-nuc-2 total:159 pass:130 dwarn:1 dfail:0 fail:0 skip:28 byt-nuc total:159 pass:136 dwarn:0 dfail:0 fail:0 skip:23 hsw-brixbox total:159 pass:146 dwarn:0 dfail:0 fail:0 skip:13 hsw-gt2 total:159 pass:149 dwarn:0 dfail:0 fail:0 skip:10 ilk-hp8440p total:159 pass:111 dwarn:0 dfail:0 fail:0 skip:48 ivb-t430s total:159 pass:145 dwarn:0 dfail:0 fail:0 skip:14 skl-i5k-2 total:159 pass:144 dwarn:1 dfail:0 fail:0 skip:14 skl-i7k-2 total:159 pass:144 dwarn:1 dfail:0 fail:0 skip:14 snb-dellxps total:159 pass:137 dwarn:0 dfail:0 fail:0 skip:22 Results at /archive/results/CI_IGT_test/Patchwork_1360/ 1ff67c58aa3d7cacd37451397c740b8df27994e6 drm-intel-nightly: 2016y-02m-03d-18h-22m-38s UTC integration manifest fa2d37e36c3f9819eeacc44e2f3c3ed6ba4d81b0 drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-02-15 16:41 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-04 0:49 [PATCH] drm/i915/bxt: Save/restore HOTPLUG_CTL during suspend/resume Matt Roper 2016-02-04 1:23 ` Matt Roper 2016-02-04 1:47 ` Thulasimani, Sivakumar 2016-02-04 1:51 ` Matt Roper 2016-02-04 1:59 ` Thulasimani, Sivakumar 2016-02-04 2:22 ` Thulasimani, Sivakumar 2016-02-04 8:41 ` Jani Nikula 2016-02-15 16:41 ` Daniel Vetter 2016-02-04 8:15 ` ✗ Fi.CI.BAT: warning for " Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).