From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 11/16] drm/i915: Introduce intel_master_crtc()
Date: Wed, 15 Sep 2021 15:21:14 +0300 [thread overview]
Message-ID: <YUHlOrV+bHC1Vx5O@intel.com> (raw)
In-Reply-To: <875yv2yw4f.fsf@intel.com>
On Wed, Sep 15, 2021 at 01:24:32PM +0300, Jani Nikula wrote:
> On Mon, 13 Sep 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Add a helper to determine the master crtc for bigjoiner usage.
> > Also name the variables consistently.
>
> I think this would be a good time to to rename bigjoiner {master, slave}
> to {primary, secondary}. The change is coming to bspec too though it's
> not there yet.
Argh. "Primary" is really an overloaded terms so not a fan of that
rename. Anyway, I'll wait until the spec catches up because
otherewise I'll have to remap in my brain which is just pointless
cognitive load.
>
> BR,
> Jani.
>
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 6 +--
> > drivers/gpu/drm/i915/display/intel_display.c | 57 +++++++++++---------
> > 2 files changed, 33 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 4f7420dda8b5..4fbffce501dc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -2882,12 +2882,10 @@ static void intel_ddi_post_disable(struct intel_atomic_state *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 =
> > + struct intel_crtc *slave_crtc =
> > old_crtc_state->bigjoiner_linked_crtc;
> > const struct intel_crtc_state *old_slave_crtc_state =
> > - intel_atomic_get_old_crtc_state(state, slave);
> > + intel_atomic_get_old_crtc_state(state, slave_crtc);
> >
> > intel_crtc_vblank_off(old_slave_crtc_state);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 24214e6249a9..a5450ac9e2d0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -227,6 +227,14 @@ is_trans_port_sync_mode(const struct intel_crtc_state *crtc_state)
> > is_trans_port_sync_slave(crtc_state);
> > }
> >
> > +static struct intel_crtc *intel_master_crtc(const struct intel_crtc_state *crtc_state)
> > +{
> > + if (crtc_state->bigjoiner_slave)
> > + return crtc_state->bigjoiner_linked_crtc;
> > + else
> > + return to_intel_crtc(crtc_state->uapi.crtc);
> > +}
> > +
> > static bool pipe_scanline_is_moving(struct drm_i915_private *dev_priv,
> > enum pipe pipe)
> > {
> > @@ -2978,21 +2986,19 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state)
> > static void icl_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 drm_i915_private *dev_priv = to_i915(master->base.dev);
> > + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > struct intel_crtc_state *master_crtc_state;
> > + struct intel_crtc *master_crtc;
> > 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);
> > + master_crtc = intel_master_crtc(crtc_state);
> > + master_crtc_state = intel_atomic_get_new_crtc_state(state, master_crtc);
> >
> > for_each_new_connector_in_state(&state->base, conn, conn_state, i) {
> > - if (conn_state->crtc != &master->base)
> > + if (conn_state->crtc != &master_crtc->base)
> > continue;
> >
> > encoder = to_intel_encoder(conn_state->best_encoder);
> > @@ -3006,10 +3012,10 @@ static void icl_ddi_bigjoiner_pre_enable(struct intel_atomic_state *state,
> > /*
> > * Enable sequence steps 1-7 on bigjoiner master
> > */
> > - intel_encoders_pre_pll_enable(state, master);
> > + intel_encoders_pre_pll_enable(state, master_crtc);
> > if (master_crtc_state->shared_dpll)
> > intel_enable_shared_dpll(master_crtc_state);
> > - intel_encoders_pre_enable(state, master);
> > + intel_encoders_pre_enable(state, master_crtc);
> >
> > /* and DSC on slave */
> > intel_dsc_enable(NULL, crtc_state);
> > @@ -8462,7 +8468,7 @@ verify_crtc_state(struct intel_crtc *crtc,
> > struct intel_encoder *encoder;
> > struct intel_crtc_state *pipe_config = old_crtc_state;
> > struct drm_atomic_state *state = old_crtc_state->uapi.state;
> > - struct intel_crtc *master = crtc;
> > + struct intel_crtc *master_crtc;
> >
> > __drm_atomic_helper_crtc_destroy_state(&old_crtc_state->uapi);
> > intel_crtc_free_hw_state(old_crtc_state);
> > @@ -8490,10 +8496,9 @@ verify_crtc_state(struct intel_crtc *crtc,
> > "(expected %i, found %i)\n",
> > new_crtc_state->hw.active, crtc->active);
> >
> > - if (new_crtc_state->bigjoiner_slave)
> > - master = new_crtc_state->bigjoiner_linked_crtc;
> > + master_crtc = intel_master_crtc(new_crtc_state);
> >
> > - for_each_encoder_on_crtc(dev, &master->base, encoder) {
> > + for_each_encoder_on_crtc(dev, &master_crtc->base, encoder) {
> > enum pipe pipe;
> > bool active;
> >
> > @@ -8503,7 +8508,7 @@ verify_crtc_state(struct intel_crtc *crtc,
> > encoder->base.base.id, active,
> > new_crtc_state->hw.active);
> >
> > - I915_STATE_WARN(active && master->pipe != pipe,
> > + I915_STATE_WARN(active && master_crtc->pipe != pipe,
> > "Encoder connected to wrong pipe %c\n",
> > pipe_name(pipe));
> >
> > @@ -9192,13 +9197,13 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
> > struct intel_crtc_state *new_crtc_state)
> > {
> > struct intel_crtc_state *slave_crtc_state, *master_crtc_state;
> > - struct intel_crtc *slave, *master;
> > + struct intel_crtc *slave_crtc, *master_crtc;
> >
> > /* slave being enabled, is master is still claiming this crtc? */
> > if (old_crtc_state->bigjoiner_slave) {
> > - slave = crtc;
> > - master = old_crtc_state->bigjoiner_linked_crtc;
> > - master_crtc_state = intel_atomic_get_new_crtc_state(state, master);
> > + slave_crtc = crtc;
> > + master_crtc = old_crtc_state->bigjoiner_linked_crtc;
> > + master_crtc_state = intel_atomic_get_new_crtc_state(state, master_crtc);
> > if (!master_crtc_state || !intel_crtc_needs_modeset(master_crtc_state))
> > goto claimed;
> > }
> > @@ -9206,17 +9211,17 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
> > if (!new_crtc_state->bigjoiner)
> > return 0;
> >
> > - slave = intel_dsc_get_bigjoiner_secondary(crtc);
> > - if (!slave) {
> > + slave_crtc = intel_dsc_get_bigjoiner_secondary(crtc);
> > + if (!slave_crtc) {
> > DRM_DEBUG_KMS("[CRTC:%d:%s] Big joiner configuration requires "
> > "CRTC + 1 to be used, doesn't exist\n",
> > crtc->base.base.id, crtc->base.name);
> > return -EINVAL;
> > }
> >
> > - new_crtc_state->bigjoiner_linked_crtc = slave;
> > - slave_crtc_state = intel_atomic_get_crtc_state(&state->base, slave);
> > - master = crtc;
> > + new_crtc_state->bigjoiner_linked_crtc = slave_crtc;
> > + slave_crtc_state = intel_atomic_get_crtc_state(&state->base, slave_crtc);
> > + master_crtc = crtc;
> > if (IS_ERR(slave_crtc_state))
> > return PTR_ERR(slave_crtc_state);
> >
> > @@ -9225,15 +9230,15 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
> > goto claimed;
> >
> > DRM_DEBUG_KMS("[CRTC:%d:%s] Used as slave for big joiner\n",
> > - slave->base.base.id, slave->base.name);
> > + slave_crtc->base.base.id, slave_crtc->base.name);
> >
> > return copy_bigjoiner_crtc_state(slave_crtc_state, new_crtc_state);
> >
> > claimed:
> > DRM_DEBUG_KMS("[CRTC:%d:%s] Slave is enabled as normal CRTC, but "
> > "[CRTC:%d:%s] claiming this CRTC for bigjoiner.\n",
> > - slave->base.base.id, slave->base.name,
> > - master->base.base.id, master->base.name);
> > + slave_crtc->base.base.id, slave_crtc->base.name,
> > + master_crtc->base.base.id, master_crtc->base.name);
> > return -EINVAL;
> > }
>
> --
> Jani Nikula, Intel Open Source Graphics Center
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2021-09-15 12:21 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-13 14:44 [Intel-gfx] [PATCH 00/16] drm/i915: Fix bigjoiner state readout Ville Syrjala
2021-09-13 14:44 ` [Intel-gfx] [PATCH 01/16] Revert "drm/i915/display: Disable audio, DRRS and PSR before planes" Ville Syrjala
2021-09-13 16:28 ` Souza, Jose
2021-09-14 8:20 ` Ville Syrjälä
2021-09-14 23:24 ` Souza, Jose
2021-09-15 0:00 ` Souza, Jose
2021-09-15 12:30 ` Ville Syrjälä
2021-09-15 20:19 ` Souza, Jose
2021-09-16 13:21 ` Ville Syrjälä
2021-09-17 0:14 ` Souza, Jose
2021-09-13 14:44 ` [Intel-gfx] [PATCH 02/16] drm/i915: Disable all planes before modesetting any pipes Ville Syrjala
2021-09-20 7:52 ` Navare, Manasi
2021-09-21 12:49 ` Ville Syrjälä
2021-09-27 11:20 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 03/16] drm/i915: Extract intel_dp_use_bigjoiner() Ville Syrjala
2021-09-15 10:12 ` Jani Nikula
2021-09-15 15:39 ` Ville Syrjälä
2021-09-21 11:10 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 04/16] drm/i915: Flatten hsw_crtc_compute_clock() Ville Syrjala
2021-09-15 10:13 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 05/16] drm/i915: s/pipe/transcoder/ when dealing with PIPECONF/TRANSCONF Ville Syrjala
2021-09-15 10:16 ` Jani Nikula
2021-09-15 13:00 ` Ville Syrjälä
2021-09-15 13:17 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 06/16] drm/i915: Introduce with_intel_display_power_if_enabled() Ville Syrjala
2021-09-15 13:22 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 07/16] drm/i915: Adjust intel_dsc_power_domain() calling convention Ville Syrjala
2021-09-15 10:19 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 08/16] drm/i915: Extract hsw_panel_transcoders() Ville Syrjala
2021-09-15 10:20 ` Jani Nikula
2021-09-13 14:44 ` [Intel-gfx] [PATCH 09/16] drm/i915: Pimp HSW+ transcoder state readout Ville Syrjala
2021-09-21 11:46 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 10/16] drm/i915: Configure TRANSCONF just the once with bigjoiner Ville Syrjala
2021-09-21 11:51 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 11/16] drm/i915: Introduce intel_master_crtc() Ville Syrjala
2021-09-15 10:24 ` Jani Nikula
2021-09-15 12:21 ` Ville Syrjälä [this message]
2021-10-21 23:27 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 12/16] drm/i915: Simplify intel_crtc_copy_uapi_to_hw_state_nomodeset() Ville Syrjala
2021-09-27 11:27 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 13/16] drm/i915: Split PPS write from DSC enable Ville Syrjala
2021-09-27 12:01 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 14/16] drm/i915: Perform correct cpu_transcoder readout for bigjoiner Ville Syrjala
2021-10-20 20:35 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 15/16] drm/i915: Reduce bigjoiner special casing Ville Syrjala
2021-10-20 23:52 ` Navare, Manasi
2021-09-13 14:44 ` [Intel-gfx] [PATCH 16/16] drm/i915: Nuke PIPE_CONFIG_QUIRK_BIGJOINER_SLAVE Ville Syrjala
2021-10-20 23:53 ` Navare, Manasi
2021-09-13 16:05 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix bigjoiner state readout Patchwork
2021-09-13 16:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-09-13 18:39 ` [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=YUHlOrV+bHC1Vx5O@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@linux.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