diff mbox series

[v3,21/22] drm/amdgpu: Provide ddc symlink in connector sysfs directory

Message ID 5e355b8bec8fb3907566a741db8cc3e356246a32.1561735433.git.andrzej.p@collabora.com (mailing list archive)
State New, archived
Headers show
Series [v3,01/22] drm: Include ddc adapter pointer in struct drm_connector | expand

Commit Message

Andrzej Pietrasiewicz June 28, 2019, 4:01 p.m. UTC 
Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 ++++++++++++++-----
 1 file changed, 51 insertions(+), 19 deletions(-)

Comments

Alex Deucher July 2, 2019, 8:54 p.m. UTC | #1 
On Fri, Jun 28, 2019 at 12:31 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Use the ddc pointer provided by the generic connector.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 ++++++++++++++-----
>  1 file changed, 51 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index 73b2ede773d3..5f8a7e3818b9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -1573,11 +1573,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         goto failed;
>                 amdgpu_connector->con_priv = amdgpu_dig_connector;
>                 if (i2c_bus->valid) {
> -                       amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                       if (amdgpu_connector->ddc_bus)
> +                       struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                       acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                       if (acn->ddc_bus) {

This hunk seems pointless unless I'm missing something.  Can you drop
this hunk?  Same comment on each instance of this below.  This also
only covers the legacy modesetting code which is not used by default
on most chips.  The DC code in amd/display/ is probably more relevant.

Alex

>                                 has_aux = true;
> -                       else
> +                               connector->ddc = &acn->ddc_bus->adapter;
> +                       } else {
>                                 DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                       }
>                 }
>                 switch (connector_type) {
>                 case DRM_MODE_CONNECTOR_VGA:
> @@ -1662,9 +1666,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_vga_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_vga_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("VGA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         amdgpu_connector->dac_load_detect = true;
>                         drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1682,9 +1690,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_vga_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_vga_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("DVIA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         amdgpu_connector->dac_load_detect = true;
>                         drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1707,9 +1719,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dvi_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dvi_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("DVI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         subpixel_order = SubPixelHorizontalRGB;
>                         drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1757,9 +1773,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dvi_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dvi_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("HDMI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         drm_object_attach_property(&amdgpu_connector->base.base,
>                                                       adev->mode_info.coherent_mode_property,
> @@ -1799,11 +1819,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dp_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dp_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (acn->ddc_bus) {
>                                         has_aux = true;
> -                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
> +                               } else {
>                                         DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               }
>                         }
>                         subpixel_order = SubPixelHorizontalRGB;
>                         drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1841,11 +1865,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_edp_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dp_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (acn->ddc_bus) {
>                                         has_aux = true;
> -                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
> +                               } else {
>                                         DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               }
>                         }
>                         drm_object_attach_property(&amdgpu_connector->base.base,
>                                                       dev->mode_config.scaling_mode_property,
> @@ -1862,9 +1890,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_lvds_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_lvds_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("LVDS: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         drm_object_attach_property(&amdgpu_connector->base.base,
>                                                       dev->mode_config.scaling_mode_property,
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Andrzej Pietrasiewicz July 4, 2019, 1:17 p.m. UTC | #2 
W dniu 02.07.2019 o 22:54, Alex Deucher pisze:
> On Fri, Jun 28, 2019 at 12:31 PM Andrzej Pietrasiewicz
> <andrzej.p@collabora.com> wrote:
>>
>> Use the ddc pointer provided by the generic connector.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>> ---
>>   .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 ++++++++++++++-----
>>   1 file changed, 51 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> index 73b2ede773d3..5f8a7e3818b9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> @@ -1573,11 +1573,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>>                          goto failed;
>>                  amdgpu_connector->con_priv = amdgpu_dig_connector;
>>                  if (i2c_bus->valid) {
>> -                       amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
>> -                       if (amdgpu_connector->ddc_bus)
>> +                       struct amdgpu_connector *acn = amdgpu_connector;
>> +
>> +                       acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
>> +                       if (acn->ddc_bus) {
> 
> This hunk seems pointless unless I'm missing something.  Can you drop
> this hunk?  Same comment on each instance of this below.  This also
> only covers the legacy modesetting code which is not used by default
> on most chips.  The DC code in amd/display/ is probably more relevant.
> 

If I don't do that checkpatch reports that lines I created exceed 80 characters.

Andrzej
Alex Deucher July 5, 2019, 8:17 p.m. UTC | #3 
On Thu, Jul 4, 2019 at 9:17 AM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> W dniu 02.07.2019 o 22:54, Alex Deucher pisze:
> > On Fri, Jun 28, 2019 at 12:31 PM Andrzej Pietrasiewicz
> > <andrzej.p@collabora.com> wrote:
> >>
> >> Use the ddc pointer provided by the generic connector.
> >>
> >> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> >> ---
> >>   .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 ++++++++++++++-----
> >>   1 file changed, 51 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> >> index 73b2ede773d3..5f8a7e3818b9 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> >> @@ -1573,11 +1573,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
> >>                          goto failed;
> >>                  amdgpu_connector->con_priv = amdgpu_dig_connector;
> >>                  if (i2c_bus->valid) {
> >> -                       amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> >> -                       if (amdgpu_connector->ddc_bus)
> >> +                       struct amdgpu_connector *acn = amdgpu_connector;
> >> +
> >> +                       acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> >> +                       if (acn->ddc_bus) {
> >
> > This hunk seems pointless unless I'm missing something.  Can you drop
> > this hunk?  Same comment on each instance of this below.  This also
> > only covers the legacy modesetting code which is not used by default
> > on most chips.  The DC code in amd/display/ is probably more relevant.
> >
>
> If I don't do that checkpatch reports that lines I created exceed 80 characters.

Don't worry about that.

Alex
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 73b2ede773d3..5f8a7e3818b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -1573,11 +1573,15 @@  amdgpu_connector_add(struct amdgpu_device *adev,
 			goto failed;
 		amdgpu_connector->con_priv = amdgpu_dig_connector;
 		if (i2c_bus->valid) {
-			amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-			if (amdgpu_connector->ddc_bus)
+			struct amdgpu_connector *acn = amdgpu_connector;
+
+			acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+			if (acn->ddc_bus) {
 				has_aux = true;
-			else
+				connector->ddc = &acn->ddc_bus->adapter;
+			} else {
 				DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+			}
 		}
 		switch (connector_type) {
 		case DRM_MODE_CONNECTOR_VGA:
@@ -1662,9 +1666,13 @@  amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_vga_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_vga_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("VGA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			amdgpu_connector->dac_load_detect = true;
 			drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1682,9 +1690,13 @@  amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_vga_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_vga_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("DVIA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			amdgpu_connector->dac_load_detect = true;
 			drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1707,9 +1719,13 @@  amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dvi_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dvi_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("DVI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			subpixel_order = SubPixelHorizontalRGB;
 			drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1757,9 +1773,13 @@  amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dvi_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dvi_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("HDMI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			drm_object_attach_property(&amdgpu_connector->base.base,
 						      adev->mode_info.coherent_mode_property,
@@ -1799,11 +1819,15 @@  amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dp_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dp_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (acn->ddc_bus) {
 					has_aux = true;
-				else
+					connector->ddc = &acn->ddc_bus->adapter;
+				} else {
 					DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				}
 			}
 			subpixel_order = SubPixelHorizontalRGB;
 			drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1841,11 +1865,15 @@  amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_edp_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dp_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (acn->ddc_bus) {
 					has_aux = true;
-				else
+					connector->ddc = &acn->ddc_bus->adapter;
+				} else {
 					DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				}
 			}
 			drm_object_attach_property(&amdgpu_connector->base.base,
 						      dev->mode_config.scaling_mode_property,
@@ -1862,9 +1890,13 @@  amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_lvds_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_lvds_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("LVDS: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			drm_object_attach_property(&amdgpu_connector->base.base,
 						      dev->mode_config.scaling_mode_property,