On Monday, July 31, 2023 2:44 AM Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, Jul 20, 2023 at 12:40 PM Ying Liu <victor.liu@xxxxxxx> wrote: > > > > Introduce ->get_input_bus_fmts() callback to struct dw_mipi_dsi_plat_data > > so that vendor drivers can implement specific methods to get input bus > > formats for Synopsys DW MIPI DSI. > > > > While at it, implement a generic callback for - > >atomic_get_input_bus_fmts(), > > where we try to get the input bus formats through pdata- > >get_input_bus_fmts() > > first. If it's unavailable, fall back to the only format - > MEDIA_BUS_FMT_FIXED, > > which matches the default behavior if ->atomic_get_input_bus_fmts() is > not > > implemented as ->atomic_get_input_bus_fmts()'s kerneldoc indicates. > > > > Signed-off-by: Liu Ying <victor.liu@xxxxxxx> > > --- > > v1->v2: > > * No change. > > > > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 30 > +++++++++++++++++++ > > include/drm/bridge/dw_mipi_dsi.h | 11 +++++++ > > 2 files changed, 41 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > index 57eae0fdd970..8580b8a97fb1 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > @@ -12,6 +12,7 @@ > > #include <linux/component.h> > > #include <linux/debugfs.h> > > #include <linux/iopoll.h> > > +#include <linux/media-bus-format.h> > > #include <linux/module.h> > > #include <linux/of_device.h> > > #include <linux/pm_runtime.h> > > @@ -536,6 +537,34 @@ static const struct mipi_dsi_host_ops > dw_mipi_dsi_host_ops = { > > .transfer = dw_mipi_dsi_host_transfer, > > }; > > > > +static u32 * > > +dw_mipi_dsi_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) > > +{ > > + struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge); > > + const struct dw_mipi_dsi_plat_data *pdata = dsi->plat_data; > > + u32 *input_fmts; > > + > > + if (pdata->get_input_bus_fmts) > > + return pdata->get_input_bus_fmts(pdata->priv_data, > > + bridge, bridge_state, > > + crtc_state, conn_state, > > + output_fmt, num_input_fmts); > > Why do we need platform-controlled bus formats? The supported bridge > formats are common across all platforms and it is specific to the list > of formats supported by DW-MIPI DSI. isn't it? Ideally, yes. But, if we consider some SoC logics introduced due to DW MIPI DSI integration(like the DISPLAY_MUX register in i.MX93 media blk-ctrl as a syscon), then pdata->get_input_bus_fmts is handy to cover the logics. Meson's MIPI_DSI_TOP_CNTL register and stm's DSI_WCFGR register are the similar cases for integration. Rockchip directly sets display controller's output bus format in ->atomic_check(). Granted those SoC logics can be a standalone DRM bridge as the previous bridge of the DW MIPI DSI bridge. However, the question is that are the logics worth a separate DRM bridge driver, considering pdata->get_input_bus_fmts is quite handy? Implementing a separate DRM bridge driver for i.MX93 DISPLAY_MUX to add two DRM bridges is a bit hard, because the single register controls both external parallel display pixel format and DSI controller input pixel format with different bit fields. And, I've already sent a patch series[1] to add a DRM bridge driver for the external parallel display pixel format control. [1] https://patchwork.freedesktop.org/series/113457/ Regards, Liu Ying > > Thanks, > Jagan.