[PATCH v4 01/14] drm: Centralize format information
Hi Tomi,
On Friday 16 Sep 2016 12:44:31 Tomi Valkeinen wrote:
> On 16/09/16 02:30, Laurent Pinchart wrote:
> > I've checked the existing code that this patch series is replacing, and
> > the [ARGB]{4} formats are currently reported as having a depth of 32.
> > I'm not sure why that's the case, but I'd rather not touch it in this
> > patch. If this is a bug we should fix it in a separate patch.
>
> I agree, I don't want this series to be held up. But this depth is
> clearly broken, in some way or another. Even more reason to move it to
> fb code =).
And to then drop fbdev ;-)
> >>> I'm not sure if it's worth the hassle, but if the depth is only for
> >>> fbdev compat code, maybe a separate format->depth table in fbdev code
> >>> (for the formats fbdev supports) would make this cleaner and avoid any
> >>> future mistakes with new drm drivers.
> >>
> >> I agree actually, having it here will encourage anyone to use it. If you
> >> don't want to split it out, at least add a comment along the lines of
> >> your reply:
> >>
> This is used to implement the fbdev compatibility code, as fbdev
> (unlike kms) makes use of that information.
> >
> > I've double-checked the existing usage of the depth value, and it turns
> > out that quite a few drivers still use it for various purpose, through
> > struct drm_framebuffer.depth. I thus wonder whether splitting the depth
> > value from the format information table would really help, as drivers
> > would have a way to access it anyway.
>
> Ok.
>
> Btw, are you sure alpha is not counted into depth? With a quick glance,
> also drm_mode_legacy_fb_format() seems to expect depth to include alpha.
>
> I'm just thinking here that if "depth" is not clearly defined anywhere,
> and the most common formats, 24bit RGB formats, are defined with depth
> including alpha, well, maybe then that's how depth should be defined.
>
> Then again, I had a quick glance at the fbdev code, and
> fb_get_color_depth() suggests that alpha is not counted in...
I think depth is just ill-defined and shouldn't be used in drivers anymore, at
least certainly not by new code.
--
Regards,
Laurent Pinchart
[PATCH v4 01/14] drm: Centralize format information
On 16/09/16 02:30, Laurent Pinchart wrote:
> I've checked the existing code that this patch series is replacing, and the
> [ARGB]{4} formats are currently reported as having a depth of 32. I'm not
> sure why that's the case, but I'd rather not touch it in this patch. If this
> is a bug we should fix it in a separate patch.
I agree, I don't want this series to be held up. But this depth is
clearly broken, in some way or another. Even more reason to move it to
fb code =).
>>> I'm not sure if it's worth the hassle, but if the depth is only for
>>> fbdev compat code, maybe a separate format->depth table in fbdev code
>>> (for the formats fbdev supports) would make this cleaner and avoid any
>>> future mistakes with new drm drivers.
>>
>> I agree actually, having it here will encourage anyone to use it. If you
>> don't want to split it out, at least add a comment along the lines of
>> your reply:
>>
This is used to implement the fbdev compatibility code, as fbdev (unlike
kms) makes use of that information.
>
> I've double-checked the existing usage of the depth value, and it turns out
> that quite a few drivers still use it for various purpose, through struct
> drm_framebuffer.depth. I thus wonder whether splitting the depth value from
> the format information table would really help, as drivers would have a way
> to
> access it anyway.
Ok.
Btw, are you sure alpha is not counted into depth? With a quick glance,
also drm_mode_legacy_fb_format() seems to expect depth to include alpha.
I'm just thinking here that if "depth" is not clearly defined anywhere,
and the most common formats, 24bit RGB formats, are defined with depth
including alpha, well, maybe then that's how depth should be defined.
Then again, I had a quick glance at the fbdev code, and
fb_get_color_depth() suggests that alpha is not counted in...
Tomi
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL:
[PATCH v4 01/14] drm: Centralize format information
Hello,
On Thursday 15 Sep 2016 17:12:12 Eric Engestrom wrote:
> On Thu, Sep 15, 2016 at 09:22:54AM +0300, Tomi Valkeinen wrote:
> > On 15/09/16 01:22, Laurent Pinchart wrote:
> >> No, the depth value is the number of colour bits, excluding the alpha
> >> bits. This is used to implement the fbdev compatibility code, as fbdev
> >> (unlike kms) makes use of that information.
> >>
> >> The total number of bits per pixel is not stored in the table as it can
> >> be computed by cpp[0]*8 + (cpp[1]+cpp[2])*8/hsub/vsub. For RGB formats,
> >> which are the only formats supported by the existing
> >> drm_fb_get_bpp_depth() function, this simplifies to just cpp[0] * 8.
> >
> > Ok. Then the ARGB & co. formats have depth wrong. I presumed those
> > were right as they're the "normal" ones =).
>
> Good catch, these should be 24 not 32.
> I must admit I kinda skipped over that table the first time, and only
> checked a few random values.
> I just checked the whole table, and all the C and RGB formats are all
> good (with the 4 /[ARGB]{4}/ formats set to .depth=24), and all the
> YUV formats I know (~3/4) are good, but I don't know them all :)
I've checked the existing code that this patch series is replacing, and the
[ARGB]{4} formats are currently reported as having a depth of 32. I'm not
sure why that's the case, but I'd rather not touch it in this patch. If this
is a bug we should fix it in a separate patch.
> > I'm not sure if it's worth the hassle, but if the depth is only for
> > fbdev compat code, maybe a separate format->depth table in fbdev code
> > (for the formats fbdev supports) would make this cleaner and avoid any
> > future mistakes with new drm drivers.
>
> I agree actually, having it here will encourage anyone to use it. If you
> don't want to split it out, at least add a comment along the lines of
> your reply:
>
> >> This is used to implement the fbdev compatibility code, as fbdev (unlike
> >> kms) makes use of that information.
I've double-checked the existing usage of the depth value, and it turns out
that quite a few drivers still use it for various purpose, through struct
drm_framebuffer.depth. I thus wonder whether splitting the depth value from
the format information table would really help, as drivers would have a way to
access it anyway.
--
Regards,
Laurent Pinchart
[PATCH v4 01/14] drm: Centralize format information
On Thu, Sep 15, 2016 at 09:22:54AM +0300, Tomi Valkeinen wrote:
> On 15/09/16 01:22, Laurent Pinchart wrote:
> > No, the depth value is the number of colour bits, excluding the alpha bits.
> > This is used to implement the fbdev compatibility code, as fbdev (unlike
> > kms)
> > makes use of that information.
> >
> > The total number of bits per pixel is not stored in the table as it can be
> > computed by cpp[0]*8 + (cpp[1]+cpp[2])*8/hsub/vsub. For RGB formats, which
> > are
> > the only formats supported by the existing drm_fb_get_bpp_depth() function,
> > this simplifies to just cpp[0] * 8.
>
> Ok. Then the ARGB & co. formats have depth wrong. I presumed those
> were right as they're the "normal" ones =).
Good catch, these should be 24 not 32.
I must admit I kinda skipped over that table the first time, and only
checked a few random values.
I just checked the whole table, and all the C and RGB formats are all
good (with the 4 /[ARGB]{4}/ formats set to .depth=24), and all the
YUV formats I know (~3/4) are good, but I don't know them all :)
>
> I'm not sure if it's worth the hassle, but if the depth is only for
> fbdev compat code, maybe a separate format->depth table in fbdev code
> (for the formats fbdev supports) would make this cleaner and avoid any
> future mistakes with new drm drivers.
I agree actually, having it here will encourage anyone to use it. If you
don't want to split it out, at least add a comment along the lines of
your reply:
> > This is used to implement the fbdev compatibility code, as fbdev (unlike
> > kms)
> > makes use of that information.
Cheers,
Eric
[PATCH v4 01/14] drm: Centralize format information
On 15/09/16 01:22, Laurent Pinchart wrote:
> Hi Tomi,
>
> Thank you for the review.
>
> On Wednesday 14 Sep 2016 14:49:26 Tomi Valkeinen wrote:
>> On 08/09/16 17:44, Laurent Pinchart wrote:
>>> Various pieces of information about DRM formats (number of planes, color
>>> depth, chroma subsampling, ...) are scattered across different helper
>>> functions in the DRM core. Callers of those functions often need to
>>> access more than a single parameter of the format, leading to
>>> inefficiencies due to multiple lookups.
>>>
>>> Centralize all format information in a data structure and create a
>>> function to look up information based on the format 4CC.
>>>
>>> Signed-off-by: Laurent Pinchart
>>> ---
>>>
>>> Documentation/gpu/drm-kms.rst | 3 ++
>>> drivers/gpu/drm/drm_fourcc.c | 84 ++
>>> include/drm/drm_fourcc.h | 19 ++
>>> 3 files changed, 106 insertions(+)
>
> [snip]
>
>>> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
>>> index 29c56b4331e0..6b91bd8a510d 100644
>>> --- a/drivers/gpu/drm/drm_fourcc.c
>>> +++ b/drivers/gpu/drm/drm_fourcc.c
>>> @@ -103,6 +103,90 @@ char *drm_get_format_name(uint32_t format)
>>> EXPORT_SYMBOL(drm_get_format_name);
>>>
>>> /**
>>> + * drm_format_info - query information for a given format
>>> + * @format: pixel format (DRM_FORMAT_*)
>>> + *
>>> + * Returns:
>>> + * The instance of struct drm_format_info that describes the pixel
>>> format, or
>>> + * NULL if the format is unsupported.
>>> + */
>>> +const struct drm_format_info *drm_format_info(u32 format)
>>> +{
>>> + static const struct drm_format_info formats[] = {
>>> + { .format = DRM_FORMAT_C8, .depth = 8,
>>> .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_RGB332, .depth = 8,
>>> .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_BGR233, .depth = 8,
>>> .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_XRGB,.depth = 12,
>>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_XBGR,.depth = 12,
>>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_RGBX,.depth = 12,
>>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_BGRX,.depth = 12,
>>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_ARGB,.depth = 12,
>>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_ABGR,.depth = 12,
>>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_RGBA,.depth = 12,
>>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>>> + { .format = DRM_FORMAT_BGRA,.depth = 12,
>>> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>>
>> Aren't these 16 bit deep modes? 4+4+4+4 = 16.
>
> No, the depth value is the number of colour bits, excluding the alpha bits.
> This is used to implement the fbdev compatibility code, as fbdev (unlike kms)
> makes use of that information.
>
> The total number of bits per pixel is not stored in the table as it can be
> computed by cpp[0]*8 + (cpp[1]+cpp[2])*8/hsub/vsub. For RGB formats, which
> are
> the only formats supported by the existing drm_fb_get_bpp_depth() function,
> this simplifies to just cpp[0] * 8.
Ok. Then the ARGB & co. formats have depth wrong. I presumed those
were right as they're the "normal" ones =).
I'm not sure if it's worth the hassle, but if the depth is only for
fbdev compat code, maybe a separate format->depth table in fbdev code
(for the formats fbdev supports) would make this cleaner and avoid any
future mistakes with new drm drivers.
>>> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
>>> index 30c30fa87ee8..4e79d6159856 100644
>>> --- a/include/drm/drm_fourcc.h
>>> +++ b/include/drm/drm_fourcc.h
>>> @@ -25,6 +25,25 @@
>>> #include
>>> #include
>>>
>>> +/**
>>> + * struct drm_format_info - information about a DRM format
>>> + * @format: 4CC format identifier (DRM_FORMAT_*)
>>> + * @depth: color depth (number of bits per pixel excluding padding bits)
>>
>> Depth is zero for yuv formats. Maybe that should be made clear here.
>
> How about
>
> "color depth (number of bits per pixel excluding padding and alpha bits),
> valid for RGB formats only" ?
Yes, that makes it clear.
Tomi
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL:
[PATCH v4 01/14] drm: Centralize format information
Hi Tomi,
Thank you for the review.
On Wednesday 14 Sep 2016 14:49:26 Tomi Valkeinen wrote:
> On 08/09/16 17:44, Laurent Pinchart wrote:
> > Various pieces of information about DRM formats (number of planes, color
> > depth, chroma subsampling, ...) are scattered across different helper
> > functions in the DRM core. Callers of those functions often need to
> > access more than a single parameter of the format, leading to
> > inefficiencies due to multiple lookups.
> >
> > Centralize all format information in a data structure and create a
> > function to look up information based on the format 4CC.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> >
> > Documentation/gpu/drm-kms.rst | 3 ++
> > drivers/gpu/drm/drm_fourcc.c | 84 ++
> > include/drm/drm_fourcc.h | 19 ++
> > 3 files changed, 106 insertions(+)
[snip]
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index 29c56b4331e0..6b91bd8a510d 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -103,6 +103,90 @@ char *drm_get_format_name(uint32_t format)
> > EXPORT_SYMBOL(drm_get_format_name);
> >
> > /**
> > + * drm_format_info - query information for a given format
> > + * @format: pixel format (DRM_FORMAT_*)
> > + *
> > + * Returns:
> > + * The instance of struct drm_format_info that describes the pixel
> > format, or
> > + * NULL if the format is unsupported.
> > + */
> > +const struct drm_format_info *drm_format_info(u32 format)
> > +{
> > + static const struct drm_format_info formats[] = {
> > + { .format = DRM_FORMAT_C8, .depth = 8,
> > .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_RGB332, .depth = 8,
> > .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_BGR233, .depth = 8,
> > .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_XRGB,.depth = 12,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_XBGR,.depth = 12,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_RGBX,.depth = 12,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_BGRX,.depth = 12,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_ARGB,.depth = 12,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_ABGR,.depth = 12,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_RGBA,.depth = 12,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_BGRA,.depth = 12,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>
> Aren't these 16 bit deep modes? 4+4+4+4 = 16.
No, the depth value is the number of colour bits, excluding the alpha bits.
This is used to implement the fbdev compatibility code, as fbdev (unlike kms)
makes use of that information.
The total number of bits per pixel is not stored in the table as it can be
computed by cpp[0]*8 + (cpp[1]+cpp[2])*8/hsub/vsub. For RGB formats, which are
the only formats supported by the existing drm_fb_get_bpp_depth() function,
this simplifies to just cpp[0] * 8.
Same for the other formats below.
> > + { .format = DRM_FORMAT_XRGB1555,.depth = 15,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_XBGR1555,.depth = 15,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_RGBX5551,.depth = 15,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_BGRX5551,.depth = 15,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_ARGB1555,.depth = 15,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_ABGR1555,.depth = 15,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_RGBA5551,.depth = 15,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_BGRA5551,.depth = 15,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>
> Aren't these 16 bit deep modes? 5+5+5+1 = 16.
>
> > + { .format = DRM_FORMAT_RGB565, .depth = 16,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_BGR565, .depth = 16,
> > .num_planes = 1, .cpp = { 2, 0, 0 },
[PATCH v4 01/14] drm: Centralize format information
On 08/09/16 17:44, Laurent Pinchart wrote:
> Various pieces of information about DRM formats (number of planes, color
> depth, chroma subsampling, ...) are scattered across different helper
> functions in the DRM core. Callers of those functions often need to
> access more than a single parameter of the format, leading to
> inefficiencies due to multiple lookups.
>
> Centralize all format information in a data structure and create a
> function to look up information based on the format 4CC.
>
> Signed-off-by: Laurent Pinchart
> ---
> Documentation/gpu/drm-kms.rst | 3 ++
> drivers/gpu/drm/drm_fourcc.c | 84
> +++
> include/drm/drm_fourcc.h | 19 ++
> 3 files changed, 106 insertions(+)
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index f9a991bb87d4..85c4c49f4436 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -63,6 +63,9 @@ Frame Buffer Functions Reference
> DRM Format Handling
> ===
>
> +.. kernel-doc:: include/drm/drm_fourcc.h
> + :internal:
> +
> .. kernel-doc:: drivers/gpu/drm/drm_fourcc.c
> :export:
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 29c56b4331e0..6b91bd8a510d 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -103,6 +103,90 @@ char *drm_get_format_name(uint32_t format)
> EXPORT_SYMBOL(drm_get_format_name);
>
> /**
> + * drm_format_info - query information for a given format
> + * @format: pixel format (DRM_FORMAT_*)
> + *
> + * Returns:
> + * The instance of struct drm_format_info that describes the pixel format, or
> + * NULL if the format is unsupported.
> + */
> +const struct drm_format_info *drm_format_info(u32 format)
> +{
> + static const struct drm_format_info formats[] = {
> + { .format = DRM_FORMAT_C8, .depth = 8,
> .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGB332, .depth = 8,
> .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGR233, .depth = 8,
> .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_XRGB,.depth = 12,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_XBGR,.depth = 12,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGBX,.depth = 12,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGRX,.depth = 12,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_ARGB,.depth = 12,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_ABGR,.depth = 12,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGBA,.depth = 12,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGRA,.depth = 12,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
Aren't these 16 bit deep modes? 4+4+4+4 = 16.
> + { .format = DRM_FORMAT_XRGB1555,.depth = 15,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_XBGR1555,.depth = 15,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGBX5551,.depth = 15,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGRX5551,.depth = 15,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_ARGB1555,.depth = 15,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_ABGR1555,.depth = 15,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGBA5551,.depth = 15,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGRA5551,.depth = 15,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
Aren't these 16 bit deep modes? 5+5+5+1 = 16.
> + { .format = DRM_FORMAT_RGB565, .depth = 16,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGR565, .depth = 16,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGB888, .depth = 24,
> .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGR888, .depth = 24,
> .num_planes = 1, .cpp = { 3, 0,
[PATCH v4 01/14] drm: Centralize format information
Various pieces of information about DRM formats (number of planes, color
depth, chroma subsampling, ...) are scattered across different helper
functions in the DRM core. Callers of those functions often need to
access more than a single parameter of the format, leading to
inefficiencies due to multiple lookups.
Centralize all format information in a data structure and create a
function to look up information based on the format 4CC.
Signed-off-by: Laurent Pinchart
---
Documentation/gpu/drm-kms.rst | 3 ++
drivers/gpu/drm/drm_fourcc.c | 84 +++
include/drm/drm_fourcc.h | 19 ++
3 files changed, 106 insertions(+)
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index f9a991bb87d4..85c4c49f4436 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -63,6 +63,9 @@ Frame Buffer Functions Reference
DRM Format Handling
===
+.. kernel-doc:: include/drm/drm_fourcc.h
+ :internal:
+
.. kernel-doc:: drivers/gpu/drm/drm_fourcc.c
:export:
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 29c56b4331e0..6b91bd8a510d 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -103,6 +103,90 @@ char *drm_get_format_name(uint32_t format)
EXPORT_SYMBOL(drm_get_format_name);
/**
+ * drm_format_info - query information for a given format
+ * @format: pixel format (DRM_FORMAT_*)
+ *
+ * Returns:
+ * The instance of struct drm_format_info that describes the pixel format, or
+ * NULL if the format is unsupported.
+ */
+const struct drm_format_info *drm_format_info(u32 format)
+{
+ static const struct drm_format_info formats[] = {
+ { .format = DRM_FORMAT_C8, .depth = 8,
.num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGB332, .depth = 8,
.num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_BGR233, .depth = 8,
.num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_XRGB,.depth = 12,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_XBGR,.depth = 12,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGBX,.depth = 12,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_BGRX,.depth = 12,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_ARGB,.depth = 12,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_ABGR,.depth = 12,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGBA,.depth = 12,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_BGRA,.depth = 12,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_XRGB1555,.depth = 15,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_XBGR1555,.depth = 15,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGBX5551,.depth = 15,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_BGRX5551,.depth = 15,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_ARGB1555,.depth = 15,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_ABGR1555,.depth = 15,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGBA5551,.depth = 15,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_BGRA5551,.depth = 15,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGB565, .depth = 16,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_BGR565, .depth = 16,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGB888, .depth = 24,
.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_BGR888, .depth = 24,
.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_XRGB,.depth = 24,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_XBGR,.depth = 24,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub =
