On Fri, Mar 20, 2015 at 05:04:31PM -0700, Chandra Konduru wrote:
> This function is called from commit path of a plane or crtc.
> It programs scaler registers to detach (aka. unbinds) scaler
> from requested plane or crtc if it isn't in use. It also resets
> scaler_id in crtc/plane state.
>
> v2:
> -improved a log message (me)
>
> Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 2 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 976bfb1..7150c33 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2836,6 +2836,45 @@ u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> }
> }
>
> +/*
> + * This function detaches (aka. unbinds) a scaler from plane or crtc
You might want to clarify that detach/unbind refers to the actual
hardware programming, not the state calculation. I'm a bit surprised we
need this function; I figured we'd just be looping over all scalers at
the end of the commit step and progamming them to either on or off
depending on what the scaling state contained.
As I mentioned on a previous patch, these overloaded functions that
might operate on a plane or might operate on a CRTC can be a bit
confusing, especially when we have multi-nested ternary operators like
you do below.
> + * if scaler is not in use.
> + * It resets scaler_id in plane or crtc
> + * To request detach a scaler from crtc, call plane as NULL
> + */
> +void skl_detach_scaler(struct drm_crtc *crtc, struct drm_plane *plane)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc_state *crtc_state;
> + struct intel_crtc *intel_crtc;
> + struct intel_plane *intel_plane;
> + struct intel_plane_state *plane_state;
> + int *scaler_id;
> +
> + intel_crtc = to_intel_crtc(crtc);
> + intel_plane = plane ? to_intel_plane(plane) : NULL;
> + crtc_state = intel_crtc->config;
> + plane_state = plane ? to_intel_plane_state(plane->state) : NULL;
> +
> + scaler_id = plane ? (plane_state ? &plane_state->scaler_id : NULL) :
> + &crtc_state->scaler_state.scaler_id;
> +
> + if (!scaler_id || (scaler_id && *scaler_id < 0))
> + return;
> +
> + /* if scaler is not in use, free */
> + if (!crtc_state->scaler_state.scalers[*scaler_id].in_use) {
> + I915_WRITE(SKL_PS_CTRL(intel_crtc->pipe, (*scaler_id)), 0);
> + I915_WRITE(SKL_PS_WIN_POS(intel_crtc->pipe, (*scaler_id)), 0);
> + I915_WRITE(SKL_PS_WIN_SZ(intel_crtc->pipe, (*scaler_id)), 0);
> + DRM_DEBUG_KMS("Detached and disabled scaler id %u.%u from %s:%d\n",
> + intel_crtc->pipe, *scaler_id, plane ? "PLANE" : "CRTC",
> + plane ? plane->base.id : crtc->base.id);
> + *scaler_id = -1;
This confuses me...why are we updating the state here at the end of the
commit step? State should be immutable at this point, right?
Matt
> + }
> +}
> +
> static void skylake_update_primary_plane(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> int x, int y)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a9d787d..f25d14d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1141,6 +1141,7 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);
> int skl_update_scaler_users(struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state, struct intel_plane *intel_plane,
> struct intel_plane_state *plane_state, int force_detach);
> +void skl_detach_scaler(struct drm_crtc *crtc, struct drm_plane *plane);
>
> /* intel_dp.c */
> void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
> --
> 1.7.9.5
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
