diff mbox

radeon testing

Message ID 20120820193057.GA5176@growl (mailing list archive)
State New, archived
Headers show

Commit Message

Luca Tettamanti Aug. 20, 2012, 7:30 p.m. UTC
On Mon, Aug 20, 2012 at 10:24:01AM -0400, Alex Deucher wrote:
> > I just tested the rebased acpi_patches branch: ACPI part works fine, but
> > I see a big slow down during radeon driver initialization when the
> > screen goes black for a couple of seconds (with 3.5.0 + acpi patches the
> > screen just flickers during boot). With initcall_debug I see:
> >
> > [    2.355300] calling  radeon_init+0x0/0x1000 [radeon] @ 552
> > ...
> > [    5.530310] initcall radeon_init+0x0/0x1000 [radeon] returned 0 after 3102147 usecs
> >
> > For reference 3.5 takes ~1 sec. With drm.debug=2 the log (attached) is not very
> > informative, I'll try and get more info...
> 
> Can you bisect?

I've put in some printk, this is the result:

[    2.403138] evergreen_mc_program
[    2.403196] evergreen_mc_stop
...
[    4.268491] evergreen_mc_resume done
[    4.268548] evergreen_mc_program done

This is the patch:

	

Any printk between evergreen_mc_stop and evergreen_mc_resume locks up
the machine. The likely culprit is commit 023e188e:

commit 023e188ec102330eb05ad264f675e869637478eb
Author: Alex Deucher <alexander.deucher@amd.com>
Date:   Wed Aug 15 17:18:42 2012 -0400

    drm/radeon: properly handle mc_stop/mc_resume on evergreen+
    
    - Stop the displays from accessing the FB
    - Block CPU access
    - Turn off MC client access
    
    This should fix issues some users have seen, especially
    with UEFI, when changing the MC FB location that result
    in hangs or display corruption.

<i don't know what i'm talking about>

I haven't tried backing out the commit yet, but looking at the diff I
see that you call radeon_wait_for_vblank and radeon_get_vblank_counter,
but evergreen_mc_program is called way before IRQ is set up. Is the
vblank counter running? Looks like we just hitting the timeout here...

</i don't know what i'm talking about>


Luca

Comments

Alex Deucher Aug. 21, 2012, 1:51 p.m. UTC | #1
On Mon, Aug 20, 2012 at 3:30 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
> On Mon, Aug 20, 2012 at 10:24:01AM -0400, Alex Deucher wrote:
>> > I just tested the rebased acpi_patches branch: ACPI part works fine, but
>> > I see a big slow down during radeon driver initialization when the
>> > screen goes black for a couple of seconds (with 3.5.0 + acpi patches the
>> > screen just flickers during boot). With initcall_debug I see:
>> >
>> > [    2.355300] calling  radeon_init+0x0/0x1000 [radeon] @ 552
>> > ...
>> > [    5.530310] initcall radeon_init+0x0/0x1000 [radeon] returned 0 after 3102147 usecs
>> >
>> > For reference 3.5 takes ~1 sec. With drm.debug=2 the log (attached) is not very
>> > informative, I'll try and get more info...
>>
>> Can you bisect?
>
> I've put in some printk, this is the result:
>
> [    2.403138] evergreen_mc_program
> [    2.403196] evergreen_mc_stop
> ...
> [    4.268491] evergreen_mc_resume done
> [    4.268548] evergreen_mc_program done
>
> This is the patch:
>
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -1349,6 +1349,8 @@ void evergreen_mc_program(struct radeon_device *rdev)
>         u32 tmp;
>         int i, j;
>
> +       printk(KERN_INFO "%s\n", __func__);
> +
>         /* Initialize HDP */
>         for (i = 0, j = 0; i < 32; i++, j += 0x18) {
>                 WREG32((0x2c14 + j), 0x00000000);
> @@ -1359,10 +1361,14 @@ void evergreen_mc_program(struct radeon_device *rdev)
>         }
>         WREG32(HDP_REG_COHERENCY_FLUSH_CNTL, 0);
>
> +       printk(KERN_INFO "evergreen_mc_stop\n");
>         evergreen_mc_stop(rdev, &save);
> +//     printk(KERN_INFO "evergreen_mc_wait_for_idle\n");
>         if (evergreen_mc_wait_for_idle(rdev)) {
>                 dev_warn(rdev->dev, "Wait for MC idle timedout !\n");
>         }
> +//     printk(KERN_INFO "evergreen_mc_wait_for_idle done\n");
> +
>         /* Lockout access through VGA aperture*/
>         WREG32(VGA_HDP_CONTROL, VGA_MEMORY_DISABLE);
>         /* Update configuration */
> @@ -1411,10 +1417,13 @@ void evergreen_mc_program(struct radeon_device *rdev)
>                 WREG32(MC_VM_AGP_TOP, 0x0FFFFFFF);
>                 WREG32(MC_VM_AGP_BOT, 0x0FFFFFFF);
>         }
> +//     printk(KERN_INFO "evergreen_mc_wait_for_idle\n");
>         if (evergreen_mc_wait_for_idle(rdev)) {
>                 dev_warn(rdev->dev, "Wait for MC idle timedout !\n");
>         }
> +//     printk(KERN_INFO "evergreen_mc_resume\n");
>         evergreen_mc_resume(rdev, &save);
> +       printk(KERN_INFO "evergreen_mc_resume done\n");
>         /* we need to own VRAM, so turn off the VGA renderer here
>          * to stop it overwriting our objects */
>         rv515_vga_render_disable(rdev);
>
>
> Any printk between evergreen_mc_stop and evergreen_mc_resume locks up
> the machine. The likely culprit is commit 023e188e:

yeah, vram is locked out at that point.  I guess we probably need to
block anyone from trying to access it.

>
> commit 023e188ec102330eb05ad264f675e869637478eb
> Author: Alex Deucher <alexander.deucher@amd.com>
> Date:   Wed Aug 15 17:18:42 2012 -0400
>
>     drm/radeon: properly handle mc_stop/mc_resume on evergreen+
>
>     - Stop the displays from accessing the FB
>     - Block CPU access
>     - Turn off MC client access
>
>     This should fix issues some users have seen, especially
>     with UEFI, when changing the MC FB location that result
>     in hangs or display corruption.
>
> <i don't know what i'm talking about>
>
> I haven't tried backing out the commit yet, but looking at the diff I
> see that you call radeon_wait_for_vblank and radeon_get_vblank_counter,
> but evergreen_mc_program is called way before IRQ is set up. Is the
> vblank counter running? Looks like we just hitting the timeout here...
>

We aren't waiting for an interrupt, just polling the current crtc
status until it enters the vblank region.  The status and counters
should be working as we only wait on displays that are enabled.  I'll
double check the code however.  Thanks for testing.

Alex

> </i don't know what i'm talking about>
>
>
> Luca
Christian K├Ânig Aug. 21, 2012, 2:17 p.m. UTC | #2
On 21.08.2012 15:51, Alex Deucher wrote:
> On Mon, Aug 20, 2012 at 3:30 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>> On Mon, Aug 20, 2012 at 10:24:01AM -0400, Alex Deucher wrote:
>>>> I just tested the rebased acpi_patches branch: ACPI part works fine, but
>>>> I see a big slow down during radeon driver initialization when the
>>>> screen goes black for a couple of seconds (with 3.5.0 + acpi patches the
>>>> screen just flickers during boot). With initcall_debug I see:
>>>>
>>>> [    2.355300] calling  radeon_init+0x0/0x1000 [radeon] @ 552
>>>> ...
>>>> [    5.530310] initcall radeon_init+0x0/0x1000 [radeon] returned 0 after 3102147 usecs
>>>>
>>>> For reference 3.5 takes ~1 sec. With drm.debug=2 the log (attached) is not very
>>>> informative, I'll try and get more info...
>>> Can you bisect?
>> I've put in some printk, this is the result:
>>
>> [    2.403138] evergreen_mc_program
>> [    2.403196] evergreen_mc_stop
>> ...
>> [    4.268491] evergreen_mc_resume done
>> [    4.268548] evergreen_mc_program done
>>
>> This is the patch:
>>
>> --- a/drivers/gpu/drm/radeon/evergreen.c
>> +++ b/drivers/gpu/drm/radeon/evergreen.c
>> @@ -1349,6 +1349,8 @@ void evergreen_mc_program(struct radeon_device *rdev)
>>          u32 tmp;
>>          int i, j;
>>
>> +       printk(KERN_INFO "%s\n", __func__);
>> +
>>          /* Initialize HDP */
>>          for (i = 0, j = 0; i < 32; i++, j += 0x18) {
>>                  WREG32((0x2c14 + j), 0x00000000);
>> @@ -1359,10 +1361,14 @@ void evergreen_mc_program(struct radeon_device *rdev)
>>          }
>>          WREG32(HDP_REG_COHERENCY_FLUSH_CNTL, 0);
>>
>> +       printk(KERN_INFO "evergreen_mc_stop\n");
>>          evergreen_mc_stop(rdev, &save);
>> +//     printk(KERN_INFO "evergreen_mc_wait_for_idle\n");
>>          if (evergreen_mc_wait_for_idle(rdev)) {
>>                  dev_warn(rdev->dev, "Wait for MC idle timedout !\n");
>>          }
>> +//     printk(KERN_INFO "evergreen_mc_wait_for_idle done\n");
>> +
>>          /* Lockout access through VGA aperture*/
>>          WREG32(VGA_HDP_CONTROL, VGA_MEMORY_DISABLE);
>>          /* Update configuration */
>> @@ -1411,10 +1417,13 @@ void evergreen_mc_program(struct radeon_device *rdev)
>>                  WREG32(MC_VM_AGP_TOP, 0x0FFFFFFF);
>>                  WREG32(MC_VM_AGP_BOT, 0x0FFFFFFF);
>>          }
>> +//     printk(KERN_INFO "evergreen_mc_wait_for_idle\n");
>>          if (evergreen_mc_wait_for_idle(rdev)) {
>>                  dev_warn(rdev->dev, "Wait for MC idle timedout !\n");
>>          }
>> +//     printk(KERN_INFO "evergreen_mc_resume\n");
>>          evergreen_mc_resume(rdev, &save);
>> +       printk(KERN_INFO "evergreen_mc_resume done\n");
>>          /* we need to own VRAM, so turn off the VGA renderer here
>>           * to stop it overwriting our objects */
>>          rv515_vga_render_disable(rdev);
>>
>>
>> Any printk between evergreen_mc_stop and evergreen_mc_resume locks up
>> the machine. The likely culprit is commit 023e188e:
> yeah, vram is locked out at that point.  I guess we probably need to
> block anyone from trying to access it.
IIRC we have a rw-semaphore that you can writelock to prevent anything 
from accessing vram.

But if you try to access VRAM from within the critical section (by using 
a printk that tries to write something to the console) you probably end 
up deadlocking the kernel.

Christian.
>
>> commit 023e188ec102330eb05ad264f675e869637478eb
>> Author: Alex Deucher <alexander.deucher@amd.com>
>> Date:   Wed Aug 15 17:18:42 2012 -0400
>>
>>      drm/radeon: properly handle mc_stop/mc_resume on evergreen+
>>
>>      - Stop the displays from accessing the FB
>>      - Block CPU access
>>      - Turn off MC client access
>>
>>      This should fix issues some users have seen, especially
>>      with UEFI, when changing the MC FB location that result
>>      in hangs or display corruption.
>>
>> <i don't know what i'm talking about>
>>
>> I haven't tried backing out the commit yet, but looking at the diff I
>> see that you call radeon_wait_for_vblank and radeon_get_vblank_counter,
>> but evergreen_mc_program is called way before IRQ is set up. Is the
>> vblank counter running? Looks like we just hitting the timeout here...
>>
> We aren't waiting for an interrupt, just polling the current crtc
> status until it enters the vblank region.  The status and counters
> should be working as we only wait on displays that are enabled.  I'll
> double check the code however.  Thanks for testing.
>
> Alex
>
>> </i don't know what i'm talking about>
>>
>>
>> Luca
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
diff mbox

Patch

--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -1349,6 +1349,8 @@  void evergreen_mc_program(struct radeon_device *rdev)
 	u32 tmp;
 	int i, j;
 
+	printk(KERN_INFO "%s\n", __func__);
+
 	/* Initialize HDP */
 	for (i = 0, j = 0; i < 32; i++, j += 0x18) {
 		WREG32((0x2c14 + j), 0x00000000);
@@ -1359,10 +1361,14 @@  void evergreen_mc_program(struct radeon_device *rdev)
 	}
 	WREG32(HDP_REG_COHERENCY_FLUSH_CNTL, 0);
 
+	printk(KERN_INFO "evergreen_mc_stop\n");
 	evergreen_mc_stop(rdev, &save);
+//	printk(KERN_INFO "evergreen_mc_wait_for_idle\n");
 	if (evergreen_mc_wait_for_idle(rdev)) {
 		dev_warn(rdev->dev, "Wait for MC idle timedout !\n");
 	}
+//	printk(KERN_INFO "evergreen_mc_wait_for_idle done\n");
+
 	/* Lockout access through VGA aperture*/
 	WREG32(VGA_HDP_CONTROL, VGA_MEMORY_DISABLE);
 	/* Update configuration */
@@ -1411,10 +1417,13 @@  void evergreen_mc_program(struct radeon_device *rdev)
 		WREG32(MC_VM_AGP_TOP, 0x0FFFFFFF);
 		WREG32(MC_VM_AGP_BOT, 0x0FFFFFFF);
 	}
+//	printk(KERN_INFO "evergreen_mc_wait_for_idle\n");
 	if (evergreen_mc_wait_for_idle(rdev)) {
 		dev_warn(rdev->dev, "Wait for MC idle timedout !\n");
 	}
+//	printk(KERN_INFO "evergreen_mc_resume\n");
 	evergreen_mc_resume(rdev, &save);
+	printk(KERN_INFO "evergreen_mc_resume done\n");
 	/* we need to own VRAM, so turn off the VGA renderer here
 	 * to stop it overwriting our objects */
 	rv515_vga_render_disable(rdev);