Re: [PATCH] properly enable the blc controller on the right pipe

[Carsten Emde <C.Emde@osadl.org>]

On 07/19/2012 04:40 PM, Daniel Vetter wrote:
> On Thu, Jul 19, 2012 at 4:00 PM, Carsten Emde<C.Emde@osadl.org>  wrote:
>> On 06/11/2012 10:51 AM, Daniel Vetter wrote:
>>> I've updated my backlight-confusion branch with new patches for gen4
>>> (it doesn't seem to work for these machines yet). Can you please test
>>> the updated branch?
>>
>> I have added an additional test rack (#6) to our QA farm (osadl.org/QA) and
>> populated this rack solely with notebooks that are equipped with Intel
>> graphics board:
>>
>> Slot #1: Lenovo T61/7661W4G, Intel GMA X3100
>> Slot #2: IBM T601951-A47, Intel GMA 950
>> Slot #4: Acer Homa/Extensa 5230E-901G16N, Intel GMA 4500MHD
>> Slot #5: Acer Morar/TravelMate 2410, Intel GMA 900
>> Slot #6: Dell Inspirion 1300/0RJ272, Intel GMA 900
>> Slot #7: Acer Aspire 5734Z, Intel GMA 4500M
>>
>> With the exception of slot #4 that runs a 3.2-based kernel, all other
>> notebooks run a 3.4-based kernel. The notebook in Slot #7 is the one with
>> the backlight headache. Unfortunately :-; no other notebook had any
>> backlight problem when running a vanilla kernel, even the two new Acers are
>> behaving well.
>>
>> In the meantime, however, I have received a report that the Acer 5732Z has
>> the same problem. I am sure that the reporter will be willing to help with
>> testing. Looks like the backlight problem is very specific to Acer Aspire
>> 573xZ. But I agree that a generic solution always is better than a quirk.
>>
>> I've checked out the backlight-confusion branch of your git tree and tested
>> it with and without the Acer quirk. I can report that the backlight works
>> correctly with either version. I will now contact the owner of the Acer
>> Aspire 5732Z and ask him to test the backlight-confusion branch without the
>> Acer quirk on his notebook as well.
>>
>> Do you wish me to do any other test?
>
> Well, the backlight-confusion branch had a bug and outright disabled
> the intel backlight :( drm-intel-next-queued should have fixes for all
> the backlight goofus I've accidentally created, so testing feedback on
> that branch would be highly appreciated.
This branch is now broken again. The original drm-intel-next-queued 
branch works well (it still has the quirk), but after I reverted the 
Acer quirk, the screen remains dark.

I double-checked it and used exactly the same patch to revert the quirk 
in the two trees: The backlight-confusion branch works, and the 
drm-intel-next-queued branch does not.

I then used drivers/gpu/drm/i915/intel_panel.c from backlight-confusion 
in drm-intel-next-queued and - it worked again. When testing the two 
differences separately it turned out this patch did the magic:

Index: drm-intel/drivers/gpu/drm/i915/intel_panel.c
===================================================================
--- drm-intel.orig/drivers/gpu/drm/i915/intel_panel.c
+++ drm-intel/drivers/gpu/drm/i915/intel_panel.c
@@ -121,6 +121,8 @@ static int is_backlight_combination_mode
  {
  	struct drm_i915_private *dev_priv = dev->dev_private;

+	return 0;
+
  	if (INTEL_INFO(dev)->gen >= 4)
  		return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE;

Finally, only the first condition is obviously harmful - this patch also 
works well:

Index: drm-intel/drivers/gpu/drm/i915/intel_panel.c
===================================================================
--- drm-intel.orig/drivers/gpu/drm/i915/intel_panel.c
+++ drm-intel/drivers/gpu/drm/i915/intel_panel.c
@@ -121,8 +121,10 @@ static int is_backlight_combination_mode
  {
  	struct drm_i915_private *dev_priv = dev->dev_private;

+#if 0
  	if (INTEL_INFO(dev)->gen >= 4)
  		return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE;
+#endif

  	if (IS_GEN2(dev))
  		return I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE;

Hope this helps,

	-Carsten.