Apparently UVD doesn't yet work everywhere - so allow it to be
disabled. Shaves off some reboot and suspend/resume time on machines
where it doesn't work. Might be useful for problematic chips in the
future as well.
Patch attached as well as inline below.
Signed-off-by: Parag Warudkar <[email protected]>
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 1442ce7..f131d8f 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -79,6 +79,7 @@
* Modules parameters.
*/
extern int radeon_no_wb;
+extern int radeon_no_uvd;
extern int radeon_modeset;
extern int radeon_dynclks;
extern int radeon_r4xx_atom;
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index d33f484..7e5b171 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -147,6 +147,7 @@ static inline void radeon_unregister_atpx_handler(void) {}
#endif
int radeon_no_wb;
+int radeon_no_uvd;
int radeon_modeset = 1;
int radeon_dynclks = -1;
int radeon_r4xx_atom = 0;
@@ -168,6 +169,9 @@ int radeon_fastfb = 0;
MODULE_PARM_DESC(no_wb, "Disable AGP writeback for scratch registers");
module_param_named(no_wb, radeon_no_wb, int, 0444);
+MODULE_PARM_DESC(no_uvd, "Disable UVD");
+module_param_named(no_uvd, radeon_no_uvd, int, 0444);
+
MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
module_param_named(modeset, radeon_modeset, int, 0400);
diff --git a/drivers/gpu/drm/radeon/radeon_drv.h
b/drivers/gpu/drm/radeon/radeon_drv.h
index b369d42..4320973 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.h
+++ b/drivers/gpu/drm/radeon/radeon_drv.h
@@ -329,6 +329,7 @@ typedef struct drm_radeon_kcmd_buffer {
} drm_radeon_kcmd_buffer_t;
extern int radeon_no_wb;
+extern int radeon_no_uvd;
extern struct drm_ioctl_desc radeon_ioctls[];
extern int radeon_max_ioctl;
diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c
b/drivers/gpu/drm/radeon/radeon_uvd.c
index 906e5c0..93a7dbb 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -58,7 +58,8 @@ int radeon_uvd_init(struct radeon_device *rdev)
unsigned long bo_size;
const char *fw_name;
int i, r;
-
+ if (radeon_no_uvd)
+ return -EINVAL;
INIT_DELAYED_WORK(&rdev->uvd.idle_work, radeon_uvd_idle_work_handler);
pdev = platform_device_register_simple("radeon_uvd", 0, NULL, 0);
On Mon, 2013-05-06 at 19:11 -0400, Parag Warudkar wrote:
> Apparently UVD doesn't yet work everywhere - so allow it to be
> disabled. Shaves off some reboot and suspend/resume time on machines
> where it doesn't work. Might be useful for problematic chips in the
> future as well.
>
> Patch attached as well as inline below.
>
> Signed-off-by: Parag Warudkar <[email protected]>
[...]
> @@ -168,6 +169,9 @@ int radeon_fastfb = 0;
> MODULE_PARM_DESC(no_wb, "Disable AGP writeback for scratch registers");
> module_param_named(no_wb, radeon_no_wb, int, 0444);
>
> +MODULE_PARM_DESC(no_uvd, "Disable UVD");
> +module_param_named(no_uvd, radeon_no_uvd, int, 0444);
No more negative boolean options please.
> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c
> b/drivers/gpu/drm/radeon/radeon_uvd.c
> index 906e5c0..93a7dbb 100644
> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
> @@ -58,7 +58,8 @@ int radeon_uvd_init(struct radeon_device *rdev)
> unsigned long bo_size;
> const char *fw_name;
> int i, r;
> -
> + if (radeon_no_uvd)
> + return -EINVAL;
> INIT_DELAYED_WORK(&rdev->uvd.idle_work, radeon_uvd_idle_work_handler);
Returning -EINVAL here results in rather misleading dmesg output I
think. This should probably be handled more gracefully.
Anyway, the return statement would need to be indented per the kernel
coding style, and please leave empty lines before the if and after the
return.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Debian, X and DRI developer
Am 07.05.2013 09:02, schrieb Michel Dänzer:
> On Mon, 2013-05-06 at 19:11 -0400, Parag Warudkar wrote:
>> Apparently UVD doesn't yet work everywhere - so allow it to be
>> disabled. Shaves off some reboot and suspend/resume time on machines
>> where it doesn't work. Might be useful for problematic chips in the
>> future as well.
>>
>> Patch attached as well as inline below.
>>
>> Signed-off-by: Parag Warudkar <[email protected]>
The patch shouldn't be necessary because just removing the firmware
should have pretty much the same effect.
On the other hand we only have three reports of failed VCPU bootup,
while it just seems to be a setup problems in two of those cases
(identical hardware seems to work for other people).
The only case where we indeed seems to have a problem are Macs with
integrated cards, and we can always just blacklist those if the problem
doesn't seems to be solvable.
Christian.
> [...]
>
>> @@ -168,6 +169,9 @@ int radeon_fastfb = 0;
>> MODULE_PARM_DESC(no_wb, "Disable AGP writeback for scratch registers");
>> module_param_named(no_wb, radeon_no_wb, int, 0444);
>>
>> +MODULE_PARM_DESC(no_uvd, "Disable UVD");
>> +module_param_named(no_uvd, radeon_no_uvd, int, 0444);
> No more negative boolean options please.
>
>
>> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c
>> b/drivers/gpu/drm/radeon/radeon_uvd.c
>> index 906e5c0..93a7dbb 100644
>> --- a/drivers/gpu/drm/radeon/radeon_uvd.c
>> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c
>> @@ -58,7 +58,8 @@ int radeon_uvd_init(struct radeon_device *rdev)
>> unsigned long bo_size;
>> const char *fw_name;
>> int i, r;
>> -
>> + if (radeon_no_uvd)
>> + return -EINVAL;
>> INIT_DELAYED_WORK(&rdev->uvd.idle_work, radeon_uvd_idle_work_handler);
> Returning -EINVAL here results in rather misleading dmesg output I
> think. This should probably be handled more gracefully.
>
> Anyway, the return statement would need to be indented per the kernel
> coding style, and please leave empty lines before the if and after the
> return.
>
>
On Tue, May 7, 2013 at 4:44 AM, Christian K?nig <[email protected]> wrote:
>
> The patch shouldn't be necessary because just removing the firmware should
> have pretty much the same effect.
Soon distros will ship the UVD firmware by default and then users will
need to manually remove it and then do the same with every update.
Besides, I just discovered that when UVD is enabled suspend resume
breaks - tried 3 times with SUMO_uvd loaded - machine suspends but
resumes instantly.
Without SUMO_uvd.bin - suspends fine and only wakes up when I want it to.
> The only case where we indeed seems to have a problem are Macs with
> integrated cards, and we can always just blacklist those if the problem
> doesn't seems to be solvable.
I happen to have the problematic card in my iMac - I'd be glad to
provide any info or try any patches. Just let me know.
For now I will remove the firmware - I reboot /suspend-resume often
and it is a bit annoying to have to go through those mdelays only to
fail.
Thanks,
Parag
Am 07.05.2013 23:13, schrieb Parag Warudkar:
> On Tue, May 7, 2013 at 4:44 AM, Christian K?nig <[email protected]> wrote:
>
>> The patch shouldn't be necessary because just removing the firmware should
>> have pretty much the same effect.
> Soon distros will ship the UVD firmware by default and then users will
> need to manually remove it and then do the same with every update.
> Besides, I just discovered that when UVD is enabled suspend resume
> breaks - tried 3 times with SUMO_uvd loaded - machine suspends but
> resumes instantly.
> Without SUMO_uvd.bin - suspends fine and only wakes up when I want it to.
Hui? Wait a second, the firmware doesn't work but still causes an
instant resume on suspend? Very strange.
>> The only case where we indeed seems to have a problem are Macs with
>> integrated cards, and we can always just blacklist those if the problem
>> doesn't seems to be solvable.
> I happen to have the problematic card in my iMac - I'd be glad to
> provide any info or try any patches. Just let me know.
> For now I will remove the firmware - I reboot /suspend-resume often
> and it is a bit annoying to have to go through those mdelays only to
> fail.
Yeah, perfectly understandable.
My best guess is that it has something todo with a different clock
routing on Macs, but without access to the hardware (or precise
documentation from Apple what the heck they did different) I don't
really see a chance to solve that problem.
If you want to hack a bit on it you could try commenting out the calls
to "radeon_set_uvd_clocks" in radeon_uvd.c. That should give you the
default clocks of 100Mhz, not enough for usable decoding, but on SUMO
based UVD blocks a very failsafe default.
Whatever it is, please send me an output of lspci, so I can blacklist
the offending chip.
Christian.
> Thanks,
> Parag
>
On Wed, 8 May 2013, Christian K?nig wrote:
> Am 07.05.2013 23:13, schrieb Parag Warudkar:
> > On Tue, May 7, 2013 at 4:44 AM, Christian K?nig <[email protected]>
> > wrote:
> > Without SUMO_uvd.bin - suspends fine and only wakes up when I want it to.
>
> Hui? Wait a second, the firmware doesn't work but still causes an instant
> resume on suspend? Very strange.
Yes, I tested this multiple times - if firmware loads and the init bails
out, machine resume instantly, like this -
[ 3631.441257] PM: Entering mem sleep
[ 3631.441274] Suspending console(s) (use no_console_suspend to debug)
[ 3631.441825] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[ 3631.442003] sd 0:0:0:0: [sda] Stopping disk
[ 3631.755076] PM: suspend of devices complete after 313.257 msecs
[ 3631.755219] PM: late suspend of devices complete after 0.134 msecs
[ 3631.795185] PM: noirq suspend of devices complete after 39.904 msecs
[ 3631.821922] pcieport 0000:00:1c.1: power state changed by ACPI to D0
[ 3631.915378] PM: noirq resume of devices complete after 119.999 msecs
[ 3631.915459] PM: early resume of devices complete after 0.062 msecs
[ 3631.915525] ahci 0000:00:1f.2: setting latency timer to 64
[ 3631.915609] ehci-pci 0000:00:1a.7: setting latency timer to 64
[ 3631.915654] uhci_hcd 0000:00:1a.0: setting latency timer to 64
[ 3631.915690] usb usb1: root hub lost power or was reset
[ 3631.915709] snd_hda_intel 0000:00:1b.0: irq 46 for MSI/MSI-X
[ 3631.915770] uhci_hcd 0000:00:1d.0: setting latency timer to 64
[ 3631.915792] usb usb2: root hub lost power or was reset
[ 3631.915804] ehci-pci 0000:00:1d.7: setting latency timer to 64
[ 3631.915821] snd_hda_intel 0000:01:00.1: irq 48 for MSI/MSI-X
[ 3631.918662] [drm] probing gen 2 caps for device 8086:101 = 2212d02/0
[ 3631.918665] [drm] PCIE gen 2 link speeds already enabled
[ 3631.921479] [drm] PCIE GART of 512M enabled (table at
0x0000000000040000).
[ 3631.921584] radeon 0000:01:00.0: WB enabled
Right now, I have removed SUMO_uvd.bin and suspend resume is working
without fail. Strange indeed, and I only noticed it when the machine did
not instant resume when running with the UVD disable patch and no_uvd=1
which skips uvd init just like firmware lodaing failure does I suppose.
>
> My best guess is that it has something todo with a different clock routing on
> Macs, but without access to the hardware (or precise documentation from Apple
> what the heck they did different) I don't really see a chance to solve that
> problem.
Hopefully it is not that hard and we'll find a way! I will experiment the
clocks and see how that goes.
> If you want to hack a bit on it you could try commenting out the calls to
> "radeon_set_uvd_clocks" in radeon_uvd.c. That should give you the default
> clocks of 100Mhz, not enough for usable decoding, but on SUMO based UVD blocks
> a very failsafe default.
>
> Whatever it is, please send me an output of lspci, so I can blacklist the
> offending chip.
>
Here is the lspci -nn output :
01:00.0 VGA compatible controller [0300]: Advanced Micro Devices [AMD] nee
ATI Whistler [Radeon HD 6600M/6700M/7600M Series] [1002:6741]
Parag
On Wed, 8 May 2013, Parag Warudkar wrote:
>
>
> On Wed, 8 May 2013, Christian K?nig wrote:
>
> > If you want to hack a bit on it you could try commenting out the calls to
> > "radeon_set_uvd_clocks" in radeon_uvd.c. That should give you the default
> > clocks of 100Mhz, not enough for usable decoding, but on SUMO based UVD blocks
> > a very failsafe default.
Commenting out the two calls to radeon_set_uvd_clocks() did not make any
difference - still fails to initialize. BTW, this is also an EFI install.
Sounds like for some people BIOS mode works based on the bug report.
Also confirming that the suspend/resume issue persists with the
SUMO_uvd.bin firmware loaded even if the UVD init fails. It is only by
removing the firmware I can get the machine to suspend reliably.
Parag