| Message ID | 20201016103917.26838-5-nikhil.nd@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/tidss: Use new connector model for tidss | expand |
Hi Nikhil, Thank you for the patch. On Fri, Oct 16, 2020 at 04:09:16PM +0530, Nikhil Devshatwar wrote: > With new connector model, tfp410 will not create the connector and > SoC driver will rely on format negotiation to setup the encoder format. > > Support format negotiations hooks in the drm_bridge_funcs. > Use helper functions for state management. > Support one of the two RGB formats as selected from DT bindings. > > Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com> > --- > drivers/gpu/drm/bridge/ti-tfp410.c | 32 ++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c > index ba3fa2a9b8a4..b65e48e080c7 100644 > --- a/drivers/gpu/drm/bridge/ti-tfp410.c > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > @@ -204,12 +204,44 @@ static enum drm_mode_status tfp410_mode_valid(struct drm_bridge *bridge, > return MODE_OK; > } > > +static u32 *tfp410_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 tfp410 *dvi = drm_bridge_to_tfp410(bridge); > + u32 *input_fmts; > + > + *num_input_fmts = 0; > + > + /* > + * This bridge does not support media_bus_format conversion > + * Propagate only if supported > + */ > + if (output_fmt != dvi->bus_format && output_fmt != MEDIA_BUS_FMT_FIXED) > + return NULL; On the output side, DVI transmits RGB24 data over three TMDS links (plus one link for the clock). There's no media bus format that specifically describe this, but among the possible values for dvi->bus_format (MEDIA_BUS_FMT_RGB888_2X12_LE and MEDIA_BUS_FMT_RGB888_1X24), MEDIA_BUS_FMT_RGB888_2X12_LE doesn't make any sense to describe the output. We probably would need to introduce a specific media bus format if we wanted to describe this accurately (MEDIA_BUS_FMT_RGB888_DVI_SINGLE ?), which seems a bit overkill to support single link DVI. If we take dual link DVI into account, more bit depths are supported, as well as faster rates by transmitting to RGB888 pixels per clock, so more codes would be needed. With support for single-link DVI only, we could decide, subsystem-wide, to always use MEDIA_BUS_FMT_FIXED for DVI. We could also decide to additional accept MEDIA_BUS_FMT_RGB888_1X24 to describe single-link DVI, as a convention. If the goal of the above check is to make the format negotiation fail when the desired output format can't be supported by the TFP410, then I would use if (output_fmt != dvi->MEDIA_BUS_FMT_RGB888_1X24 && output_fmt != MEDIA_BUS_FMT_FIXED) return NULL; or simply if (output_fmt != MEDIA_BUS_FMT_FIXED) return NULL; depending on what convention we enforce in the subsystem for DVI media bus formats. I however wonder if this is needed at all, are there cases where the output could support multiple bus formats and the tfp410 driver would need to make sure that only the 24-bit single link DVI gets selected ? I suppose there are if we consider dual link DVI, but the DVI connector driver (drivers/gpu/drm/bridge/display-connector.c) doesn't report bus formats anyway. > + > + input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL); > + if (!input_fmts) > + return NULL; > + > + *num_input_fmts = 1; > + input_fmts[0] = dvi->bus_format; > + return input_fmts; > +} > + > static const struct drm_bridge_funcs tfp410_bridge_funcs = { > .attach = tfp410_attach, > .detach = tfp410_detach, > .enable = tfp410_enable, > .disable = tfp410_disable, > .mode_valid = tfp410_mode_valid, > + .atomic_reset = drm_atomic_helper_bridge_reset, > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > + .atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts, > }; > > static const struct drm_bridge_timings tfp410_default_timings = {
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index ba3fa2a9b8a4..b65e48e080c7 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -204,12 +204,44 @@ static enum drm_mode_status tfp410_mode_valid(struct drm_bridge *bridge, return MODE_OK; } +static u32 *tfp410_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 tfp410 *dvi = drm_bridge_to_tfp410(bridge); + u32 *input_fmts; + + *num_input_fmts = 0; + + /* + * This bridge does not support media_bus_format conversion + * Propagate only if supported + */ + if (output_fmt != dvi->bus_format && output_fmt != MEDIA_BUS_FMT_FIXED) + return NULL; + + input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL); + if (!input_fmts) + return NULL; + + *num_input_fmts = 1; + input_fmts[0] = dvi->bus_format; + return input_fmts; +} + static const struct drm_bridge_funcs tfp410_bridge_funcs = { .attach = tfp410_attach, .detach = tfp410_detach, .enable = tfp410_enable, .disable = tfp410_disable, .mode_valid = tfp410_mode_valid, + .atomic_reset = drm_atomic_helper_bridge_reset, + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, + .atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts, }; static const struct drm_bridge_timings tfp410_default_timings = {
With new connector model, tfp410 will not create the connector and SoC driver will rely on format negotiation to setup the encoder format. Support format negotiations hooks in the drm_bridge_funcs. Use helper functions for state management. Support one of the two RGB formats as selected from DT bindings. Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com> --- drivers/gpu/drm/bridge/ti-tfp410.c | 32 ++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)