| Message ID | 20241003082006.2728617-1-w.egorov@phytec.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] drm/bridge: sii902x: Provide data-lines property to input endpoint | expand |
Hi Wadim, Thanks for the patch. Probably a nit, but the dt-binding patch should come before the driver patch. On 03-10-2024 13:50, Wadim Egorov wrote: > Introduce a data-lines property to define the number of parallel RGB > input pins connected to the transmitter. The input bus formats are updated > accordingly. If the property is not specified, default to 24 data lines. > > Signed-off-by: Wadim Egorov <w.egorov@phytec.de> > --- > drivers/gpu/drm/bridge/sii902x.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c > index 7f91b0db161e..3565c3533597 100644 > --- a/drivers/gpu/drm/bridge/sii902x.c > +++ b/drivers/gpu/drm/bridge/sii902x.c > @@ -180,6 +180,8 @@ struct sii902x { > struct gpio_desc *reset_gpio; > struct i2c_mux_core *i2cmux; > bool sink_is_hdmi; > + u32 pd_lines; /* number of Parallel Port Input Data Lines */ > + > /* > * Mutex protects audio and video functions from interfering > * each other, by keeping their i2c command sequences atomic. > @@ -477,6 +479,8 @@ static u32 *sii902x_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, > u32 output_fmt, > unsigned int *num_input_fmts) > { > + > + struct sii902x *sii902x = bridge_to_sii902x(bridge); > u32 *input_fmts; > > *num_input_fmts = 0; > @@ -485,7 +489,19 @@ static u32 *sii902x_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, > if (!input_fmts) > return NULL; > > - input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24; > + switch (sii902x->pd_lines) { > + case 16: > + input_fmts[0] = MEDIA_BUS_FMT_RGB565_1X16; > + break; > + case 18: > + input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X18; > + break; > + default: For backward compatibility - in cases where the property is absent - you have already defaulted sii902x->pd_lines to 24 below, which I think is the right way. So, the default case should be kept separately, as an error case - which should then return back NULL / num_input_fmts = 0. > + case 24: > + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24; > + break; > + } > + > *num_input_fmts = 1; > > return input_fmts; > @@ -1167,6 +1183,15 @@ static int sii902x_probe(struct i2c_client *client) > return PTR_ERR(sii902x->reset_gpio); > } > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); > + if (endpoint) { > + ret = of_property_read_u32(endpoint, "data-lines", &sii902x->pd_lines); > + if (ret) { > + dev_dbg(dev, "Could not get data-lines, fallback to 24 data-lines\n"); > + sii902x->pd_lines = 24; > + } > + } > + > endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1); > if (endpoint) { > struct device_node *remote = of_graph_get_remote_port_parent(endpoint); -- Regards Aradhya
Hi, On 03-10-2024 15:13, Aradhya Bhatia wrote: > Hi Wadim, > > Thanks for the patch. > > Probably a nit, but the dt-binding patch should come before the driver > patch. > > On 03-10-2024 13:50, Wadim Egorov wrote: >> Introduce a data-lines property to define the number of parallel RGB >> input pins connected to the transmitter. The input bus formats are updated >> accordingly. If the property is not specified, default to 24 data lines. One more thing. This driver also supports the older way of encoder-bridge connections - that is, with no DRM_BRIDGE_ATTACH_NO_CONNECTOR flag. In the sii902x_bridge_attach, you will see that the bus_format is being statically assigned to MEDIA_BUS_FMT_RGB888_1X24. When the ATTACH_NO_CONNECTOR flag is set, it doesn't matter. But when it is not, the RGB888 bus format will get trickled back to the encoder even if the dt property says differently. >> >> Signed-off-by: Wadim Egorov <w.egorov@phytec.de> >> --- >> drivers/gpu/drm/bridge/sii902x.c | 27 ++++++++++++++++++++++++++- >> 1 file changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c >> index 7f91b0db161e..3565c3533597 100644 >> --- a/drivers/gpu/drm/bridge/sii902x.c >> +++ b/drivers/gpu/drm/bridge/sii902x.c >> @@ -180,6 +180,8 @@ struct sii902x { >> struct gpio_desc *reset_gpio; >> struct i2c_mux_core *i2cmux; >> bool sink_is_hdmi; >> + u32 pd_lines; /* number of Parallel Port Input Data Lines */ >> + >> /* >> * Mutex protects audio and video functions from interfering >> * each other, by keeping their i2c command sequences atomic. >> @@ -477,6 +479,8 @@ static u32 *sii902x_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, >> u32 output_fmt, >> unsigned int *num_input_fmts) >> { >> + And this is probably a stray. >> + struct sii902x *sii902x = bridge_to_sii902x(bridge); >> u32 *input_fmts; >> >> *num_input_fmts = 0; >> @@ -485,7 +489,19 @@ static u32 *sii902x_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, >> if (!input_fmts) >> return NULL; >> >> - input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24; >> + switch (sii902x->pd_lines) { >> + case 16: >> + input_fmts[0] = MEDIA_BUS_FMT_RGB565_1X16; >> + break; >> + case 18: >> + input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X18; >> + break; >> + default: > > For backward compatibility - in cases where the property is absent - you > have already defaulted sii902x->pd_lines to 24 below, which I think is > the right way. > > So, the default case should be kept separately, as an error case - > which should then return back NULL / num_input_fmts = 0. > >> + case 24: >> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24; >> + break; >> + } >> + >> *num_input_fmts = 1; >> >> return input_fmts; >> @@ -1167,6 +1183,15 @@ static int sii902x_probe(struct i2c_client *client) >> return PTR_ERR(sii902x->reset_gpio); >> } >> >> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); >> + if (endpoint) { >> + ret = of_property_read_u32(endpoint, "data-lines", &sii902x->pd_lines); >> + if (ret) { >> + dev_dbg(dev, "Could not get data-lines, fallback to 24 data-lines\n"); >> + sii902x->pd_lines = 24; >> + } >> + } >> + >> endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1); >> if (endpoint) { >> struct device_node *remote = of_graph_get_remote_port_parent(endpoint); > > -- > Regards > Aradhya > -- Regards Aradhya
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 7f91b0db161e..3565c3533597 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -180,6 +180,8 @@ struct sii902x { struct gpio_desc *reset_gpio; struct i2c_mux_core *i2cmux; bool sink_is_hdmi; + u32 pd_lines; /* number of Parallel Port Input Data Lines */ + /* * Mutex protects audio and video functions from interfering * each other, by keeping their i2c command sequences atomic. @@ -477,6 +479,8 @@ static u32 *sii902x_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, u32 output_fmt, unsigned int *num_input_fmts) { + + struct sii902x *sii902x = bridge_to_sii902x(bridge); u32 *input_fmts; *num_input_fmts = 0; @@ -485,7 +489,19 @@ static u32 *sii902x_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, if (!input_fmts) return NULL; - input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24; + switch (sii902x->pd_lines) { + case 16: + input_fmts[0] = MEDIA_BUS_FMT_RGB565_1X16; + break; + case 18: + input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X18; + break; + default: + case 24: + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24; + break; + } + *num_input_fmts = 1; return input_fmts; @@ -1167,6 +1183,15 @@ static int sii902x_probe(struct i2c_client *client) return PTR_ERR(sii902x->reset_gpio); } + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); + if (endpoint) { + ret = of_property_read_u32(endpoint, "data-lines", &sii902x->pd_lines); + if (ret) { + dev_dbg(dev, "Could not get data-lines, fallback to 24 data-lines\n"); + sii902x->pd_lines = 24; + } + } + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1); if (endpoint) { struct device_node *remote = of_graph_get_remote_port_parent(endpoint);
Introduce a data-lines property to define the number of parallel RGB input pins connected to the transmitter. The input bus formats are updated accordingly. If the property is not specified, default to 24 data lines. Signed-off-by: Wadim Egorov <w.egorov@phytec.de> --- drivers/gpu/drm/bridge/sii902x.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)