Re: [Intel-gfx] [PATCH v2] drm/i915: Don't enable backlight at setup time.

2017-06-15 Thread Ville Syrjälä
On Wed, Jun 14, 2017 at 01:18:42PM -0700, Puthikorn Voravootivat wrote:
> I tested this on actual system and it is working fine.

Thank you everyone. Patch pushed to dinq.

> 
> On Tue, Jun 13, 2017 at 1:03 PM, Dhinakaran Pandiyan <
> dhinakaran.pandi...@intel.com> wrote:
> 
> > Maarten and Ville noticed that we are enabling backlight via DP aux very
> > early in the modeset_init path via the intel_dp_aux_setup_backlight()
> > function, since commit e7156c833903 ("drm/i915: Add Backlight Control using
> > DPCD for eDP connectors (v9)"). Looks like all we need to do during
> > _setup_backlight() is read the current brightness state instead of
> > modifying it.
> >
> > v2: Rewrote commit message.
> >
> > Cc: Ville Syrjala 
> > Cc: Maarten Lankhorst 
> > Cc: Jani Nikula 
> > Cc: Yetunde Adebisi 
> > Signed-off-by: Dhinakaran Pandiyan 
> > Reviewed-by: Maarten Lankhorst 
> > Acked-by: Jani Nikula 
> > ---
> >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 10 --
> >  1 file changed, 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > index 6cc6298..228ca06 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > @@ -80,10 +80,6 @@ static uint32_t intel_dp_aux_get_backlight(struct
> > intel_connector *connector)
> >  static void
> >  intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state,
> > u32 level)
> >  {
> > -   /*
> > -* conn_state->best_encoder is likely NULL when called from
> > -* intel_dp_aux_setup_backlight()
> > -*/
> > struct intel_connector *connector = to_intel_connector(conn_state-
> > >connector);
> > struct intel_dp *intel_dp = enc_to_intel_dp(>
> > encoder->base);
> > uint8_t vals[2] = { 0x0 };
> > @@ -106,10 +102,6 @@ static void intel_dp_aux_enable_backlight(const
> > struct intel_crtc_state *crtc_st
> >   const struct drm_connector_state
> > *conn_state)
> >  {
> > struct intel_connector *connector = to_intel_connector(conn_state-
> > >connector);
> > -   /*
> > -* conn_state->best_encoder (and crtc_state) are NULL when called
> > from
> > -* intel_dp_aux_setup_backlight()
> > -*/
> > struct intel_dp *intel_dp = enc_to_intel_dp(>
> > encoder->base);
> > uint8_t dpcd_buf = 0;
> > uint8_t edp_backlight_mode = 0;
> > @@ -156,8 +148,6 @@ static int intel_dp_aux_setup_backlight(struct
> > intel_connector *connector,
> > struct intel_dp *intel_dp = enc_to_intel_dp(>
> > encoder->base);
> > struct intel_panel *panel = >panel;
> >
> > -   intel_dp_aux_enable_backlight(NULL, connector->base.state);
> > -
> > if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_
> > BYTE_COUNT)
> > panel->backlight.max = 0x;
> > else
> > --
> > 2.7.4
> >
> >

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] drm/i915: Don't enable backlight at setup time.

2017-06-14 Thread Puthikorn Voravootivat
I tested this on actual system and it is working fine.

On Tue, Jun 13, 2017 at 1:03 PM, Dhinakaran Pandiyan <
dhinakaran.pandi...@intel.com> wrote:

> Maarten and Ville noticed that we are enabling backlight via DP aux very
> early in the modeset_init path via the intel_dp_aux_setup_backlight()
> function, since commit e7156c833903 ("drm/i915: Add Backlight Control using
> DPCD for eDP connectors (v9)"). Looks like all we need to do during
> _setup_backlight() is read the current brightness state instead of
> modifying it.
>
> v2: Rewrote commit message.
>
> Cc: Ville Syrjala 
> Cc: Maarten Lankhorst 
> Cc: Jani Nikula 
> Cc: Yetunde Adebisi 
> Signed-off-by: Dhinakaran Pandiyan 
> Reviewed-by: Maarten Lankhorst 
> Acked-by: Jani Nikula 
> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 10 --
>  1 file changed, 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index 6cc6298..228ca06 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -80,10 +80,6 @@ static uint32_t intel_dp_aux_get_backlight(struct
> intel_connector *connector)
>  static void
>  intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state,
> u32 level)
>  {
> -   /*
> -* conn_state->best_encoder is likely NULL when called from
> -* intel_dp_aux_setup_backlight()
> -*/
> struct intel_connector *connector = to_intel_connector(conn_state-
> >connector);
> struct intel_dp *intel_dp = enc_to_intel_dp(>
> encoder->base);
> uint8_t vals[2] = { 0x0 };
> @@ -106,10 +102,6 @@ static void intel_dp_aux_enable_backlight(const
> struct intel_crtc_state *crtc_st
>   const struct drm_connector_state
> *conn_state)
>  {
> struct intel_connector *connector = to_intel_connector(conn_state-
> >connector);
> -   /*
> -* conn_state->best_encoder (and crtc_state) are NULL when called
> from
> -* intel_dp_aux_setup_backlight()
> -*/
> struct intel_dp *intel_dp = enc_to_intel_dp(>
> encoder->base);
> uint8_t dpcd_buf = 0;
> uint8_t edp_backlight_mode = 0;
> @@ -156,8 +148,6 @@ static int intel_dp_aux_setup_backlight(struct
> intel_connector *connector,
> struct intel_dp *intel_dp = enc_to_intel_dp(>
> encoder->base);
> struct intel_panel *panel = >panel;
>
> -   intel_dp_aux_enable_backlight(NULL, connector->base.state);
> -
> if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_
> BYTE_COUNT)
> panel->backlight.max = 0x;
> else
> --
> 2.7.4
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx