On Fri, Oct 04, 2019 at 01:34:54PM +0200, Maarten Lankhorst wrote:
> We have a src and dect rectangle, use it instead of relying on
> the core drm properties.
>
> This removes the special case in the watermark code for cursor w/h.
I like getting rid of special cases. I guess the only concern I once
had is whether we'd need the clipped coordinates for something,
but I don't think we actually do.
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 57 +++++++++++--------
> drivers/gpu/drm/i915/intel_pm.c | 58 +++++++-------------
> 2 files changed, 53 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index c3ac5a5c5185..9e34be48c770 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -10591,16 +10591,16 @@ static u32 intel_cursor_base(const struct
> intel_plane_state *plane_state)
> /* ILK+ do this automagically */
> if (HAS_GMCH(dev_priv) &&
> plane_state->base.rotation & DRM_MODE_ROTATE_180)
> - base += (plane_state->base.crtc_h *
> - plane_state->base.crtc_w - 1) * fb->format->cpp[0];
> + base += (drm_rect_height(&plane_state->base.dst) *
> + drm_rect_width(&plane_state->base.dst) - 1) *
> fb->format->cpp[0];
>
> return base;
> }
>
> static u32 intel_cursor_position(const struct intel_plane_state *plane_state)
> {
> - int x = plane_state->base.crtc_x;
> - int y = plane_state->base.crtc_y;
> + int x = plane_state->base.dst.x1;
> + int y = plane_state->base.dst.y1;
> u32 pos = 0;
>
> if (x < 0) {
> @@ -10622,8 +10622,8 @@ static bool intel_cursor_size_ok(const struct
> intel_plane_state *plane_state)
> {
> const struct drm_mode_config *config =
> &plane_state->base.plane->dev->mode_config;
> - int width = plane_state->base.crtc_w;
> - int height = plane_state->base.crtc_h;
> + int width = drm_rect_width(&plane_state->base.dst);
> + int height = drm_rect_height(&plane_state->base.dst);
>
> return width > 0 && width <= config->cursor_width &&
> height > 0 && height <= config->cursor_height;
> @@ -10642,8 +10642,8 @@ static int intel_cursor_check_surface(struct
> intel_plane_state *plane_state)
> if (!plane_state->base.visible)
> return 0;
>
> - src_x = plane_state->base.src_x >> 16;
> - src_y = plane_state->base.src_y >> 16;
> + src_x = plane_state->base.src.x1 >> 16;
> + src_y = plane_state->base.src.y1 >> 16;
>
> intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
> offset = intel_plane_compute_aligned_offset(&src_x, &src_y,
> @@ -10678,6 +10678,10 @@ static int intel_check_cursor(struct
> intel_crtc_state *crtc_state,
> if (ret)
> return ret;
>
> + /* Use the unclipped src/dst rectangles, which we program to hw */
> + plane_state->base.src = drm_plane_state_src(&plane_state->base);
> + plane_state->base.dst = drm_plane_state_dest(&plane_state->base);
> +
> ret = intel_cursor_check_surface(plane_state);
> if (ret)
> return ret;
> @@ -10720,7 +10724,7 @@ static u32 i845_cursor_ctl(const struct
> intel_crtc_state *crtc_state,
>
> static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state)
> {
> - int width = plane_state->base.crtc_w;
> + int width = drm_rect_width(&plane_state->base.dst);
>
> /*
> * 845g/865g are only limited by the width of their cursors,
> @@ -10746,8 +10750,8 @@ static int i845_check_cursor(struct intel_crtc_state
> *crtc_state,
> /* Check for which cursor types we support */
> if (!i845_cursor_size_ok(plane_state)) {
> DRM_DEBUG("Cursor dimension %dx%d not supported\n",
> - plane_state->base.crtc_w,
> - plane_state->base.crtc_h);
> + drm_rect_width(&plane_state->base.dst),
> + drm_rect_height(&plane_state->base.dst));
> return -EINVAL;
> }
>
> @@ -10780,8 +10784,8 @@ static void i845_update_cursor(struct intel_plane
> *plane,
> unsigned long irqflags;
>
> if (plane_state && plane_state->base.visible) {
> - unsigned int width = plane_state->base.crtc_w;
> - unsigned int height = plane_state->base.crtc_h;
> + unsigned int width = drm_rect_width(&plane_state->base.src);
> + unsigned int height = drm_rect_height(&plane_state->base.dst);
>
> cntl = plane_state->ctl |
> i845_cursor_ctl_crtc(crtc_state);
> @@ -10883,7 +10887,7 @@ static u32 i9xx_cursor_ctl(const struct
> intel_crtc_state *crtc_state,
> if (IS_GEN(dev_priv, 6) || IS_IVYBRIDGE(dev_priv))
> cntl |= MCURSOR_TRICKLE_FEED_DISABLE;
>
> - switch (plane_state->base.crtc_w) {
> + switch (drm_rect_width(&plane_state->base.dst)) {
> case 64:
> cntl |= MCURSOR_MODE_64_ARGB_AX;
> break;
> @@ -10894,7 +10898,7 @@ static u32 i9xx_cursor_ctl(const struct
> intel_crtc_state *crtc_state,
> cntl |= MCURSOR_MODE_256_ARGB_AX;
> break;
> default:
> - MISSING_CASE(plane_state->base.crtc_w);
> + MISSING_CASE(drm_rect_width(&plane_state->base.dst));
> return 0;
> }
>
> @@ -10908,8 +10912,8 @@ static bool i9xx_cursor_size_ok(const struct
> intel_plane_state *plane_state)
> {
> struct drm_i915_private *dev_priv =
> to_i915(plane_state->base.plane->dev);
> - int width = plane_state->base.crtc_w;
> - int height = plane_state->base.crtc_h;
> + int width = drm_rect_width(&plane_state->base.dst);
> + int height = drm_rect_height(&plane_state->base.dst);
>
> if (!intel_cursor_size_ok(plane_state))
> return false;
> @@ -10962,17 +10966,19 @@ static int i9xx_check_cursor(struct
> intel_crtc_state *crtc_state,
> /* Check for which cursor types we support */
> if (!i9xx_cursor_size_ok(plane_state)) {
> DRM_DEBUG("Cursor dimension %dx%d not supported\n",
> - plane_state->base.crtc_w,
> - plane_state->base.crtc_h);
> + drm_rect_width(&plane_state->base.dst),
> + drm_rect_height(&plane_state->base.dst));
> return -EINVAL;
> }
>
> WARN_ON(plane_state->base.visible &&
> plane_state->color_plane[0].stride != fb->pitches[0]);
>
> - if (fb->pitches[0] != plane_state->base.crtc_w * fb->format->cpp[0]) {
> + if (fb->pitches[0] !=
> + drm_rect_width(&plane_state->base.dst) * fb->format->cpp[0]) {
> DRM_DEBUG_KMS("Invalid cursor stride (%u) (cursor width %d)\n",
> - fb->pitches[0], plane_state->base.crtc_w);
> + fb->pitches[0],
> + drm_rect_width(&plane_state->base.dst));
> return -EINVAL;
> }
>
> @@ -10987,7 +10993,7 @@ static int i9xx_check_cursor(struct intel_crtc_state
> *crtc_state,
> * Refuse the put the cursor into that compromised position.
> */
> if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_C &&
> - plane_state->base.visible && plane_state->base.crtc_x < 0) {
> + plane_state->base.visible && plane_state->base.dst.x1 < 0) {
> DRM_DEBUG_KMS("CHV cursor C not allowed to straddle the left
> screen edge\n");
> return -EINVAL;
> }
> @@ -11007,11 +11013,14 @@ static void i9xx_update_cursor(struct intel_plane
> *plane,
> unsigned long irqflags;
>
> if (plane_state && plane_state->base.visible) {
> + unsigned width = drm_rect_width(&plane_state->base.dst);
> + unsigned height = drm_rect_height(&plane_state->base.dst);
> +
> cntl = plane_state->ctl |
> i9xx_cursor_ctl_crtc(crtc_state);
>
> - if (plane_state->base.crtc_h != plane_state->base.crtc_w)
> - fbc_ctl = CUR_FBC_CTL_EN | (plane_state->base.crtc_h -
> 1);
> + if (width != height)
> + fbc_ctl = CUR_FBC_CTL_EN | (height - 1);
>
> base = intel_cursor_base(plane_state);
> pos = intel_cursor_position(plane_state);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6aeaad587a20..53358e33df1b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1117,10 +1117,7 @@ static u16 g4x_compute_wm(const struct
> intel_crtc_state *crtc_state,
> clock = adjusted_mode->crtc_clock;
> htotal = adjusted_mode->crtc_htotal;
>
> - if (plane->id == PLANE_CURSOR)
> - width = plane_state->base.crtc_w;
> - else
> - width = drm_rect_width(&plane_state->base.dst);
> + width = drm_rect_width(&plane_state->base.dst);
>
> if (plane->id == PLANE_CURSOR) {
> wm = intel_wm_method2(clock, htotal, width, cpp, latency);
> @@ -2549,7 +2546,8 @@ static u32 ilk_compute_cur_wm(const struct
> intel_crtc_state *crtc_state,
>
> return ilk_wm_method2(crtc_state->pixel_rate,
> crtc_state->base.adjusted_mode.crtc_htotal,
> - plane_state->base.crtc_w, cpp, mem_value);
> + drm_rect_width(&plane_state->base.dst),
> + cpp, mem_value);
> }
>
> /* Only for WM_LP. */
> @@ -4046,7 +4044,6 @@ static uint_fixed_16_16_t
> skl_plane_downscale_amount(const struct intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state)
> {
> - struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> u32 src_w, src_h, dst_w, dst_h;
> uint_fixed_16_16_t fp_w_ratio, fp_h_ratio;
> uint_fixed_16_16_t downscale_h, downscale_w;
> @@ -4054,27 +4051,17 @@ skl_plane_downscale_amount(const struct
> intel_crtc_state *crtc_state,
> if (WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)))
> return u32_to_fixed16(0);
>
> - /* n.b., src is 16.16 fixed point, dst is whole integer */
> - if (plane->id == PLANE_CURSOR) {
> - /*
> - * Cursors only support 0/180 degree rotation,
> - * hence no need to account for rotation here.
> - */
> - src_w = plane_state->base.src_w >> 16;
> - src_h = plane_state->base.src_h >> 16;
> - dst_w = plane_state->base.crtc_w;
> - dst_h = plane_state->base.crtc_h;
> - } else {
> - /*
> - * Src coordinates are already rotated by 270 degrees for
> - * the 90/270 degree plane rotation cases (to match the
> - * GTT mapping), hence no need to account for rotation here.
> - */
> - src_w = drm_rect_width(&plane_state->base.src) >> 16;
> - src_h = drm_rect_height(&plane_state->base.src) >> 16;
> - dst_w = drm_rect_width(&plane_state->base.dst);
> - dst_h = drm_rect_height(&plane_state->base.dst);
> - }
> + /*
> + * Src coordinates are already rotated by 270 degrees for
> + * the 90/270 degree plane rotation cases (to match the
> + * GTT mapping), hence no need to account for rotation here.
> + *
> + * n.b., src is 16.16 fixed point, dst is whole integer.
> + */
> + src_w = drm_rect_width(&plane_state->base.src) >> 16;
> + src_h = drm_rect_height(&plane_state->base.src) >> 16;
> + dst_w = drm_rect_width(&plane_state->base.dst);
> + dst_h = drm_rect_height(&plane_state->base.dst);
>
> fp_w_ratio = div_fixed16(src_w, dst_w);
> fp_h_ratio = div_fixed16(src_h, dst_h);
> @@ -4698,20 +4685,15 @@ skl_compute_plane_wm_params(const struct
> intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state,
> struct skl_wm_params *wp, int color_plane)
> {
> - struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> const struct drm_framebuffer *fb = plane_state->base.fb;
> int width;
>
> - if (plane->id == PLANE_CURSOR) {
> - width = plane_state->base.crtc_w;
> - } else {
> - /*
> - * Src coordinates are already rotated by 270 degrees for
> - * the 90/270 degree plane rotation cases (to match the
> - * GTT mapping), hence no need to account for rotation here.
> - */
> - width = drm_rect_width(&plane_state->base.src) >> 16;
> - }
> + /*
> + * Src coordinates are already rotated by 270 degrees for
> + * the 90/270 degree plane rotation cases (to match the
> + * GTT mapping), hence no need to account for rotation here.
> + */
> + width = drm_rect_width(&plane_state->base.src) >> 16;
>
> return skl_compute_wm_params(crtc_state, width,
> fb->format, fb->modifier,
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
