[PATCH] drm/radeon: fix endian bugs in radeon_atom_get_clock_dividers()

Alex Deucher alexdeucher at gmail.com
Mon Apr 22 07:18:09 PDT 2013


On Mon, Apr 22, 2013 at 10:08 AM, Dan Carpenter
<dan.carpenter at oracle.com> wrote:
> On Mon, Apr 22, 2013 at 10:03:13AM -0400, alexdeucher at gmail.com wrote:
>> From: Alex Deucher <alexander.deucher at amd.com>
>>
>> Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>> ---
>>  drivers/gpu/drm/radeon/atombios.h        |    2 ++
>>  drivers/gpu/drm/radeon/radeon_atombios.c |    6 ++----
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/atombios.h b/drivers/gpu/drm/radeon/atombios.h
>> index 4b04ba3..de678dd 100644
>> --- a/drivers/gpu/drm/radeon/atombios.h
>> +++ b/drivers/gpu/drm/radeon/atombios.h
>> @@ -459,6 +459,7 @@ typedef struct _COMPUTE_MEMORY_ENGINE_PLL_PARAMETERS_V3
>>    {
>>      ATOM_COMPUTE_CLOCK_FREQ  ulClock;         //Input Parameter
>>      ATOM_S_MPLL_FB_DIVIDER   ulFbDiv;         //Output Parameter
>> +    ULONG ulClockFbDiv;
>
> Why is this a long instead of an __le32 or u32?

atombios.h is shared across OSes and has it's own types.

>
>
>>    };
>>    UCHAR   ucRefDiv;                           //Output Parameter
>>    UCHAR   ucPostDiv;                          //Output Parameter
>> @@ -491,6 +492,7 @@ typedef struct _COMPUTE_MEMORY_ENGINE_PLL_PARAMETERS_V5
>>    {
>>      ATOM_COMPUTE_CLOCK_FREQ  ulClock;         //Input Parameter
>>      ATOM_S_MPLL_FB_DIVIDER   ulFbDiv;         //Output Parameter
>> +    ULONG ulClockFbDiv;
>>    };
>>    UCHAR   ucRefDiv;                           //Output Parameter
>>    UCHAR   ucPostDiv;                          //Output Parameter
>> diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c
>> index 8c1779c..2fc444e 100644
>> --- a/drivers/gpu/drm/radeon/radeon_atombios.c
>> +++ b/drivers/gpu/drm/radeon/radeon_atombios.c
>> @@ -2710,8 +2710,7 @@ int radeon_atom_get_clock_dividers(struct radeon_device *rdev,
>>                               dividers->enable_post_div = (dividers->fb_div & 1) ? true : false;
>>               } else {
>>                       if (clock_type == COMPUTE_ENGINE_PLL_PARAM) {
>> -                             args.v3.ulClock.ulComputeClockFlag = clock_type;
>> -                             args.v3.ulClock.ulClockFreq = cpu_to_le32(clock);       /* 10 khz */
>> +                             args.v3.ulClockFbDiv = cpu_to_le32((clock_type << 24) | clock);
>>
>>                               atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
>>
>> @@ -2726,8 +2725,7 @@ int radeon_atom_get_clock_dividers(struct radeon_device *rdev,
>>                               dividers->vco_mode = (args.v3.ucCntlFlag &
>>                                                     ATOM_PLL_CNTL_FLAG_MPLL_VCO_MODE) ? 1 : 0;
>>                       } else {
>> -                             args.v5.ulClock.ulComputeClockFlag = clock_type;
>> -                             args.v5.ulClock.ulClockFreq = cpu_to_le32(clock);       /* 10 khz */
>> +                             args.v3.ulClockFbDiv = cpu_to_le32((clock_type << 24) | clock);
>
> We've changed from v5 to v3.  Was that intentional?

Copy paste typo.  fixed in v2 which is attached.

>
> I'm confused by this patch as well.  I assumed the datatypes were
> determined by the hardware spec.

I'm not sure I follow.  The atombios interpretor requires data in
little endian format.

Alex
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-radeon-fix-endian-bugs-in-radeon_atom_get_clock_.patch
Type: text/x-patch
Size: 2723 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130422/94213d69/attachment.bin>


More information about the dri-devel mailing list