[Intel-gfx] [PATCH] drm/i915/dsb: Increase log level if DSB engine gets busy

[Lucas De Marchi]

On Thu, Dec 26, 2019 at 9:34 PM Sharma, Swati2 <swati2.sharma at intel.com> wrote:
>
> On 27-Dec-19 2:39 AM, Lucas De Marchi wrote:
> > On Wed, Dec 25, 2019 at 10:07 AM Swati Sharma <swati2.sharma at intel.com> wrote:
> >>
> >> Increase the log level if DSB engine gets busy. If dsb engine
> >> is busy, it should be an error condition to indicate there might be
> >> some difficulty with the hardware.
> >>
> >> If DSB engine gets busy, load luts will fail and as per current
> >> driver design if one instance of DSB engine gets busy, we are not
> >> allocating the other instance. So, increase the log level to indicate there
> >> could be an issue with driver/hardware.
> >>
> >> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_dsb.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> >> index ada006a690df..6f67b5dfa128 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> >> @@ -52,7 +52,7 @@ static inline bool intel_dsb_enable_engine(struct intel_dsb *dsb)
> >>
> >>          dsb_ctrl = I915_READ(DSB_CTRL(pipe, dsb->id));
> >>          if (DSB_STATUS & dsb_ctrl) {
> >> -               DRM_DEBUG_KMS("DSB engine is busy.\n");
> >> +               DRM_ERROR("DSB engine is busy.\n");
> >
> > are we seeing this? Isn't it a dbg message because in this case we
> > would fallback to direct mmio?
> We are seeing this issue and is already under debug.
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7630/shard-tglb5/igt@kms_available_modes_crc@available_mode_test_crc.html

I'm not sure what benefit it has in just raising the log level here.
Btw, the only caller of this function already has a pointless check
for "engine is busy". You may want to remove that too if you follow this route.

I think it would be more interesting to root cause the issue:  if DSB
*may* get busy, then we'd better leave this as a dbg and fallback on a
chain of
MMIO writes or a delayed commit or failed its initialization early. If
this is really unexpected, why are we hitting this?

As why DSB is busy: is it because we had a previous dsb_commit() that
is keeping DSB busy so we can't have another subsequent commit? Why
didn't we fail the call to dsb_init() early Since it's not possible to
have unpaired dsb_init() / dsb_commit(), if that is the cause then if
DSB is busy on dsb_commit, it should as well be busy on dsb_init().

Lucas De Marchi

>
> <7> [303.727858] [drm:intel_dsb_commit [i915]] DSB engine is busy.
> <7> [303.727975] [drm:icl_load_luts [i915]] DSB engine is busy.
> >
> > Lucas De Marchi
> >
> >>                  return false;
> >>          }
> >>
> >> @@ -72,7 +72,7 @@ static inline bool intel_dsb_disable_engine(struct intel_dsb *dsb)
> >>
> >>          dsb_ctrl = I915_READ(DSB_CTRL(pipe, dsb->id));
> >>          if (DSB_STATUS & dsb_ctrl) {
> >> -               DRM_DEBUG_KMS("DSB engine is busy.\n");
> >> +               DRM_ERROR("DSB engine is busy.\n");
> >>                  return false;
> >>          }
> >>
> >> --
> >> 2.24.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> >
>
>
> --
> ~Swati Sharma



-- 
Lucas De Marchi