[Intel-gfx] [PATCH] drm/i915/display: Force the state compute phase once to enable PSR
Mun, Gwan-gyeong
gwan-gyeong.mun at intel.com
Thu Dec 5 09:17:58 UTC 2019
Hi,
On Mon, 2019-11-25 at 15:38 -0800, José Roberto de Souza wrote:
> Recent improvements in the state tracking in i915 caused PSR to not
> be
> enabled when reusing firmware/BIOS modeset, this is due to all
> initial
> commits returning ealier in intel_atomic_check() as needs_modeset()
> is always false.
>
> To fix that here forcing the state compute phase in CRTC that is
> driving the eDP that supports PSR once. Enable or disable PSR do not
> require a fullmodeset, so user will still experience glitch free boot
> process plus the power savings that PSR brings.
>
> It was tried to set mode_changed in intel_initial_commit() but at
> this point the connectors are not registered causing a crash when
> computing encoder state.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112253
> Reported-by: <s.zharkoff at gmail.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_atomic.c | 6 ++++
> drivers/gpu/drm/i915/display/intel_psr.c | 32
> +++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_psr.h | 5 ++++
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> 4 files changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index fd0026fc3618..6b57488d9238 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -37,6 +37,7 @@
> #include "intel_atomic.h"
> #include "intel_display_types.h"
> #include "intel_hdcp.h"
> +#include "intel_psr.h"
> #include "intel_sprite.h"
>
> /**
> @@ -127,6 +128,7 @@ int intel_digital_connector_atomic_check(struct
> drm_connector *conn,
> struct intel_digital_connector_state *old_conn_state =
> to_intel_digital_connector_state(old_state);
> struct drm_crtc_state *crtc_state;
> + int ret;
>
> intel_hdcp_atomic_check(conn, old_state, new_state);
>
> @@ -149,6 +151,10 @@ int intel_digital_connector_atomic_check(struct
> drm_connector *conn,
> old_conn_state->base.hdr_output_metadata))
> crtc_state->mode_changed = true;
>
> + ret = intel_psr_atomic_check(conn, state);
Because intel_psr_atomic_check() returns always 0, IMHO we don't need
to use this checking block.
And can we move calling of this fuction to below of calling
intel_hdcp_atomic_check(conn, old_state, new_state);?
> + if (ret)
> + return ret;
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index c1d133362b76..a1acae8d72f7 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1444,3 +1444,35 @@ bool intel_psr_enabled(struct intel_dp
> *intel_dp)
>
> return ret;
> }
> +
> +int
> +intel_psr_atomic_check(struct drm_connector *conn,
> + struct drm_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(conn->dev);
> + struct drm_connector_state *conn_state;
> + struct intel_digital_port *dig_port;
> + struct intel_connector *intel_conn;
> + struct drm_crtc_state *crtc_state;
> +
> + if (!CAN_PSR(dev_priv))
> + return 0;
> +
> + conn_state = drm_atomic_get_new_connector_state(state, conn);
should we get conn_state again?
> + if (!conn_state->crtc)
> + return 0;
> +
> + intel_conn = to_intel_connector(conn);
> + dig_port = enc_to_dig_port(&intel_conn->encoder->base);
> + if (dev_priv->psr.dp != &dig_port->dp)
> + return 0;
> +
> + if (dev_priv->psr.initially_probed)
> + return 0;
> +
> + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state-
> >crtc);
> + crtc_state->mode_changed = true;
> + dev_priv->psr.initially_probed = true;
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> b/drivers/gpu/drm/i915/display/intel_psr.h
> index 46e4de8b8cd5..b3535e5752a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -6,6 +6,9 @@
> #ifndef __INTEL_PSR_H__
> #define __INTEL_PSR_H__
>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_connector.h>
> +
> #include "intel_frontbuffer.h"
>
> struct drm_i915_private;
> @@ -35,5 +38,7 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp);
> int intel_psr_wait_for_idle(const struct intel_crtc_state
> *new_crtc_state,
> u32 *out_value);
> bool intel_psr_enabled(struct intel_dp *intel_dp);
> +int intel_psr_atomic_check(struct drm_connector *conn,
> + struct drm_atomic_state *state);
>
> #endif /* __INTEL_PSR_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index fdae5a919bc8..d834924ba7c3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -508,6 +508,7 @@ struct i915_psr {
> bool dc3co_enabled;
> u32 dc3co_exit_delay;
> struct delayed_work idle_work;
> + bool initially_probed;
> };
>
> #define QUIRK_LVDS_SSC_DISABLE (1<<1)
More information about the Intel-gfx
mailing list