Re: [PATCH 19/20] drm/i915: Enable skylake primary plane scaling using shared scalers

Matt Roper
2015-04-02T23:05:22Z

On Wed, Apr 01, 2015 at 07:59:48PM -0700, Chandra Konduru wrote:
> This patch enables skylake primary plane display scaling using shared
> scalers atomic desgin.
>
> v2:
> -use single copy of scaler limits (Matt)
>
> v3:
> -move detach_scalers to crtc commit path (Matt)
> -use values in plane_state->src as regular integers (me)
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 5 ++-
> drivers/gpu/drm/i915/intel_display.c | 72 +++++++++++++++++++++++++++++++---
> 2 files changed, 71 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 7b8efd4..f14c60e 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -167,7 +167,7 @@ int intel_atomic_commit(struct drm_device *dev,
> plane->state->state = NULL;
> }
>
> - /* swap crtc_state */
> + /* swap crtc_scaler_state */
> for (i = 0; i < dev->mode_config.num_crtc; i++) {
> struct drm_crtc *crtc = state->crtcs[i];
> if (!crtc) {
> @@ -176,6 +176,9 @@ int intel_atomic_commit(struct drm_device *dev,
>
> to_intel_crtc(crtc)->config->scaler_state =
> to_intel_crtc_state(state->crtc_states[i])->scaler_state;
> +
> + if (INTEL_INFO(dev)->gen >= 9)
> + skl_detach_scalers(to_intel_crtc(crtc));
> }
>
> drm_atomic_helper_commit_planes(dev, state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 664b7fb..4e9d9f6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> int pipe = intel_crtc->pipe;
> u32 plane_ctl, stride_div;
> unsigned long surf_addr;
> + struct intel_crtc_state *crtc_state = intel_crtc->config;
> + struct intel_plane_state *plane_state;
> + int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
> + int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
> + int scaler_id = -1;
> +
> + plane_state = crtc->primary ?
> + to_intel_plane_state(crtc->primary->state) : NULL;
>
> if (!intel_crtc->primary_enabled) {
> I915_WRITE(PLANE_CTL(pipe, 0), 0);
> @@ -3046,12 +3054,41 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> fb->pixel_format);
> surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj);
>
> + if (plane_state) {
> + scaler_id = plane_state->scaler_id;
> + src_x = plane_state->src.x1;
> + src_y = plane_state->src.y1;
> + src_w = drm_rect_width(&plane_state->src);
> + src_h = drm_rect_height(&plane_state->src);
> + dst_x = plane_state->dst.x1;
> + dst_y = plane_state->dst.y1;
> + dst_w = drm_rect_width(&plane_state->dst);
> + dst_h = drm_rect_height(&plane_state->dst);
> + }
> +
> I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> I915_WRITE(PLANE_POS(pipe, 0), 0);
> +
> + if (src_w && src_h && dst_w && dst_h && scaler_id >= 0) {
> + uint32_t ps_ctrl = 0;
> +
> + WARN_ON(x != src_x || y != src_y);
> + ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) |
> + crtc_state->scaler_state.scalers[scaler_id].mode |
> + crtc_state->scaler_state.scalers[scaler_id].filter;
> + I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
> + I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> + I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (dst_x << 16) | dst_y);
> + I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | dst_h);
> +
> + I915_WRITE(PLANE_SIZE(pipe, 0), ((src_h - 1) << 16) | (src_w - 1));
> + } else {
> + I915_WRITE(PLANE_SIZE(pipe, 0),
> + (intel_crtc->config->pipe_src_h - 1) << 16 |
> + (intel_crtc->config->pipe_src_w - 1));
> + }
> +
> I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
> - I915_WRITE(PLANE_SIZE(pipe, 0),
> - (intel_crtc->config->pipe_src_h - 1) << 16 |
> - (intel_crtc->config->pipe_src_w - 1));
> I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
> I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
>
> @@ -12821,19 +12858,33 @@ intel_check_primary_plane(struct drm_plane *plane,
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *crtc = state->base.crtc;
> struct intel_crtc *intel_crtc;
> + struct intel_crtc_state *crtc_state;
> struct drm_framebuffer *fb = state->base.fb;
> struct drm_rect *dest = &state->dst;
> struct drm_rect *src = &state->src;
> const struct drm_rect *clip = &state->clip;
> + struct intel_crtc_scaler_state *scaler_state;
> + int max_scale = DRM_PLANE_HELPER_NO_SCALING;
> + int min_scale = DRM_PLANE_HELPER_NO_SCALING;
> int ret;
>
> crtc = crtc ? crtc : plane->crtc;
> intel_crtc = to_intel_crtc(crtc);
> + crtc_state = state->base.state ?
> + intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
> + scaler_state = crtc_state ? &crtc_state->scaler_state : NULL;
> +
> + if (INTEL_INFO(dev)->gen >= 9) {
> + if (scaler_state && scaler_state->num_scalers) {
> + min_scale = 1;
> + max_scale = (100 << 16) / scaler_state->min_hsr;
> + }
> + }
>
> ret = drm_plane_helper_check_update(plane, crtc, fb,
> src, dest, clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> + min_scale,
> + max_scale,
> false, true, &state->visible);
> if (ret)
> return ret;
> @@ -12886,6 +12937,13 @@ intel_check_primary_plane(struct drm_plane *plane,
> src->y1 >>= 16;
> src->y2 >>= 16;
>
> + if (INTEL_INFO(dev)->gen >= 9) {
> + ret = skl_update_scaler_users(intel_crtc, crtc_state,
> + to_intel_plane(plane), state, 0);
> + if (ret)
> + return ret;
> + }
> +
> return 0;
> }
>
> @@ -13065,6 +13123,10 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>
> primary->can_scale = false;
> primary->max_downscale = 1;
> + if (INTEL_INFO(dev)->gen >= 9) {
> + primary->can_scale = true;
> + primary->max_downscale = 2; /* updated later */

I don't see this ever updated again in this patch series? As far as I
can tell, this value only gets used in intel_check_sprite_plane(), so
it's never actually used for primary plane code. Can we just drop the
assignment completely?

Matt

> + }
> state->scaler_id = -1;
> primary->pipe = pipe;
> primary->plane = pipe;
> --
> 1.7.9.5
>

--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795