On 30/11/2020 11:46, Laurent Pinchart wrote:
Hi Nikhil,
On Mon, Nov 30, 2020 at 12:05:03PM +0530, Nikhil Devshatwar wrote:
On 14:51-20201125, Tomi Valkeinen wrote:
On 19/11/2020 18:01, Nikhil Devshatwar wrote:
Remove the old code to iterate over the bridge chain, as this is already done by the framework. The bridge state should have the negotiated bus format and flags. Use these from the bridge's state. If the bridge does not support format negotiation, error out and fail.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com Reviewed-by: Tomi Valkeinen tomi.valkeinen@ti.com
Notes: changes from v2: * Remove the old code and use the flags from the bridge state
drivers/gpu/drm/tidss/tidss_encoder.c | 36 +++++++++++---------------- 1 file changed, 14 insertions(+), 22 deletions(-)
If a first bridge (after the crtc) supports two bus formats as input, how does tidss choose between those? This patch just picks bstate->input_bus_cfg.format, and it's not clear to me which one that is (the first one?). Also, we don't check if tidss actually supports the bus format.
The selection is done by the framework in select_bus_fmt_recursive at drivers/gpu/drm/drm_bridge.c:810
My understanding is that currently, the format negotiation logic does not negotiate all the way till encoder, it stops only at the first_bridge.
Should we then implement a bridge in the tidss driver to model the internal encoder, in order to support format negotiation all the way to the tidss ?
I don't know, but it feels perhaps a bit odd. Then we would have crtc + encoder + bridge, which are actually all about the same HW block. And this would have to be done for all DRM drivers.
Tomi