From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alex Deucher <alexdeucher@gmail.com>
Subject: Re: [PATCH 2/6] drm/radeon: ATOM Endian fix for
	atombios_crtc_program_pll()
Date: Wed, 13 Jul 2011 10:38:12 -0400
Message-ID: <CADnq5_Om6yhCC3Jo0mcqmXWj=moBu6RV1c3=Hns9pmQOTcLqDg@mail.gmail.com>
References: <1310538495.4968.92.camel@pasglop>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org>
Received: from mail-iy0-f177.google.com (mail-iy0-f177.google.com
	[209.85.210.177])
	by gabe.freedesktop.org (Postfix) with ESMTP id 8E9519F5F0
	for <dri-devel@lists.freedesktop.org>;
	Wed, 13 Jul 2011 07:38:14 -0700 (PDT)
Received: by iyn15 with SMTP id 15so6615608iyn.36
	for <dri-devel@lists.freedesktop.org>;
	Wed, 13 Jul 2011 07:38:14 -0700 (PDT)
In-Reply-To: <1310538495.4968.92.camel@pasglop>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: xorg-driver-ati@lists.x.org, dri-devel@lists.freedesktop.org, =?ISO-8859-1?Q?C=E9dric_Cano?= <ccano@interfaceconcept.com>
List-Id: dri-devel@lists.freedesktop.org

On Wed, Jul 13, 2011 at 2:28 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> v6 of the structure was programmed incorrectly:
>
> =A0args.v6.ulCrtcPclkFreq.ulPixelClock =3D 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)
>
> =A0drivers/gpu/drm/radeon/atombios_crtc.c | =A0 =A05 ++---
> =A01 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/rad=
eon/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,
> =A0}
>
> =A0static void atombios_crtc_program_pll(struct drm_crtc *crtc,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 int crtc_id,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 u32 crtc_id,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0int pll_id,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0u32 encoder_mode,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0u32 encoder_id,
> @@ -851,8 +851,7 @@ static void atombios_crtc_program_pll(struct drm_crtc=
 *crtc,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args.v5.ucPpll =3D pll_id;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 6:
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 args.v6.ulCrtcPclkFreq.ucCR=
TC =3D crtc_id;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 args.v6.ulCrtcPclkFreq.ulPi=
xelClock =3D cpu_to_le32(clock / 10);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 args.v6.ulDispEngClkFreq =
=3D 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>


> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args.v6.ucRefDiv =3D ref_d=
iv;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args.v6.usFbDiv =3D cpu_to=
_le16(fb_div);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args.v6.ulFbDivDecFrac =3D=
 cpu_to_le32(frac_fb_div * 100000);
>
>
>
>