Re: [Intel-gfx] [PATCH 02/10] drm/i915: Add counters in the drm_dp_aux struct for I2C NACKs and DEFERs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



To address previous feedback, I'll quote below and answer.

It would be nice if you could cite on the commit message the name of
the specification and the name of the test(s) that use it.

Done. For reference, in the Displayport Link CTS spec, tests 4.2.2.4 and 4.2.2.5 are the ones that use these, specifically.

Does it really need to be uint8_t? I see on patch 7 that you don't
really write this value to a place that only accepts uint8_t-sized
arguments, so I fear that if we get 256 NACKs or DEFERs we may end up
doing the wrong thing.

There are no compliance tests that are concerned with the number of native DEFERs or NACKs received, hence why they are not addressed here. There's no requirement for the size of this value and I selected uint8_t as the smallest reasonable size for it. It's only used to count the number of NACKs and DEFERs during compliance testing and it gets reset to 0 at the beginning of the test block where it's used in a later patch. It's unlikely that a case would occur during this compliance test where exactly 256 NACKs or DEFERs occur, but your point is well-taken. I'll make them uint32s and be done with it. I don't think 6 extra bytes is going to run the kernel out of memory. :)

Also, why don't we need to count the native NACKs and DEFERs?

There are no compliance tests that are concerned with the number of native DEFERs or NACKs received.

New patch will be here shortly.

-T

On 10/21/2014 10:10 AM, Paulo Zanoni wrote:
2014-10-09 12:38 GMT-03:00 Todd Previte <tprevite@xxxxxxxxx>:
These counters are used for Displayort complinace testing to detect error conditions
when executing certain compliance tests. Currently these are used in the EDID tests
to determine if the video mode needs to be set to the preferred mode or the failsafe
mode.

Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
I see that this patch and a few others in your series still have
unaddressed/unanswered review comments, given on the first time you
sent the patches. Please take a look at them.

Signed-off-by: Todd Previte <tprevite@xxxxxxxxx>
---
  drivers/gpu/drm/drm_dp_helper.c | 2 ++
  include/drm/drm_dp_helper.h     | 1 +
  2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 08e33b8..8353051 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -654,10 +654,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)

                 case DP_AUX_I2C_REPLY_NACK:
                         DRM_DEBUG_KMS("I2C nack\n");
+                       aux->i2c_nack_count++;
                         return -EREMOTEIO;

                 case DP_AUX_I2C_REPLY_DEFER:
                         DRM_DEBUG_KMS("I2C defer\n");
+                       aux->i2c_defer_count++;
                         usleep_range(400, 500);
                         continue;

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8edeed0..45f3ee8 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -551,6 +551,7 @@ struct drm_dp_aux {
         struct mutex hw_mutex;
         ssize_t (*transfer)(struct drm_dp_aux *aux,
                             struct drm_dp_aux_msg *msg);
+       uint8_t i2c_nack_count, i2c_defer_count;
  };

  ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
--
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx



_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux