[v2] drm/radeon: add late_register for connector

[PATCH v2] drm/radeon: add late_register for connector

Posted by Wu Hoi Pok 1 year, 4 months ago

The patch is to solve null dereference in 'aux.dev', which is
introduced in recent radeon rework. By having 'late_register',
the connector should be registered after 'drm_dev_register'
automatically, where in before it is the opposite.

Fixes: 90985660ba48 ("drm/radeon: remove load callback from kms_driver")
Tested-by: Hans de Goede <hdegoede@redhat.com>
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Wu Hoi Pok <wuhoipok@gmail.com>
---
 drivers/gpu/drm/radeon/atombios_dp.c       |  9 ++-------
 drivers/gpu/drm/radeon/radeon_connectors.c | 17 +++++++++++++++++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
index fca8b08535a5..6328627b7c34 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -228,10 +228,8 @@ void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
 {
 	struct drm_device *dev = radeon_connector->base.dev;
 	struct radeon_device *rdev = dev->dev_private;
-	int ret;
 
 	radeon_connector->ddc_bus->rec.hpd = radeon_connector->hpd.hpd;
-	radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev;
 	radeon_connector->ddc_bus->aux.drm_dev = radeon_connector->base.dev;
 	if (ASIC_IS_DCE5(rdev)) {
 		if (radeon_auxch)
@@ -242,11 +240,8 @@ void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
 		radeon_connector->ddc_bus->aux.transfer = radeon_dp_aux_transfer_atom;
 	}
 
-	ret = drm_dp_aux_register(&radeon_connector->ddc_bus->aux);
-	if (!ret)
-		radeon_connector->ddc_bus->has_aux = true;
-
-	WARN(ret, "drm_dp_aux_register() failed with error %d\n", ret);
+	drm_dp_aux_init(&radeon_connector->ddc_bus->aux);
+	radeon_connector->ddc_bus->has_aux = true;
 }
 
 /***** general DP utility functions *****/
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 528a8f3677c2..f9c73c55f04f 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1786,6 +1786,20 @@ static enum drm_mode_status radeon_dp_mode_valid(struct drm_connector *connector
 	return MODE_OK;
 }
 
+static int
+radeon_connector_late_register(struct drm_connector *connector)
+{
+	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
+	int r = 0;
+
+	if (radeon_connector->ddc_bus->has_aux) {
+		radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev;
+		r = drm_dp_aux_register(&radeon_connector->ddc_bus->aux);
+	}
+
+	return r;
+}
+
 static const struct drm_connector_helper_funcs radeon_dp_connector_helper_funcs = {
 	.get_modes = radeon_dp_get_modes,
 	.mode_valid = radeon_dp_mode_valid,
@@ -1800,6 +1814,7 @@ static const struct drm_connector_funcs radeon_dp_connector_funcs = {
 	.early_unregister = radeon_connector_unregister,
 	.destroy = radeon_connector_destroy,
 	.force = radeon_dvi_force,
+	.late_register = radeon_connector_late_register,
 };
 
 static const struct drm_connector_funcs radeon_edp_connector_funcs = {
@@ -1810,6 +1825,7 @@ static const struct drm_connector_funcs radeon_edp_connector_funcs = {
 	.early_unregister = radeon_connector_unregister,
 	.destroy = radeon_connector_destroy,
 	.force = radeon_dvi_force,
+	.late_register = radeon_connector_late_register,
 };
 
 static const struct drm_connector_funcs radeon_lvds_bridge_connector_funcs = {
@@ -1820,6 +1836,7 @@ static const struct drm_connector_funcs radeon_lvds_bridge_connector_funcs = {
 	.early_unregister = radeon_connector_unregister,
 	.destroy = radeon_connector_destroy,
 	.force = radeon_dvi_force,
+	.late_register = radeon_connector_late_register,
 };
 
 void
-- 
2.46.2

Re: [PATCH v2] drm/radeon: add late_register for connector

Posted by Alex Deucher 1 year, 4 months ago

On Thu, Oct 3, 2024 at 9:18 PM Wu Hoi Pok <wuhoipok@gmail.com> wrote:
>
> The patch is to solve null dereference in 'aux.dev', which is
> introduced in recent radeon rework. By having 'late_register',
> the connector should be registered after 'drm_dev_register'
> automatically, where in before it is the opposite.
>
> Fixes: 90985660ba48 ("drm/radeon: remove load callback from kms_driver")
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Wu Hoi Pok <wuhoipok@gmail.com>

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/atombios_dp.c       |  9 ++-------
>  drivers/gpu/drm/radeon/radeon_connectors.c | 17 +++++++++++++++++
>  2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
> index fca8b08535a5..6328627b7c34 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -228,10 +228,8 @@ void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
>  {
>         struct drm_device *dev = radeon_connector->base.dev;
>         struct radeon_device *rdev = dev->dev_private;
> -       int ret;
>
>         radeon_connector->ddc_bus->rec.hpd = radeon_connector->hpd.hpd;
> -       radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev;
>         radeon_connector->ddc_bus->aux.drm_dev = radeon_connector->base.dev;
>         if (ASIC_IS_DCE5(rdev)) {
>                 if (radeon_auxch)
> @@ -242,11 +240,8 @@ void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
>                 radeon_connector->ddc_bus->aux.transfer = radeon_dp_aux_transfer_atom;
>         }
>
> -       ret = drm_dp_aux_register(&radeon_connector->ddc_bus->aux);
> -       if (!ret)
> -               radeon_connector->ddc_bus->has_aux = true;
> -
> -       WARN(ret, "drm_dp_aux_register() failed with error %d\n", ret);
> +       drm_dp_aux_init(&radeon_connector->ddc_bus->aux);
> +       radeon_connector->ddc_bus->has_aux = true;
>  }
>
>  /***** general DP utility functions *****/
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 528a8f3677c2..f9c73c55f04f 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -1786,6 +1786,20 @@ static enum drm_mode_status radeon_dp_mode_valid(struct drm_connector *connector
>         return MODE_OK;
>  }
>
> +static int
> +radeon_connector_late_register(struct drm_connector *connector)
> +{
> +       struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> +       int r = 0;
> +
> +       if (radeon_connector->ddc_bus->has_aux) {
> +               radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev;
> +               r = drm_dp_aux_register(&radeon_connector->ddc_bus->aux);
> +       }
> +
> +       return r;
> +}
> +
>  static const struct drm_connector_helper_funcs radeon_dp_connector_helper_funcs = {
>         .get_modes = radeon_dp_get_modes,
>         .mode_valid = radeon_dp_mode_valid,
> @@ -1800,6 +1814,7 @@ static const struct drm_connector_funcs radeon_dp_connector_funcs = {
>         .early_unregister = radeon_connector_unregister,
>         .destroy = radeon_connector_destroy,
>         .force = radeon_dvi_force,
> +       .late_register = radeon_connector_late_register,
>  };
>
>  static const struct drm_connector_funcs radeon_edp_connector_funcs = {
> @@ -1810,6 +1825,7 @@ static const struct drm_connector_funcs radeon_edp_connector_funcs = {
>         .early_unregister = radeon_connector_unregister,
>         .destroy = radeon_connector_destroy,
>         .force = radeon_dvi_force,
> +       .late_register = radeon_connector_late_register,
>  };
>
>  static const struct drm_connector_funcs radeon_lvds_bridge_connector_funcs = {
> @@ -1820,6 +1836,7 @@ static const struct drm_connector_funcs radeon_lvds_bridge_connector_funcs = {
>         .early_unregister = radeon_connector_unregister,
>         .destroy = radeon_connector_destroy,
>         .force = radeon_dvi_force,
> +       .late_register = radeon_connector_late_register,
>  };
>
>  void
> --
> 2.46.2
>

[PATCH v2] drm/radeon: add late_register for connector - Please test

Posted by Christian Zigotzky 1 year, 4 months ago

Hi All,

I compiled a new RC1 of kernel 6.12 with this patch today. Please test it.

Downloads:

- https://github.com/chzigotzky/kernels/releases/tag/v6.12.0-rc1-2
- https://www.xenosoft.de/linux-image-6.12-rc1-2-X1000_X5000.tar.gz

Thanks,
Christian

On 04 October 2024 at 4:00pm, Alex Deucher wrote:
> On Thu, Oct 3, 2024 at 9:18 PM Wu Hoi Pok <wuhoipok@gmail.com> wrote:
>> The patch is to solve null dereference in 'aux.dev', which is
>> introduced in recent radeon rework. By having 'late_register',
>> the connector should be registered after 'drm_dev_register'
>> automatically, where in before it is the opposite.
>>
>> Fixes: 90985660ba48 ("drm/radeon: remove load callback from kms_driver")
>> Tested-by: Hans de Goede <hdegoede@redhat.com>
>> Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Signed-off-by: Wu Hoi Pok <wuhoipok@gmail.com>
> Applied.  Thanks!
>
> Alex
>
>> ---
>>   drivers/gpu/drm/radeon/atombios_dp.c       |  9 ++-------
>>   drivers/gpu/drm/radeon/radeon_connectors.c | 17 +++++++++++++++++
>>   2 files changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
>> index fca8b08535a5..6328627b7c34 100644
>> --- a/drivers/gpu/drm/radeon/atombios_dp.c
>> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
>> @@ -228,10 +228,8 @@ void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
>>   {
>>          struct drm_device *dev = radeon_connector->base.dev;
>>          struct radeon_device *rdev = dev->dev_private;
>> -       int ret;
>>
>>          radeon_connector->ddc_bus->rec.hpd = radeon_connector->hpd.hpd;
>> -       radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev;
>>          radeon_connector->ddc_bus->aux.drm_dev = radeon_connector->base.dev;
>>          if (ASIC_IS_DCE5(rdev)) {
>>                  if (radeon_auxch)
>> @@ -242,11 +240,8 @@ void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
>>                  radeon_connector->ddc_bus->aux.transfer = radeon_dp_aux_transfer_atom;
>>          }
>>
>> -       ret = drm_dp_aux_register(&radeon_connector->ddc_bus->aux);
>> -       if (!ret)
>> -               radeon_connector->ddc_bus->has_aux = true;
>> -
>> -       WARN(ret, "drm_dp_aux_register() failed with error %d\n", ret);
>> +       drm_dp_aux_init(&radeon_connector->ddc_bus->aux);
>> +       radeon_connector->ddc_bus->has_aux = true;
>>   }
>>
>>   /***** general DP utility functions *****/
>> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
>> index 528a8f3677c2..f9c73c55f04f 100644
>> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
>> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
>> @@ -1786,6 +1786,20 @@ static enum drm_mode_status radeon_dp_mode_valid(struct drm_connector *connector
>>          return MODE_OK;
>>   }
>>
>> +static int
>> +radeon_connector_late_register(struct drm_connector *connector)
>> +{
>> +       struct radeon_connector *radeon_connector = to_radeon_connector(connector);
>> +       int r = 0;
>> +
>> +       if (radeon_connector->ddc_bus->has_aux) {
>> +               radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev;
>> +               r = drm_dp_aux_register(&radeon_connector->ddc_bus->aux);
>> +       }
>> +
>> +       return r;
>> +}
>> +
>>   static const struct drm_connector_helper_funcs radeon_dp_connector_helper_funcs = {
>>          .get_modes = radeon_dp_get_modes,
>>          .mode_valid = radeon_dp_mode_valid,
>> @@ -1800,6 +1814,7 @@ static const struct drm_connector_funcs radeon_dp_connector_funcs = {
>>          .early_unregister = radeon_connector_unregister,
>>          .destroy = radeon_connector_destroy,
>>          .force = radeon_dvi_force,
>> +       .late_register = radeon_connector_late_register,
>>   };
>>
>>   static const struct drm_connector_funcs radeon_edp_connector_funcs = {
>> @@ -1810,6 +1825,7 @@ static const struct drm_connector_funcs radeon_edp_connector_funcs = {
>>          .early_unregister = radeon_connector_unregister,
>>          .destroy = radeon_connector_destroy,
>>          .force = radeon_dvi_force,
>> +       .late_register = radeon_connector_late_register,
>>   };
>>
>>   static const struct drm_connector_funcs radeon_lvds_bridge_connector_funcs = {
>> @@ -1820,6 +1836,7 @@ static const struct drm_connector_funcs radeon_lvds_bridge_connector_funcs = {
>>          .early_unregister = radeon_connector_unregister,
>>          .destroy = radeon_connector_destroy,
>>          .force = radeon_dvi_force,
>> +       .late_register = radeon_connector_late_register,
>>   };
>>
>>   void
>> --
>> 2.46.2
>>