Re: i915/kms/backlight-combo mode problem

From: Takashi Iwai
Date: Mon May 09 2011 - 05:00:18 EST


At Mon, 09 May 2011 02:50:50 -0600,
Joey Lee wrote:
>
> Add Cc. Michael Chang for he is our i915 expert.
>
> Hi Melchior,
>
> æ æï2011-05-08 æ 16:05 +0200ïMelchior FRANZ æåï
> >
> > > Does it work to you direct control brightness by access
> > > by /sys/class/backlight/acer-wmi/brightness ?
> >
> > No. A number written to this virtual file is accepted and remembered,
> > but it doesn't actually change the brightness. It takes setpci to do
> > that.
> >
>
> I thought the video driver still is the KEY component for backlight
> issues, need fix the problem in video driver first.
>
> >
> >
> > > As I remember, use setpci to control brightness is not recommended
> > > because BIOS or ACPI will also touch brightness level. That will be
> > > better control brightness by the function that was provided by BIOS.
> > > e.g. ACPI or WMI interface, or direct control by EC.
> >
> > Well, sounds plausible. And I wouldn't do it if it weren't the only
> > way at the moment. :-)
> >
> >
> >
> > > That means that will be better fix your Fn key control brightness like
> > > before, you just need press Fn key to change brightness and don't need
> > > have any workaround.
> >
> > OK. I have added a lot of debug messages to intel_panel.c yesterday.
> > All it told me was that it seems to work correctly wiht acpi_osi=Linux.
> > Except that it doesn't actually change the brightness. Without acpi_osi
> > the functions aren't called at all and none of my messages showed up.
> >
>
> I traced _Q event in your DSDT, when acpi_osi=Linux, it run the Intel
> OpRegion logic for change brightness. And, finally, intel_opregion will
> access the function the were provided by intel_panel. So, the problem
> still back to intel_panel.
>
> After discuss with Michael chang, we thought there have problem in your
> brightness level after add combination mode:
>
> vi driver/gpu/drm/i915/intel_panel.c
>
> void intel_panel_set_backlight(struct drm_device *dev, u32 level)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 tmp;
>
> DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level);
>
> if (HAS_PCH_SPLIT(dev))
> return intel_pch_panel_set_backlight(dev, level);
>
> if (is_backlight_combination_mode(dev)){
> u32 max = intel_panel_get_max_backlight(dev);
> u8 lbpc;
>
> lbpc = level * 0xfe / max + 1;
> level /= lbpc; /* maybe the level changed by lbpc */
> pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc);
> }
>
> tmp = I915_READ(BLC_PWM_CTL);
> if (IS_PINEVIEW(dev)) {
> tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
> level <<= 1;
> } else
> tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
> I915_WRITE(BLC_PWM_CTL, tmp | level);
> }
>
> We need to know some run time value when intel_panel_set_backlight call by funciton key.

Yes, that'll help understanding.

> Please help to apply the attached debug patch to intel_panel.c then attached dmesg.

The patch has an obvious typo :)
Also, we should track the value in intel_panel_get_backlight(), too.


Takashi


> > > Looks like current status is we try to fix bko#31522 but the patch
> > > causes your brightness no work by press Fn key even with acpi_osi=Linux.
> > > Does it right?
> >
> > The history is: with acpi_osi=Linux everything worked with 2.6.38-rc8.
> > With 2.6.38 the screen stayed black. The patch that only ignored lbpc=0
> > worked (IIRC) including key adjustment. Later patches broke keys.
> >
> >
> >
> > > replace acpi_osi=Linux by acpi_osi="!Windows 2006"
> > >
> > > Does it also works to you for backlight control?
> >
> > No, doesn't work.
> >
>
> We can test the acpi_osi="!Windows 2006" again after we fix the i915's
> problem.
>
>
> Thank's
> Joey Lee
>
> The following is debug patch, and please add kernel parameter
> drm.debug=0x02 :
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index f8f86e5..f62dbd9 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -236,17 +236,22 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 level)
> u32 max = intel_panel_get_max_backlight(dev);
> u8 lbpc;
>
> + DRM_DEBUG_DRIVER("set backlight max = % lbpc = level * 0xfe / max + 1;
> + DRM_DEBUG_DRIVER("set backlight lbpc = %d\n", lbpc);
> level /= lbpc;
> pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc);
> }
>
> tmp = I915_READ(BLC_PWM_CTL);
> + DRM_DEBUG_DRIVER("set backlight tmp(1) = %d\n", tmp);
> if (IS_PINEVIEW(dev)) {
> tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
> level <<= 1;
> } else
> tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
> + DRM_DEBUG_DRIVER("set backlight tmp(2) = %d\n", tmp);
> + DRM_DEBUG_DRIVER("set backlight level = %d\n", level);
> I915_WRITE(BLC_PWM_CTL, tmp | level);
> }
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/