Re: [PATCH v3 2/4] drm: mxsfb: Add optional DPI output bus-width configuration

[Francesco Dolcini <francesco@dolcini.it>]

On Wed, Aug 12, 2026 at 05:02:51PM +0200, Marco Felsch wrote:
> Hi Francesco,
> 
> On 26-08-12, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > LCDIF programs LCD_DATABUS_WIDTH from the selected media bus format. The
> > format reported by the downstream panel or bridge describes the display
> > input, but it does not describe how the LCDIF data pins are physically
> > wired on the board.
> > 
> > These can differ. For example, a 16-bit LCDIF bus can be connected to a
> > 24-bit display by wiring the available color bits to the corresponding
> > display inputs. In that case, using the display's 24-bit format to
> > configure LCDIF selects the wrong data-bus mode and changes the assignment
> > of color bits on the LCD_DATA pins.
> > 
> > Read the optional bus-width endpoint property from the LCDIF output port
> > and use it to select the media bus format used to configure LCDIF. This
> > allows the LCDIF bus mode to describe the physical interface
> > independently of the downstream display format.
> > 
> > When the optional property is absent, or it has an invalid value, continue
> > using the format reported by the downstream display device, preserving
> > the existing behavior. The code is not validating the DT for
> > correctness and just fall back to the previous behavior in case of
> > errors.
> > 
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> > v3: improve commit message and make it clear that the code assumes that
> >     the DT is correct, when the DT contains an invalid property value,
> >     the code safely fall-back to the previous behavior
> > v2: use the common bus-width property instead of the legacy interface-pix-fmt
> > ---
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 24 ++++++++++++++++++++++++
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.h |  2 ++
> >  drivers/gpu/drm/mxsfb/mxsfb_kms.c | 12 ++++++++++++
> >  3 files changed, 38 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > index 8b4a500347fb..f8ecc8bbd63c 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > @@ -12,7 +12,10 @@
> >  #include <linux/clk.h>
> >  #include <linux/dma-mapping.h>
> >  #include <linux/io.h>
> > +#include <linux/media-bus-format.h>
> >  #include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_graph.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/property.h>
> >  #include <linux/pm_runtime.h>
> > @@ -211,7 +214,10 @@ static int mxsfb_load(struct drm_device *drm,
> >  		      const struct mxsfb_devdata *devdata)
> >  {
> >  	struct platform_device *pdev = to_platform_device(drm->dev);
> > +	struct device_node *np = pdev->dev.of_node;
> >  	struct mxsfb_drm_private *mxsfb;
> > +	struct device_node *ep;
> > +	u32 bus_width = 0;
> 
> IMHO this default value makes no sense with..
> 
> >  	int ret;
> >  
> >  	mxsfb = devm_kzalloc(&pdev->dev, sizeof(*mxsfb), GFP_KERNEL);
> > @@ -238,6 +244,24 @@ static int mxsfb_load(struct drm_device *drm,
> >  	if (IS_ERR(mxsfb->clk_disp_axi))
> >  		mxsfb->clk_disp_axi = NULL;
> >  
> > +	ep = of_graph_get_next_endpoint(np, NULL);
> > +	if (ep) {
> > +		of_property_read_u32(ep, "bus-width", &bus_width);
> > +		of_node_put(ep);
> > +	}
> > +
> > +	switch (bus_width) {
> > +	case 16:
> > +		mxsfb->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
> > +		break;
> > +	case 18:
> > +		mxsfb->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
> > +		break;
> > +	case 24:
> > +		mxsfb->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> > +		break;
> 
> these cases in case no bus-width was specified without the knowledge of
> the mxsfb_kms.c since the '0' value is not covered. However, I'm not the
> maintainer of this driver. Thanks for making use of the bus-width
> property :)

I think it's needed, otherwise the variable might be non-initialized if
the bus-width property is not present.

And I need it to be 0, by default, so that we do leave bus_format to 0,
and with that we maintain the previous existing behavior.

Francesco