| Message ID | 20240906125807.3960642-8-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Ultrajoiner basic functionality series | expand |
On Fri, Sep 06, 2024 at 06:28:00PM +0530, Ankit Nautiyal wrote: > From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > Adding sanity checks for primary and secondary bigjoiner/uncompressed > bitmasks, should make it easier to spot possible issues. > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 24 ++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index cdc7531311fc..6f098383479f 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3638,11 +3638,17 @@ static void enabled_bigjoiner_pipes(struct drm_i915_private *dev_priv, > } > } > > +static u8 expected_secondary_pipes(u8 primary_pipes) > +{ > + return primary_pipes << 1; > +} Extracting this should be a separate patch really since it was already in the code until the previous patch erroneously removed it. > + > static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, > u8 *primary_pipes, u8 *secondary_pipes) > { > u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes; > u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes; > + u8 uncompressed_joiner_pipes, bigjoiner_pipes; > > enabled_uncompressed_joiner_pipes(dev_priv, &primary_uncompressed_joiner_pipes, > &secondary_uncompressed_joiner_pipes); > @@ -3650,6 +3656,24 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, > enabled_bigjoiner_pipes(dev_priv, &primary_bigjoiner_pipes, > &secondary_bigjoiner_pipes); > > + uncompressed_joiner_pipes = primary_uncompressed_joiner_pipes | > + secondary_uncompressed_joiner_pipes; > + bigjoiner_pipes = primary_bigjoiner_pipes | secondary_bigjoiner_pipes; > + > + drm_WARN(&dev_priv->drm, (uncompressed_joiner_pipes & bigjoiner_pipes) != 0, > + "Uncomressed joiner pipes(%x) and bigjoiner pipes(%x) can't intersect\n", > + uncompressed_joiner_pipes, bigjoiner_pipes); Please use 0x%x for printing hex numbers Side note: maybe we should switch to the standard %#x instead? Though we'd need to double check whether prink() follows the idiotic C standard or not: printf("%#x", 1) -> 0x1, for any non-zero value bs. printf("%#x", 0) -> 0 (sigh) If printk() behaves the same way then I want to see %#x anywhere (pretty sure we do use it in a few places already though). > + drm_WARN(&dev_priv->drm, secondary_bigjoiner_pipes != > + expected_secondary_pipes(primary_bigjoiner_pipes), > + "Wrong secondary bigjoiner pipes(expected %x, current %x)\n", > + expected_secondary_pipes(primary_bigjoiner_pipes), > + secondary_bigjoiner_pipes); > + drm_WARN(&dev_priv->drm, secondary_uncompressed_joiner_pipes != > + expected_secondary_pipes(primary_uncompressed_joiner_pipes), > + "Wrong secondary uncompressed joiner pipes(expected %x, current %x)\n", > + expected_secondary_pipes(primary_uncompressed_joiner_pipes), > + secondary_uncompressed_joiner_pipes); > + > *primary_pipes = 0; > *secondary_pipes = 0; > > -- > 2.45.2
On 9/6/2024 9:09 PM, Ville Syrjälä wrote: > On Fri, Sep 06, 2024 at 06:28:00PM +0530, Ankit Nautiyal wrote: >> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> >> >> Adding sanity checks for primary and secondary bigjoiner/uncompressed >> bitmasks, should make it easier to spot possible issues. >> >> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> >> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 24 ++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index cdc7531311fc..6f098383479f 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -3638,11 +3638,17 @@ static void enabled_bigjoiner_pipes(struct drm_i915_private *dev_priv, >> } >> } >> >> +static u8 expected_secondary_pipes(u8 primary_pipes) >> +{ >> + return primary_pipes << 1; >> +} > Extracting this should be a separate patch really since it > was already in the code until the previous patch erroneously > removed it. Sure, will move this to another patch. > >> + >> static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, >> u8 *primary_pipes, u8 *secondary_pipes) >> { >> u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes; >> u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes; >> + u8 uncompressed_joiner_pipes, bigjoiner_pipes; >> >> enabled_uncompressed_joiner_pipes(dev_priv, &primary_uncompressed_joiner_pipes, >> &secondary_uncompressed_joiner_pipes); >> @@ -3650,6 +3656,24 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, >> enabled_bigjoiner_pipes(dev_priv, &primary_bigjoiner_pipes, >> &secondary_bigjoiner_pipes); >> >> + uncompressed_joiner_pipes = primary_uncompressed_joiner_pipes | >> + secondary_uncompressed_joiner_pipes; >> + bigjoiner_pipes = primary_bigjoiner_pipes | secondary_bigjoiner_pipes; >> + >> + drm_WARN(&dev_priv->drm, (uncompressed_joiner_pipes & bigjoiner_pipes) != 0, >> + "Uncomressed joiner pipes(%x) and bigjoiner pipes(%x) can't intersect\n", >> + uncompressed_joiner_pipes, bigjoiner_pipes); > Please use 0x%x for printing hex numbers > > Side note: maybe we should switch to the standard %#x instead? > Though we'd need to double check whether prink() follows the > idiotic C standard or not: > printf("%#x", 1) -> 0x1, for any non-zero value > bs. > printf("%#x", 0) -> 0 (sigh) > > If printk() behaves the same way then I want to see %#x anywhere > (pretty sure we do use it in a few places already though). I will check how %#x behaves with 0 with printk, accordingly will use 0x%x or %x#x, (we do use it in some places). Regards, Ankit > > >> + drm_WARN(&dev_priv->drm, secondary_bigjoiner_pipes != >> + expected_secondary_pipes(primary_bigjoiner_pipes), >> + "Wrong secondary bigjoiner pipes(expected %x, current %x)\n", >> + expected_secondary_pipes(primary_bigjoiner_pipes), >> + secondary_bigjoiner_pipes); >> + drm_WARN(&dev_priv->drm, secondary_uncompressed_joiner_pipes != >> + expected_secondary_pipes(primary_uncompressed_joiner_pipes), >> + "Wrong secondary uncompressed joiner pipes(expected %x, current %x)\n", >> + expected_secondary_pipes(primary_uncompressed_joiner_pipes), >> + secondary_uncompressed_joiner_pipes); >> + >> *primary_pipes = 0; >> *secondary_pipes = 0; >> >> -- >> 2.45.2
On 9/9/2024 2:01 PM, Nautiyal, Ankit K wrote: > > On 9/6/2024 9:09 PM, Ville Syrjälä wrote: >> On Fri, Sep 06, 2024 at 06:28:00PM +0530, Ankit Nautiyal wrote: >>> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> >>> >>> Adding sanity checks for primary and secondary bigjoiner/uncompressed >>> bitmasks, should make it easier to spot possible issues. >>> >>> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> >>> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> >>> --- >>> drivers/gpu/drm/i915/display/intel_display.c | 24 >>> ++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c >>> b/drivers/gpu/drm/i915/display/intel_display.c >>> index cdc7531311fc..6f098383479f 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_display.c >>> +++ b/drivers/gpu/drm/i915/display/intel_display.c >>> @@ -3638,11 +3638,17 @@ static void enabled_bigjoiner_pipes(struct >>> drm_i915_private *dev_priv, >>> } >>> } >>> +static u8 expected_secondary_pipes(u8 primary_pipes) >>> +{ >>> + return primary_pipes << 1; >>> +} >> Extracting this should be a separate patch really since it >> was already in the code until the previous patch erroneously >> removed it. > > Sure, will move this to another patch. > > >> >>> + >>> static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, >>> u8 *primary_pipes, u8 *secondary_pipes) >>> { >>> u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes; >>> u8 secondary_uncompressed_joiner_pipes, >>> secondary_bigjoiner_pipes; >>> + u8 uncompressed_joiner_pipes, bigjoiner_pipes; >>> enabled_uncompressed_joiner_pipes(dev_priv, >>> &primary_uncompressed_joiner_pipes, >>> &secondary_uncompressed_joiner_pipes); >>> @@ -3650,6 +3656,24 @@ static void enabled_joiner_pipes(struct >>> drm_i915_private *dev_priv, >>> enabled_bigjoiner_pipes(dev_priv, &primary_bigjoiner_pipes, >>> &secondary_bigjoiner_pipes); >>> + uncompressed_joiner_pipes = primary_uncompressed_joiner_pipes | >>> + secondary_uncompressed_joiner_pipes; >>> + bigjoiner_pipes = primary_bigjoiner_pipes | >>> secondary_bigjoiner_pipes; >>> + >>> + drm_WARN(&dev_priv->drm, (uncompressed_joiner_pipes & >>> bigjoiner_pipes) != 0, >>> + "Uncomressed joiner pipes(%x) and bigjoiner pipes(%x) >>> can't intersect\n", >>> + uncompressed_joiner_pipes, bigjoiner_pipes); >> Please use 0x%x for printing hex numbers >> >> Side note: maybe we should switch to the standard %#x instead? >> Though we'd need to double check whether prink() follows the >> idiotic C standard or not: >> printf("%#x", 1) -> 0x1, for any non-zero value >> bs. >> printf("%#x", 0) -> 0 (sigh) >> >> If printk() behaves the same way then I want to see %#x anywhere >> (pretty sure we do use it in a few places already though). > > I will check how %#x behaves with 0 with printk, accordingly will use > 0x%x or %x#x, (we do use it in some places). Just checked, with printk(), %x#x with 0, prints 0x0 unlike printf() which prints 0, so %x#x it is then. > > Regards, > > Ankit > >> >> >>> + drm_WARN(&dev_priv->drm, secondary_bigjoiner_pipes != >>> + expected_secondary_pipes(primary_bigjoiner_pipes), >>> + "Wrong secondary bigjoiner pipes(expected %x, current %x)\n", >>> + expected_secondary_pipes(primary_bigjoiner_pipes), >>> + secondary_bigjoiner_pipes); >>> + drm_WARN(&dev_priv->drm, secondary_uncompressed_joiner_pipes != >>> + expected_secondary_pipes(primary_uncompressed_joiner_pipes), >>> + "Wrong secondary uncompressed joiner pipes(expected %x, >>> current %x)\n", >>> + expected_secondary_pipes(primary_uncompressed_joiner_pipes), >>> + secondary_uncompressed_joiner_pipes); >>> + >>> *primary_pipes = 0; >>> *secondary_pipes = 0; >>> -- >>> 2.45.2
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index cdc7531311fc..6f098383479f 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3638,11 +3638,17 @@ static void enabled_bigjoiner_pipes(struct drm_i915_private *dev_priv, } } +static u8 expected_secondary_pipes(u8 primary_pipes) +{ + return primary_pipes << 1; +} + static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, u8 *primary_pipes, u8 *secondary_pipes) { u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes; u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes; + u8 uncompressed_joiner_pipes, bigjoiner_pipes; enabled_uncompressed_joiner_pipes(dev_priv, &primary_uncompressed_joiner_pipes, &secondary_uncompressed_joiner_pipes); @@ -3650,6 +3656,24 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv, enabled_bigjoiner_pipes(dev_priv, &primary_bigjoiner_pipes, &secondary_bigjoiner_pipes); + uncompressed_joiner_pipes = primary_uncompressed_joiner_pipes | + secondary_uncompressed_joiner_pipes; + bigjoiner_pipes = primary_bigjoiner_pipes | secondary_bigjoiner_pipes; + + drm_WARN(&dev_priv->drm, (uncompressed_joiner_pipes & bigjoiner_pipes) != 0, + "Uncomressed joiner pipes(%x) and bigjoiner pipes(%x) can't intersect\n", + uncompressed_joiner_pipes, bigjoiner_pipes); + drm_WARN(&dev_priv->drm, secondary_bigjoiner_pipes != + expected_secondary_pipes(primary_bigjoiner_pipes), + "Wrong secondary bigjoiner pipes(expected %x, current %x)\n", + expected_secondary_pipes(primary_bigjoiner_pipes), + secondary_bigjoiner_pipes); + drm_WARN(&dev_priv->drm, secondary_uncompressed_joiner_pipes != + expected_secondary_pipes(primary_uncompressed_joiner_pipes), + "Wrong secondary uncompressed joiner pipes(expected %x, current %x)\n", + expected_secondary_pipes(primary_uncompressed_joiner_pipes), + secondary_uncompressed_joiner_pipes); + *primary_pipes = 0; *secondary_pipes = 0;