| Message ID | 20230125064030.1222121-3-victor.liu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: lcdif: Add i.MX93 LCDIF support | expand |
On 1/25/23 07:40, Liu Ying wrote: > A valid bridge is already found in lcdif_attach_bridge() and set > to lcdif->bridge, so lcdif->bridge cannot be a NULL pointer. Drop > the unnecessary NULL pointer check in KMS stage. Is it possible that a panel (instead of a bridge) be attached to LCDIFv3 e.g. in case of iMXRT ? > Signed-off-by: Liu Ying <victor.liu@nxp.com> > --- > v1->v2: > * Split from patch 2/2 in v1. (Marek, Alexander) Much appreciated, thanks. [...]
On Wed, 2023-01-25 at 14:56 +0100, Marek Vasut wrote: > On 1/25/23 07:40, Liu Ying wrote: > > A valid bridge is already found in lcdif_attach_bridge() and set > > to lcdif->bridge, so lcdif->bridge cannot be a NULL pointer. Drop > > the unnecessary NULL pointer check in KMS stage. > > Is it possible that a panel (instead of a bridge) be attached to > LCDIFv3 > e.g. in case of iMXRT ? According to IMXRT1160 and IMXRT1170 reference manuals, it looks like LCDIFv2(similar to LCDIFv3, but not the same) may connect to a parallel display panel through a mux(either eLCDIF or LCDIFv2). That mux could be a bridge. But, it doesn't matter, because devm_drm_of_get_bridge() may create a panel bridge if a panel is connected directly(See kerneldoc of devm_drm_of_get_bridge()). Regards, Liu Ying > > > Signed-off-by: Liu Ying <victor.liu@nxp.com> > > --- > > v1->v2: > > * Split from patch 2/2 in v1. (Marek, Alexander) > > Much appreciated, thanks. > > [...]
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; - /* 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);
A valid bridge is already found in lcdif_attach_bridge() and set to lcdif->bridge, so lcdif->bridge cannot be a NULL pointer. Drop the unnecessary NULL pointer check in KMS stage. Signed-off-by: Liu Ying <victor.liu@nxp.com> --- v1->v2: * Split from patch 2/2 in v1. (Marek, Alexander) drivers/gpu/drm/mxsfb/lcdif_kms.c | 33 +++++++++++-------------------- 1 file changed, 12 insertions(+), 21 deletions(-)