| Message ID | 1511836053-9489-3-git-send-email-hyun.kwon@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 27, 2017 at 06:27:32PM -0800, Hyun Kwon wrote: > From: Jeffrey Mouroux <jmouroux@xilinx.com> > > The Xilinx Video Mixer and Xilinx Video Framebuffer DMA IP > support video memory formats that are not represented in the > current DRM fourcc library. This patch adds those missing > fourcc codes. > > Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com> > Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > --- > include/uapi/drm/drm_fourcc.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > index 3ad838d..83806d5 100644 > --- a/include/uapi/drm/drm_fourcc.h > +++ b/include/uapi/drm/drm_fourcc.h > @@ -112,6 +112,13 @@ extern "C" { > #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */ > > #define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ > +#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] Cr:Cb:Y little endian */ > +#define DRM_FORMAT_XVUY8888 fourcc_code('X', 'V', '2', '4') /* [31:0] x:Cr:Cb:Y 8:8:8:8 little endian */ > +#define DRM_FORMAT_XVUY2101010 fourcc_code('X', 'V', '3', '0') /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */ > + > +/* Grey scale */ > +#define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit packed greyscale Y:Y:Y:Y 8:8:8:8 */ > +#define DRM_FORMAT_Y10 fourcc_code('Y', '1', '0', ' ') /* 10-bit packed greyscale X:Y:Y:Y 2:10:10:10 */ These don't make sense to me. What does it even mean to have Y replicated three or four times for each pixel? > > /* > * 2 plane RGB + A > @@ -140,6 +147,8 @@ extern "C" { > #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 */ > +#define DRM_FORMAT_XV15 fourcc_code('X', 'V', '1', '5') /* 2x2 subsampled Cr:Cb plane 2:10:10:10 */ > +#define DRM_FORMAT_XV20 fourcc_code('X', 'V', '2', '0') /* 2x1 subsampled Cr:Cb plane 2:10:10:10 */ Huh. Where are the Cb:Cr in that 2:10:10:10 layout? Better docs please. > > /* > * 3 plane YCbCr > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > Sent: Tuesday, November 28, 2017 6:44 AM > To: Hyun Kwon <hyunk@xilinx.com> > Cc: Daniel Vetter <daniel.vetter@intel.com>; Jani Nikula > <jani.nikula@linux.intel.com>; Sean Paul <seanpaul@chromium.org>; David > Airlie <airlied@linux.ie>; dri-devel@lists.freedesktop.org; > monstr@monstr.eu; Laurent Pinchart > <laurent.pinchart@ideasonboard.com>; Satish Kumar Nagireddy > <SATISHNA@xilinx.com>; Jeff Mouroux <jmouroux@xilinx.com> > Subject: Re: [PATCH 2/3] uapi: drm: New fourcc codes needed by Xilinx > Video IP > > On Mon, Nov 27, 2017 at 06:27:32PM -0800, Hyun Kwon wrote: > > From: Jeffrey Mouroux <jmouroux@xilinx.com> > > > > The Xilinx Video Mixer and Xilinx Video Framebuffer DMA IP > > support video memory formats that are not represented in the > > current DRM fourcc library. This patch adds those missing > > fourcc codes. > > > > Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com> > > Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > --- > > include/uapi/drm/drm_fourcc.h | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/include/uapi/drm/drm_fourcc.h > b/include/uapi/drm/drm_fourcc.h > > index 3ad838d..83806d5 100644 > > --- a/include/uapi/drm/drm_fourcc.h > > +++ b/include/uapi/drm/drm_fourcc.h > > @@ -112,6 +112,13 @@ extern "C" { > > #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') > /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */ > > > > #define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') > /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ > > +#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] > Cr:Cb:Y little endian */ > > +#define DRM_FORMAT_XVUY8888 fourcc_code('X', 'V', '2', '4') /* [31:0] > x:Cr:Cb:Y 8:8:8:8 little endian */ > > +#define DRM_FORMAT_XVUY2101010 fourcc_code('X', 'V', '3', '0') > /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */ > > + > > +/* Grey scale */ > > +#define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit > packed greyscale Y:Y:Y:Y 8:8:8:8 */ > > +#define DRM_FORMAT_Y10 fourcc_code('Y', '1', '0', ' ') /* > 10-bit packed greyscale X:Y:Y:Y 2:10:10:10 */ > > These don't make sense to me. What does it even mean to have Y > replicated three or four times for each pixel? Each pixel has one Y component. The comment is to describe how components are stored in 32bit, but I agree it's confusing. Will describe better. > > > > > /* > > * 2 plane RGB + A > > @@ -140,6 +147,8 @@ extern "C" { > > #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 */ > > +#define DRM_FORMAT_XV15 fourcc_code('X', 'V', '1', '5') > /* 2x2 subsampled Cr:Cb plane 2:10:10:10 */ > > +#define DRM_FORMAT_XV20 fourcc_code('X', 'V', '2', '0') > /* 2x1 subsampled Cr:Cb plane 2:10:10:10 */ > > Huh. Where are the Cb:Cr in that 2:10:10:10 layout? > > Better docs please. Sure. Will add more details on how components are laid out. Thanks, -hyun > > > > > /* > > * 3 plane YCbCr > > -- > > 2.7.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC
On Tue, Nov 28, 2017 at 05:25:46PM +0000, Hyun Kwon wrote: > > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > > Sent: Tuesday, November 28, 2017 6:44 AM > > To: Hyun Kwon <hyunk@xilinx.com> > > Cc: Daniel Vetter <daniel.vetter@intel.com>; Jani Nikula > > <jani.nikula@linux.intel.com>; Sean Paul <seanpaul@chromium.org>; David > > Airlie <airlied@linux.ie>; dri-devel@lists.freedesktop.org; > > monstr@monstr.eu; Laurent Pinchart > > <laurent.pinchart@ideasonboard.com>; Satish Kumar Nagireddy > > <SATISHNA@xilinx.com>; Jeff Mouroux <jmouroux@xilinx.com> > > Subject: Re: [PATCH 2/3] uapi: drm: New fourcc codes needed by Xilinx > > Video IP > > > > On Mon, Nov 27, 2017 at 06:27:32PM -0800, Hyun Kwon wrote: > > > From: Jeffrey Mouroux <jmouroux@xilinx.com> > > > > > > The Xilinx Video Mixer and Xilinx Video Framebuffer DMA IP > > > support video memory formats that are not represented in the > > > current DRM fourcc library. This patch adds those missing > > > fourcc codes. > > > > > > Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com> > > > Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > > --- > > > include/uapi/drm/drm_fourcc.h | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h > > b/include/uapi/drm/drm_fourcc.h > > > index 3ad838d..83806d5 100644 > > > --- a/include/uapi/drm/drm_fourcc.h > > > +++ b/include/uapi/drm/drm_fourcc.h > > > @@ -112,6 +112,13 @@ extern "C" { > > > #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') > > /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */ > > > > > > #define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') > > /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ > > > +#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] > > Cr:Cb:Y little endian */ > > > +#define DRM_FORMAT_XVUY8888 fourcc_code('X', 'V', '2', '4') /* [31:0] > > x:Cr:Cb:Y 8:8:8:8 little endian */ > > > +#define DRM_FORMAT_XVUY2101010 fourcc_code('X', 'V', '3', '0') > > /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */ > > > + > > > +/* Grey scale */ > > > +#define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit > > packed greyscale Y:Y:Y:Y 8:8:8:8 */ > > > +#define DRM_FORMAT_Y10 fourcc_code('Y', '1', '0', ' ') /* > > 10-bit packed greyscale X:Y:Y:Y 2:10:10:10 */ > > > > These don't make sense to me. What does it even mean to have Y > > replicated three or four times for each pixel? > > Each pixel has one Y component. The comment is to describe how components are stored in 32bit, but I agree it's confusing. Will describe better. For the 8 bit Y format you should then define it as 8 bits. Unless of course it really is defined as a 32bit word containing 4 pixels. The 10 bit case would be even funky since there would have to be two bits of padding after every 3 pixels. So are these really defined as 32 bit wits 3 or 4 pixels and potentially a few bits of extra padding packed into each word? That seems rather nuts to me because you can't even byte address each pixel. I think such crazyness has no business living right next to the sane formats, hence we should put all these into their own little section of the header, and the names should somehow reflect that they are in fact "special".
> -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > Sent: Tuesday, November 28, 2017 10:12 AM > To: Hyun Kwon <hyunk@xilinx.com> > Cc: Daniel Vetter <daniel.vetter@intel.com>; Jani Nikula > <jani.nikula@linux.intel.com>; Sean Paul <seanpaul@chromium.org>; David > Airlie <airlied@linux.ie>; dri-devel@lists.freedesktop.org; monstr@monstr.eu; > Laurent Pinchart <laurent.pinchart@ideasonboard.com>; Satish Kumar > Nagireddy <SATISHNA@xilinx.com>; Jeff Mouroux <jmouroux@xilinx.com> > Subject: Re: [PATCH 2/3] uapi: drm: New fourcc codes needed by Xilinx Video IP > > On Tue, Nov 28, 2017 at 05:25:46PM +0000, Hyun Kwon wrote: > > > > > > > -----Original Message----- > > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > > > Sent: Tuesday, November 28, 2017 6:44 AM > > > To: Hyun Kwon <hyunk@xilinx.com> > > > Cc: Daniel Vetter <daniel.vetter@intel.com>; Jani Nikula > > > <jani.nikula@linux.intel.com>; Sean Paul <seanpaul@chromium.org>; David > > > Airlie <airlied@linux.ie>; dri-devel@lists.freedesktop.org; > > > monstr@monstr.eu; Laurent Pinchart > > > <laurent.pinchart@ideasonboard.com>; Satish Kumar Nagireddy > > > <SATISHNA@xilinx.com>; Jeff Mouroux <jmouroux@xilinx.com> > > > Subject: Re: [PATCH 2/3] uapi: drm: New fourcc codes needed by Xilinx > > > Video IP > > > > > > On Mon, Nov 27, 2017 at 06:27:32PM -0800, Hyun Kwon wrote: > > > > From: Jeffrey Mouroux <jmouroux@xilinx.com> > > > > > > > > The Xilinx Video Mixer and Xilinx Video Framebuffer DMA IP > > > > support video memory formats that are not represented in the > > > > current DRM fourcc library. This patch adds those missing > > > > fourcc codes. > > > > > > > > Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com> > > > > Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > > > --- > > > > include/uapi/drm/drm_fourcc.h | 9 +++++++++ > > > > 1 file changed, 9 insertions(+) > > > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h > > > b/include/uapi/drm/drm_fourcc.h > > > > index 3ad838d..83806d5 100644 > > > > --- a/include/uapi/drm/drm_fourcc.h > > > > +++ b/include/uapi/drm/drm_fourcc.h > > > > @@ -112,6 +112,13 @@ extern "C" { > > > > #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') > > > /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */ > > > > > > > > #define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') > > > /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ > > > > +#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] > > > Cr:Cb:Y little endian */ > > > > +#define DRM_FORMAT_XVUY8888 fourcc_code('X', 'V', '2', '4') /* > [31:0] > > > x:Cr:Cb:Y 8:8:8:8 little endian */ > > > > +#define DRM_FORMAT_XVUY2101010 fourcc_code('X', 'V', '3', '0') > > > /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */ > > > > + > > > > +/* Grey scale */ > > > > +#define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* > 8-bit > > > packed greyscale Y:Y:Y:Y 8:8:8:8 */ > > > > +#define DRM_FORMAT_Y10 fourcc_code('Y', '1', '0', ' ') /* > > > 10-bit packed greyscale X:Y:Y:Y 2:10:10:10 */ > > > > > > These don't make sense to me. What does it even mean to have Y > > > replicated three or four times for each pixel? > > > > Each pixel has one Y component. The comment is to describe how > components are stored in 32bit, but I agree it's confusing. Will describe better. > > For the 8 bit Y format you should then define it as 8 bits. Unless of > course it really is defined as a 32bit word containing 4 pixels. The > 10 bit case would be even funky since there would have to be two > bits of padding after every 3 pixels. > > So are these really defined as 32 bit wits 3 or 4 pixels and potentially > a few bits of extra padding packed into each word? That seems rather > nuts to me because you can't even byte address each pixel. > > I think such crazyness has no business living right next to the > sane formats, hence we should put all these into their own little > section of the header, and the names should somehow reflect that > they are in fact "special". Make sense to have a separate section for this kind of formats. Repeating the cover letter message for more details: --- This series is to add new drm formats needed by some Xilinx IPs. Some formats have unique characteristics such as pixels not being byte-aligned. For instance, some 10bit formats have 2bit padding after every 3-10bit components: 32b[0]: 10b comp0 - 10b comp1 - 10b comp2 - 2b padding 32b[1]: 10b comp3 - 10b comp4 - 10b comp5 - 2b padding ... To model this, additional information is added to struct drm_format_info. The patch has been tested with downstream drivers as well as the downstream user space component (ex, modified modetest). --- Thanks, -hyun > > -- > Ville Syrjälä > Intel OTC
Hi Ville, On Tuesday, 28 November 2017 20:11:42 EET Ville Syrjälä wrote: > On Tue, Nov 28, 2017 at 05:25:46PM +0000, Hyun Kwon wrote: > > On Tuesday, November 28, 2017 6:44 AM Ville Syrjälä wrote: > >> On Mon, Nov 27, 2017 at 06:27:32PM -0800, Hyun Kwon wrote: > >>> From: Jeffrey Mouroux <jmouroux@xilinx.com> > >>> > >>> The Xilinx Video Mixer and Xilinx Video Framebuffer DMA IP > >>> support video memory formats that are not represented in the > >>> current DRM fourcc library. This patch adds those missing > >>> fourcc codes. > >>> > >>> Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com> > >>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > >>> --- > >>> > >>> include/uapi/drm/drm_fourcc.h | 9 +++++++++ > >>> 1 file changed, 9 insertions(+) > >>> > >>> diff --git a/include/uapi/drm/drm_fourcc.h > >>> b/include/uapi/drm/drm_fourcc.h > >>> index 3ad838d..83806d5 100644 > >>> --- a/include/uapi/drm/drm_fourcc.h > >>> +++ b/include/uapi/drm/drm_fourcc.h > >>> @@ -112,6 +112,13 @@ extern "C" { > >>> > >>> #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') > >>> /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */ > >>> #define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') > >>> /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ > >>> +#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] > >>> Cr:Cb:Y little endian */ > >>> +#define DRM_FORMAT_XVUY8888 fourcc_code('X', 'V', '2', '4') /* [31:0] > >>> x:Cr:Cb:Y 8:8:8:8 little endian */ > >>> +#define DRM_FORMAT_XVUY2101010 fourcc_code('X', 'V', '3', '0') > >>> /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */ > >>> + > >>> +/* Grey scale */ > >>> +#define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit > >>> packed greyscale Y:Y:Y:Y 8:8:8:8 */ > >>> +#define DRM_FORMAT_Y10 fourcc_code('Y', '1', '0', ' ') /* > >>> 10-bit packed greyscale X:Y:Y:Y 2:10:10:10 */ > >> > >> These don't make sense to me. What does it even mean to have Y > >> replicated three or four times for each pixel? > > > > Each pixel has one Y component. The comment is to describe how components > > are stored in 32bit, but I agree it's confusing. Will describe better. > For the 8 bit Y format you should then define it as 8 bits. Unless of > course it really is defined as a 32bit word containing 4 pixels. The > 10 bit case would be even funky since there would have to be two > bits of padding after every 3 pixels. > > So are these really defined as 32 bit wits 3 or 4 pixels and potentially > a few bits of extra padding packed into each word? That seems rather > nuts to me because you can't even byte address each pixel. > > I think such crazyness has no business living right next to the > sane formats, hence we should put all these into their own little > section of the header, and the names should somehow reflect that > they are in fact "special". Thee Y8 format isn't special as it's just a plain 8-bit format. The Y10 format is indeed not byte-addressable, but don't be too fast to dismiss it as crazy, this kind of format is pretty common outside of the desktop graphics world. There are even 10-bit formats with 3 components where the 8 LSBs of each component are stored in three bytes, and the 2 MSBs are grouped in a fourth byte with 2 bits of padding (or possibly with LSB and MSB switched, I can't remember right now).
On Mon, Dec 11, 2017 at 04:08:07PM +0200, Laurent Pinchart wrote: > Hi Ville, > > On Tuesday, 28 November 2017 20:11:42 EET Ville Syrjälä wrote: > > On Tue, Nov 28, 2017 at 05:25:46PM +0000, Hyun Kwon wrote: > > > On Tuesday, November 28, 2017 6:44 AM Ville Syrjälä wrote: > > >> On Mon, Nov 27, 2017 at 06:27:32PM -0800, Hyun Kwon wrote: > > >>> From: Jeffrey Mouroux <jmouroux@xilinx.com> > > >>> > > >>> The Xilinx Video Mixer and Xilinx Video Framebuffer DMA IP > > >>> support video memory formats that are not represented in the > > >>> current DRM fourcc library. This patch adds those missing > > >>> fourcc codes. > > >>> > > >>> Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com> > > >>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > >>> --- > > >>> > > >>> include/uapi/drm/drm_fourcc.h | 9 +++++++++ > > >>> 1 file changed, 9 insertions(+) > > >>> > > >>> diff --git a/include/uapi/drm/drm_fourcc.h > > >>> b/include/uapi/drm/drm_fourcc.h > > >>> index 3ad838d..83806d5 100644 > > >>> --- a/include/uapi/drm/drm_fourcc.h > > >>> +++ b/include/uapi/drm/drm_fourcc.h > > >>> @@ -112,6 +112,13 @@ extern "C" { > > >>> > > >>> #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') > > >>> /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */ > > >>> #define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') > > >>> /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ > > >>> +#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] > > >>> Cr:Cb:Y little endian */ > > >>> +#define DRM_FORMAT_XVUY8888 fourcc_code('X', 'V', '2', '4') /* [31:0] > > >>> x:Cr:Cb:Y 8:8:8:8 little endian */ > > >>> +#define DRM_FORMAT_XVUY2101010 fourcc_code('X', 'V', '3', '0') > > >>> /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */ > > >>> + > > >>> +/* Grey scale */ > > >>> +#define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit > > >>> packed greyscale Y:Y:Y:Y 8:8:8:8 */ > > >>> +#define DRM_FORMAT_Y10 fourcc_code('Y', '1', '0', ' ') /* > > >>> 10-bit packed greyscale X:Y:Y:Y 2:10:10:10 */ > > >> > > >> These don't make sense to me. What does it even mean to have Y > > >> replicated three or four times for each pixel? > > > > > > Each pixel has one Y component. The comment is to describe how components > > > are stored in 32bit, but I agree it's confusing. Will describe better. > > For the 8 bit Y format you should then define it as 8 bits. Unless of > > course it really is defined as a 32bit word containing 4 pixels. The > > 10 bit case would be even funky since there would have to be two > > bits of padding after every 3 pixels. > > > > So are these really defined as 32 bit wits 3 or 4 pixels and potentially > > a few bits of extra padding packed into each word? That seems rather > > nuts to me because you can't even byte address each pixel. > > > > I think such crazyness has no business living right next to the > > sane formats, hence we should put all these into their own little > > section of the header, and the names should somehow reflect that > > they are in fact "special". > > Thee Y8 format isn't special as it's just a plain 8-bit format. Is it? The patch made it out to be something else perhaps. Impossible to judge without reading the relevant xilinx docs I suppose. > The Y10 format > is indeed not byte-addressable, but don't be too fast to dismiss it as crazy, > this kind of format is pretty common outside of the desktop graphics world. > There are even 10-bit formats with 3 components where the 8 LSBs of each > component are stored in three bytes, and the 2 MSBs are grouped in a fourth > byte with 2 bits of padding (or possibly with LSB and MSB switched, I can't > remember right now). So you're saying the formats in this patch aren't quite as crazy as some other formats out there? ;)
Hi Ville, On Monday, 11 December 2017 16:24:32 EET Ville Syrjälä wrote: > On Mon, Dec 11, 2017 at 04:08:07PM +0200, Laurent Pinchart wrote: > > On Tuesday, 28 November 2017 20:11:42 EET Ville Syrjälä wrote: > >> On Tue, Nov 28, 2017 at 05:25:46PM +0000, Hyun Kwon wrote: > >>> On Tuesday, November 28, 2017 6:44 AM Ville Syrjälä wrote: > >>>> On Mon, Nov 27, 2017 at 06:27:32PM -0800, Hyun Kwon wrote: > >>>>> From: Jeffrey Mouroux <jmouroux@xilinx.com> > >>>>> > >>>>> The Xilinx Video Mixer and Xilinx Video Framebuffer DMA IP > >>>>> support video memory formats that are not represented in the > >>>>> current DRM fourcc library. This patch adds those missing > >>>>> fourcc codes. > >>>>> > >>>>> Signed-off-by: Jeffrey Mouroux <jmouroux@xilinx.com> > >>>>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > >>>>> --- > >>>>> > >>>>> include/uapi/drm/drm_fourcc.h | 9 +++++++++ > >>>>> 1 file changed, 9 insertions(+) > >>>>> > >>>>> diff --git a/include/uapi/drm/drm_fourcc.h > >>>>> b/include/uapi/drm/drm_fourcc.h > >>>>> index 3ad838d..83806d5 100644 > >>>>> --- a/include/uapi/drm/drm_fourcc.h > >>>>> +++ b/include/uapi/drm/drm_fourcc.h > >>>>> @@ -112,6 +112,13 @@ extern "C" { > >>>>> > >>>>> #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') > >>>>> > >>>>> /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */ > >>>>> > >>>>> #define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') > >>>>> > >>>>> /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ > >>>>> +#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] > >>>>> Cr:Cb:Y little endian */ > >>>>> +#define DRM_FORMAT_XVUY8888 fourcc_code('X', 'V', '2', '4') /* > >>>>> [31:0] > >>>>> x:Cr:Cb:Y 8:8:8:8 little endian */ > >>>>> +#define DRM_FORMAT_XVUY2101010 fourcc_code('X', 'V', '3', '0') > >>>>> /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */ > >>>>> + > >>>>> +/* Grey scale */ > >>>>> +#define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit > >>>>> packed greyscale Y:Y:Y:Y 8:8:8:8 */ > >>>>> +#define DRM_FORMAT_Y10 fourcc_code('Y', '1', '0', ' ') /* > >>>>> 10-bit packed greyscale X:Y:Y:Y 2:10:10:10 */ > >>>> > >>>> These don't make sense to me. What does it even mean to have Y > >>>> replicated three or four times for each pixel? > >>> > >>> Each pixel has one Y component. The comment is to describe how > >>> components are stored in 32bit, but I agree it's confusing. Will > >>> describe better. > >> > >> For the 8 bit Y format you should then define it as 8 bits. Unless of > >> course it really is defined as a 32bit word containing 4 pixels. The > >> 10 bit case would be even funky since there would have to be two > >> bits of padding after every 3 pixels. > >> > >> So are these really defined as 32 bit wits 3 or 4 pixels and potentially > >> a few bits of extra padding packed into each word? That seems rather > >> nuts to me because you can't even byte address each pixel. > >> > >> I think such crazyness has no business living right next to the > >> sane formats, hence we should put all these into their own little > >> section of the header, and the names should somehow reflect that > >> they are in fact "special". > > > > Thee Y8 format isn't special as it's just a plain 8-bit format. > > Is it? The patch made it out to be something else perhaps. Impossible > to judge without reading the relevant xilinx docs I suppose. What I meant is that despite the presentation of this 8-bit greyscale format as storing 4 pixels in 32 bits, it's really an 8-bit greyscale format without anything special. > > The Y10 format is indeed not byte-addressable, but don't be too fast to > > dismiss it as crazy, this kind of format is pretty common outside of the > > desktop graphics world. There are even 10-bit formats with 3 components > > where the 8 LSBs of each component are stored in three bytes, and the 2 > > MSBs are grouped in a fourth byte with 2 bits of padding (or possibly with > > LSB and MSB switched, I can't remember right now). > > So you're saying the formats in this patch aren't quite as crazy as some > other formats out there? ;) Don't get me started about some of the GPU tiled formats ;-) The graphics and camera industries have taken different paths because they have different needs, but both have invented their share of convoluted formats that are not always easy to support.
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 3ad838d..83806d5 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -112,6 +112,13 @@ extern "C" { #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */ #define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ +#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] Cr:Cb:Y little endian */ +#define DRM_FORMAT_XVUY8888 fourcc_code('X', 'V', '2', '4') /* [31:0] x:Cr:Cb:Y 8:8:8:8 little endian */ +#define DRM_FORMAT_XVUY2101010 fourcc_code('X', 'V', '3', '0') /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */ + +/* Grey scale */ +#define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit packed greyscale Y:Y:Y:Y 8:8:8:8 */ +#define DRM_FORMAT_Y10 fourcc_code('Y', '1', '0', ' ') /* 10-bit packed greyscale X:Y:Y:Y 2:10:10:10 */ /* * 2 plane RGB + A @@ -140,6 +147,8 @@ extern "C" { #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 */ +#define DRM_FORMAT_XV15 fourcc_code('X', 'V', '1', '5') /* 2x2 subsampled Cr:Cb plane 2:10:10:10 */ +#define DRM_FORMAT_XV20 fourcc_code('X', 'V', '2', '0') /* 2x1 subsampled Cr:Cb plane 2:10:10:10 */ /* * 3 plane YCbCr