Re: [Intel-gfx] [PATCH 08/43] drm/i915/hdmi: convert to encoder-disable/enable

2012-07-26 Thread Paulo Zanoni
Hi

2012/7/3 Daniel Vetter daniel.vet...@ffwll.ch:
 diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
 b/drivers/gpu/drm/i915/intel_ddi.c
 index f33fe1a..b71303c 100644
 --- a/drivers/gpu/drm/i915/intel_ddi.c
 +++ b/drivers/gpu/drm/i915/intel_ddi.c
 @@ -752,3 +747,18 @@ void intel_ddi_dpms(struct drm_encoder *encoder, int 
 mode)
 I915_WRITE(DDI_BUF_CTL(port),
 temp);
  }
 +
 +void intel_disable_ddi(struct intel_encoder *encoder)
 +{
 +   struct drm_device *dev = encoder-base.dev;
 +   struct drm_i915_private *dev_priv = dev-dev_private;
 +   struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder-base);
 +   int port = intel_hdmi-ddi_port;
 +   u32 temp;
 +
 +   temp = I915_READ(DDI_BUF_CTL(port));
 +   temp = ~DDI_BUF_CTL_ENABLE;
 +
 +   I915_WRITE(DDI_BUF_CTL(port),
 +   temp);

bikeshed
Since you're creating 2 new functions you might want to move temp to
the upper line in both intel_disable_did and intel_enable_ddi.

Every time I look at this I think about fixing it. And now there are
two. Maybe it's just my obsessive compulsive disorder... :)
/bikeshed

-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 08/43] drm/i915/hdmi: convert to encoder-disable/enable

2012-07-03 Thread Daniel Vetter
I've picked hdmi as the first encoder to convert because it's rather
simple:
- no cloning possible
- no differences between prepare/commit and dpms off/on switching.

A few changes are required to do so:
- Split up the dpms code into an enable/disable function and wire it
  up with the intel encoder.
- Noop out the existing encoder prepare/commit functions used by the
  crtc helper - our crtc enable/disable code now calls back into the
  encoder enable/disable code at the right spot.
- Create new helper functions to handle dpms changes.
- Add intel_encoder-connectors_active to better track dpms state. Atm
  this is unused, but it will be useful to correctly disable the
  entire display pipe for cloned configurations. Also note that for
  now this is only useful in the dpms code - thanks to the crtc
  helper's dpms confusion across a modeset operation we can't (yet)
  rely on this having a sensible value in all circumstances.
- Rip out the encoder helper dpms callback, if this is still getting
  called somewhere we have a bug. The slight issue with that is that
  the crtc helper abuses dpms off to disable unused functions. Hence
  we also need to implement a default encoder disable function to do
  just that with the new encoder-disable callback.
- Note that we drop the cpt modeset verification in the commit
  callback, too. The right place to do this would be in the crtc's
  enable function, _after_ all the encoders are set up. But because
  not all encoders are converted yet, we can't do that. Hence disable
  this check temporarily as a minor concession to bisectability.

v2: Squash the dpms mode to only the supported values -
connector-dpms is for internal tracking only, we can hence avoid
needless state-changes a bit whithout causing harm.

Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_ddi.c |   28 +---
 drivers/gpu/drm/i915/intel_display.c |   49 +
 drivers/gpu/drm/i915/intel_drv.h |8 ++-
 drivers/gpu/drm/i915/intel_hdmi.c|  126 +++---
 4 files changed, 160 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f33fe1a..b71303c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -729,21 +729,16 @@ void intel_ddi_mode_set(struct drm_encoder *encoder,
intel_hdmi-set_infoframes(encoder, adjusted_mode);
 }
 
-void intel_ddi_dpms(struct drm_encoder *encoder, int mode)
+void intel_enable_ddi(struct intel_encoder *encoder)
 {
-   struct drm_device *dev = encoder-dev;
+   struct drm_device *dev = encoder-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
-   struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+   struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder-base);
int port = intel_hdmi-ddi_port;
u32 temp;
 
temp = I915_READ(DDI_BUF_CTL(port));
-
-   if (mode != DRM_MODE_DPMS_ON) {
-   temp = ~DDI_BUF_CTL_ENABLE;
-   } else {
-   temp |= DDI_BUF_CTL_ENABLE;
-   }
+   temp |= DDI_BUF_CTL_ENABLE;
 
/* Enable DDI_BUF_CTL. In HDMI/DVI mode, the port width,
 * and swing/emphasis values are ignored so nothing special needs
@@ -752,3 +747,18 @@ void intel_ddi_dpms(struct drm_encoder *encoder, int mode)
I915_WRITE(DDI_BUF_CTL(port),
temp);
 }
+
+void intel_disable_ddi(struct intel_encoder *encoder)
+{
+   struct drm_device *dev = encoder-base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder-base);
+   int port = intel_hdmi-ddi_port;
+   u32 temp;
+
+   temp = I915_READ(DDI_BUF_CTL(port));
+   temp = ~DDI_BUF_CTL_ENABLE;
+
+   I915_WRITE(DDI_BUF_CTL(port),
+   temp);
+}
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 562ad86..62acf98 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3511,6 +3511,17 @@ void intel_encoder_commit(struct drm_encoder *encoder)
intel_cpt_verify_modeset(dev, intel_crtc-pipe);
 }
 
+void intel_encoder_noop(struct drm_encoder *encoder)
+{
+}
+
+void intel_encoder_disable(struct drm_encoder *encoder)
+{
+   struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
+
+   intel_encoder-disable(intel_encoder);
+}
+
 void intel_encoder_destroy(struct drm_encoder *encoder)
 {
struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
@@ -3519,6 +3530,44 @@ void intel_encoder_destroy(struct drm_encoder *encoder)
kfree(intel_encoder);
 }
 
+/* Simple dpms helper for encodres with just one connector, no cloning and only
+ * one kind of off state. It clamps all !ON modes to fully OFF and changes the
+ * state of the entire output pipe. */
+void