| Message ID | a593a187fe3e6fc1ca5bf3db001ed87457e3d4ee.1646406653.git.dave.stevenson@raspberrypi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DSI host and peripheral initialisation ordering | expand |
On 04/03/2022 18:17, Dave Stevenson 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> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > 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; > + > drm_bridge_add(&panel_bridge->bridge); > > return &panel_bridge->bridge; > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h > index 1ba2d424a53f..c0f39dfbd071 100644 > --- a/include/drm/drm_panel.h > +++ b/include/drm/drm_panel.h > @@ -179,6 +179,16 @@ struct drm_panel { > * Panel entry in registry. > */ > struct list_head list; > + > + /** > + * @prepare_upstream_first: > + * > + * The upstream controller should be prepared first, before the prepare > + * for the panel is called. This is largely required for DSI panels > + * where the DSI host controller should be initialised to LP-11 before > + * the panel is powered up. > + */ > + bool prepare_upstream_first; > }; > > void drm_panel_init(struct drm_panel *panel, struct device *dev,
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. Thanks, Jagan.
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. prepare_upstream_first is coming from a struct drm_panel, generally for DSI panels are still panel drivers. An example would be Ilitek ILI9881C. It's a feature of the panel that would dictate that they want their source initialised first. The source bridge for the panel will call devm_drm_of_get_bridge, which will call devm_drm_panel_bridge_add. Nothing outside of those two functions have both the panel and bridge handles, so are you proposing that the assignment should be done in devm_drm_of_get_bridge [1]? That would leave the behaviour of drivers calling drm_panel_bridge_add(_typed) directly as they were. Thanks. Dave [1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/panel.c#L418
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. Thanks, Jagan.
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
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; + drm_bridge_add(&panel_bridge->bridge); return &panel_bridge->bridge; diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 1ba2d424a53f..c0f39dfbd071 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -179,6 +179,16 @@ struct drm_panel { * Panel entry in registry. */ struct list_head list; + + /** + * @prepare_upstream_first: + * + * The upstream controller should be prepared first, before the prepare + * for the panel is called. This is largely required for DSI panels + * where the DSI host controller should be initialised to LP-11 before + * the panel is powered up. + */ + bool prepare_upstream_first; }; void drm_panel_init(struct drm_panel *panel, struct device *dev,
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(+)