[Intel-gfx] [PATCH 2/2] drm/i915: Add 90/270 rotation for NV12 format.
Konduru, Chandra
chandra.konduru at intel.com
Mon May 11 16:32:07 PDT 2015
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Monday, May 11, 2015 5:03 AM
> To: Konduru, Chandra
> Cc: intel-gfx at lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville
> Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add 90/270 rotation for NV12
> format.
>
> On Fri, May 08, 2015 at 08:22:47PM -0700, Chandra Konduru wrote:
> > Adding NV12 90/270 rotation support for primary and sprite planes.
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru at intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 23 ++++++++++++++++-------
> > drivers/gpu/drm/i915/intel_sprite.c | 32 +++++++++++++++++++++++++------
> -
> > 2 files changed, 41 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> > index c385a3b..77d7f69 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3105,7 +3105,8 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
> > 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;
> > - u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
> > + unsigned long aux_dist = 0;
> > + u32 aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
> > u32 tile_row_adjustment = 0;
> >
> > plane_state = to_intel_plane_state(plane->state);
> > @@ -3163,12 +3164,16 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
> > x_offset = stride * tile_height - y - src_h;
> > y_offset = x;
> > plane_size = (src_w - 1) << 16 | (src_h - 1);
> > - /*
> > - * TBD: For NV12 90/270 rotation, Y and UV subplanes should
> > - * be treated as separate surfaces and GTT remapping for
> > - * rotation should be done separately for each subplane.
> > - * Enable support once seperate remappings are available.
> > - */
> > +
> > + if (fb->pixel_format == DRM_FORMAT_NV12) {
> > + u32 uv_tile_height = intel_tile_height(dev, fb-
> >pixel_format,
> > + fb->modifier[0], 1);
> > + aux_stride = DIV_ROUND_UP(fb->height / 2,
> uv_tile_height);
> > + aux_dist =
> intel_plane_obj_offset(to_intel_plane(plane), obj, 1) -
> > + surf_addr;
> > + aux_x_offset = aux_stride * uv_tile_height - y / 2 - fb-
> >height / 2;
> > + aux_y_offset = x / 2;
> > + }
> > } else {
> > stride = fb->pitches[0] / stride_div;
> > x_offset = x;
> > @@ -13144,6 +13149,10 @@ intel_check_primary_plane(struct drm_plane
> *plane,
> > if (fb && format_is_yuv(fb->pixel_format)) {
> > src->x1 &= ~0x10000;
> > src->x2 &= ~0x10000;
> > + if (intel_rotation_90_or_270(state->base.rotation)) {
> > + src->y1 &= ~0x10000;
> > + src->y2 &= ~0x10000;
> > + }
>
> This feels fishy. Why do we need to make the Y coordinates even? The
> reson for making the X coordinates even is to make them macropixel
> aligned, but there are no macropixels in the Y direction so this
> doesn't make much sense to me.
Hi Ville,
Per skl spec, it is expecting even lines aligned with 90/270 rotation not only
for NV12 but also for 422 formats. Perhaps we might have missed when 90/270
enabled for packed YUV formats.
>
> > }
> >
> > if (INTEL_INFO(dev)->gen >= 9) {
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> > index e23fe8e..d4ce7eb 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -190,7 +190,8 @@ skl_update_plane(struct drm_plane *drm_plane,
> struct drm_crtc *crtc,
> > int x_offset, y_offset;
> > struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config;
> > int scaler_id;
> > - u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
> > + unsigned long aux_dist = 0;
> > + u32 aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
> > u32 tile_row_adjustment = 0;
> >
> > plane_ctl = PLANE_CTL_ENABLE |
> > @@ -239,12 +240,14 @@ skl_update_plane(struct drm_plane *drm_plane,
> struct drm_crtc *crtc,
> > x_offset = stride * tile_height - y - (src_h + 1);
> > y_offset = x;
> >
> > - /*
> > - * TBD: For NV12 90/270 rotation, Y and UV subplanes should
> > - * be treated as separate surfaces and GTT remapping for
> > - * rotation should be done separately for each subplane.
> > - * Enable support once seperate remappings are available.
> > - */
> > + if (fb->pixel_format == DRM_FORMAT_NV12) {
> > + u32 uv_tile_height = intel_tile_height(dev, fb-
> >pixel_format,
> > + fb->modifier[0], 1);
> > + aux_stride = DIV_ROUND_UP(fb->height / 2,
> uv_tile_height);
> > + aux_dist = intel_plane_obj_offset(intel_plane, obj, 1) -
> surf_addr;
> > + aux_x_offset = aux_stride * uv_tile_height - y / 2 - fb-
> >height / 2;
> > + aux_y_offset = x / 2;
> > + }
> > } else {
> > stride = fb->pitches[0] / stride_div;
> > plane_size = (src_h << 16) | src_w;
> > @@ -909,6 +912,21 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >
> > if (crtc_w == 0)
> > state->visible = false;
> > +
> > + if (intel_rotation_90_or_270(state->base.rotation)) {
> > + src_y &= ~1;
> > + src_h &= ~1;
> > +
> > + /*
> > + * Must keep src and dst the
> > + * same if we can't scale.
> > + */
> > + if (!intel_plane->can_scale)
> > + crtc_h &= ~1;
> > +
> > + if (crtc_h == 0)
> > + state->visible = false;
> > + }
>
> Same here.
>
> > }
> > }
> >
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
More information about the Intel-gfx
mailing list