From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 3/4] drm: Add NV24 and NV42 pixel formats Date: Wed, 30 May 2012 16:09:57 +0200 Message-ID: <10624621.D9eOIsUKNs@avalon> References: <1338381179-13290-1-git-send-email-laurent.pinchart@ideasonboard.com> <10458627.VBMA7d1R5q@avalon> <20120530140510.GS13065@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [95.142.166.194]) by gabe.freedesktop.org (Postfix) with ESMTP id 6DD1C9E954 for ; Wed, 30 May 2012 07:09:35 -0700 (PDT) In-Reply-To: <20120530140510.GS13065@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org Hi Ville, On Wednesday 30 May 2012 17:05:10 Ville Syrj=E4l=E4 wrote: > On Wed, May 30, 2012 at 03:20:09PM +0200, Laurent Pinchart wrote: > > On Wednesday 30 May 2012 16:09:25 Ville Syrj=E4l=E4 wrote: > > > On Wed, May 30, 2012 at 02:32:58PM +0200, Laurent Pinchart wrote: > > > > Signed-off-by: Laurent Pinchart > > > > --- > > > > = > > > > include/drm/drm_fourcc.h | 2 ++ > > > > 1 files changed, 2 insertions(+), 0 deletions(-) > > > > = > > > > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h > > > > index bdf0152..fac7235 100644 > > > > --- a/include/drm/drm_fourcc.h > > > > +++ b/include/drm/drm_fourcc.h > > > > @@ -106,6 +106,8 @@ > > > > = > > > > #define DRM_FORMAT_NV21 fourcc_code('N', 'V', '2', '1') /* 2x2 > > > > subsampled Cb:Cr plane */ > > > > #define DRM_FORMAT_NV16 fourcc_code('N', 'V', '1', '6') /* 2x1 > > > > subsampled Cr:Cb plane */ > > > > #define DRM_FORMAT_NV61 fourcc_code('N', 'V', '6', '1') /* 2x1 > > > > subsampled Cb:Cr plane */ > > > > +#define DRM_FORMAT_NV24 fourcc_code('N', 'V', '2', '4') /* > > > > non-subsampled Cr:Cb plane */ > > > > +#define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /* > > > > non-subsampled Cb:Cr plane */ > > > = > > > If you want these to reach the driver you need to add them to > > > format_check(). > > = > > Oops, my bad, indeed. > > = > > > Also you should update drm_format_num_planes() and > > > drm_format_plane_cpp() > > > appropriately. > > = > > Will do. > > = > > I'm a bit puzzled by drm_format_plane_cpp(). I would have expected the > > return value to be 1 for NV12/21 and NV16/61 formats (2 U/V components, > > but 1/2 horizontal subsampling). Is that a bug, or am I missing somethi= ng > > ? > The way I used it originally was to calculate the minimum stride for a > plane. So the formula was something like this: > min_stride =3D width / plane_horiz_subsampling * plane_cpp I had missed the division by the horizontal subsampling factor in = framebuffer_check(). It makes sense now. > I guess you could also call it pixel stride (as opposed to line stride). > That is when you're walking the Cb (or Cr) samples you need to step two > bytes to the get to the next sample. > = > Or if you just think about the chroma plane as just another packed pixel > format then it also makes sense to have cpp=3D2. > = > YUYV and co. are more problematic though since you have the subsampled > and non-subsampled stuff in the same plane. There you could argue that > both 2 and 4 are sensible values. I used 2 there since it meant that I > didn't have to add special cases to the minimum stride calculations. > = > There might be a need to add drm_format_macropixel_size() or some such > function in the future which would return 4 for YUYV. Especially if > someone wants to add funky formats such as Y41P. -- = Regards, Laurent Pinchart