From: Daniel Vetter <daniel@ffwll.ch>
To: ville.syrjala@linux.intel.com
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v3 08/18] drm/i915: Pass 90/270 vs. 0/180 rotation info for intel_gen4_compute_page_offset()
Date: Mon, 25 Jan 2016 18:30:17 +0100 [thread overview]
Message-ID: <20160125173017.GF11240@phenom.ffwll.local> (raw)
In-Reply-To: <1453316739-13296-9-git-send-email-ville.syrjala@linux.intel.com>
On Wed, Jan 20, 2016 at 09:05:29PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The page aligned surface address calculation needs to know which way
> things are rotated. The contract now says that the caller must pass the
> rotate x/y coordinates, as well as the tile_height aligned stride in
> the tile_height direction. This will make it fairly simple to deal with
> 90/270 degree rotation on SKL+ where we have to deal with the rotated
> view into the GTT.
>
> v2: Pass rotation instead of bool even thoughwe only care about 0/180 vs. 90/270
> v3: Introduce intel_tile_dims(), and don't mix up different units so much
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v2)
> ---
> drivers/gpu/drm/i915/intel_display.c | 66 +++++++++++++++++++++++++-----------
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_sprite.c | 18 +++++-----
> 3 files changed, 59 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cfd52ea68e34..bda3224021b2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2269,6 +2269,18 @@ unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
> intel_tile_width(dev_priv, fb_modifier, cpp);
> }
>
> +/* Return the tile dimensions in pixel units */
> +static void intel_tile_dims(const struct drm_i915_private *dev_priv,
> + unsigned int *tile_width,
> + unsigned int *tile_height,
> + uint64_t fb_modifier,
> + unsigned int cpp)
> +{
> + *tile_width = intel_tile_width(dev_priv, fb_modifier, cpp);
> + *tile_height = intel_tile_size(dev_priv) / *tile_width;
> + *tile_width /= cpp;
Seems like my suggestion to use tile_width in cpp wasn't that awesome, at
least we still have a big confusion right here. Not sure what to do,
especially since these kind of changes are super-painful to review. Could
we perhaps go back to the old versions and untangle this mess afterwards?
Iirc I've follow-up with r-b tags to all patches in the old series for
that approach.
No idea really what would be best here ...
-Daniel
> +}
> +
> unsigned int
> intel_fb_align_height(struct drm_device *dev, unsigned int height,
> uint32_t pixel_format, uint64_t fb_modifier)
> @@ -2306,19 +2318,19 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
> tile_size = intel_tile_size(dev_priv);
>
> cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> - tile_width = intel_tile_width(dev_priv, fb->modifier[0], cpp);
> - tile_height = tile_size / tile_width;
> + intel_tile_dims(dev_priv, &tile_width, &tile_height,
> + fb->modifier[0], cpp);
>
> - info->width_pages = DIV_ROUND_UP(fb->pitches[0], tile_width);
> + info->width_pages = DIV_ROUND_UP(fb->pitches[0], tile_width * cpp);
> info->height_pages = DIV_ROUND_UP(fb->height, tile_height);
> info->size = info->width_pages * info->height_pages * tile_size;
>
> if (info->pixel_format == DRM_FORMAT_NV12) {
> cpp = drm_format_plane_cpp(fb->pixel_format, 1);
> - tile_width = intel_tile_width(dev_priv, fb->modifier[1], cpp);
> - tile_height = tile_size / tile_width;
> + intel_tile_dims(dev_priv, &tile_width, &tile_height,
> + fb->modifier[1], cpp);
>
> - info->width_pages_uv = DIV_ROUND_UP(fb->pitches[1], tile_width);
> + info->width_pages_uv = DIV_ROUND_UP(fb->pitches[1], tile_width * cpp);
> info->height_pages_uv = DIV_ROUND_UP(fb->height / 2, tile_height);
> info->size_uv = info->width_pages_uv * info->height_pages_uv * tile_size;
> }
> @@ -2446,29 +2458,43 @@ static void intel_unpin_fb_obj(struct drm_framebuffer *fb,
> i915_gem_object_unpin_from_display_plane(obj, &view);
> }
>
> -/* Computes the linear offset to the base tile and adjusts x, y. bytes per pixel
> - * is assumed to be a power-of-two. */
> +/*
> + * Computes the linear offset to the base tile and adjusts
> + * x, y. bytes per pixel is assumed to be a power-of-two.
> + *
> + * In the 90/270 rotated case, x and y are assumed
> + * to be already rotated to match the rotated GTT view, and
> + * pitch is the tile_height aligned framebuffer height.
> + */
> u32 intel_compute_tile_offset(struct drm_i915_private *dev_priv,
> int *x, int *y,
> uint64_t fb_modifier,
> unsigned int cpp,
> - unsigned int pitch)
> + unsigned int pitch,
> + unsigned int rotation)
> {
> if (fb_modifier != DRM_FORMAT_MOD_NONE) {
> unsigned int tile_size, tile_width, tile_height;
> - unsigned int tile_rows, tiles;
> + unsigned int tile_rows, tiles, pitch_tiles;
>
> tile_size = intel_tile_size(dev_priv);
> - tile_width = intel_tile_width(dev_priv, fb_modifier, cpp);
> - tile_height = tile_size / tile_width;
> + intel_tile_dims(dev_priv, &tile_width, &tile_height,
> + fb_modifier, cpp);
> +
> + if (intel_rotation_90_or_270(rotation)) {
> + pitch_tiles = pitch / tile_height;
> + swap(tile_width, tile_height);
> + } else {
> + pitch_tiles = pitch / (tile_width * cpp);
> + }
>
> tile_rows = *y / tile_height;
> *y %= tile_height;
>
> - tiles = *x / (tile_width/cpp);
> - *x %= tile_width/cpp;
> + tiles = *x / tile_width;
> + *x %= tile_width;
>
> - return tile_rows * pitch * tile_height + tiles * tile_size;
> + return (tile_rows * pitch_tiles + tiles) * tile_size;
> } else {
> unsigned int alignment = intel_linear_alignment(dev_priv) - 1;
> unsigned int offset;
> @@ -2709,6 +2735,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
> u32 linear_offset;
> u32 dspcntr;
> i915_reg_t reg = DSPCNTR(plane);
> + unsigned int rotation = plane_state->base.rotation;
> int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> int x = plane_state->src.x1 >> 16;
> int y = plane_state->src.y1 >> 16;
> @@ -2775,13 +2802,13 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
> intel_crtc->dspaddr_offset =
> intel_compute_tile_offset(dev_priv, &x, &y,
> fb->modifier[0], cpp,
> - fb->pitches[0]);
> + fb->pitches[0], rotation);
> linear_offset -= intel_crtc->dspaddr_offset;
> } else {
> intel_crtc->dspaddr_offset = linear_offset;
> }
>
> - if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> dspcntr |= DISPPLANE_ROTATE_180;
>
> x += (crtc_state->pipe_src_w - 1);
> @@ -2839,6 +2866,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
> u32 linear_offset;
> u32 dspcntr;
> i915_reg_t reg = DSPCNTR(plane);
> + unsigned int rotation = plane_state->base.rotation;
> int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> int x = plane_state->src.x1 >> 16;
> int y = plane_state->src.y1 >> 16;
> @@ -2882,9 +2910,9 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
> intel_crtc->dspaddr_offset =
> intel_compute_tile_offset(dev_priv, &x, &y,
> fb->modifier[0], cpp,
> - fb->pitches[0]);
> + fb->pitches[0], rotation);
> linear_offset -= intel_crtc->dspaddr_offset;
> - if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> dspcntr |= DISPPLANE_ROTATE_180;
>
> if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f620023ed134..223693dbfe7c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1176,7 +1176,8 @@ u32 intel_compute_tile_offset(struct drm_i915_private *dev_priv,
> int *x, int *y,
> uint64_t fb_modifier,
> unsigned int cpp,
> - unsigned int pitch);
> + unsigned int pitch,
> + unsigned int rotation);
> void intel_prepare_reset(struct drm_device *dev);
> void intel_finish_reset(struct drm_device *dev);
> void hsw_enable_pc8(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index a2582c455b36..7dc2b8b2a4ac 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -193,7 +193,7 @@ skl_update_plane(struct drm_plane *drm_plane,
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> u32 surf_addr;
> u32 tile_height, plane_offset, plane_size;
> - unsigned int rotation;
> + unsigned int rotation = plane_state->base.rotation;
> int x_offset, y_offset;
> int crtc_x = plane_state->dst.x1;
> int crtc_y = plane_state->dst.y1;
> @@ -213,7 +213,6 @@ skl_update_plane(struct drm_plane *drm_plane,
> plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
> plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]);
>
> - rotation = plane_state->base.rotation;
> plane_ctl |= skl_plane_ctl_rotation(rotation);
>
> stride_div = intel_fb_stride_alignment(dev_priv, fb->modifier[0],
> @@ -351,6 +350,7 @@ vlv_update_plane(struct drm_plane *dplane,
> int plane = intel_plane->plane;
> u32 sprctl;
> u32 sprsurf_offset, linear_offset;
> + unsigned int rotation = dplane->state->rotation;
> int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> int crtc_x = plane_state->dst.x1;
> @@ -425,10 +425,10 @@ vlv_update_plane(struct drm_plane *dplane,
> linear_offset = y * fb->pitches[0] + x * cpp;
> sprsurf_offset = intel_compute_tile_offset(dev_priv, &x, &y,
> fb->modifier[0], cpp,
> - fb->pitches[0]);
> + fb->pitches[0], rotation);
> linear_offset -= sprsurf_offset;
>
> - if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> sprctl |= SP_ROTATE_180;
>
> x += src_w;
> @@ -493,6 +493,7 @@ ivb_update_plane(struct drm_plane *plane,
> enum pipe pipe = intel_plane->pipe;
> u32 sprctl, sprscale = 0;
> u32 sprsurf_offset, linear_offset;
> + unsigned int rotation = plane_state->base.rotation;
> int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> int crtc_x = plane_state->dst.x1;
> @@ -558,10 +559,10 @@ ivb_update_plane(struct drm_plane *plane,
> linear_offset = y * fb->pitches[0] + x * cpp;
> sprsurf_offset = intel_compute_tile_offset(dev_priv, &x, &y,
> fb->modifier[0], cpp,
> - fb->pitches[0]);
> + fb->pitches[0], rotation);
> linear_offset -= sprsurf_offset;
>
> - if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> sprctl |= SPRITE_ROTATE_180;
>
> /* HSW and BDW does this automagically in hardware */
> @@ -634,6 +635,7 @@ ilk_update_plane(struct drm_plane *plane,
> int pipe = intel_plane->pipe;
> u32 dvscntr, dvsscale;
> u32 dvssurf_offset, linear_offset;
> + unsigned int rotation = plane_state->base.rotation;
> int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> int crtc_x = plane_state->dst.x1;
> @@ -695,10 +697,10 @@ ilk_update_plane(struct drm_plane *plane,
> linear_offset = y * fb->pitches[0] + x * cpp;
> dvssurf_offset = intel_compute_tile_offset(dev_priv, &x, &y,
> fb->modifier[0], cpp,
> - fb->pitches[0]);
> + fb->pitches[0], rotation);
> linear_offset -= dvssurf_offset;
>
> - if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> dvscntr |= DVS_ROTATE_180;
>
> x += src_w;
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-01-25 17:30 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-20 19:05 [PATCH v2 00/18] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v2) ville.syrjala
2016-01-20 19:05 ` [PATCH 01/18] drm/i915: Rename the rotated gtt view member to 'rotated' ville.syrjala
2016-01-25 16:50 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH 02/18] drm/i915: Pass stride to rotate_pages() ville.syrjala
2016-01-25 16:52 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH 03/18] drm/i915: Pass rotation_info to intel_rotate_fb_obj_pages() ville.syrjala
2016-01-25 16:53 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH 04/18] drm/i915: Make display gtt offsets u32 ville.syrjala
2016-01-25 17:00 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH 05/18] drm/i915: Standardize on 'cpp' for bytes per pixel ville.syrjala
2016-01-25 17:05 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH v2 06/18] drm: Add drm_format_plane_width() and drm_format_plane_height() ville.syrjala
2016-01-25 17:08 ` Daniel Vetter
2016-01-28 18:15 ` Ville Syrjälä
2016-01-29 18:01 ` [PATCH v3 " ville.syrjala
2016-02-09 9:08 ` Daniel Vetter
2016-02-09 15:29 ` [PATCH v4 " ville.syrjala
2016-01-20 19:05 ` [PATCH 07/18] drm/i915: Fix intel_tile_width() parameters ville.syrjala
2016-01-25 17:12 ` Daniel Vetter
2016-01-28 18:35 ` Ville Syrjälä
2016-01-28 19:04 ` Ville Syrjälä
2016-01-20 19:05 ` [PATCH v3 08/18] drm/i915: Pass 90/270 vs. 0/180 rotation info for intel_gen4_compute_page_offset() ville.syrjala
2016-01-25 17:30 ` Daniel Vetter [this message]
2016-01-28 18:51 ` Ville Syrjälä
2016-02-10 7:35 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH v2 09/18] drm/i915: Support for extra alignment for tiled surfaces ville.syrjala
2016-01-25 17:24 ` Daniel Vetter
2016-01-25 17:55 ` Ville Syrjälä
2016-01-20 19:05 ` [PATCH v2 10/18] drm/i915: Don't pass plane+plane_state to intel_pin_and_fence_fb_obj() ville.syrjala
2016-01-20 19:05 ` [PATCH 11/18] drm/i915: Pass drm_frambuffer to intel_compute_page_offset() ville.syrjala
2016-01-20 19:05 ` [PATCH 12/18] drm/i915: Reorganize intel_rotation_info ville.syrjala
2016-01-25 17:38 ` Daniel Vetter
2016-01-25 18:03 ` Ville Syrjälä
2016-01-20 19:05 ` [PATCH 13/18] drm/i915: Move the NULL sg handling out from rotate_pages() ville.syrjala
2016-01-25 17:40 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH 14/18] drm/i915: Embed rotation_info under intel_framebuffer ville.syrjala
2016-01-20 21:08 ` Chris Wilson
2016-01-21 12:06 ` Ville Syrjälä
2016-01-21 12:10 ` Chris Wilson
2016-01-25 17:42 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH v3 15/18] drm/i915: Rewrite fb rotation GTT handling ville.syrjala
2016-01-20 19:05 ` [PATCH v2 16/18] drm/i915: Don't pass pitch to intel_compute_page_offset() ville.syrjala
2016-01-25 17:53 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH 17/18] drm/i915: Pass around plane_state instead of fb+rotation ville.syrjala
2016-01-25 17:55 ` Daniel Vetter
2016-01-20 19:05 ` [PATCH v2 18/18] drm/i915: Make sure fb offset is (macro)pixel aligned ville.syrjala
2016-01-21 13:35 ` ✓ Fi.CI.BAT: success for drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v2) Patchwork
2016-01-30 8:31 ` ✗ Fi.CI.BAT: failure for drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v2) (rev2) Patchwork
2016-02-01 8:31 ` Patchwork
2016-02-09 16:31 ` ✗ Fi.CI.BAT: failure for drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v2) (rev3) Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160125173017.GF11240@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.