Skip to contentSkip to conversation

mxsfb/nwl/panels: media bus format fixes

PATCH In review Guido Günther 4 participants 11 messages Oct 11, 2021 – Oct 12, 2021

Subsystems (8)
lore
Share & tools
lore

Guido Günther

author maintainer
#0 lore raw

v3 cover mxsfb/nwl/panels: media bus format fixes

commit b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present") added bus format probing to mxsfb this exposed several issues in the display stack as used on the Librem 5:

The nwl bridge and the panels didn't bother to set any media bus formats and in that case mxsfb would not pick a reasonable default. This series aims to fix this.

This series includes the patch from
https://lore.kernel.org/dri-devel/YVLYh%2FSgBritG%[email protected]/
with a dev_warn added.

The patches are against 5.15-rc3. I've marked a single patch with a 'fixes' which is enough to unbreak the display stack in 5.15.

All patches of this series can be applied independently.

Changes from v1:
- Review comment by Marek Vasut
https://lore.kernel.org/dri-devel/[email protected]/
Improve warning message
- Move mxsfb patches to the end of the queue and the actual nwl fix to the front.

Changes from v2:
- Review comments by Lucas Stach
https://lore.kernel.org/dri-devel/[email protected]/T/#m961c22bf2a437a16b29214913d6f0d523fbe88e2
Drop fixes tag from mxsfb driver and tweak commit message
- Add Reviewed-by from Lucas
https://lore.kernel.org/dri-devel/[email protected]/T/#m9f3337a673ae1adbb1f0152a48270b0e1104b0c1
https://lore.kernel.org/dri-devel/[email protected]/T/#m5706d5b93cb6879b6bb4a1810b88800fe4180f74
https://lore.kernel.org/dri-devel/[email protected]/T/#m961c22bf2a437a16b29214913d6f0d523fbe88e2
- Add Reviewd-by from Rober Foss
https://lore.kernel.org/dri-devel/CAG3jFyteqcPNZqrFsggCrnpDfsGGQvibfHBezW+1Hcf3-z91QQ@mail.gmail.com/

Guido Günther (5):
drm/bridge: nwl-dsi: Add atomic_get_input_bus_fmts drm/panel: mantix: Add media bus format
drm/panel: st7703: Add media bus format
drm: mxsfb: Print failed bus format in hex
drm: mxsfb: Set fallback bus format when the bridge doesn't provide
one

 drivers/gpu/drm/bridge/nwl-dsi.c              | 35 +++++++++++++++++++
 drivers/gpu/drm/mxsfb/mxsfb_kms.c             |  8 ++++-
 .../gpu/drm/panel/panel-mantix-mlaf057we51.c  |  9 +++++
 drivers/gpu/drm/panel/panel-sitronix-st7703.c |  8 +++++
 4 files changed, 59 insertions(+), 1 deletion(-)
Signature · 1 line
2.33.0
author maintainer
#1 lore raw

PATCH v3 1/5 drm/bridge: nwl-dsi: Add atomic_get_input_bus_fmts

Components further up in the chain might ask us for supported formats.

Without this MEDIA_BUS_FMT_FIXED is assumed which then breaks display output with mxsfb since it can't determine a proper bus format.

We handle the bus formats that correspond to the DSI formats the bridge can potentially output (see chapter 13.6 of the i.MX 8MQ reference manual) - which matches what xsfb can input.

Fixes b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present")
---
 drivers/gpu/drm/bridge/nwl-dsi.c | 35 ++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
drivers/gpu/drm/bridge/nwl-dsi.c +35 -0
index a251cc1f088f..27c80d36846b 100644
@@ -1234,6 +1234,40 @@ static void nwl_dsi_bridge_detach(struct drm_bridge *bridge)
 	drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);
 }
 
+static u32 *nwl_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+						 struct drm_bridge_state *bridge_state,
+						 struct drm_crtc_state *crtc_state,
+						 struct drm_connector_state *conn_state,
+						 u32 output_fmt,
+						 unsigned int *num_input_fmts)
+{
+	u32 *input_fmts, input_fmt;
+
+	*num_input_fmts = 0;
+
+	switch (output_fmt) {
+	/* If MEDIA_BUS_FMT_FIXED is tested, return default bus format */
+	case MEDIA_BUS_FMT_FIXED:
+		input_fmt = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB666_1X18:
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		input_fmt = output_fmt;
+		break;
+	default:
+		return NULL;
+	}
+
+	input_fmts = kcalloc(1, sizeof(*input_fmts), GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+	input_fmts[0] = input_fmt;
+	*num_input_fmts = 1;
+
+	return input_fmts;
+}
+
 static const struct drm_bridge_funcs nwl_dsi_bridge_funcs = {
 	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
@@ -1241,6 +1275,7 @@ static const struct drm_bridge_funcs nwl_dsi_bridge_funcs = {
 	.atomic_check		= nwl_dsi_bridge_atomic_check,
 	.atomic_enable		= nwl_dsi_bridge_atomic_enable,
 	.atomic_disable		= nwl_dsi_bridge_atomic_disable,
+	.atomic_get_input_bus_fmts = nwl_bridge_atomic_get_input_bus_fmts,
 	.mode_set		= nwl_dsi_bridge_mode_set,
 	.mode_valid		= nwl_dsi_bridge_mode_valid,
 	.attach			= nwl_dsi_bridge_attach,
Signature · 1 line
2.33.0
author maintainer
#2 lore raw

PATCH v3 2/5 drm/panel: mantix: Add media bus format

In reply to Guido Günther

This allows the DSI bridge to detect the correct bus format. We currently only support MEDIA_BUS_FMT_RGB888_1X24.

---
 drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c +9 -0
index f0e9bce23c41..d6bcf1045255 100644
@@ -8,6 +8,7 @@
 #include <linux/backlight.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
+#include <linux/media-bus-format.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/regulator/consumer.h>
@@ -232,6 +233,10 @@ static const struct drm_display_mode default_mode_ys = {
 	.height_mm   = 130,
 };
 
+static const u32 mantix_bus_formats[] = {
+	MEDIA_BUS_FMT_RGB888_1X24,
+};
+
 static int mantix_get_modes(struct drm_panel *panel,
 			    struct drm_connector *connector)
 {
@@ -253,6 +258,10 @@ static int mantix_get_modes(struct drm_panel *panel,
 	connector->display_info.height_mm = mode->height_mm;
 	drm_mode_probed_add(connector, mode);
 
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 mantix_bus_formats,
+					 ARRAY_SIZE(mantix_bus_formats));
+
 	return 1;
 }
 
Signature · 1 line
2.33.0
author maintainer
#3 lore raw

PATCH v3 3/5 drm/panel: st7703: Add media bus format

In reply to Guido Günther

This allows the DSI bridge to detect the correct bus format. We currently only support MEDIA_BUS_FMT_RGB888_1X24.

---
 drivers/gpu/drm/panel/panel-sitronix-st7703.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/gpu/drm/panel/panel-sitronix-st7703.c +8 -0
index a2c303e5732c..73f69c929a75 100644
@@ -453,6 +453,10 @@ static int st7703_prepare(struct drm_panel *panel)
 	return ret;
 }
 
+static const u32 mantix_bus_formats[] = {
+	MEDIA_BUS_FMT_RGB888_1X24,
+};
+
 static int st7703_get_modes(struct drm_panel *panel,
 			    struct drm_connector *connector)
 {
@@ -474,6 +478,10 @@ static int st7703_get_modes(struct drm_panel *panel,
 	connector->display_info.height_mm = mode->height_mm;
 	drm_mode_probed_add(connector, mode);
 
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 mantix_bus_formats,
+					 ARRAY_SIZE(mantix_bus_formats));
+
 	return 1;
 }
 
Signature · 1 line
2.33.0
author maintainer
#4 lore raw

PATCH v3 4/5 drm: mxsfb: Print failed bus format in hex

In reply to Guido Günther

media-bus-formats.h has them in hexadecimal as well so matching with that file saves one conversion when debugging.

---
 drivers/gpu/drm/mxsfb/mxsfb_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/mxsfb/mxsfb_kms.c +1 -1
index af6c620adf6e..d6abd2077114 100644
@@ -89,7 +89,7 @@ static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb,
 		ctrl |= CTRL_BUS_WIDTH_24;
 		break;
 	default:
-		dev_err(drm->dev, "Unknown media bus format %d\n", bus_format);
+		dev_err(drm->dev, "Unknown media bus format 0x%x\n", bus_format);
 		break;
 	}
 
Signature · 1 line
2.33.0
reviewer Reviewed-by on v3 4/5
#5 lore raw

Re: [PATCH v3 4/5] drm: mxsfb: Print failed bus format in hex

Hi Guido,

Thank you for the patch.

Quoted 15 lines · Guido Günther

media-bus-formats.h has them in hexadecimal as well so matching with that file saves one conversion when debugging.

---
 drivers/gpu/drm/mxsfb/mxsfb_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/mxsfb/mxsfb_kms.c +0 -0
index af6c620adf6e..d6abd2077114 100644

drivers/gpu/drm/mxsfb/mxsfb_kms.c static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb, · line 92

Quoted 5 lines
drivers/gpu/drm/mxsfb/mxsfb_kms.c +1 -1
@@ -89,7 +89,7 @@ static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb,
 		ctrl |= CTRL_BUS_WIDTH_24;
 		break;
 	default:
-		dev_err(drm->dev, "Unknown media bus format %d\n", bus_format);
+		dev_err(drm->dev, "Unknown media bus format 0x%x\n", bus_format);

I may have gone for 0x%04x as current media bus formats all hold in 16 bits, but it's not big deal.

Quoted 3 lines
drivers/gpu/drm/mxsfb/mxsfb_kms.c +0 -0
 		break;
 	}
 
Signature · 3 lines
Regards,

Laurent Pinchart
maintainer Acked-by on v3 4/5
#6 lore raw

Re: [PATCH v3 4/5] drm: mxsfb: Print failed bus format in hex

In reply to Guido Günther
Quoted 6 lines · Guido Günther

media-bus-formats.h has them in hexadecimal as well so matching with that file saves one conversion when debugging.

Quoted 17 lines
---
 drivers/gpu/drm/mxsfb/mxsfb_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/mxsfb/mxsfb_kms.c +1 -1
index af6c620adf6e..d6abd2077114 100644
@@ -89,7 +89,7 @@ static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb,
 		ctrl |= CTRL_BUS_WIDTH_24;
 		break;
 	default:
-		dev_err(drm->dev, "Unknown media bus format %d\n", bus_format);
+		dev_err(drm->dev, "Unknown media bus format 0x%x\n", bus_format);
 		break;
 	}
author maintainer
#7 lore raw

PATCH v3 5/5 drm: mxsfb: Set fallback bus format when the bridge doesn't provide one

In reply to Guido Günther

If a bridge doesn't do any bus format handling MEDIA_BUS_FMT_FIXED is returned. Fallback to a reasonable default (MEDIA_BUS_FMT_RGB888_1X24) in that case.

This unbreaks e.g. using mxsfb with the nwl bridge and mipi dsi panels.

---
 drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/gpu/drm/mxsfb/mxsfb_kms.c +6 -0
index d6abd2077114..e3fbb8b58d5d 100644
@@ -369,6 +369,12 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc,
 			drm_atomic_get_new_bridge_state(state,
 							mxsfb->bridge);
 		bus_format = bridge_state->input_bus_cfg.format;
+		if (bus_format == MEDIA_BUS_FMT_FIXED) {
+			dev_warn_once(drm->dev,
+				      "Bridge does not provide bus format, assuming MEDIA_BUS_FMT_RGB888_1X24.\n"
+				      "Please fix bridge driver by handling atomic_get_input_bus_fmts.\n");
+			bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+		}
 	}
 
 	/* If there is no bridge, use bus format from connector */
Signature · 1 line
2.33.0
maintainer Acked-by on v3 5/5
#8 lore raw

Re: [PATCH v3 5/5] drm: mxsfb: Set fallback bus format when the bridge doesn't provide one

Quoted 9 lines · Guido Günther

If a bridge doesn't do any bus format handling MEDIA_BUS_FMT_FIXED is returned. Fallback to a reasonable default (MEDIA_BUS_FMT_RGB888_1X24) in that case.

This unbreaks e.g. using mxsfb with the nwl bridge and mipi dsi panels.

Quoted 23 lines
---
 drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/gpu/drm/mxsfb/mxsfb_kms.c +3 -0
index d6abd2077114..e3fbb8b58d5d 100644
@@ -369,6 +369,12 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc,
 			drm_atomic_get_new_bridge_state(state,
 							mxsfb->bridge);
 		bus_format = bridge_state->input_bus_cfg.format;
+		if (bus_format == MEDIA_BUS_FMT_FIXED) {
+			dev_warn_once(drm->dev,
+				      "Bridge does not provide bus format, assuming

MEDIA_BUS_FMT_RGB888_1X24.\n"

drivers/gpu/drm/mxsfb/mxsfb_kms.c +3 -0
+				      "Please fix bridge driver by handling atomic_get_input_bus_fmts.\n");
+			bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+		}
 	}
 
 	/* If there is no bridge, use bus format from connector */
Reviewed-by on the v3 series
#9 lore raw
In reply to Guido Günther

Hi Guido,

Quoted 37 lines · Guido Günther

commit b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present") added bus format probing to mxsfb this exposed several issues in the display stack as used on the Librem 5:

The nwl bridge and the panels didn't bother to set any media bus formats and in that case mxsfb would not pick a reasonable default. This series aims to fix this.

This series includes the patch from
https://lore.kernel.org/dri-devel/YVLYh%2FSgBritG%[email protected]/
with a dev_warn added.

The patches are against 5.15-rc3. I've marked a single patch with a 'fixes' which is enough to unbreak the display stack in 5.15.

All patches of this series can be applied independently.

Changes from v1:
- Review comment by Marek Vasut
https://lore.kernel.org/dri-devel/[email protected]/
Improve warning message
- Move mxsfb patches to the end of the queue and the actual nwl fix to the front.

Changes from v2:
- Review comments by Lucas Stach
https://lore.kernel.org/dri-devel/[email protected]/T/#m961c22bf2a437a16b29214913d6f0d523fbe88e2
Drop fixes tag from mxsfb driver and tweak commit message
- Add Reviewed-by from Lucas
https://lore.kernel.org/dri-devel/[email protected]/T/#m9f3337a673ae1adbb1f0152a48270b0e1104b0c1
https://lore.kernel.org/dri-devel/[email protected]/T/#m5706d5b93cb6879b6bb4a1810b88800fe4180f74
https://lore.kernel.org/dri-devel/[email protected]/T/#m961c22bf2a437a16b29214913d6f0d523fbe88e2
- Add Reviewd-by from Rober Foss
https://lore.kernel.org/dri-devel/CAG3jFyteqcPNZqrFsggCrnpDfsGGQvibfHBezW+1Hcf3-z91QQ@mail.gmail.com/

Quoted 7 lines

Guido Günther (5):
drm/bridge: nwl-dsi: Add atomic_get_input_bus_fmts drm/panel: mantix: Add media bus format
drm/panel: st7703: Add media bus format
drm: mxsfb: Print failed bus format in hex
drm: mxsfb: Set fallback bus format when the bridge doesn't provide
one

Full series are:

Sam

author maintainer
#10 lore raw

Hi Sam,

Quoted 48 lines · Sam Ravnborg

Hi Guido,

Quoted 37 lines · Guido Günther

commit b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present") added bus format probing to mxsfb this exposed several issues in the display stack as used on the Librem 5:

The nwl bridge and the panels didn't bother to set any media bus formats and in that case mxsfb would not pick a reasonable default. This series aims to fix this.

This series includes the patch from
https://lore.kernel.org/dri-devel/YVLYh%2FSgBritG%[email protected]/
with a dev_warn added.

The patches are against 5.15-rc3. I've marked a single patch with a 'fixes' which is enough to unbreak the display stack in 5.15.

All patches of this series can be applied independently.

Changes from v1:
- Review comment by Marek Vasut
https://lore.kernel.org/dri-devel/[email protected]/
Improve warning message
- Move mxsfb patches to the end of the queue and the actual nwl fix to the front.

Changes from v2:
- Review comments by Lucas Stach
https://lore.kernel.org/dri-devel/[email protected]/T/#m961c22bf2a437a16b29214913d6f0d523fbe88e2
Drop fixes tag from mxsfb driver and tweak commit message
- Add Reviewed-by from Lucas
https://lore.kernel.org/dri-devel/[email protected]/T/#m9f3337a673ae1adbb1f0152a48270b0e1104b0c1
https://lore.kernel.org/dri-devel/[email protected]/T/#m5706d5b93cb6879b6bb4a1810b88800fe4180f74
https://lore.kernel.org/dri-devel/[email protected]/T/#m961c22bf2a437a16b29214913d6f0d523fbe88e2
- Add Reviewd-by from Rober Foss
https://lore.kernel.org/dri-devel/CAG3jFyteqcPNZqrFsggCrnpDfsGGQvibfHBezW+1Hcf3-z91QQ@mail.gmail.com/

Quoted 7 lines

Guido Günther (5):
drm/bridge: nwl-dsi: Add atomic_get_input_bus_fmts drm/panel: mantix: Add media bus format
drm/panel: st7703: Add media bus format
drm: mxsfb: Print failed bus format in hex
drm: mxsfb: Set fallback bus format when the bridge doesn't provide
one

Quoted 2 lines

Full series are:

Pushed the whole series to drm-misc-next.
Thanks,
-- Guido

Quoted 3 lines

Sam