[PATCH drm/amdgpu] Use correct mask in dce_v8_0_afmt_setmode() and fix comment typos.

[PATCH drm/amdgpu] Use correct mask in dce_v8_0_afmt_setmode() and fix comment typos.

[Alexandre Demers]

We were using the same mask twice. Looking at radeon, it seems
we should be using HDMI_AVI_INFO_CONT instead as the second mask.

Being there, fix typos in comments and improved readability.

I haven't looked at other DCEs, the mask may also be wrong for them.

Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index c7e5d5f..e424ecc 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1693,6 +1693,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
 	/* Silent, r600_hdmi_enable will raise WARN for us */
 	if (!dig->afmt->enabled)
 		return;
+
 	offset = dig->afmt->offset;
 
 	/* hdmi deep color mode general control packets setup, if bpc > 8 */
@@ -1817,7 +1818,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
 
 	WREG32_OR(mmHDMI_INFOFRAME_CONTROL0 + offset,
 		  HDMI_INFOFRAME_CONTROL0__HDMI_AVI_INFO_SEND_MASK | /* enable AVI info frames */
-		  HDMI_INFOFRAME_CONTROL0__HDMI_AVI_INFO_SEND_MASK); /* required for audio info values to be updated */
+		  HDMI_INFOFRAME_CONTROL0__HDMI_AVI_INFO_CONT_MASK); /* required for audio info values to be updated */
 
 	WREG32_P(mmHDMI_INFOFRAME_CONTROL1 + offset,
 		 (2 << HDMI_INFOFRAME_CONTROL1__HDMI_AVI_INFO_LINE__SHIFT), /* anything other than 0 */
@@ -1826,13 +1827,13 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
 	WREG32_OR(mmAFMT_AUDIO_PACKET_CONTROL + offset,
 		  AFMT_AUDIO_PACKET_CONTROL__AFMT_AUDIO_SAMPLE_SEND_MASK); /* send audio packets */
 
-	/* it's unknown what these bits do excatly, but it's indeed quite useful for debugging */
+	/* it's unknown what these bits do exactly, but it's indeed quite useful for debugging */
 	WREG32(mmAFMT_RAMP_CONTROL0 + offset, 0x00FFFFFF);
 	WREG32(mmAFMT_RAMP_CONTROL1 + offset, 0x007FFFFF);
 	WREG32(mmAFMT_RAMP_CONTROL2 + offset, 0x00000001);
 	WREG32(mmAFMT_RAMP_CONTROL3 + offset, 0x00000001);
 
-	/* enable audio after to setting up hw */
+	/* enable audio after setting up hw */
 	dce_v8_0_audio_enable(adev, dig->afmt->pin, true);
 }
 
-- 
2.9.3

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

Re: [PATCH drm/amdgpu] Use correct mask in dce_v8_0_afmt_setmode() and fix comment typos.

[Edward O'Callaghan]

[-- Attachment #1.1.1: Type: text/plain, Size: 2588 bytes --]



On 08/22/2016 10:38 AM, Alexandre Demers wrote:
> We were using the same mask twice. Looking at radeon, it seems
> we should be using HDMI_AVI_INFO_CONT instead as the second mask.
> 
> Being there, fix typos in comments and improved readability.
> 
> I haven't looked at other DCEs, the mask may also be wrong for them.
> 
> Signed-off-by: Alexandre Demers <alexandre.f.demers-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Edward O'Callaghan <funfunctor-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org>

> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index c7e5d5f..e424ecc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1693,6 +1693,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
>  	/* Silent, r600_hdmi_enable will raise WARN for us */
>  	if (!dig->afmt->enabled)
>  		return;
> +
>  	offset = dig->afmt->offset;
>  
>  	/* hdmi deep color mode general control packets setup, if bpc > 8 */
> @@ -1817,7 +1818,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
>  
>  	WREG32_OR(mmHDMI_INFOFRAME_CONTROL0 + offset,
>  		  HDMI_INFOFRAME_CONTROL0__HDMI_AVI_INFO_SEND_MASK | /* enable AVI info frames */
> -		  HDMI_INFOFRAME_CONTROL0__HDMI_AVI_INFO_SEND_MASK); /* required for audio info values to be updated */
> +		  HDMI_INFOFRAME_CONTROL0__HDMI_AVI_INFO_CONT_MASK); /* required for audio info values to be updated */
>  
>  	WREG32_P(mmHDMI_INFOFRAME_CONTROL1 + offset,
>  		 (2 << HDMI_INFOFRAME_CONTROL1__HDMI_AVI_INFO_LINE__SHIFT), /* anything other than 0 */
> @@ -1826,13 +1827,13 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
>  	WREG32_OR(mmAFMT_AUDIO_PACKET_CONTROL + offset,
>  		  AFMT_AUDIO_PACKET_CONTROL__AFMT_AUDIO_SAMPLE_SEND_MASK); /* send audio packets */
>  
> -	/* it's unknown what these bits do excatly, but it's indeed quite useful for debugging */
> +	/* it's unknown what these bits do exactly, but it's indeed quite useful for debugging */
>  	WREG32(mmAFMT_RAMP_CONTROL0 + offset, 0x00FFFFFF);
>  	WREG32(mmAFMT_RAMP_CONTROL1 + offset, 0x007FFFFF);
>  	WREG32(mmAFMT_RAMP_CONTROL2 + offset, 0x00000001);
>  	WREG32(mmAFMT_RAMP_CONTROL3 + offset, 0x00000001);
>  
> -	/* enable audio after to setting up hw */
> +	/* enable audio after setting up hw */
>  	dce_v8_0_audio_enable(adev, dig->afmt->pin, true);
>  }
>  
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH drm/amdgpu] Use correct mask in dce_v8_0_afmt_setmode() and fix comment typos.

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 3136 bytes --]

Am 22.08.2016 um 02:41 schrieb Edward O'Callaghan:
>
> On 08/22/2016 10:38 AM, Alexandre Demers wrote:
>> We were using the same mask twice. Looking at radeon, it seems
>> we should be using HDMI_AVI_INFO_CONT instead as the second mask.
>>
>> Being there, fix typos in comments and improved readability.
>>
>> I haven't looked at other DCEs, the mask may also be wrong for them.
>>
>> Signed-off-by: Alexandre Demers <alexandre.f.demers-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Edward O'Callaghan <funfunctor-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org>
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> index c7e5d5f..e424ecc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> @@ -1693,6 +1693,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
>>   	/* Silent, r600_hdmi_enable will raise WARN for us */
>>   	if (!dig->afmt->enabled)
>>   		return;
>> +
>>   	offset = dig->afmt->offset;
>>   
>>   	/* hdmi deep color mode general control packets setup, if bpc > 8 */
>> @@ -1817,7 +1818,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
>>   
>>   	WREG32_OR(mmHDMI_INFOFRAME_CONTROL0 + offset,
>>   		  HDMI_INFOFRAME_CONTROL0__HDMI_AVI_INFO_SEND_MASK | /* enable AVI info frames */
>> -		  HDMI_INFOFRAME_CONTROL0__HDMI_AVI_INFO_SEND_MASK); /* required for audio info values to be updated */
>> +		  HDMI_INFOFRAME_CONTROL0__HDMI_AVI_INFO_CONT_MASK); /* required for audio info values to be updated */
>>   
>>   	WREG32_P(mmHDMI_INFOFRAME_CONTROL1 + offset,
>>   		 (2 << HDMI_INFOFRAME_CONTROL1__HDMI_AVI_INFO_LINE__SHIFT), /* anything other than 0 */
>> @@ -1826,13 +1827,13 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
>>   	WREG32_OR(mmAFMT_AUDIO_PACKET_CONTROL + offset,
>>   		  AFMT_AUDIO_PACKET_CONTROL__AFMT_AUDIO_SAMPLE_SEND_MASK); /* send audio packets */
>>   
>> -	/* it's unknown what these bits do excatly, but it's indeed quite useful for debugging */
>> +	/* it's unknown what these bits do exactly, but it's indeed quite useful for debugging */

You can actually completely drop this comment now. I added it when I 
originally reverse engineered the hardware to figure out how HDMI audio 
works.

With that fixed the patch is Reviewed-by: Christian König 
<christian.koenig-5C7GfCeVMHo@public.gmane.org>.

Regards,
Christian.

>>   	WREG32(mmAFMT_RAMP_CONTROL0 + offset, 0x00FFFFFF);
>>   	WREG32(mmAFMT_RAMP_CONTROL1 + offset, 0x007FFFFF);
>>   	WREG32(mmAFMT_RAMP_CONTROL2 + offset, 0x00000001);
>>   	WREG32(mmAFMT_RAMP_CONTROL3 + offset, 0x00000001);
>>   
>> -	/* enable audio after to setting up hw */
>> +	/* enable audio after setting up hw */
>>   	dce_v8_0_audio_enable(adev, dig->afmt->pin, true);
>>   }
>>   
>>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[-- Attachment #1.2: Type: text/html, Size: 4534 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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