On Wed, Oct 04, 2023 at 01:25:04PM +0300, Mika Kahola wrote:
> Every know and then we receive the following error when running
> for example IGT test kms_flip.
>
> [drm] *ERROR* PHY G Read 0d80 failed after 3 retries.
> [drm] *ERROR* PHY G Write 0d81 failed after 3 retries.
>
> Since the error is sporadic in nature, the patch proposes
> to reset the message bus after every successful or unsuccessful
> read or write operation. However, testing revealed that this
> alone is not sufficient method an additiona delay is also
> introduces anything from 200us to 300us. This delay is experimental
> value and has no specification to back it up.
have you tried the delays without the bus_reset?
have you talked to hw architects about this?
I wonder if we should add the delay inside the bus_reset itself?
although the bit 15 clear check should be enough by itself and
it doesn't look like it is a hw/fw reset involved to justify the
extra delay.
well, at least some /* FIXME: */ or /* XXX: */ comments is
desired along with the messages if we are going with this
hack without understanding why...
>
> Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index abd607b564f1..a71b8a29d6b0 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -220,9 +220,12 @@ static u8 __intel_cx0_read(struct drm_i915_private *i915, enum port port,
> /* 3 tries is assumed to be enough to read successfully */
> for (i = 0; i < 3; i++) {
> status = __intel_cx0_read_once(i915, port, lane, addr);
> + intel_cx0_bus_reset(i915, port, lane);
>
> if (status >= 0)
> return status;
> +
> + usleep_range(200, 300);
> }
>
> drm_err_once(&i915->drm, "PHY %c Read %04x failed after %d retries.\n",
> @@ -299,9 +302,12 @@ static void __intel_cx0_write(struct drm_i915_private *i915, enum port port,
> /* 3 tries is assumed to be enough to write successfully */
> for (i = 0; i < 3; i++) {
> status = __intel_cx0_write_once(i915, port, lane, addr, data, committed);
> + intel_cx0_bus_reset(i915, port, lane);
>
> if (status == 0)
> return;
> +
> + usleep_range(200, 300);
> }
>
> drm_err_once(&i915->drm,
> --
> 2.34.1
>
