Re: [PATCH 2/7] drm/i915: Resolving the memory region conflict for Stolen area

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jan 09, 2014 at 11:00:28AM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
> 
> There is a conflict seen when requesting the kernel to reserve
> the physical space used for the stolen area. This is because
> as somehow the start/base location of the Parent region
> of Stolen area which is PCI Bus 0000:00 is not coinciding with
> the start of Stolen area and is conflicting with it. For ex.
> from the device memory map info provided by '/proc/iomem',
> we have seen that somehow a region of PCI Bus 0000:00 is being
> shown to start from 0x7b000001,
> i.e. (7b000001-ffffffff : PCI Bus 0000:00) & not from
> 0x7b000000. And the stolen base is coming as 0x7b000000,
> thus there is a conflict & stolen area remains unused by driver.
> So to circumvent this issue we adjust the base of stolen area by
> 1 byte when registering it with the kernel.
> Otherwise if we don't resolve this conflict or don't remove the
> conflict error check from the driver, the stolen area will remain
> disabled.
> 
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 1a24e84..29b3693 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -82,9 +82,30 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>  	r = devm_request_mem_region(dev->dev, base, dev_priv->gtt.stolen_size,
>  				    "Graphics Stolen Memory");
>  	if (r == NULL) {
> -		DRM_ERROR("conflict detected with stolen region: [0x%08x - 0x%08x]\n",
> -			  base, base + (uint32_t)dev_priv->gtt.stolen_size);
> -		base = 0;
> +		/* One more attempt but this time requesting region from
> +		 * base + 1, as we have seen that this resolves the region
> +		 * conflict with the PCI Bus.
> +		 * This is because as somehow the start/base location of
> +		 * the Parent region of Stolen area which is PCI Bus 0000:00
> +		 * is not coinciding with the start of Stolen area and is
> +		 * conflicting with it. For ex. from the device memory map
> +		 * info provided by '/proc/iomem', we have seen that somehow
> +		 * a region of PCI Bus 0000:00 is being shown to start from
> +		 * 0x7b000001, i.e. (7b000001-ffffffff : PCI Bus 0000:00).
> +		 * And the stolen base is coming as 0x7b000000, thus there is a
> +		 * conflict and stolen memory remains unused by the driver.
> +		 * So to circumvent this issue we adjust the base of stolen
> +		 * area by 1 when registering it with the kernel.
> +		 */

I think this comment can be shortened a bit:

		/*
		 * BIOS w/a: Some BIOS wrap stolen in the root PCI bus,
		 * but have an off-by-one error. Hence retry the
		 * reservation starting from 1 instead of 0.
		 */

Curious readers can then bring up the full details with git blame. Also
note that we now prefer the multi-line comment delimiters /* and */ on
separate lines.

Cheers, Daniel

> +		r = devm_request_mem_region(dev->dev, base + 1,
> +					    dev_priv->gtt.stolen_size - 1,
> +					    "Graphics Stolen Memory");
> +		if (r == NULL) {
> +			DRM_ERROR("conflict detected with stolen region:"\
> +				  "[0x%08x - 0x%08x]\n",
> +				  base, base + (uint32_t)dev_priv->gtt.stolen_size);
> +			base = 0;
> +		}
>  	}
>  
>  	return base;
> -- 
> 1.8.5.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch