Re: [RFC PATCH 1/3] drm/panel: Support panel detection

[Boris Brezillon <boris.brezillon@bootlin.com>]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel