drm: Supporting new connector model in tidss

drm: Supporting new connector model in tidss

[Nikhil Devshatwar]

Hi all,

I am trying to convert the upstream tidss drm driver to new
connector model.
The connector is getting created by the tidss driver and bridges are
attached with flag DRM_BRIDGE_ATTACH_NO_CONNECTOR
Here are some questions, regarding this:

1) Most of the info regarding bus_format and bus flags is coming from
the bridges. Is it okay to not populate connector->display_info with
bus_format and flags?

2) The "drm_atomic_bridge_chain_select_bus_fmts" does the format
negotiation. So is it okay for the encoder to simply pick the bus_format
from the first bridge's state?

3) What is the meaning of MEDIA_BUS_FMT_FIXED? Does it mean that the
bridge does not change the format from input to output?

4) The bus_flags are available in bridge->timings->input_bus_flags and
also in bridge_state->input_bus_cfg.flags. Which one should be used?

Regards,
Nikhil D
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: drm: Supporting new connector model in tidss

[Daniel Vetter]

On Mon, Oct 5, 2020 at 11:43 PM Nikhil Devshatwar <nikhil.nd@ti.com> wrote:
>
> Hi all,
>
> I am trying to convert the upstream tidss drm driver to new
> connector model.
> The connector is getting created by the tidss driver and bridges are
> attached with flag DRM_BRIDGE_ATTACH_NO_CONNECTOR
> Here are some questions, regarding this:
>
> 1) Most of the info regarding bus_format and bus flags is coming from
> the bridges. Is it okay to not populate connector->display_info with
> bus_format and flags?
>
> 2) The "drm_atomic_bridge_chain_select_bus_fmts" does the format
> negotiation. So is it okay for the encoder to simply pick the bus_format
> from the first bridge's state?
>
> 3) What is the meaning of MEDIA_BUS_FMT_FIXED? Does it mean that the
> bridge does not change the format from input to output?
>
> 4) The bus_flags are available in bridge->timings->input_bus_flags and
> also in bridge_state->input_bus_cfg.flags. Which one should be used?

Whatever the answers, please make sure that they're recorded as
updates to the kerneldoc comments for these functions/flags/struct
members.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: drm: Supporting new connector model in tidss

[Tomi Valkeinen]

Adding Boris who added bus format negotiation.

On 06/10/2020 00:31, Nikhil Devshatwar wrote:
> Hi all,
> 
> I am trying to convert the upstream tidss drm driver to new
> connector model.
> The connector is getting created by the tidss driver and bridges are
> attached with flag DRM_BRIDGE_ATTACH_NO_CONNECTOR
> Here are some questions, regarding this:

I was looking at this a bit, and below is my understanding. And I'm mostly talking about how things
should be with new code, not legacy code. Things are probably a bit more complex if you mix bridges
which implement different styles on how to deal with bus formats.

> 1) Most of the info regarding bus_format and bus flags is coming from
> the bridges. Is it okay to not populate connector->display_info with
> bus_format and flags?

drm_display_info describes the connected display and what goes on the wire to the display.

For monitors that's quite clear, and the data in display_info would reflect what the last bridge
needs to output. Most of the data comes from EDID, but I think bus format and flags do not. So a
bridge would need to fill them in, which doesn't make sense when we have a chain of bridges (which
would be the bridge to fill the data?). So for monitors, I think bus flags and formats in
display_info are unused.

For panels, I'm not sure. We have the bridge/panel.c which wraps the actual panel driver, so afaics
the panel is essentially the last bridge in the chain, and the connector is kind of a dummy
connector. But the panel driver fills in the display_info, and that's where the bridge/panel.c gets
the bus formats & flags for the negotiation.

Probably the above could be changed so that the panels take part of the negotiation process, and
then the bus formats and flags fields in the display_info could be removed.

> 2) The "drm_atomic_bridge_chain_select_bus_fmts" does the format
> negotiation. So is it okay for the encoder to simply pick the bus_format
> from the first bridge's state?

Yes, I think that is the idea. The first bridge's input is what the display controller's encoder
should output, and the negotiation should take care to provide something in the first bridge's state
for the input.

> 3) What is the meaning of MEDIA_BUS_FMT_FIXED? Does it mean that the
> bridge does not change the format from input to output?

I think it just means "undefined" here, and it's up to the drivers to decide what to do. I presume
this is mostly for drivers that don't support the new stuff, as each bridge should be able to tell
what formats & flags it supports.

> 4) The bus_flags are available in bridge->timings->input_bus_flags and
> also in bridge_state->input_bus_cfg.flags. Which one should be used?

I think bridge_state->input_bus_cfg. Although bridge->timings->input_bus_flags has some data that's
not in input_bus_cfg. If the drivers support the negotiation, I don't think
bridge->timings->input_bus_flags has any use.

Probably bridge->timings->input_bus_flags should be used as a fallback. So if a bridge is asked to
use MEDIA_BUS_FMT_FIXED as output (i.e. the next bridge doesn't support negotiation), then the
bridge might use a default format and also see if the next bridge has bridge->timings->input_bus_flags.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: drm: Supporting new connector model in tidss

[Boris Brezillon]

On Tue, 6 Oct 2020 10:07:39 +0300
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> Adding Boris who added bus format negotiation.
> 
> On 06/10/2020 00:31, Nikhil Devshatwar wrote:
> > Hi all,
> > 
> > I am trying to convert the upstream tidss drm driver to new
> > connector model.
> > The connector is getting created by the tidss driver and bridges are
> > attached with flag DRM_BRIDGE_ATTACH_NO_CONNECTOR
> > Here are some questions, regarding this:  
> 
> I was looking at this a bit, and below is my understanding. And I'm mostly talking about how things
> should be with new code, not legacy code. Things are probably a bit more complex if you mix bridges
> which implement different styles on how to deal with bus formats.
> 
> > 1) Most of the info regarding bus_format and bus flags is coming from
> > the bridges. Is it okay to not populate connector->display_info with
> > bus_format and flags?  
> 
> drm_display_info describes the connected display and what goes on the wire to the display.
> 
> For monitors that's quite clear, and the data in display_info would reflect what the last bridge
> needs to output. Most of the data comes from EDID, but I think bus format and flags do not. So a
> bridge would need to fill them in, which doesn't make sense when we have a chain of bridges (which
> would be the bridge to fill the data?). So for monitors, I think bus flags and formats in
> display_info are unused.
> 
> For panels, I'm not sure. We have the bridge/panel.c which wraps the actual panel driver, so afaics
> the panel is essentially the last bridge in the chain, and the connector is kind of a dummy
> connector. But the panel driver fills in the display_info, and that's where the bridge/panel.c gets
> the bus formats & flags for the negotiation.
> 
> Probably the above could be changed so that the panels take part of the negotiation process, and
> then the bus formats and flags fields in the display_info could be removed.

Yep, that'd be better to have the bus format/flags info provided by the
panel itself rather than passed through display info.

> 
> > 2) The "drm_atomic_bridge_chain_select_bus_fmts" does the format
> > negotiation. So is it okay for the encoder to simply pick the bus_format
> > from the first bridge's state?  
> 
> Yes, I think that is the idea. The first bridge's input is what the display controller's encoder
> should output, and the negotiation should take care to provide something in the first bridge's state
> for the input.

Exactly.

> 
> > 3) What is the meaning of MEDIA_BUS_FMT_FIXED? Does it mean that the
> > bridge does not change the format from input to output?  
> 
> I think it just means "undefined" here, and it's up to the drivers to decide what to do. I presume
> this is mostly for drivers that don't support the new stuff, as each bridge should be able to tell
> what formats & flags it supports.

Correct.

> 
> > 4) The bus_flags are available in bridge->timings->input_bus_flags and
> > also in bridge_state->input_bus_cfg.flags. Which one should be used?  
> 
> I think bridge_state->input_bus_cfg. Although bridge->timings->input_bus_flags has some data that's
> not in input_bus_cfg. If the drivers support the negotiation, I don't think
> bridge->timings->input_bus_flags has any use.

Oh, I didn't realize there was an input_bus_flags in the timings
struct. We should probably propagate those in
drm_atomic_bridge_propagate_bus_flags().

> 
> Probably bridge->timings->input_bus_flags should be used as a fallback. So if a bridge is asked to
> use MEDIA_BUS_FMT_FIXED as output (i.e. the next bridge doesn't support negotiation), then the
> bridge might use a default format and also see if the next bridge has bridge->timings->input_bus_flags.

I think this could be automated in
drm_atomic_bridge_propagate_bus_flags(). Right now we simply propagate
the output bus flags to the input end [1], but it probably makes more
sense to use the value in bridge->timings->input_bus_flags if present.

[1]https://elixir.bootlin.com/linux/v5.9-rc8/source/drivers/gpu/drm/drm_bridge.c#L971
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel