[PATCH] drm/arm/hdlcd: Replace struct simplefb_format with custom type
Thomas Zimmermann
tzimmermann at suse.de
Tue Jun 10 07:07:07 UTC 2025
Hi
Am 09.06.25 um 12:17 schrieb Liviu Dudau:
> On Tue, May 27, 2025 at 11:42:57AM +0200, Thomas Zimmermann wrote:
>> Map DRM FourCC codes to pixel descriptions with internal type struct
>> hdlcd_format. Reorder formats by preference. Avoid simplefb's struct
>> simplefb_format, which is for parsing "simple-framebuffer" DT nodes.
>>
>> The HDLCD drivers uses struct simplefb_format and its default
>> initializer SIMPLEFB_FORMATS to map DRM_FORMAT_ constants to pixel
>> descriptions. The simplefb helpers are for parsing "simple-framebuffer"
>> DT nodes and should be avoided in other context. Therefore replace
>> it in hdlcd with the custom type struct hdlcd_format and the pixel
>> descriptions from PIXEL_FORMAT_ constants.
>>
>> Plane formats exported to userspace are roughly sorted as preferred
>> by hardware and/or driver. SIMPLEFB_FORMATS currently puts 16-bit
>> formats to the top of the list. Changing to struct hdlcd_format
>> allows for reordering the format list. 32-bit formats are now the
>> preferred ones.
>>
>> This change also removes including <linux/platform_data/simplefb.h>,
>> which includes several unrelated headers, such as <linux/fb.h>.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>> drivers/gpu/drm/arm/hdlcd_crtc.c | 32 +++++++++++++++++++++++---------
>> include/video/pixel_format.h | 15 +++++++++++++++
>> 2 files changed, 38 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
>> index 3cfefadc7c9d..6fabe65ec0a2 100644
>> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
>> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
>> @@ -11,8 +11,8 @@
>>
>> #include <linux/clk.h>
>> #include <linux/of_graph.h>
>> -#include <linux/platform_data/simplefb.h>
>>
>> +#include <video/pixel_format.h>
>> #include <video/videomode.h>
>>
>> #include <drm/drm_atomic.h>
>> @@ -28,6 +28,25 @@
>> #include "hdlcd_drv.h"
>> #include "hdlcd_regs.h"
>>
>> +struct hdlcd_format {
>> + u32 fourcc;
>> + struct pixel_format pixel;
>> +};
>> +
>> +static const struct hdlcd_format supported_formats[] = {
>> + { DRM_FORMAT_XRGB8888, PIXEL_FORMAT_XRGB8888 },
>> + { DRM_FORMAT_ARGB8888, PIXEL_FORMAT_ARGB8888 },
>> + { DRM_FORMAT_XBGR8888, PIXEL_FORMAT_XBGR8888 },
>> + { DRM_FORMAT_ABGR8888, PIXEL_FORMAT_ABGR8888 },
>> + { DRM_FORMAT_XRGB2101010, PIXEL_FORMAT_XRGB2101010},
>> + { DRM_FORMAT_ARGB2101010, PIXEL_FORMAT_ARGB2101010},
>> + { DRM_FORMAT_RGB565, PIXEL_FORMAT_RGB565 },
>> + { DRM_FORMAT_RGBA5551, PIXEL_FORMAT_RGBA5551 },
>> + { DRM_FORMAT_XRGB1555, PIXEL_FORMAT_XRGB1555 },
>> + { DRM_FORMAT_ARGB1555, PIXEL_FORMAT_ARGB1555 },
>> + { DRM_FORMAT_RGB888, PIXEL_FORMAT_RGB888 },
>> +};
>> +
>> /*
>> * The HDLCD controller is a dumb RGB streamer that gets connected to
>> * a single HDMI transmitter or in the case of the ARM Models it gets
>> @@ -73,8 +92,6 @@ static const struct drm_crtc_funcs hdlcd_crtc_funcs = {
>> .disable_vblank = hdlcd_crtc_disable_vblank,
>> };
>>
>> -static struct simplefb_format supported_formats[] = SIMPLEFB_FORMATS;
> Sorry, I was on holiday when you've sent the patch and it fell to the bottom of the pile.
No worries.
>
>
> When I did the initial patch for HDLCD using the SIMPLEFB_FORMATS was convenient as I
> didn't had to type all the "supported" formats, even if the one carrying the alpha
> channel were ignored (HDLCD only has one plane). If we're going to move the supported
> formats in this file I would suggest trimming it down to remove all the alpha-channel
> formats as they are pointless to list as supported. If there is no other user of the
> formats added in pixel_format.h then that should slim down the patch considerably.
That's even better. I suspected that not all formats would be supported
by hdlcd. I'll prepare an update then.
Best regards
Thomas
>
> Best regards,
> Liviu
>
>> -
>> /*
>> * Setup the HDLCD registers for decoding the pixels out of the framebuffer
>> */
>> @@ -83,15 +100,12 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
>> unsigned int btpp;
>> struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
>> const struct drm_framebuffer *fb = crtc->primary->state->fb;
>> - uint32_t pixel_format;
>> - struct simplefb_format *format = NULL;
>> + const struct pixel_format *format = NULL;
>> int i;
>>
>> - pixel_format = fb->format->format;
>> -
>> for (i = 0; i < ARRAY_SIZE(supported_formats); i++) {
>> - if (supported_formats[i].fourcc == pixel_format)
>> - format = &supported_formats[i];
>> + if (supported_formats[i].fourcc == fb->format->format)
>> + format = &supported_formats[i].pixel;
>> }
>>
>> if (WARN_ON(!format))
>> diff --git a/include/video/pixel_format.h b/include/video/pixel_format.h
>> index b5104b2a3a13..5ad2386e2014 100644
>> --- a/include/video/pixel_format.h
>> +++ b/include/video/pixel_format.h
>> @@ -23,6 +23,12 @@ struct pixel_format {
>> #define PIXEL_FORMAT_XRGB1555 \
>> { 16, false, { .alpha = {0, 0}, .red = {10, 5}, .green = {5, 5}, .blue = {0, 5} } }
>>
>> +#define PIXEL_FORMAT_ARGB1555 \
>> + { 16, false, { .alpha = {15, 1}, .red = {10, 5}, .green = {5, 5}, .blue = {0, 5} } }
>> +
>> +#define PIXEL_FORMAT_RGBA5551 \
>> + { 16, false, { .alpha = {0, 1}, .red = {11, 5}, .green = {6, 5}, .blue = {1, 5} } }
>> +
>> #define PIXEL_FORMAT_RGB565 \
>> { 16, false, { .alpha = {0, 0}, .red = {11, 5}, .green = {5, 6}, .blue = {0, 5} } }
>>
>> @@ -32,10 +38,19 @@ struct pixel_format {
>> #define PIXEL_FORMAT_XRGB8888 \
>> { 32, false, { .alpha = {0, 0}, .red = {16, 8}, .green = {8, 8}, .blue = {0, 8} } }
>>
>> +#define PIXEL_FORMAT_ARGB8888 \
>> + { 32, false, { .alpha = {24, 8}, .red = {16, 8}, .green = {8, 8}, .blue = {0, 8} } }
>> +
>> #define PIXEL_FORMAT_XBGR8888 \
>> { 32, false, { .alpha = {0, 0}, .red = {0, 8}, .green = {8, 8}, .blue = {16, 8} } }
>>
>> +#define PIXEL_FORMAT_ABGR8888 \
>> + { 32, false, { .alpha = {24, 8}, .red = {0, 8}, .green = {8, 8}, .blue = {16, 8} } }
>> +
>> #define PIXEL_FORMAT_XRGB2101010 \
>> { 32, false, { .alpha = {0, 0}, .red = {20, 10}, .green = {10, 10}, .blue = {0, 10} } }
>>
>> +#define PIXEL_FORMAT_ARGB2101010 \
>> + { 32, false, { .alpha = {30, 1}, .red = {20, 10}, .green = {10, 10}, .blue = {0, 10} } }
>> +
>> #endif
>> --
>> 2.49.0
>>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
More information about the dri-devel
mailing list