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 <tzimmerm...@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)
