[Intel-gfx] [PATCH 18/18] drm/i915/gen9: Removed byte swapping for csr firmware.

[Sunil Kamath]

On Tuesday 28 July 2015 04:39 PM, Sunil Kamath wrote:
> On Tuesday 28 July 2015 01:38 PM, Nagaraju, Vathsala wrote:
>> Signed-off-by: Vatsala Nagaraju <vatsala.nagraju at intel.com>
>> It's   Vathsala Nagaraju <Vathsala.Nagaraju at intel.com>
>>
>> Commit message: Removed byte swapping for csr firmware.
>>
>> Commit message does not convey as to why the change was made. "This 
>> change is needed as DMC firmware loading failed on skylake due  byte 
>> swap  done in the driver"
>
> yes. agree.
>
> Same review comments as other patches. Bug fixing patches should go 
> first in sequence and also commit message should contain relevant info 
> on fix/patch.
>
> This byte swapping is not needed at all.
>
> Minimal things to take care.
>
> - Sunil
>
>
>
>>
>>
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On 
>> Behalf Of Animesh Manna
>> Sent: Sunday, July 26, 2015 12:31 AM
>> To: intel-gfx at lists.freedesktop.org
>> Cc: Vatsala Nagaraju
>> Subject: [Intel-gfx] [PATCH 18/18] drm/i915/gen9: Removed byte 
>> swapping for csr firmware.
>>
>> Cc: Damien Lespiau <damien.lespiau at intel.com>
>> Cc: Imre Deak <imre.deak at intel.com>
>> Cc: Sunil Kamath <sunil.kamath at intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna at intel.com>
>> Signed-off-by: Vatsala Nagaraju <vatsala.nagraju at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_csr.c | 11 +----------
>>   1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_csr.c 
>> b/drivers/gpu/drm/i915/intel_csr.c
>> index 1858f02..50779e0 100644
>> --- a/drivers/gpu/drm/i915/intel_csr.c
>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>> @@ -328,16 +328,7 @@ static uint32_t *parse_csr_fw(struct 
>> drm_i915_private *dev_priv,
>>           return NULL;
>>       }
>>   -    for (i = 0; i < dmc_header->fw_size; i++) {
>> -        uint32_t *tmp = (u32 *)&fw->data[readcount + i * 4];
>> -        /*
>> -         * The firmware payload is an array of 32 bit words stored in
>> -         * little-endian format in the firmware image and programmed
>> -         * as 32 bit big-endian format to memory.
>> -         */
>> -        dmc_payload[i] = (uint32_t __force) cpu_to_be32(*tmp);
>> -    }
>> -
>> +    memcpy(dmc_payload, &fw->data[readcount], dmc_header->fw_size);

*4 missing for byte conversion here.
memcpy(dmc_payload, &fw->data[readcount], (dmc_header->fw_size)*4); ?
Still issue to be addressed here to confirm that fw is loaded fine.

- Sunil
>>       return dmc_payload;
>>   }
>