Sorry for the slow response, I've been really busy ;_;
On Fri, 2019-04-12 at 12:05 -0400, sunpeng.li@xxxxxxx wrote:
> From: Leo Li <sunpeng.li@xxxxxxx>
>
> In preparation for adding aux devices for DP MST:
>
> 1. A non-cyclic idr is used for the device minor version. That way,
> hotplug cycling MST devices won't needlessly increment the minor
> version index.
I really like this btw, cyclic idrs are really annoying for drm_dp_aux_dev,
even outside of MST (try reloading a drm driver a couple of times and you'll
understand ;). I think we should split this into another commit though
> 2. A suffix option is added to the aux device file name. It can be used
> to identify the corresponding MST device.
I like this idea, but I think there may be a better way that we can do this.
Using /dev/nvme* as an example, we have the standard dev paths (/dev/nvme0,
/dev/nvme0n1, etc.). But we can also access them through /dev/disk/by-
{id,label,partlabel,partuuid,path,uuid}.
So what if we added something similar for aux devices? We start off with the
standard /dev/drm_dp_aux*, then provide more descriptive symlinks and
directories:
(feel free to come up with better naming then this if you can)
/dev/drm_dp_aux/card0-DP-1 -> /dev/drm_dp_aux1
/dev/drm_dp_aux/card0-DP-2 -> /dev/drm_dp_aux2
etc.
>
> Signed-off-by: Leo Li <sunpeng.li@xxxxxxx>
> ---
> drivers/gpu/drm/drm_crtc_helper_internal.h | 5 +++--
> drivers/gpu/drm/drm_dp_aux_dev.c | 8 ++++----
> drivers/gpu/drm/drm_dp_helper.c | 2 +-
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h
> b/drivers/gpu/drm/drm_crtc_helper_internal.h
> index b5ac158..9f0907b 100644
> --- a/drivers/gpu/drm/drm_crtc_helper_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
> @@ -46,7 +46,7 @@ static inline int drm_fb_helper_modinit(void)
> #ifdef CONFIG_DRM_DP_AUX_CHARDEV
> int drm_dp_aux_dev_init(void);
> void drm_dp_aux_dev_exit(void);
> -int drm_dp_aux_register_devnode(struct drm_dp_aux *aux);
> +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux, const char
> *suffix);
> void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux);
> #else
> static inline int drm_dp_aux_dev_init(void)
> @@ -58,7 +58,8 @@ static inline void drm_dp_aux_dev_exit(void)
> {
> }
>
> -static inline int drm_dp_aux_register_devnode(struct drm_dp_aux *aux)
> +static inline int drm_dp_aux_register_devnode(struct drm_dp_aux *aux,
> + const char *suffix)
> {
> return 0;
> }
> diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> b/drivers/gpu/drm/drm_dp_aux_dev.c
> index 0e4f25d..2310a67 100644
> --- a/drivers/gpu/drm/drm_dp_aux_dev.c
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -80,8 +80,7 @@ static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct
> drm_dp_aux *aux)
> kref_init(&aux_dev->refcount);
>
> mutex_lock(&aux_idr_mutex);
> - index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS,
> - GFP_KERNEL);
> + index = idr_alloc(&aux_idr, aux_dev, 0, DRM_AUX_MINORS, GFP_KERNEL);
> mutex_unlock(&aux_idr_mutex);
> if (index < 0) {
> kfree(aux_dev);
> @@ -290,7 +289,7 @@ void drm_dp_aux_unregister_devnode(struct drm_dp_aux
> *aux)
> kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
> }
>
> -int drm_dp_aux_register_devnode(struct drm_dp_aux *aux)
> +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux, const char *suffix)
> {
> struct drm_dp_aux_dev *aux_dev;
> int res;
> @@ -301,7 +300,8 @@ int drm_dp_aux_register_devnode(struct drm_dp_aux *aux)
>
> aux_dev->dev = device_create(drm_dp_aux_dev_class, aux->dev,
> MKDEV(drm_dev_major, aux_dev->index),
> NULL,
> - "drm_dp_aux%d", aux_dev->index);
> + "drm_dp_aux%d%s", aux_dev->index,
> + suffix ? suffix : "");
> if (IS_ERR(aux_dev->dev)) {
> res = PTR_ERR(aux_dev->dev);
> aux_dev->dev = NULL;
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index e2266ac..13f1005 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1143,7 +1143,7 @@ int drm_dp_aux_register(struct drm_dp_aux *aux)
> strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
> sizeof(aux->ddc.name));
>
> - ret = drm_dp_aux_register_devnode(aux);
> + ret = drm_dp_aux_register_devnode(aux, NULL);
> if (ret)
> return ret;
>
--
Cheers,
Lyude Paul
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
