Re: [PATCH 3/5] drm: bridge: Propagate the bus flags from bridge->timings

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Nikhil Devshatwar <nikhil.nd@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Sekhar Nori <nsekhar@ti.com>,
	Swapnil Jakhade <sjakhade@cadence.com>,
	dri-devel@lists.freedesktop.org,
	Boris Brezillon <boris.brezillon@collabora.com>
Subject: Re: [PATCH 3/5] drm: bridge: Propagate the bus flags from bridge->timings
Date: Fri, 30 Oct 2020 00:48:17 +0200	[thread overview]
Message-ID: <20201029224817.GH15024@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20201028143453.njt3lak542wwx6jb@NiksLab>

Hello,

On Wed, Oct 28, 2020 at 08:04:53PM +0530, Nikhil Devshatwar wrote:
> On 14:31-20201021, Tomi Valkeinen wrote:
> > On 16/10/2020 13:39, Nikhil Devshatwar wrote:
> > > When the next bridge does not specify any bus flags, use the
> > > bridge->timings->input_bus_flags as fallback when propagating
> > > bus flags from next bridge to current bridge.
> > > 
> > > Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
> > > ---
> > >  drivers/gpu/drm/drm_bridge.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index 64f0effb52ac..8353723323ab 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -975,6 +975,13 @@ drm_atomic_bridge_propagate_bus_flags(struct drm_bridge *bridge,
> > >  	 * duplicate the "dummy propagation" logic.
> > >  	 */
> > >  	bridge_state->input_bus_cfg.flags = output_flags;
> > > +
> > > +	/*
> > > +	 * Use the bridge->timings->input_bus_flags as fallback if the next bridge
> > > +	 * does not specify the flags
> > > +	 */
> > > +	if (!bridge_state->input_bus_cfg.flags)
> > > +		bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags;
> > 
> > According to docs, timings can be NULL.

Correct.

> > And, hmm... It's too easy to get confused with these, but... If the bridge defines timings, and
> > timings->input_bus_flags != 0, should we always pick that, even if we got something via
> > output_flags? Logic being, if this bridge defines timings->input_bus_flags, it probably wants that
> > to be used regardless whether we got something from the next bridge.

timings->input_bus_flags is an API that predates format and flags
propagation along the pipeline. I would assume that drivers that
implement propagation should do so in a way that prioritize that API,
and either not report flags in the timings (in which case giving
priority to one or another wouldn't make a difference as both would
never be provided together), or would report flags in the timings as a
best effort fallback for display controller drivers that would look at
them exclusively without supporting the new API. I would thus think that
the flags obtained through format negotiation should be prioritized.

> Got it, the flags from timings if present should be prioritized rather
> than treating them as fallback.

-- 
Regards,

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