Re: [PATCH V2 3/4] drm/panel: Add prepare_upstream_first flag to drm_panel

From: Dave Stevenson <dave.stevenson@raspberrypi.com>
To: Jagan Teki <jagan@amarulasolutions.com>
Cc: Marek Vasut <marex@denx.de>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Neil Armstrong <narmstrong@baylibre.com>,
	David Airlie <airlied@linux.ie>,
	Robert Foss <robert.foss@linaro.org>,
	Jonas Karlman <jonas@kwiboo.se>,
	Douglas Anderson <dianders@chromium.org>,
	dri-devel@lists.freedesktop.org, andrzej.hajda@gmail.com,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH V2 3/4] drm/panel: Add prepare_upstream_first flag to drm_panel
Date: Tue, 18 Oct 2022 18:10:28 +0100	[thread overview]
Message-ID: <CAPY8ntAG-kWS0fPrXsKVjn0--SatjWgPq2jvSgaUgT_4Ciy3mQ@mail.gmail.com> (raw)
In-Reply-To: <CAMty3ZANdQvghZzdXcK1cKvmFVC5XPVFhUg11BGFHxK9VTdsyg@mail.gmail.com>

Hi Jagan

On Mon, 17 Oct 2022 at 03:44, Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Dave,
>
> On Fri, Oct 7, 2022 at 6:26 PM Dave Stevenson
> <dave.stevenson@raspberrypi.com> wrote:
> >
> > Hi Jagan
> >
> > On Thu, 6 Oct 2022 at 15:25, Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > On Fri, Mar 4, 2022 at 8:48 PM Dave Stevenson
> > > <dave.stevenson@raspberrypi.com> wrote:
> > > >
> > > > Mapping to the drm_bridge flag pre_enable_upstream_first,
> > > > add a new flag prepare_upstream_first to drm_panel to allow
> > > > the panel driver to request that the upstream bridge should
> > > > be pre_enabled before the panel prepare.
> > > >
> > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > > ---
> > > >  drivers/gpu/drm/bridge/panel.c |  3 +++
> > > >  include/drm/drm_panel.h        | 10 ++++++++++
> > > >  2 files changed, 13 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> > > > index 5be057575183..2ea08b3ba326 100644
> > > > --- a/drivers/gpu/drm/bridge/panel.c
> > > > +++ b/drivers/gpu/drm/bridge/panel.c
> > > > @@ -234,6 +234,9 @@ struct drm_bridge *drm_panel_bridge_add_typed(struct drm_panel *panel,
> > > >         panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES;
> > > >         panel_bridge->bridge.type = connector_type;
> > > >
> > > > +       panel_bridge->bridge.pre_enable_upstream_first =
> > > > +                                               panel->prepare_upstream_first;
> > > > +
> > >
> > > panel_bridge is common for bridge users who used panel and those who
> > > might not need upstream first, so better to handle per bridge user
> > > whoever needs this.
> >
> > Sorry, I don't follow you.
>
> panel_bridge driver is a common bridge for drm_panel_bridge_add
> registered bridges. If we enable pre_enable_upstream_first globally in
> panel_bridge driver then it affects panes that don't require
> pre_enable first for that bridge chain. Hope you understand.

No, sorry, I'm still not getting your point.

It is not enabled globally.
If (and only if) the specific panel driver has set
prepare_upstream_first in the struct drm_panel passed to
drm_panel_add(), then that setting is replicated in the associated
struct drm_bridge pre_enable_upstream_first.

Can you give an example of where you see this being an issue?

You proposed handling this on a per bridge user basis? What exactly
are you calling a bridge user in that case? The DSI host (or
equivalent) source to the panel? Because the panel driver has no idea
it is being wrapped into a drm_bridge.
However that source device can't alter the bridge chain call order
(breaking the chain as Exynos and vc4 do does not work with the atomic
API in "entertaining" ways), and it has no knowledge of the behaviour
the attached panel wants, nor does it know that it's going through
panel_bridge.

As per my previous email, devm_drm_of_get_bridge is the only other
place in the callstack that has both the drm_panel and drm_bridge
handles. Does putting the assignment from drm_panel to drm_bridge in
there solve your concern?

Thanks.
  Dave