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

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

[Dhinakaran Pandiyan]

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 <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
---
 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(&connector->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(&connector->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(&connector->encoder->base);
 	struct intel_panel *panel = &connector->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 = 0xFFFF;
 	else
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Don't enable backlight at setup time. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Don't enable backlight at setup time. (rev2)
URL   : https://patchwork.freedesktop.org/series/25677/
State : success

== Summary ==

Series 25677v2 drm/i915: Don't enable backlight at setup time.
https://patchwork.freedesktop.org/api/1.0/series/25677/revisions/2/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-r) fdo#100125

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:441s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:429s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:584s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:505s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:485s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:479s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:593s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:426s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:414s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:415s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:498s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:462s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:565s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:573s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:459s
fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:401s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:462s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:477s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:433s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:533s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time:402s

d4bedb8b0f9ba91df2e8cb136a489145a83e96a7 drm-tip: 2017y-06m-13d-14h-22m-46s UTC integration manifest
dc18a9b drm/i915: Don't enable backlight at setup time.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4945/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Puthikorn Voravootivat]

[-- Attachment #1.1: Type: text/plain, Size: 2966 bytes --]

I tested this on actual system and it is working fine.

On Tue, Jun 13, 2017 at 1:03 PM, Dhinakaran Pandiyan <
dhinakaran.pandiyan@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 <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  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(&connector->
> 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(&connector->
> 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(&connector->
> encoder->base);
>         struct intel_panel *panel = &connector->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 = 0xFFFF;
>         else
> --
> 2.7.4
>
>

[-- Attachment #1.2: Type: text/html, Size: 4260 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[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.pandiyan@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 <ville.syrjala@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Acked-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> >  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(&connector->
> > 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(&connector->
> > 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(&connector->
> > encoder->base);
> >         struct intel_panel *panel = &connector->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 = 0xFFFF;
> >         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