| Message ID | 20240926072638.3689367-9-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Ultrajoiner basic functionality series | expand |
On Thu, Sep 26, 2024 at 12:56:31PM +0530, Ankit Nautiyal wrote: > Pass the current pipe into enabled_joiner_pipes(), and let it figure out > the proper bitmasks for us. Since the enabled_joiner_pipes now gets the > primary and secondary pipe wrt a given pipe, the helpers > to get primary pipe and secondary pipes are no longer required. > > v2: > -Simplify helper get_joiner_primary_pipes. (Ville) > -Nuke get_joiner_secondary_pipes. (Ville) > -Add more drm_WARNs final primary/secondary pipes. (Ville) > v3: > -Drop ultrajoiner stuff and add it in subsequent patches. (Ville) > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 89 ++++++++++---------- > 1 file changed, 44 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 2d6260c3bca5..ea259b142786 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3663,7 +3663,15 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes) > return expected_secondary_pipes(bigjoiner_primary_pipes, 2); > } > > +static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes) > +{ > + primary_pipes &= GENMASK(pipe, 0); > + > + return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0; > +} > + > static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, > + enum pipe pipe, > u8 *primary_pipes, u8 *secondary_pipes) > { > struct intel_display *display = to_intel_display(&dev_priv->drm); > @@ -3703,45 +3711,38 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, > expected_uncompressed_joiner_secondary_pipes(primary_uncompressed_joiner_pipes), > secondary_uncompressed_joiner_pipes); > > - *primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes; > - > - *secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes; > - > - /* Joiner pipes should always be consecutive primary and secondary */ > - drm_WARN(display->drm, *secondary_pipes != *primary_pipes << 1, > - "Joiner misconfigured (primary pipes %#x, secondary pipes %#x)\n", > - *primary_pipes, *secondary_pipes); > -} > - > -static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes) > -{ > - if ((secondary_pipes & BIT(pipe)) == 0) > - return pipe; > - > - /* ignore everything above our pipe */ > - primary_pipes &= ~GENMASK(7, pipe); > - > - /* highest remaining bit should be our primary pipe */ > - return fls(primary_pipes) - 1; > -} > - > -static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes) > -{ > - enum pipe primary_pipe, next_primary_pipe; > - > - primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes); > - > - if ((primary_pipes & BIT(primary_pipe)) == 0) > - return 0; > - > - /* ignore our primary pipe and everything below it */ > - primary_pipes &= ~GENMASK(primary_pipe, 0); > - /* make sure a high bit is set for the ffs() */ > - primary_pipes |= BIT(7); > - /* lowest remaining bit should be the next primary pipe */ > - next_primary_pipe = ffs(primary_pipes) - 1; > - > - return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe); > + *primary_pipes = 0; > + *secondary_pipes = 0; > + > + if (uncompressed_joiner_pipes & BIT(pipe)) { > + *primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes); > + *secondary_pipes = secondary_uncompressed_joiner_pipes & > + expected_uncompressed_joiner_secondary_pipes(*primary_pipes); > + > + drm_WARN(display->drm, > + expected_uncompressed_joiner_secondary_pipes(*primary_pipes) != > + *secondary_pipes, > + "Wrong uncompressed joiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n", > + *primary_pipes, > + expected_uncompressed_joiner_secondary_pipes(*primary_pipes), > + *secondary_pipes); > + return; > + } > + > + if (bigjoiner_pipes & BIT(pipe)) { > + *primary_pipes = get_joiner_primary_pipe(pipe, primary_bigjoiner_pipes); > + *secondary_pipes = secondary_bigjoiner_pipes & > + expected_bigjoiner_secondary_pipes(*primary_pipes); > + > + drm_WARN(display->drm, > + expected_bigjoiner_secondary_pipes(*primary_pipes) != > + *secondary_pipes, > + "Wrong bigjoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n", > + *primary_pipes, > + expected_bigjoiner_secondary_pipes(*primary_pipes), > + *secondary_pipes); > + return; > + } > } > > static u8 hsw_panel_transcoders(struct drm_i915_private *i915) > @@ -3813,10 +3814,10 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc) > enabled_transcoders |= BIT(cpu_transcoder); > > /* joiner secondary -> consider the primary pipe's transcoder as well */ > - enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes); > + enabled_joiner_pipes(dev_priv, crtc->pipe, &primary_pipes, &secondary_pipes); > if (secondary_pipes & BIT(crtc->pipe)) { > cpu_transcoder = (enum transcoder) > - get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes); > + ffs(get_joiner_primary_pipe(crtc->pipe, primary_pipes)) - 1; The get_joiner_primary_pipe() shouldn't be needed here since enabled_joiner_pipes() guarantees that only one bit is set. With that Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder)) > enabled_transcoders |= BIT(cpu_transcoder); > } > @@ -3950,14 +3951,12 @@ static void intel_joiner_get_config(struct intel_crtc_state *crtc_state) > u8 primary_pipes, secondary_pipes; > enum pipe pipe = crtc->pipe; > > - enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes); > + enabled_joiner_pipes(i915, pipe, &primary_pipes, &secondary_pipes); > > if (((primary_pipes | secondary_pipes) & BIT(pipe)) == 0) > return; > > - crtc_state->joiner_pipes = > - BIT(get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes)) | > - get_joiner_secondary_pipes(pipe, primary_pipes, secondary_pipes); > + crtc_state->joiner_pipes = primary_pipes | secondary_pipes; > } > > static bool hsw_get_pipe_config(struct intel_crtc *crtc, > -- > 2.45.2
On 9/26/2024 4:49 PM, Ville Syrjälä wrote: > On Thu, Sep 26, 2024 at 12:56:31PM +0530, Ankit Nautiyal wrote: >> Pass the current pipe into enabled_joiner_pipes(), and let it figure out >> the proper bitmasks for us. Since the enabled_joiner_pipes now gets the >> primary and secondary pipe wrt a given pipe, the helpers >> to get primary pipe and secondary pipes are no longer required. >> >> v2: >> -Simplify helper get_joiner_primary_pipes. (Ville) >> -Nuke get_joiner_secondary_pipes. (Ville) >> -Add more drm_WARNs final primary/secondary pipes. (Ville) >> v3: >> -Drop ultrajoiner stuff and add it in subsequent patches. (Ville) >> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 89 ++++++++++---------- >> 1 file changed, 44 insertions(+), 45 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index 2d6260c3bca5..ea259b142786 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -3663,7 +3663,15 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes) >> return expected_secondary_pipes(bigjoiner_primary_pipes, 2); >> } >> >> +static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes) >> +{ >> + primary_pipes &= GENMASK(pipe, 0); >> + >> + return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0; >> +} >> + >> static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, >> + enum pipe pipe, >> u8 *primary_pipes, u8 *secondary_pipes) >> { >> struct intel_display *display = to_intel_display(&dev_priv->drm); >> @@ -3703,45 +3711,38 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, >> expected_uncompressed_joiner_secondary_pipes(primary_uncompressed_joiner_pipes), >> secondary_uncompressed_joiner_pipes); >> >> - *primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes; >> - >> - *secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes; >> - >> - /* Joiner pipes should always be consecutive primary and secondary */ >> - drm_WARN(display->drm, *secondary_pipes != *primary_pipes << 1, >> - "Joiner misconfigured (primary pipes %#x, secondary pipes %#x)\n", >> - *primary_pipes, *secondary_pipes); >> -} >> - >> -static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes) >> -{ >> - if ((secondary_pipes & BIT(pipe)) == 0) >> - return pipe; >> - >> - /* ignore everything above our pipe */ >> - primary_pipes &= ~GENMASK(7, pipe); >> - >> - /* highest remaining bit should be our primary pipe */ >> - return fls(primary_pipes) - 1; >> -} >> - >> -static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes) >> -{ >> - enum pipe primary_pipe, next_primary_pipe; >> - >> - primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes); >> - >> - if ((primary_pipes & BIT(primary_pipe)) == 0) >> - return 0; >> - >> - /* ignore our primary pipe and everything below it */ >> - primary_pipes &= ~GENMASK(primary_pipe, 0); >> - /* make sure a high bit is set for the ffs() */ >> - primary_pipes |= BIT(7); >> - /* lowest remaining bit should be the next primary pipe */ >> - next_primary_pipe = ffs(primary_pipes) - 1; >> - >> - return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe); >> + *primary_pipes = 0; >> + *secondary_pipes = 0; >> + >> + if (uncompressed_joiner_pipes & BIT(pipe)) { >> + *primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes); >> + *secondary_pipes = secondary_uncompressed_joiner_pipes & >> + expected_uncompressed_joiner_secondary_pipes(*primary_pipes); >> + >> + drm_WARN(display->drm, >> + expected_uncompressed_joiner_secondary_pipes(*primary_pipes) != >> + *secondary_pipes, >> + "Wrong uncompressed joiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n", >> + *primary_pipes, >> + expected_uncompressed_joiner_secondary_pipes(*primary_pipes), >> + *secondary_pipes); >> + return; >> + } >> + >> + if (bigjoiner_pipes & BIT(pipe)) { >> + *primary_pipes = get_joiner_primary_pipe(pipe, primary_bigjoiner_pipes); >> + *secondary_pipes = secondary_bigjoiner_pipes & >> + expected_bigjoiner_secondary_pipes(*primary_pipes); >> + >> + drm_WARN(display->drm, >> + expected_bigjoiner_secondary_pipes(*primary_pipes) != >> + *secondary_pipes, >> + "Wrong bigjoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n", >> + *primary_pipes, >> + expected_bigjoiner_secondary_pipes(*primary_pipes), >> + *secondary_pipes); >> + return; >> + } >> } >> >> static u8 hsw_panel_transcoders(struct drm_i915_private *i915) >> @@ -3813,10 +3814,10 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc) >> enabled_transcoders |= BIT(cpu_transcoder); >> >> /* joiner secondary -> consider the primary pipe's transcoder as well */ >> - enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes); >> + enabled_joiner_pipes(dev_priv, crtc->pipe, &primary_pipes, &secondary_pipes); >> if (secondary_pipes & BIT(crtc->pipe)) { >> cpu_transcoder = (enum transcoder) >> - get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes); >> + ffs(get_joiner_primary_pipe(crtc->pipe, primary_pipes)) - 1; > The get_joiner_primary_pipe() shouldn't be needed here since > enabled_joiner_pipes() guarantees that only one bit is set. I agree. Additionally, I was considering changing the input variable name from `primary_pipes` to `primary_pipe` in enabled_joiner_pipes for clarity. Does that make sense? Regards, Ankit > > With that > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder)) >> enabled_transcoders |= BIT(cpu_transcoder); >> } >> @@ -3950,14 +3951,12 @@ static void intel_joiner_get_config(struct intel_crtc_state *crtc_state) >> u8 primary_pipes, secondary_pipes; >> enum pipe pipe = crtc->pipe; >> >> - enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes); >> + enabled_joiner_pipes(i915, pipe, &primary_pipes, &secondary_pipes); >> >> if (((primary_pipes | secondary_pipes) & BIT(pipe)) == 0) >> return; >> >> - crtc_state->joiner_pipes = >> - BIT(get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes)) | >> - get_joiner_secondary_pipes(pipe, primary_pipes, secondary_pipes); >> + crtc_state->joiner_pipes = primary_pipes | secondary_pipes; >> } >> >> static bool hsw_get_pipe_config(struct intel_crtc *crtc, >> -- >> 2.45.2
On Thu, Sep 26, 2024 at 06:45:14PM +0530, Nautiyal, Ankit K wrote: > > On 9/26/2024 4:49 PM, Ville Syrjälä wrote: > > On Thu, Sep 26, 2024 at 12:56:31PM +0530, Ankit Nautiyal wrote: > >> Pass the current pipe into enabled_joiner_pipes(), and let it figure out > >> the proper bitmasks for us. Since the enabled_joiner_pipes now gets the > >> primary and secondary pipe wrt a given pipe, the helpers > >> to get primary pipe and secondary pipes are no longer required. > >> > >> v2: > >> -Simplify helper get_joiner_primary_pipes. (Ville) > >> -Nuke get_joiner_secondary_pipes. (Ville) > >> -Add more drm_WARNs final primary/secondary pipes. (Ville) > >> v3: > >> -Drop ultrajoiner stuff and add it in subsequent patches. (Ville) > >> > >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > >> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_display.c | 89 ++++++++++---------- > >> 1 file changed, 44 insertions(+), 45 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > >> index 2d6260c3bca5..ea259b142786 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display.c > >> +++ b/drivers/gpu/drm/i915/display/intel_display.c > >> @@ -3663,7 +3663,15 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes) > >> return expected_secondary_pipes(bigjoiner_primary_pipes, 2); > >> } > >> > >> +static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes) > >> +{ > >> + primary_pipes &= GENMASK(pipe, 0); > >> + > >> + return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0; > >> +} > >> + > >> static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, > >> + enum pipe pipe, > >> u8 *primary_pipes, u8 *secondary_pipes) > >> { > >> struct intel_display *display = to_intel_display(&dev_priv->drm); > >> @@ -3703,45 +3711,38 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, > >> expected_uncompressed_joiner_secondary_pipes(primary_uncompressed_joiner_pipes), > >> secondary_uncompressed_joiner_pipes); > >> > >> - *primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes; > >> - > >> - *secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes; > >> - > >> - /* Joiner pipes should always be consecutive primary and secondary */ > >> - drm_WARN(display->drm, *secondary_pipes != *primary_pipes << 1, > >> - "Joiner misconfigured (primary pipes %#x, secondary pipes %#x)\n", > >> - *primary_pipes, *secondary_pipes); > >> -} > >> - > >> -static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes) > >> -{ > >> - if ((secondary_pipes & BIT(pipe)) == 0) > >> - return pipe; > >> - > >> - /* ignore everything above our pipe */ > >> - primary_pipes &= ~GENMASK(7, pipe); > >> - > >> - /* highest remaining bit should be our primary pipe */ > >> - return fls(primary_pipes) - 1; > >> -} > >> - > >> -static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes) > >> -{ > >> - enum pipe primary_pipe, next_primary_pipe; > >> - > >> - primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes); > >> - > >> - if ((primary_pipes & BIT(primary_pipe)) == 0) > >> - return 0; > >> - > >> - /* ignore our primary pipe and everything below it */ > >> - primary_pipes &= ~GENMASK(primary_pipe, 0); > >> - /* make sure a high bit is set for the ffs() */ > >> - primary_pipes |= BIT(7); > >> - /* lowest remaining bit should be the next primary pipe */ > >> - next_primary_pipe = ffs(primary_pipes) - 1; > >> - > >> - return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe); > >> + *primary_pipes = 0; > >> + *secondary_pipes = 0; > >> + > >> + if (uncompressed_joiner_pipes & BIT(pipe)) { > >> + *primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes); > >> + *secondary_pipes = secondary_uncompressed_joiner_pipes & > >> + expected_uncompressed_joiner_secondary_pipes(*primary_pipes); > >> + > >> + drm_WARN(display->drm, > >> + expected_uncompressed_joiner_secondary_pipes(*primary_pipes) != > >> + *secondary_pipes, > >> + "Wrong uncompressed joiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n", > >> + *primary_pipes, > >> + expected_uncompressed_joiner_secondary_pipes(*primary_pipes), > >> + *secondary_pipes); > >> + return; > >> + } > >> + > >> + if (bigjoiner_pipes & BIT(pipe)) { > >> + *primary_pipes = get_joiner_primary_pipe(pipe, primary_bigjoiner_pipes); > >> + *secondary_pipes = secondary_bigjoiner_pipes & > >> + expected_bigjoiner_secondary_pipes(*primary_pipes); > >> + > >> + drm_WARN(display->drm, > >> + expected_bigjoiner_secondary_pipes(*primary_pipes) != > >> + *secondary_pipes, > >> + "Wrong bigjoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n", > >> + *primary_pipes, > >> + expected_bigjoiner_secondary_pipes(*primary_pipes), > >> + *secondary_pipes); > >> + return; > >> + } > >> } > >> > >> static u8 hsw_panel_transcoders(struct drm_i915_private *i915) > >> @@ -3813,10 +3814,10 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc) > >> enabled_transcoders |= BIT(cpu_transcoder); > >> > >> /* joiner secondary -> consider the primary pipe's transcoder as well */ > >> - enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes); > >> + enabled_joiner_pipes(dev_priv, crtc->pipe, &primary_pipes, &secondary_pipes); > >> if (secondary_pipes & BIT(crtc->pipe)) { > >> cpu_transcoder = (enum transcoder) > >> - get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes); > >> + ffs(get_joiner_primary_pipe(crtc->pipe, primary_pipes)) - 1; > > The get_joiner_primary_pipe() shouldn't be needed here since > > enabled_joiner_pipes() guarantees that only one bit is set. > > > I agree. Additionally, I was considering changing the input variable > name from `primary_pipes` to `primary_pipe` in enabled_joiner_pipes for > clarity. Does that make sense? Probably a good idea. It'll be a bit weird to have it as a bitmask, but feels like returning just a single enum pipe could make things more complicated. So yeah, 'u8 *primary_pipe' seems perhaps the best compromise.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 2d6260c3bca5..ea259b142786 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3663,7 +3663,15 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes) return expected_secondary_pipes(bigjoiner_primary_pipes, 2); } +static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes) +{ + primary_pipes &= GENMASK(pipe, 0); + + return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0; +} + static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, + enum pipe pipe, u8 *primary_pipes, u8 *secondary_pipes) { struct intel_display *display = to_intel_display(&dev_priv->drm); @@ -3703,45 +3711,38 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, expected_uncompressed_joiner_secondary_pipes(primary_uncompressed_joiner_pipes), secondary_uncompressed_joiner_pipes); - *primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes; - - *secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes; - - /* Joiner pipes should always be consecutive primary and secondary */ - drm_WARN(display->drm, *secondary_pipes != *primary_pipes << 1, - "Joiner misconfigured (primary pipes %#x, secondary pipes %#x)\n", - *primary_pipes, *secondary_pipes); -} - -static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes) -{ - if ((secondary_pipes & BIT(pipe)) == 0) - return pipe; - - /* ignore everything above our pipe */ - primary_pipes &= ~GENMASK(7, pipe); - - /* highest remaining bit should be our primary pipe */ - return fls(primary_pipes) - 1; -} - -static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes) -{ - enum pipe primary_pipe, next_primary_pipe; - - primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes); - - if ((primary_pipes & BIT(primary_pipe)) == 0) - return 0; - - /* ignore our primary pipe and everything below it */ - primary_pipes &= ~GENMASK(primary_pipe, 0); - /* make sure a high bit is set for the ffs() */ - primary_pipes |= BIT(7); - /* lowest remaining bit should be the next primary pipe */ - next_primary_pipe = ffs(primary_pipes) - 1; - - return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe); + *primary_pipes = 0; + *secondary_pipes = 0; + + if (uncompressed_joiner_pipes & BIT(pipe)) { + *primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes); + *secondary_pipes = secondary_uncompressed_joiner_pipes & + expected_uncompressed_joiner_secondary_pipes(*primary_pipes); + + drm_WARN(display->drm, + expected_uncompressed_joiner_secondary_pipes(*primary_pipes) != + *secondary_pipes, + "Wrong uncompressed joiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n", + *primary_pipes, + expected_uncompressed_joiner_secondary_pipes(*primary_pipes), + *secondary_pipes); + return; + } + + if (bigjoiner_pipes & BIT(pipe)) { + *primary_pipes = get_joiner_primary_pipe(pipe, primary_bigjoiner_pipes); + *secondary_pipes = secondary_bigjoiner_pipes & + expected_bigjoiner_secondary_pipes(*primary_pipes); + + drm_WARN(display->drm, + expected_bigjoiner_secondary_pipes(*primary_pipes) != + *secondary_pipes, + "Wrong bigjoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n", + *primary_pipes, + expected_bigjoiner_secondary_pipes(*primary_pipes), + *secondary_pipes); + return; + } } static u8 hsw_panel_transcoders(struct drm_i915_private *i915) @@ -3813,10 +3814,10 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc) enabled_transcoders |= BIT(cpu_transcoder); /* joiner secondary -> consider the primary pipe's transcoder as well */ - enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes); + enabled_joiner_pipes(dev_priv, crtc->pipe, &primary_pipes, &secondary_pipes); if (secondary_pipes & BIT(crtc->pipe)) { cpu_transcoder = (enum transcoder) - get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes); + ffs(get_joiner_primary_pipe(crtc->pipe, primary_pipes)) - 1; if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder)) enabled_transcoders |= BIT(cpu_transcoder); } @@ -3950,14 +3951,12 @@ static void intel_joiner_get_config(struct intel_crtc_state *crtc_state) u8 primary_pipes, secondary_pipes; enum pipe pipe = crtc->pipe; - enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes); + enabled_joiner_pipes(i915, pipe, &primary_pipes, &secondary_pipes); if (((primary_pipes | secondary_pipes) & BIT(pipe)) == 0) return; - crtc_state->joiner_pipes = - BIT(get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes)) | - get_joiner_secondary_pipes(pipe, primary_pipes, secondary_pipes); + crtc_state->joiner_pipes = primary_pipes | secondary_pipes; } static bool hsw_get_pipe_config(struct intel_crtc *crtc,
Pass the current pipe into enabled_joiner_pipes(), and let it figure out the proper bitmasks for us. Since the enabled_joiner_pipes now gets the primary and secondary pipe wrt a given pipe, the helpers to get primary pipe and secondary pipes are no longer required. v2: -Simplify helper get_joiner_primary_pipes. (Ville) -Nuke get_joiner_secondary_pipes. (Ville) -Add more drm_WARNs final primary/secondary pipes. (Ville) v3: -Drop ultrajoiner stuff and add it in subsequent patches. (Ville) Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 89 ++++++++++---------- 1 file changed, 44 insertions(+), 45 deletions(-)