lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 26 Feb 2014 20:38:48 -0500
From:	Josh Boyer <jwboyer@...oraproject.org>
To:	Jani Nikula <jani.nikula@...el.com>
Cc:	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	David Airlie <airlied@...ux.ie>,
	intel-gfx@...ts.freedesktop.org,
	DRI mailing list <dri-devel@...ts.freedesktop.org>,
	"Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>
Subject: Re: 3.13 i915 brightness settings broken when going from docked -> undocked

On Tue, Feb 25, 2014 at 3:36 AM, Jani Nikula <jani.nikula@...el.com> wrote:
> On Sun, 23 Feb 2014, Josh Boyer <jwboyer@...oraproject.org> wrote:
>> Backport of upstream commit c91c9f328
>>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>>  drivers/gpu/drm/i915/intel_opregion.c | 31 ++++++-------------------------
>>  drivers/gpu/drm/i915/intel_panel.c    |  4 ++++
>>  3 files changed, 11 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 221ac62..d6d4349 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1371,6 +1371,7 @@ typedef struct drm_i915_private {
>>
>>       /* backlight */
>>       struct {
>> +             bool present;
>>               int level;
>>               bool enabled;
>>               spinlock_t lock; /* bl registers and the above bl fields */
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>> index 6d69a9b..b2a51ae 100644
>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> @@ -414,38 +414,19 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>>               return ASLC_BACKLIGHT_FAILED;
>>
>>       mutex_lock(&dev->mode_config.mutex);
>> -     /*
>> -      * Could match the OpRegion connector here instead, but we'd also need
>> -      * to verify the connector could handle a backlight call.
>> -      */
>> -     list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
>> -             if (encoder->crtc == crtc) {
>> -                     found = true;
>> -                     break;
>> -             }
>> -
>> -     if (!found) {
>> -             ret = ASLC_BACKLIGHT_FAILED;
>> -             goto out;
>> -     }
>>
>> -     list_for_each_entry(connector, &dev->mode_config.connector_list, head)
>> -             if (connector->encoder == encoder)
>> -                     intel_connector = to_intel_connector(connector);
>> -
>> -     if (!intel_connector) {
>> -             ret = ASLC_BACKLIGHT_FAILED;
>> -             goto out;
>> +     DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
>> +     list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>> +             intel_connector = to_intel_connector(connector);
>> +             if (dev_priv->backlight.present)
>> +                     intel_panel_set_backlight(intel_connector, bclp, 255);
>>       }
>
> This is not correct because in v3.13 dev_priv->backlight is still driver
> global, not per connector. This means that if you have at least one
> connector with backlight control, the backlight is attempted to change
> on all connectors. In your case, I'm not sure if it leads to anything
> more than extra adjusting of the same backlight.

Well, empirically it leads to the backlight actually changing after
undocking my machine whereas without it, it doesn't.  So maybe by
changing it globally, it is hitting the connector that does have
backlight control.

Anyway, I'm not arguing my patch is correct.  Just that it does do
_something_ to make the backlight work :).

> If you move the 'bool present' field to intel_connector or intel_panel,
> I think this is all right.

That sounds like more of an invasive change.  I could poke at it, but
I'm not sure it would be suitable for e.g. 3.13.y stable.  Thoughts on
that?  Admittedly it is a somewhat minor problem, so if it's not
easily stable material, I don't think anyone is going to lose sleep
over it.

josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists