| Message ID | 1404491916-5030-5-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 04, 2014 at 01:38:36PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > We may reach this point while the machine is still runtime suspended, > so we'll hit a WARN. The other encoders also don't touch registers at > this point, so instead of waking the machine up, write some code to > keep the register always at the same state, including after we runtime > suspend/resume. > > Testcase: igt/pm_rpm > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80463 > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> I would store lvds_bpp directly in the encoder structure, have the logic around A3 power in the _init() function and don't bother re-writing the A3,B3 control. But anyway, this looks correct to me. Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
On Fri, Jul 04, 2014 at 01:38:36PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > We may reach this point while the machine is still runtime suspended, > so we'll hit a WARN. The other encoders also don't touch registers at > this point, so instead of waking the machine up, write some code to > keep the register always at the same state, including after we runtime > suspend/resume. Excellent catch - we really don't want to touch the hw at all in the compute stage: Once we have atomic modeset we run this code for the test-only mode when userspace tries to figure out what's possible. Waking up the hardware here is really not what we want to do. > Testcase: igt/pm_rpm > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80463 > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Entire series pulled into dinq, thanks. -Daniel > --- > drivers/gpu/drm/i915/intel_lvds.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 8aa7973..c511287 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -51,6 +51,7 @@ struct intel_lvds_encoder { > > bool is_dual_link; > u32 reg; > + u32 a3_power; > > struct intel_lvds_connector *attached_connector; > }; > @@ -170,8 +171,11 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder) > > /* It would be nice to set 24 vs 18-bit mode (LVDS_A3_POWER_UP) > * appropriately here, but we need to look more thoroughly into how > - * panels behave in the two modes. > + * panels behave in the two modes. For now, let's just maintain the > + * value we got from the BIOS. > */ > + temp &= ~LVDS_A3_POWER_MASK; > + temp |= lvds_encoder->a3_power; > > /* Set the dithering flag on LVDS as needed, note that there is no > * special lvds dither control bit on pch-split platforms, dithering is > @@ -269,7 +273,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder, > struct intel_crtc_config *pipe_config) > { > struct drm_device *dev = intel_encoder->base.dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_lvds_encoder *lvds_encoder = > to_lvds_encoder(&intel_encoder->base); > struct intel_connector *intel_connector = > @@ -284,8 +287,7 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder, > return false; > } > > - if ((I915_READ(lvds_encoder->reg) & LVDS_A3_POWER_MASK) == > - LVDS_A3_POWER_UP) > + if (lvds_encoder->a3_power == LVDS_A3_POWER_UP) > lvds_bpp = 8*3; > else > lvds_bpp = 6*3; > @@ -1086,6 +1088,9 @@ out: > DRM_DEBUG_KMS("detected %s-link lvds configuration\n", > lvds_encoder->is_dual_link ? "dual" : "single"); > > + lvds_encoder->a3_power = I915_READ(lvds_encoder->reg) & > + LVDS_A3_POWER_MASK; > + > /* > * Unlock registers and just > * leave them unlocked > -- > 2.0.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 8aa7973..c511287 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -51,6 +51,7 @@ struct intel_lvds_encoder { bool is_dual_link; u32 reg; + u32 a3_power; struct intel_lvds_connector *attached_connector; }; @@ -170,8 +171,11 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder) /* It would be nice to set 24 vs 18-bit mode (LVDS_A3_POWER_UP) * appropriately here, but we need to look more thoroughly into how - * panels behave in the two modes. + * panels behave in the two modes. For now, let's just maintain the + * value we got from the BIOS. */ + temp &= ~LVDS_A3_POWER_MASK; + temp |= lvds_encoder->a3_power; /* Set the dithering flag on LVDS as needed, note that there is no * special lvds dither control bit on pch-split platforms, dithering is @@ -269,7 +273,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder, struct intel_crtc_config *pipe_config) { struct drm_device *dev = intel_encoder->base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&intel_encoder->base); struct intel_connector *intel_connector = @@ -284,8 +287,7 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder, return false; } - if ((I915_READ(lvds_encoder->reg) & LVDS_A3_POWER_MASK) == - LVDS_A3_POWER_UP) + if (lvds_encoder->a3_power == LVDS_A3_POWER_UP) lvds_bpp = 8*3; else lvds_bpp = 6*3; @@ -1086,6 +1088,9 @@ out: DRM_DEBUG_KMS("detected %s-link lvds configuration\n", lvds_encoder->is_dual_link ? "dual" : "single"); + lvds_encoder->a3_power = I915_READ(lvds_encoder->reg) & + LVDS_A3_POWER_MASK; + /* * Unlock registers and just * leave them unlocked