| Message ID | 20230720071430.259892-3-victor.liu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: imx: Add i.MX93 MIPI DSI support | expand |
On Thu, Jul 20, 2023 at 12:40 PM Ying Liu <victor.liu@nxp.com> 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@nxp.com> > --- > 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? Thanks, Jagan.
On Monday, July 31, 2023 2:44 AM Jagan Teki <jagan@amarulasolutions.com> wrote: > > On Thu, Jul 20, 2023 at 12:40 PM Ying Liu <victor.liu@nxp.com> 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@nxp.com> > > --- > > 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.
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); + + /* Fall back to MEDIA_BUS_FMT_FIXED as the only input format. */ + input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL); + if (!input_fmts) + return NULL; + input_fmts[0] = MEDIA_BUS_FMT_FIXED; + *num_input_fmts = 1; + + return input_fmts; +} + static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi) { u32 val; @@ -1003,6 +1032,7 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge, static const struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = { .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, + .atomic_get_input_bus_fmts = dw_mipi_dsi_bridge_atomic_get_input_bus_fmts, .atomic_reset = drm_atomic_helper_bridge_reset, .atomic_enable = dw_mipi_dsi_bridge_atomic_enable, .atomic_post_disable = dw_mipi_dsi_bridge_post_atomic_disable, diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h index f54621b17a69..246650f2814f 100644 --- a/include/drm/bridge/dw_mipi_dsi.h +++ b/include/drm/bridge/dw_mipi_dsi.h @@ -11,7 +11,10 @@ #include <linux/types.h> +#include <drm/drm_atomic.h> #include <drm/drm_bridge.h> +#include <drm/drm_connector.h> +#include <drm/drm_crtc.h> #include <drm/drm_modes.h> struct drm_display_mode; @@ -56,6 +59,14 @@ struct dw_mipi_dsi_plat_data { unsigned long mode_flags, u32 lanes, u32 format); + u32 *(*get_input_bus_fmts)(void *priv_data, + 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); + const struct dw_mipi_dsi_phy_ops *phy_ops; const struct dw_mipi_dsi_host_ops *host_ops;
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@nxp.com> --- 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(+)