diff mbox

[RFC,1/2] drm/radeon: remove unneeded HDMI r/w ops on DCE4+

Message ID 1381092408-27135-1-git-send-email-zajec5@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rafał Miłecki Oct. 6, 2013, 8:46 p.m. UTC
Write to HDMI_VBI_PACKET_CONTROL was duplicated.
Writes to AFMT_AUDIO_CRC_CONTROL and AFMT_RAMP_CONTROL[0-3] came from
DCE2/3 code (copy & paste) and were never needed on DCE4+.

See https://bugzilla.kernel.org/show_bug.cgi?id=62591 for details.
---
That patches weren't tested with the HW, please don't apply them yet.
---
 drivers/gpu/drm/radeon/evergreen_hdmi.c |   11 -----------
 1 file changed, 11 deletions(-)

Comments

Alex Deucher Oct. 7, 2013, 4:35 p.m. UTC | #1
On Sun, Oct 6, 2013 at 4:46 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> Write to HDMI_VBI_PACKET_CONTROL was duplicated.
> Writes to AFMT_AUDIO_CRC_CONTROL and AFMT_RAMP_CONTROL[0-3] came from
> DCE2/3 code (copy & paste) and were never needed on DCE4+.
>
> See https://bugzilla.kernel.org/show_bug.cgi?id=62591 for details.

I think in general it would be better to do a read/modify/write
sequences for all hdmi register updates rather than just writing.
That's generally what the hw team recommends and what the catalyst
driver does.

Alex

> ---
> That patches weren't tested with the HW, please don't apply them yet.
> ---
>  drivers/gpu/drm/radeon/evergreen_hdmi.c |   11 -----------
>  1 file changed, 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen_hdmi.c b/drivers/gpu/drm/radeon/evergreen_hdmi.c
> index f71ce39..50f6299 100644
> --- a/drivers/gpu/drm/radeon/evergreen_hdmi.c
> +++ b/drivers/gpu/drm/radeon/evergreen_hdmi.c
> @@ -257,11 +257,6 @@ void evergreen_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode
>         evergreen_audio_set_dto(encoder, mode->clock);
>
>         WREG32(HDMI_VBI_PACKET_CONTROL + offset,
> -              HDMI_NULL_SEND); /* send null packets when required */
> -
> -       WREG32(AFMT_AUDIO_CRC_CONTROL + offset, 0x1000);
> -
> -       WREG32(HDMI_VBI_PACKET_CONTROL + offset,
>                HDMI_NULL_SEND | /* send null packets when required */
>                HDMI_GC_SEND | /* send general control packets */
>                HDMI_GC_CONT); /* send general control packets every frame */
> @@ -349,12 +344,6 @@ void evergreen_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode
>
>         WREG32_OR(AFMT_AUDIO_PACKET_CONTROL + offset,
>                   AFMT_AUDIO_SAMPLE_SEND); /* send audio packets */
> -
> -       /* it's unknown what these bits do excatly, but it's indeed quite useful for debugging */
> -       WREG32(AFMT_RAMP_CONTROL0 + offset, 0x00FFFFFF);
> -       WREG32(AFMT_RAMP_CONTROL1 + offset, 0x007FFFFF);
> -       WREG32(AFMT_RAMP_CONTROL2 + offset, 0x00000001);
> -       WREG32(AFMT_RAMP_CONTROL3 + offset, 0x00000001);
>  }
>
>  void evergreen_hdmi_enable(struct drm_encoder *encoder, bool enable)
> --
> 1.7.10.4
>
Rafał Miłecki Oct. 7, 2013, 4:47 p.m. UTC | #2
2013/10/7 Alex Deucher <alexdeucher@gmail.com>:
> On Sun, Oct 6, 2013 at 4:46 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>> Write to HDMI_VBI_PACKET_CONTROL was duplicated.
>> Writes to AFMT_AUDIO_CRC_CONTROL and AFMT_RAMP_CONTROL[0-3] came from
>> DCE2/3 code (copy & paste) and were never needed on DCE4+.
>>
>> See https://bugzilla.kernel.org/show_bug.cgi?id=62591 for details.
>
> I think in general it would be better to do a read/modify/write
> sequences for all hdmi register updates rather than just writing.
> That's generally what the hw team recommends and what the catalyst
> driver does.

See my patch 3/2 for that :) I agree with you.
Alex Deucher Oct. 7, 2013, 5:04 p.m. UTC | #3
On Mon, Oct 7, 2013 at 12:47 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> 2013/10/7 Alex Deucher <alexdeucher@gmail.com>:
>> On Sun, Oct 6, 2013 at 4:46 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>>> Write to HDMI_VBI_PACKET_CONTROL was duplicated.
>>> Writes to AFMT_AUDIO_CRC_CONTROL and AFMT_RAMP_CONTROL[0-3] came from
>>> DCE2/3 code (copy & paste) and were never needed on DCE4+.
>>>
>>> See https://bugzilla.kernel.org/show_bug.cgi?id=62591 for details.
>>
>> I think in general it would be better to do a read/modify/write
>> sequences for all hdmi register updates rather than just writing.
>> That's generally what the hw team recommends and what the catalyst
>> driver does.
>
> See my patch 3/2 for that :) I agree with you.

Patch set looks good.  Let me know if you have any further questions
about the registers.

Alex
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/evergreen_hdmi.c b/drivers/gpu/drm/radeon/evergreen_hdmi.c
index f71ce39..50f6299 100644
--- a/drivers/gpu/drm/radeon/evergreen_hdmi.c
+++ b/drivers/gpu/drm/radeon/evergreen_hdmi.c
@@ -257,11 +257,6 @@  void evergreen_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode
 	evergreen_audio_set_dto(encoder, mode->clock);
 
 	WREG32(HDMI_VBI_PACKET_CONTROL + offset,
-	       HDMI_NULL_SEND); /* send null packets when required */
-
-	WREG32(AFMT_AUDIO_CRC_CONTROL + offset, 0x1000);
-
-	WREG32(HDMI_VBI_PACKET_CONTROL + offset,
 	       HDMI_NULL_SEND | /* send null packets when required */
 	       HDMI_GC_SEND | /* send general control packets */
 	       HDMI_GC_CONT); /* send general control packets every frame */
@@ -349,12 +344,6 @@  void evergreen_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode
 
 	WREG32_OR(AFMT_AUDIO_PACKET_CONTROL + offset,
 		  AFMT_AUDIO_SAMPLE_SEND); /* send audio packets */
-
-	/* it's unknown what these bits do excatly, but it's indeed quite useful for debugging */
-	WREG32(AFMT_RAMP_CONTROL0 + offset, 0x00FFFFFF);
-	WREG32(AFMT_RAMP_CONTROL1 + offset, 0x007FFFFF);
-	WREG32(AFMT_RAMP_CONTROL2 + offset, 0x00000001);
-	WREG32(AFMT_RAMP_CONTROL3 + offset, 0x00000001);
 }
 
 void evergreen_hdmi_enable(struct drm_encoder *encoder, bool enable)