diff mbox

[v3,6/9] radeon: Switch to arch_phys_wc_add and add a missing ..._del

Message ID bf7788cfd4695d05f2b414735744105906650e42.1368485053.git.luto@amacapital.net (mailing list archive)
State New, archived
Headers show

Commit Message

Andy Lutomirski May 13, 2013, 11:58 p.m. UTC 
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 drivers/gpu/drm/radeon/radeon_object.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Alex Deucher May 14, 2013, 12:58 p.m. UTC | #1 
On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/radeon/radeon_object.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index d3aface..15cd34b 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -321,8 +321,8 @@ void radeon_bo_force_delete(struct radeon_device *rdev)
>  int radeon_bo_init(struct radeon_device *rdev)
>  {
>         /* Add an MTRR for the VRAM */
> -       rdev->mc.vram_mtrr = mtrr_add(rdev->mc.aper_base, rdev->mc.aper_size,
> -                       MTRR_TYPE_WRCOMB, 1);
> +       rdev->mc.vram_mtrr = arch_phys_wc_add(rdev->mc.aper_base,
> +                                             rdev->mc.aper_size);
>         DRM_INFO("Detected VRAM RAM=%lluM, BAR=%lluM\n",
>                 rdev->mc.mc_vram_size >> 20,
>                 (unsigned long long)rdev->mc.aper_size >> 20);
> @@ -334,6 +334,7 @@ int radeon_bo_init(struct radeon_device *rdev)
>  void radeon_bo_fini(struct radeon_device *rdev)
>  {
>         radeon_ttm_fini(rdev);
> +       arch_phys_wc_del(rdev->mc.vram_mtrr);
>  }
>
>  void radeon_bo_list_add_object(struct radeon_bo_list *lobj,
> --
> 1.8.1.4
>
Jerome Glisse May 14, 2013, 1:37 p.m. UTC | #2 
On Tue, May 14, 2013 at 8:58 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

I believe it will break something but we could deal with the fallout
once it happens.

>> ---
>>  drivers/gpu/drm/radeon/radeon_object.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
>> index d3aface..15cd34b 100644
>> --- a/drivers/gpu/drm/radeon/radeon_object.c
>> +++ b/drivers/gpu/drm/radeon/radeon_object.c
>> @@ -321,8 +321,8 @@ void radeon_bo_force_delete(struct radeon_device *rdev)
>>  int radeon_bo_init(struct radeon_device *rdev)
>>  {
>>         /* Add an MTRR for the VRAM */
>> -       rdev->mc.vram_mtrr = mtrr_add(rdev->mc.aper_base, rdev->mc.aper_size,
>> -                       MTRR_TYPE_WRCOMB, 1);
>> +       rdev->mc.vram_mtrr = arch_phys_wc_add(rdev->mc.aper_base,
>> +                                             rdev->mc.aper_size);
>>         DRM_INFO("Detected VRAM RAM=%lluM, BAR=%lluM\n",
>>                 rdev->mc.mc_vram_size >> 20,
>>                 (unsigned long long)rdev->mc.aper_size >> 20);
>> @@ -334,6 +334,7 @@ int radeon_bo_init(struct radeon_device *rdev)
>>  void radeon_bo_fini(struct radeon_device *rdev)
>>  {
>>         radeon_ttm_fini(rdev);
>> +       arch_phys_wc_del(rdev->mc.vram_mtrr);
>>  }
>>
>>  void radeon_bo_list_add_object(struct radeon_bo_list *lobj,
>> --
>> 1.8.1.4
>>
Andy Lutomirski May 14, 2013, 9:35 p.m. UTC | #3 
On Tue, May 14, 2013 at 6:37 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Tue, May 14, 2013 at 8:58 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> I believe it will break something but we could deal with the fallout
> once it happens.

FWIW, I'm running with this code on my machine right now using the
radeon driver.  Everything seems fine.  If I build without MTRRs and
without PAT, though, graphics are slow (as expected).  So I think
everything's okay.

--Andy
Jerome Glisse May 15, 2013, 2:49 p.m. UTC | #4 
On Tue, May 14, 2013 at 5:35 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Tue, May 14, 2013 at 6:37 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
>> On Tue, May 14, 2013 at 8:58 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>>
>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> I believe it will break something but we could deal with the fallout
>> once it happens.
>
> FWIW, I'm running with this code on my machine right now using the
> radeon driver.  Everything seems fine.  If I build without MTRRs and
> without PAT, though, graphics are slow (as expected).  So I think
> everything's okay.
>
> --Andy

I am worried on p4 where i last see issue with that notably with agp.

Cheers,
Jerome
Andy Lutomirski May 15, 2013, 6:22 p.m. UTC | #5 
On Wed, May 15, 2013 at 7:49 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Tue, May 14, 2013 at 5:35 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Tue, May 14, 2013 at 6:37 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
>>> On Tue, May 14, 2013 at 8:58 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>> On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>>>
>>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>>
>>> I believe it will break something but we could deal with the fallout
>>> once it happens.
>>
>> FWIW, I'm running with this code on my machine right now using the
>> radeon driver.  Everything seems fine.  If I build without MTRRs and
>> without PAT, though, graphics are slow (as expected).  So I think
>> everything's okay.
>>
>> --Andy
>
> I am worried on p4 where i last see issue with that notably with agp.

Do you remember any details?  It looks like PAT is enabled on Pentium
4 (i.e. famliy 0xF).

--Andy
Jerome Glisse May 16, 2013, 1:50 p.m. UTC | #6 
On Wed, May 15, 2013 at 2:22 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, May 15, 2013 at 7:49 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
>> On Tue, May 14, 2013 at 5:35 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Tue, May 14, 2013 at 6:37 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
>>>> On Tue, May 14, 2013 at 8:58 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>>> On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>>>>
>>>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>>>
>>>> I believe it will break something but we could deal with the fallout
>>>> once it happens.
>>>
>>> FWIW, I'm running with this code on my machine right now using the
>>> radeon driver.  Everything seems fine.  If I build without MTRRs and
>>> without PAT, though, graphics are slow (as expected).  So I think
>>> everything's okay.
>>>
>>> --Andy
>>
>> I am worried on p4 where i last see issue with that notably with agp.
>
> Do you remember any details?  It looks like PAT is enabled on Pentium
> 4 (i.e. famliy 0xF).
>
> --Andy

No i don't, i think it was some pat errata on those about non real ram
address and with agp. Memory is fuzzy. I might have time in couple of
week to plug back my p4 and see how it behave.

Cheers,
Jerome
Andy Lutomirski May 16, 2013, 9 p.m. UTC | #7 
On Thu, May 16, 2013 at 6:50 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Wed, May 15, 2013 at 2:22 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Wed, May 15, 2013 at 7:49 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
>>> On Tue, May 14, 2013 at 5:35 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Tue, May 14, 2013 at 6:37 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
>>>>> On Tue, May 14, 2013 at 8:58 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>>>> On Mon, May 13, 2013 at 7:58 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>>>>>
>>>>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>>>>
>>>>> I believe it will break something but we could deal with the fallout
>>>>> once it happens.
>>>>
>>>> FWIW, I'm running with this code on my machine right now using the
>>>> radeon driver.  Everything seems fine.  If I build without MTRRs and
>>>> without PAT, though, graphics are slow (as expected).  So I think
>>>> everything's okay.
>>>>
>>>> --Andy
>>>
>>> I am worried on p4 where i last see issue with that notably with agp.
>>
>> Do you remember any details?  It looks like PAT is enabled on Pentium
>> 4 (i.e. famliy 0xF).
>>
>> --Andy
>
> No i don't, i think it was some pat errata on those about non real ram
> address and with agp. Memory is fuzzy. I might have time in couple of
> week to plug back my p4 and see how it behave.

Hmm.  I couldn't find any PAT errata related to AGP.

Is it possible that the issue was that, if there was no MTRR covering
the AGP aperture, that the mapping ended up as writeback?  If so, I
fixed that in this series.

In any case, if you see any problems, please let me know.

--Andy
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index d3aface..15cd34b 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -321,8 +321,8 @@  void radeon_bo_force_delete(struct radeon_device *rdev)
 int radeon_bo_init(struct radeon_device *rdev)
 {
 	/* Add an MTRR for the VRAM */
-	rdev->mc.vram_mtrr = mtrr_add(rdev->mc.aper_base, rdev->mc.aper_size,
-			MTRR_TYPE_WRCOMB, 1);
+	rdev->mc.vram_mtrr = arch_phys_wc_add(rdev->mc.aper_base,
+					      rdev->mc.aper_size);
 	DRM_INFO("Detected VRAM RAM=%lluM, BAR=%lluM\n",
 		rdev->mc.mc_vram_size >> 20,
 		(unsigned long long)rdev->mc.aper_size >> 20);
@@ -334,6 +334,7 @@  int radeon_bo_init(struct radeon_device *rdev)
 void radeon_bo_fini(struct radeon_device *rdev)
 {
 	radeon_ttm_fini(rdev);
+	arch_phys_wc_del(rdev->mc.vram_mtrr);
 }
 
 void radeon_bo_list_add_object(struct radeon_bo_list *lobj,