Re: [Intel-gfx] [PATCH 05/76] drm/i915: prepare load-detect pipe code for dpms changes

[Jesse Barnes]

On Thu, 26 Jul 2012 20:48:30 +0200
Daniel Vetter  wrote:

> A few things need adjustement:
> - Change the dpms state by calling the dpms connector function and
>   not some crtc helper internal callbacks. Otherwise this will break
>   once we switch to our own dpms handling.
> - Instead of tracking and restoring intel_crtc->dpms_mode use the
>   connector's dpms variable - the former relies on the dpms compuation
>   rules used by the crtc helper. And it would break when the encoder
>   is cloned and the other output has a different dpms state. But
> luckily no one is crazy enough for that.
> - Properly clear the connector -> encoder -> crtc linking, even when
>   failing (note that the crtc helper removes the encoder -> crtc link
>   in disabled_unused_functions for us).

Can this be split into 2 patches?  One for the switch to using the
connector helper, and the other to clean up the fail logic and change
to using the connector dpms state?

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

[Intel-gfx] [PATCH 05/76] drm/i915: prepare load-detect pipe code for dpms changes

[Daniel Vetter]

A few things need adjustement:
- Change the dpms state by calling the dpms connector function and
  not some crtc helper internal callbacks. Otherwise this will break
  once we switch to our own dpms handling.
- Instead of tracking and restoring intel_crtc->dpms_mode use the
  connector's dpms variable - the former relies on the dpms compuation
  rules used by the crtc helper. And it would break when the encoder
  is cloned and the other output has a different dpms state. But luckily
  no one is crazy enough for that.
- Properly clear the connector -> encoder -> crtc linking, even when
  failing (note that the crtc helper removes the encoder -> crtc link
  in disabled_unused_functions for us).

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c |   38 +-
 1 file changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 1504c36..7224055 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5625,21 +5625,12 @@ bool intel_get_load_detect_pipe(struct intel_encoder 
*intel_encoder,
if (encoder->crtc) {
crtc = encoder->crtc;
 
-   intel_crtc = to_intel_crtc(crtc);
-   old->dpms_mode = intel_crtc->dpms_mode;
+   old->dpms_mode = connector->dpms;
old->load_detect_temp = false;
 
/* Make sure the crtc and connector are running */
-   if (intel_crtc->dpms_mode != DRM_MODE_DPMS_ON) {
-   struct drm_encoder_helper_funcs *encoder_funcs;
-   struct drm_crtc_helper_funcs *crtc_funcs;
-
-   crtc_funcs = crtc->helper_private;
-   crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON);
-
-   encoder_funcs = encoder->helper_private;
-   encoder_funcs->dpms(encoder, DRM_MODE_DPMS_ON);
-   }
+   if (connector->dpms != DRM_MODE_DPMS_ON)
+   connector->funcs->dpms(connector, DRM_MODE_DPMS_ON);
 
return true;
}
@@ -5667,7 +5658,7 @@ bool intel_get_load_detect_pipe(struct intel_encoder 
*intel_encoder,
connector->encoder = encoder;
 
intel_crtc = to_intel_crtc(crtc);
-   old->dpms_mode = intel_crtc->dpms_mode;
+   old->dpms_mode = connector->dpms;
old->load_detect_temp = true;
old->release_fb = NULL;
 
@@ -5692,22 +5683,25 @@ bool intel_get_load_detect_pipe(struct intel_encoder 
*intel_encoder,
DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
if (IS_ERR(crtc->fb)) {
DRM_DEBUG_KMS("failed to allocate framebuffer for 
load-detection\n");
-   crtc->fb = old_fb;
-   return false;
+   goto fail;
}
 
if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, old_fb)) {
DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
if (old->release_fb)
old->release_fb->funcs->destroy(old->release_fb);
-   crtc->fb = old_fb;
-   return false;
+   goto fail;
}
 
/* let the connector get through one full cycle before testing */
intel_wait_for_vblank(dev, intel_crtc->pipe);
 
return true;
+fail:
+   connector->encoder = NULL;
+   encoder->crtc = NULL;
+   crtc->fb = old_fb;
+   return false;
 }
 
 void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder,
@@ -5716,9 +5710,6 @@ void intel_release_load_detect_pipe(struct intel_encoder 
*intel_encoder,
 {
struct drm_encoder *encoder = &intel_encoder->base;
struct drm_device *dev = encoder->dev;
-   struct drm_crtc *crtc = encoder->crtc;
-   struct drm_encoder_helper_funcs *encoder_funcs = 
encoder->helper_private;
-   struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
 
DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
  connector->base.id, drm_get_connector_name(connector),
@@ -5726,6 +5717,7 @@ void intel_release_load_detect_pipe(struct intel_encoder 
*intel_encoder,
 
if (old->load_detect_temp) {
connector->encoder = NULL;
+   encoder->crtc = NULL;
drm_helper_disable_unused_functions(dev);
 
if (old->release_fb)
@@ -5735,10 +5727,8 @@ void intel_release_load_detect_pipe(struct intel_encoder 
*intel_encoder,
}
 
/* Switch crtc and encoder back off if necessary */
-   if (old->dpms_mode != DRM_MODE_DPMS_ON) {
-   encoder_funcs->dpms(encoder, old->dpms_mode);
-   crtc_funcs->dpms(crtc, old->dpms_mode);
-   }
+   if (old->dpms_mode != DRM_MODE_DPMS_ON)
+   connector->funcs->dpms(connector, old->dpms_mode);
 }
 
 /* Returns the clock of the currently programmed mode