Re: [PATCH v2] drm/amdgpu/display: Fix reload driver error

["Kazlauskas, Nicholas" <Nicholas.Kazlauskas-5C7GfCeVMHo@public.gmane.org>]

On 5/28/19 11:12 PM, Emily Deng wrote:
> Issue:
> Will have follow error when reload driver:
> [ 3986.567739] sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:07.0/drm_dp_aux_dev'
> [ 3986.567743] CPU: 6 PID: 1767 Comm: modprobe Tainted: G           OE     5.0.0-rc1-custom #1
> [ 3986.567745] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
> [ 3986.567746] Call Trace:
> ......
> [ 3986.567808]  drm_dp_aux_register_devnode+0xdc/0x140 [drm_kms_helper]
> ......
> [ 3986.569081] kobject_add_internal failed for drm_dp_aux_dev with -EEXIST, don't try to register things with the same name in the same directory.
> 
> Reproduce sequences:
> 1.modprobe amdgpu
> 2.modprobe -r amdgpu
> 3.modprobe amdgpu
> 
> Root cause:
> When unload driver, it doesn't unregister aux.
> 
> v2: Don't use has_aux
> 
> Signed-off-by: Emily Deng <Emily.Deng@amd.com>
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Only a minor nitpick about not mentioning that you're also removing the 
i2c that we added on the connector, which looks correct to me, but isn't 
related to the aux that was registered.

Looks fine to me other than that.

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 8fe1685..941313b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3760,6 +3760,13 @@ int amdgpu_dm_connector_atomic_get_property(struct drm_connector *connector,
>   	return ret;
>   }
>   
> +static void amdgpu_dm_connector_unregister(struct drm_connector *connector)
> +{
> +	struct amdgpu_dm_connector *amdgpu_dm_connector = to_amdgpu_dm_connector(connector);
> +
> +	drm_dp_aux_unregister(&amdgpu_dm_connector->dm_dp_aux.aux);
> +}
> +
>   static void amdgpu_dm_connector_destroy(struct drm_connector *connector)
>   {
>   	struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);
> @@ -3788,6 +3795,11 @@ static void amdgpu_dm_connector_destroy(struct drm_connector *connector)
>   	drm_dp_cec_unregister_connector(&aconnector->dm_dp_aux.aux);
>   	drm_connector_unregister(connector);
>   	drm_connector_cleanup(connector);
> +	if (aconnector->i2c) {
> +		i2c_del_adapter(&aconnector->i2c->base);
> +		kfree(aconnector->i2c);
> +	}
> +
>   	kfree(connector);
>   }
>   
> @@ -3846,7 +3858,8 @@ static const struct drm_connector_funcs amdgpu_dm_connector_funcs = {
>   	.atomic_duplicate_state = amdgpu_dm_connector_atomic_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   	.atomic_set_property = amdgpu_dm_connector_atomic_set_property,
> -	.atomic_get_property = amdgpu_dm_connector_atomic_get_property
> +	.atomic_get_property = amdgpu_dm_connector_atomic_get_property,
> +	.early_unregister = amdgpu_dm_connector_unregister
>   };
>   
>   static int get_modes(struct drm_connector *connector)
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx