On Wed, Jul 13, 2011 at 2:28 AM, Benjamin Herrenschmidt benh@kernel.crashing.org wrote:

v6 of the structure was programmed incorrectly:

args.v6.ulCrtcPclkFreq.ulPixelClock = cpu_to_le32(clock / 10);

ulPixelClock is a 24-bit bitfield. This statement would thus do a 32-bit swap of (clock / 10) and drop the top 8 bits which are ... the LSB. Not what we want. Instead use masks & shifts.

Signed-off-by: Benjamin Herrenschmidt benh@kernel.crashing.org

(resent adding dri-devel to the CC list to hit patchwork)

drivers/gpu/drm/radeon/atombios_crtc.c |    5 ++---  1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c index 9541995..c742944 100644 --- a/drivers/gpu/drm/radeon/atombios_crtc.c +++ b/drivers/gpu/drm/radeon/atombios_crtc.c @@ -764,7 +764,7 @@ static void atombios_crtc_set_dcpll(struct drm_crtc *crtc,  }

static void atombios_crtc_program_pll(struct drm_crtc *crtc,

• int crtc_id,

• u32 crtc_id,

int pll_id,                                      u32 encoder_mode,                                      u32 encoder_id, @@ -851,8 +851,7 @@ static void atombios_crtc_program_pll(struct drm_crtc *crtc,                        args.v5.ucPpll = pll_id;                        break;                case 6:

• args.v6.ulCrtcPclkFreq.ucCRTC = crtc_id;
• args.v6.ulCrtcPclkFreq.ulPixelClock = cpu_to_le32(clock / 10);

• args.v6.ulDispEngClkFreq = cpu_to_le32(crtc_id << 24 | clock / 10);

For clarity (i can never remember op precedence), you might put:

cpu_to_le32((crtc_id << 24) | (clock / 10));

Other than that,

Reviewed-by: Alex Deucher alexander.deucher@amd.com

args.v6.ucRefDiv = ref_div;                        args.v6.usFbDiv = cpu_to_le16(fb_div);                        args.v6.ulFbDivDecFrac = cpu_to_le32(frac_fb_div * 100000);