diff mbox

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

Message ID 1389610521-12158-1-git-send-email-akash.goel@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

akash.goel@intel.com Jan. 13, 2014, 10:55 a.m. UTC
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
some BIOS are wrapping the stolen area in the root PCI bus, but have
an off-by-one error. As a workaround we retry the reservation with an
offset of 1 instead of 0.

v2: updated commit message & the comment in source file (Daniel)

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Comments

Daniel Vetter Jan. 28, 2014, 9:14 a.m. UTC | #1
On Tue, Jan 28, 2014 at 10:10 AM, Goel, Akash <akash.goel@intel.com> wrote:
> Hi Daniel,
>
> Please let us know about this patch, will this be up-streamed.

1. I've just sent out the merge confirmation mail minutes ago.
2. Please _never_ drop mailing lists when poking me (or in general,
really). See the internally documented bkm in the patch submission
guidelines.

Cheers, Daniel

>
> Best Regards
> Akash
>
> -----Original Message-----
> From: Goel, Akash
> Sent: Monday, January 13, 2014 4:25 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Goel, Akash
> Subject: [PATCH 2/7] drm/i915: Resolving the memory region conflict for Stolen area
>
> 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 some BIOS are wrapping the stolen area in the root PCI bus, but have an off-by-one error. As a workaround we retry the reservation with an offset of 1 instead of 0.
>
> v2: updated commit message & the comment in source file (Daniel)
>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++++++++++++---
>  1 file changed, 17 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..114a806 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -82,9 +82,23 @@ 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 a 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.
> +                */
> +               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
>
Jesse Barnes Feb. 18, 2014, 7:49 p.m. UTC | #2
On Tue, 28 Jan 2014 10:14:03 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Jan 28, 2014 at 10:10 AM, Goel, Akash <akash.goel@intel.com> wrote:
> > Hi Daniel,
> >
> > Please let us know about this patch, will this be up-streamed.
> 
> 1. I've just sent out the merge confirmation mail minutes ago.
> 2. Please _never_ drop mailing lists when poking me (or in general,
> really). See the internally documented bkm in the patch submission
> guidelines.
> 
> Cheers, Daniel
> 
> >
> > Best Regards
> > Akash
> >
> > -----Original Message-----
> > From: Goel, Akash
> > Sent: Monday, January 13, 2014 4:25 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Goel, Akash
> > Subject: [PATCH 2/7] drm/i915: Resolving the memory region conflict for Stolen area
> >
> > 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 some BIOS are wrapping the stolen area in the root PCI bus, but have an off-by-one error. As a workaround we retry the reservation with an offset of 1 instead of 0.
> >
> > v2: updated commit message & the comment in source file (Daniel)
> >
> > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++++++++++++---
> >  1 file changed, 17 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..114a806 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -82,9 +82,23 @@ 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 a 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.
> > +                */
> > +               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;

It doesn't look like this was actually merged, and it belongs in -fixes
with a cc: stable since it affects existing machines with these BIOS
issues.

Cc'ing Jani so he can pick it up when he returns.
Jani Nikula Feb. 24, 2014, 7:22 p.m. UTC | #3
On Tue, 18 Feb 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Tue, 28 Jan 2014 10:14:03 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
>> On Tue, Jan 28, 2014 at 10:10 AM, Goel, Akash <akash.goel@intel.com> wrote:
>> > Hi Daniel,
>> >
>> > Please let us know about this patch, will this be up-streamed.
>> 
>> 1. I've just sent out the merge confirmation mail minutes ago.
>> 2. Please _never_ drop mailing lists when poking me (or in general,
>> really). See the internally documented bkm in the patch submission
>> guidelines.
>> 
>> Cheers, Daniel
>> 
>> >
>> > Best Regards
>> > Akash
>> >
>> > -----Original Message-----
>> > From: Goel, Akash
>> > Sent: Monday, January 13, 2014 4:25 PM
>> > To: intel-gfx@lists.freedesktop.org
>> > Cc: Goel, Akash
>> > Subject: [PATCH 2/7] drm/i915: Resolving the memory region conflict for Stolen area
>> >
>> > 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 some BIOS are wrapping the stolen area in the root PCI bus, but have an off-by-one error. As a workaround we retry the reservation with an offset of 1 instead of 0.
>> >
>> > v2: updated commit message & the comment in source file (Daniel)
>> >
>> > Signed-off-by: Akash Goel <akash.goel@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++++++++++++---
>> >  1 file changed, 17 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..114a806 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> > @@ -82,9 +82,23 @@ 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 a 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.
>> > +                */
>> > +               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;
>
> It doesn't look like this was actually merged, and it belongs in -fixes
> with a cc: stable since it affects existing machines with these BIOS
> issues.

It looks like Daniel was referring to [1] which includes

commit ec14ba47791965d2c08e0a681ff44eacbf3c4553
Author: Akash Goel <akash.goel@intel.com>
Date:   Mon Jan 13 16:24:45 2014 +0530

    drm/i915: Fix the offset issue for the stolen GEM objects

i.e. something else.

> Cc'ing Jani so he can pick it up when he returns.

I'm going to need a Reviewed-by and preferrably a Tested-by on this.

Thanks,
Jani.


[1] http://marc.info/?i=20140128085325.GA7422@phenom.ffwll.local
Jesse Barnes Feb. 26, 2014, 4:21 p.m. UTC | #4
On Mon, 13 Jan 2014 16:25:21 +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
> some BIOS are wrapping the stolen area in the root PCI bus, but have
> an off-by-one error. As a workaround we retry the reservation with an
> offset of 1 instead of 0.
> 
> v2: updated commit message & the comment in source file (Daniel)
> 
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++++++++++++---
>  1 file changed, 17 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..114a806 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -82,9 +82,23 @@ 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 a 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.
> +		 */
> +		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;

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Tested-by: Arjan van de Ven <arjan@linux.intel.com>

Thanks,
Jani Nikula Feb. 27, 2014, 8:59 a.m. UTC | #5
On Wed, 26 Feb 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 13 Jan 2014 16:25:21 +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
>> some BIOS are wrapping the stolen area in the root PCI bus, but have
>> an off-by-one error. As a workaround we retry the reservation with an
>> offset of 1 instead of 0.
>> 
>> v2: updated commit message & the comment in source file (Daniel)
>> 
>> Signed-off-by: Akash Goel <akash.goel@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++++++++++++---
>>  1 file changed, 17 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..114a806 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> @@ -82,9 +82,23 @@ 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 a 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.
>> +		 */
>> +		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;
>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Tested-by: Arjan van de Ven <arjan@linux.intel.com>

Pushed to -fixes, thanks for the patch, review, and testing.

Jani.


>
> Thanks,
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Feb. 27, 2014, 9:01 a.m. UTC | #6
On Thu, 27 Feb 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Wed, 26 Feb 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> On Mon, 13 Jan 2014 16:25:21 +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
>>> some BIOS are wrapping the stolen area in the root PCI bus, but have
>>> an off-by-one error. As a workaround we retry the reservation with an
>>> offset of 1 instead of 0.
>>> 
>>> v2: updated commit message & the comment in source file (Daniel)
>>> 
>>> Signed-off-by: Akash Goel <akash.goel@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++++++++++++---
>>>  1 file changed, 17 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..114a806 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>>> @@ -82,9 +82,23 @@ 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 a 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.
>>> +		 */
>>> +		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;
>>
>> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>> Tested-by: Arjan van de Ven <arjan@linux.intel.com>
>
> Pushed to -fixes, thanks for the patch, review, and testing.

Oh, forgot to add that I fixed the DRM_ERROR string split up that
checkpatch complained about while applying.


>
> Jani.
>
>
>>
>> Thanks,
>> -- 
>> Jesse Barnes, Intel Open Source Technology Center
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Jani Nikula, Intel Open Source Technology Center
Jesse Barnes March 3, 2014, 7:14 p.m. UTC | #7
On Thu, 27 Feb 2014 11:01:08 +0200
Jani Nikula <jani.nikula@linux.intel.com> wrote:

> On Thu, 27 Feb 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Wed, 26 Feb 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >> On Mon, 13 Jan 2014 16:25:21 +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
> >>> some BIOS are wrapping the stolen area in the root PCI bus, but have
> >>> an off-by-one error. As a workaround we retry the reservation with an
> >>> offset of 1 instead of 0.
> >>> 
> >>> v2: updated commit message & the comment in source file (Daniel)
> >>> 
> >>> Signed-off-by: Akash Goel <akash.goel@intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++++++++++++---
> >>>  1 file changed, 17 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..114a806 100644
> >>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> >>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> >>> @@ -82,9 +82,23 @@ 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 a 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.
> >>> +		 */
> >>> +		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;
> >>
> >> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> >> Tested-by: Arjan van de Ven <arjan@linux.intel.com>
> >
> > Pushed to -fixes, thanks for the patch, review, and testing.
> 
> Oh, forgot to add that I fixed the DRM_ERROR string split up that
> checkpatch complained about while applying.

Note I think we should do this at the x86 quirk level as well since
otherwise the kernel may put MMIO space on top of our stolen range.
Jesse Barnes March 3, 2014, 10:33 p.m. UTC | #8
On Mon, 3 Mar 2014 11:14:09 -0800
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Thu, 27 Feb 2014 11:01:08 +0200
> Jani Nikula <jani.nikula@linux.intel.com> wrote:
> 
> > On Thu, 27 Feb 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > > On Wed, 26 Feb 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > >> On Mon, 13 Jan 2014 16:25:21 +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
> > >>> some BIOS are wrapping the stolen area in the root PCI bus, but have
> > >>> an off-by-one error. As a workaround we retry the reservation with an
> > >>> offset of 1 instead of 0.
> > >>> 
> > >>> v2: updated commit message & the comment in source file (Daniel)
> > >>> 
> > >>> Signed-off-by: Akash Goel <akash.goel@intel.com>
> > >>> ---
> > >>>  drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++++++++++++---
> > >>>  1 file changed, 17 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..114a806 100644
> > >>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > >>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > >>> @@ -82,9 +82,23 @@ 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 a 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.
> > >>> +		 */
> > >>> +		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;
> > >>
> > >> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > >> Tested-by: Arjan van de Ven <arjan@linux.intel.com>
> > >
> > > Pushed to -fixes, thanks for the patch, review, and testing.
> > 
> > Oh, forgot to add that I fixed the DRM_ERROR string split up that
> > checkpatch complained about while applying.
> 
> Note I think we should do this at the x86 quirk level as well since
> otherwise the kernel may put MMIO space on top of our stolen range.

Nevermind, I think the stolen core code will do the right thing and
mark this region reserved... but it's worth checking on an affected
system.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 1a24e84..114a806 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -82,9 +82,23 @@  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 a 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.
+		 */
+		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;