Re: [PATCH v2 0/5] allow override of bus format in bridges

[Peter Rosin]

On 2018-04-09 14:51, Laurent Pinchart wrote:
> Hi Peter,
> 
> On Monday, 9 April 2018 10:04:05 EEST Peter Rosin wrote:
>> On 2018-04-04 14:35, Peter Rosin wrote:
>>> On 2018-04-04 11:07, Laurent Pinchart wrote:
 On Wednesday, 4 April 2018 09:34:41 EEST Daniel Vetter wrote:
> On Wed, Apr 4, 2018 at 12:28 AM, Laurent Pinchart wrote:
>> On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote:
>>> On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
 Hi!

 [I got to v2 sooner than expected]

 I have an Atmel sama5d31 hooked up to an lvds encoder and then
 on to an lvds panel. Which seems like something that has been
 done one or two times before...

 The problem is that the bus_format of the SoC and the panel do
 not agree. The SoC driver (atmel-hlcdc) can handle the
 rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
 wired for the rgb565 case. The lvds encoder supports rgb888 on
 its input side with the LSB wires for each color simply pulled
 down internally in the encoder in my case which means that the
 rgb565 bus_format is the format that works best. And the panel
 is expecting lvds (vesa-24), which is what the encoder outputs.

 The reason I "blame" the bus_format of the drm_connector is that
 with the below DT snippet, things do not work *exactly* due to
 that. At least, it starts to work if I hack the panel-lvds driver
 to report the rgb565 bus_format instead of vesa-24.

 panel: panel {
 compatible = "panel-lvds";
 
 width-mm = <304>;
 height-mm = <228;
 
 data-mapping = "vesa-24";
 
 panel-timing {
 // 1024x768 @ 60Hz (typical)
 clock-frequency = <5214 6500 7110>;
 hactive = <1024>;
 vactive = <768>;
 hfront-porch = <48 88 88>;
 hback-porch = <96 168 168>;
 hsync-len = <32 64 64>;
 vfront-porch = <8 13 14>;
 vback-porch = <8 13 14>;
 vsync-len = <8 12 14>;
 };
 
 port {
 panel_input: endpoint {
 remote-endpoint = <_encoder_output>;
 };
 };
 };
 
 lvds-encoder {
 compatible = "ti,ds90c185", "lvds-encoder";
 
 ports {
 #address-cells = <1>;
 #size-cells = <0>;
 
 port@0 {
 reg = <0>;
 
 lvds_encoder_input: endpoint {
 remote-endpoint =
 <_output>;
 };
 };
 
 port@1 {
 reg = <1>;
 
 lvds_encoder_output: endpoint {
 remote-endpoint = <_input>;
 };
 };
 };
 };

 But, instead of perverting the panel-lvds driver with support
 for a totally fake non-lvds bus_format, I intruduce an API that
 allows display controller drivers to query the required bus_format of
 any intermediate bridges, and match up with that instead of the
 formats given by the drm_connector. I trigger this with this addition
 to the lvds-encoder DT node:
 interface-pix-fmt = "rgb565";

 Naming is hard though, so I'm not sure if that's good?

 I threw in the first patch, since that is the actual lvds encoder
 I have in this case.

 Suggestions welcome.
>>>
>>> Took a quick look, feels rather un-atomic. And there's beend
>>> discussing for other bridge related state that we might want to track
>>> (like the full adjusted_mode that might need to be adjusted at each
>>> stage in the chain). So here's my suggestions:
>>>
>>> - Add an optional per-bridge internal state struct using the support
>>>   in https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-> 
>>> >   

Re: [PATCH v2 0/5] allow override of bus format in bridges

[Peter Rosin]

On 2018-04-04 14:35, Peter Rosin wrote:
> On 2018-04-04 11:07, Laurent Pinchart wrote:
>> Hi Daniel,
>>
>> On Wednesday, 4 April 2018 09:34:41 EEST Daniel Vetter wrote:
>>> On Wed, Apr 4, 2018 at 12:28 AM, Laurent Pinchart wrote:
 On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote:
> On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
>> Hi!
>>
>> [I got to v2 sooner than expected]
>>
>> I have an Atmel sama5d31 hooked up to an lvds encoder and then
>> on to an lvds panel. Which seems like something that has been
>> done one or two times before...
>>
>> The problem is that the bus_format of the SoC and the panel do
>> not agree. The SoC driver (atmel-hlcdc) can handle the
>> rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
>> wired for the rgb565 case. The lvds encoder supports rgb888 on
>> its input side with the LSB wires for each color simply pulled
>> down internally in the encoder in my case which means that the
>> rgb565 bus_format is the format that works best. And the panel
>> is expecting lvds (vesa-24), which is what the encoder outputs.
>>
>> The reason I "blame" the bus_format of the drm_connector is that
>> with the below DT snippet, things do not work *exactly* due to
>> that. At least, it starts to work if I hack the panel-lvds driver
>> to report the rgb565 bus_format instead of vesa-24.
>>
>> panel: panel {
>> compatible = "panel-lvds";
>> 
>> width-mm = <304>;
>> height-mm = <228;
>> 
>> data-mapping = "vesa-24";
>> 
>> panel-timing {
>> // 1024x768 @ 60Hz (typical)
>> clock-frequency = <5214 6500 7110>;
>> hactive = <1024>;
>> vactive = <768>;
>> hfront-porch = <48 88 88>;
>> hback-porch = <96 168 168>;
>> hsync-len = <32 64 64>;
>> vfront-porch = <8 13 14>;
>> vback-porch = <8 13 14>;
>> vsync-len = <8 12 14>;
>> };
>> 
>> port {
>> panel_input: endpoint {
>> remote-endpoint = <_encoder_output>;
>> };
>> };
>> };
>> 
>> lvds-encoder {
>> compatible = "ti,ds90c185", "lvds-encoder";
>> 
>> ports {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> 
>> port@0 {
>> reg = <0>;
>> 
>> lvds_encoder_input: endpoint {
>> remote-endpoint = <_output>;
>> };
>> };
>> 
>> port@1 {
>> reg = <1>;
>> 
>> lvds_encoder_output: endpoint {
>> remote-endpoint = <_input>;
>> };
>> };
>> };
>> };
>>
>> But, instead of perverting the panel-lvds driver with support
>> for a totally fake non-lvds bus_format, I intruduce an API that allows
>> display controller drivers to query the required bus_format of any
>> intermediate bridges, and match up with that instead of the formats
>> given by the drm_connector. I trigger this with this addition to the
>>
>> lvds-encoder DT node:
>> interface-pix-fmt = "rgb565";
>>
>> Naming is hard though, so I'm not sure if that's good?
>>
>> I threw in the first patch, since that is the actual lvds encoder
>> I have in this case.
>>
>> Suggestions welcome.
>
> Took a quick look, feels rather un-atomic. And there's beend discussing
> for other bridge related state that we might want to track (like the full
> adjusted_mode that might need to be adjusted at each stage in the chain).
> So here's my suggestions:
>
> - Add an optional per-bridge internal state struct using the support in
>   https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-> 
> >>   private-state
>
>   Yes it says "driver private", but since bridge is just helper stuff
>   that's all included. "driver private" == "not exposed as uapi" here.
>   Include all the usual convenience wrappers to get at the state for a
>   bridge.
>
> - Then stuff your bus_format into that new drm_bridge_state struct.
>

Re: [PATCH v2 0/5] allow override of bus format in bridges

[Laurent Pinchart]

Hi Peter,

On Monday, 9 April 2018 10:04:05 EEST Peter Rosin wrote:
> On 2018-04-04 14:35, Peter Rosin wrote:
> > On 2018-04-04 11:07, Laurent Pinchart wrote:
> >> On Wednesday, 4 April 2018 09:34:41 EEST Daniel Vetter wrote:
> >>> On Wed, Apr 4, 2018 at 12:28 AM, Laurent Pinchart wrote:
>  On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote:
> > On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
> >> Hi!
> >> 
> >> [I got to v2 sooner than expected]
> >> 
> >> I have an Atmel sama5d31 hooked up to an lvds encoder and then
> >> on to an lvds panel. Which seems like something that has been
> >> done one or two times before...
> >> 
> >> The problem is that the bus_format of the SoC and the panel do
> >> not agree. The SoC driver (atmel-hlcdc) can handle the
> >> rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
> >> wired for the rgb565 case. The lvds encoder supports rgb888 on
> >> its input side with the LSB wires for each color simply pulled
> >> down internally in the encoder in my case which means that the
> >> rgb565 bus_format is the format that works best. And the panel
> >> is expecting lvds (vesa-24), which is what the encoder outputs.
> >> 
> >> The reason I "blame" the bus_format of the drm_connector is that
> >> with the below DT snippet, things do not work *exactly* due to
> >> that. At least, it starts to work if I hack the panel-lvds driver
> >> to report the rgb565 bus_format instead of vesa-24.
> >> 
> >> panel: panel {
> >> compatible = "panel-lvds";
> >> 
> >> width-mm = <304>;
> >> height-mm = <228;
> >> 
> >> data-mapping = "vesa-24";
> >> 
> >> panel-timing {
> >> // 1024x768 @ 60Hz (typical)
> >> clock-frequency = <5214 6500 7110>;
> >> hactive = <1024>;
> >> vactive = <768>;
> >> hfront-porch = <48 88 88>;
> >> hback-porch = <96 168 168>;
> >> hsync-len = <32 64 64>;
> >> vfront-porch = <8 13 14>;
> >> vback-porch = <8 13 14>;
> >> vsync-len = <8 12 14>;
> >> };
> >> 
> >> port {
> >> panel_input: endpoint {
> >> remote-endpoint = <_encoder_output>;
> >> };
> >> };
> >> };
> >> 
> >> lvds-encoder {
> >> compatible = "ti,ds90c185", "lvds-encoder";
> >> 
> >> ports {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> 
> >> port@0 {
> >> reg = <0>;
> >> 
> >> lvds_encoder_input: endpoint {
> >> remote-endpoint =
> >> <_output>;
> >> };
> >> };
> >> 
> >> port@1 {
> >> reg = <1>;
> >> 
> >> lvds_encoder_output: endpoint {
> >> remote-endpoint = <_input>;
> >> };
> >> };
> >> };
> >> };
> >> 
> >> But, instead of perverting the panel-lvds driver with support
> >> for a totally fake non-lvds bus_format, I intruduce an API that
> >> allows display controller drivers to query the required bus_format of
> >> any intermediate bridges, and match up with that instead of the
> >> formats given by the drm_connector. I trigger this with this addition
> >> to the lvds-encoder DT node:
> >> interface-pix-fmt = "rgb565";
> >> 
> >> Naming is hard though, so I'm not sure if that's good?
> >> 
> >> I threw in the first patch, since that is the actual lvds encoder
> >> I have in this case.
> >> 
> >> Suggestions welcome.
> > 
> > Took a quick look, feels rather un-atomic. And there's beend
> > discussing for other bridge related state that we might want to track
> > (like the full adjusted_mode that might need to be adjusted at each
> > stage in the chain). So here's my suggestions:
> > 
> > - Add an optional per-bridge internal state struct using the support
> >   in https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-> 
> > >   driver-private-state
> >   
> >   Yes it 

Re: [PATCH v2 0/5] allow override of bus format in bridges

[Peter Rosin]

On 2018-04-04 11:07, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Wednesday, 4 April 2018 09:34:41 EEST Daniel Vetter wrote:
>> On Wed, Apr 4, 2018 at 12:28 AM, Laurent Pinchart wrote:
>>> On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote:
 On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
> Hi!
>
> [I got to v2 sooner than expected]
>
> I have an Atmel sama5d31 hooked up to an lvds encoder and then
> on to an lvds panel. Which seems like something that has been
> done one or two times before...
>
> The problem is that the bus_format of the SoC and the panel do
> not agree. The SoC driver (atmel-hlcdc) can handle the
> rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
> wired for the rgb565 case. The lvds encoder supports rgb888 on
> its input side with the LSB wires for each color simply pulled
> down internally in the encoder in my case which means that the
> rgb565 bus_format is the format that works best. And the panel
> is expecting lvds (vesa-24), which is what the encoder outputs.
>
> The reason I "blame" the bus_format of the drm_connector is that
> with the below DT snippet, things do not work *exactly* due to
> that. At least, it starts to work if I hack the panel-lvds driver
> to report the rgb565 bus_format instead of vesa-24.
>
> panel: panel {
> compatible = "panel-lvds";
> 
> width-mm = <304>;
> height-mm = <228;
> 
> data-mapping = "vesa-24";
> 
> panel-timing {
> // 1024x768 @ 60Hz (typical)
> clock-frequency = <5214 6500 7110>;
> hactive = <1024>;
> vactive = <768>;
> hfront-porch = <48 88 88>;
> hback-porch = <96 168 168>;
> hsync-len = <32 64 64>;
> vfront-porch = <8 13 14>;
> vback-porch = <8 13 14>;
> vsync-len = <8 12 14>;
> };
> 
> port {
> panel_input: endpoint {
> remote-endpoint = <_encoder_output>;
> };
> };
> };
> 
> lvds-encoder {
> compatible = "ti,ds90c185", "lvds-encoder";
> 
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
> 
> port@0 {
> reg = <0>;
> 
> lvds_encoder_input: endpoint {
> remote-endpoint = <_output>;
> };
> };
> 
> port@1 {
> reg = <1>;
> 
> lvds_encoder_output: endpoint {
> remote-endpoint = <_input>;
> };
> };
> };
> };
>
> But, instead of perverting the panel-lvds driver with support
> for a totally fake non-lvds bus_format, I intruduce an API that allows
> display controller drivers to query the required bus_format of any
> intermediate bridges, and match up with that instead of the formats
> given by the drm_connector. I trigger this with this addition to the
>
> lvds-encoder DT node:
> interface-pix-fmt = "rgb565";
>
> Naming is hard though, so I'm not sure if that's good?
>
> I threw in the first patch, since that is the actual lvds encoder
> I have in this case.
>
> Suggestions welcome.

 Took a quick look, feels rather un-atomic. And there's beend discussing
 for other bridge related state that we might want to track (like the full
 adjusted_mode that might need to be adjusted at each stage in the chain).
 So here's my suggestions:

 - Add an optional per-bridge internal state struct using the support in
   https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-> 
 >>   private-state

   Yes it says "driver private", but since bridge is just helper stuff
   that's all included. "driver private" == "not exposed as uapi" here.
   Include all the usual convenience wrappers to get at the state for a
   bridge.

 - Then stuff your bus_format into that new drm_bridge_state struct.

 - Add a new bridge callback atomic_check, which gets that bridge state as
   parameter (similar to all the other atomic_check functions).

Re: [PATCH v2 0/5] allow override of bus format in bridges

[Laurent Pinchart]

Hi Daniel,

On Wednesday, 4 April 2018 09:34:41 EEST Daniel Vetter wrote:
> On Wed, Apr 4, 2018 at 12:28 AM, Laurent Pinchart wrote:
> > On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote:
> >> On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
> >>> Hi!
> >>> 
> >>> [I got to v2 sooner than expected]
> >>> 
> >>> I have an Atmel sama5d31 hooked up to an lvds encoder and then
> >>> on to an lvds panel. Which seems like something that has been
> >>> done one or two times before...
> >>> 
> >>> The problem is that the bus_format of the SoC and the panel do
> >>> not agree. The SoC driver (atmel-hlcdc) can handle the
> >>> rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
> >>> wired for the rgb565 case. The lvds encoder supports rgb888 on
> >>> its input side with the LSB wires for each color simply pulled
> >>> down internally in the encoder in my case which means that the
> >>> rgb565 bus_format is the format that works best. And the panel
> >>> is expecting lvds (vesa-24), which is what the encoder outputs.
> >>> 
> >>> The reason I "blame" the bus_format of the drm_connector is that
> >>> with the below DT snippet, things do not work *exactly* due to
> >>> that. At least, it starts to work if I hack the panel-lvds driver
> >>> to report the rgb565 bus_format instead of vesa-24.
> >>> 
> >>> panel: panel {
> >>> compatible = "panel-lvds";
> >>> 
> >>> width-mm = <304>;
> >>> height-mm = <228;
> >>> 
> >>> data-mapping = "vesa-24";
> >>> 
> >>> panel-timing {
> >>> // 1024x768 @ 60Hz (typical)
> >>> clock-frequency = <5214 6500 7110>;
> >>> hactive = <1024>;
> >>> vactive = <768>;
> >>> hfront-porch = <48 88 88>;
> >>> hback-porch = <96 168 168>;
> >>> hsync-len = <32 64 64>;
> >>> vfront-porch = <8 13 14>;
> >>> vback-porch = <8 13 14>;
> >>> vsync-len = <8 12 14>;
> >>> };
> >>> 
> >>> port {
> >>> panel_input: endpoint {
> >>> remote-endpoint = <_encoder_output>;
> >>> };
> >>> };
> >>> };
> >>> 
> >>> lvds-encoder {
> >>> compatible = "ti,ds90c185", "lvds-encoder";
> >>> 
> >>> ports {
> >>> #address-cells = <1>;
> >>> #size-cells = <0>;
> >>> 
> >>> port@0 {
> >>> reg = <0>;
> >>> 
> >>> lvds_encoder_input: endpoint {
> >>> remote-endpoint = <_output>;
> >>> };
> >>> };
> >>> 
> >>> port@1 {
> >>> reg = <1>;
> >>> 
> >>> lvds_encoder_output: endpoint {
> >>> remote-endpoint = <_input>;
> >>> };
> >>> };
> >>> };
> >>> };
> >>> 
> >>> But, instead of perverting the panel-lvds driver with support
> >>> for a totally fake non-lvds bus_format, I intruduce an API that allows
> >>> display controller drivers to query the required bus_format of any
> >>> intermediate bridges, and match up with that instead of the formats
> >>> given by the drm_connector. I trigger this with this addition to the
> >>> 
> >>> lvds-encoder DT node:
> >>> interface-pix-fmt = "rgb565";
> >>> 
> >>> Naming is hard though, so I'm not sure if that's good?
> >>> 
> >>> I threw in the first patch, since that is the actual lvds encoder
> >>> I have in this case.
> >>> 
> >>> Suggestions welcome.
> >> 
> >> Took a quick look, feels rather un-atomic. And there's beend discussing
> >> for other bridge related state that we might want to track (like the full
> >> adjusted_mode that might need to be adjusted at each stage in the chain).
> >> So here's my suggestions:
> >> 
> >> - Add an optional per-bridge internal state struct using the support in
> >>   https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-> 
> >> >>   private-state
> >> 
> >>   Yes it says "driver private", but since bridge is just helper stuff
> >>   that's all included. "driver private" == "not exposed as uapi" here.
> >>   Include all the usual convenience wrappers to get at the state for a
> >>   bridge.
> >> 
> >> - Then stuff your bus_format into that new drm_bridge_state struct.
> >> 
> >> - Add a new bridge callback atomic_check, which gets that bridge state as
> >>   parameter (similar to all the other atomic_check functions).
> >> 
> >> This way we can even handle the 

Re: [PATCH v2 0/5] allow override of bus format in bridges

[Daniel Vetter]

On Wed, Apr 4, 2018 at 12:28 AM, Laurent Pinchart
 wrote:
> Hi Daniel,
>
> On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote:
>> On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
>> > Hi!
>> >
>> > [I got to v2 sooner than expected]
>> >
>> > I have an Atmel sama5d31 hooked up to an lvds encoder and then
>> > on to an lvds panel. Which seems like something that has been
>> > done one or two times before...
>> >
>> > The problem is that the bus_format of the SoC and the panel do
>> > not agree. The SoC driver (atmel-hlcdc) can handle the
>> > rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
>> > wired for the rgb565 case. The lvds encoder supports rgb888 on
>> > its input side with the LSB wires for each color simply pulled
>> > down internally in the encoder in my case which means that the
>> > rgb565 bus_format is the format that works best. And the panel
>> > is expecting lvds (vesa-24), which is what the encoder outputs.
>> >
>> > The reason I "blame" the bus_format of the drm_connector is that
>> > with the below DT snippet, things do not work *exactly* due to
>> > that. At least, it starts to work if I hack the panel-lvds driver
>> > to report the rgb565 bus_format instead of vesa-24.
>> >
>> > panel: panel {
>> > compatible = "panel-lvds";
>> >
>> > width-mm = <304>;
>> > height-mm = <228;
>> >
>> > data-mapping = "vesa-24";
>> >
>> > panel-timing {
>> > // 1024x768 @ 60Hz (typical)
>> > clock-frequency = <5214 6500 7110>;
>> > hactive = <1024>;
>> > vactive = <768>;
>> > hfront-porch = <48 88 88>;
>> > hback-porch = <96 168 168>;
>> > hsync-len = <32 64 64>;
>> > vfront-porch = <8 13 14>;
>> > vback-porch = <8 13 14>;
>> > vsync-len = <8 12 14>;
>> > };
>> >
>> > port {
>> > panel_input: endpoint {
>> > remote-endpoint = <_encoder_output>;
>> > };
>> > };
>> > };
>> >
>> > lvds-encoder {
>> > compatible = "ti,ds90c185", "lvds-encoder";
>> >
>> > ports {
>> > #address-cells = <1>;
>> > #size-cells = <0>;
>> >
>> > port@0 {
>> > reg = <0>;
>> >
>> > lvds_encoder_input: endpoint {
>> > remote-endpoint = <_output>;
>> > };
>> > };
>> >
>> > port@1 {
>> > reg = <1>;
>> >
>> > lvds_encoder_output: endpoint {
>> > remote-endpoint = <_input>;
>> > };
>> > };
>> > };
>> > };
>> >
>> > But, instead of perverting the panel-lvds driver with support
>> > for a totally fake non-lvds bus_format, I intruduce an API that allows
>> > display controller drivers to query the required bus_format of any
>> > intermediate bridges, and match up with that instead of the formats
>> > given by the drm_connector. I trigger this with this addition to the
>> >
>> > lvds-encoder DT node:
>> > interface-pix-fmt = "rgb565";
>> >
>> > Naming is hard though, so I'm not sure if that's good?
>> >
>> > I threw in the first patch, since that is the actual lvds encoder
>> > I have in this case.
>> >
>> > Suggestions welcome.
>>
>> Took a quick look, feels rather un-atomic. And there's beend discussing
>> for other bridge related state that we might want to track (like the full
>> adjusted_mode that might need to be adjusted at each stage in the chain).
>> So here's my suggestions:
>>
>> - Add an optional per-bridge internal state struct using the support in
>>
>> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-privat
>> e-state
>>
>>   Yes it says "driver private", but since bridge is just helper stuff
>>   that's all included. "driver private" == "not exposed as uapi" here.
>>   Include all the usual convenience wrappers to get at the state for a
>>   bridge.
>>
>> - Then stuff your bus_format into that new drm_bridge_state struct.
>>
>> - Add a new bridge callback atomic_check, which gets that bridge state as
>>   parameter (similar to all the other atomic_check functions).
>>
>> This way we can even handle the bus_format dynamically, through the atomic
>> framework your bridge's atomic_check callback can look at the entire
>> atomic state (both up and down the chain if needed), it all neatly fits
>> into atomic overall and it's much easier to extend.
>
> While I think we'll eventually need bridge states, I don't think that's need
> yet. The bus formats 

Re: [PATCH v2 0/5] allow override of bus format in bridges

[Laurent Pinchart]

Hi again,

On Wednesday, 4 April 2018 01:28:29 EEST Laurent Pinchart wrote:
> On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote:
> > On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
> > > Hi!
> > > 
> > > [I got to v2 sooner than expected]
> > > 
> > > I have an Atmel sama5d31 hooked up to an lvds encoder and then
> > > on to an lvds panel. Which seems like something that has been
> > > done one or two times before...
> > > 
> > > The problem is that the bus_format of the SoC and the panel do
> > > not agree. The SoC driver (atmel-hlcdc) can handle the
> > > rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
> > > wired for the rgb565 case. The lvds encoder supports rgb888 on
> > > its input side with the LSB wires for each color simply pulled
> > > down internally in the encoder in my case which means that the
> > > rgb565 bus_format is the format that works best. And the panel
> > > is expecting lvds (vesa-24), which is what the encoder outputs.
> > > 
> > > The reason I "blame" the bus_format of the drm_connector is that
> > > with the below DT snippet, things do not work *exactly* due to
> > > that. At least, it starts to work if I hack the panel-lvds driver
> > > to report the rgb565 bus_format instead of vesa-24.
> > > 
> > >   panel: panel {
> > >   compatible = "panel-lvds";
> > >   
> > >   width-mm = <304>;
> > >   height-mm = <228;
> > >   
> > >   data-mapping = "vesa-24";
> > >   
> > >   panel-timing {
> > >   // 1024x768 @ 60Hz (typical)
> > >   clock-frequency = <5214 6500 7110>;
> > >   hactive = <1024>;
> > >   vactive = <768>;
> > >   hfront-porch = <48 88 88>;
> > >   hback-porch = <96 168 168>;
> > >   hsync-len = <32 64 64>;
> > >   vfront-porch = <8 13 14>;
> > >   vback-porch = <8 13 14>;
> > >   vsync-len = <8 12 14>;
> > >   };
> > >   
> > >   port {
> > >   panel_input: endpoint {
> > >   remote-endpoint = <_encoder_output>;
> > >   };
> > >   };
> > >   };
> > >   
> > >   lvds-encoder {
> > >   compatible = "ti,ds90c185", "lvds-encoder";
> > >   
> > >   ports {
> > >   #address-cells = <1>;
> > >   #size-cells = <0>;
> > >   
> > >   port@0 {
> > >   reg = <0>;
> > >   
> > >   lvds_encoder_input: endpoint {
> > >   remote-endpoint = <_output>;
> > >   };
> > >   };
> > >   
> > >   port@1 {
> > >   reg = <1>;
> > >   
> > >   lvds_encoder_output: endpoint {
> > >   remote-endpoint = <_input>;
> > >   };
> > >   };
> > >   };
> > >   };
> > > 
> > > But, instead of perverting the panel-lvds driver with support
> > > for a totally fake non-lvds bus_format, I intruduce an API that allows
> > > display controller drivers to query the required bus_format of any
> > > intermediate bridges, and match up with that instead of the formats
> > > given by the drm_connector. I trigger this with this addition to the
> > > 
> > > lvds-encoder DT node:
> > >   interface-pix-fmt = "rgb565";
> > > 
> > > Naming is hard though, so I'm not sure if that's good?
> > > 
> > > I threw in the first patch, since that is the actual lvds encoder
> > > I have in this case.
> > > 
> > > Suggestions welcome.
> > 
> > Took a quick look, feels rather un-atomic. And there's beend discussing
> > for other bridge related state that we might want to track (like the full
> > adjusted_mode that might need to be adjusted at each stage in the chain).
> > So here's my suggestions:
> > 
> > - Add an optional per-bridge internal state struct using the support in
> > 
> > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-priv
> > at e-state
> > 
> >   Yes it says "driver private", but since bridge is just helper stuff
> >   that's all included. "driver private" == "not exposed as uapi" here.
> >   Include all the usual convenience wrappers to get at the state for a
> >   bridge.
> > 
> > - Then stuff your bus_format into that new drm_bridge_state struct.
> > 
> > - Add a new bridge callback atomic_check, which gets that bridge state as
> >   parameter (similar to all the other atomic_check functions).
> > 
> > This way we can even handle the bus_format dynamically, through the atomic
> > framework your bridge's atomic_check callback can look at the entire
> > atomic state (both up and down the chain if needed), it all neatly fits

Re: [PATCH v2 0/5] allow override of bus format in bridges

[Laurent Pinchart]

Hi Daniel,

On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote:
> On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
> > Hi!
> > 
> > [I got to v2 sooner than expected]
> > 
> > I have an Atmel sama5d31 hooked up to an lvds encoder and then
> > on to an lvds panel. Which seems like something that has been
> > done one or two times before...
> > 
> > The problem is that the bus_format of the SoC and the panel do
> > not agree. The SoC driver (atmel-hlcdc) can handle the
> > rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
> > wired for the rgb565 case. The lvds encoder supports rgb888 on
> > its input side with the LSB wires for each color simply pulled
> > down internally in the encoder in my case which means that the
> > rgb565 bus_format is the format that works best. And the panel
> > is expecting lvds (vesa-24), which is what the encoder outputs.
> > 
> > The reason I "blame" the bus_format of the drm_connector is that
> > with the below DT snippet, things do not work *exactly* due to
> > that. At least, it starts to work if I hack the panel-lvds driver
> > to report the rgb565 bus_format instead of vesa-24.
> > 
> > panel: panel {
> > compatible = "panel-lvds";
> > 
> > width-mm = <304>;
> > height-mm = <228;
> > 
> > data-mapping = "vesa-24";
> > 
> > panel-timing {
> > // 1024x768 @ 60Hz (typical)
> > clock-frequency = <5214 6500 7110>;
> > hactive = <1024>;
> > vactive = <768>;
> > hfront-porch = <48 88 88>;
> > hback-porch = <96 168 168>;
> > hsync-len = <32 64 64>;
> > vfront-porch = <8 13 14>;
> > vback-porch = <8 13 14>;
> > vsync-len = <8 12 14>;
> > };
> > 
> > port {
> > panel_input: endpoint {
> > remote-endpoint = <_encoder_output>;
> > };
> > };
> > };
> > 
> > lvds-encoder {
> > compatible = "ti,ds90c185", "lvds-encoder";
> > 
> > ports {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > 
> > port@0 {
> > reg = <0>;
> > 
> > lvds_encoder_input: endpoint {
> > remote-endpoint = <_output>;
> > };
> > };
> > 
> > port@1 {
> > reg = <1>;
> > 
> > lvds_encoder_output: endpoint {
> > remote-endpoint = <_input>;
> > };
> > };
> > };
> > };
> > 
> > But, instead of perverting the panel-lvds driver with support
> > for a totally fake non-lvds bus_format, I intruduce an API that allows
> > display controller drivers to query the required bus_format of any
> > intermediate bridges, and match up with that instead of the formats
> > given by the drm_connector. I trigger this with this addition to the
> > 
> > lvds-encoder DT node:
> > interface-pix-fmt = "rgb565";
> > 
> > Naming is hard though, so I'm not sure if that's good?
> > 
> > I threw in the first patch, since that is the actual lvds encoder
> > I have in this case.
> > 
> > Suggestions welcome.
> 
> Took a quick look, feels rather un-atomic. And there's beend discussing
> for other bridge related state that we might want to track (like the full
> adjusted_mode that might need to be adjusted at each stage in the chain).
> So here's my suggestions:
> 
> - Add an optional per-bridge internal state struct using the support in
> 
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-privat
> e-state
> 
>   Yes it says "driver private", but since bridge is just helper stuff
>   that's all included. "driver private" == "not exposed as uapi" here.
>   Include all the usual convenience wrappers to get at the state for a
>   bridge.
> 
> - Then stuff your bus_format into that new drm_bridge_state struct.
> 
> - Add a new bridge callback atomic_check, which gets that bridge state as
>   parameter (similar to all the other atomic_check functions).
> 
> This way we can even handle the bus_format dynamically, through the atomic
> framework your bridge's atomic_check callback can look at the entire
> atomic state (both up and down the chain if needed), it all neatly fits
> into atomic overall and it's much easier to extend.

While I think we'll eventually need bridge states, I don't think that's need 
yet. The bus formats reported by this patch series are 

Re: [PATCH v2 0/5] allow override of bus format in bridges

[Daniel Vetter]

On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
> Hi!
> 
> [I got to v2 sooner than expected]
> 
> I have an Atmel sama5d31 hooked up to an lvds encoder and then
> on to an lvds panel. Which seems like something that has been
> done one or two times before...
> 
> The problem is that the bus_format of the SoC and the panel do
> not agree. The SoC driver (atmel-hlcdc) can handle the
> rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
> wired for the rgb565 case. The lvds encoder supports rgb888 on
> its input side with the LSB wires for each color simply pulled
> down internally in the encoder in my case which means that the
> rgb565 bus_format is the format that works best. And the panel
> is expecting lvds (vesa-24), which is what the encoder outputs.
> 
> The reason I "blame" the bus_format of the drm_connector is that
> with the below DT snippet, things do not work *exactly* due to
> that. At least, it starts to work if I hack the panel-lvds driver
> to report the rgb565 bus_format instead of vesa-24.
> 
>   panel: panel {
>   compatible = "panel-lvds";
> 
>   width-mm = <304>;
>   height-mm = <228;
> 
>   data-mapping = "vesa-24";
> 
>   panel-timing {
>   // 1024x768 @ 60Hz (typical)
>   clock-frequency = <5214 6500 7110>;
>   hactive = <1024>;
>   vactive = <768>;
>   hfront-porch = <48 88 88>;
>   hback-porch = <96 168 168>;
>   hsync-len = <32 64 64>;
>   vfront-porch = <8 13 14>;
>   vback-porch = <8 13 14>;
>   vsync-len = <8 12 14>;
>   };
> 
>   port {
>   panel_input: endpoint {
>   remote-endpoint = <_encoder_output>;
>   };
>   };
>   };
> 
>   lvds-encoder {
>   compatible = "ti,ds90c185", "lvds-encoder";
> 
>   ports {
>   #address-cells = <1>;
>   #size-cells = <0>;
> 
>   port@0 {
>   reg = <0>;
> 
>   lvds_encoder_input: endpoint {
>   remote-endpoint = <_output>;
>   };
>   };
> 
>   port@1 {
>   reg = <1>;
> 
>   lvds_encoder_output: endpoint {
>   remote-endpoint = <_input>;
>   };
>   };
>   };
>   };
> 
> But, instead of perverting the panel-lvds driver with support
> for a totally fake non-lvds bus_format, I intruduce an API that allows
> display controller drivers to query the required bus_format of any
> intermediate bridges, and match up with that instead of the formats
> given by the drm_connector. I trigger this with this addition to the
> lvds-encoder DT node:
> 
>   interface-pix-fmt = "rgb565";
> 
> Naming is hard though, so I'm not sure if that's good?
> 
> I threw in the first patch, since that is the actual lvds encoder
> I have in this case.
> 
> Suggestions welcome.

Took a quick look, feels rather un-atomic. And there's beend discussing
for other bridge related state that we might want to track (like the full
adjusted_mode that might need to be adjusted at each stage in the chain).
So here's my suggestions:

- Add an optional per-bridge internal state struct using the support in

https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-private-state

  Yes it says "driver private", but since bridge is just helper stuff
  that's all included. "driver private" == "not exposed as uapi" here.
  Include all the usual convenience wrappers to get at the state for a
  bridge.

- Then stuff your bus_format into that new drm_bridge_state struct.

- Add a new bridge callback atomic_check, which gets that bridge state as
  parameter (similar to all the other atomic_check functions).

This way we can even handle the bus_format dynamically, through the atomic
framework your bridge's atomic_check callback can look at the entire
atomic state (both up and down the chain if needed), it all neatly fits
into atomic overall and it's much easier to extend.

Please also cc Laurent Pinchart on this.

Cheers, Daniel


> 
> Cheers,
> Peter
> 
> Changes since v1 https://lkml.org/lkml/2018/3/17/221
> - Add a proper bridge API to query the bus_format instead of abusing
>   the ->get_modes part of the code. This is cleaner but requires
>   changes to all display controller drivers wishing to participate.
> - Add patch to adjust the atmel-hlcdc driver according to the above.
> - Hook the new info into the bridge local to the lvds-encoder instead
>   of messing about with 

[PATCH v2 0/5] allow override of bus format in bridges

[Peter Rosin]

Hi!

[I got to v2 sooner than expected]

I have an Atmel sama5d31 hooked up to an lvds encoder and then
on to an lvds panel. Which seems like something that has been
done one or two times before...

The problem is that the bus_format of the SoC and the panel do
not agree. The SoC driver (atmel-hlcdc) can handle the
rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
wired for the rgb565 case. The lvds encoder supports rgb888 on
its input side with the LSB wires for each color simply pulled
down internally in the encoder in my case which means that the
rgb565 bus_format is the format that works best. And the panel
is expecting lvds (vesa-24), which is what the encoder outputs.

The reason I "blame" the bus_format of the drm_connector is that
with the below DT snippet, things do not work *exactly* due to
that. At least, it starts to work if I hack the panel-lvds driver
to report the rgb565 bus_format instead of vesa-24.

panel: panel {
compatible = "panel-lvds";

width-mm = <304>;
height-mm = <228;

data-mapping = "vesa-24";

panel-timing {
// 1024x768 @ 60Hz (typical)
clock-frequency = <5214 6500 7110>;
hactive = <1024>;
vactive = <768>;
hfront-porch = <48 88 88>;
hback-porch = <96 168 168>;
hsync-len = <32 64 64>;
vfront-porch = <8 13 14>;
vback-porch = <8 13 14>;
vsync-len = <8 12 14>;
};

port {
panel_input: endpoint {
remote-endpoint = <_encoder_output>;
};
};
};

lvds-encoder {
compatible = "ti,ds90c185", "lvds-encoder";

ports {
#address-cells = <1>;
#size-cells = <0>;

port@0 {
reg = <0>;

lvds_encoder_input: endpoint {
remote-endpoint = <_output>;
};
};

port@1 {
reg = <1>;

lvds_encoder_output: endpoint {
remote-endpoint = <_input>;
};
};
};
};

But, instead of perverting the panel-lvds driver with support
for a totally fake non-lvds bus_format, I intruduce an API that allows
display controller drivers to query the required bus_format of any
intermediate bridges, and match up with that instead of the formats
given by the drm_connector. I trigger this with this addition to the
lvds-encoder DT node:

interface-pix-fmt = "rgb565";

Naming is hard though, so I'm not sure if that's good?

I threw in the first patch, since that is the actual lvds encoder
I have in this case.

Suggestions welcome.

Cheers,
Peter

Changes since v1 https://lkml.org/lkml/2018/3/17/221
- Add a proper bridge API to query the bus_format instead of abusing
  the ->get_modes part of the code. This is cleaner but requires
  changes to all display controller drivers wishing to participate.
- Add patch to adjust the atmel-hlcdc driver according to the above.
- Hook the new info into the bridge local to the lvds-encoder instead
  of messing about with new interfaces for the panel-bridge driver.
- Add patch with a DT parsing function of bus_formats in a central place.
- Rephrase the addition of ti,ds90c185 to the lvds-transmitter binding.

Peter Rosin (5):
  dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185
  drm: bridge: add API to query the expected input formats of bridges
  drm: of: add display bus-format parser
  drm: bridge: lvds-encoder: allow specifying the input bus format
  drm/atmel-hlcdc: take bridges into account when selecting output
format

 .../bindings/display/bridge/lvds-transmitter.txt   | 14 -
 .../devicetree/bindings/display/bus-format.txt | 35 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 49 --
 drivers/gpu/drm/bridge/lvds-encoder.c  | 25 +
 drivers/gpu/drm/drm_bridge.c   | 32 
 drivers/gpu/drm/drm_of.c   | 59 ++
 include/drm/drm_bridge.h   | 18 +++
 include/drm/drm_of.h   |  9 
 8 files changed, 237 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bus-format.txt

-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org