From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: "Navare, Manasi" <manasi.d.navare@intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v8 06/11] drm/i915: Enable big joiner support in enable and disable sequences.
Date: Fri, 28 Aug 2020 08:26:12 +0200 [thread overview]
Message-ID: <16390edb-fca7-d2a7-d1ae-c943b3f3e0a9@linux.intel.com> (raw)
In-Reply-To: <20200827233509.GA26009@labuser-Z97X-UD5H>
Op 28-08-2020 om 01:35 schreef Navare, Manasi:
> On Mon, Aug 10, 2020 at 04:28:28PM -0700, Manasi Navare wrote:
>> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> Make vdsc work when no output is enabled. The big joiner needs VDSC
>> on the slave, so enable it and set the appropriate bits.
>> Also update timestamping constants, because slave crtc's are not
>> updated in drm_atomic_helper_update_legacy_modeset_state().
>>
>> This should be enough to bring up CRTC's in a big joiner configuration,
>> without any plane configuration on the second pipe yet.
>>
>> HOWEVER, we still bring up the crtc's in the wrong order. We need to
>> make sure that the master crtc is brought up after the slave crtc.
>> This is done correctly later in this series.
>>
>> The next steps are to enable planes correctly, and make sure we enable
>> and update both master and slave in the correct order.
>>
>> v2:
>> * Manual rebase (Manasi)
>>
>> v3:
>> * Rebase (Manasi)
>>
>> v4:
>> * Rebase (Manasi)
>>
>> v5:
>> * Get dsc power domain in ddi_init (Manasi)
>>
>> v6:
>> * Remove dsc power put from dsc_disable (Maarten)
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/icl_dsi.c | 2 -
>> drivers/gpu/drm/i915/display/intel_ddi.c | 68 +++-
>> drivers/gpu/drm/i915/display/intel_display.c | 377 ++++++++++++------
>> .../drm/i915/display/intel_display_types.h | 1 +
>> drivers/gpu/drm/i915/display/intel_dp.c | 6 +-
>> drivers/gpu/drm/i915/display/intel_vdsc.c | 201 +++++-----
>> drivers/gpu/drm/i915/display/intel_vdsc.h | 7 +-
>> 7 files changed, 413 insertions(+), 249 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
>> index 8c55f5bee9ab..26f7372b4c25 100644
>> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> @@ -1454,8 +1454,6 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>> struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
>> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>
>> - intel_dsc_get_config(encoder, pipe_config);
> Maarten,
> Why do we need to remove this from dsi_get_config()?
This is read by the pipe now, which is the only place that does get_config().
>
>> -
>> /* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
>> pipe_config->port_clock = intel_dpll_get_freq(i915,
>> pipe_config->shared_dpll);
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index de5b216561d8..6de13c67f5b8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -28,6 +28,7 @@
>> #include <drm/drm_scdc_helper.h>
>>
>> #include "i915_drv.h"
>> +#include "i915_trace.h"
>> #include "intel_audio.h"
>> #include "intel_combo_phy.h"
>> #include "intel_connector.h"
>> @@ -2093,12 +2094,6 @@ static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
>> intel_display_power_get(dev_priv,
>> intel_ddi_main_link_aux_domain(dig_port));
>>
>> - /*
>> - * VDSC power is needed when DSC is enabled
>> - */
>> - if (crtc_state->dsc.compression_enable)
>> - intel_display_power_get(dev_priv,
>> - intel_dsc_power_domain(crtc_state));
>> }
>>
>> void intel_ddi_enable_pipe_clock(struct intel_encoder *encoder,
>> @@ -3387,7 +3382,8 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>>
>> /* 7.l Configure and enable FEC if needed */
>> intel_ddi_enable_fec(encoder, crtc_state);
>> - intel_dsc_enable(encoder, crtc_state);
>> + if (!crtc_state->bigjoiner)
>> + intel_dsc_enable(encoder, crtc_state);
>> }
>>
>> static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>> @@ -3458,7 +3454,8 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>> if (!is_mst)
>> intel_ddi_enable_pipe_clock(encoder, crtc_state);
>>
>> - intel_dsc_enable(encoder, crtc_state);
>> + if (!crtc_state->bigjoiner)
>> + intel_dsc_enable(encoder, crtc_state);
>> }
>>
>> static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
>> @@ -3713,6 +3710,21 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
>> ilk_pfit_disable(old_crtc_state);
>> }
>>
>> + if (old_crtc_state->bigjoiner_linked_crtc) {
>> + struct intel_atomic_state *state =
>> + to_intel_atomic_state(old_crtc_state->uapi.state);
>> + struct intel_crtc *slave =
>> + old_crtc_state->bigjoiner_linked_crtc;
>> + const struct intel_crtc_state *old_slave_crtc_state =
>> + intel_atomic_get_old_crtc_state(state, slave);
>> +
>> + intel_crtc_vblank_off(old_slave_crtc_state);
>> + trace_intel_pipe_disable(slave);
>> +
>> + intel_dsc_disable(old_slave_crtc_state);
>> + skl_scaler_disable(old_slave_crtc_state);
>> + }
>> +
>> /*
>> * When called from DP MST code:
>> * - old_conn_state will be NULL
>> @@ -3927,7 +3939,8 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>> {
>> drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
>>
>> - intel_ddi_enable_transcoder_func(encoder, crtc_state);
>> + if (!crtc_state->bigjoiner_slave)
>> + intel_ddi_enable_transcoder_func(encoder, crtc_state);
>>
>> intel_enable_pipe(crtc_state);
>>
>> @@ -4274,8 +4287,8 @@ static void bdw_get_trans_port_sync_config(struct intel_crtc_state *crtc_state)
>> crtc_state->sync_mode_slaves_mask);
>> }
>>
>> -void intel_ddi_get_config(struct intel_encoder *encoder,
>> - struct intel_crtc_state *pipe_config)
>> +static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
>> + struct intel_crtc_state *pipe_config)
>> {
>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->uapi.crtc);
>> @@ -4283,13 +4296,10 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>> 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)))
>> + temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
>> + if (!(temp & TRANS_DDI_FUNC_ENABLE))
>> return;
>>
>> - intel_dsc_get_config(encoder, pipe_config);
>> -
>> - temp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
>> if (temp & TRANS_DDI_PHSYNC)
>> flags |= DRM_MODE_FLAG_PHSYNC;
>> else
>> @@ -4397,6 +4407,29 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>> intel_dp->regs.dp_tp_ctl = TGL_DP_TP_CTL(transcoder);
>> intel_dp->regs.dp_tp_status = TGL_DP_TP_STATUS(transcoder);
>> }
>> +}
>> +
>> +void intel_ddi_get_config(struct intel_encoder *encoder,
>> + struct intel_crtc_state *pipe_config)
>> +{
>> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> + enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>> +
>> + /* XXX: DSI transcoder paranoia */
>> + if (WARN_ON(transcoder_is_dsi(cpu_transcoder)))
>> + return;
>> +
>> + intel_ddi_read_func_ctl(encoder, pipe_config);
>> + if (pipe_config->bigjoiner_slave) {
>> + /* read out pipe settings from master */
>> + enum transcoder save = pipe_config->cpu_transcoder;
>> +
>> + /* Our own transcoder needs to be disabled when reading it in intel_ddi_read_func_ctl() */
>> + WARN_ON(pipe_config->output_types);
>> + pipe_config->cpu_transcoder = (enum transcoder)pipe_config->bigjoiner_linked_crtc->pipe;
>> + intel_ddi_read_func_ctl(encoder, pipe_config);
>> + pipe_config->cpu_transcoder = save;
>> + }
>>
>> pipe_config->has_audio =
>> intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder);
>> @@ -4422,7 +4455,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>> dev_priv->vbt.edp.bpp = pipe_config->pipe_bpp;
>> }
>>
>> - intel_ddi_clock_get(encoder, pipe_config);
>> + if (!pipe_config->bigjoiner_slave)
>> + intel_ddi_clock_get(encoder, pipe_config);
>>
>> if (IS_GEN9_LP(dev_priv))
>> pipe_config->lane_lat_optim_mask =
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 41af7f1011fd..7223b6d916e7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -7024,6 +7024,45 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state)
>> intel_de_write(dev_priv, reg, val);
>> }
>>
>> +static void tgl_ddi_bigjoiner_pre_enable(struct intel_atomic_state *state,
>> + const struct intel_crtc_state *crtc_state)
>> +{
>> + struct intel_crtc *master = to_intel_crtc(crtc_state->uapi.crtc);
>> + struct intel_crtc_state *master_crtc_state;
>> + struct drm_connector_state *conn_state;
>> + struct drm_connector *conn;
>> + struct intel_encoder *encoder = NULL;
>> + int i;
>> +
>> + if (crtc_state->bigjoiner_slave)
>> + master = crtc_state->bigjoiner_linked_crtc;
>> +
>> + master_crtc_state = intel_atomic_get_new_crtc_state(state, master);
>> +
>> + for_each_new_connector_in_state(&state->base, conn, conn_state, i) {
>> + if (conn_state->crtc != &master->base)
>> + continue;
>> +
>> + encoder = to_intel_encoder(conn_state->best_encoder);
>> + break;
>> + }
>> +
>> + if (!crtc_state->bigjoiner_slave) {
>> + /* need to enable VDSC, which we skipped in pre-enable */
>> + intel_dsc_enable(encoder, crtc_state);
>> + } else {
>> + /*
>> + * Enable sequence steps 1-7 on bigjoiner master
>> + */
>> + intel_encoders_pre_pll_enable(state, master);
>> + intel_enable_shared_dpll(master_crtc_state);
>> + intel_encoders_pre_enable(state, master);
>> +
>> + /* and DSC on slave */
> Also here fec_enable never gets called for the slave since we call intel_encoders_pre_enable
> only for the master. So essentially on slave we are just doing dsc enable without FEC which is wrong.
>
> Regard
It is? I thought FEC was only required for the master when I was carefully reading the spec.
The DP encoder on the slave is completely unused, so no fec should be needed there.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-08-28 6:26 UTC|newest]
Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-15 22:42 [Intel-gfx] [PATCH v6 01/11] HAX to make DSC work on the icelake test system Manasi Navare
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode Manasi Navare
2020-08-17 7:26 ` Manna, Animesh
2020-09-03 17:49 ` Ville Syrjälä
2020-09-03 18:04 ` Navare, Manasi
2020-09-03 18:40 ` Ville Syrjälä
2020-09-07 12:35 ` Ville Syrjälä
2020-09-14 18:32 ` Navare, Manasi
2020-09-14 18:52 ` Ville Syrjälä
2020-09-21 21:01 ` Navare, Manasi
2020-09-22 10:19 ` Ville Syrjälä
2020-09-22 18:52 ` Navare, Manasi
2020-09-23 14:54 ` Navare, Manasi
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 03/11] drm/i915: Add hw.pipe_mode to allow bigjoiner pipe/transcoder split Manasi Navare
2020-08-10 12:38 ` Maarten Lankhorst
2020-08-17 7:32 ` Manna, Animesh
2020-09-03 17:54 ` Ville Syrjälä
2020-09-14 18:45 ` Navare, Manasi
2020-09-14 18:48 ` Ville Syrjälä
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 04/11] drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3 Manasi Navare
2020-08-10 12:40 ` Maarten Lankhorst
2020-08-21 9:41 ` Manna, Animesh
2020-08-21 21:51 ` Navare, Manasi
2020-09-07 11:20 ` Ville Syrjälä
2020-09-14 19:00 ` Navare, Manasi
2020-09-14 19:17 ` Ville Syrjälä
2020-09-14 19:38 ` Navare, Manasi
2020-09-14 19:47 ` Ville Syrjälä
2020-09-15 23:03 ` Navare, Manasi
2020-09-17 12:20 ` Ville Syrjälä
2020-09-23 5:46 ` Navare, Manasi
2020-09-23 9:57 ` Ville Syrjälä
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 05/11] drm/i915: Try to make bigjoiner work in atomic check Manasi Navare
2020-08-21 10:16 ` Manna, Animesh
2020-08-21 18:22 ` Navare, Manasi
2020-09-03 18:38 ` Ville Syrjälä
2020-09-23 22:58 ` Navare, Manasi
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 06/11] drm/i915: Enable big joiner support in enable and disable sequences Manasi Navare
2020-07-16 19:27 ` Manasi Navare
2020-08-10 12:45 ` Maarten Lankhorst
2020-08-10 23:04 ` Navare, Manasi
2020-07-16 21:12 ` [Intel-gfx] [PATCH v7 " Manasi Navare
2020-08-10 23:28 ` [Intel-gfx] [PATCH v8 " Manasi Navare
2020-08-27 23:35 ` Navare, Manasi
2020-08-28 6:26 ` Maarten Lankhorst [this message]
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 07/11] drm/i915: Make hardware readout work on i915 Manasi Navare
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 08/11] drm/i915: Link planes in a bigjoiner configuration, v3 Manasi Navare
2020-09-03 19:19 ` Ville Syrjälä
2020-09-14 19:14 ` Navare, Manasi
2020-09-14 19:20 ` Ville Syrjälä
2020-09-14 19:27 ` Navare, Manasi
2020-09-14 19:34 ` Ville Syrjälä
2020-09-14 19:45 ` Navare, Manasi
2020-09-14 20:05 ` Ville Syrjälä
2020-09-15 22:40 ` Navare, Manasi
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 09/11] drm/i915: Add bigjoiner aware plane clipping checks Manasi Navare
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 10/11] drm/i915: Add intel_update_bigjoiner handling Manasi Navare
2020-08-24 22:15 ` Navare, Manasi
2020-09-03 19:23 ` Ville Syrjälä
2020-09-14 19:21 ` Navare, Manasi
2020-09-21 21:18 ` Navare, Manasi
2020-09-22 10:27 ` Ville Syrjälä
2020-09-22 18:54 ` Navare, Manasi
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 11/11] drm/i915: Add debugfs dumping for bigjoiner, v3 Manasi Navare
2020-08-10 12:47 ` Maarten Lankhorst
2020-07-15 22:50 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,01/11] HAX to make DSC work on the icelake test system Patchwork
2020-07-15 22:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-07-15 23:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-07-16 5:48 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-07-16 21:53 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,01/11] HAX to make DSC work on the icelake test system (rev2) Patchwork
2020-07-16 21:54 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-07-16 22:14 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-07-17 1:04 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-08-10 23:47 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,01/11] HAX to make DSC work on the icelake test system (rev3) Patchwork
2020-08-10 23:48 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-08-11 0:09 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-08-11 18:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,01/11] HAX to make DSC work on the icelake test system (rev4) Patchwork
2020-08-11 18:35 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-08-11 18:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-08-11 20:15 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=16390edb-fca7-d2a7-d1ae-c943b3f3e0a9@linux.intel.com \
--to=maarten.lankhorst@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=manasi.d.navare@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).