Re: [PATCH 01/12] drm/amdgpu: Write masked value to control register

["André Almeida" <andrealmeid@igalia.com>]

Hi Maíra,

Thank you for your patch,

Às 13:44 de 14/07/22, Maíra Canal escreveu:
> On the dce_v6_0 and dce_v8_0 hpd tear down callback, the tmp variable
> should be written into the control register instead of 0.
> 

Why? I do see that tmp was unused before your patch, but why should we
write it into this register? Did you manage to test this somehow?

> Fixes: b00861b9 ("drm/amd/amdgpu: port of DCE v6 to new headers (v3)")
> Fixes: 2285b91c ("drm/amdgpu/dce8: simplify hpd code")

Checking both commits, I can see that 0 is written at `mmDC_HPD1_CONTROL
+ N` register in _hpd_fini() in them, so if you are trying to fix the
commit that inserted that behavior, I think aren't those ones. For instance:

$ git show 2285b91c

[...]

@@ -479,28 +372,11 @@ static void dce_v8_0_hpd_fini(struct amdgpu_device
*adev)
        list_for_each_entry(connector, &dev->mode_config.connector_list,
head) {
                struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(connector);

-               switch (amdgpu_connector->hpd.hpd) {
-               case AMDGPU_HPD_1:
-                       WREG32(mmDC_HPD1_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_2:
-                       WREG32(mmDC_HPD2_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_3:
-                       WREG32(mmDC_HPD3_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_4:
-                       WREG32(mmDC_HPD4_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_5:
-                       WREG32(mmDC_HPD5_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_6:
-                       WREG32(mmDC_HPD6_CONTROL, 0);
-                       break;
-               default:
-                       break;
-               }
+               if (amdgpu_connector->hpd.hpd >= adev->mode_info.num_hpd)
+                       continue;
+
+               WREG32(mmDC_HPD1_CONTROL +
hpd_offsets[amdgpu_connector->hpd.hpd], 0);
+

0 was the valued written here before this commit. The commit basically
replaces the switch case with a sum in this snippet.

thanks,
	andré