| Message ID | 1418242565-7181-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 10, 2014 at 12:16:05PM -0800, Jesse Barnes wrote: > Should probably just init this in the GMbus code all the time, based on > the cdclk and HPLL like we do on newer platforms. Ville has code for > that in a rework branch, but until then we can fix this bug fairly > easily. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76301 > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> My cdclk extraction code doesn't seem to agree with this register for this particular bug reporter at least. So I think I need to go double check my code. The other options are that GMBUS clock isn't derived from cdclk on that platform, or that the HPLL/cdclk bits in configdb are simply not valid for this particular chipset. In the meantime however, we can at least get some machines working with this patch. I'm not entirely sure which platforms have this register, but IS_GEN4() looks safe enough since my 946GZ has it and the reporter has a G41. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/i915_suspend.c | 8 ++++++++ > 3 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 2725243..f33102b 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -925,6 +925,7 @@ struct i915_suspend_saved_registers { > u32 savePIPEB_LINK_N1; > u32 saveMCHBAR_RENDER_STANDBY; > u32 savePCH_PORT_HOTPLUG; > + u16 saveGCDGMBUS; > }; > > struct vlv_s0ix_state { > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index dc03fac..f7c2856 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -76,6 +76,7 @@ > #define I915_GC_RENDER_CLOCK_166_MHZ (0 << 0) > #define I915_GC_RENDER_CLOCK_200_MHZ (1 << 0) > #define I915_GC_RENDER_CLOCK_333_MHZ (4 << 0) > +#define GCDGMBUS 0xcc > #define PCI_LBPC 0xf4 /* legacy/combination backlight modes, also called LBB */ > > > diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c > index dfe6617..2636882 100644 > --- a/drivers/gpu/drm/i915/i915_suspend.c > +++ b/drivers/gpu/drm/i915/i915_suspend.c > @@ -303,6 +303,10 @@ int i915_save_state(struct drm_device *dev) > } > } > > + if (IS_GEN4(dev)) > + pci_read_config_word(dev->pdev, GCDGMBUS, > + &dev_priv->regfile.saveGCDGMBUS); > + > /* Cache mode state */ > if (INTEL_INFO(dev)->gen < 7) > dev_priv->regfile.saveCACHE_MODE_0 = I915_READ(CACHE_MODE_0); > @@ -331,6 +335,10 @@ int i915_restore_state(struct drm_device *dev) > mutex_lock(&dev->struct_mutex); > > i915_gem_restore_fences(dev); > + > + if (IS_GEN4(dev)) > + pci_write_config_word(dev->pdev, GCDGMBUS, > + dev_priv->regfile.saveGCDGMBUS); > i915_restore_display(dev); > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) { > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 10 Dec 2014 22:35:37 +0200 Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Dec 10, 2014 at 12:16:05PM -0800, Jesse Barnes wrote: > > Should probably just init this in the GMbus code all the time, > > based on the cdclk and HPLL like we do on newer platforms. Ville > > has code for that in a rework branch, but until then we can fix > > this bug fairly easily. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76301 > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > My cdclk extraction code doesn't seem to agree with this register for > this particular bug reporter at least. So I think I need to go double > check my code. The other options are that GMBUS clock isn't derived > from cdclk on that platform, or that the HPLL/cdclk bits in configdb > are simply not valid for this particular chipset. > > In the meantime however, we can at least get some machines working > with this patch. I'm not entirely sure which platforms have this > register, but IS_GEN4() looks safe enough since my 946GZ has it and > the reporter has a G41. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Great, thanks. Jani, can you pick this up? I'll bounce the original over to stable@ too. Thanks, Jesse
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 364/364 364/364
ILK +1 364/366 365/366
SNB 448/450 448/450
IVB 497/498 497/498
BYT 289/289 289/289
HSW 563/564 563/564
BDW 417/417 417/417
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
ILK igt_kms_flip_wf_vblank-ts-check DMESG_WARN(6, M26)PASS(21, M26M37) PASS(1, M37)
Note: You need to pay more attention to line start with '*'
On Wed, 10 Dec 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > On Wed, 10 Dec 2014 22:35:37 +0200 > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > >> On Wed, Dec 10, 2014 at 12:16:05PM -0800, Jesse Barnes wrote: >> > Should probably just init this in the GMbus code all the time, >> > based on the cdclk and HPLL like we do on newer platforms. Ville >> > has code for that in a rework branch, but until then we can fix >> > this bug fairly easily. >> > >> > References: https://bugs.freedesktop.org/show_bug.cgi?id=76301 >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> >> >> My cdclk extraction code doesn't seem to agree with this register for >> this particular bug reporter at least. So I think I need to go double >> check my code. The other options are that GMBUS clock isn't derived >> from cdclk on that platform, or that the HPLL/cdclk bits in configdb >> are simply not valid for this particular chipset. >> >> In the meantime however, we can at least get some machines working >> with this patch. I'm not entirely sure which platforms have this >> register, but IS_GEN4() looks safe enough since my 946GZ has it and >> the reporter has a G41. >> >> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Great, thanks. Jani, can you pick this up? I'll bounce the original > over to stable@ too. Pushed to drm-intel-next-fixes, with cc: stable added. Thanks for the patch and review. BR, Jani > > Thanks, > Jesse > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Dec 10, 2014 at 10:35:37PM +0200, Ville Syrjälä wrote: > On Wed, Dec 10, 2014 at 12:16:05PM -0800, Jesse Barnes wrote: > > Should probably just init this in the GMbus code all the time, based on > > the cdclk and HPLL like we do on newer platforms. Ville has code for > > that in a rework branch, but until then we can fix this bug fairly > > easily. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76301 > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > My cdclk extraction code doesn't seem to agree with this register for > this particular bug reporter at least. So I think I need to go double > check my code. The other options are that GMBUS clock isn't derived from > cdclk on that platform, or that the HPLL/cdclk bits in configdb are > simply not valid for this particular chipset. > > In the meantime however, we can at least get some machines working with > this patch. I'm not entirely sure which platforms have this register, > but IS_GEN4() looks safe enough since my 946GZ has it and the reporter > has a G41. Yeah I really don't like the save/restore_state functions since they just bash registers without any regard to ordering constraints and stuff. I guess your cdclk series will address this? - At least we have the vlv_update_cdclk sprinkled all over already, probably better to call the intel_i2c_update_cdlk and move it to intel_i2c.c, too. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2725243..f33102b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -925,6 +925,7 @@ struct i915_suspend_saved_registers { u32 savePIPEB_LINK_N1; u32 saveMCHBAR_RENDER_STANDBY; u32 savePCH_PORT_HOTPLUG; + u16 saveGCDGMBUS; }; struct vlv_s0ix_state { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index dc03fac..f7c2856 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -76,6 +76,7 @@ #define I915_GC_RENDER_CLOCK_166_MHZ (0 << 0) #define I915_GC_RENDER_CLOCK_200_MHZ (1 << 0) #define I915_GC_RENDER_CLOCK_333_MHZ (4 << 0) +#define GCDGMBUS 0xcc #define PCI_LBPC 0xf4 /* legacy/combination backlight modes, also called LBB */ diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c index dfe6617..2636882 100644 --- a/drivers/gpu/drm/i915/i915_suspend.c +++ b/drivers/gpu/drm/i915/i915_suspend.c @@ -303,6 +303,10 @@ int i915_save_state(struct drm_device *dev) } } + if (IS_GEN4(dev)) + pci_read_config_word(dev->pdev, GCDGMBUS, + &dev_priv->regfile.saveGCDGMBUS); + /* Cache mode state */ if (INTEL_INFO(dev)->gen < 7) dev_priv->regfile.saveCACHE_MODE_0 = I915_READ(CACHE_MODE_0); @@ -331,6 +335,10 @@ int i915_restore_state(struct drm_device *dev) mutex_lock(&dev->struct_mutex); i915_gem_restore_fences(dev); + + if (IS_GEN4(dev)) + pci_write_config_word(dev->pdev, GCDGMBUS, + dev_priv->regfile.saveGCDGMBUS); i915_restore_display(dev); if (!drm_core_check_feature(dev, DRIVER_MODESET)) {
Should probably just init this in the GMbus code all the time, based on the cdclk and HPLL like we do on newer platforms. Ville has code for that in a rework branch, but until then we can fix this bug fairly easily. References: https://bugs.freedesktop.org/show_bug.cgi?id=76301 Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/i915_suspend.c | 8 ++++++++ 3 files changed, 10 insertions(+)