On Tue, May 9, 2023 at 10:14 AM Marek Vasut <marex@xxxxxxx> wrote: > > On 5/8/23 07:57, Liu Ying wrote: > > Hi, Hi, > > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > index 262bc43b1079..e54200a9fcb9 100644 > > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > @@ -394,7 +394,7 @@ static void lcdif_crtc_mode_set_nofb(struct lcdif_drm_private *lcdif, > > struct drm_display_mode *m = &lcdif->crtc.state->adjusted_mode; > > u32 bus_flags = 0; > > > > - if (lcdif->bridge && lcdif->bridge->timings) > > + if (lcdif->bridge->timings) > > bus_flags = lcdif->bridge->timings->input_bus_flags; > > else if (bridge_state) > > bus_flags = bridge_state->input_bus_cfg.flags; > > @@ -463,30 +463,21 @@ static void lcdif_crtc_atomic_enable(struct drm_crtc *crtc, > > struct drm_display_mode *m = &lcdif->crtc.state->adjusted_mode; > > struct drm_bridge_state *bridge_state = NULL; > > struct drm_device *drm = lcdif->drm; > > - u32 bus_format = 0; > > + u32 bus_format; > > dma_addr_t paddr; > > > > - /* If there is a bridge attached to the LCDIF, use its bus format */ > > - if (lcdif->bridge) { > > - bridge_state = > > - drm_atomic_get_new_bridge_state(state, > > - lcdif->bridge); > > - if (!bridge_state) > > - bus_format = MEDIA_BUS_FMT_FIXED; > > - else > > - 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; > > - } > > - } > > + bridge_state = drm_atomic_get_new_bridge_state(state, lcdif->bridge); > > + if (!bridge_state) > > + bus_format = MEDIA_BUS_FMT_FIXED; > > + else > > + bus_format = bridge_state->input_bus_cfg.format; > > The code below seems to change the logic slightly. > > Could it happen that: > - bridge_state is valid (i.e. non-NULL) > - bridge_state->input_bus_cfg.format is set to 0 (i.e. not set) ? > (note that MEDIA_BUS_FMT_FIXED is defined as 0x0001) Yes, bridge_state->input_bus_cfg.format could be zero. Will keep the below default MEDIA_BUS_FMT_RGB888_1X24 bus format setting in next version. Regards, Liu Ying > > > - /* If all else fails, default to RGB888_1X24 */ > > - if (!bus_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; > > + } > > > > clk_set_rate(lcdif->clk, m->crtc_clock * 1000); > > [...]