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

2020-01-07 Thread Lucas De Marchi
On Thu, Dec 26, 2019 at 9:34 PM Sharma, Swati2  wrote:
>
> On 27-Dec-19 2:39 AM, Lucas De Marchi wrote:
> > On Wed, Dec 25, 2019 at 10:07 AM Swati Sharma  
> > 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 
> >> ---
> >>   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@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> >
>
>
> --
> ~Swati Sharma



-- 
Lucas De Marchi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2019-12-26 Thread Sharma, Swati2

On 27-Dec-19 2:39 AM, Lucas De Marchi wrote:

On Wed, Dec 25, 2019 at 10:07 AM Swati Sharma  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 
---
  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

<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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx







--
~Swati Sharma
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2019-12-26 Thread Lucas De Marchi
On Wed, Dec 25, 2019 at 10:07 AM Swati Sharma  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 
> ---
>  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?

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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Lucas De Marchi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2019-12-25 Thread Swati Sharma
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 
---
 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");
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx