> -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel Vetter > Sent: Tuesday, May 19, 2015 1:24 AM > To: Konduru, Chandra > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Vetter, Daniel; Syrjala, Ville > Subject: Re: [PATCH 08/12] drm/i915: Add NV12 support to > intel_framebuffer_init > > On Sun, May 17, 2015 at 10:11:01PM -0700, Chandra Konduru wrote: > > This patch adds NV12 as supported format to > > intel_framebuffer_init and performs various checks. > > > > Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx> > > Testcase: igt/kms_nv12 > > --- > > drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++++ > > 1 file changed, 27 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > > index 42924a6..41cd26f 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -14043,6 +14043,33 @@ static int intel_framebuffer_init(struct > drm_device *dev, > > return -EINVAL; > > } > > break; > > + case DRM_FORMAT_NV12: > > + if (INTEL_INFO(dev)->gen < 9) { > > + DRM_DEBUG("unsupported pixel format: %s\n", > > + drm_get_format_name(mode_cmd- > >pixel_format)); > > + return -EINVAL; > > + } > > + if (!mode_cmd->offsets[1]) { > > + DRM_DEBUG("uv start offset not set\n"); > > + return -EINVAL; > > + } > > Nope. It's perfectly ok to have NV12 with a 0 offset for the uv plane, if > it's e.g. in a separate buffer object. Which is the part this series seems > to be completely missing - there's no code at all to look up (and store in > intel_framebuffer the 2nd i915_bo pointer) the 2nd buffer handle afaics. > > You should also change your igts to use 2 separate buffers, just for test > coverage. > -Daniel Hi Daniel, Agree, in general that is very well ok. But as skl hw requires uv to be after y in gtt. This can be guaranteed by having a single bo and y and uv offsets into it. Above sanity checks in i915 specific fb init call are for that reason. There are definitely ways to guarantee uv to be after y even with two separate bos (by uv remapping), but I see that is unnecessary complication and not sure value by allowing that. Or am I missing something here? > > > + if (mode_cmd->pitches[0] != mode_cmd->pitches[1] || > > + mode_cmd->handles[0] != mode_cmd->handles[1]) { > > + DRM_DEBUG("y and uv subplanes have different > parameters\n"); > > + return -EINVAL; > > + } > > + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED > && > > + (mode_cmd->offsets[1] & 0xFFF)) { > > + DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on > new tile-row\n", > > + mode_cmd->offsets[1]); > > + return -EINVAL; > > + } > > + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED > && > > + ((mode_cmd->offsets[1] % mode_cmd->pitches[1]) % > 4)) { > > + DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line > aligned\n", > > + mode_cmd->offsets[1]); > > + } > > + break; > > default: > > DRM_DEBUG("unsupported pixel format: %s\n", > > drm_get_format_name(mode_cmd->pixel_format)); > > -- > > 1.7.9.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
