On Mon, 31 Mar 2014, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> wrote:
> With the new checks in place, we can see we're doing things backwards,
> so fix them up per the spec.
>
> Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b6f7087..d540fbe 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1273,8 +1273,6 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
>
> DRM_DEBUG_KMS("Turn eDP power off\n");
>
> - edp_wait_backlight_off(intel_dp);
> -
Please justify moving this wait to intel_edp_backlight_off. I thought
the point of these wait calls is such that we'll only end up waiting
when we really have to. If this is left as-is, we can do useful stuff
*while* waiting (case in point, intel_dp_sink_dpms in intel_disable_dp).
Otherwise this looks good to me, although I didn't find proper
explanations for everything. Do I understand correctly that the
EDP_BLC_ENABLE bit in PP_CONTROL is basically the final switch for
enabling/disabling the PWM signal to the eDP? So before this patch, we
let the disabled PWM signal through to the panel?
BR,
Jani.
> WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
>
> /* By this time the PWM and BLC bits should be off already */
> @@ -1316,6 +1314,9 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp)
> return;
>
> DRM_DEBUG_KMS("\n");
> +
> + intel_panel_enable_backlight(intel_dp->attached_connector);
> +
> /*
> * If we enable the backlight right away following a panel power
> * on, we may see slight flicker as the panel syncs with the eDP
> @@ -1333,8 +1334,6 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp)
>
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> -
> - intel_panel_enable_backlight(intel_dp->attached_connector);
> }
>
> void intel_edp_backlight_off(struct intel_dp *intel_dp)
> @@ -1350,8 +1349,6 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
> /* PWM must still be enabled here */
> assert_pwm_enabled(intel_dp->attached_connector);
>
> - intel_panel_disable_backlight(intel_dp->attached_connector);
> -
> DRM_DEBUG_KMS("\n");
> pp = ironlake_get_pp_control(intel_dp);
> pp &= ~EDP_BLC_ENABLE;
> @@ -1361,6 +1358,10 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> intel_dp->last_backlight_off = jiffies;
> +
> + edp_wait_backlight_off(intel_dp);
> +
> + intel_panel_disable_backlight(intel_dp->attached_connector);
> }
>
> static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
> --
> 1.8.4.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
