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

[PATCH] 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. Looks like all we need to do during _setup_backlight() is
read the current brightness state instead of modifying it, so I don't
why need to _enable_backlight() from _setup_backlight().

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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.

[Patchwork]

== Series Details ==

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

== Summary ==

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

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (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:443s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:434s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:585s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:510s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:488s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:477s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:589s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:428s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:419s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:492s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:464s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:567s
fi-kbl-r         total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:577s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:465s
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:463s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:483s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:437s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:402s

4f89fbd3f94f4ba47f779909a2c7bcd1cfb958ad drm-tip: 2017y-06m-12d-19h-18m-52s UTC integration manifest
d459aa6 drm/i915: Don't enable backlight at setup time.

== Logs ==

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

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

[Maarten Lankhorst]

Op 12-06-17 om 22:16 schreef 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. Looks like all we need to do during _setup_backlight() is
> read the current brightness state instead of modifying it, so I don't
> why need to _enable_backlight() from _setup_backlight().
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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);
Maybe change this one to conn_state->best_encoder now? Same as for set_backlight. Needs some runtime testing though. :)
>  	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
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Cheers,

Maarten

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

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

[Pandiyan, Dhinakaran]

On Tue, 2017-06-13 at 08:29 +0200, Maarten Lankhorst wrote:
> Op 12-06-17 om 22:16 schreef 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. Looks like all we need to do during _setup_backlight() is
> > read the current brightness state instead of modifying it, so I don't
> > why need to _enable_backlight() from _setup_backlight().
> >
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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);
> Maybe change this one to conn_state->best_encoder now? Same as for set_backlight. Needs some runtime testing though. :)

I see four instances that need to be converted, I should perhaps do it a
in a separate patch. But, I can't seem to get brightness control working
on the SDP I have here. So, I'm a bit wary of doing it until I can bench
test properly.

Thanks for the review.

-DK


> >  	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
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> Cheers,
> 
> Maarten
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

[Jani Nikula]

On Mon, 12 Jun 2017, 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. Looks like all we need to do during _setup_backlight() is
> read the current brightness state instead of modifying it, so I don't
> why need to _enable_backlight() from _setup_backlight().

Please always use git blame to find the commit, and Cc the relevant
folks. Done now. It's e7156c833903 ("drm/i915: Add Backlight Control
using DPCD for eDP connectors (v9)").

The changelog says the call was intentionally moved there. ("v5: Moved
call to initialize backlight registers to dp_aux_setup_backlight"). I
have no recollection of why, and seems wrong, regardless of me signing
off on the patch.

We might be seeing some of the fallout also because of the recent
changes to the enable implementation by Puthikorn, though I agree the
whole call does seem wrong to begin with. Cc Puthikorn too.


Acked-by: Jani Nikula <jani.nikula@intel.com>


BR,
Jani.


>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Pandiyan, Dhinakaran]

On Tue, 2017-06-13 at 09:48 +0300, Jani Nikula wrote:
> On Mon, 12 Jun 2017, 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. Looks like all we need to do during _setup_backlight() is
> > read the current brightness state instead of modifying it, so I don't
> > why need to _enable_backlight() from _setup_backlight().
> 
> Please always use git blame to find the commit, and Cc the relevant
> folks. Done now. It's e7156c833903 ("drm/i915: Add Backlight Control
> using DPCD for eDP connectors (v9)").

I'll amend the commit message to refer to the orginal patch and resend.

> 
> The changelog says the call was intentionally moved there. ("v5: Moved
> call to initialize backlight registers to dp_aux_setup_backlight"). I
> have no recollection of why, and seems wrong, regardless of me signing
> off on the patch.

I looked at the conversation history, looks like it was intentionally
moved from _display_control_capable() to _setup_backlight() but not
clear why it was added to _display_control_capable() in the first place.

> 
> We might be seeing some of the fallout also because of the recent
> changes to the enable implementation by Puthikorn, though I agree the
> whole call does seem wrong to begin with. Cc Puthikorn too.
> 
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> BR,
> Jani.
> 
> 
> >
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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
> 

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

Re: Fixes that failed to backport to v4.12-rc1

[Jani Nikula]

On Mon, 15 May 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> Continuing [1] for v4.12-rc1
>
> The following commits have been marked as Cc: stable or fixing something
> in v4.12-rc1 or earlier, but failed to cherry-pick to
> drm-intel-fixes. Please see if they are worth backporting, and please do
> so if they are.

Update:

f6262bda462e ("drm/i915: Don't enable backlight at setup time.")
e27ab73d17ef ("drm/i915: Mark CPU cache as dirty on every transition for CPU writes")
071750e550af ("drm/i915: Disable EXEC_OBJECT_ASYNC when doing relocations")

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] 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>
Tested-by: Puthikorn Voravootivat <puthik@chromium.org>
Fixes: e7156c833903 ("drm/i915: Add Backlight Control using DPCD for eDP connectors (v9)")
Link: http://patchwork.freedesktop.org/patch/msgid/1497384239-2965-1-git-send-email-dhinakaran.pandiyan@intel.com
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
(cherry picked from commit f6262bda462e81e959b80a96dac799bd9df27f73)
---
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index 6532e22..40ba313 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -119,8 +119,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(connector);
-
 	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

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

[Pandiyan, Dhinakaran]

On Mon, 2017-06-19 at 11:08 -0700, Dhinakaran Pandiyan 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>
> Tested-by: Puthikorn Voravootivat <puthik@chromium.org>
> Fixes: e7156c833903 ("drm/i915: Add Backlight Control using DPCD for eDP connectors (v9)")
> Link: http://patchwork.freedesktop.org/patch/msgid/1497384239-2965-1-git-send-email-dhinakaran.pandiyan@intel.com
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> (cherry picked from commit f6262bda462e81e959b80a96dac799bd9df27f73)


I am sending a backport for the first time, please let me know if I
messed up something. 



> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index 6532e22..40ba313 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -119,8 +119,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(connector);
> -
>  	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
>  		panel->backlight.max = 0xFFFF;
>  	else

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

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

[Jani Nikula]

On Mon, 19 Jun 2017, 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>
> Tested-by: Puthikorn Voravootivat <puthik@chromium.org>
> Fixes: e7156c833903 ("drm/i915: Add Backlight Control using DPCD for eDP connectors (v9)")
> Link: http://patchwork.freedesktop.org/patch/msgid/1497384239-2965-1-git-send-email-dhinakaran.pandiyan@intel.com
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> (cherry picked from commit f6262bda462e81e959b80a96dac799bd9df27f73)

Pushed to drm-intel-fixes, thanks for the backport.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index 6532e22..40ba313 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -119,8 +119,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(connector);
> -
>  	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
>  		panel->backlight.max = 0xFFFF;
>  	else

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx