On Sun, 19 Aug 2012 21:12:20 +0200
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> Just prep work, not yet put to some use.
>
> Note that because we're still using the crtc helper to switch modes
> (and their complicated way to do partial modesets), we need to call
> the encoder's disable function unconditionally.
>
> But once this is cleaned up we shouldn't call the encoder's disable
> function unconditionally any more, because then we know that we'll
> only call it if the encoder is actually enabled. Also note that we
> then need to be careful about which crtc we're filtering the encoder
> list on: We want to filter on the crtc of the _current_ mode, not the
> one we're about to set up.
>
> For the enabling side we need to do the same trick. And again, we
> should be able to simplify this quite a bit when things have settled
> into place.
>
> Also note that this simply does not take cloning into account, so dpms
> needs to be handled specially for the few outputs where we even bother
> with it.
>
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 38
> ++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_drv.h | 2 ++ 2 files changed, 38
> insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 04bec4b..82aaded 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3209,13 +3209,16 @@ static void ironlake_crtc_enable(struct
> drm_crtc *crtc) struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
> u32 temp;
> bool is_pch_port;
>
> + /* XXX: For compatability with the crtc helper code, call
> the encoder's
> + * enable function unconditionally for now. */
> if (intel_crtc->active)
> - return;
> + goto encoders;
>
> intel_crtc->active = true;
> intel_update_watermarks(dev);
> @@ -3262,6 +3265,12 @@ static void ironlake_crtc_enable(struct
> drm_crtc *crtc) mutex_unlock(&dev->struct_mutex);
>
> intel_crtc_update_cursor(crtc, true);
> +
> +encoders:
> + for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->enable)
> + encoder->enable(encoder);
> + }
> }
>
> static void ironlake_crtc_disable(struct drm_crtc *crtc)
> @@ -3269,10 +3278,18 @@ static void ironlake_crtc_disable(struct
> drm_crtc *crtc) struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
> u32 reg, temp;
>
> + /* XXX: For compatability with the crtc helper code, call
> the encoder's
> + * disable function unconditionally for now. */
> + for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->disable)
> + encoder->disable(encoder);
> + }
> +
> if (!intel_crtc->active)
> return;
>
> @@ -3371,11 +3388,14 @@ static void i9xx_crtc_enable(struct drm_crtc
> *crtc) struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
>
> + /* XXX: For compatability with the crtc helper code, call
> the encoder's
> + * enable function unconditionally for now. */
> if (intel_crtc->active)
> - return;
> + goto encoders;
>
> intel_crtc->active = true;
> intel_update_watermarks(dev);
> @@ -3390,6 +3410,12 @@ static void i9xx_crtc_enable(struct drm_crtc
> *crtc) /* Give the overlay scaler a chance to enable if it's on this
> pipe */ intel_crtc_dpms_overlay(intel_crtc, true);
> intel_crtc_update_cursor(crtc, true);
> +
> +encoders:
> + for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->enable)
> + encoder->enable(encoder);
> + }
> }
>
> static void i9xx_crtc_disable(struct drm_crtc *crtc)
> @@ -3397,9 +3423,17 @@ static void i9xx_crtc_disable(struct drm_crtc
> *crtc) struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
>
> + /* XXX: For compatability with the crtc helper code, call
> the encoder's
> + * disable function unconditionally for now. */
> + for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->disable)
> + encoder->disable(encoder);
> + }
> +
> if (!intel_crtc->active)
> return;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h index 95f635b..9a5adcc 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -141,6 +141,8 @@ struct intel_encoder {
> */
> bool cloneable;
> void (*hot_plug)(struct intel_encoder *);
> + void (*enable)(struct intel_encoder *);
> + void (*disable)(struct intel_encoder *);
> int crtc_mask;
> };
>
Yep, looks good.
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
