Re: [PATCH v2 17/21] drm/imx: pd: Use bus format/flags provided by the bridge when available

From: Philipp Zabel <p.zabel@pengutronix.de>
To: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Andrey Smirnov <andrew.smirnov@gmail.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	dri-devel@lists.freedesktop.org,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Chris Healy <Chris.Healy@zii.aero>,
	kernel@collabora.com, Sam Ravnborg <sam@ravnborg.org>
Subject: Re: [PATCH v2 17/21] drm/imx: pd: Use bus format/flags provided by the bridge when available
Date: Tue, 27 Aug 2019 15:49:02 +0200	[thread overview]
Message-ID: <1566913742.4102.13.camel@pengutronix.de> (raw)
In-Reply-To: <20190827152013.0d7aff4e@collabora.com>

On Tue, 2019-08-27 at 15:20 +0200, Boris Brezillon wrote:
> On Tue, 27 Aug 2019 14:51:20 +0200
> Philipp Zabel <p.zabel@pengutronix.de> wrote:
>  
> > [...]
> > > > > I can do that if you like. Note that we are forwarding
> > > > > the ->output_bus_cfg.fmt value to the IPU DI, not ->input_bus_cfg.fmt.
> > > > > I just assumed that input format wouldn't be used in the dummy bridge
> > > > > element (the one embedded in the encoder) since encoders only have an
> > > > > output end (input port is likely to be a SoC specific link between the
> > > > > CRTC and the encoder which we probably don't need/want to expose).    
> > > > 
> > > > Then why (would this driver have to) implement get_input_fmts at all?  
> > > 
> > > That's the only way we can check if an output format is supported: bus
> > > format negotiation is based on a trial and error logic that starts from
> > > the end of the pipeline (last bridge element) and goes backward until
> > > it reaches the first bridge (the dummy encoder bridge). A bus format
> > > setting is validated when all ->get_input_bus_fmts() hooks returned at
> > > least one possible format (*num_input_formats > 0) for the output format
> > > being tested by the next element in the chain. And that's why even the
> > > dummy encoder bridge has to implement this hook unless it only supports
> > > one output format (the core returns MEDIA_BUS_FMT_FIXED when  
> > > ->get_input_bus_fmts is NULL).  
> > 
> > This function (currently) also only returns MEDIA_BUS_FMT_FIXED, so
> > there is no difference in return value (if queried with a supported
> > output_fmt).
> 
> There's a small difference actually: when output_fmt !=
> MEDIA_BUS_FMT_FIXED, we make sure output_fmt is something we support. If
> you don't implement ->get_input_bus_fmts() you'll just accept any format
> requested by the next element in the pipeline.

I see. My whole point was predicated on the incorrect assumption that
get_input_bus_fmts would not have to be fed usupported output_fmts.

> > select_bus_fmt_recursive could just check atomic_get_output_bus_fmts if
> > that is implemented, but atomic_get_input_bus_fmts isn't.
> 
> I'd like to use ->get_output_bus_fmts() only to retrieve supported
> output formats on the last bridge element, otherwise it makes the
> whole thing even more complex.

Ok, I should have paid more attention to the documentation in patch 16.

> > That point is moot though, if we propagate the output format to the
> > input format.
> 
> I'll do that then (and mandate that all encoder drivers do the same).

Sounds good. At least then there aren't two classes of bridges that
implement the negotiation callbacks differently.

regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel