mbox series

[SRU,F/G/Unstable/OEM-5.6,0/3] Enable brightness control on HP DreamColor panel

Message ID 20201007115403.21938-1-kai.heng.feng@canonical.com
Headers show
Series Enable brightness control on HP DreamColor panel | expand

Message

Kai-Heng Feng Oct. 7, 2020, 11:53 a.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1898865

[Impact]
Brightness on HP DreamColor panel, which can be found on new HP Zbook
Studio, cannot be changed.

[Fix]
Let DRM know the panel should use DPCD instead of PWM to control
backlight.

[Test]
With the patch applied, we can change the brightness on HP Zbook Studio.

[Regression Potential]
If there's any panel in the quirk list depends on checking brightness
control capability then this change will have an impact. However it's
rather unlikely because it defeats the purpose of the quirk list.

Kai-Heng Feng (2):
  UBUNTU: SAUCE: drm/i915/dpcd_bl: Skip testing control capability with
    force DPCD quirk
  UBUNTU: SAUCE: drm/dp: HP DreamColor panel brigntness fix

Lyude Paul (1):
  drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam

 drivers/gpu/drm/drm_dp_helper.c                       | 1 +
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 9 ++++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

Comments

Stefan Bader Oct. 8, 2020, 9:07 a.m. UTC | #1
On 07.10.20 13:53, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/1898865
> 
> [Impact]
> Brightness on HP DreamColor panel, which can be found on new HP Zbook
> Studio, cannot be changed.
> 
> [Fix]
> Let DRM know the panel should use DPCD instead of PWM to control
> backlight.
> 
> [Test]
> With the patch applied, we can change the brightness on HP Zbook Studio.
> 
> [Regression Potential]
> If there's any panel in the quirk list depends on checking brightness
> control capability then this change will have an impact. However it's
> rather unlikely because it defeats the purpose of the quirk list.
> 
> Kai-Heng Feng (2):
>   UBUNTU: SAUCE: drm/i915/dpcd_bl: Skip testing control capability with
>     force DPCD quirk
>   UBUNTU: SAUCE: drm/dp: HP DreamColor panel brigntness fix
> 
> Lyude Paul (1):
>   drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
> 
>  drivers/gpu/drm/drm_dp_helper.c                       | 1 +
>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 9 ++++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
For Focal I am not really happy to ack because it seems to add a lot of new
SAUCE which could mean it is not accepted upstream or is rather new that there
was not much baking time. All things that do not really comply with the concept
of a stable release.

Though the first patch now is upstream:

commit d082119f4277ff4a63e44d293864aa9f2112b217
Author: Lyude Paul <lyude@redhat.com>
Date:   Mon Apr 13 17:44:06 2020 -0400
    drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam

So the following should be put before your sign-off when things get committed:

(cherry picked from commit d7fb38ae36a2dc97924b075ad1d1a88792777ea9)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

(cherry picked from commit d082119f4277ff4a63e44d293864aa9f2112b217)
Signed-off-by: ...

The other two patches, at least they appear harmless enough, are you planning on
upstreaming those?

For now, as this looks isolated enough...

Acked-by: Stefan Bader <stefan.bader@canonical.com>
Kai-Heng Feng Oct. 8, 2020, 9:15 a.m. UTC | #2
> On Oct 8, 2020, at 17:07, Stefan Bader <stefan.bader@canonical.com> wrote:
> 
> On 07.10.20 13:53, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1898865
>> 
>> [Impact]
>> Brightness on HP DreamColor panel, which can be found on new HP Zbook
>> Studio, cannot be changed.
>> 
>> [Fix]
>> Let DRM know the panel should use DPCD instead of PWM to control
>> backlight.
>> 
>> [Test]
>> With the patch applied, we can change the brightness on HP Zbook Studio.
>> 
>> [Regression Potential]
>> If there's any panel in the quirk list depends on checking brightness
>> control capability then this change will have an impact. However it's
>> rather unlikely because it defeats the purpose of the quirk list.
>> 
>> Kai-Heng Feng (2):
>>  UBUNTU: SAUCE: drm/i915/dpcd_bl: Skip testing control capability with
>>    force DPCD quirk
>>  UBUNTU: SAUCE: drm/dp: HP DreamColor panel brigntness fix
>> 
>> Lyude Paul (1):
>>  drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
>> 
>> drivers/gpu/drm/drm_dp_helper.c                       | 1 +
>> drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 9 ++++++---
>> 2 files changed, 7 insertions(+), 3 deletions(-)
>> 
> For Focal I am not really happy to ack because it seems to add a lot of new
> SAUCE which could mean it is not accepted upstream or is rather new that there
> was not much baking time. All things that do not really comply with the concept
> of a stable release.

I hope users who want to stick to GA kernel can get fixes too.

> 
> Though the first patch now is upstream:
> 
> commit d082119f4277ff4a63e44d293864aa9f2112b217
> Author: Lyude Paul <lyude@redhat.com>
> Date:   Mon Apr 13 17:44:06 2020 -0400
>    drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
> 
> So the following should be put before your sign-off when things get committed:
> 
> (cherry picked from commit d7fb38ae36a2dc97924b075ad1d1a88792777ea9)
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> (cherry picked from commit d082119f4277ff4a63e44d293864aa9f2112b217)
> Signed-off-by: ...
> 
> The other two patches, at least they appear harmless enough, are you planning on
> upstreaming those?

Yes, and it's under review. Will send another SRU if upstream version is different to this one.

Kai-Heng

> 
> For now, as this looks isolated enough...
> 
> Acked-by: Stefan Bader <stefan.bader@canonical.com>
>
Stefan Bader Oct. 8, 2020, 9:27 a.m. UTC | #3
On 08.10.20 11:15, Kai-Heng Feng wrote:
> 
> 
>> On Oct 8, 2020, at 17:07, Stefan Bader <stefan.bader@canonical.com> wrote:
>>
>> On 07.10.20 13:53, Kai-Heng Feng wrote:
>>> BugLink: https://bugs.launchpad.net/bugs/1898865
>>>
>>> [Impact]
>>> Brightness on HP DreamColor panel, which can be found on new HP Zbook
>>> Studio, cannot be changed.
>>>
>>> [Fix]
>>> Let DRM know the panel should use DPCD instead of PWM to control
>>> backlight.
>>>
>>> [Test]
>>> With the patch applied, we can change the brightness on HP Zbook Studio.
>>>
>>> [Regression Potential]
>>> If there's any panel in the quirk list depends on checking brightness
>>> control capability then this change will have an impact. However it's
>>> rather unlikely because it defeats the purpose of the quirk list.
>>>
>>> Kai-Heng Feng (2):
>>>  UBUNTU: SAUCE: drm/i915/dpcd_bl: Skip testing control capability with
>>>    force DPCD quirk
>>>  UBUNTU: SAUCE: drm/dp: HP DreamColor panel brigntness fix
>>>
>>> Lyude Paul (1):
>>>  drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
>>>
>>> drivers/gpu/drm/drm_dp_helper.c                       | 1 +
>>> drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 9 ++++++---
>>> 2 files changed, 7 insertions(+), 3 deletions(-)
>>>
>> For Focal I am not really happy to ack because it seems to add a lot of new
>> SAUCE which could mean it is not accepted upstream or is rather new that there
>> was not much baking time. All things that do not really comply with the concept
>> of a stable release.
> 
> I hope users who want to stick to GA kernel can get fixes too.

Yes, of course. This was not so much about the change itself but more about the
timing. I understand this causes more work but I think a staged approach (first
oem and devel and later stable) would be better. In this case it is probably too
simple to be a risk but as you say it might become a different approach upstream
and so we should avoid back and forth.

-Stefan

> 
>>
>> Though the first patch now is upstream:
>>
>> commit d082119f4277ff4a63e44d293864aa9f2112b217
>> Author: Lyude Paul <lyude@redhat.com>
>> Date:   Mon Apr 13 17:44:06 2020 -0400
>>    drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
>>
>> So the following should be put before your sign-off when things get committed:
>>
>> (cherry picked from commit d7fb38ae36a2dc97924b075ad1d1a88792777ea9)
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>
>> (cherry picked from commit d082119f4277ff4a63e44d293864aa9f2112b217)
>> Signed-off-by: ...
>>
>> The other two patches, at least they appear harmless enough, are you planning on
>> upstreaming those?
> 
> Yes, and it's under review. Will send another SRU if upstream version is different to this one.
> 
> Kai-Heng
> 
>>
>> For now, as this looks isolated enough...
>>
>> Acked-by: Stefan Bader <stefan.bader@canonical.com>
>>
>
Kleber Sacilotto de Souza Oct. 8, 2020, 11:21 a.m. UTC | #4
On 07.10.20 13:53, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/1898865
> 
> [Impact]
> Brightness on HP DreamColor panel, which can be found on new HP Zbook
> Studio, cannot be changed.
> 
> [Fix]
> Let DRM know the panel should use DPCD instead of PWM to control
> backlight.
> 
> [Test]
> With the patch applied, we can change the brightness on HP Zbook Studio.
> 
> [Regression Potential]
> If there's any panel in the quirk list depends on checking brightness
> control capability then this change will have an impact. However it's
> rather unlikely because it defeats the purpose of the quirk list.
> 
> Kai-Heng Feng (2):
>   UBUNTU: SAUCE: drm/i915/dpcd_bl: Skip testing control capability with
>     force DPCD quirk
>   UBUNTU: SAUCE: drm/dp: HP DreamColor panel brigntness fix
> 
> Lyude Paul (1):
>   drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
> 
>  drivers/gpu/drm/drm_dp_helper.c                       | 1 +
>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 9 ++++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 

With the (cherry picked from ...) line added as pointed out by
Stefan:

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Kai-Heng Feng Oct. 8, 2020, 1:30 p.m. UTC | #5
> On Oct 8, 2020, at 17:27, Stefan Bader <stefan.bader@canonical.com> wrote:
> 
> On 08.10.20 11:15, Kai-Heng Feng wrote:
>> 
>> 
>>> On Oct 8, 2020, at 17:07, Stefan Bader <stefan.bader@canonical.com> wrote:
>>> 
>>> On 07.10.20 13:53, Kai-Heng Feng wrote:
>>>> BugLink: https://bugs.launchpad.net/bugs/1898865
>>>> 
>>>> [Impact]
>>>> Brightness on HP DreamColor panel, which can be found on new HP Zbook
>>>> Studio, cannot be changed.
>>>> 
>>>> [Fix]
>>>> Let DRM know the panel should use DPCD instead of PWM to control
>>>> backlight.
>>>> 
>>>> [Test]
>>>> With the patch applied, we can change the brightness on HP Zbook Studio.
>>>> 
>>>> [Regression Potential]
>>>> If there's any panel in the quirk list depends on checking brightness
>>>> control capability then this change will have an impact. However it's
>>>> rather unlikely because it defeats the purpose of the quirk list.
>>>> 
>>>> Kai-Heng Feng (2):
>>>> UBUNTU: SAUCE: drm/i915/dpcd_bl: Skip testing control capability with
>>>>   force DPCD quirk
>>>> UBUNTU: SAUCE: drm/dp: HP DreamColor panel brigntness fix
>>>> 
>>>> Lyude Paul (1):
>>>> drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
>>>> 
>>>> drivers/gpu/drm/drm_dp_helper.c                       | 1 +
>>>> drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 9 ++++++---
>>>> 2 files changed, 7 insertions(+), 3 deletions(-)
>>>> 
>>> For Focal I am not really happy to ack because it seems to add a lot of new
>>> SAUCE which could mean it is not accepted upstream or is rather new that there
>>> was not much baking time. All things that do not really comply with the concept
>>> of a stable release.
>> 
>> I hope users who want to stick to GA kernel can get fixes too.
> 
> Yes, of course. This was not so much about the change itself but more about the
> timing. I understand this causes more work but I think a staged approach (first
> oem and devel and later stable) would be better. In this case it is probably too
> simple to be a risk but as you say it might become a different approach upstream
> and so we should avoid back and forth.

Ok, will see if I can manage to do that next time...

Kai-Heng

> 
> -Stefan
> 
>> 
>>> 
>>> Though the first patch now is upstream:
>>> 
>>> commit d082119f4277ff4a63e44d293864aa9f2112b217
>>> Author: Lyude Paul <lyude@redhat.com>
>>> Date:   Mon Apr 13 17:44:06 2020 -0400
>>>   drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
>>> 
>>> So the following should be put before your sign-off when things get committed:
>>> 
>>> (cherry picked from commit d7fb38ae36a2dc97924b075ad1d1a88792777ea9)
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> 
>>> (cherry picked from commit d082119f4277ff4a63e44d293864aa9f2112b217)
>>> Signed-off-by: ...
>>> 
>>> The other two patches, at least they appear harmless enough, are you planning on
>>> upstreaming those?
>> 
>> Yes, and it's under review. Will send another SRU if upstream version is different to this one.
>> 
>> Kai-Heng
>> 
>>> 
>>> For now, as this looks isolated enough...
>>> 
>>> Acked-by: Stefan Bader <stefan.bader@canonical.com>
Seth Forshee Oct. 8, 2020, 9:56 p.m. UTC | #6
On Wed, Oct 07, 2020 at 07:53:56PM +0800, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/1898865
> 
> [Impact]
> Brightness on HP DreamColor panel, which can be found on new HP Zbook
> Studio, cannot be changed.
> 
> [Fix]
> Let DRM know the panel should use DPCD instead of PWM to control
> backlight.
> 
> [Test]
> With the patch applied, we can change the brightness on HP Zbook Studio.
> 
> [Regression Potential]
> If there's any panel in the quirk list depends on checking brightness
> control capability then this change will have an impact. However it's
> rather unlikely because it defeats the purpose of the quirk list.

Applied to groovy/master-next and unstable/master, thanks!
Timo Aaltonen Oct. 16, 2020, 9:27 a.m. UTC | #7
On 7.10.2020 14.53, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/1898865
> 
> [Impact]
> Brightness on HP DreamColor panel, which can be found on new HP Zbook
> Studio, cannot be changed.
> 
> [Fix]
> Let DRM know the panel should use DPCD instead of PWM to control
> backlight.
> 
> [Test]
> With the patch applied, we can change the brightness on HP Zbook Studio.
> 
> [Regression Potential]
> If there's any panel in the quirk list depends on checking brightness
> control capability then this change will have an impact. However it's
> rather unlikely because it defeats the purpose of the quirk list.
> 
> Kai-Heng Feng (2):
>    UBUNTU: SAUCE: drm/i915/dpcd_bl: Skip testing control capability with
>      force DPCD quirk
>    UBUNTU: SAUCE: drm/dp: HP DreamColor panel brigntness fix
> 
> Lyude Paul (1):
>    drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
> 
>   drivers/gpu/drm/drm_dp_helper.c                       | 1 +
>   drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 9 ++++++---
>   2 files changed, 7 insertions(+), 3 deletions(-)
> 

applied to oem-5.6, thanks
Stefan Bader Oct. 23, 2020, 8:05 a.m. UTC | #8
On 07.10.20 13:53, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/1898865
> 
> [Impact]
> Brightness on HP DreamColor panel, which can be found on new HP Zbook
> Studio, cannot be changed.
> 
> [Fix]
> Let DRM know the panel should use DPCD instead of PWM to control
> backlight.
> 
> [Test]
> With the patch applied, we can change the brightness on HP Zbook Studio.
> 
> [Regression Potential]
> If there's any panel in the quirk list depends on checking brightness
> control capability then this change will have an impact. However it's
> rather unlikely because it defeats the purpose of the quirk list.
> 
> Kai-Heng Feng (2):
>   UBUNTU: SAUCE: drm/i915/dpcd_bl: Skip testing control capability with
>     force DPCD quirk
>   UBUNTU: SAUCE: drm/dp: HP DreamColor panel brigntness fix
> 
> Lyude Paul (1):
>   drm/i915/dpcd_bl: Unbreak enable_dpcd_backlight modparam
> 
>  drivers/gpu/drm/drm_dp_helper.c                       | 1 +
>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 9 ++++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
Applied to focal/master-next (with the added sha1. Thanks.

-Stefan