| Message ID | 1431141767-29455-3-git-send-email-chandra.konduru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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. > } > > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > Sent: Monday, May 11, 2015 5:03 AM > To: Konduru, Chandra > Cc: intel-gfx@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@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@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC
On Mon, May 11, 2015 at 11:32:07PM +0000, Konduru, Chandra wrote: > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > > Sent: Monday, May 11, 2015 5:03 AM > > To: Konduru, Chandra > > Cc: intel-gfx@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@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. 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?
I'll take a look. -----Original Message----- From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] Sent: Tuesday, May 12, 2015 3:44 AM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville; Jindal, Sonika; Runyan, Arthur J Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add 90/270 rotation for NV12 format. On Mon, May 11, 2015 at 11:32:07PM +0000, Konduru, Chandra wrote: > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > > Sent: Monday, May 11, 2015 5:03 AM > > To: Konduru, Chandra > > Cc: intel-gfx@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@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. 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?
> From: Runyan, Arthur J > > I'll take a look. Art, Any update to close on this? [snip] > > > > @@ -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
The statement is correct - " the X offset must always be even for YUV422+NV12, and the Y offset must be even when rotated 90/270 degrees." >From: Konduru, Chandra >> From: Runyan, Arthur J >> >> I'll take a look. > >Art, Any update to close on this? > >[snip] > >> > > > @@ -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
> -----Original Message----- > From: Runyan, Arthur J > Sent: Monday, May 18, 2015 12:19 PM > To: Konduru, Chandra; Ville Syrjälä > Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville; Jindal, Sonika > Subject: RE: [Intel-gfx] [PATCH 2/2] drm/i915: Add 90/270 rotation for NV12 > format. > > The statement is correct - " the X offset must always be even for YUV422+NV12, > and the Y offset must be even when rotated 90/270 degrees." Thanks Art. Then below code to take care evenness for both X and Y offsets when YUV 90/270 is ok. Ville, with this, can you give R-b tag on the ones you reviewed? > > >From: Konduru, Chandra > >> From: Runyan, Arthur J > >> > >> I'll take a look. > > > >Art, Any update to close on this? > > > >[snip] > > > >> > > > @@ -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
On Mon, May 18, 2015 at 07:19:19PM +0000, Runyan, Arthur J wrote: > The statement is correct - " the X offset must always be even for YUV422+NV12, and the Y offset must be even when rotated 90/270 degrees." Hmm. Can you elaborate a bit? I'm curious where this limitation comes from. > > >From: Konduru, Chandra > >> From: Runyan, Arthur J > >> > >> I'll take a look. > > > >Art, Any update to close on this? > > > >[snip] > > > >> > > > @@ -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
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >On Mon, May 18, 2015 at 07:19:19PM +0000, Runyan, Arthur J wrote: >> The statement is correct - " the X offset must always be even for YUV422+NV12, >and the Y offset must be even when rotated 90/270 degrees." > >Hmm. Can you elaborate a bit? I'm curious where this limitation comes >from. > Ah, I get what you are asking now. They don't both have to be even when roated. The text should say " the X offset must be even for YUV422+NV12 ***when not rotated 90/270***, and the Y offset must be even when rotated 90/270 degrees." I'll fix that text.
On Fri, May 22, 2015 at 06:07:36PM +0000, Runyan, Arthur J wrote: > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > > >On Mon, May 18, 2015 at 07:19:19PM +0000, Runyan, Arthur J wrote: > >> The statement is correct - " the X offset must always be even for YUV422+NV12, > >and the Y offset must be even when rotated 90/270 degrees." > > > >Hmm. Can you elaborate a bit? I'm curious where this limitation comes > >from. > > > Ah, I get what you are asking now. They don't both have to be even when roated. The text should say " the X offset must be even for YUV422+NV12 ***when not rotated 90/270***, and the Y offset must be even when rotated 90/270 degrees." I'll fix that text. Excellent. Thanks for confirming my hunch :)
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; + } } 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; + } } }
Adding NV12 90/270 rotation support for primary and sprite planes. Signed-off-by: Chandra Konduru <chandra.konduru@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(-)