From: Matt Roper <matthew.d.roper@intel.com>
To: Sonika Jindal <sonika.jindal@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/skl: Support for 90/270 rotation
Date: Thu, 9 Apr 2015 15:54:46 -0700 [thread overview]
Message-ID: <20150409225446.GK9016@intel.com> (raw)
In-Reply-To: <1428395188-12201-1-git-send-email-sonika.jindal@intel.com>
On Tue, Apr 07, 2015 at 01:56:28PM +0530, Sonika Jindal wrote:
> v2: Moving creation of property in a function, checking for 90/270
> rotation simultaneously (Chris)
> Letting primary plane to be positioned
> v3: Adding if/else for 90/270 and rest params programming, adding check for
> pixel_format, some cleanup (review comments)
> v4: Adding right pixel_formats, using src_* params instead of crtc_* for offset
> and size programming (Ville)
> v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
> v6: Rebased on -nightly (Tvrtko's series merged)
> v7: Moving pixel_format check to intel_atomic_plane_check (Matt)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 2 +
> drivers/gpu/drm/i915/intel_atomic_plane.c | 23 ++++++++
> drivers/gpu/drm/i915/intel_display.c | 88 ++++++++++++++++++++---------
> drivers/gpu/drm/i915/intel_drv.h | 6 ++
> drivers/gpu/drm/i915/intel_sprite.c | 52 ++++++++++++-----
> 5 files changed, 130 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b522eb6..564bbd5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
> #define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
> #define PLANE_CTL_ROTATE_MASK 0x3
> #define PLANE_CTL_ROTATE_0 0x0
> +#define PLANE_CTL_ROTATE_90 0x1
> #define PLANE_CTL_ROTATE_180 0x2
> +#define PLANE_CTL_ROTATE_270 0x3
> #define _PLANE_STRIDE_1_A 0x70188
> #define _PLANE_STRIDE_2_A 0x70288
> #define _PLANE_STRIDE_3_A 0x70388
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 976b891..ef8c291 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -162,6 +162,29 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
> (1 << drm_plane_index(plane));
> }
>
> + if (state->fb && intel_rotation_90_or_270(state->rotation)) {
> + if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> + state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
> + DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * 90/270 is not allowed with RGB64 16:16:16:16,
> + * RGB 16-bit 5:6:5, and Indexed 8-bit.
> + */
> + switch (state->fb->pixel_format) {
> + case DRM_FORMAT_C8:
> + case DRM_FORMAT_RGB565:
Your comment mentions RGB64, which isn't handled in the case statement
here. But we don't support RGB64 at all today (in skl_plane_formats[]),
so I'd just remove it from the comment to avoid confusion.
Aside from that, you can consider this
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> + DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
> + drm_get_format_name(state->fb->pixel_format));
> + return -EINVAL;
> +
> + default:
> + break;
> + }
> + }
> +
> return intel_plane->check_plane(plane, intel_state);
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f0bbc22..4de544c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2311,13 +2311,6 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
> info->pitch = fb->pitches[0];
> info->fb_modifier = fb->modifier[0];
>
> - if (!(info->fb_modifier == I915_FORMAT_MOD_Y_TILED ||
> - info->fb_modifier == I915_FORMAT_MOD_Yf_TILED)) {
> - DRM_DEBUG_KMS(
> - "Y or Yf tiling is needed for 90/270 rotation!\n");
> - return -EINVAL;
> - }
> -
> return 0;
> }
>
> @@ -2919,8 +2912,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct drm_i915_gem_object *obj;
> int pipe = intel_crtc->pipe;
> - u32 plane_ctl, stride_div;
> + u32 plane_ctl, stride_div, stride;
> + u32 tile_height, plane_offset, plane_size;
> + unsigned int rotation;
> + int x_offset, y_offset;
> unsigned long surf_addr;
> + struct drm_plane *plane;
>
> if (!intel_crtc->primary_enabled) {
> I915_WRITE(PLANE_CTL(pipe, 0), 0);
> @@ -2981,21 +2978,51 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> }
>
> plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> - if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180))
> +
> + plane = crtc->primary;
> + rotation = plane->state->rotation;
> + switch (rotation) {
> + case BIT(DRM_ROTATE_90):
> + plane_ctl |= PLANE_CTL_ROTATE_90;
> + break;
> +
> + case BIT(DRM_ROTATE_180):
> plane_ctl |= PLANE_CTL_ROTATE_180;
> + break;
> +
> + case BIT(DRM_ROTATE_270):
> + plane_ctl |= PLANE_CTL_ROTATE_270;
> + break;
> + }
>
> obj = intel_fb_obj(fb);
> stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> fb->pixel_format);
> - surf_addr = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj);
> + surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj);
> +
> + if (intel_rotation_90_or_270(rotation)) {
> + /* stride = Surface height in tiles */
> + tile_height = intel_tile_height(dev, fb->bits_per_pixel,
> + fb->modifier[0]);
> + stride = DIV_ROUND_UP(fb->height, tile_height);
> + x_offset = stride * tile_height - y - (plane->state->src_h >> 16);
> + y_offset = x;
> + plane_size = ((plane->state->src_w >> 16) - 1) << 16 |
> + ((plane->state->src_h >> 16) - 1);
> + } else {
> + stride = fb->pitches[0] / stride_div;
> + x_offset = x;
> + y_offset = y;
> + plane_size = ((plane->state->src_h >> 16) - 1) << 16 |
> + ((plane->state->src_w >> 16) - 1);
> + }
> + plane_offset = y_offset << 16 | x_offset;
>
> I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> I915_WRITE(PLANE_POS(pipe, 0), 0);
> - 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_OFFSET(pipe, 0), plane_offset);
> + I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
> + I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
>
> POSTING_READ(PLANE_SURF(pipe, 0));
> @@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> intel_primary_formats, num_formats,
> DRM_PLANE_TYPE_PRIMARY);
>
> - if (INTEL_INFO(dev)->gen >= 4) {
> - if (!dev->mode_config.rotation_property)
> - dev->mode_config.rotation_property =
> - drm_mode_create_rotation_property(dev,
> - BIT(DRM_ROTATE_0) |
> - BIT(DRM_ROTATE_180));
> - if (dev->mode_config.rotation_property)
> - drm_object_attach_property(&primary->base.base,
> - dev->mode_config.rotation_property,
> - state->base.rotation);
> - }
> + if (INTEL_INFO(dev)->gen >= 4)
> + intel_create_rotation_property(dev, primary);
>
> drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
>
> return &primary->base;
> }
>
> +void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane)
> +{
> + if (!dev->mode_config.rotation_property) {
> + unsigned long flags = BIT(DRM_ROTATE_0) |
> + BIT(DRM_ROTATE_180);
> +
> + if (INTEL_INFO(dev)->gen >= 9)
> + flags |= BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270);
> +
> + dev->mode_config.rotation_property =
> + drm_mode_create_rotation_property(dev, flags);
> + }
> + if (dev->mode_config.rotation_property)
> + drm_object_attach_property(&plane->base.base,
> + dev->mode_config.rotation_property,
> + plane->base.state->rotation);
> +}
> +
> static int
> intel_check_cursor_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 811a1db..d32025a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)
> return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));
> }
>
> +unsigned int
> +intel_tile_height(struct drm_device *dev, uint32_t bits_per_pixel,
> + uint64_t fb_modifier);
> +void intel_create_rotation_property(struct drm_device *dev,
> + struct intel_plane *plane);
> +
> bool intel_wm_need_update(struct drm_plane *plane,
> struct drm_plane_state *state);
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index f41e872..83adc9b 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> const int pipe = intel_plane->pipe;
> const int plane = intel_plane->plane + 1;
> - u32 plane_ctl, stride_div;
> + u32 plane_ctl, stride_div, stride;
> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;
> unsigned long surf_addr;
> + u32 tile_height, plane_offset, plane_size;
> + unsigned int rotation;
> + int x_offset, y_offset;
>
> plane_ctl = PLANE_CTL_ENABLE |
> PLANE_CTL_PIPE_CSC_ENABLE;
> @@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> MISSING_CASE(fb->modifier[0]);
> }
>
> - if (drm_plane->state->rotation == BIT(DRM_ROTATE_180))
> + rotation = drm_plane->state->rotation;
> + switch (rotation) {
> + case BIT(DRM_ROTATE_90):
> + plane_ctl |= PLANE_CTL_ROTATE_90;
> + break;
> +
> + case BIT(DRM_ROTATE_180):
> plane_ctl |= PLANE_CTL_ROTATE_180;
> + break;
> +
> + case BIT(DRM_ROTATE_270):
> + plane_ctl |= PLANE_CTL_ROTATE_270;
> + break;
> + }
>
> intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h,
> pixel_size, true,
> @@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>
> surf_addr = intel_plane_obj_offset(intel_plane, obj);
>
> - I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
> - I915_WRITE(PLANE_STRIDE(pipe, plane), fb->pitches[0] / stride_div);
> + if (intel_rotation_90_or_270(rotation)) {
> + /* stride: Surface height in tiles */
> + tile_height = intel_tile_height(dev, fb->bits_per_pixel,
> + fb->modifier[0]);
> + stride = DIV_ROUND_UP(fb->height, tile_height);
> + plane_size = (src_w << 16) | src_h;
> + x_offset = stride * tile_height - y - (src_h + 1);
> + y_offset = x;
> + } else {
> + stride = fb->pitches[0] / stride_div;
> + plane_size = (src_h << 16) | src_w;
> + x_offset = x;
> + y_offset = y;
> + }
> + plane_offset = y_offset << 16 | x_offset;
> +
> + I915_WRITE(PLANE_OFFSET(pipe, plane), plane_offset);
> + I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
> I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
> - I915_WRITE(PLANE_SIZE(pipe, plane), (crtc_h << 16) | crtc_w);
> + I915_WRITE(PLANE_SIZE(pipe, plane), plane_size);
> I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
> I915_WRITE(PLANE_SURF(pipe, plane), surf_addr);
> POSTING_READ(PLANE_SURF(pipe, plane));
> @@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> goto out;
> }
>
> - if (!dev->mode_config.rotation_property)
> - dev->mode_config.rotation_property =
> - drm_mode_create_rotation_property(dev,
> - BIT(DRM_ROTATE_0) |
> - BIT(DRM_ROTATE_180));
> -
> - if (dev->mode_config.rotation_property)
> - drm_object_attach_property(&intel_plane->base.base,
> - dev->mode_config.rotation_property,
> - state->base.rotation);
> + intel_create_rotation_property(dev, intel_plane);
>
> drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
>
> --
> 1.7.10.4
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-04-09 22:54 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-30 8:34 [PATCH 1/2] drm/i915/skl: Allow universal planes to position Sonika Jindal
2015-03-30 8:34 ` [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation Sonika Jindal
2015-03-30 12:15 ` shuang.he
2015-04-01 18:22 ` Matt Roper
2015-04-02 4:54 ` Jindal, Sonika
2015-04-02 8:55 ` Tvrtko Ursulin
2015-04-02 15:59 ` Matt Roper
2015-04-06 12:27 ` Jindal, Sonika
2015-04-07 8:13 ` Daniel Vetter
2015-04-07 8:22 ` Jindal, Sonika
2015-04-07 8:26 ` [PATCH] " Sonika Jindal
2015-04-07 10:59 ` shuang.he
2015-04-09 22:54 ` Matt Roper [this message]
2015-04-01 18:21 ` [PATCH 1/2] drm/i915/skl: Allow universal planes to position Matt Roper
2015-04-02 4:38 ` Jindal, Sonika
2015-04-02 15:48 ` Matt Roper
2015-04-06 5:20 ` Jindal, Sonika
2015-04-07 8:16 ` Daniel Vetter
2015-04-07 8:21 ` Jindal, Sonika
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=20150409225446.GK9016@intel.com \
--to=matthew.d.roper@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=sonika.jindal@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.