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

2019-05-30 Thread Kazlauskas, Nicholas
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/pci:00/: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 
Reviewed-by: Nicholas Kazlauskas 

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

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

2019-05-29 Thread Deng, Emily
Hi Kazlauskas,
I have modified the patch as your suggestion, could you please help to 
review it again?

Best wishes
Emily Deng



>-Original Message-
>From: amd-gfx  On Behalf Of Emily
>Deng
>Sent: Wednesday, May 29, 2019 11:12 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily 
>Subject: [PATCH v2] drm/amdgpu/display: Fix reload driver error
>
>Issue:
>Will have follow error when reload driver:
>[ 3986.567739] sysfs: cannot create duplicate filename
>'/devices/pci:00/: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 
>---
> 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)
>--
>2.7.4
>
>___
>amd-gfx mailing list
>amd-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx