| Message ID | 1420206085-2913-2-git-send-email-shobhit.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote: > For scenarios where OF is not available, we can use panel identification by > name. > > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > --- > drivers/gpu/drm/drm_panel.c | 18 ++++++++++++++++++ > include/drm/drm_panel.h | 3 +++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index 2ef988e..773ebd6 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c > @@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np) > EXPORT_SYMBOL(of_drm_find_panel); > #endif > > +struct drm_panel *name_drm_find_panel(const char *name) > +{ > + struct drm_panel *panel; > + > + mutex_lock(&panel_lock); > + > + list_for_each_entry(panel, &panel_list, list) { > + if (strcmp(panel->name, name) == 0) { > + mutex_unlock(&panel_lock); > + return panel; > + } > + } > + > + mutex_unlock(&panel_lock); > + return NULL; > +} > +EXPORT_SYMBOL(name_drm_find_panel); This patch needs to be sent to drm-devel. The name should probably be something like drm_find_panel_by_name. I have a slightly uneasy feeling about handing out drm_panel pointers (both from here and of_drm_find_panel) without refcounting. If the panel driver gets removed, whoever called the find functions will have a dangling pointer. I supposed this will be discussed on drm-devel. BR, Jani. > + > MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>"); > MODULE_DESCRIPTION("DRM panel infrastructure"); > MODULE_LICENSE("GPL and additional rights"); > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h > index 1fbcc96..b120b5d 100644 > --- a/include/drm/drm_panel.h > +++ b/include/drm/drm_panel.h > @@ -74,6 +74,7 @@ struct drm_panel { > struct drm_device *drm; > struct drm_connector *connector; > struct device *dev; > + char name[NAME_MAX]; > > const struct drm_panel_funcs *funcs; > > @@ -137,4 +138,6 @@ static inline struct drm_panel *of_drm_find_panel(struct device_node *np) > } > #endif > > +struct drm_panel *name_drm_find_panel(const char *name); > + > #endif > -- > 1.9.1 >
On 1/9/2015 6:20 PM, Jani Nikula wrote: > On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote: >> For scenarios where OF is not available, we can use panel identification by >> name. >> >> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> >> --- >> drivers/gpu/drm/drm_panel.c | 18 ++++++++++++++++++ >> include/drm/drm_panel.h | 3 +++ >> 2 files changed, 21 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c >> index 2ef988e..773ebd6 100644 >> --- a/drivers/gpu/drm/drm_panel.c >> +++ b/drivers/gpu/drm/drm_panel.c >> @@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np) >> EXPORT_SYMBOL(of_drm_find_panel); >> #endif >> >> +struct drm_panel *name_drm_find_panel(const char *name) >> +{ >> + struct drm_panel *panel; >> + >> + mutex_lock(&panel_lock); >> + >> + list_for_each_entry(panel, &panel_list, list) { >> + if (strcmp(panel->name, name) == 0) { >> + mutex_unlock(&panel_lock); >> + return panel; >> + } >> + } >> + >> + mutex_unlock(&panel_lock); >> + return NULL; >> +} >> +EXPORT_SYMBOL(name_drm_find_panel); > > This patch needs to be sent to drm-devel. > > The name should probably be something like drm_find_panel_by_name. > > I have a slightly uneasy feeling about handing out drm_panel pointers > (both from here and of_drm_find_panel) without refcounting. If the panel > driver gets removed, whoever called the find functions will have a > dangling pointer. I supposed this will be discussed on drm-devel. Right its a valid point. I Will post the updated patch there and see what all comes up. Regards Shobhit
On Fri, Jan 9, 2015 at 1:50 PM, Jani Nikula <jani.nikula@intel.com> wrote: > I have a slightly uneasy feeling about handing out drm_panel pointers > (both from here and of_drm_find_panel) without refcounting. If the panel > driver gets removed, whoever called the find functions will have a > dangling pointer. I supposed this will be discussed on drm-devel. There's been some discussion already about exactly this problem (but with drm bridges) with Thierry and some other people. Cc'ed them all hopefully. Especially when the panel/bridge is a separate driver there's imo indeed an issue. -Daniel
On 01/13/2015 12:08 AM, Daniel Vetter wrote: > On Fri, Jan 9, 2015 at 1:50 PM, Jani Nikula <jani.nikula@intel.com> wrote: >> I have a slightly uneasy feeling about handing out drm_panel pointers >> (both from here and of_drm_find_panel) without refcounting. If the panel >> driver gets removed, whoever called the find functions will have a >> dangling pointer. I supposed this will be discussed on drm-devel. refcounting does not seems to me a good solution, drm_panel is exposed by device driver and device driver can be unbound unconditionally at any time. This problem affects many frameworks not only drm_panel. I work on resource tracking framework which tries to solve the problem in a generic way[1]. [1]: https://lkml.org/lkml/2014/12/10/342 Regards Andrzej > There's been some discussion already about exactly this problem (but > with drm bridges) with Thierry and some other people. Cc'ed them all > hopefully. Especially when the panel/bridge is a separate driver > there's imo indeed an issue. > -Daniel
On Tue, Jan 13, 2015 at 12:08:11AM +0100, Daniel Vetter wrote: > On Fri, Jan 9, 2015 at 1:50 PM, Jani Nikula <jani.nikula@intel.com> wrote: > > I have a slightly uneasy feeling about handing out drm_panel pointers > > (both from here and of_drm_find_panel) without refcounting. If the panel > > driver gets removed, whoever called the find functions will have a > > dangling pointer. I supposed this will be discussed on drm-devel. > > There's been some discussion already about exactly this problem (but > with drm bridges) with Thierry and some other people. Cc'ed them all > hopefully. Especially when the panel/bridge is a separate driver > there's imo indeed an issue. I posted patches some time ago to create a generic registry to do the actual ref-counting[0]. It didn't seem to be very well received by Greg for the core, so perhaps we could test-drive it in DRM first for panels and bridges and once it's matured a bit it could still be promoted to the driver core, or maybe lib/. The difficult part about it is that while reference counting gives you the benefit of keeping a valid pointer around, you may still want to have a method of getting notified of the panel going away. I've thought a bit about that and I think we could probably integrate that into the registry, since that will notice anyway. Thierry [0]: http://www.spinics.net/linux/lists/kernel/msg1859333.html
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index 2ef988e..773ebd6 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np) EXPORT_SYMBOL(of_drm_find_panel); #endif +struct drm_panel *name_drm_find_panel(const char *name) +{ + struct drm_panel *panel; + + mutex_lock(&panel_lock); + + list_for_each_entry(panel, &panel_list, list) { + if (strcmp(panel->name, name) == 0) { + mutex_unlock(&panel_lock); + return panel; + } + } + + mutex_unlock(&panel_lock); + return NULL; +} +EXPORT_SYMBOL(name_drm_find_panel); + MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>"); MODULE_DESCRIPTION("DRM panel infrastructure"); MODULE_LICENSE("GPL and additional rights"); diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 1fbcc96..b120b5d 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -74,6 +74,7 @@ struct drm_panel { struct drm_device *drm; struct drm_connector *connector; struct device *dev; + char name[NAME_MAX]; const struct drm_panel_funcs *funcs; @@ -137,4 +138,6 @@ static inline struct drm_panel *of_drm_find_panel(struct device_node *np) } #endif +struct drm_panel *name_drm_find_panel(const char *name); + #endif
For scenarios where OF is not available, we can use panel identification by name. Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> --- drivers/gpu/drm/drm_panel.c | 18 ++++++++++++++++++ include/drm/drm_panel.h | 3 +++ 2 files changed, 21 insertions(+)