Re: [RFC 1/7] drm/dp: get/set phy compliance pattern

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

 





On 11/18/2019 9:34 AM, Manasi Navare wrote:
On Fri, Nov 15, 2019 at 08:55:43PM +0530, Animesh Manna wrote:
During phy complaince auto test mode source need to read
             ^^ typo

Please also send this patch to dri-devel M-L

Thanks Manasi for review. I have sent to dri-devel also after adding suggested changes.

Regards,
Animesh


requested test pattern from sink through DPCD. After processing
the request source need to set the pattern. So set/get method
added in drm layer as it is DP protocol.

Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx>
---
  drivers/gpu/drm/drm_dp_helper.c | 97 +++++++++++++++++++++++++++++++++
  include/drm/drm_dp_helper.h     | 34 +++++++++++-
  2 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 2c7870aef469..7d0f9986a95a 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1371,3 +1371,100 @@ int drm_dp_dsc_sink_supported_input_bpcs(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_S
  	return num_bpc;
  }
  EXPORT_SYMBOL(drm_dp_dsc_sink_supported_input_bpcs);
+
+/**
+ * drm_dp_get_phy_test_pattern() - get the requested pattern from the sink.
+ * @aux: DisplayPort AUX channel
+ * @data: DP phy compliance test parameters.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_get_phy_test_pattern(struct drm_dp_aux *aux,
+				struct drm_dp_phy_test_params *data)
+{
+	int err;
+	u8 rate, lanes;
+
+	err = drm_dp_dpcd_readb(aux, DP_DPCD_REV, &data->dp_rev);
+	if (err < 0)
+		return err;
+
+	err = drm_dp_dpcd_readb(aux, DP_TEST_LINK_RATE, &rate);
+	if (err < 0)
+		return err;
+	data->link_rate = drm_dp_bw_code_to_link_rate(rate);
+
+	err = drm_dp_dpcd_readb(aux, DP_TEST_LANE_COUNT, &lanes);
+	if (err < 0)
+		return err;
+	data->num_lanes = lanes & DP_MAX_LANE_COUNT_MASK;
+
+	if (lanes & DP_ENHANCED_FRAME_CAP)
+		data->enahanced_frame_cap = true;
+
+	err = drm_dp_dpcd_readb(aux, DP_PHY_TEST_PATTERN, &data->phy_pattern);
+	if (err < 0)
+		return err;
+
+	switch (data->phy_pattern) {
+	case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
+		err = drm_dp_dpcd_read(aux, DP_TEST_80BIT_CUSTOM_PATTERN_7_0,
+				       &data->custom80, 10);
+		if (err < 0)
+			return err;
+
+		break;
+	case DP_PHY_TEST_PATTERN_CP2520:
+		err = drm_dp_dpcd_read(aux, DP_TEST_HBR2_SCRAMBLER_RESET,
+				       &data->hbr2_reset, 2);
+		if (err < 0)
+			return err;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_get_phy_test_pattern);
+
+/**
+ * drm_dp_set_phy_test_pattern() - set the pattern to the sink.
+ * @aux: DisplayPort AUX channel
+ * @data: DP phy compliance test parameters.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_set_phy_test_pattern(struct drm_dp_aux *aux,
+				struct drm_dp_phy_test_params *data)
+{
+	int err, i;
+	u8 link_config[2];
+	u8 test_pattern;
+
+	link_config[0] = drm_dp_link_rate_to_bw_code(data->link_rate);
+	link_config[1] = data->num_lanes;
+	if (data->enahanced_frame_cap)
+		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
+	err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, link_config, 2);
+	if (err < 0)
+		return err;
+
+	test_pattern = data->phy_pattern;
+	if (data->dp_rev < 0x12) {
+		test_pattern = (test_pattern << 2) &
+			       DP_LINK_QUAL_PATTERN_11_MASK;
+		err = drm_dp_dpcd_writeb(aux, DP_TRAINING_PATTERN_SET,
+					 test_pattern);
+		if (err < 0)
+			return err;
+	} else {
+		for (i = 0; i < data->num_lanes; i++) {
+			err = drm_dp_dpcd_writeb(aux,
+						 DP_LINK_QUAL_LANE0_SET + i,
+						 test_pattern);
+			if (err < 0)
+				return err;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_set_phy_test_pattern);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 51ecb5112ef8..628e484318e4 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -699,7 +699,16 @@
  # define DP_TEST_CRC_SUPPORTED		    (1 << 5)
  # define DP_TEST_COUNT_MASK		    0xf
-#define DP_TEST_PHY_PATTERN 0x248
+#define DP_PHY_TEST_PATTERN                 0x248
+# define DP_PHY_TEST_PATTERN_SEL_MASK       0x7
+# define DP_PHY_TEST_PATTERN_NONE           0x0
+# define DP_PHY_TEST_PATTERN_D10_2          0x1
+# define DP_PHY_TEST_PATTERN_ERROR_COUNT    0x2
+# define DP_PHY_TEST_PATTERN_PRBS7          0x3
+# define DP_PHY_TEST_PATTERN_80BIT_CUSTOM   0x4
+# define DP_PHY_TEST_PATTERN_CP2520         0x5
+
+#define DP_TEST_HBR2_SCRAMBLER_RESET        0x24A
  #define DP_TEST_80BIT_CUSTOM_PATTERN_7_0    0x250
  #define	DP_TEST_80BIT_CUSTOM_PATTERN_15_8   0x251
  #define	DP_TEST_80BIT_CUSTOM_PATTERN_23_16  0x252
@@ -1568,4 +1577,27 @@ static inline void drm_dp_cec_unset_edid(struct drm_dp_aux *aux)
#endif +/**
+ * struct drm_dp_phy_test_params - DP Phy Compliance parameters
+ * @link: Link information.
+ * @phy_pattern: DP Phy test pattern from DPCD 0x248 (sink)
+ * @hb2_reset: DP HBR2_COMPLIANCE_SCRAMBLER_RESET from DCPD
+ *            0x24A and 0x24B (sink)
+ * @custom80: DP Test_80BIT_CUSTOM_PATTERN from DPCDs 0x250
+ *               through 0x259.
+ */
+struct drm_dp_phy_test_params {
+	u8 dp_rev;
Why is dp_rev part of phy_test_params since its a generic dpcd rev and
not specific to phy testing.
Is it possible to just read the dpcd rev into a local variable in the _set_phy_params()
function and use it to set the phy patterns accordingly instead of having it as part
of this struct?
having dp_rev as part of phy test params struct is misleading since it is not phy compliance parameter.

Actually even better would be to have dpcd as an input argument for set function like
what we have in drm_dp_helper : const u8 dpcd[DP_RECEIVER_CAP_SIZE] as an argument
and then just use  dpcd[DP_DPCD_REV]

That should work IMO.

+	u8 phy_pattern;
+	u8 hbr2_reset[2];
+	u8 custom80[10];
+	u32 link_rate;
+	u32 num_lanes;
Is there a reason why link_rate is not int and lane count is not u8
as we have for link layer compliance test link rate and lane count in i915?

If no specific reason then I would prefer having it as same data type as
the link layer link rate and lane count because eventually at some point
we would want to combine both structs and move link layer structs to drm as well

Other than the above changes it looks good to me.

Manasi

+	bool enahanced_frame_cap;
+};
+
+int drm_dp_get_phy_test_pattern(struct drm_dp_aux *aux,
+				struct drm_dp_phy_test_params *data);
+int drm_dp_set_phy_test_pattern(struct drm_dp_aux *aux,
+				struct drm_dp_phy_test_params *data);
  #endif /* _DRM_DP_HELPER_H_ */
--
2.22.0


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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux