diff mbox series

[v2] drm/i915: Refine eDP aux backlight enable/disable sequence

Message ID 1561476910-23655-1-git-send-email-shawn.c.lee@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915: Refine eDP aux backlight enable/disable sequence | expand

Commit Message

Lee, Shawn C June 25, 2019, 3:35 p.m. UTC
Modify aux backlight enable/disable sequence just like what we
did for genernal eDP panel. Setup PWM freq and brightness level
before enable display backlight.

v2: According to spec, it is not necessary to set PWM to 0 if
    backlight enable already off. Remove intel_dp_aux_set_backlight to 0.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Jose Roberto de Souza <jose.souza@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>

Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jani Nikula June 25, 2019, 3:13 p.m. UTC | #1
On Tue, 25 Jun 2019, Lee Shawn C <shawn.c.lee@intel.com> wrote:
> Modify aux backlight enable/disable sequence just like what we
> did for genernal eDP panel. Setup PWM freq and brightness level
> before enable display backlight.
>
> v2: According to spec, it is not necessary to set PWM to 0 if
>     backlight enable already off. Remove intel_dp_aux_set_backlight to 0.

Please think it through again. The same applies here. The brightness
change before enable is ignored according to eDP spec.

BR,
Jani.

>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Jose Roberto de Souza <jose.souza@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
>
> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index 6b0b73479fb8..70d9b9c9586f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -216,8 +216,8 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
>  		}
>  	}
>  
> -	set_aux_backlight_enable(intel_dp, true);
>  	intel_dp_aux_set_backlight(conn_state, connector->panel.backlight.level);
> +	set_aux_backlight_enable(intel_dp, true);
>  }
>  
>  static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old_conn_state)
Lee, Shawn C June 25, 2019, 3:29 p.m. UTC | #2
On Tue, 25 Jun 2019, Jani Nikula <jani.nikula@intel.com> wrote:
>On Tue, 25 Jun 2019, Lee Shawn C <shawn.c.lee@intel.com> wrote:
>> Modify aux backlight enable/disable sequence just like what we did for 
>> genernal eDP panel. Setup PWM freq and brightness level before enable 
>> display backlight.
>>
>> v2: According to spec, it is not necessary to set PWM to 0 if
>>     backlight enable already off. Remove intel_dp_aux_set_backlight to 0.
>
>Please think it through again. The same applies here. The brightness change before enable is ignored according to eDP spec.
>
>BR,
>Jani.
>

This is what the problem we see on a new eDP panel from customer. TCON did not turn backlight on successful.
After sawp the sequence, backlight works well as expected. 

In the other words. Looks like this TCON backlight enable flow violate eDP spec. What do you think?

>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Jose Roberto de Souza <jose.souza@intel.com>
>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>>
>> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c 
>> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> index 6b0b73479fb8..70d9b9c9586f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> @@ -216,8 +216,8 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
>>  		}
>>  	}
>>  
>> -	set_aux_backlight_enable(intel_dp, true);
>>  	intel_dp_aux_set_backlight(conn_state, 
>> connector->panel.backlight.level);
>> +	set_aux_backlight_enable(intel_dp, true);
>>  }
>>  
>>  static void intel_dp_aux_disable_backlight(const struct 
>> drm_connector_state *old_conn_state)
>
>--
>Jani Nikula, Intel Open Source Graphics Center
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index 6b0b73479fb8..70d9b9c9586f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -216,8 +216,8 @@  static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
 		}
 	}
 
-	set_aux_backlight_enable(intel_dp, true);
 	intel_dp_aux_set_backlight(conn_state, connector->panel.backlight.level);
+	set_aux_backlight_enable(intel_dp, true);
 }
 
 static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old_conn_state)