All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] Fix backlight control on Toshiba Satellite Z830
@ 2022-08-24 18:49 Arvid Norlander
  2022-08-24 18:49 ` [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk Arvid Norlander
  0 siblings, 1 reply; 13+ messages in thread
From: Arvid Norlander @ 2022-08-24 18:49 UTC (permalink / raw)
  To: linux-acpi; +Cc: Rafael J. Wysocki, Hans de Goede, Len Brown, Arvid Norlander

Hi,

First: I'm very new to this whole kernel development thing, so applogies in
advance for any mistakes.

I am submitting a patch that fixes backlight on Toshiba Satellite/Portege
Z830. This is a so called "ultrabook" from 2011. It needs the quirk
video_disable_backlight_sysfs_if. Without it backlight control breaks after
a suspend/resume cycle. However, control via intel_backlight still works.
If I disable acpi_video via acpi_backlight=none however, the backlight
never turns on at all after resume.

Thanks to Hans de Goede for suggesting this fix.

Reference:
* https://www.spinics.net/lists/platform-driver-x86/msg34394.html

Best regards,
Arvid Norlander

Arvid Norlander (1):
  ACPI: video: Add Toshiba Satellite/Portege Z830 quirk

 drivers/acpi/acpi_video.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)


base-commit: 1c23f9e627a7b412978b4e852793c5e3c3efc555
-- 
2.37.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-24 18:49 [PATCH 0/1] Fix backlight control on Toshiba Satellite Z830 Arvid Norlander
@ 2022-08-24 18:49 ` Arvid Norlander
  2022-08-24 21:14   ` Hans de Goede
  2022-08-26 11:46   ` Hans de Goede
  0 siblings, 2 replies; 13+ messages in thread
From: Arvid Norlander @ 2022-08-24 18:49 UTC (permalink / raw)
  To: linux-acpi; +Cc: Rafael J. Wysocki, Hans de Goede, Len Brown, Arvid Norlander

Toshiba Satellite Z830 needs the quirk video_disable_backlight_sysfs_if
for proper backlight control after suspend/resume cycles.

Toshiba Portege Z830 is simply the same laptop rebranded for certain
markets (I looked through the manual to other language sections to confirm
this) and thus also needs this quirk.

Thanks to Hans de Goede for suggesting this fix.

Link: https://www.spinics.net/lists/platform-driver-x86/msg34394.html
Signed-off-by: Arvid Norlander <lkml@vorpal.se>

---
 drivers/acpi/acpi_video.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 5cbe2196176d..2a4990733cf0 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -496,6 +496,22 @@ static const struct dmi_system_id video_dmi_table[] = {
 		DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE R830"),
 		},
 	},
+	{
+	 .callback = video_disable_backlight_sysfs_if,
+	 .ident = "Toshiba Satellite Z830",
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+		DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE Z830"),
+		},
+	},
+	{
+	 .callback = video_disable_backlight_sysfs_if,
+	 .ident = "Toshiba Portege Z830",
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+		DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE Z830"),
+		},
+	},
 	/*
 	 * Some machine's _DOD IDs don't have bit 31(Device ID Scheme) set
 	 * but the IDs actually follow the Device ID Scheme.
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-24 18:49 ` [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk Arvid Norlander
@ 2022-08-24 21:14   ` Hans de Goede
  2022-08-26 11:46   ` Hans de Goede
  1 sibling, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2022-08-24 21:14 UTC (permalink / raw)
  To: Arvid Norlander, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi,

On 8/24/22 20:49, Arvid Norlander wrote:
> Toshiba Satellite Z830 needs the quirk video_disable_backlight_sysfs_if
> for proper backlight control after suspend/resume cycles.
> 
> Toshiba Portege Z830 is simply the same laptop rebranded for certain
> markets (I looked through the manual to other language sections to confirm
> this) and thus also needs this quirk.
> 
> Thanks to Hans de Goede for suggesting this fix.
> 
> Link: https://www.spinics.net/lists/platform-driver-x86/msg34394.html
> Signed-off-by: Arvid Norlander <lkml@vorpal.se>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> 
> ---
>  drivers/acpi/acpi_video.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 5cbe2196176d..2a4990733cf0 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -496,6 +496,22 @@ static const struct dmi_system_id video_dmi_table[] = {
>  		DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE R830"),
>  		},
>  	},
> +	{
> +	 .callback = video_disable_backlight_sysfs_if,
> +	 .ident = "Toshiba Satellite Z830",
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE Z830"),
> +		},
> +	},
> +	{
> +	 .callback = video_disable_backlight_sysfs_if,
> +	 .ident = "Toshiba Portege Z830",
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE Z830"),
> +		},
> +	},
>  	/*
>  	 * Some machine's _DOD IDs don't have bit 31(Device ID Scheme) set
>  	 * but the IDs actually follow the Device ID Scheme.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-24 18:49 ` [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk Arvid Norlander
  2022-08-24 21:14   ` Hans de Goede
@ 2022-08-26 11:46   ` Hans de Goede
  2022-08-27 11:23     ` Arvid Norlander
  1 sibling, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2022-08-26 11:46 UTC (permalink / raw)
  To: Arvid Norlander, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi All,

On 8/24/22 20:49, Arvid Norlander wrote:
> Toshiba Satellite Z830 needs the quirk video_disable_backlight_sysfs_if
> for proper backlight control after suspend/resume cycles.
> 
> Toshiba Portege Z830 is simply the same laptop rebranded for certain
> markets (I looked through the manual to other language sections to confirm
> this) and thus also needs this quirk.
> 
> Thanks to Hans de Goede for suggesting this fix.
> 
> Link: https://www.spinics.net/lists/platform-driver-x86/msg34394.html
> Signed-off-by: Arvid Norlander <lkml@vorpal.se>

So I've been thinking a bit about this and this is going to
be a problem with my pending backlight refactor work.

*** Note this patch should still be merged as a fix for 6.0
and older ***

On these models acpi_video_get_backlight_type() returns
acpi_backlight_video, otherwise setting disable_backlight_sysfs_if
would not do anything.

After my "drm/kms: Stop registering multiple /sys/class/backlight
devs for a single display" series (1), the intel_backlight will no
longer gets registered, that now only happens when
acpi_video_get_backlight_type() returns acpi_backlight_native.

This will break the disable_backlight_sysfs_if workaround
because after this there then won't be any devices under
/sys/class/backlight at all.

I have been thinking about how to fix this, here are my notes
I have written on this.

-toshiba r830 / z830 problem after series:
 -duplicate DMI match in video_detect, force native
 -make disable_backlight_sysfs_if still work even when
  acpi_video_register_backlight() does not get called, just do
  the backlight_init directly from acpi_video_register()
  when disable_backlight_sysfs_if is set, as before
  my refactor series

This will work, but it makes the code (even more) ugly /
convoluted.

Arvid, I wonder if instead of using disable_backlight_sysfs_if
you can try:

0. Remove disable_backlight_sysfs_if from cmdline / quirk
1. Adding acpi_backlight=native to the kernel commandline
2. In toshiba_acpi_resume() add a HCI_PANEL_POWER_ON PANEL_ON

and see if that also fixes things ?

If that is the case then we can:

1. Move the DMI quirks for disable_backlight_sysfs_if
   from acpi_video.c to video_detect.c to force native
   mode by quirk
2. Add the DMI table with the models needing this to
   toshiba_acpi.c and then based on that call
   HCI_PANEL_POWER_ON PANEL_ON on resume from there
3. Since there are no more quirks using it, remove the
   disable_backlight_sysfs_if hack / workaround from
   acpi_video.c

This will give a nice-cleanup of the generic acpi_video.c
code moving the toshiba specific fixup to toshiba_acpi
where it really belongs.

Regards,

Hans



1) https://lore.kernel.org/platform-driver-x86/20220825143726.269890-1-hdegoede@redhat.com/T/


> ---
>  drivers/acpi/acpi_video.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 5cbe2196176d..2a4990733cf0 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -496,6 +496,22 @@ static const struct dmi_system_id video_dmi_table[] = {
>  		DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE R830"),
>  		},
>  	},
> +	{
> +	 .callback = video_disable_backlight_sysfs_if,
> +	 .ident = "Toshiba Satellite Z830",
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE Z830"),
> +		},
> +	},
> +	{
> +	 .callback = video_disable_backlight_sysfs_if,
> +	 .ident = "Toshiba Portege Z830",
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE Z830"),
> +		},
> +	},
>  	/*
>  	 * Some machine's _DOD IDs don't have bit 31(Device ID Scheme) set
>  	 * but the IDs actually follow the Device ID Scheme.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-26 11:46   ` Hans de Goede
@ 2022-08-27 11:23     ` Arvid Norlander
  2022-08-27 13:49       ` Hans de Goede
  0 siblings, 1 reply; 13+ messages in thread
From: Arvid Norlander @ 2022-08-27 11:23 UTC (permalink / raw)
  To: Hans de Goede, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi,

On 2022-08-26 13:46, Hans de Goede wrote:
> Hi All,
> 
> [...]
> 
> Arvid, I wonder if instead of using disable_backlight_sysfs_if
> you can try:
> 
> 0. Remove disable_backlight_sysfs_if from cmdline / quirk
> 1. Adding acpi_backlight=native to the kernel commandline
> 2. In toshiba_acpi_resume() add a HCI_PANEL_POWER_ON PANEL_ON
> 
> and see if that also fixes things ?
> 
Yes, this works. I do not have a patch for this (I assume it
would involve creating quirk tables, checking for support for
HCI_PANEL_POWER_ON, etc). I simply hard coded the call in for
the test. I very much doubt I will have time to code this in
the near future as well.

However, do we know what the other Toshiba's that need this
quirk also supports HCI_PANEL_POWER_ON? I obviously can only
test the Z830 that I own.

> If that is the case then we can:
> 
> 1. Move the DMI quirks for disable_backlight_sysfs_if
>    from acpi_video.c to video_detect.c to force native
>    mode by quirk
> 2. Add the DMI table with the models needing this to
>    toshiba_acpi.c and then based on that call
>    HCI_PANEL_POWER_ON PANEL_ON on resume from there
> 3. Since there are no more quirks using it, remove the
>    disable_backlight_sysfs_if hack / workaround from
>    acpi_video.c
> 
> This will give a nice-cleanup of the generic acpi_video.c
> code moving the toshiba specific fixup to toshiba_acpi
> where it really belongs.
> 
> Regards,
> 
> Hans
> 
> 

Best regards,
Arvid Norlander


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-27 11:23     ` Arvid Norlander
@ 2022-08-27 13:49       ` Hans de Goede
  2022-08-29 14:12         ` Hans de Goede
  0 siblings, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2022-08-27 13:49 UTC (permalink / raw)
  To: Arvid Norlander, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

hI,

On 8/27/22 13:23, Arvid Norlander wrote:
> Hi,
> 
> On 2022-08-26 13:46, Hans de Goede wrote:
>> Hi All,
>>
>> [...]
>>
>> Arvid, I wonder if instead of using disable_backlight_sysfs_if
>> you can try:
>>
>> 0. Remove disable_backlight_sysfs_if from cmdline / quirk
>> 1. Adding acpi_backlight=native to the kernel commandline
>> 2. In toshiba_acpi_resume() add a HCI_PANEL_POWER_ON PANEL_ON
>>
>> and see if that also fixes things ?
>>
> Yes, this works.

Great, thank you for testing this!

In hindsight the disable_backlight_sysfs_if flag was a mistake
and I should have fixed this differently (I wrote the code adding
that flag).  And now it is sorta getting in the way of cleaning
up the backlight handling. So IMHO removing disable_backlight_sysfs_if
is the best thing to do here.

> I do not have a patch for this (I assume it
> would involve creating quirk tables, checking for support for
> HCI_PANEL_POWER_ON, etc). I simply hard coded the call in for
> the test. I very much doubt I will have time to code this in
> the near future as well.

No problem I will prepare a patch series for you to test. Note
this will be on top of my other backlight cleanups, so I
will just send you an email pointing to a git branch to tes,
I hope this will be ok?

> However, do we know what the other Toshiba's that need this
> quirk also supports HCI_PANEL_POWER_ON? I obviously can only
> test the Z830 that I own.

It seems that all models which need this are all from the same
generation so I would expect the same fix to work. If I get
regression reports from users after my cleanup series lands
I can then take a closer look at the DSDT tables of the
other models if necessary.

Regards,

Hans




> 
>> If that is the case then we can:
>>
>> 1. Move the DMI quirks for disable_backlight_sysfs_if
>>    from acpi_video.c to video_detect.c to force native
>>    mode by quirk
>> 2. Add the DMI table with the models needing this to
>>    toshiba_acpi.c and then based on that call
>>    HCI_PANEL_POWER_ON PANEL_ON on resume from there
>> 3. Since there are no more quirks using it, remove the
>>    disable_backlight_sysfs_if hack / workaround from
>>    acpi_video.c
>>
>> This will give a nice-cleanup of the generic acpi_video.c
>> code moving the toshiba specific fixup to toshiba_acpi
>> where it really belongs.
>>
>> Regards,
>>
>> Hans
>>
>>
> 
> Best regards,
> Arvid Norlander
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-27 13:49       ` Hans de Goede
@ 2022-08-29 14:12         ` Hans de Goede
  2022-08-29 18:30           ` Arvid Norlander
  0 siblings, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2022-08-29 14:12 UTC (permalink / raw)
  To: Arvid Norlander, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi,

On 8/27/22 15:49, Hans de Goede wrote:
> hI,
> 
> On 8/27/22 13:23, Arvid Norlander wrote:
>> Hi,
>>
>> On 2022-08-26 13:46, Hans de Goede wrote:
>>> Hi All,
>>>
>>> [...]
>>>
>>> Arvid, I wonder if instead of using disable_backlight_sysfs_if
>>> you can try:
>>>
>>> 0. Remove disable_backlight_sysfs_if from cmdline / quirk
>>> 1. Adding acpi_backlight=native to the kernel commandline
>>> 2. In toshiba_acpi_resume() add a HCI_PANEL_POWER_ON PANEL_ON
>>>
>>> and see if that also fixes things ?
>>>
>> Yes, this works.
> 
> Great, thank you for testing this!
> 
> In hindsight the disable_backlight_sysfs_if flag was a mistake
> and I should have fixed this differently (I wrote the code adding
> that flag).  And now it is sorta getting in the way of cleaning
> up the backlight handling. So IMHO removing disable_backlight_sysfs_if
> is the best thing to do here.
> 
>> I do not have a patch for this (I assume it
>> would involve creating quirk tables, checking for support for
>> HCI_PANEL_POWER_ON, etc). I simply hard coded the call in for
>> the test. I very much doubt I will have time to code this in
>> the near future as well.
> 
> No problem I will prepare a patch series for you to test. Note
> this will be on top of my other backlight cleanups, so I
> will just send you an email pointing to a git branch to test,
> I hope this will be ok?

Arvid, here is a git branch with my backlight-refactor for you
to test:

https://github.com/jwrdegoede/linux-sunxi/commits/backlight-refactor-for-arvid

If you can give this a test spin (without any special kernel
commandline options) then that would be great.

>> However, do we know what the other Toshiba's that need this
>> quirk also supports HCI_PANEL_POWER_ON? I obviously can only
>> test the Z830 that I own.
> 
> It seems that all models which need this are all from the same
> generation so I would expect the same fix to work. If I get
> regression reports from users after my cleanup series lands
> I can then take a closer look at the DSDT tables of the
> other models if necessary.

Quick update on this I've taken a look at the DSTD's _BCM method
which is the magic call done by acpi_video on resume which
turns the panel back on and in both a R700 as well as a R830
it ends up doing this:

SMBR (0xFF00, 0x2A, Local0, 0x00, 0xB2)

Which translates to:

HCI_SET HCI_BRIGHTNESS <brightness-level>

Note that in this case brightness-level used by the acpi_video
code is the initial brightness at power-on since acpi_video
is not used to actually control the brightness.

Which makes using HCI_PANEL_POWER_ON a better idea since
that does not touches the brightness.

So I believe that replacing disable_backlight_sysfs_if with
a HCI call by toshiba_acpi on resume should work on all
affected models.

Regards,

Hans


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-29 14:12         ` Hans de Goede
@ 2022-08-29 18:30           ` Arvid Norlander
  2022-08-29 18:58             ` Hans de Goede
  0 siblings, 1 reply; 13+ messages in thread
From: Arvid Norlander @ 2022-08-29 18:30 UTC (permalink / raw)
  To: Hans de Goede, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi,

On 2022-08-29 16:12, Hans de Goede wrote:
> Hi,
> 

<snip>

> 
> Arvid, here is a git branch with my backlight-refactor for you
> to test:
> 
> https://github.com/jwrdegoede/linux-sunxi/commits/backlight-refactor-for-arvid
> 
> If you can give this a test spin (without any special kernel
> commandline options) then that would be great.

I'll set up a PKGBUILD and get this built (I'm building on other computers).
It may take a couple of days before I get around to that however. I hope
this is okay with you.

<snip>

> Regards,
> 
> Hans
> 

Best regards,
Arvid Norlander

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-29 18:30           ` Arvid Norlander
@ 2022-08-29 18:58             ` Hans de Goede
  2022-08-31 13:44               ` Arvid Norlander
  0 siblings, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2022-08-29 18:58 UTC (permalink / raw)
  To: Arvid Norlander, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi,

On 8/29/22 20:30, Arvid Norlander wrote:
> Hi,
> 
> On 2022-08-29 16:12, Hans de Goede wrote:
>> Hi,
>>
> 
> <snip>
> 
>>
>> Arvid, here is a git branch with my backlight-refactor for you
>> to test:
>>
>> https://github.com/jwrdegoede/linux-sunxi/commits/backlight-refactor-for-arvid
>>
>> If you can give this a test spin (without any special kernel
>> commandline options) then that would be great.
> 
> I'll set up a PKGBUILD and get this built (I'm building on other computers).
> It may take a couple of days before I get around to that however. I hope
> this is okay with you.

Yes that is fine, thank you.

Regards,

Hans


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-29 18:58             ` Hans de Goede
@ 2022-08-31 13:44               ` Arvid Norlander
  2022-09-01 10:42                 ` Hans de Goede
  0 siblings, 1 reply; 13+ messages in thread
From: Arvid Norlander @ 2022-08-31 13:44 UTC (permalink / raw)
  To: Hans de Goede, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi,


On 2022-08-29 20:58, Hans de Goede wrote:
> Hi,
> 
> On 8/29/22 20:30, Arvid Norlander wrote:
>> Hi,
>>
>> On 2022-08-29 16:12, Hans de Goede wrote:
>>> Hi,
>>>
>>
>> <snip>
>>
>>>
>>> Arvid, here is a git branch with my backlight-refactor for you
>>> to test:
>>>
>>> https://github.com/jwrdegoede/linux-sunxi/commits/backlight-refactor-for-arvid
>>>
>>> If you can give this a test spin (without any special kernel
>>> commandline options) then that would be great.
>>
>> I'll set up a PKGBUILD and get this built (I'm building on other computers).
>> It may take a couple of days before I get around to that however. I hope
>> this is okay with you.
> 
> Yes that is fine, thank you.

Just and update to let you know that your tree works, at least for suspend.

I'm not set up to test hibernation (using swap file on btrfs). Nor do I
know if it even works on this laptop. It has some sort of auto hibernate
feature in BIOS called Intel Rapid Start. It supposedly auto transitions to
hibernation after being asleep for a while. I have not looked into if this
is supported on Linux, and what setup would be required to support it in
that case.

> 
> Regards,
> 
> Hans
> 

Best regards,
Arvid Norlander

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-08-31 13:44               ` Arvid Norlander
@ 2022-09-01 10:42                 ` Hans de Goede
  2022-09-01 14:15                   ` Arvid Norlander
  0 siblings, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2022-09-01 10:42 UTC (permalink / raw)
  To: Arvid Norlander, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi,

On 8/31/22 15:44, Arvid Norlander wrote:
> Hi,
> 
> 
> On 2022-08-29 20:58, Hans de Goede wrote:
>> Hi,
>>
>> On 8/29/22 20:30, Arvid Norlander wrote:
>>> Hi,
>>>
>>> On 2022-08-29 16:12, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>
>>> <snip>
>>>
>>>>
>>>> Arvid, here is a git branch with my backlight-refactor for you
>>>> to test:
>>>>
>>>> https://github.com/jwrdegoede/linux-sunxi/commits/backlight-refactor-for-arvid
>>>>
>>>> If you can give this a test spin (without any special kernel
>>>> commandline options) then that would be great.
>>>
>>> I'll set up a PKGBUILD and get this built (I'm building on other computers).
>>> It may take a couple of days before I get around to that however. I hope
>>> this is okay with you.
>>
>> Yes that is fine, thank you.
> 
> Just and update to let you know that your tree works, at least for suspend.

Great, thank you so much for testing this!

Is it ok if I add a:

Tested-by: Arvid Norlander <lkml@vorpal.se>

to the 2 patches for fixing this to give you credit for your testing ?

> I'm not set up to test hibernation (using swap file on btrfs). Nor do I
> know if it even works on this laptop. It has some sort of auto hibernate
> feature in BIOS called Intel Rapid Start. It supposedly auto transitions to
> hibernation after being asleep for a while. I have not looked into if this
> is supported on Linux, and what setup would be required to support it in
> that case.

Regular suspend/resume testing is what I was looking for. On restore
from hibernation the backlight is already on when restoring the state so
I don't expect any problems there.  And as you indicate getting hibernation
to work is tricky in general, IMHO there is no need to go through all
the trouble necessary to (maybe) get that to work.

Regards,

Hans


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-09-01 10:42                 ` Hans de Goede
@ 2022-09-01 14:15                   ` Arvid Norlander
  2022-09-01 15:03                     ` Hans de Goede
  0 siblings, 1 reply; 13+ messages in thread
From: Arvid Norlander @ 2022-09-01 14:15 UTC (permalink / raw)
  To: Hans de Goede, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi,

On 2022-09-01 12:42, Hans de Goede wrote:
> Hi,
> 

<snip>

> 
> Great, thank you so much for testing this!
> 
> Is it ok if I add a:
> 
> Tested-by: Arvid Norlander <lkml@vorpal.se>
> 
> to the 2 patches for fixing this to give you credit for your testing ?

Yes, of course. As a newcommer it is hard to learn and remember all these
little rules you have in kernel development. It is quite different from the
forge-based workflow I'm used to.

> 
>> I'm not set up to test hibernation (using swap file on btrfs). Nor do I
>> know if it even works on this laptop. It has some sort of auto hibernate
>> feature in BIOS called Intel Rapid Start. It supposedly auto transitions to
>> hibernation after being asleep for a while. I have not looked into if this
>> is supported on Linux, and what setup would be required to support it in
>> that case.
> 
> Regular suspend/resume testing is what I was looking for. On restore
> from hibernation the backlight is already on when restoring the state so
> I don't expect any problems there.  And as you indicate getting hibernation
> to work is tricky in general, IMHO there is no need to go through all
> the trouble necessary to (maybe) get that to work.

Hm, I have not found it tricky when I have had swap partitions. At least on
my Thinkpads as long as I avoid nvidia drivers.

> 
> Regards,
> 
> Hans
> 

Best regards,
Arvid Norlander

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk
  2022-09-01 14:15                   ` Arvid Norlander
@ 2022-09-01 15:03                     ` Hans de Goede
  0 siblings, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2022-09-01 15:03 UTC (permalink / raw)
  To: Arvid Norlander, linux-acpi; +Cc: Rafael J. Wysocki, Len Brown

Hi,

On 9/1/22 16:15, Arvid Norlander wrote:
> Hi,
> 
> On 2022-09-01 12:42, Hans de Goede wrote:
>> Hi,
>>
> 
> <snip>
> 
>>
>> Great, thank you so much for testing this!
>>
>> Is it ok if I add a:
>>
>> Tested-by: Arvid Norlander <lkml@vorpal.se>
>>
>> to the 2 patches for fixing this to give you credit for your testing ?
> 
> Yes, of course. As a newcommer it is hard to learn and remember all these
> little rules you have in kernel development. It is quite different from the
> forge-based workflow I'm used to.

No worries, I think you are doing great. Actually I could just have
added your Tested-by tags if I wanted too. Only the Signed-off-by tag
is one which you must explicitly give.

But asking seemed like the polite thing to do :)

>>> I'm not set up to test hibernation (using swap file on btrfs). Nor do I
>>> know if it even works on this laptop. It has some sort of auto hibernate
>>> feature in BIOS called Intel Rapid Start. It supposedly auto transitions to
>>> hibernation after being asleep for a while. I have not looked into if this
>>> is supported on Linux, and what setup would be required to support it in
>>> that case.
>>
>> Regular suspend/resume testing is what I was looking for. On restore
>> from hibernation the backlight is already on when restoring the state so
>> I don't expect any problems there.  And as you indicate getting hibernation
>> to work is tricky in general, IMHO there is no need to go through all
>> the trouble necessary to (maybe) get that to work.
> 
> Hm, I have not found it tricky when I have had swap partitions. At least on
> my Thinkpads as long as I avoid nvidia drivers.

As long as you have large enough swap partitions, yes. Sometimes figuring out
what large enough is is tricky though. Anyway we are getting offtopic.

Regards,

Hans


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-09-01 15:03 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-24 18:49 [PATCH 0/1] Fix backlight control on Toshiba Satellite Z830 Arvid Norlander
2022-08-24 18:49 ` [PATCH 1/1] ACPI: video: Add Toshiba Satellite/Portege Z830 quirk Arvid Norlander
2022-08-24 21:14   ` Hans de Goede
2022-08-26 11:46   ` Hans de Goede
2022-08-27 11:23     ` Arvid Norlander
2022-08-27 13:49       ` Hans de Goede
2022-08-29 14:12         ` Hans de Goede
2022-08-29 18:30           ` Arvid Norlander
2022-08-29 18:58             ` Hans de Goede
2022-08-31 13:44               ` Arvid Norlander
2022-09-01 10:42                 ` Hans de Goede
2022-09-01 14:15                   ` Arvid Norlander
2022-09-01 15:03                     ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.