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.
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 proper default bus format when using a bridge
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(-)
--
2.33.0
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 <[email protected]>
---
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,
--
2.33.0
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 <[email protected]>
---
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;
}
--
2.33.0
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 <[email protected]>
---
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;
}
--
2.33.0
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 <[email protected]>
---
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;
}
--
2.33.0
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 <[email protected]>
Signed-off-by: Guido Günther <[email protected]>
---
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
index d6abd2077114..e3fbb8b58d5d 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
@@ -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 */
--
2.33.0
Am Montag, dem 04.10.2021 um 09:27 +0200 schrieb Guido Günther:
> 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 <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
> ---
> 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,
Am Montag, dem 04.10.2021 um 09:27 +0200 schrieb Guido Günther:
> 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 <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
> ---
> 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;
> }
>
Hi,
On Mon, Oct 04, 2021 at 09:58:37AM +0200, Lucas Stach wrote:
> Am Montag, dem 04.10.2021 um 09:27 +0200 schrieb 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 panels.
> >
> > Fixes: b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present")
> >
> I don't think this qualifies for stable, so I would drop this tag, as
> the stable maintainers are quite trigger happy to pull in patches with
> a fixes tag. Also the subject isn't quite correct, this isn't setting a
> "proper" bus format, but rather adds a fallback. Other than that:
Adjusted for v3 (which I'll hold off a bit in case there are more
comments) and dropped the Fixes: tag which is on the nwl driver
only now. thanks!
-- Guido
>
> Reviewed-by: Lucas Stach <[email protected]>
>
> Regards,
> Lucas
>
> > Reported-by: Martin Kepplinger <[email protected]>
> > Signed-off-by: Guido G?nther <[email protected]>
>
> > ---
> > drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> > index d6abd2077114..e3fbb8b58d5d 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> > @@ -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 */
>
>
Am Montag, dem 04.10.2021 um 09:27 +0200 schrieb 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 panels.
>
> Fixes: b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present")
>
I don't think this qualifies for stable, so I would drop this tag, as
the stable maintainers are quite trigger happy to pull in patches with
a fixes tag. Also the subject isn't quite correct, this isn't setting a
"proper" bus format, but rather adds a fallback. Other than that:
Reviewed-by: Lucas Stach <[email protected]>
Regards,
Lucas
> Reported-by: Martin Kepplinger <[email protected]>
> Signed-off-by: Guido Günther <[email protected]>
> ---
> drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> index d6abd2077114..e3fbb8b58d5d 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> @@ -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 */