On Mon, May 11, 2015 at 11:32:07PM +0000, Konduru, Chandra wrote: > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > > Sent: Monday, May 11, 2015 5:03 AM > > To: Konduru, Chandra > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Vetter, Daniel; Syrjala, Ville > > Subject: Re: [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@xxxxxxxxx> > > > --- > > > 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. The src coordinates are always in the fb orientation, so macropixels appear in the src.x direction only. And when we do 90/270 rotation the hardware Y offset comes from src.x coordinates. The spec does seem a bit confused though; It claims the X offset must always be even for YUV422+NV12, and the Y offset must be even when rotated 90/270 degrees. I suspect the X offset text just didn't get updated when 90/270 rotation was added. Art, can you confirm? -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
