Re: [PATCH 3/7] drm/panel: sitronix-st7789v: Specify the expected bus format

[Miquel Raynal]

Hi Sam,

s...@ravnborg.org wrote on Sat, 10 Jun 2023 22:12:46 +0200:

> On Fri, Jun 09, 2023 at 04:59:47PM +0200, Miquel Raynal wrote:
> > The LCD controller supports RGB444, RGB565 and RGB888. The value that is
> > written in the COLMOD register indicates using RGB888, so let's clearly
> > specify the in-use bus format.  
> 
> Confused.
> MEDIA_BUS_FMT_RGB666_1X18 assumes 6 bits per color.
> But RGB888 is 8 bits per color.
> 
> Something that I have forgotten, or is this inconsistent?

That is a typo indeed, I meant RBG666.

Thanks, Miquèl

Re: [PATCH 3/7] drm/panel: sitronix-st7789v: Specify the expected bus format

[Maxime Ripard]

Hi,

On Fri, Jun 09, 2023 at 04:59:47PM +0200, Miquel Raynal wrote:
> The LCD controller supports RGB444, RGB565 and RGB888. The value that is
> written in the COLMOD register indicates using RGB888, so let's clearly
> specify the in-use bus format.
> 
> Signed-off-by: Miquel Raynal 
> ---
>  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
> b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> index e9ca7ebb458a..0abb45bea18d 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -170,6 +171,7 @@ static int st7789v_get_modes(struct drm_panel *panel,
>struct drm_connector *connector)
>  {
>   struct drm_display_mode *mode;
> + u32 bus_format = MEDIA_BUS_FMT_RGB666_1X18;

I'm not sure it should be set in stone in the driver.

This is a property of the panel/controller, but also one of the RGB
controller upstream that will and how the board has been wired.

It's not unusual for boards with a limited number of GPIOs to take a
RGB888 panel and connect only as a RGB565 with the lowest bits to the
ground for example.

So I think this should come from the device tree, ideally from the media
graph endpoint.

Maxime

Re: [PATCH 3/7] drm/panel: sitronix-st7789v: Specify the expected bus format

[Sam Ravnborg]

On Fri, Jun 09, 2023 at 04:59:47PM +0200, Miquel Raynal wrote:
> The LCD controller supports RGB444, RGB565 and RGB888. The value that is
> written in the COLMOD register indicates using RGB888, so let's clearly
> specify the in-use bus format.

Confused.
MEDIA_BUS_FMT_RGB666_1X18 assumes 6 bits per color.
But RGB888 is 8 bits per color.

Something that I have forgotten, or is this inconsistent?

Sam

> 
> Signed-off-by: Miquel Raynal 
> ---
>  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
> b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> index e9ca7ebb458a..0abb45bea18d 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -170,6 +171,7 @@ static int st7789v_get_modes(struct drm_panel *panel,
>struct drm_connector *connector)
>  {
>   struct drm_display_mode *mode;
> + u32 bus_format = MEDIA_BUS_FMT_RGB666_1X18;
>  
>   mode = drm_mode_duplicate(connector->dev, _mode);
>   if (!mode) {
> @@ -186,6 +188,8 @@ static int st7789v_get_modes(struct drm_panel *panel,
>  
>   connector->display_info.width_mm = 61;
>   connector->display_info.height_mm = 103;
> + drm_display_info_set_bus_formats(>display_info,
> +  _format, 1);
>  
>   return 1;
>  }
> -- 
> 2.34.1

[PATCH 3/7] drm/panel: sitronix-st7789v: Specify the expected bus format

[Miquel Raynal]

The LCD controller supports RGB444, RGB565 and RGB888. The value that is
written in the COLMOD register indicates using RGB888, so let's clearly
specify the in-use bus format.

Signed-off-by: Miquel Raynal 
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index e9ca7ebb458a..0abb45bea18d 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -170,6 +171,7 @@ static int st7789v_get_modes(struct drm_panel *panel,
 struct drm_connector *connector)
 {
struct drm_display_mode *mode;
+   u32 bus_format = MEDIA_BUS_FMT_RGB666_1X18;
 
mode = drm_mode_duplicate(connector->dev, _mode);
if (!mode) {
@@ -186,6 +188,8 @@ static int st7789v_get_modes(struct drm_panel *panel,
 
connector->display_info.width_mm = 61;
connector->display_info.height_mm = 103;
+   drm_display_info_set_bus_formats(>display_info,
+_format, 1);
 
return 1;
 }
-- 
2.34.1