Re: [Intel-gfx] [RFC v2 1/4] drm: Add support to find drm_panel by name
On Tue, Jan 13, 2015 at 12:08:11AM +0100, Daniel Vetter wrote: > On Fri, Jan 9, 2015 at 1:50 PM, Jani Nikula 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 pgpt2u9WBzrMC.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2 1/4] drm: Add support to find drm_panel by name
On 01/13/2015 12:08 AM, Daniel Vetter wrote: > On Fri, Jan 9, 2015 at 1:50 PM, Jani Nikula 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2 1/4] drm: Add support to find drm_panel by name
On Fri, Jan 9, 2015 at 1:50 PM, Jani Nikula 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2 1/4] drm: Add support to find drm_panel by name
On 1/9/2015 6:20 PM, Jani Nikula wrote:
On Fri, 02 Jan 2015, Shobhit Kumar wrote:
For scenarios where OF is not available, we can use panel identification by
name.
Signed-off-by: Shobhit Kumar
---
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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2 1/4] drm: Add support to find drm_panel by name
On Fri, 02 Jan 2015, Shobhit Kumar wrote:
> For scenarios where OF is not available, we can use panel identification by
> name.
>
> Signed-off-by: Shobhit Kumar
> ---
> 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 ");
> 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
>
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC v2 1/4] drm: Add support to find drm_panel by name
For scenarios where OF is not available, we can use panel identification by
name.
Signed-off-by: Shobhit Kumar
---
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);
+
MODULE_AUTHOR("Thierry Reding ");
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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
