| Message ID | 20180430144323.9233-2-boris.brezillon@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 30, 2018 at 04:43:21PM +0200, Boris Brezillon wrote: > Some panels might be connected through extension boards and might not > be available when the system boots. Extend the panel interface to > support panel detection. > > An optional ->detect() hook is added and, if implemented, will be called > every time the panel user wants to know if the panel is connected or > disconnected. > > We also add a ->polled field which should encode the type of polling the > DRM core should do (DRM_CONNECTOR_POLL_HPD, DRM_CONNECTOR_POLL_CONNECT > and DRM_CONNECTOR_POLL_DISCONNECT flags). > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> Hm, not sure panel detection makes much sense, the entire idea behind drm_panel is to hard-code a fixed/built-in panel. The only thing panels may report through the connection status is whether the lid is closed or not. Yes we should probably document that somewhere. If you have a panel-that-can-be-hotplugged a drm_bridge that implemens the drm_connector is probably the way to go. > --- > include/drm/drm_panel.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h > index 14ac240a1f64..718cc1f746ab 100644 > --- a/include/drm/drm_panel.h > +++ b/include/drm/drm_panel.h > @@ -24,6 +24,7 @@ > #ifndef __DRM_PANEL_H__ > #define __DRM_PANEL_H__ > > +#include <drm/drm_connector.h> > #include <linux/errno.h> > #include <linux/list.h> > > @@ -68,6 +69,7 @@ struct display_timing; > * the panel. This is the job of the .unprepare() function. > */ > struct drm_panel_funcs { > + int (*detect)(struct drm_panel *panel); Kerneldoc for this please. Would also be really good to switch this struct of function pointers over to the in-line style, and put the paragraphs relevant for a given callback into it's dedicated comment. -Daniel > int (*disable)(struct drm_panel *panel); > int (*unprepare)(struct drm_panel *panel); > int (*prepare)(struct drm_panel *panel); > @@ -90,6 +92,7 @@ struct drm_panel { > struct drm_connector *connector; > struct device *dev; > > + u8 polled; > const struct drm_panel_funcs *funcs; > > struct list_head list; > @@ -186,6 +189,15 @@ static inline int drm_panel_get_modes(struct drm_panel *panel) > return panel ? -ENOSYS : -EINVAL; > } > > +static inline int drm_panel_detect(struct drm_panel *panel) > +{ > + if (panel && panel->funcs && panel->funcs->detect) > + return panel->funcs->detect(panel); > + > + /* Consider the panel as connected by default. */ > + return panel ? connector_status_connected : -EINVAL; > +} > + > void drm_panel_init(struct drm_panel *panel); > > int drm_panel_add(struct drm_panel *panel); > -- > 2.14.1 >
Hi Daniel, On Mon, 30 Apr 2018 18:08:47 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Apr 30, 2018 at 04:43:21PM +0200, Boris Brezillon wrote: > > Some panels might be connected through extension boards and might not > > be available when the system boots. Extend the panel interface to > > support panel detection. > > > > An optional ->detect() hook is added and, if implemented, will be called > > every time the panel user wants to know if the panel is connected or > > disconnected. > > > > We also add a ->polled field which should encode the type of polling the > > DRM core should do (DRM_CONNECTOR_POLL_HPD, DRM_CONNECTOR_POLL_CONNECT > > and DRM_CONNECTOR_POLL_DISCONNECT flags). > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > Hm, not sure panel detection makes much sense, the entire idea behind > drm_panel is to hard-code a fixed/built-in panel. The only thing panels > may report through the connection status is whether the lid is closed or > not. Yes we should probably document that somewhere. > > If you have a panel-that-can-be-hotplugged a drm_bridge that implemens the > drm_connector is probably the way to go. That does not really work for the rpi panel case because we use an I2C message to detect the presence of the panel. So, unless we decide to attach the I2C client to the bridge device instead of attaching it to the panel that's not really possible, and I'm not sure doing that is a good thing either. I guess the only alternative to solve the issue I'm trying to solve is drm_bridge hotplug, so that we allow the DRM device to be registered even if not all of its leaf nodes (i.e. bridges and connectors) are ready to be used. > > > --- > > include/drm/drm_panel.h | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h > > index 14ac240a1f64..718cc1f746ab 100644 > > --- a/include/drm/drm_panel.h > > +++ b/include/drm/drm_panel.h > > @@ -24,6 +24,7 @@ > > #ifndef __DRM_PANEL_H__ > > #define __DRM_PANEL_H__ > > > > +#include <drm/drm_connector.h> > > #include <linux/errno.h> > > #include <linux/list.h> > > > > @@ -68,6 +69,7 @@ struct display_timing; > > * the panel. This is the job of the .unprepare() function. > > */ > > struct drm_panel_funcs { > > + int (*detect)(struct drm_panel *panel); > > Kerneldoc for this please. Would also be really good to switch this struct > of function pointers over to the in-line style, and put the paragraphs > relevant for a given callback into it's dedicated comment. Not useful anymore since the idea appeared to be bad ;-). Thanks for the quick feedback. Boris
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 14ac240a1f64..718cc1f746ab 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -24,6 +24,7 @@ #ifndef __DRM_PANEL_H__ #define __DRM_PANEL_H__ +#include <drm/drm_connector.h> #include <linux/errno.h> #include <linux/list.h> @@ -68,6 +69,7 @@ struct display_timing; * the panel. This is the job of the .unprepare() function. */ struct drm_panel_funcs { + int (*detect)(struct drm_panel *panel); int (*disable)(struct drm_panel *panel); int (*unprepare)(struct drm_panel *panel); int (*prepare)(struct drm_panel *panel); @@ -90,6 +92,7 @@ struct drm_panel { struct drm_connector *connector; struct device *dev; + u8 polled; const struct drm_panel_funcs *funcs; struct list_head list; @@ -186,6 +189,15 @@ static inline int drm_panel_get_modes(struct drm_panel *panel) return panel ? -ENOSYS : -EINVAL; } +static inline int drm_panel_detect(struct drm_panel *panel) +{ + if (panel && panel->funcs && panel->funcs->detect) + return panel->funcs->detect(panel); + + /* Consider the panel as connected by default. */ + return panel ? connector_status_connected : -EINVAL; +} + void drm_panel_init(struct drm_panel *panel); int drm_panel_add(struct drm_panel *panel);
Some panels might be connected through extension boards and might not be available when the system boots. Extend the panel interface to support panel detection. An optional ->detect() hook is added and, if implemented, will be called every time the panel user wants to know if the panel is connected or disconnected. We also add a ->polled field which should encode the type of polling the DRM core should do (DRM_CONNECTOR_POLL_HPD, DRM_CONNECTOR_POLL_CONNECT and DRM_CONNECTOR_POLL_DISCONNECT flags). Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- include/drm/drm_panel.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)