| Message ID | 1440032556-9920-16-git-send-email-chandra.konduru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 19, 2015 at 06:02:36PM -0700, Chandra Konduru wrote: > Adding NV12 90/270 rotation support for primary and sprite planes. > > v2: > -For 90/270 adjust pixel boundary only in Y-direction (bspec) > > v3: > -Rebased (me) > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > Testcase: igt/kms_nv12 > --- > drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++------ > drivers/gpu/drm/i915/intel_sprite.c | 56 ++++++++++++++++++++++------------ > 2 files changed, 56 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2158b8f..19d0f8b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3096,7 +3096,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; > u32 hphase = 0, vphase = 0; > > @@ -3155,12 +3156,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; > @@ -11697,8 +11702,13 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, > > /* Adjust (macro)pixel boundary */ > if (fb && intel_format_is_yuv(fb->pixel_format)) { > - to_intel_plane_state(plane_state)->src.x1 &= ~0x10000; > - to_intel_plane_state(plane_state)->src.x2 &= ~0x10000; > + if (intel_rotation_90_or_270(plane_state->rotation)) { > + to_intel_plane_state(plane_state)->src.y1 &= ~0x10000; > + to_intel_plane_state(plane_state)->src.y2 &= ~0x10000; > + } else { > + to_intel_plane_state(plane_state)->src.x1 &= ~0x10000; > + to_intel_plane_state(plane_state)->src.x2 &= ~0x10000; > + } IIRC we concluded (with Art's help) that this is not needed. We always want to align in src.x. > } > > if (crtc_state && INTEL_INFO(dev)->gen >= 9 && > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 9d1c5b9..3522cb0 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -188,7 +188,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; > u32 hphase = 0, vphase = 0; > > @@ -238,12 +239,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; > @@ -900,18 +903,33 @@ intel_check_sprite_plane(struct drm_plane *plane, > src_h = drm_rect_height(src) >> 16; > > if (intel_format_is_yuv(fb->pixel_format)) { > - src_x &= ~1; > - src_w &= ~1; > - > - /* > - * Must keep src and dst the > - * same if we can't scale. > - */ > - if (!can_scale) > - crtc_w &= ~1; > - > - 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 (!can_scale) > + crtc_h &= ~1; > + > + if (crtc_h == 0) > + state->visible = false; > + } else { > + src_x &= ~1; > + src_w &= ~1; > + > + /* > + * Must keep src and dst the > + * same if we can't scale. > + */ > + if (!can_scale) > + crtc_w &= ~1; > + > + if (crtc_w == 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
> > /* Adjust (macro)pixel boundary */ > > if (fb && intel_format_is_yuv(fb->pixel_format)) { > > - to_intel_plane_state(plane_state)->src.x1 &= ~0x10000; > > - to_intel_plane_state(plane_state)->src.x2 &= ~0x10000; > > + if (intel_rotation_90_or_270(plane_state->rotation)) { > > + to_intel_plane_state(plane_state)->src.y1 &= > ~0x10000; > > + to_intel_plane_state(plane_state)->src.y2 &= > ~0x10000; > > + } else { > > + to_intel_plane_state(plane_state)->src.x1 &= > ~0x10000; > > + to_intel_plane_state(plane_state)->src.x2 &= > ~0x10000; > > + } > > IIRC we concluded (with Art's help) that this is not needed. We always > want to align in src.x. Initial code that I added was making both X and Y offsets as even when 90/270 as per bspec at that time. But later Art update is as below: >> " the X offset must always be even for YUV422+NV12, and >> the Y offset must be even when rotated 90/270 degrees." So, above code change is needed.
On Sat, Sep 05, 2015 at 01:38:14AM +0000, Konduru, Chandra wrote: > > > /* Adjust (macro)pixel boundary */ > > > if (fb && intel_format_is_yuv(fb->pixel_format)) { > > > - to_intel_plane_state(plane_state)->src.x1 &= ~0x10000; > > > - to_intel_plane_state(plane_state)->src.x2 &= ~0x10000; > > > + if (intel_rotation_90_or_270(plane_state->rotation)) { > > > + to_intel_plane_state(plane_state)->src.y1 &= > > ~0x10000; > > > + to_intel_plane_state(plane_state)->src.y2 &= > > ~0x10000; > > > + } else { > > > + to_intel_plane_state(plane_state)->src.x1 &= > > ~0x10000; > > > + to_intel_plane_state(plane_state)->src.x2 &= > > ~0x10000; > > > + } > > > > IIRC we concluded (with Art's help) that this is not needed. We always > > want to align in src.x. > > Initial code that I added was making both X and Y offsets as even > when 90/270 as per bspec at that time. > > But later Art update is as below: > >> " the X offset must always be even for YUV422+NV12, and > >> the Y offset must be even when rotated 90/270 degrees." > > So, above code change is needed. That's the orignal text, later update from Art was: "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."" And the spec has been updated since to include the same information. For us we just need to align src.x? since that's what gets programmed into the X offset for 0/180, and into the Y offset for 90/270. So the current code is fine as is.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2158b8f..19d0f8b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3096,7 +3096,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; u32 hphase = 0, vphase = 0; @@ -3155,12 +3156,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; @@ -11697,8 +11702,13 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, /* Adjust (macro)pixel boundary */ if (fb && intel_format_is_yuv(fb->pixel_format)) { - to_intel_plane_state(plane_state)->src.x1 &= ~0x10000; - to_intel_plane_state(plane_state)->src.x2 &= ~0x10000; + if (intel_rotation_90_or_270(plane_state->rotation)) { + to_intel_plane_state(plane_state)->src.y1 &= ~0x10000; + to_intel_plane_state(plane_state)->src.y2 &= ~0x10000; + } else { + to_intel_plane_state(plane_state)->src.x1 &= ~0x10000; + to_intel_plane_state(plane_state)->src.x2 &= ~0x10000; + } } if (crtc_state && INTEL_INFO(dev)->gen >= 9 && diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 9d1c5b9..3522cb0 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -188,7 +188,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; u32 hphase = 0, vphase = 0; @@ -238,12 +239,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; @@ -900,18 +903,33 @@ intel_check_sprite_plane(struct drm_plane *plane, src_h = drm_rect_height(src) >> 16; if (intel_format_is_yuv(fb->pixel_format)) { - src_x &= ~1; - src_w &= ~1; - - /* - * Must keep src and dst the - * same if we can't scale. - */ - if (!can_scale) - crtc_w &= ~1; - - 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 (!can_scale) + crtc_h &= ~1; + + if (crtc_h == 0) + state->visible = false; + } else { + src_x &= ~1; + src_w &= ~1; + + /* + * Must keep src and dst the + * same if we can't scale. + */ + if (!can_scale) + crtc_w &= ~1; + + if (crtc_w == 0) + state->visible = false; + } } }
Adding NV12 90/270 rotation support for primary and sprite planes. v2: -For 90/270 adjust pixel boundary only in Y-direction (bspec) v3: -Rebased (me) Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> Testcase: igt/kms_nv12 --- drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++------ drivers/gpu/drm/i915/intel_sprite.c | 56 ++++++++++++++++++++++------------ 2 files changed, 56 insertions(+), 28 deletions(-)