| Message ID | 1346678712-30513-1-git-send-email-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, 3 Sep 2012 16:25:12 +0300, Jani Nikula <jani.nikula@intel.com> wrote: > Previously intel_panel_setup_backlight() would create a sysfs backlight > interface with max brightness of 1 if it was unable to figure out the max > backlight brightness. This rendered the backlight interface useless. > > Do not create a dysfunctional backlight interface to begin with. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > > See https://bugzilla.redhat.com/show_bug.cgi?id=752595 > > There's still a bug somewhere in the userspace as well; it should always > prefer the platform backlight interface over the raw one. This patch might > have the effect of hiding that particular userspace issue, but we shouldn't > be exporting a backlight interface that's useless no matter what. It was fixed. UXA uses a hardcoded set of preferred platform drivers, SNA looks at all the backlights under /sys/class/backlight and sorts them by type. We still have the issue of multi-panel or just multiple backlight interfaces and choosing the wrong one randomly. libbacklight's idea was to expose some more details through the kernel to be able to link device + connector to a particular backlight interface. However, userspace has the Option "Backlight" for fine tuning. -Chris
On Mon, Sep 03, 2012 at 06:51:31PM +0100, Chris Wilson wrote: > On Mon, 3 Sep 2012 16:25:12 +0300, Jani Nikula <jani.nikula@intel.com> wrote: > > Previously intel_panel_setup_backlight() would create a sysfs backlight > > interface with max brightness of 1 if it was unable to figure out the max > > backlight brightness. This rendered the backlight interface useless. > > > > Do not create a dysfunctional backlight interface to begin with. > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Picked up for -fixes, thanks for the patch. -Daniel
On Mon, 03 Sep 2012, "Woodhouse, David" <david.woodhouse@intel.com> wrote: > There was a Tested-by: David Woodhouse <David.Woodhouse@intel.com> in > there somewhere too, but the broken list config ate it. Thanks again for testing. Your Tested-by was not lost: http://cgit.freedesktop.org/~danvet/drm-intel/commit/?id=28dcc2d60cb570d9f549c329b2f51400553412a1 BR, Jani.
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 3df4f5f..e019b23 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -162,19 +162,12 @@ static u32 i915_read_blc_pwm_ctl(struct drm_i915_private *dev_priv) return val; } -u32 intel_panel_get_max_backlight(struct drm_device *dev) +static u32 _intel_panel_get_max_backlight(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; u32 max; max = i915_read_blc_pwm_ctl(dev_priv); - if (max == 0) { - /* XXX add code here to query mode clock or hardware clock - * and program max PWM appropriately. - */ - pr_warn_once("fixme: max PWM is zero\n"); - return 1; - } if (HAS_PCH_SPLIT(dev)) { max >>= 16; @@ -188,6 +181,22 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev) max *= 0xff; } + return max; +} + +u32 intel_panel_get_max_backlight(struct drm_device *dev) +{ + u32 max; + + max = _intel_panel_get_max_backlight(dev); + if (max == 0) { + /* XXX add code here to query mode clock or hardware clock + * and program max PWM appropriately. + */ + pr_warn_once("fixme: max PWM is zero\n"); + return 1; + } + DRM_DEBUG_DRIVER("max backlight PWM = %d\n", max); return max; } @@ -424,7 +433,11 @@ int intel_panel_setup_backlight(struct drm_device *dev) memset(&props, 0, sizeof(props)); props.type = BACKLIGHT_RAW; - props.max_brightness = intel_panel_get_max_backlight(dev); + props.max_brightness = _intel_panel_get_max_backlight(dev); + if (props.max_brightness == 0) { + DRM_ERROR("Failed to get maximum backlight value\n"); + return -ENODEV; + } dev_priv->backlight = backlight_device_register("intel_backlight", &connector->kdev, dev,
Previously intel_panel_setup_backlight() would create a sysfs backlight interface with max brightness of 1 if it was unable to figure out the max backlight brightness. This rendered the backlight interface useless. Do not create a dysfunctional backlight interface to begin with. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- See https://bugzilla.redhat.com/show_bug.cgi?id=752595 There's still a bug somewhere in the userspace as well; it should always prefer the platform backlight interface over the raw one. This patch might have the effect of hiding that particular userspace issue, but we shouldn't be exporting a backlight interface that's useless no matter what. --- drivers/gpu/drm/i915/intel_panel.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-)