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%2FRJ@qwark.sigxcpu.org/ 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.
Guido Günther (5): drm: mxsfb: Print failed bus format in hex drm: mxsfb: Set proper default bus format when using a bridge drm/bridge: nwl-dsi: Add atomic_get_input_bus_fmts drm/panel: mantix: Add media bus format drm/panel: st7703: Add media bus format
drivers/gpu/drm/bridge/nwl-dsi.c | 35 +++++++++++++++++++ drivers/gpu/drm/mxsfb/mxsfb_kms.c | 7 +++- .../gpu/drm/panel/panel-mantix-mlaf057we51.c | 9 +++++ drivers/gpu/drm/panel/panel-sitronix-st7703.c | 8 +++++ 4 files changed, 58 insertions(+), 1 deletion(-)
media-bus-formats.h has them in hexadecimal as well so matching with that file saves one conversion when debugging.
Signed-off-by: Guido Günther agx@sigxcpu.org --- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index af6c620adf6e..d6abd2077114 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -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; }
On Tue, 28 Sept 2021 at 14:16, Guido Günther agx@sigxcpu.org wrote:
media-bus-formats.h has them in hexadecimal as well so matching with that file saves one conversion when debugging.
Signed-off-by: Guido Günther agx@sigxcpu.org
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index af6c620adf6e..d6abd2077114 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -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; }
Reviewed-by: Robert Foss robert.foss@linaro.org
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 panels.
Fixes: b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present")
Reported-by: Martin Kepplinger martink@posteo.de Signed-off-by: Guido Günther agx@sigxcpu.org --- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index d6abd2077114..f4be16f5c20b 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -369,6 +369,11 @@ 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. Please fix."); + bus_format = MEDIA_BUS_FMT_RGB888_1X24; + } }
/* If there is no bridge, use bus format from connector */
On 9/28/21 2:16 PM, Guido Günther wrote:
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 panels.
Fixes: b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present")
Reported-by: Martin Kepplinger martink@posteo.de Signed-off-by: Guido Günther agx@sigxcpu.org
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index d6abd2077114..f4be16f5c20b 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -369,6 +369,11 @@ 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. Please fix.");
Fix what ? Oh ... the bridge driver ?
Maybe such a hint should be in the warning message. And it might be useful to explain how to fix that bridge driver, or at least provide some reference to an example (like this nwl patch series).
bus_format = MEDIA_BUS_FMT_RGB888_1X24;
The warning should also mention that the driver is falling back to this mode.
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")
Signed-off-by: Guido Günther agx@sigxcpu.org --- drivers/gpu/drm/bridge/nwl-dsi.c | 35 ++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c index a251cc1f088f..27c80d36846b 100644 --- a/drivers/gpu/drm/bridge/nwl-dsi.c +++ b/drivers/gpu/drm/bridge/nwl-dsi.c @@ -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,
This allows the DSI bridge to detect the correct bus format. We currently only support MEDIA_BUS_FMT_RGB888_1X24.
Signed-off-by: Guido Günther agx@sigxcpu.org --- drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c index f0e9bce23c41..d6bcf1045255 100644 --- a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c +++ b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c @@ -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; }
This allows the DSI bridge to detect the correct bus format. We currently only support MEDIA_BUS_FMT_RGB888_1X24.
Signed-off-by: Guido Günther agx@sigxcpu.org --- drivers/gpu/drm/panel/panel-sitronix-st7703.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c b/drivers/gpu/drm/panel/panel-sitronix-st7703.c index a2c303e5732c..73f69c929a75 100644 --- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c +++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c @@ -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; }
dri-devel@lists.freedesktop.org