On Wed, Jul 18, 2018 at 10:12:02AM +0100, Brian Starkey wrote:
> Hi Ayan,
> 
> On Tue, Jul 17, 2018 at 06:13:42PM +0100, Ayan Kumar Halder wrote:
> >A lot of drivers duplicate the function to check if a format is yuv or not.
> >If we add a field (to denote whether the format is yuv or not) in the
> >drm_format_info table, all the drivers can use this field and it will
> >prevent duplication of similar logic.
> 
> This looks like a good idea to me.
> 
> I wonder if the two "has_alpha" and "is_yuv" bools should be bitfields
> to reduce the footprint (not sure what the general DRM attitude to
> bitfields is), but either way:

There are quite a few of these so tighter packing (of the other fields
as well perhaps) might be a decent idea.

For the series (except for the omapdrm patch which I'll leave for Tomi):
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

> 
> Reviewed-by: Brian Starkey <brian.star...@arm.com>
> 
> >
> >Signed-off-by: Ayan Kumar halder <ayan.hal...@arm.com>
> >---
> > drivers/gpu/drm/drm_fourcc.c | 42 +++++++++++++++++++++---------------------
> > include/drm/drm_fourcc.h     |  2 ++
> > 2 files changed, 23 insertions(+), 21 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> >index 5ca6395..35c1e27 100644
> >--- a/drivers/gpu/drm/drm_fourcc.c
> >+++ b/drivers/gpu/drm/drm_fourcc.c
> >@@ -152,27 +152,27 @@ const struct drm_format_info *__drm_format_info(u32 
> >format)
> >             { .format = DRM_FORMAT_XBGR8888_A8,     .depth = 32, 
> > .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = 
> > true },
> >             { .format = DRM_FORMAT_RGBX8888_A8,     .depth = 32, 
> > .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = 
> > true },
> >             { .format = DRM_FORMAT_BGRX8888_A8,     .depth = 32, 
> > .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = 
> > true },
> >-            { .format = DRM_FORMAT_YUV410,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
> >-            { .format = DRM_FORMAT_YVU410,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
> >-            { .format = DRM_FORMAT_YUV411,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
> >-            { .format = DRM_FORMAT_YVU411,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
> >-            { .format = DRM_FORMAT_YUV420,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
> >-            { .format = DRM_FORMAT_YVU420,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
> >-            { .format = DRM_FORMAT_YUV422,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
> >-            { .format = DRM_FORMAT_YVU422,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
> >-            { .format = DRM_FORMAT_YUV444,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
> >-            { .format = DRM_FORMAT_YVU444,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
> >-            { .format = DRM_FORMAT_NV12,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> >-            { .format = DRM_FORMAT_NV21,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> >-            { .format = DRM_FORMAT_NV16,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
> >-            { .format = DRM_FORMAT_NV61,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
> >-            { .format = DRM_FORMAT_NV24,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> >-            { .format = DRM_FORMAT_NV42,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
> >-            { .format = DRM_FORMAT_YUYV,            .depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >-            { .format = DRM_FORMAT_YVYU,            .depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >-            { .format = DRM_FORMAT_UYVY,            .depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >-            { .format = DRM_FORMAT_VYUY,            .depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >-            { .format = DRM_FORMAT_AYUV,            .depth = 0,  
> >.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true 
> >},
> >+            { .format = DRM_FORMAT_YUV410,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YVU410,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YUV411,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YVU411,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YUV420,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YVU420,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YUV422,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YVU422,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YUV444,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YVU444,          .depth = 0,  
> >.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_NV12,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true },
> >+            { .format = DRM_FORMAT_NV21,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true },
> >+            { .format = DRM_FORMAT_NV16,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_NV61,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_NV24,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_NV42,            .depth = 0,  
> >.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YUYV,            .depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_YVYU,            .depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_UYVY,            .depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_VYUY,            .depth = 0,  
> >.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >+            { .format = DRM_FORMAT_AYUV,            .depth = 0,  
> >.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = 
> >true, .is_yuv = true },
> >     };
> >
> >     unsigned int i;
> >diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> >index 3e86408..f9c1584 100644
> >--- a/include/drm/drm_fourcc.h
> >+++ b/include/drm/drm_fourcc.h
> >@@ -39,6 +39,7 @@ struct drm_mode_fb_cmd2;
> >  * @hsub: Horizontal chroma subsampling factor
> >  * @vsub: Vertical chroma subsampling factor
> >  * @has_alpha: Does the format embeds an alpha component?
> >+ * @is_yuv: Is it a YUV format?
> >  */
> > struct drm_format_info {
> >     u32 format;
> >@@ -48,6 +49,7 @@ struct drm_format_info {
> >     u8 hsub;
> >     u8 vsub;
> >     bool has_alpha;
> >+    bool is_yuv;
> > };
> >
> > /**
> >-- 
> >2.7.4
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to