[Intel-gfx] [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Lucas De Marchi]

On Thu, Nov 21, 2019 at 03:09:03PM +0200, Jani Nikula wrote:
>On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> The unaligned ioread32() will make us read byte by byte looking for the
>> vbt. We could just as well have done a ioread8() + a shift and avoid the
>> extra confusion on how we are looking for "$VBT".
>>
>> However when using ACPI it's guaranteed the VBT is 4-byte aligned
>> per spec, so we can probably assume it here as well.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bios.c | 19 ++++++-------------
>>  1 file changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index aa9b182efee5..6bf57b1ad056 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
>>  	void __iomem *p = NULL, *oprom;
>>  	struct vbt_header *vbt;
>>  	u16 vbt_size;
>> -	size_t i, size;
>> +	size_t size;
>>
>>  	oprom = pci_map_rom(pdev, &size);
>>  	if (!oprom)
>>  		return NULL;
>>
>>  	/* Scour memory looking for the VBT signature. */
>> -	for (i = 0; i + 4 < size; i++) {
>> -		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
>> -			continue;
>> -
>> -		p = oprom + i;
>> -		size -= i;
>> -		break;
>> -	}
>> -
>> -	if (!p)
>> -		goto err_unmap_oprom;
>> +	for (p = oprom; size >= 4; p += 4, size -= 4)
>> +		if (ioread32(p) == *((const u32 *)"$VBT"))
>> +			break;
>
>I think the original is easier to read. You only really need to change
>"i++" to "i += 4" and be done with it.

I really liked this version much more... shorter and with only one control
variable rather than keeping the control in 3 different places (i, size
and p).

Lucas De Marchi

>
>BR,
>Jani.
>
>>
>>  	if (sizeof(struct vbt_header) > size) {
>> -		DRM_DEBUG_DRIVER("VBT header incomplete\n");
>> +		if (size >= 4)
>> +			DRM_DEBUG_DRIVER("VBT header incomplete\n");
>>  		goto err_unmap_oprom;
>>  	}
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center