| Message ID | 20220719080845.22122-5-a-bhatia1@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/8] dt-bindings: display: ti, am65x-dss: Add port properties for DSS | expand |
On 19/07/2022 11:08, Aradhya Bhatia wrote: > The 2 OLDI TXes in the AM625 SoC can be synced together to output a 2K > resolution video. > > Add support in the driver for the discovery of such a dual mode > connection on the OLDI video port, using the values of "ti,oldi-mode" > property. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > --- > drivers/gpu/drm/tidss/tidss_dispc.c | 39 +++++++++++++++++++++-------- > 1 file changed, 28 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c > index add725fa682b..fb1fdecfc83a 100644 > --- a/drivers/gpu/drm/tidss/tidss_dispc.c > +++ b/drivers/gpu/drm/tidss/tidss_dispc.c > @@ -853,25 +853,36 @@ void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask) > } > } > > -enum dispc_oldi_mode_reg_val { SPWG_18 = 0, JEIDA_24 = 1, SPWG_24 = 2 }; > +enum dispc_oldi_mode_reg_val { > + SPWG_18 = 0, > + JEIDA_24 = 1, > + SPWG_24 = 2, > + DL_SPWG_18 = 4, > + DL_JEIDA_24 = 5, > + DL_SPWG_24 = 6, > +}; > > struct dispc_bus_format { > u32 bus_fmt; > u32 data_width; > bool is_oldi_fmt; > + bool is_dual_link; > enum dispc_oldi_mode_reg_val oldi_mode_reg_val; > }; > > static const struct dispc_bus_format dispc_bus_formats[] = { > - { MEDIA_BUS_FMT_RGB444_1X12, 12, false, 0 }, > - { MEDIA_BUS_FMT_RGB565_1X16, 16, false, 0 }, > - { MEDIA_BUS_FMT_RGB666_1X18, 18, false, 0 }, > - { MEDIA_BUS_FMT_RGB888_1X24, 24, false, 0 }, > - { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, 0 }, > - { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, 0 }, > - { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, SPWG_18 }, > - { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, SPWG_24 }, > - { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, JEIDA_24 }, > + { MEDIA_BUS_FMT_RGB444_1X12, 12, false, false, 0 }, > + { MEDIA_BUS_FMT_RGB565_1X16, 16, false, false, 0 }, > + { MEDIA_BUS_FMT_RGB666_1X18, 18, false, false, 0 }, > + { MEDIA_BUS_FMT_RGB888_1X24, 24, false, false, 0 }, > + { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, false, 0 }, > + { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, false, 0 }, > + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, false, SPWG_18 }, > + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, false, SPWG_24 }, > + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, false, JEIDA_24 }, > + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, true, DL_SPWG_18 }, > + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, true, DL_SPWG_24 }, > + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, true, DL_JEIDA_24 }, > }; So the dual link sends two pixels per clock, right? Are there panel or bridge drivers that support this? My initial thought was that it should be a new bus format. Tomi
On 28/07/2022 14:03, Tomi Valkeinen wrote: > On 19/07/2022 11:08, Aradhya Bhatia wrote: >> The 2 OLDI TXes in the AM625 SoC can be synced together to output a 2K >> resolution video. >> >> Add support in the driver for the discovery of such a dual mode >> connection on the OLDI video port, using the values of "ti,oldi-mode" >> property. >> >> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >> --- >> drivers/gpu/drm/tidss/tidss_dispc.c | 39 +++++++++++++++++++++-------- >> 1 file changed, 28 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c >> b/drivers/gpu/drm/tidss/tidss_dispc.c >> index add725fa682b..fb1fdecfc83a 100644 >> --- a/drivers/gpu/drm/tidss/tidss_dispc.c >> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c >> @@ -853,25 +853,36 @@ void dispc_set_irqenable(struct dispc_device >> *dispc, dispc_irq_t mask) >> } >> } >> -enum dispc_oldi_mode_reg_val { SPWG_18 = 0, JEIDA_24 = 1, SPWG_24 = 2 }; >> +enum dispc_oldi_mode_reg_val { >> + SPWG_18 = 0, >> + JEIDA_24 = 1, >> + SPWG_24 = 2, >> + DL_SPWG_18 = 4, >> + DL_JEIDA_24 = 5, >> + DL_SPWG_24 = 6, >> +}; >> struct dispc_bus_format { >> u32 bus_fmt; >> u32 data_width; >> bool is_oldi_fmt; >> + bool is_dual_link; >> enum dispc_oldi_mode_reg_val oldi_mode_reg_val; >> }; >> static const struct dispc_bus_format dispc_bus_formats[] = { >> - { MEDIA_BUS_FMT_RGB444_1X12, 12, false, 0 }, >> - { MEDIA_BUS_FMT_RGB565_1X16, 16, false, 0 }, >> - { MEDIA_BUS_FMT_RGB666_1X18, 18, false, 0 }, >> - { MEDIA_BUS_FMT_RGB888_1X24, 24, false, 0 }, >> - { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, 0 }, >> - { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, 0 }, >> - { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, SPWG_18 }, >> - { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, SPWG_24 }, >> - { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, JEIDA_24 }, >> + { MEDIA_BUS_FMT_RGB444_1X12, 12, false, false, 0 }, >> + { MEDIA_BUS_FMT_RGB565_1X16, 16, false, false, 0 }, >> + { MEDIA_BUS_FMT_RGB666_1X18, 18, false, false, 0 }, >> + { MEDIA_BUS_FMT_RGB888_1X24, 24, false, false, 0 }, >> + { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, false, 0 }, >> + { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, false, 0 }, >> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, false, SPWG_18 }, >> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, false, SPWG_24 }, >> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, false, JEIDA_24 }, >> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, true, DL_SPWG_18 }, >> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, true, DL_SPWG_24 }, >> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, true, >> DL_JEIDA_24 }, >> }; > > So the dual link sends two pixels per clock, right? Are there panel or > bridge drivers that support this? My initial thought was that it should > be a new bus format. Looks like we have drm bridges supporting dual link, and they use the "normal" bus format. Did you have a look at them? They require two port nodes for dual link, and use the existence of the second one to decide if dual link is used or not. There are also lvds helpers in drm_of.c. I didn't look closely, but it looked to me that the helpers can tell you if the ports are connected to a dual link bridge. If not, you could fall back to cloning. This way no extra properties are needed. But you will need to add a port node, which I think you need to add anyway for cloning. Tomi
Hi Tomi, On 28-Jul-22 17:15, Tomi Valkeinen wrote: > On 28/07/2022 14:03, Tomi Valkeinen wrote: >> On 19/07/2022 11:08, Aradhya Bhatia wrote: >>> The 2 OLDI TXes in the AM625 SoC can be synced together to output a 2K >>> resolution video. >>> >>> Add support in the driver for the discovery of such a dual mode >>> connection on the OLDI video port, using the values of "ti,oldi-mode" >>> property. >>> >>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >>> --- >>> drivers/gpu/drm/tidss/tidss_dispc.c | 39 +++++++++++++++++++++-------- >>> 1 file changed, 28 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c >>> b/drivers/gpu/drm/tidss/tidss_dispc.c >>> index add725fa682b..fb1fdecfc83a 100644 >>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c >>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c >>> @@ -853,25 +853,36 @@ void dispc_set_irqenable(struct dispc_device >>> *dispc, dispc_irq_t mask) >>> } >>> } >>> -enum dispc_oldi_mode_reg_val { SPWG_18 = 0, JEIDA_24 = 1, SPWG_24 = >>> 2 }; >>> +enum dispc_oldi_mode_reg_val { >>> + SPWG_18 = 0, >>> + JEIDA_24 = 1, >>> + SPWG_24 = 2, >>> + DL_SPWG_18 = 4, >>> + DL_JEIDA_24 = 5, >>> + DL_SPWG_24 = 6, >>> +}; >>> struct dispc_bus_format { >>> u32 bus_fmt; >>> u32 data_width; >>> bool is_oldi_fmt; >>> + bool is_dual_link; >>> enum dispc_oldi_mode_reg_val oldi_mode_reg_val; >>> }; >>> static const struct dispc_bus_format dispc_bus_formats[] = { >>> - { MEDIA_BUS_FMT_RGB444_1X12, 12, false, 0 }, >>> - { MEDIA_BUS_FMT_RGB565_1X16, 16, false, 0 }, >>> - { MEDIA_BUS_FMT_RGB666_1X18, 18, false, 0 }, >>> - { MEDIA_BUS_FMT_RGB888_1X24, 24, false, 0 }, >>> - { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, 0 }, >>> - { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, 0 }, >>> - { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, SPWG_18 }, >>> - { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, SPWG_24 }, >>> - { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, JEIDA_24 }, >>> + { MEDIA_BUS_FMT_RGB444_1X12, 12, false, false, 0 }, >>> + { MEDIA_BUS_FMT_RGB565_1X16, 16, false, false, 0 }, >>> + { MEDIA_BUS_FMT_RGB666_1X18, 18, false, false, 0 }, >>> + { MEDIA_BUS_FMT_RGB888_1X24, 24, false, false, 0 }, >>> + { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, false, 0 }, >>> + { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, false, 0 }, >>> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, false, SPWG_18 }, >>> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, false, SPWG_24 }, >>> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, false, JEIDA_24 }, >>> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, true, DL_SPWG_18 }, >>> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, true, DL_SPWG_24 }, >>> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, true, DL_JEIDA_24 }, >>> }; >> >> So the dual link sends two pixels per clock, right? Are there panel or >> bridge drivers that support this? My initial thought was that it >> should be a new bus format. > > Looks like we have drm bridges supporting dual link, and they use the > "normal" bus format. Did you have a look at them? They require two port > nodes for dual link, and use the existence of the second one to decide > if dual link is used or not. The above edits were not for adding a new bus format for dual link connections. I added them in order to be able to write the correct OLDI config values in the register. > > There are also lvds helpers in drm_of.c. I didn't look closely, but it > looked to me that the helpers can tell you if the ports are connected to > a dual link bridge. If not, you could fall back to cloning. This way no > extra properties are needed. But you will need to add a port node, which > I think you need to add anyway for cloning. I have now seen drm_of.c and examples (renesas' rcar lvds) that use the apis that drm_of.c is offering. In those cases, the OLDI TXes are being modeled as separate devices, which is not the case with the tidss' OLDI TXes. Since the only few OLDI registers are in the DSS address space, they were just being configured through the tidss driver. Even in DT, the dss port (for OLDI) connects to the panel port's endpoint directly. Even in cases of dual link or cloning, it's only a singular remote-to-endpoint connection between the (OLDI) VP and the panel port. Hence the requirement of the properties in the earlier patches of the series. The use of lvds helper functions does not seem feasible in this case, because even they read DT properties to determine the dual link connection and those properties need to be a part of a lvds bridge device. I have also been considering the idea of implementing a new device driver for the OLDI TXes, not unlike the renesas' one. That way the driver could have the properties and the lvds helper functions at their disposal. I am just slightly unsure if that would allow space for any conflicts because of the shared register space. Do let me know what you think! Regards Aradhya
Hi, On 09/08/2022 08:58, Aradhya Bhatia wrote: > Hi Tomi, > > On 28-Jul-22 17:15, Tomi Valkeinen wrote: >> On 28/07/2022 14:03, Tomi Valkeinen wrote: >>> On 19/07/2022 11:08, Aradhya Bhatia wrote: >>>> The 2 OLDI TXes in the AM625 SoC can be synced together to output a 2K >>>> resolution video. >>>> >>>> Add support in the driver for the discovery of such a dual mode >>>> connection on the OLDI video port, using the values of "ti,oldi-mode" >>>> property. >>>> >>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >>>> --- >>>> drivers/gpu/drm/tidss/tidss_dispc.c | 39 >>>> +++++++++++++++++++++-------- >>>> 1 file changed, 28 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c >>>> b/drivers/gpu/drm/tidss/tidss_dispc.c >>>> index add725fa682b..fb1fdecfc83a 100644 >>>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c >>>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c >>>> @@ -853,25 +853,36 @@ void dispc_set_irqenable(struct dispc_device >>>> *dispc, dispc_irq_t mask) >>>> } >>>> } >>>> -enum dispc_oldi_mode_reg_val { SPWG_18 = 0, JEIDA_24 = 1, SPWG_24 = >>>> 2 }; >>>> +enum dispc_oldi_mode_reg_val { >>>> + SPWG_18 = 0, >>>> + JEIDA_24 = 1, >>>> + SPWG_24 = 2, >>>> + DL_SPWG_18 = 4, >>>> + DL_JEIDA_24 = 5, >>>> + DL_SPWG_24 = 6, >>>> +}; >>>> struct dispc_bus_format { >>>> u32 bus_fmt; >>>> u32 data_width; >>>> bool is_oldi_fmt; >>>> + bool is_dual_link; >>>> enum dispc_oldi_mode_reg_val oldi_mode_reg_val; >>>> }; >>>> static const struct dispc_bus_format dispc_bus_formats[] = { >>>> - { MEDIA_BUS_FMT_RGB444_1X12, 12, false, 0 }, >>>> - { MEDIA_BUS_FMT_RGB565_1X16, 16, false, 0 }, >>>> - { MEDIA_BUS_FMT_RGB666_1X18, 18, false, 0 }, >>>> - { MEDIA_BUS_FMT_RGB888_1X24, 24, false, 0 }, >>>> - { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, 0 }, >>>> - { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, 0 }, >>>> - { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, SPWG_18 }, >>>> - { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, SPWG_24 }, >>>> - { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, JEIDA_24 }, >>>> + { MEDIA_BUS_FMT_RGB444_1X12, 12, false, false, 0 }, >>>> + { MEDIA_BUS_FMT_RGB565_1X16, 16, false, false, 0 }, >>>> + { MEDIA_BUS_FMT_RGB666_1X18, 18, false, false, 0 }, >>>> + { MEDIA_BUS_FMT_RGB888_1X24, 24, false, false, 0 }, >>>> + { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, false, 0 }, >>>> + { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, false, 0 }, >>>> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, false, SPWG_18 }, >>>> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, false, SPWG_24 }, >>>> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, false, >>>> JEIDA_24 }, >>>> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, true, >>>> DL_SPWG_18 }, >>>> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, true, >>>> DL_SPWG_24 }, >>>> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, true, >>>> DL_JEIDA_24 }, >>>> }; >>> >>> So the dual link sends two pixels per clock, right? Are there panel >>> or bridge drivers that support this? My initial thought was that it >>> should be a new bus format. >> >> Looks like we have drm bridges supporting dual link, and they use the >> "normal" bus format. Did you have a look at them? They require two >> port nodes for dual link, and use the existence of the second one to >> decide if dual link is used or not. > The above edits were not for adding a new bus format for dual link > connections. I added them in order to be able to write the correct OLDI > config values in the register. > >> >> There are also lvds helpers in drm_of.c. I didn't look closely, but it >> looked to me that the helpers can tell you if the ports are connected >> to a dual link bridge. If not, you could fall back to cloning. This >> way no extra properties are needed. But you will need to add a port >> node, which I think you need to add anyway for cloning. > I have now seen drm_of.c and examples (renesas' rcar lvds) that use the > apis that drm_of.c is offering. In those cases, the OLDI TXes are being > modeled as separate devices, which is not the case with the tidss' OLDI > TXes. Since the only few OLDI registers are in the DSS address space, > they were just being configured through the tidss driver. I think it's irrelevant (in the bigger picture) whether the TXes are separate devices, single device or part of some other device. Or why do you think it matters? > Even in DT, the dss port (for OLDI) connects to the panel port's > endpoint directly. Even in cases of dual link or cloning, it's only a > singular remote-to-endpoint connection between the (OLDI) VP and the > panel port. Hence the requirement of the properties in the earlier > patches of the series. Sorry, I don't follow. If you use cloning, you have two TX outputs, going to two panels, right? So you need two panel DT nodes, and those would connect to two OLDI TX ports in the DSS. Afaics the existing dual link bridge/panel drivers also use two ports for the connection, so to use the dual link you need two ports in the DSS. I admit I'm not familiar with LVDS dual link, but it's not clear to me how you see the dual OLDI TX being used with other drivers if you have only one port. What kind of setups have you tested? > The use of lvds helper functions does not seem feasible in this case, > because even they read DT properties to determine the dual link > connection and those properties need to be a part of a lvds bridge > device. Can you elaborate a bit more why the DRM helpers couldn't be used here? > I have also been considering the idea of implementing a new device > driver for the OLDI TXes, not unlike the renesas' one. That way the > driver could have the properties and the lvds helper functions at their > disposal. I am just slightly unsure if that would allow space for any > conflicts because of the shared register space. No, I don't think new devices are needed here. Tomi
Hi Tomi, On 09-Aug-22 11:58, Tomi Valkeinen wrote: > Hi, > > On 09/08/2022 08:58, Aradhya Bhatia wrote: >> Hi Tomi, >> >> On 28-Jul-22 17:15, Tomi Valkeinen wrote: >>> On 28/07/2022 14:03, Tomi Valkeinen wrote: >>>> On 19/07/2022 11:08, Aradhya Bhatia wrote: >>>>> The 2 OLDI TXes in the AM625 SoC can be synced together to output a 2K >>>>> resolution video. >>>>> >>>>> Add support in the driver for the discovery of such a dual mode >>>>> connection on the OLDI video port, using the values of "ti,oldi-mode" >>>>> property. >>>>> >>>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >>>>> --- >>>>> drivers/gpu/drm/tidss/tidss_dispc.c | 39 >>>>> +++++++++++++++++++++-------- >>>>> 1 file changed, 28 insertions(+), 11 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c >>>>> b/drivers/gpu/drm/tidss/tidss_dispc.c >>>>> index add725fa682b..fb1fdecfc83a 100644 >>>>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c >>>>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c >>>>> @@ -853,25 +853,36 @@ void dispc_set_irqenable(struct dispc_device >>>>> *dispc, dispc_irq_t mask) >>>>> } >>>>> } >>>>> -enum dispc_oldi_mode_reg_val { SPWG_18 = 0, JEIDA_24 = 1, SPWG_24 >>>>> = 2 }; >>>>> +enum dispc_oldi_mode_reg_val { >>>>> + SPWG_18 = 0, >>>>> + JEIDA_24 = 1, >>>>> + SPWG_24 = 2, >>>>> + DL_SPWG_18 = 4, >>>>> + DL_JEIDA_24 = 5, >>>>> + DL_SPWG_24 = 6, >>>>> +}; >>>>> struct dispc_bus_format { >>>>> u32 bus_fmt; >>>>> u32 data_width; >>>>> bool is_oldi_fmt; >>>>> + bool is_dual_link; >>>>> enum dispc_oldi_mode_reg_val oldi_mode_reg_val; >>>>> }; >>>>> static const struct dispc_bus_format dispc_bus_formats[] = { >>>>> - { MEDIA_BUS_FMT_RGB444_1X12, 12, false, 0 }, >>>>> - { MEDIA_BUS_FMT_RGB565_1X16, 16, false, 0 }, >>>>> - { MEDIA_BUS_FMT_RGB666_1X18, 18, false, 0 }, >>>>> - { MEDIA_BUS_FMT_RGB888_1X24, 24, false, 0 }, >>>>> - { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, 0 }, >>>>> - { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, 0 }, >>>>> - { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, SPWG_18 }, >>>>> - { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, SPWG_24 }, >>>>> - { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, JEIDA_24 }, >>>>> + { MEDIA_BUS_FMT_RGB444_1X12, 12, false, false, 0 }, >>>>> + { MEDIA_BUS_FMT_RGB565_1X16, 16, false, false, 0 }, >>>>> + { MEDIA_BUS_FMT_RGB666_1X18, 18, false, false, 0 }, >>>>> + { MEDIA_BUS_FMT_RGB888_1X24, 24, false, false, 0 }, >>>>> + { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, false, 0 }, >>>>> + { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, false, 0 }, >>>>> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, false, SPWG_18 }, >>>>> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, false, SPWG_24 }, >>>>> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, false, >>>>> JEIDA_24 }, >>>>> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, true, >>>>> DL_SPWG_18 }, >>>>> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, true, >>>>> DL_SPWG_24 }, >>>>> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, true, >>>>> DL_JEIDA_24 }, >>>>> }; >>>> >>>> So the dual link sends two pixels per clock, right? Are there panel >>>> or bridge drivers that support this? My initial thought was that it >>>> should be a new bus format. >>> >>> Looks like we have drm bridges supporting dual link, and they use the >>> "normal" bus format. Did you have a look at them? They require two >>> port nodes for dual link, and use the existence of the second one to >>> decide if dual link is used or not. >> The above edits were not for adding a new bus format for dual link >> connections. I added them in order to be able to write the correct OLDI >> config values in the register. >> >>> >>> There are also lvds helpers in drm_of.c. I didn't look closely, but >>> it looked to me that the helpers can tell you if the ports are >>> connected to a dual link bridge. If not, you could fall back to >>> cloning. This way no extra properties are needed. But you will need >>> to add a port node, which I think you need to add anyway for cloning. >> I have now seen drm_of.c and examples (renesas' rcar lvds) that use the >> apis that drm_of.c is offering. In those cases, the OLDI TXes are being >> modeled as separate devices, which is not the case with the tidss' OLDI >> TXes. Since the only few OLDI registers are in the DSS address space, >> they were just being configured through the tidss driver. > > I think it's irrelevant (in the bigger picture) whether the TXes are > separate devices, single device or part of some other device. Or why do > you think it matters? > Sorry, by separate I meant having a separate driver irrespective of whether or not its a part of another device. >> Even in DT, the dss port (for OLDI) connects to the panel port's >> endpoint directly. Even in cases of dual link or cloning, it's only a >> singular remote-to-endpoint connection between the (OLDI) VP and the >> panel port. Hence the requirement of the properties in the earlier >> patches of the series. > > Sorry, I don't follow. If you use cloning, you have two TX outputs, > going to two panels, right? So you need two panel DT nodes, and those > would connect to two OLDI TX ports in the DSS. > > Afaics the existing dual link bridge/panel drivers also use two ports > for the connection, so to use the dual link you need two ports in the DSS. > > I admit I'm not familiar with LVDS dual link, but it's not clear to me > how you see the dual OLDI TX being used with other drivers if you have > only one port. What kind of setups have you tested? > In the DTs, the OLDIs are not modeled at all. Since the DSS only has a single VP for OLDI, the DT dss port (for OLDI) is connected to a single simple-panel node for dual link, bypassing the OLDI TX in DT. I have this same OLDI setup and have been testing on this. I do not have a cloning display setup with me, but I have seen DT DSS port connected to one of 2 panel nodes while the other panel (remains as a companion panel to the first) without any endpoint connections. Since, the OLDI TXes (0 and 1), receive the same clocks and inputs from DSS OLDI VP, this 'method' has worked too. >> The use of lvds helper functions does not seem feasible in this case, >> because even they read DT properties to determine the dual link >> connection and those properties need to be a part of a lvds bridge >> device. > > Can you elaborate a bit more why the DRM helpers couldn't be used here? > The drm_of.c helpers use DT properties to ascertain the presence of a dual-link connection. While there wasn't a specific helper to determine dual-link or not, the drivers use the odd/even pixel order helper which is based on the properties "dual-lvds-odd-pixels" and "dual-lvds-odd- pixels". If either of the properties are absent, the helper returns an error making the driver to use single link. These properties are LVDS specific, but they could not be added in the DT because there is no OLDI TX DT node for our case. >> I have also been considering the idea of implementing a new device >> driver for the OLDI TXes, not unlike the renesas' one. That way the >> driver could have the properties and the lvds helper functions at their >> disposal. I am just slightly unsure if that would allow space for any >> conflicts because of the shared register space. > > No, I don't think new devices are needed here. Okay... I am not quite sure I understand completely what you are recommending the OLDI to be. It seems to me that you want the OLDI TXes to be modeled as nodes, right? Wouldn't that automatically require some sort of standalone driver arrangement for them? Or am I missing something important here? Regards Aradhya
On 09/08/2022 12:06, Aradhya Bhatia wrote: >>> Even in DT, the dss port (for OLDI) connects to the panel port's >>> endpoint directly. Even in cases of dual link or cloning, it's only a >>> singular remote-to-endpoint connection between the (OLDI) VP and the >>> panel port. Hence the requirement of the properties in the earlier >>> patches of the series. >> >> Sorry, I don't follow. If you use cloning, you have two TX outputs, >> going to two panels, right? So you need two panel DT nodes, and those >> would connect to two OLDI TX ports in the DSS. >> > Afaics the existing dual link bridge/panel drivers also use two ports >> for the connection, so to use the dual link you need two ports in the >> DSS. >> >> I admit I'm not familiar with LVDS dual link, but it's not clear to me >> how you see the dual OLDI TX being used with other drivers if you have >> only one port. What kind of setups have you tested? >> > In the DTs, the OLDIs are not modeled at all. Since the DSS only has a > single VP for OLDI, the DT dss port (for OLDI) is connected to a single > simple-panel node for dual link, bypassing the OLDI TX in DT. I have > this same OLDI setup and have been testing on this. A DSS VP is a DSS internal port, whereas a port node in the DT is an external port. There doesn't have to be a 1:1 match between those. The port in the DT represents some kind of "connector" to the outside world, which is usually a collection of pins that provide a video bus. Here, as far as I can see, the DSS clearly has three external ports, two OLDI ports and one DPI port. > I do not have a cloning display setup with me, but I have seen DT DSS > port connected to one of 2 panel nodes while the other panel (remains as > a companion panel to the first) without any endpoint connections. Since, > the OLDI TXes (0 and 1), receive the same clocks and inputs from DSS > OLDI VP, this 'method' has worked too. This, and using simple-panel for dual link with single port connection, sounds like a hack. A practical example: TI's customer wants to use AM625 and THC63LVD1024 bridge. How does it work? THC63LVD1024 driver uses two LVDS ports for input, both of which are used in dual-link mode. >>> The use of lvds helper functions does not seem feasible in this case, >>> because even they read DT properties to determine the dual link >>> connection and those properties need to be a part of a lvds bridge >>> device. >> >> Can you elaborate a bit more why the DRM helpers couldn't be used here? >> > The drm_of.c helpers use DT properties to ascertain the presence of a > dual-link connection. While there wasn't a specific helper to determine > dual-link or not, the drivers use the odd/even pixel order helper which > is based on the properties "dual-lvds-odd-pixels" and "dual-lvds-odd- > pixels". If either of the properties are absent, the helper returns an > error making the driver to use single link. > > These properties are LVDS specific, but they could not be added in the > DT because there is no OLDI TX DT node for our case. If I'm not mistaken, those properties are in the port node, not the device node, and also, I believe those properties are on the sink side, so they wouldn't even be in the AM625 data. See, for example: arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts >>> I have also been considering the idea of implementing a new device >>> driver for the OLDI TXes, not unlike the renesas' one. That way the >>> driver could have the properties and the lvds helper functions at their >>> disposal. I am just slightly unsure if that would allow space for any >>> conflicts because of the shared register space. >> >> No, I don't think new devices are needed here. > Okay... > > I am not quite sure I understand completely what you are recommending > the OLDI to be. It seems to me that you want the OLDI TXes to be modeled > as nodes, right? Wouldn't that automatically require some sort of > standalone driver arrangement for them? Or am I missing something > important here? No, I'm only talking about the DT port nodes. At the moment the AM65x DT bindings doc says that there are two ports, port@0 for OLDI and port@1 for DPI. I'm saying AM625 needs three ports. Tomi
Hi Tomi, On 09-Aug-22 15:21, Tomi Valkeinen wrote: > On 09/08/2022 12:06, Aradhya Bhatia wrote: > >>>> Even in DT, the dss port (for OLDI) connects to the panel port's >>>> endpoint directly. Even in cases of dual link or cloning, it's only a >>>> singular remote-to-endpoint connection between the (OLDI) VP and the >>>> panel port. Hence the requirement of the properties in the earlier >>>> patches of the series. >>> >>> Sorry, I don't follow. If you use cloning, you have two TX outputs, >>> going to two panels, right? So you need two panel DT nodes, and those >>> would connect to two OLDI TX ports in the DSS. >>> > Afaics the existing dual link bridge/panel drivers also use two ports >>> for the connection, so to use the dual link you need two ports in the >>> DSS. >>> >>> I admit I'm not familiar with LVDS dual link, but it's not clear to >>> me how you see the dual OLDI TX being used with other drivers if you >>> have only one port. What kind of setups have you tested? >>> >> In the DTs, the OLDIs are not modeled at all. Since the DSS only has a >> single VP for OLDI, the DT dss port (for OLDI) is connected to a single >> simple-panel node for dual link, bypassing the OLDI TX in DT. I have >> this same OLDI setup and have been testing on this. > > A DSS VP is a DSS internal port, whereas a port node in the DT is an > external port. There doesn't have to be a 1:1 match between those. > > The port in the DT represents some kind of "connector" to the outside > world, which is usually a collection of pins that provide a video bus. > Okay, I now understand what you are saying. Indeed, I was mapping the DSS VP and DT DSS port as 1:1 in my mind, which should not be the case. > Here, as far as I can see, the DSS clearly has three external ports, two > OLDI ports and one DPI port. > >> I do not have a cloning display setup with me, but I have seen DT DSS >> port connected to one of 2 panel nodes while the other panel (remains as >> a companion panel to the first) without any endpoint connections. Since, >> the OLDI TXes (0 and 1), receive the same clocks and inputs from DSS >> OLDI VP, this 'method' has worked too. > > This, and using simple-panel for dual link with single port connection, > sounds like a hack. > > A practical example: TI's customer wants to use AM625 and THC63LVD1024 > bridge. How does it work? THC63LVD1024 driver uses two LVDS ports for > input, both of which are used in dual-link mode. > Right. There has to be 2 ports for OLDI in DSS, to be connected to 2 ports of a single panel (dual link) or 2 ports of 2 panels (cloning). >>>> The use of lvds helper functions does not seem feasible in this case, >>>> because even they read DT properties to determine the dual link >>>> connection and those properties need to be a part of a lvds bridge >>>> device. >>> >>> Can you elaborate a bit more why the DRM helpers couldn't be used here? >>> >> The drm_of.c helpers use DT properties to ascertain the presence of a >> dual-link connection. While there wasn't a specific helper to determine >> dual-link or not, the drivers use the odd/even pixel order helper which >> is based on the properties "dual-lvds-odd-pixels" and "dual-lvds-odd- >> pixels". If either of the properties are absent, the helper returns an >> error making the driver to use single link. >> >> These properties are LVDS specific, but they could not be added in the >> DT because there is no OLDI TX DT node for our case. > > If I'm not mistaken, those properties are in the port node, not the > device node, and also, I believe those properties are on the sink side, > so they wouldn't even be in the AM625 data. See, for example: > > arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts Yeah, they are indeed on the sink side. I was misunderstood about this. And the onus for properties is not on DSS nodes anymore. This probably is a different discussion, but since the sink is now responsible for those properties, these should get introduced in the panel-common bindings, right? The above example has a separate binding, but many dumb dual-link panels will require those properties in panel-common. > >>>> I have also been considering the idea of implementing a new device >>>> driver for the OLDI TXes, not unlike the renesas' one. That way the >>>> driver could have the properties and the lvds helper functions at their >>>> disposal. I am just slightly unsure if that would allow space for any >>>> conflicts because of the shared register space. >>> >>> No, I don't think new devices are needed here. >> Okay... >> >> I am not quite sure I understand completely what you are recommending >> the OLDI to be. It seems to me that you want the OLDI TXes to be modeled >> as nodes, right? Wouldn't that automatically require some sort of >> standalone driver arrangement for them? Or am I missing something >> important here? > > No, I'm only talking about the DT port nodes. At the moment the AM65x DT > bindings doc says that there are two ports, port@0 for OLDI and port@1 > for DPI. I'm saying AM625 needs three ports. Agreed. Moreover, I will update the binding to reflect 3 ports for am625-dss. Regards Aradhya
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c index add725fa682b..fb1fdecfc83a 100644 --- a/drivers/gpu/drm/tidss/tidss_dispc.c +++ b/drivers/gpu/drm/tidss/tidss_dispc.c @@ -853,25 +853,36 @@ void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask) } } -enum dispc_oldi_mode_reg_val { SPWG_18 = 0, JEIDA_24 = 1, SPWG_24 = 2 }; +enum dispc_oldi_mode_reg_val { + SPWG_18 = 0, + JEIDA_24 = 1, + SPWG_24 = 2, + DL_SPWG_18 = 4, + DL_JEIDA_24 = 5, + DL_SPWG_24 = 6, +}; struct dispc_bus_format { u32 bus_fmt; u32 data_width; bool is_oldi_fmt; + bool is_dual_link; enum dispc_oldi_mode_reg_val oldi_mode_reg_val; }; static const struct dispc_bus_format dispc_bus_formats[] = { - { MEDIA_BUS_FMT_RGB444_1X12, 12, false, 0 }, - { MEDIA_BUS_FMT_RGB565_1X16, 16, false, 0 }, - { MEDIA_BUS_FMT_RGB666_1X18, 18, false, 0 }, - { MEDIA_BUS_FMT_RGB888_1X24, 24, false, 0 }, - { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, 0 }, - { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, 0 }, - { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, SPWG_18 }, - { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, SPWG_24 }, - { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, JEIDA_24 }, + { MEDIA_BUS_FMT_RGB444_1X12, 12, false, false, 0 }, + { MEDIA_BUS_FMT_RGB565_1X16, 16, false, false, 0 }, + { MEDIA_BUS_FMT_RGB666_1X18, 18, false, false, 0 }, + { MEDIA_BUS_FMT_RGB888_1X24, 24, false, false, 0 }, + { MEDIA_BUS_FMT_RGB101010_1X30, 30, false, false, 0 }, + { MEDIA_BUS_FMT_RGB121212_1X36, 36, false, false, 0 }, + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, false, SPWG_18 }, + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, false, SPWG_24 }, + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, false, JEIDA_24 }, + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, true, true, DL_SPWG_18 }, + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, true, true, DL_SPWG_24 }, + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, true, true, DL_JEIDA_24 }, }; static const @@ -880,9 +891,15 @@ struct dispc_bus_format *dispc_vp_find_bus_fmt(struct dispc_device *dispc, u32 bus_fmt, u32 bus_flags) { unsigned int i; + bool is_dual_link = false; + + if (dispc->feat->vp_bus_type[hw_videoport] == DISPC_VP_OLDI && + dispc->oldi_mode == OLDI_DUAL_LINK) + is_dual_link = true; for (i = 0; i < ARRAY_SIZE(dispc_bus_formats); ++i) { - if (dispc_bus_formats[i].bus_fmt == bus_fmt) + if (dispc_bus_formats[i].bus_fmt == bus_fmt && + dispc_bus_formats[i].is_dual_link == is_dual_link) return &dispc_bus_formats[i]; }
The 2 OLDI TXes in the AM625 SoC can be synced together to output a 2K resolution video. Add support in the driver for the discovery of such a dual mode connection on the OLDI video port, using the values of "ti,oldi-mode" property. Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> --- drivers/gpu/drm/tidss/tidss_dispc.c | 39 +++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 11 deletions(-)