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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@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@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.

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx