diff mbox series

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

Message ID 20230609145951.853533-4-miquel.raynal@bootlin.com (mailing list archive)
State New, archived
Headers show
Series drm/panel: sitronix-st7789v: Support ET028013DMA panel | expand

Commit Message

Miquel Raynal June 9, 2023, 2:59 p.m. UTC
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 <miquel.raynal@bootlin.com>
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Sam Ravnborg June 10, 2023, 8:12 p.m. UTC | #1
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 <miquel.raynal@bootlin.com>
> ---
>  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 <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
> +#include <linux/media-bus-format.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
>  
> @@ -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, &default_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(&connector->display_info,
> +					 &bus_format, 1);
>  
>  	return 1;
>  }
> -- 
> 2.34.1
Maxime Ripard June 12, 2023, 8:44 a.m. UTC | #2
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 <miquel.raynal@bootlin.com>
> ---
>  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 <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
> +#include <linux/media-bus-format.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
>  
> @@ -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
Miquel Raynal June 16, 2023, 9:56 a.m. UTC | #3
Hi Sam,

sam@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
diff mbox series

Patch

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 <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
+#include <linux/media-bus-format.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 
@@ -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, &default_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(&connector->display_info,
+					 &bus_format, 1);
 
 	return 1;
 }