| Message ID | 1400231431-4844-2-git-send-email-zajec5@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 16, 2014 at 5:10 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: > What initially seemed to be a typo in fglrx (using register 0x740c > instead of 0x74dc) appeared to be a correct behavior. DCE3 has ACR and > CRC registers swapped which explains why we needed > WREG32(HDMI0_AUDIO_CRC_CONTROL + offset, 0x1000); > > This has been tested for possible regressions on DCE3 HD3470 (RV620). > > Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com> > --- > V2: Use defines for swapped regs, update commit message. > --- > drivers/gpu/drm/radeon/r600_hdmi.c | 8 +++++--- > drivers/gpu/drm/radeon/r600d.h | 2 ++ > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c > index b8b2efa..a828222e 100644 > --- a/drivers/gpu/drm/radeon/r600_hdmi.c > +++ b/drivers/gpu/drm/radeon/r600_hdmi.c > @@ -332,6 +332,7 @@ void r600_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode *mod > u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE]; > struct hdmi_avi_infoframe frame; > uint32_t offset; > + uint32_t acr_ctl; > ssize_t err; > > if (!dig || !dig->afmt) > @@ -351,15 +352,16 @@ void r600_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode *mod > WREG32(HDMI0_VBI_PACKET_CONTROL + offset, > HDMI0_NULL_SEND); /* send null packets when required */ > > - WREG32(HDMI0_AUDIO_CRC_CONTROL + offset, 0x1000); > - > WREG32(HDMI0_AUDIO_PACKET_CONTROL + offset, > HDMI0_AUDIO_SAMPLE_SEND | /* send audio packets */ > HDMI0_AUDIO_DELAY_EN(1) | /* default audio delay */ > HDMI0_AUDIO_PACKETS_PER_LINE(3) | /* should be suffient for all audio modes and small enough for all hblanks */ > HDMI0_60958_CS_UPDATE); /* allow 60958 channel status fields to be updated */ > > - WREG32(HDMI0_ACR_PACKET_CONTROL + offset, > + /* DCE 3.0 uses register that's normally for CRC_CONTROL */ > + acr_ctl = ASIC_IS_DCE3(rdev) ? DCE3_HDMI0_ACR_PACKET_CONTROL : > + HDMI0_ACR_PACKET_CONTROL; > + WREG32(acr_ctl + offset, > HDMI0_ACR_SOURCE | /* select SW CTS value - XXX verify that hw CTS works on all families */ > HDMI0_ACR_AUTO_SEND); /* allow hw to sent ACR packets when required */ > > diff --git a/drivers/gpu/drm/radeon/r600d.h b/drivers/gpu/drm/radeon/r600d.h > index 37455f6..10380c5 100644 > --- a/drivers/gpu/drm/radeon/r600d.h > +++ b/drivers/gpu/drm/radeon/r600d.h > @@ -1038,6 +1038,7 @@ > # define HDMI0_AZ_FORMAT_WTRIG_ACK (1 << 29) > #define HDMI0_AUDIO_CRC_CONTROL 0x740c > # define HDMI0_AUDIO_CRC_EN (1 << 0) > +#define DCE3_HDMI0_ACR_PACKET_CONTROL 0x740c > #define HDMI0_VBI_PACKET_CONTROL 0x7410 > # define HDMI0_NULL_SEND (1 << 0) > # define HDMI0_GC_SEND (1 << 4) > @@ -1166,6 +1167,7 @@ > # define HDMI0_ACR_48 3 > # define HDMI0_ACR_SOURCE (1 << 8) /* 0 - hw; 1 - cts value */ > # define HDMI0_ACR_AUTO_SEND (1 << 12) > +#define DCE3_HDMI0_AUDIO_CRC_CONTROL 0x74dc They aren't swapped in hw, the register defines were just accidentally swapped in the header (probably a copy paste typo). Just swap the defines. No need to keep the old values. Alex > #define HDMI0_RAMP_CONTROL0 0x74e0 > # define HDMI0_RAMP_MAX_COUNT(x) (((x) & 0xffffff) << 0) > #define HDMI0_RAMP_CONTROL1 0x74e4 > -- > 1.8.4.5 >
On 17 May 2014 00:00, Alex Deucher <alexdeucher@gmail.com> wrote: > On Fri, May 16, 2014 at 5:10 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >> +#define DCE3_HDMI0_AUDIO_CRC_CONTROL 0x74dc > > They aren't swapped in hw, the register defines were just accidentally > swapped in the header (probably a copy paste typo). Just swap the > defines. No need to keep the old values. Are you sure about this? I've doubts because fglrx indeed uses 0x74dc for DCE2 0x740c for DCE3 Previously you wrote: "On DCE3, they should be ...". I suspect we may still need old defines to support DCE2.
On 17 May 2014 01:34, Rafa? Mi?ecki <zajec5@gmail.com> wrote: > On 17 May 2014 00:00, Alex Deucher <alexdeucher@gmail.com> wrote: >> On Fri, May 16, 2014 at 5:10 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>> +#define DCE3_HDMI0_AUDIO_CRC_CONTROL 0x74dc >> >> They aren't swapped in hw, the register defines were just accidentally >> swapped in the header (probably a copy paste typo). Just swap the >> defines. No need to keep the old values. > > Are you sure about this? I've doubts because fglrx indeed uses > 0x74dc for DCE2 > 0x740c for DCE3 > > Previously you wrote: "On DCE3, they should be ...". > > I suspect we may still need old defines to support DCE2. I was right. I've just compiled radeon with: acr_ctl = 0x740c; and tested it on DCE2. Audio didn't work. Some debugging: # avivotool regsrange 0x740c 0x740c 0000740c 00001100 (4352) # avivotool regsrange 0x74dc 0x74dc 000074dc 00000000 (0) As expected, audio was fixed by exeuting: # avivotool regset 0x74dc 0x00001100 So it's the fact: DCE2 uses 0x74dc DCE2 uses 0x740c So I believe my patch is OK.
On 17 May 2014 02:14, Rafa? Mi?ecki <zajec5@gmail.com> wrote: > So it's the fact: > DCE2 uses 0x74dc > DCE2 uses 0x740c I meant: DCE2 uses 0x74dc DCE3 uses 0x740c
On Fri, May 16, 2014 at 8:15 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: > On 17 May 2014 02:14, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >> So it's the fact: >> DCE2 uses 0x74dc >> DCE2 uses 0x740c > > I meant: > DCE2 uses 0x74dc > DCE3 uses 0x740c Yeah, I just double checked DCE2 and you are correct. So the patch is fine as is. Alex
diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c index b8b2efa..a828222e 100644 --- a/drivers/gpu/drm/radeon/r600_hdmi.c +++ b/drivers/gpu/drm/radeon/r600_hdmi.c @@ -332,6 +332,7 @@ void r600_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode *mod u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE]; struct hdmi_avi_infoframe frame; uint32_t offset; + uint32_t acr_ctl; ssize_t err; if (!dig || !dig->afmt) @@ -351,15 +352,16 @@ void r600_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode *mod WREG32(HDMI0_VBI_PACKET_CONTROL + offset, HDMI0_NULL_SEND); /* send null packets when required */ - WREG32(HDMI0_AUDIO_CRC_CONTROL + offset, 0x1000); - WREG32(HDMI0_AUDIO_PACKET_CONTROL + offset, HDMI0_AUDIO_SAMPLE_SEND | /* send audio packets */ HDMI0_AUDIO_DELAY_EN(1) | /* default audio delay */ HDMI0_AUDIO_PACKETS_PER_LINE(3) | /* should be suffient for all audio modes and small enough for all hblanks */ HDMI0_60958_CS_UPDATE); /* allow 60958 channel status fields to be updated */ - WREG32(HDMI0_ACR_PACKET_CONTROL + offset, + /* DCE 3.0 uses register that's normally for CRC_CONTROL */ + acr_ctl = ASIC_IS_DCE3(rdev) ? DCE3_HDMI0_ACR_PACKET_CONTROL : + HDMI0_ACR_PACKET_CONTROL; + WREG32(acr_ctl + offset, HDMI0_ACR_SOURCE | /* select SW CTS value - XXX verify that hw CTS works on all families */ HDMI0_ACR_AUTO_SEND); /* allow hw to sent ACR packets when required */ diff --git a/drivers/gpu/drm/radeon/r600d.h b/drivers/gpu/drm/radeon/r600d.h index 37455f6..10380c5 100644 --- a/drivers/gpu/drm/radeon/r600d.h +++ b/drivers/gpu/drm/radeon/r600d.h @@ -1038,6 +1038,7 @@ # define HDMI0_AZ_FORMAT_WTRIG_ACK (1 << 29) #define HDMI0_AUDIO_CRC_CONTROL 0x740c # define HDMI0_AUDIO_CRC_EN (1 << 0) +#define DCE3_HDMI0_ACR_PACKET_CONTROL 0x740c #define HDMI0_VBI_PACKET_CONTROL 0x7410 # define HDMI0_NULL_SEND (1 << 0) # define HDMI0_GC_SEND (1 << 4) @@ -1166,6 +1167,7 @@ # define HDMI0_ACR_48 3 # define HDMI0_ACR_SOURCE (1 << 8) /* 0 - hw; 1 - cts value */ # define HDMI0_ACR_AUTO_SEND (1 << 12) +#define DCE3_HDMI0_AUDIO_CRC_CONTROL 0x74dc #define HDMI0_RAMP_CONTROL0 0x74e0 # define HDMI0_RAMP_MAX_COUNT(x) (((x) & 0xffffff) << 0) #define HDMI0_RAMP_CONTROL1 0x74e4
What initially seemed to be a typo in fglrx (using register 0x740c instead of 0x74dc) appeared to be a correct behavior. DCE3 has ACR and CRC registers swapped which explains why we needed WREG32(HDMI0_AUDIO_CRC_CONTROL + offset, 0x1000); This has been tested for possible regressions on DCE3 HD3470 (RV620). Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com> --- V2: Use defines for swapped regs, update commit message. --- drivers/gpu/drm/radeon/r600_hdmi.c | 8 +++++--- drivers/gpu/drm/radeon/r600d.h | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-)