* [PATCH] drm/arm/hdlcd: Replace struct simplefb_format with custom type
@ 2025-05-27 9:42 Thomas Zimmermann
2025-05-30 10:10 ` Javier Martinez Canillas
2025-06-09 10:17 ` Liviu Dudau
0 siblings, 2 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2025-05-27 9:42 UTC (permalink / raw)
To: liviu.dudau, javierm; +Cc: dri-devel, Thomas Zimmermann
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@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;
-
/*
* 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/arm/hdlcd: Replace struct simplefb_format with custom type
2025-05-27 9:42 [PATCH] drm/arm/hdlcd: Replace struct simplefb_format with custom type Thomas Zimmermann
@ 2025-05-30 10:10 ` Javier Martinez Canillas
2025-05-30 10:40 ` Thomas Zimmermann
2025-06-09 10:17 ` Liviu Dudau
1 sibling, 1 reply; 5+ messages in thread
From: Javier Martinez Canillas @ 2025-05-30 10:10 UTC (permalink / raw)
To: Thomas Zimmermann, liviu.dudau; +Cc: dri-devel, Thomas Zimmermann
Thomas Zimmermann <tzimmermann@suse.de> writes:
> 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.
>
Is this change in the preferred format a concern ? It seems reasonable
to default to 32-bit formats but I wonder if something was relying on
the old 16-bit format as the preferred one.
> 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@suse.de>
> ---
The patch makes sense to me though.
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/arm/hdlcd: Replace struct simplefb_format with custom type
2025-05-30 10:10 ` Javier Martinez Canillas
@ 2025-05-30 10:40 ` Thomas Zimmermann
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2025-05-30 10:40 UTC (permalink / raw)
To: Javier Martinez Canillas, liviu.dudau; +Cc: dri-devel
Hi
Am 30.05.25 um 12:10 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
>> 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.
>>
> Is this change in the preferred format a concern ? It seems reasonable
> to default to 32-bit formats but I wonder if something was relying on
> the old 16-bit format as the preferred one.
That would have not been a good idea. :D I can put this into a separate
patch, tough. So that it will be revertable easily.
Best regards
Thomas
>
>> 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@suse.de>
>> ---
> The patch makes sense to me though.
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>
--
--
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)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/arm/hdlcd: Replace struct simplefb_format with custom type
2025-05-27 9:42 [PATCH] drm/arm/hdlcd: Replace struct simplefb_format with custom type Thomas Zimmermann
2025-05-30 10:10 ` Javier Martinez Canillas
@ 2025-06-09 10:17 ` Liviu Dudau
2025-06-10 7:07 ` Thomas Zimmermann
1 sibling, 1 reply; 5+ messages in thread
From: Liviu Dudau @ 2025-06-09 10:17 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: javierm, dri-devel
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@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.
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.
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
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/arm/hdlcd: Replace struct simplefb_format with custom type
2025-06-09 10:17 ` Liviu Dudau
@ 2025-06-10 7:07 ` Thomas Zimmermann
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2025-06-10 7:07 UTC (permalink / raw)
To: Liviu Dudau; +Cc: javierm, dri-devel
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@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)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-10 7:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-27 9:42 [PATCH] drm/arm/hdlcd: Replace struct simplefb_format with custom type Thomas Zimmermann
2025-05-30 10:10 ` Javier Martinez Canillas
2025-05-30 10:40 ` Thomas Zimmermann
2025-06-09 10:17 ` Liviu Dudau
2025-06-10 7:07 ` Thomas Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).