Re: acpi suspend debugging techniques?

2015-09-03 Thread Adrian Chadd 
oioo, would you please put that radeon patch into a review?

I have an older machine with a radeon card in it that doesn't yet
suspend/resume; I can now test it out!


-a


On 3 September 2015 at 10:50, Andriy Gapon  wrote:
> On 31/08/2015 11:53, Adrian Chadd wrote:
>> Try disabling hardware one at a time. Ie, unload usb; unload wifi;
>> leave kms loaded for mostly obvious reasons.
>
> Adrian, Garrett,
>
> thank you very much for your tips.
> Turned out that it was radeonkms that was causing the problem :-)
>
> BTW, here is another tool for the toolkit: on sufficiently recent system 
> devctl
> suspend and devctl resume can be used to test individual drivers.
>
> So, I noticed that I could suspend/resume drmn0 device just fine but with
> vgapci0 I had a trouble suspending.  One thing led to another and here is a
> patch that seems to fix the problem for me:
> ---
> commit fecb5e8a90631f06600d87165cc8b6de3e035dfc
> Author: Andriy Gapon 
> Date:   Thu Sep 3 17:24:23 2015 +0300
>
> radeon_suspend_kms: don't mess with pci state that's managed by the bus
>
> The pci bus driver handles the power state and configuration state saving
> and restoring for its child devices.
>
> diff --git a/sys/dev/drm2/radeon/radeon_device.c
> b/sys/dev/drm2/radeon/radeon_device.c
> index e5c676b11ed47..73b2f4c51ada2 100644
> --- a/sys/dev/drm2/radeon/radeon_device.c
> +++ b/sys/dev/drm2/radeon/radeon_device.c
> @@ -1342,14 +1342,10 @@ int radeon_suspend_kms(struct drm_device *dev)
>
> radeon_agp_suspend(rdev);
>
> -   pci_save_state(device_get_parent(dev->dev));
>  #ifdef FREEBSD_WIP
> if (state.event == PM_EVENT_SUSPEND) {
> /* Shut down the device */
> pci_disable_device(dev->pdev);
> -#endif /* FREEBSD_WIP */
> -   pci_set_powerstate(dev->dev, PCI_POWERSTATE_D3);
> -#ifdef FREEBSD_WIP
> }
> console_lock();
>  #endif /* FREEBSD_WIP */
> @@ -1380,10 +1376,6 @@ int radeon_resume_kms(struct drm_device *dev)
>
>  #ifdef FREEBSD_WIP
> console_lock();
> -#endif /* FREEBSD_WIP */
> -   pci_set_powerstate(device_get_parent(dev->dev), PCI_POWERSTATE_D0);
> -   pci_restore_state(device_get_parent(dev->dev));
> -#ifdef FREEBSD_WIP
> if (pci_enable_device(dev->pdev)) {
> console_unlock();
> return -1;
> ---
>
> However, I am not sure about an exact mechanism of the hard system hang that I
> experienced without the patch.
>
> BTW, I noticed that only very few drivers make explicit calls to
> pci_set_powerstate and pci_save_state/pci_restore_state.
> sys/dev/usb/controller/ohci_pci.c looks like a good use of pci_set_powerstate.
> sys/dev/ixgbe/if_ix.c looks like an incorrect / redundant use of the 
> functions.
>
> --
> Andriy Gapon
___
freebsd-acpi@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"

Re: acpi suspend debugging techniques?

2015-09-03 Thread Jung-uk Kim 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 09/03/2015 13:50, Andriy Gapon wrote:
> On 31/08/2015 11:53, Adrian Chadd wrote:
>> Try disabling hardware one at a time. Ie, unload usb; unload
>> wifi; leave kms loaded for mostly obvious reasons.
> 
> Adrian, Garrett,
> 
> thank you very much for your tips. Turned out that it was radeonkms
> that was causing the problem :-)
> 
> BTW, here is another tool for the toolkit: on sufficiently recent
> system devctl suspend and devctl resume can be used to test
> individual drivers.
> 
> So, I noticed that I could suspend/resume drmn0 device just fine
> but with vgapci0 I had a trouble suspending.  One thing led to
> another and here is a patch that seems to fix the problem for me: 
> --
- -
>
> 
commit fecb5e8a90631f06600d87165cc8b6de3e035dfc
> Author: Andriy Gapon  Date:   Thu Sep 3 17:24:23
> 2015 +0300
> 
> radeon_suspend_kms: don't mess with pci state that's managed by the
> bus
> 
> The pci bus driver handles the power state and configuration state
> saving and restoring for its child devices.
> 
> diff --git a/sys/dev/drm2/radeon/radeon_device.c 
> b/sys/dev/drm2/radeon/radeon_device.c index
> e5c676b11ed47..73b2f4c51ada2 100644 ---
> a/sys/dev/drm2/radeon/radeon_device.c +++
> b/sys/dev/drm2/radeon/radeon_device.c @@ -1342,14 +1342,10 @@ int
> radeon_suspend_kms(struct drm_device *dev)
> 
> radeon_agp_suspend(rdev);
> 
> - pci_save_state(device_get_parent(dev->dev)); #ifdef FREEBSD_WIP 
> if (state.event == PM_EVENT_SUSPEND) { /* Shut down the device */ 
> pci_disable_device(dev->pdev); -#endif /* FREEBSD_WIP */ -
> pci_set_powerstate(dev->dev, PCI_POWERSTATE_D3); -#ifdef
> FREEBSD_WIP } console_lock(); #endif /* FREEBSD_WIP */ @@ -1380,10
> +1376,6 @@ int radeon_resume_kms(struct drm_device *dev)
> 
> #ifdef FREEBSD_WIP console_lock(); -#endif /* FREEBSD_WIP */ -
> pci_set_powerstate(device_get_parent(dev->dev),
> PCI_POWERSTATE_D0); -
> pci_restore_state(device_get_parent(dev->dev)); -#ifdef
> FREEBSD_WIP if (pci_enable_device(dev->pdev)) { console_unlock(); 
> return -1; 
> --
- -
>
>  However, I am not sure about an exact mechanism of the hard system
> hang that I experienced without the patch.
> 
> BTW, I noticed that only very few drivers make explicit calls to 
> pci_set_powerstate and pci_save_state/pci_restore_state. 
> sys/dev/usb/controller/ohci_pci.c looks like a good use of
> pci_set_powerstate. sys/dev/ixgbe/if_ix.c looks like an incorrect /
> redundant use of the functions.

AFAICT, the whole suspend/resume code looks incomplete and very messy.
 In fact, I'll be very surprised if it ever worked. :-(

Jung-uk Kim
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQEcBAEBCAAGBQJV6MdGAAoJEHyflib82/FGJ68H/2W6IfhDrtciL2LmA67T0VHU
Nagp1Oghp+JDw/iVB28Sxf/EXptsZKUeKvSYYilIFHsl/d/+uPvhbaxLVWUSyhGe
C5vVCbSkyRwnz3I5iiMab9Ou+VFTVqHhNLgrCFfDvjeHssbIkD7+bEuldWyqOIFH
rvvvZ8qGebVV7jcfU3lVVUz3tNwLwgdtVPuZIohxc8M7n1pE185hnUa1b37pytC9
btrCYLstGRNBbaD530iMN0bXM02aWgUTbf9cVH31nYduQaYOPYe/JgNKLzbmJ0gL
JIhGh4eubyR4W2SonRsg1ahHHzSr1pe1o7TVuU+2G1fycz4GSLoFWzYnSTxDMc4=
=IAfV
-END PGP SIGNATURE-
___
freebsd-acpi@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"

Re: acpi suspend debugging techniques?

2015-09-03 Thread David Wolfskill 
[Much trimming, both of older content and recipient addresses -- dhw]

On Thu, Sep 03, 2015 at 06:18:52PM -0400, Jung-uk Kim wrote:
> ...
> AFAICT, the whole suspend/resume code looks incomplete and very messy.
>  In fact, I'll be very surprised if it ever worked. :-(
> 
> Jung-uk Kim
> ...

While I bow to your expertise in the area, I point out that I
routinely suspend my laptop before putting it in my backpack, then
cycling between the shuttle stop and my house during my daily
commutes to & from work -- usually running stable/10, but I'm
sometimes running head at the time.

And I track both stable/10 and head daily on that laptop (in different
slices).

While I do encounter "issues" from time to time, those are rare enough
that I'd call them "unusual."  (To quantify that, I think I've had 3 - 5
such incidents since November 2014, while generally commuting 5
days/week.)

I'll be happy to test. :-)

Peace,
david
-- 
David H. Wolfskill  da...@catwhisker.org
Those who would murder in the name of God or prophet are blasphemous cowards.

See http://www.catwhisker.org/~david/publickey.gpg for my public key.


pgpa4SmpUTu8S.pgp
Description: PGP signature

Re: acpi suspend debugging techniques?

2015-09-03 Thread Jung-uk Kim 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 09/03/2015 18:30, David Wolfskill wrote:
> [Much trimming, both of older content and recipient addresses --
> dhw]
> 
> On Thu, Sep 03, 2015 at 06:18:52PM -0400, Jung-uk Kim wrote:
>> ... AFAICT, the whole suspend/resume code looks incomplete and
>> very messy. In fact, I'll be very surprised if it ever worked.
>> :-(
>> 
>> Jung-uk Kim ...
> 
> While I bow to your expertise in the area, I point out that I 
> routinely suspend my laptop before putting it in my backpack, then 
> cycling between the shuttle stop and my house during my daily 
> commutes to & from work -- usually running stable/10, but I'm 
> sometimes running head at the time.
> 
> And I track both stable/10 and head daily on that laptop (in
> different slices).
> 
> While I do encounter "issues" from time to time, those are rare
> enough that I'd call them "unusual."  (To quantify that, I think
> I've had 3 - 5 such incidents since November 2014, while generally
> commuting 5 days/week.)
> 
> I'll be happy to test. :-)

I am not saying the patch is wrong.  Actually, it is in the right
direction.  If you can, please test the following patch, too.

- --- sys/dev/drm2/radeon/radeon_drv.c  (revision 287437)
+++ sys/dev/drm2/radeon/radeon_drv.c(working copy)
@@ -327,14 +327,14 @@ radeon_suspend(device_t kdev)
struct drm_device *dev;
int ret;

+   ret = bus_generic_suspend(kdev);
+   if (ret)
+   return (ret);
+
dev = device_get_softc(kdev);
ret = radeon_suspend_kms(dev);
- - if (ret)
- - return (-ret);

- - ret = bus_generic_suspend(kdev);
- -
- - return (ret);
+   return (-ret);
 }

 static int


Jung-uk Kim
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQEcBAEBCAAGBQJV6M6cAAoJEHyflib82/FGX1UH/0BaQl+/e7Cqc2+3jdcmuusd
P8gJBGIFu89+6KsA2J1btQQYO2wwA9tJkpkZx4oi/pT+L+pIqZGx7/w7klsfXvXd
gfI0looWxzB5ZALCrzYq50Nk67E9s6iXymRMJ95oyZ2GLkbwLqY6gOStqld7vBuE
Z4iEBYHMrDtojd33w/9SRa8zNSpvwXZJliNjhpFd680ApkSO2xN/dIxI/z1JjlEw
oquRpvFlR4urqCdhYmKyjoXuR7rYdl0K2imfA7EjL2RFzlFyacS+ny4BqnbvMqzC
tMNxYFUOEvxMW+336DKZjiRWgAyfmJiOuoFxRoDiCq42zzjcLF+2gnLlcd4/j+4=
=/r95
-END PGP SIGNATURE-
___
freebsd-acpi@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"

Re: acpi suspend debugging techniques?

2015-09-03 Thread Andriy Gapon 
On 04/09/2015 01:18, Jung-uk Kim wrote:
> AFAICT, the whole suspend/resume code looks incomplete and very messy.
>  In fact, I'll be very surprised if it ever worked. :-(

It does seem to work for me with the patch and other people report that the code
works for them even without the patch.

-- 
Andriy Gapon
___
freebsd-acpi@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"

Re: acpi suspend debugging techniques?

2015-09-03 Thread Anthony Jenkins via freebsd-acpi 
My Radeon card comes back from resume, but the backlight doesn't (it
stays off).  Think this patch might help with that?  I was gonna start a
separate thread to ask for help debugging my backlight issue...

Thanks,
Anthony

On 09/03/15 16:06, Adrian Chadd wrote:
> oioo, would you please put that radeon patch into a review?
>
> I have an older machine with a radeon card in it that doesn't yet
> suspend/resume; I can now test it out!
>
>
> -a
>
>
> On 3 September 2015 at 10:50, Andriy Gapon  wrote:
>> On 31/08/2015 11:53, Adrian Chadd wrote:
>>> Try disabling hardware one at a time. Ie, unload usb; unload wifi;
>>> leave kms loaded for mostly obvious reasons.
>> Adrian, Garrett,
>>
>> thank you very much for your tips.
>> Turned out that it was radeonkms that was causing the problem :-)
>>
>> BTW, here is another tool for the toolkit: on sufficiently recent system 
>> devctl
>> suspend and devctl resume can be used to test individual drivers.
>>
>> So, I noticed that I could suspend/resume drmn0 device just fine but with
>> vgapci0 I had a trouble suspending.  One thing led to another and here is a
>> patch that seems to fix the problem for me:
>> ---
>> commit fecb5e8a90631f06600d87165cc8b6de3e035dfc
>> Author: Andriy Gapon 
>> Date:   Thu Sep 3 17:24:23 2015 +0300
>>
>> radeon_suspend_kms: don't mess with pci state that's managed by the bus
>>
>> The pci bus driver handles the power state and configuration state saving
>> and restoring for its child devices.
>>
>> diff --git a/sys/dev/drm2/radeon/radeon_device.c
>> b/sys/dev/drm2/radeon/radeon_device.c
>> index e5c676b11ed47..73b2f4c51ada2 100644
>> --- a/sys/dev/drm2/radeon/radeon_device.c
>> +++ b/sys/dev/drm2/radeon/radeon_device.c
>> @@ -1342,14 +1342,10 @@ int radeon_suspend_kms(struct drm_device *dev)
>>
>> radeon_agp_suspend(rdev);
>>
>> -   pci_save_state(device_get_parent(dev->dev));
>>  #ifdef FREEBSD_WIP
>> if (state.event == PM_EVENT_SUSPEND) {
>> /* Shut down the device */
>> pci_disable_device(dev->pdev);
>> -#endif /* FREEBSD_WIP */
>> -   pci_set_powerstate(dev->dev, PCI_POWERSTATE_D3);
>> -#ifdef FREEBSD_WIP
>> }
>> console_lock();
>>  #endif /* FREEBSD_WIP */
>> @@ -1380,10 +1376,6 @@ int radeon_resume_kms(struct drm_device *dev)
>>
>>  #ifdef FREEBSD_WIP
>> console_lock();
>> -#endif /* FREEBSD_WIP */
>> -   pci_set_powerstate(device_get_parent(dev->dev), PCI_POWERSTATE_D0);
>> -   pci_restore_state(device_get_parent(dev->dev));
>> -#ifdef FREEBSD_WIP
>> if (pci_enable_device(dev->pdev)) {
>> console_unlock();
>> return -1;
>> ---
>>
>> However, I am not sure about an exact mechanism of the hard system hang that 
>> I
>> experienced without the patch.
>>
>> BTW, I noticed that only very few drivers make explicit calls to
>> pci_set_powerstate and pci_save_state/pci_restore_state.
>> sys/dev/usb/controller/ohci_pci.c looks like a good use of 
>> pci_set_powerstate.
>> sys/dev/ixgbe/if_ix.c looks like an incorrect / redundant use of the 
>> functions.
>>
>> --
>> Andriy Gapon
> ___
> freebsd-acpi@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
> To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"

-- 
Anthony Jenkins
Software Engineer
VTilt Digital, LLC

___
freebsd-acpi@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"