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@xxxxxxxxx>
> ---
> 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@xxxxxxxxx>
> + 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
