At Fri, 18 Nov 2011 11:25:48 -0800,
Keith Packard wrote:
>
> On Thu, 17 Nov 2011 17:33:50 +0100, Takashi Iwai <tiwai at suse.de> wrote:
>
> > If it's only with 915GM, we'll just need to change IS_PINEVEW() to
> > IS_PINEVIEW() || IS_I915GM(). This might be a safer option at this
> > moment unless we check all cases or specs...
>
> I read through the hardware docs yesterday and figured out what was
> going on. On all pre-gen4 hardware, the maximum backlight value has the
> lowest bit (of 16) hard-wired to zero. This means that the possible
> backlight duty cycle values are limited to 0 .. 0xfffe.
>
> On older hardware, this was managed by reporting this true range. This
> meant that the set_backlight path didn't need any special code; simply
> setting the values as provided should have worked just fine.
>
> On Pineview, this was changed (for reasons unknown) to use only 15 bits
> for backlight levels. The range of possible values is then
> 0 .. 0x7fff. In this case, values have to be shifted by one to convert
> between the advertised range of 0 .. 0x7fff and the hardware range of
> 0 .. 0xfffe.
>
> Exposing the range of 0 .. 0xfffe introduces a potential problem --
> write a value of 0xffff and the hardware gets mightily confused,
> and probably ends up turning the backlight off. Using the range of
> 0 .. 0x7fff avoids this issue completely.
>
> Using the narrower range does limit the precision of the backlight level
> setting, but it seems like 32767 possible values is more than sufficient...
>
> Here's a patch which just uses the pineview version everywhere (and
> cleans that up at the same time).
Thanks! It's pretty similar with my patch in the end, so in case
needed:
Reviewed-by: Takashi Iwai <tiwai at suse.de>
Maybe it'd be better to mention that actually setting bit-0 caused a
blank screen on some machines.
Takashi
> From e06789f688dc7ab1331f5f15ad3d60326b5139b4 Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp at keithp.com>
> Date: Fri, 18 Nov 2011 11:09:24 -0800
> Subject: [PATCH] drm/i915: Treat pre-gen4 backlight duty cycle value
> consistently
>
> For i945 and earlier chips, the backlight frequency value had the low
> bit (of 16) fixed to zero. The Pineview code path handled this by just
> exposing the backlight range as 15 bits while other chips had the
> backlight range limited to 0 .. 0xfffe.
>
> This patch makes everyone take the pineview code path, providing 15
> bits of backlight duty cycle range which seems more than sufficient to me.
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
> drivers/gpu/drm/i915/intel_panel.c | 16 +++++-----------
> 1 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 21f60b7..04d79fd 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -178,13 +178,10 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev)
> if (HAS_PCH_SPLIT(dev)) {
> max >>= 16;
> } else {
> - if (IS_PINEVIEW(dev)) {
> + if (INTEL_INFO(dev)->gen < 4)
> max >>= 17;
> - } else {
> + else
> max >>= 16;
> - if (INTEL_INFO(dev)->gen < 4)
> - max &= ~1;
> - }
>
> if (is_backlight_combination_mode(dev))
> max *= 0xff;
> @@ -203,13 +200,12 @@ u32 intel_panel_get_backlight(struct drm_device *dev)
> val = I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
> } else {
> val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
> - if (IS_PINEVIEW(dev))
> + if (INTEL_INFO(dev)->gen < 4)
> val >>= 1;
>
> if (is_backlight_combination_mode(dev)) {
> u8 lbpc;
>
> - val &= ~1;
> pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
> val *= lbpc;
> }
> @@ -246,11 +242,9 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev, u32 level
> }
>
> tmp = I915_READ(BLC_PWM_CTL);
> - if (IS_PINEVIEW(dev)) {
> - tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
> + if (INTEL_INFO(dev)->gen < 4)
> level <<= 1;
> - } else
> - tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
> + tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
> I915_WRITE(BLC_PWM_CTL, tmp | level);
> }
>
> --
> 1.7.7.3
>
>
>
> --
> keith.packard at intel.com
