[Intel-gfx] [PATCH 1/3] drm/i915/display: Load DP_TP_CTL/STATUS offset before use it

[Lucas De Marchi]

On Tue, Apr 14, 2020 at 4:03 PM José Roberto de Souza
<jose.souza at intel.com> wrote:
>
> Right now dp.regs.dp_tp_ctl/status are only set during the encoder
> pre_enable() hook, what is causing all reads and writes to those
> registers to go to offset 0x0 before pre_enable() is executed.
>
> So if i915 takes the BIOS state and don't do a modeset any following
> link retraing will fail.
>
> In the case that i915 needs to do a modeset, the DDI disable sequence
> will write to a wrong register not disabling DP 'Transport Enable' in
> DP_TP_CTL, making a HDMI modeset in the same port/transcoder to
> not light up the monitor.
>
> So here for GENs older than 12, that have those registers fixed at
> port offset range it is loading at encoder/port init while for GEN12
> it will keep setting it at encoder pre_enable() and during HW state
> readout.
>
> Fixes: 4444df6e205b ("drm/i915/tgl: move DP_TP_* to transcoder")
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 14 +++++++++++---
>  drivers/gpu/drm/i915/display/intel_dp.c  |  5 ++---
>  2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index be6c61bcbc9c..1aab93a94f40 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3252,9 +3252,6 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>         intel_dp_set_link_params(intel_dp, crtc_state->port_clock,
>                                  crtc_state->lane_count, is_mst);
>
> -       intel_dp->regs.dp_tp_ctl = DP_TP_CTL(port);
> -       intel_dp->regs.dp_tp_status = DP_TP_STATUS(port);

reason to be where it was is because of MST. I think what you are
doing here will break it since now this is set for the port and not
transcoder.
intel_mst_pre_enable_dp() would call here only for the first stream,
so all the others would use this same transcoder.

Lucas De Marchi

> -
>         intel_edp_panel_on(intel_dp);
>
>         intel_ddi_clk_select(encoder, crtc_state);
> @@ -4061,12 +4058,18 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>         struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>         struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->uapi.crtc);
>         enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +       struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>         u32 temp, flags = 0;
>
>         /* XXX: DSI transcoder paranoia */
>         if (drm_WARN_ON(&dev_priv->drm, transcoder_is_dsi(cpu_transcoder)))
>                 return;
>
> +       if (INTEL_GEN(dev_priv) >= 12) {
> +               intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(cpu_transcoder);
> +               intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(cpu_transcoder);
> +       }
> +
>         intel_dsc_get_config(encoder, pipe_config);
>
>         temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> @@ -4396,6 +4399,7 @@ static const struct drm_encoder_funcs intel_ddi_funcs = {
>  static struct intel_connector *
>  intel_ddi_init_dp_connector(struct intel_digital_port *intel_dig_port)
>  {
> +       struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>         struct intel_connector *connector;
>         enum port port = intel_dig_port->base.port;
>
> @@ -4406,6 +4410,10 @@ intel_ddi_init_dp_connector(struct intel_digital_port *intel_dig_port)
>         intel_dig_port->dp.output_reg = DDI_BUF_CTL(port);
>         intel_dig_port->dp.prepare_link_retrain =
>                 intel_ddi_prepare_link_retrain;
> +       if (INTEL_GEN(dev_priv) < 12) {
> +               intel_dig_port->dp.regs.dp_tp_ctl = DP_TP_CTL(port);
> +               intel_dig_port->dp.regs.dp_tp_status = DP_TP_STATUS(port);
> +       }
>
>         if (!intel_dp_init_connector(intel_dig_port, connector)) {
>                 kfree(connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index d4fcc9583869..03591ab76b0d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2671,9 +2671,6 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
>                                  intel_crtc_has_type(pipe_config,
>                                                      INTEL_OUTPUT_DP_MST));
>
> -       intel_dp->regs.dp_tp_ctl = DP_TP_CTL(port);
> -       intel_dp->regs.dp_tp_status = DP_TP_STATUS(port);
> -
>         /*
>          * There are four kinds of DP registers:
>          *
> @@ -8470,6 +8467,8 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>
>         intel_dig_port->dp.output_reg = output_reg;
>         intel_dig_port->max_lanes = 4;
> +       intel_dig_port->dp.regs.dp_tp_ctl = DP_TP_CTL(port);
> +       intel_dig_port->dp.regs.dp_tp_status = DP_TP_STATUS(port);
>
>         intel_encoder->type = INTEL_OUTPUT_DP;
>         intel_encoder->power_domain = intel_port_to_power_domain(port);
> --
> 2.26.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Lucas De Marchi