* [RFC 0/7] DP Phy compliace auto test
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
Driver changes mainly to process the request coming from Test equipment
as short pulse hpd interrupt to change link-pattern/v-swing/pre-emphasis
Complete auto test suite takes much lesser time than manual run.
Overall design:
--------------
Automate test request will come to source device as HDP short pulse
interrupt from test scope.
Read DPCD 0x201, Check for bit 1 for automated test request.
If set continue and read DPCD 0x218.
Check for bit 3 for phy test pattern, If set continue.
Get the requested test pattern through DPCD 0x248.
Compute requested voltage swing level and pre-emphasis level
from DPCD 0x206 and 0x207
Set signal level through vswing programming sequence.
Write DDI_COMP_CTL and DDI_COMP_PATx as per requested pattern.
Configure the link and write the new test pattern through DPCD.
High level patch description.
-----------------------------
patch 1: drm level api added to get/set test pattern as per vesa
DP spec. This maybe useful for other driver so added in drm layer.
patch 2: vswing/preemphasis adjustment calculation is needed during
phy compliance request processing along with existing link training
process, so moved the same function in intel_dp.c.
patch 3: Parse the test scope request regarding rquested test pattern,
vswing level, preemphasis level.
patch 4: Notify testapp through uevent.
patch 5: Register difnition of DP compliance register added.
patch 6: Funcion added to update the pattern in source side.
patch 7: This patch os mainly processing the request.
Currently through prototyping patch able to run DP compliance where
vswing, preemphasis and test pattern is changing fine but complete
test is under process. As per feedback redesigned the code. Could not test
due to unavailability of test scope, so sending as RFC again to get design
feedback.
Animesh Manna (7):
drm/dp: get/set phy compliance pattern.
drm/i915/dp: Move vswing/pre-emphasis adjustment calculation
drm/i915/dp: Preparation for DP phy compliance auto test.
drm/i915/dp: Notify testapp using uevent and debugfs entry
drm/i915/dp: Register definition for DP compliance register.
drm/i915/dp: Update the pattern as per request
drm/i915/dp: Program vswing, pre-emphasis, test-pattern
drivers/gpu/drm/drm_dp_helper.c | 97 +++++++++
drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++
drivers/gpu/drm/i915/display/intel_display.c | 5 +
.../drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 194 +++++++++++++++++-
drivers/gpu/drm/i915/display/intel_dp.h | 5 +
.../drm/i915/display/intel_dp_link_training.c | 32 ---
drivers/gpu/drm/i915/i915_debugfs.c | 10 +
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_reg.h | 20 ++
include/drm/drm_dp_helper.h | 34 ++-
11 files changed, 379 insertions(+), 35 deletions(-)
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* [Intel-gfx] [RFC 0/7] DP Phy compliace auto test
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
Driver changes mainly to process the request coming from Test equipment
as short pulse hpd interrupt to change link-pattern/v-swing/pre-emphasis
Complete auto test suite takes much lesser time than manual run.
Overall design:
--------------
Automate test request will come to source device as HDP short pulse
interrupt from test scope.
Read DPCD 0x201, Check for bit 1 for automated test request.
If set continue and read DPCD 0x218.
Check for bit 3 for phy test pattern, If set continue.
Get the requested test pattern through DPCD 0x248.
Compute requested voltage swing level and pre-emphasis level
from DPCD 0x206 and 0x207
Set signal level through vswing programming sequence.
Write DDI_COMP_CTL and DDI_COMP_PATx as per requested pattern.
Configure the link and write the new test pattern through DPCD.
High level patch description.
-----------------------------
patch 1: drm level api added to get/set test pattern as per vesa
DP spec. This maybe useful for other driver so added in drm layer.
patch 2: vswing/preemphasis adjustment calculation is needed during
phy compliance request processing along with existing link training
process, so moved the same function in intel_dp.c.
patch 3: Parse the test scope request regarding rquested test pattern,
vswing level, preemphasis level.
patch 4: Notify testapp through uevent.
patch 5: Register difnition of DP compliance register added.
patch 6: Funcion added to update the pattern in source side.
patch 7: This patch os mainly processing the request.
Currently through prototyping patch able to run DP compliance where
vswing, preemphasis and test pattern is changing fine but complete
test is under process. As per feedback redesigned the code. Could not test
due to unavailability of test scope, so sending as RFC again to get design
feedback.
Animesh Manna (7):
drm/dp: get/set phy compliance pattern.
drm/i915/dp: Move vswing/pre-emphasis adjustment calculation
drm/i915/dp: Preparation for DP phy compliance auto test.
drm/i915/dp: Notify testapp using uevent and debugfs entry
drm/i915/dp: Register definition for DP compliance register.
drm/i915/dp: Update the pattern as per request
drm/i915/dp: Program vswing, pre-emphasis, test-pattern
drivers/gpu/drm/drm_dp_helper.c | 97 +++++++++
drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++
drivers/gpu/drm/i915/display/intel_display.c | 5 +
.../drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 194 +++++++++++++++++-
drivers/gpu/drm/i915/display/intel_dp.h | 5 +
.../drm/i915/display/intel_dp_link_training.c | 32 ---
drivers/gpu/drm/i915/i915_debugfs.c | 10 +
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_reg.h | 20 ++
include/drm/drm_dp_helper.h | 34 ++-
11 files changed, 379 insertions(+), 35 deletions(-)
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC 1/7] drm/dp: get/set phy compliance pattern
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
During phy complaince auto test mode source need to read
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@intel.com>
---
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;
+ u8 phy_pattern;
+ u8 hbr2_reset[2];
+ u8 custom80[10];
+ u32 link_rate;
+ u32 num_lanes;
+ 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Intel-gfx] [RFC 1/7] drm/dp: get/set phy compliance pattern
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
During phy complaince auto test mode source need to read
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@intel.com>
---
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;
+ u8 phy_pattern;
+ u8 hbr2_reset[2];
+ u8 custom80[10];
+ u32 link_rate;
+ u32 num_lanes;
+ 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC 2/7] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
vswing/pre-emphasis adjustment calculation is needed in processing
of auto phy compliance request other than link training, so moved
the same function in intel_dp.c.
No functional change.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 32 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dp.h | 3 ++
.../drm/i915/display/intel_dp_link_training.c | 32 -------------------
3 files changed, 35 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 3123958e2081..cc8e0d1a73af 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4062,6 +4062,38 @@ ivb_cpu_edp_signal_levels(u8 train_set)
}
}
+void
+intel_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 *link_status)
+{
+ u8 v = 0;
+ u8 p = 0;
+ int lane;
+ u8 voltage_max;
+ u8 preemph_max;
+
+ for (lane = 0; lane < intel_dp->lane_count; lane++) {
+ u8 this_v = drm_dp_get_adjust_request_voltage(link_status, lane);
+ u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane);
+
+ if (this_v > v)
+ v = this_v;
+ if (this_p > p)
+ p = this_p;
+ }
+
+ voltage_max = intel_dp_voltage_max(intel_dp);
+ if (v >= voltage_max)
+ v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
+
+ preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
+ if (p >= preemph_max)
+ p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
+
+ for (lane = 0; lane < 4; lane++)
+ intel_dp->train_set[lane] = v | p;
+}
+
void
intel_dp_set_signal_levels(struct intel_dp *intel_dp)
{
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 3da166054788..0d0cb692f701 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -91,6 +91,9 @@ void
intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
u8 dp_train_pat);
void
+intel_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 *link_status);
+void
intel_dp_set_signal_levels(struct intel_dp *intel_dp);
void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
u8
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 2a1130dd1ad0..1e38584e7d56 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -34,38 +34,6 @@ intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE])
link_status[3], link_status[4], link_status[5]);
}
-static void
-intel_get_adjust_train(struct intel_dp *intel_dp,
- const u8 link_status[DP_LINK_STATUS_SIZE])
-{
- u8 v = 0;
- u8 p = 0;
- int lane;
- u8 voltage_max;
- u8 preemph_max;
-
- for (lane = 0; lane < intel_dp->lane_count; lane++) {
- u8 this_v = drm_dp_get_adjust_request_voltage(link_status, lane);
- u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane);
-
- if (this_v > v)
- v = this_v;
- if (this_p > p)
- p = this_p;
- }
-
- voltage_max = intel_dp_voltage_max(intel_dp);
- if (v >= voltage_max)
- v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
-
- preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
- if (p >= preemph_max)
- p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
-
- for (lane = 0; lane < 4; lane++)
- intel_dp->train_set[lane] = v | p;
-}
-
static bool
intel_dp_set_link_train(struct intel_dp *intel_dp,
u8 dp_train_pat)
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Intel-gfx] [RFC 2/7] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
vswing/pre-emphasis adjustment calculation is needed in processing
of auto phy compliance request other than link training, so moved
the same function in intel_dp.c.
No functional change.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 32 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dp.h | 3 ++
.../drm/i915/display/intel_dp_link_training.c | 32 -------------------
3 files changed, 35 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 3123958e2081..cc8e0d1a73af 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4062,6 +4062,38 @@ ivb_cpu_edp_signal_levels(u8 train_set)
}
}
+void
+intel_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 *link_status)
+{
+ u8 v = 0;
+ u8 p = 0;
+ int lane;
+ u8 voltage_max;
+ u8 preemph_max;
+
+ for (lane = 0; lane < intel_dp->lane_count; lane++) {
+ u8 this_v = drm_dp_get_adjust_request_voltage(link_status, lane);
+ u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane);
+
+ if (this_v > v)
+ v = this_v;
+ if (this_p > p)
+ p = this_p;
+ }
+
+ voltage_max = intel_dp_voltage_max(intel_dp);
+ if (v >= voltage_max)
+ v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
+
+ preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
+ if (p >= preemph_max)
+ p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
+
+ for (lane = 0; lane < 4; lane++)
+ intel_dp->train_set[lane] = v | p;
+}
+
void
intel_dp_set_signal_levels(struct intel_dp *intel_dp)
{
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 3da166054788..0d0cb692f701 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -91,6 +91,9 @@ void
intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
u8 dp_train_pat);
void
+intel_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 *link_status);
+void
intel_dp_set_signal_levels(struct intel_dp *intel_dp);
void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
u8
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 2a1130dd1ad0..1e38584e7d56 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -34,38 +34,6 @@ intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE])
link_status[3], link_status[4], link_status[5]);
}
-static void
-intel_get_adjust_train(struct intel_dp *intel_dp,
- const u8 link_status[DP_LINK_STATUS_SIZE])
-{
- u8 v = 0;
- u8 p = 0;
- int lane;
- u8 voltage_max;
- u8 preemph_max;
-
- for (lane = 0; lane < intel_dp->lane_count; lane++) {
- u8 this_v = drm_dp_get_adjust_request_voltage(link_status, lane);
- u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane);
-
- if (this_v > v)
- v = this_v;
- if (this_p > p)
- p = this_p;
- }
-
- voltage_max = intel_dp_voltage_max(intel_dp);
- if (v >= voltage_max)
- v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
-
- preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
- if (p >= preemph_max)
- p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
-
- for (lane = 0; lane < 4; lane++)
- intel_dp->train_set[lane] = v | p;
-}
-
static bool
intel_dp_set_link_train(struct intel_dp *intel_dp,
u8 dp_train_pat)
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC 3/7] drm/i915/dp: Preparation for DP phy compliance auto test
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
During DP phy compliance auto test mode, sink will request
combination of different test pattern with differnt level of
vswing, pre-emphasis. Function added to prepare for it.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
.../drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 24 +++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 83ea04149b77..c17866f126c9 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1205,6 +1205,7 @@ struct intel_dp_compliance_data {
u8 video_pattern;
u16 hdisplay, vdisplay;
u8 bpc;
+ struct drm_dp_phy_test_params phytest;
};
struct intel_dp_compliance {
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index cc8e0d1a73af..338d3744c5d5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4936,9 +4936,33 @@ static u8 intel_dp_autotest_edid(struct intel_dp *intel_dp)
return test_result;
}
+static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
+{
+ struct drm_dp_phy_test_params *data =
+ &intel_dp->compliance.test_data.phytest;
+
+ if (!drm_dp_get_phy_test_pattern(&intel_dp->aux, data)) {
+ DRM_DEBUG_KMS("DP Phy Test pattern AUX read failure\n");
+ return DP_TEST_NAK;
+ }
+
+ /*
+ * link_mst is set to false to avoid executing mst related code
+ * during compliance testing.
+ */
+ intel_dp->link_mst = false;
+
+ return DP_TEST_ACK;
+}
+
static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
{
u8 test_result = DP_TEST_NAK;
+
+ test_result = intel_dp_prepare_phytest(intel_dp);
+ if (test_result != DP_TEST_ACK)
+ DRM_ERROR("Phy test preparation failed\n");
+
return test_result;
}
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Intel-gfx] [RFC 3/7] drm/i915/dp: Preparation for DP phy compliance auto test
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
During DP phy compliance auto test mode, sink will request
combination of different test pattern with differnt level of
vswing, pre-emphasis. Function added to prepare for it.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
.../drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 24 +++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 83ea04149b77..c17866f126c9 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1205,6 +1205,7 @@ struct intel_dp_compliance_data {
u8 video_pattern;
u16 hdisplay, vdisplay;
u8 bpc;
+ struct drm_dp_phy_test_params phytest;
};
struct intel_dp_compliance {
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index cc8e0d1a73af..338d3744c5d5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4936,9 +4936,33 @@ static u8 intel_dp_autotest_edid(struct intel_dp *intel_dp)
return test_result;
}
+static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
+{
+ struct drm_dp_phy_test_params *data =
+ &intel_dp->compliance.test_data.phytest;
+
+ if (!drm_dp_get_phy_test_pattern(&intel_dp->aux, data)) {
+ DRM_DEBUG_KMS("DP Phy Test pattern AUX read failure\n");
+ return DP_TEST_NAK;
+ }
+
+ /*
+ * link_mst is set to false to avoid executing mst related code
+ * during compliance testing.
+ */
+ intel_dp->link_mst = false;
+
+ return DP_TEST_ACK;
+}
+
static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
{
u8 test_result = DP_TEST_NAK;
+
+ test_result = intel_dp_prepare_phytest(intel_dp);
+ if (test_result != DP_TEST_ACK)
+ DRM_ERROR("Phy test preparation failed\n");
+
return test_result;
}
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
To align with link compliance design existing intel_dp_compliance
tool will be used to get the phy request in userspace through uevent.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 6 ++++--
drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 338d3744c5d5..a2b860cf3b93 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5288,8 +5288,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
intel_psr_short_pulse(intel_dp);
- if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
- DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
+ if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING ||
+ intel_dp->compliance.test_type ==
+ DP_TEST_LINK_PHY_TEST_PATTERN) {
+ DRM_DEBUG_KMS("Compliance Test requested\n");
/* Send a Hotplug Uevent to userspace to start modeset */
drm_kms_helper_hotplug_event(&dev_priv->drm);
}
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index cab632791f73..e8b1a8c1015a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3212,6 +3212,16 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
intel_dp->compliance.test_data.vdisplay);
seq_printf(m, "bpc: %u\n",
intel_dp->compliance.test_data.bpc);
+ } else if (intel_dp->compliance.test_type ==
+ DP_TEST_LINK_PHY_TEST_PATTERN) {
+ seq_printf(m, "pattern: %d\n",
+ intel_dp->compliance.test_data.phytest.phy_pattern);
+ seq_printf(m, "Number of lanes: %d\n",
+ intel_dp->compliance.test_data.phytest.num_lanes);
+ seq_printf(m, "Link Rate: %d\n",
+ intel_dp->compliance.test_data.phytest.link_rate);
+ seq_printf(m, "level: %02x\n",
+ intel_dp->train_set[0]);
}
} else
seq_puts(m, "0");
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Intel-gfx] [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
To align with link compliance design existing intel_dp_compliance
tool will be used to get the phy request in userspace through uevent.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 6 ++++--
drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 338d3744c5d5..a2b860cf3b93 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5288,8 +5288,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
intel_psr_short_pulse(intel_dp);
- if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
- DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
+ if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING ||
+ intel_dp->compliance.test_type ==
+ DP_TEST_LINK_PHY_TEST_PATTERN) {
+ DRM_DEBUG_KMS("Compliance Test requested\n");
/* Send a Hotplug Uevent to userspace to start modeset */
drm_kms_helper_hotplug_event(&dev_priv->drm);
}
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index cab632791f73..e8b1a8c1015a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3212,6 +3212,16 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
intel_dp->compliance.test_data.vdisplay);
seq_printf(m, "bpc: %u\n",
intel_dp->compliance.test_data.bpc);
+ } else if (intel_dp->compliance.test_type ==
+ DP_TEST_LINK_PHY_TEST_PATTERN) {
+ seq_printf(m, "pattern: %d\n",
+ intel_dp->compliance.test_data.phytest.phy_pattern);
+ seq_printf(m, "Number of lanes: %d\n",
+ intel_dp->compliance.test_data.phytest.num_lanes);
+ seq_printf(m, "Link Rate: %d\n",
+ intel_dp->compliance.test_data.phytest.link_rate);
+ seq_printf(m, "level: %02x\n",
+ intel_dp->train_set[0]);
}
} else
seq_puts(m, "0");
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC 5/7] drm/i915/dp: Register definition for DP compliance register
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
DP_COMP_CTL and DP_COMP_PAT register used to program DP
compliance pattern.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 478f5ff6c554..87774337c2a2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9772,6 +9772,26 @@ enum skl_power_gate {
#define DDI_BUF_BALANCE_LEG_ENABLE (1 << 31)
#define DDI_BUF_TRANS_HI(port, i) _MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8 + 4)
+/* DDI DP Compliance Control */
+#define DDI_DP_COMP_CTL_A 0x605F0
+#define DDI_DP_COMP_CTL_B 0x615F0
+#define DDI_DP_COMP_CTL(port) _MMIO_PORT(port, DDI_DP_COMP_CTL_A, \
+ DDI_DP_COMP_CTL_B)
+#define DDI_DP_COMP_CTL_ENABLE (1 << 31)
+#define DDI_DP_COMP_CTL_D10_2 (0 << 28)
+#define DDI_DP_COMP_CTL_SCRAMBLED_0 (1 << 28)
+#define DDI_DP_COMP_CTL_PRBS7 (2 << 28)
+#define DDI_DP_COMP_CTL_CUSTOM80 (3 << 28)
+#define DDI_DP_COMP_CTL_HBR2 (4 << 28)
+#define DDI_DP_COMP_CTL_SCRAMBLED_1 (5 << 28)
+#define DDI_DP_COMP_CTL_HBR2_RESET (0xFC << 0)
+
+/* DDI DP Compliance Pattern */
+#define DDI_DP_COMP_PAT_A 0x605F4
+#define DDI_DP_COMP_PAT_B 0x615F4
+#define DDI_DP_COMP_PAT(port, i) _MMIO(_PORT(port, DDI_DP_COMP_PAT_A, \
+ DDI_DP_COMP_PAT_B) + (i) * 4)
+
/* Sideband Interface (SBI) is programmed indirectly, via
* SBI_ADDR, which contains the register offset; and SBI_DATA,
* which contains the payload */
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Intel-gfx] [RFC 5/7] drm/i915/dp: Register definition for DP compliance register
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
DP_COMP_CTL and DP_COMP_PAT register used to program DP
compliance pattern.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 478f5ff6c554..87774337c2a2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9772,6 +9772,26 @@ enum skl_power_gate {
#define DDI_BUF_BALANCE_LEG_ENABLE (1 << 31)
#define DDI_BUF_TRANS_HI(port, i) _MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8 + 4)
+/* DDI DP Compliance Control */
+#define DDI_DP_COMP_CTL_A 0x605F0
+#define DDI_DP_COMP_CTL_B 0x615F0
+#define DDI_DP_COMP_CTL(port) _MMIO_PORT(port, DDI_DP_COMP_CTL_A, \
+ DDI_DP_COMP_CTL_B)
+#define DDI_DP_COMP_CTL_ENABLE (1 << 31)
+#define DDI_DP_COMP_CTL_D10_2 (0 << 28)
+#define DDI_DP_COMP_CTL_SCRAMBLED_0 (1 << 28)
+#define DDI_DP_COMP_CTL_PRBS7 (2 << 28)
+#define DDI_DP_COMP_CTL_CUSTOM80 (3 << 28)
+#define DDI_DP_COMP_CTL_HBR2 (4 << 28)
+#define DDI_DP_COMP_CTL_SCRAMBLED_1 (5 << 28)
+#define DDI_DP_COMP_CTL_HBR2_RESET (0xFC << 0)
+
+/* DDI DP Compliance Pattern */
+#define DDI_DP_COMP_PAT_A 0x605F4
+#define DDI_DP_COMP_PAT_B 0x615F4
+#define DDI_DP_COMP_PAT(port, i) _MMIO(_PORT(port, DDI_DP_COMP_PAT_A, \
+ DDI_DP_COMP_PAT_B) + (i) * 4)
+
/* Sideband Interface (SBI) is programmed indirectly, via
* SBI_ADDR, which contains the register offset; and SBI_DATA,
* which contains the payload */
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC 6/7] drm/i915/dp: Update the pattern as per request
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
set pattern in DP_COMP_CTL.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 55 +++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index a2b860cf3b93..df31278a1619 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4955,6 +4955,61 @@ static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
return DP_TEST_ACK;
}
+static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *dev_priv =
+ to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_dp_phy_test_params *data =
+ &intel_dp->compliance.test_data.phytest;
+ u32 temp;
+
+ switch (data->phy_pattern) {
+ case DP_PHY_TEST_PATTERN_NONE:
+ DRM_DEBUG_KMS("Disable Phy Test Pattern\n");
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port), 0x0);
+ break;
+ case DP_PHY_TEST_PATTERN_D10_2:
+ DRM_DEBUG_KMS("Set D10.2 Phy Test Pattern\n");
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2);
+ break;
+ case DP_PHY_TEST_PATTERN_ERROR_COUNT:
+ DRM_DEBUG_KMS("Set Error Count Phy Test Pattern\n");
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE |
+ DDI_DP_COMP_CTL_SCRAMBLED_0);
+ break;
+ case DP_PHY_TEST_PATTERN_PRBS7:
+ DRM_DEBUG_KMS("Set PRBS7 Phy Test Pattern\n");
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7);
+ break;
+ case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
+ DRM_DEBUG_KMS("Set 80Bit Custom Phy Test Pattern\n");
+ temp = ((data->custom80[0] << 24) | (data->custom80[1] << 16) |
+ (data->custom80[2] << 8) | (data->custom80[3]));
+ I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 0), temp);
+ temp = ((data->custom80[4] << 24) | (data->custom80[5] << 16) |
+ (data->custom80[6] << 8) | (data->custom80[7]));
+ I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 1), temp);
+ temp = ((data->custom80[8] << 8) | data->custom80[9]);
+ I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 2), temp);
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_CUSTOM80);
+ break;
+ case DP_PHY_TEST_PATTERN_CP2520:
+ DRM_DEBUG_KMS("Set HBR2 compliance Phy Test Pattern\n");
+ temp = ((data->hbr2_reset[1] << 8) | data->hbr2_reset[0]);
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
+ temp);
+ break;
+ default:
+ DRM_ERROR("Invalid Phy Test PAttern\n");
+ }
+}
+
static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
{
u8 test_result = DP_TEST_NAK;
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Intel-gfx] [RFC 6/7] drm/i915/dp: Update the pattern as per request
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
set pattern in DP_COMP_CTL.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 55 +++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index a2b860cf3b93..df31278a1619 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4955,6 +4955,61 @@ static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
return DP_TEST_ACK;
}
+static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *dev_priv =
+ to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_dp_phy_test_params *data =
+ &intel_dp->compliance.test_data.phytest;
+ u32 temp;
+
+ switch (data->phy_pattern) {
+ case DP_PHY_TEST_PATTERN_NONE:
+ DRM_DEBUG_KMS("Disable Phy Test Pattern\n");
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port), 0x0);
+ break;
+ case DP_PHY_TEST_PATTERN_D10_2:
+ DRM_DEBUG_KMS("Set D10.2 Phy Test Pattern\n");
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2);
+ break;
+ case DP_PHY_TEST_PATTERN_ERROR_COUNT:
+ DRM_DEBUG_KMS("Set Error Count Phy Test Pattern\n");
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE |
+ DDI_DP_COMP_CTL_SCRAMBLED_0);
+ break;
+ case DP_PHY_TEST_PATTERN_PRBS7:
+ DRM_DEBUG_KMS("Set PRBS7 Phy Test Pattern\n");
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7);
+ break;
+ case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
+ DRM_DEBUG_KMS("Set 80Bit Custom Phy Test Pattern\n");
+ temp = ((data->custom80[0] << 24) | (data->custom80[1] << 16) |
+ (data->custom80[2] << 8) | (data->custom80[3]));
+ I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 0), temp);
+ temp = ((data->custom80[4] << 24) | (data->custom80[5] << 16) |
+ (data->custom80[6] << 8) | (data->custom80[7]));
+ I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 1), temp);
+ temp = ((data->custom80[8] << 8) | data->custom80[9]);
+ I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 2), temp);
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_CUSTOM80);
+ break;
+ case DP_PHY_TEST_PATTERN_CP2520:
+ DRM_DEBUG_KMS("Set HBR2 compliance Phy Test Pattern\n");
+ temp = ((data->hbr2_reset[1] << 8) | data->hbr2_reset[0]);
+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
+ temp);
+ break;
+ default:
+ DRM_ERROR("Invalid Phy Test PAttern\n");
+ }
+}
+
static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
{
u8 test_result = DP_TEST_NAK;
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
This patch process phy compliance request by programming requested
vswing, pre-emphasis and test pattern.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++
drivers/gpu/drm/i915/display/intel_display.c | 5 ++
drivers/gpu/drm/i915/display/intel_dp.c | 77 ++++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dp.h | 2 +
drivers/gpu/drm/i915/i915_drv.h | 2 +
5 files changed, 100 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 8f817de34460..c12d4ebbd352 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3700,8 +3700,17 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
enum pipe pipe = crtc->pipe;
+ if (encoder->type == INTEL_OUTPUT_DP) {
+ if (dig_port->dp.compliance.test_type ==
+ DP_TEST_LINK_PHY_TEST_PATTERN) {
+ intel_dp_process_phy_request(&dig_port->dp);
+ return;
+ }
+ }
+
/*
* When called from DP MST code:
* - conn_state will be NULL
@@ -4147,6 +4156,11 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
+ if (encoder->type == INTEL_OUTPUT_DP)
+ if (dig_port->dp.compliance.test_type ==
+ DP_TEST_LINK_PHY_TEST_PATTERN)
+ return;
+
if (is_tc_port)
intel_tc_port_get_link(dig_port, crtc_state->lane_count);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index adf50c4b38ad..4ad11df55f07 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -545,6 +545,11 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
static bool
needs_modeset(const struct intel_crtc_state *state)
{
+ struct drm_i915_private *dev_priv = to_i915(state->uapi.crtc->dev);
+
+ if (dev_priv->dp_phy_comp)
+ return true;
+
return drm_atomic_crtc_needs_modeset(&state->uapi);
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index df31278a1619..2c3d4b6c6036 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5010,14 +5010,91 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
}
}
+static void
+intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ enum port port = intel_dig_port->base.port;
+ u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
+
+ ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
+ dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
+ trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
+
+ ddi_buf_ctl_value &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
+ dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
+ trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
+ DDI_PORT_WIDTH_MASK);
+
+ I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
+ I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
+ I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
+}
+
+static void
+intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ enum port port = intel_dig_port->base.port;
+ u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
+
+ ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
+ dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
+ trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
+
+ ddi_buf_ctl_value |= DDI_BUF_CTL_ENABLE |
+ DDI_PORT_WIDTH(lane_cnt);
+ dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
+ trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
+ DDI_PORT_WIDTH(lane_cnt);
+
+ I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
+ I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
+ I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
+}
+
+void intel_dp_process_phy_request(struct intel_dp *intel_dp)
+{
+ struct drm_dp_phy_test_params *data =
+ &intel_dp->compliance.test_data.phytest;
+ u8 link_status[DP_LINK_STATUS_SIZE];
+
+ if (!intel_dp_get_link_status(intel_dp, link_status)) {
+ DRM_DEBUG_KMS("failed to get link status\n");
+ return;
+ }
+
+ /* retrieve vswing & pre-emphasis setting */
+ intel_get_adjust_train(intel_dp, link_status);
+
+ intel_dp_autotest_phy_ddi_disable(intel_dp);
+
+ intel_dp_set_signal_levels(intel_dp);
+
+ intel_dp_phy_pattern_update(intel_dp);
+
+ intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
+
+ drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
+}
+
static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
{
u8 test_result = DP_TEST_NAK;
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
test_result = intel_dp_prepare_phytest(intel_dp);
if (test_result != DP_TEST_ACK)
DRM_ERROR("Phy test preparation failed\n");
+ /* Set test active flag here so userspace doesn't interrupt things */
+ intel_dp->compliance.test_active = 1;
+ dev_priv->dp_phy_comp = true;
+
return test_result;
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 0d0cb692f701..b1274ecffc7f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -120,6 +120,8 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state,
const struct drm_connector_state *conn_state);
bool intel_digital_port_connected(struct intel_encoder *encoder);
+void intel_dp_process_phy_request(struct intel_dp *intel_dp);
+
static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
{
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1779f600fcfb..232e0dfb9d8e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1285,6 +1285,8 @@ struct drm_i915_private {
I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
+ bool dp_phy_comp;
+
/*
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
* will be rejected. Instead look for a better place.
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Intel-gfx] [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
@ 2019-11-15 15:25 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-15 15:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, nidhi1.gupta
This patch process phy compliance request by programming requested
vswing, pre-emphasis and test pattern.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++
drivers/gpu/drm/i915/display/intel_display.c | 5 ++
drivers/gpu/drm/i915/display/intel_dp.c | 77 ++++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dp.h | 2 +
drivers/gpu/drm/i915/i915_drv.h | 2 +
5 files changed, 100 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 8f817de34460..c12d4ebbd352 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3700,8 +3700,17 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
enum pipe pipe = crtc->pipe;
+ if (encoder->type == INTEL_OUTPUT_DP) {
+ if (dig_port->dp.compliance.test_type ==
+ DP_TEST_LINK_PHY_TEST_PATTERN) {
+ intel_dp_process_phy_request(&dig_port->dp);
+ return;
+ }
+ }
+
/*
* When called from DP MST code:
* - conn_state will be NULL
@@ -4147,6 +4156,11 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
+ if (encoder->type == INTEL_OUTPUT_DP)
+ if (dig_port->dp.compliance.test_type ==
+ DP_TEST_LINK_PHY_TEST_PATTERN)
+ return;
+
if (is_tc_port)
intel_tc_port_get_link(dig_port, crtc_state->lane_count);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index adf50c4b38ad..4ad11df55f07 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -545,6 +545,11 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
static bool
needs_modeset(const struct intel_crtc_state *state)
{
+ struct drm_i915_private *dev_priv = to_i915(state->uapi.crtc->dev);
+
+ if (dev_priv->dp_phy_comp)
+ return true;
+
return drm_atomic_crtc_needs_modeset(&state->uapi);
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index df31278a1619..2c3d4b6c6036 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5010,14 +5010,91 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
}
}
+static void
+intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ enum port port = intel_dig_port->base.port;
+ u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
+
+ ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
+ dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
+ trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
+
+ ddi_buf_ctl_value &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
+ dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
+ trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
+ DDI_PORT_WIDTH_MASK);
+
+ I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
+ I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
+ I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
+}
+
+static void
+intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ enum port port = intel_dig_port->base.port;
+ u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
+
+ ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
+ dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
+ trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
+
+ ddi_buf_ctl_value |= DDI_BUF_CTL_ENABLE |
+ DDI_PORT_WIDTH(lane_cnt);
+ dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
+ trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
+ DDI_PORT_WIDTH(lane_cnt);
+
+ I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
+ I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
+ I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
+}
+
+void intel_dp_process_phy_request(struct intel_dp *intel_dp)
+{
+ struct drm_dp_phy_test_params *data =
+ &intel_dp->compliance.test_data.phytest;
+ u8 link_status[DP_LINK_STATUS_SIZE];
+
+ if (!intel_dp_get_link_status(intel_dp, link_status)) {
+ DRM_DEBUG_KMS("failed to get link status\n");
+ return;
+ }
+
+ /* retrieve vswing & pre-emphasis setting */
+ intel_get_adjust_train(intel_dp, link_status);
+
+ intel_dp_autotest_phy_ddi_disable(intel_dp);
+
+ intel_dp_set_signal_levels(intel_dp);
+
+ intel_dp_phy_pattern_update(intel_dp);
+
+ intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
+
+ drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
+}
+
static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
{
u8 test_result = DP_TEST_NAK;
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
test_result = intel_dp_prepare_phytest(intel_dp);
if (test_result != DP_TEST_ACK)
DRM_ERROR("Phy test preparation failed\n");
+ /* Set test active flag here so userspace doesn't interrupt things */
+ intel_dp->compliance.test_active = 1;
+ dev_priv->dp_phy_comp = true;
+
return test_result;
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 0d0cb692f701..b1274ecffc7f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -120,6 +120,8 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state,
const struct drm_connector_state *conn_state);
bool intel_digital_port_connected(struct intel_encoder *encoder);
+void intel_dp_process_phy_request(struct intel_dp *intel_dp);
+
static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
{
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1779f600fcfb..232e0dfb9d8e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1285,6 +1285,8 @@ struct drm_i915_private {
I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
+ bool dp_phy_comp;
+
/*
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
* will be rejected. Instead look for a better place.
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 44+ messages in thread
* ✗ Fi.CI.BUILD: failure for DP Phy compliace auto test
@ 2019-11-15 19:27 ` Patchwork
0 siblings, 0 replies; 44+ messages in thread
From: Patchwork @ 2019-11-15 19:27 UTC (permalink / raw)
To: Animesh Manna; +Cc: intel-gfx
== Series Details ==
Series: DP Phy compliace auto test
URL : https://patchwork.freedesktop.org/series/69541/
State : failure
== Summary ==
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CHK include/generated/compile.h
CC [M] drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.o
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c: In function ‘dp_test_send_phy_test_pattern’:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:2187:4: error: ‘DP_TEST_PHY_PATTERN’ undeclared (first use in this function); did you mean ‘DP_TEST_PATTERN’?
DP_TEST_PHY_PATTERN,
^~~~~~~~~~~~~~~~~~~
DP_TEST_PATTERN
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:2187:4: note: each undeclared identifier is reported only once for each function it appears in
scripts/Makefile.build:265: recipe for target 'drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.o' failed
make[4]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.o] Error 1
scripts/Makefile.build:509: recipe for target 'drivers/gpu/drm/amd/amdgpu' failed
make[3]: *** [drivers/gpu/drm/amd/amdgpu] Error 2
scripts/Makefile.build:509: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:509: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1649: recipe for target 'drivers' failed
make: *** [drivers] Error 2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BUILD: failure for DP Phy compliace auto test
@ 2019-11-15 19:27 ` Patchwork
0 siblings, 0 replies; 44+ messages in thread
From: Patchwork @ 2019-11-15 19:27 UTC (permalink / raw)
To: Animesh Manna; +Cc: intel-gfx
== Series Details ==
Series: DP Phy compliace auto test
URL : https://patchwork.freedesktop.org/series/69541/
State : failure
== Summary ==
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CHK include/generated/compile.h
CC [M] drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.o
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c: In function ‘dp_test_send_phy_test_pattern’:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:2187:4: error: ‘DP_TEST_PHY_PATTERN’ undeclared (first use in this function); did you mean ‘DP_TEST_PATTERN’?
DP_TEST_PHY_PATTERN,
^~~~~~~~~~~~~~~~~~~
DP_TEST_PATTERN
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:2187:4: note: each undeclared identifier is reported only once for each function it appears in
scripts/Makefile.build:265: recipe for target 'drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.o' failed
make[4]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.o] Error 1
scripts/Makefile.build:509: recipe for target 'drivers/gpu/drm/amd/amdgpu' failed
make[3]: *** [drivers/gpu/drm/amd/amdgpu] Error 2
scripts/Makefile.build:509: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:509: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1649: recipe for target 'drivers' failed
make: *** [drivers] Error 2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 1/7] drm/dp: get/set phy compliance pattern
@ 2019-11-18 4:04 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 4:04 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
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
> 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@intel.com>
> ---
> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 1/7] drm/dp: get/set phy compliance pattern
@ 2019-11-18 4:04 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 4:04 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
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
> 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@intel.com>
> ---
> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 3/7] drm/i915/dp: Preparation for DP phy compliance auto test
@ 2019-11-18 4:47 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 4:47 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:45PM +0530, Animesh Manna wrote:
> During DP phy compliance auto test mode, sink will request
> combination of different test pattern with differnt level of
> vswing, pre-emphasis. Function added to prepare for it.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
This looks good to me,
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> .../drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 24 +++++++++++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 83ea04149b77..c17866f126c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1205,6 +1205,7 @@ struct intel_dp_compliance_data {
> u8 video_pattern;
> u16 hdisplay, vdisplay;
> u8 bpc;
> + struct drm_dp_phy_test_params phytest;
> };
>
> struct intel_dp_compliance {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index cc8e0d1a73af..338d3744c5d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4936,9 +4936,33 @@ static u8 intel_dp_autotest_edid(struct intel_dp *intel_dp)
> return test_result;
> }
>
> +static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
> +{
> + struct drm_dp_phy_test_params *data =
> + &intel_dp->compliance.test_data.phytest;
> +
> + if (!drm_dp_get_phy_test_pattern(&intel_dp->aux, data)) {
> + DRM_DEBUG_KMS("DP Phy Test pattern AUX read failure\n");
> + return DP_TEST_NAK;
> + }
> +
> + /*
> + * link_mst is set to false to avoid executing mst related code
> + * during compliance testing.
> + */
> + intel_dp->link_mst = false;
> +
> + return DP_TEST_ACK;
> +}
> +
> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> {
> u8 test_result = DP_TEST_NAK;
> +
> + test_result = intel_dp_prepare_phytest(intel_dp);
> + if (test_result != DP_TEST_ACK)
> + DRM_ERROR("Phy test preparation failed\n");
> +
> return test_result;
> }
>
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 3/7] drm/i915/dp: Preparation for DP phy compliance auto test
@ 2019-11-18 4:47 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 4:47 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:45PM +0530, Animesh Manna wrote:
> During DP phy compliance auto test mode, sink will request
> combination of different test pattern with differnt level of
> vswing, pre-emphasis. Function added to prepare for it.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
This looks good to me,
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> .../drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 24 +++++++++++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 83ea04149b77..c17866f126c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1205,6 +1205,7 @@ struct intel_dp_compliance_data {
> u8 video_pattern;
> u16 hdisplay, vdisplay;
> u8 bpc;
> + struct drm_dp_phy_test_params phytest;
> };
>
> struct intel_dp_compliance {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index cc8e0d1a73af..338d3744c5d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4936,9 +4936,33 @@ static u8 intel_dp_autotest_edid(struct intel_dp *intel_dp)
> return test_result;
> }
>
> +static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
> +{
> + struct drm_dp_phy_test_params *data =
> + &intel_dp->compliance.test_data.phytest;
> +
> + if (!drm_dp_get_phy_test_pattern(&intel_dp->aux, data)) {
> + DRM_DEBUG_KMS("DP Phy Test pattern AUX read failure\n");
> + return DP_TEST_NAK;
> + }
> +
> + /*
> + * link_mst is set to false to avoid executing mst related code
> + * during compliance testing.
> + */
> + intel_dp->link_mst = false;
> +
> + return DP_TEST_ACK;
> +}
> +
> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> {
> u8 test_result = DP_TEST_NAK;
> +
> + test_result = intel_dp_prepare_phytest(intel_dp);
> + if (test_result != DP_TEST_ACK)
> + DRM_ERROR("Phy test preparation failed\n");
> +
> return test_result;
> }
>
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry
@ 2019-11-18 4:58 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 4:58 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:46PM +0530, Animesh Manna wrote:
> To align with link compliance design existing intel_dp_compliance
> tool will be used to get the phy request in userspace through uevent.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
I would prefer splitting this patch since sending a uevent is more related
to the PHY test prep handling and debugfs handling can all be in a separate
patch.
I prefer this because debugfs nodes might need to change in the future based
on more requirements or testing feedback so its better for that to be in separate
patch.
you could add the hotplug event sending part to the prep patch (3/7) and mention that
in the commit message
Debugfs part looks good to me. Have you tested the debugfs nodes and validated if this
information is being written in the correct form?
After the split and validation of debugs nodes:
Acked-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 6 ++++--
> drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 338d3744c5d5..a2b860cf3b93 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5288,8 +5288,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>
> intel_psr_short_pulse(intel_dp);
>
> - if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> - DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
> + if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING ||
> + intel_dp->compliance.test_type ==
> + DP_TEST_LINK_PHY_TEST_PATTERN) {
> + DRM_DEBUG_KMS("Compliance Test requested\n");
> /* Send a Hotplug Uevent to userspace to start modeset */
> drm_kms_helper_hotplug_event(&dev_priv->drm);
> }
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index cab632791f73..e8b1a8c1015a 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3212,6 +3212,16 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
> intel_dp->compliance.test_data.vdisplay);
> seq_printf(m, "bpc: %u\n",
> intel_dp->compliance.test_data.bpc);
> + } else if (intel_dp->compliance.test_type ==
> + DP_TEST_LINK_PHY_TEST_PATTERN) {
> + seq_printf(m, "pattern: %d\n",
> + intel_dp->compliance.test_data.phytest.phy_pattern);
> + seq_printf(m, "Number of lanes: %d\n",
> + intel_dp->compliance.test_data.phytest.num_lanes);
> + seq_printf(m, "Link Rate: %d\n",
> + intel_dp->compliance.test_data.phytest.link_rate);
> + seq_printf(m, "level: %02x\n",
> + intel_dp->train_set[0]);
> }
> } else
> seq_puts(m, "0");
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry
@ 2019-11-18 4:58 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 4:58 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:46PM +0530, Animesh Manna wrote:
> To align with link compliance design existing intel_dp_compliance
> tool will be used to get the phy request in userspace through uevent.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
I would prefer splitting this patch since sending a uevent is more related
to the PHY test prep handling and debugfs handling can all be in a separate
patch.
I prefer this because debugfs nodes might need to change in the future based
on more requirements or testing feedback so its better for that to be in separate
patch.
you could add the hotplug event sending part to the prep patch (3/7) and mention that
in the commit message
Debugfs part looks good to me. Have you tested the debugfs nodes and validated if this
information is being written in the correct form?
After the split and validation of debugs nodes:
Acked-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 6 ++++--
> drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 338d3744c5d5..a2b860cf3b93 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5288,8 +5288,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>
> intel_psr_short_pulse(intel_dp);
>
> - if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> - DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
> + if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING ||
> + intel_dp->compliance.test_type ==
> + DP_TEST_LINK_PHY_TEST_PATTERN) {
> + DRM_DEBUG_KMS("Compliance Test requested\n");
> /* Send a Hotplug Uevent to userspace to start modeset */
> drm_kms_helper_hotplug_event(&dev_priv->drm);
> }
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index cab632791f73..e8b1a8c1015a 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3212,6 +3212,16 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
> intel_dp->compliance.test_data.vdisplay);
> seq_printf(m, "bpc: %u\n",
> intel_dp->compliance.test_data.bpc);
> + } else if (intel_dp->compliance.test_type ==
> + DP_TEST_LINK_PHY_TEST_PATTERN) {
> + seq_printf(m, "pattern: %d\n",
> + intel_dp->compliance.test_data.phytest.phy_pattern);
> + seq_printf(m, "Number of lanes: %d\n",
> + intel_dp->compliance.test_data.phytest.num_lanes);
> + seq_printf(m, "Link Rate: %d\n",
> + intel_dp->compliance.test_data.phytest.link_rate);
> + seq_printf(m, "level: %02x\n",
> + intel_dp->train_set[0]);
> }
> } else
> seq_puts(m, "0");
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 5/7] drm/i915/dp: Register definition for DP compliance register
@ 2019-11-18 5:00 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 5:00 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:47PM +0530, Animesh Manna wrote:
> DP_COMP_CTL and DP_COMP_PAT register used to program DP
> compliance pattern.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 478f5ff6c554..87774337c2a2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9772,6 +9772,26 @@ enum skl_power_gate {
> #define DDI_BUF_BALANCE_LEG_ENABLE (1 << 31)
> #define DDI_BUF_TRANS_HI(port, i) _MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8 + 4)
>
> +/* DDI DP Compliance Control */
> +#define DDI_DP_COMP_CTL_A 0x605F0
> +#define DDI_DP_COMP_CTL_B 0x615F0
> +#define DDI_DP_COMP_CTL(port) _MMIO_PORT(port, DDI_DP_COMP_CTL_A, \
> + DDI_DP_COMP_CTL_B)
> +#define DDI_DP_COMP_CTL_ENABLE (1 << 31)
> +#define DDI_DP_COMP_CTL_D10_2 (0 << 28)
> +#define DDI_DP_COMP_CTL_SCRAMBLED_0 (1 << 28)
> +#define DDI_DP_COMP_CTL_PRBS7 (2 << 28)
> +#define DDI_DP_COMP_CTL_CUSTOM80 (3 << 28)
> +#define DDI_DP_COMP_CTL_HBR2 (4 << 28)
> +#define DDI_DP_COMP_CTL_SCRAMBLED_1 (5 << 28)
> +#define DDI_DP_COMP_CTL_HBR2_RESET (0xFC << 0)
> +
> +/* DDI DP Compliance Pattern */
> +#define DDI_DP_COMP_PAT_A 0x605F4
> +#define DDI_DP_COMP_PAT_B 0x615F4
> +#define DDI_DP_COMP_PAT(port, i) _MMIO(_PORT(port, DDI_DP_COMP_PAT_A, \
> + DDI_DP_COMP_PAT_B) + (i) * 4)
> +
> /* Sideband Interface (SBI) is programmed indirectly, via
> * SBI_ADDR, which contains the register offset; and SBI_DATA,
> * which contains the payload */
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 5/7] drm/i915/dp: Register definition for DP compliance register
@ 2019-11-18 5:00 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 5:00 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:47PM +0530, Animesh Manna wrote:
> DP_COMP_CTL and DP_COMP_PAT register used to program DP
> compliance pattern.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 478f5ff6c554..87774337c2a2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9772,6 +9772,26 @@ enum skl_power_gate {
> #define DDI_BUF_BALANCE_LEG_ENABLE (1 << 31)
> #define DDI_BUF_TRANS_HI(port, i) _MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8 + 4)
>
> +/* DDI DP Compliance Control */
> +#define DDI_DP_COMP_CTL_A 0x605F0
> +#define DDI_DP_COMP_CTL_B 0x615F0
> +#define DDI_DP_COMP_CTL(port) _MMIO_PORT(port, DDI_DP_COMP_CTL_A, \
> + DDI_DP_COMP_CTL_B)
> +#define DDI_DP_COMP_CTL_ENABLE (1 << 31)
> +#define DDI_DP_COMP_CTL_D10_2 (0 << 28)
> +#define DDI_DP_COMP_CTL_SCRAMBLED_0 (1 << 28)
> +#define DDI_DP_COMP_CTL_PRBS7 (2 << 28)
> +#define DDI_DP_COMP_CTL_CUSTOM80 (3 << 28)
> +#define DDI_DP_COMP_CTL_HBR2 (4 << 28)
> +#define DDI_DP_COMP_CTL_SCRAMBLED_1 (5 << 28)
> +#define DDI_DP_COMP_CTL_HBR2_RESET (0xFC << 0)
> +
> +/* DDI DP Compliance Pattern */
> +#define DDI_DP_COMP_PAT_A 0x605F4
> +#define DDI_DP_COMP_PAT_B 0x615F4
> +#define DDI_DP_COMP_PAT(port, i) _MMIO(_PORT(port, DDI_DP_COMP_PAT_A, \
> + DDI_DP_COMP_PAT_B) + (i) * 4)
> +
> /* Sideband Interface (SBI) is programmed indirectly, via
> * SBI_ADDR, which contains the register offset; and SBI_DATA,
> * which contains the payload */
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry
@ 2019-11-18 5:06 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 5:06 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Sun, Nov 17, 2019 at 08:58:45PM -0800, Manasi Navare wrote:
> On Fri, Nov 15, 2019 at 08:55:46PM +0530, Animesh Manna wrote:
> > To align with link compliance design existing intel_dp_compliance
> > tool will be used to get the phy request in userspace through uevent.
> >
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>
> I would prefer splitting this patch since sending a uevent is more related
> to the PHY test prep handling and debugfs handling can all be in a separate
> patch.
> I prefer this because debugfs nodes might need to change in the future based
> on more requirements or testing feedback so its better for that to be in separate
> patch.
>
> you could add the hotplug event sending part to the prep patch (3/7) and mention that
> in the commit message
>
> Debugfs part looks good to me. Have you tested the debugfs nodes and validated if this
> information is being written in the correct form?
>
> After the split and validation of debugs nodes:
>
> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
>
> Manasi
>
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 6 ++++--
> > drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++++
> > 2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 338d3744c5d5..a2b860cf3b93 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5288,8 +5288,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
> >
> > intel_psr_short_pulse(intel_dp);
> >
> > - if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> > - DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
> > + if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING ||
> > + intel_dp->compliance.test_type ==
> > + DP_TEST_LINK_PHY_TEST_PATTERN) {
> > + DRM_DEBUG_KMS("Compliance Test requested\n");
One more change I think here would be good IMO for debugging is that
you should print test_type in DEBUG_KMS
Manasi
> > /* Send a Hotplug Uevent to userspace to start modeset */
> > drm_kms_helper_hotplug_event(&dev_priv->drm);
> > }
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index cab632791f73..e8b1a8c1015a 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3212,6 +3212,16 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
> > intel_dp->compliance.test_data.vdisplay);
> > seq_printf(m, "bpc: %u\n",
> > intel_dp->compliance.test_data.bpc);
> > + } else if (intel_dp->compliance.test_type ==
> > + DP_TEST_LINK_PHY_TEST_PATTERN) {
> > + seq_printf(m, "pattern: %d\n",
> > + intel_dp->compliance.test_data.phytest.phy_pattern);
> > + seq_printf(m, "Number of lanes: %d\n",
> > + intel_dp->compliance.test_data.phytest.num_lanes);
> > + seq_printf(m, "Link Rate: %d\n",
> > + intel_dp->compliance.test_data.phytest.link_rate);
> > + seq_printf(m, "level: %02x\n",
> > + intel_dp->train_set[0]);
> > }
> > } else
> > seq_puts(m, "0");
> > --
> > 2.22.0
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry
@ 2019-11-18 5:06 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 5:06 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Sun, Nov 17, 2019 at 08:58:45PM -0800, Manasi Navare wrote:
> On Fri, Nov 15, 2019 at 08:55:46PM +0530, Animesh Manna wrote:
> > To align with link compliance design existing intel_dp_compliance
> > tool will be used to get the phy request in userspace through uevent.
> >
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>
> I would prefer splitting this patch since sending a uevent is more related
> to the PHY test prep handling and debugfs handling can all be in a separate
> patch.
> I prefer this because debugfs nodes might need to change in the future based
> on more requirements or testing feedback so its better for that to be in separate
> patch.
>
> you could add the hotplug event sending part to the prep patch (3/7) and mention that
> in the commit message
>
> Debugfs part looks good to me. Have you tested the debugfs nodes and validated if this
> information is being written in the correct form?
>
> After the split and validation of debugs nodes:
>
> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
>
> Manasi
>
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 6 ++++--
> > drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++++
> > 2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 338d3744c5d5..a2b860cf3b93 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5288,8 +5288,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
> >
> > intel_psr_short_pulse(intel_dp);
> >
> > - if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> > - DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
> > + if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING ||
> > + intel_dp->compliance.test_type ==
> > + DP_TEST_LINK_PHY_TEST_PATTERN) {
> > + DRM_DEBUG_KMS("Compliance Test requested\n");
One more change I think here would be good IMO for debugging is that
you should print test_type in DEBUG_KMS
Manasi
> > /* Send a Hotplug Uevent to userspace to start modeset */
> > drm_kms_helper_hotplug_event(&dev_priv->drm);
> > }
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index cab632791f73..e8b1a8c1015a 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3212,6 +3212,16 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
> > intel_dp->compliance.test_data.vdisplay);
> > seq_printf(m, "bpc: %u\n",
> > intel_dp->compliance.test_data.bpc);
> > + } else if (intel_dp->compliance.test_type ==
> > + DP_TEST_LINK_PHY_TEST_PATTERN) {
> > + seq_printf(m, "pattern: %d\n",
> > + intel_dp->compliance.test_data.phytest.phy_pattern);
> > + seq_printf(m, "Number of lanes: %d\n",
> > + intel_dp->compliance.test_data.phytest.num_lanes);
> > + seq_printf(m, "Link Rate: %d\n",
> > + intel_dp->compliance.test_data.phytest.link_rate);
> > + seq_printf(m, "level: %02x\n",
> > + intel_dp->train_set[0]);
> > }
> > } else
> > seq_puts(m, "0");
> > --
> > 2.22.0
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 6/7] drm/i915/dp: Update the pattern as per request
@ 2019-11-18 6:41 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 6:41 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:48PM +0530, Animesh Manna wrote:
> set pattern in DP_COMP_CTL.
It would be nice to have some brief description here on
context of setting a PHY pattern for PHY compliance and that
this will be called during pre enable in atomic commit in case
of phy compliance request etc..and that the requested phy pattern
is read during test handling etc..
something that gives a reader a better idea of the context of
this function
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 55 +++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a2b860cf3b93..df31278a1619 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4955,6 +4955,61 @@ static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
> return DP_TEST_ACK;
> }
>
> +static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> +{
> + struct drm_i915_private *dev_priv =
> + to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_dp_phy_test_params *data =
> + &intel_dp->compliance.test_data.phytest;
> + u32 temp;
> +
> + switch (data->phy_pattern) {
> + case DP_PHY_TEST_PATTERN_NONE:
> + DRM_DEBUG_KMS("Disable Phy Test Pattern\n");
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port), 0x0);
> + break;
> + case DP_PHY_TEST_PATTERN_D10_2:
> + DRM_DEBUG_KMS("Set D10.2 Phy Test Pattern\n");
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2);
> + break;
> + case DP_PHY_TEST_PATTERN_ERROR_COUNT:
> + DRM_DEBUG_KMS("Set Error Count Phy Test Pattern\n");
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE |
> + DDI_DP_COMP_CTL_SCRAMBLED_0);
> + break;
> + case DP_PHY_TEST_PATTERN_PRBS7:
> + DRM_DEBUG_KMS("Set PRBS7 Phy Test Pattern\n");
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7);
> + break;
> + case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
> + DRM_DEBUG_KMS("Set 80Bit Custom Phy Test Pattern\n");
> + temp = ((data->custom80[0] << 24) | (data->custom80[1] << 16) |
> + (data->custom80[2] << 8) | (data->custom80[3]));
> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 0), temp);
> + temp = ((data->custom80[4] << 24) | (data->custom80[5] << 16) |
> + (data->custom80[6] << 8) | (data->custom80[7]));
> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 1), temp);
> + temp = ((data->custom80[8] << 8) | data->custom80[9]);
> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 2), temp);
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_CUSTOM80);
> + break;
> + case DP_PHY_TEST_PATTERN_CP2520:
> + DRM_DEBUG_KMS("Set HBR2 compliance Phy Test Pattern\n");
> + temp = ((data->hbr2_reset[1] << 8) | data->hbr2_reset[0]);
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
> + temp);
> + break;
> + default:
> + DRM_ERROR("Invalid Phy Test PAttern\n");
Here we would definitely want a WARN since ERROR at this point in commit is not good
Other than looks good to me.
Manasi
> + }
> +}
> +
> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> {
> u8 test_result = DP_TEST_NAK;
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 6/7] drm/i915/dp: Update the pattern as per request
@ 2019-11-18 6:41 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 6:41 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:48PM +0530, Animesh Manna wrote:
> set pattern in DP_COMP_CTL.
It would be nice to have some brief description here on
context of setting a PHY pattern for PHY compliance and that
this will be called during pre enable in atomic commit in case
of phy compliance request etc..and that the requested phy pattern
is read during test handling etc..
something that gives a reader a better idea of the context of
this function
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 55 +++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a2b860cf3b93..df31278a1619 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4955,6 +4955,61 @@ static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
> return DP_TEST_ACK;
> }
>
> +static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> +{
> + struct drm_i915_private *dev_priv =
> + to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_dp_phy_test_params *data =
> + &intel_dp->compliance.test_data.phytest;
> + u32 temp;
> +
> + switch (data->phy_pattern) {
> + case DP_PHY_TEST_PATTERN_NONE:
> + DRM_DEBUG_KMS("Disable Phy Test Pattern\n");
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port), 0x0);
> + break;
> + case DP_PHY_TEST_PATTERN_D10_2:
> + DRM_DEBUG_KMS("Set D10.2 Phy Test Pattern\n");
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2);
> + break;
> + case DP_PHY_TEST_PATTERN_ERROR_COUNT:
> + DRM_DEBUG_KMS("Set Error Count Phy Test Pattern\n");
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE |
> + DDI_DP_COMP_CTL_SCRAMBLED_0);
> + break;
> + case DP_PHY_TEST_PATTERN_PRBS7:
> + DRM_DEBUG_KMS("Set PRBS7 Phy Test Pattern\n");
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7);
> + break;
> + case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
> + DRM_DEBUG_KMS("Set 80Bit Custom Phy Test Pattern\n");
> + temp = ((data->custom80[0] << 24) | (data->custom80[1] << 16) |
> + (data->custom80[2] << 8) | (data->custom80[3]));
> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 0), temp);
> + temp = ((data->custom80[4] << 24) | (data->custom80[5] << 16) |
> + (data->custom80[6] << 8) | (data->custom80[7]));
> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 1), temp);
> + temp = ((data->custom80[8] << 8) | data->custom80[9]);
> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 2), temp);
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_CUSTOM80);
> + break;
> + case DP_PHY_TEST_PATTERN_CP2520:
> + DRM_DEBUG_KMS("Set HBR2 compliance Phy Test Pattern\n");
> + temp = ((data->hbr2_reset[1] << 8) | data->hbr2_reset[0]);
> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
> + temp);
> + break;
> + default:
> + DRM_ERROR("Invalid Phy Test PAttern\n");
Here we would definitely want a WARN since ERROR at this point in commit is not good
Other than looks good to me.
Manasi
> + }
> +}
> +
> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> {
> u8 test_result = DP_TEST_NAK;
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
@ 2019-11-18 7:53 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 7:53 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:49PM +0530, Animesh Manna wrote:
> This patch process phy compliance request by programming requested
> vswing, pre-emphasis and test pattern.
Again here a slightly detailed description of where in the atomic modeset
do we process the PHY com request would be good.
The design overall looks good now, few comments below:
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++
> drivers/gpu/drm/i915/display/intel_display.c | 5 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 77 ++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dp.h | 2 +
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> 5 files changed, 100 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 8f817de34460..c12d4ebbd352 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3700,8 +3700,17 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> enum pipe pipe = crtc->pipe;
>
> + if (encoder->type == INTEL_OUTPUT_DP) {
> + if (dig_port->dp.compliance.test_type ==
> + DP_TEST_LINK_PHY_TEST_PATTERN) {
> + intel_dp_process_phy_request(&dig_port->dp);
> + return;
> + }
> + }
> +
> /*
> * When called from DP MST code:
> * - conn_state will be NULL
> @@ -4147,6 +4156,11 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
>
> + if (encoder->type == INTEL_OUTPUT_DP)
> + if (dig_port->dp.compliance.test_type ==
> + DP_TEST_LINK_PHY_TEST_PATTERN)
> + return;
> +
> if (is_tc_port)
> intel_tc_port_get_link(dig_port, crtc_state->lane_count);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index adf50c4b38ad..4ad11df55f07 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -545,6 +545,11 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
> static bool
> needs_modeset(const struct intel_crtc_state *state)
> {
> + struct drm_i915_private *dev_priv = to_i915(state->uapi.crtc->dev);
> +
> + if (dev_priv->dp_phy_comp)
> + return true;
Could you double check with Jani N if this is an acceptable solution to
foce a full modeset for a PHY compliance test?
> +
> return drm_atomic_crtc_needs_modeset(&state->uapi);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index df31278a1619..2c3d4b6c6036 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5010,14 +5010,91 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> }
> }
>
> +static void
> +intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + enum port port = intel_dig_port->base.port;
> + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> +
> + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> +
> + ddi_buf_ctl_value &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
> + dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
> + trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
> + DDI_PORT_WIDTH_MASK);
> +
> + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> +}
> +
> +static void
> +intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + enum port port = intel_dig_port->base.port;
> + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> +
> + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> +
> + ddi_buf_ctl_value |= DDI_BUF_CTL_ENABLE |
> + DDI_PORT_WIDTH(lane_cnt);
> + dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
> + trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
> + DDI_PORT_WIDTH(lane_cnt);
> +
> + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> +}
> +
> +void intel_dp_process_phy_request(struct intel_dp *intel_dp)
> +{
> + struct drm_dp_phy_test_params *data =
> + &intel_dp->compliance.test_data.phytest;
> + u8 link_status[DP_LINK_STATUS_SIZE];
> +
> + if (!intel_dp_get_link_status(intel_dp, link_status)) {
> + DRM_DEBUG_KMS("failed to get link status\n");
> + return;
> + }
> +
> + /* retrieve vswing & pre-emphasis setting */
> + intel_get_adjust_train(intel_dp, link_status);
> +
> + intel_dp_autotest_phy_ddi_disable(intel_dp);
> +
> + intel_dp_set_signal_levels(intel_dp);
> +
> + intel_dp_phy_pattern_update(intel_dp);
> +
> + intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
> +
> + drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
> +}
> +
> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> {
> u8 test_result = DP_TEST_NAK;
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>
> test_result = intel_dp_prepare_phytest(intel_dp);
> if (test_result != DP_TEST_ACK)
> DRM_ERROR("Phy test preparation failed\n");
>
> + /* Set test active flag here so userspace doesn't interrupt things */
> + intel_dp->compliance.test_active = 1;
> + dev_priv->dp_phy_comp = true;
> +
This should be moved to the patch 3 I think where you handle phy test request and
call prepare function. The same patch that handles test req can set the test active to 1.
> return test_result;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 0d0cb692f701..b1274ecffc7f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -120,6 +120,8 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state,
> const struct drm_connector_state *conn_state);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
> +void intel_dp_process_phy_request(struct intel_dp *intel_dp);
> +
>
> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1779f600fcfb..232e0dfb9d8e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1285,6 +1285,8 @@ struct drm_i915_private {
>
> I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
>
> + bool dp_phy_comp;
Not sure if this is the best way to handle the full mdoeset, could you double
check with Jani?
Other than that I think now the placement in pre_enable_dp is good.
The only other concern I have is changing link rate and lane count only happens
in set_phy_patterns where we write the test link rate and lane count
directly to the link bw set. But the driver's compute config is still configuring
the pipe and plls based on the link rate and lane count that could be
different than the test link rate/count. This might work now since you said
the link layer test is already configuring it at test params but in general
shouldnt we be using the phy->link rate and lane count if in phy compliance
also in compute_config()?
Jani, any thoughts here?
Regards
Manasi
> +
> /*
> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> * will be rejected. Instead look for a better place.
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
@ 2019-11-18 7:53 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-11-18 7:53 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Nov 15, 2019 at 08:55:49PM +0530, Animesh Manna wrote:
> This patch process phy compliance request by programming requested
> vswing, pre-emphasis and test pattern.
Again here a slightly detailed description of where in the atomic modeset
do we process the PHY com request would be good.
The design overall looks good now, few comments below:
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++
> drivers/gpu/drm/i915/display/intel_display.c | 5 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 77 ++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dp.h | 2 +
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> 5 files changed, 100 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 8f817de34460..c12d4ebbd352 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3700,8 +3700,17 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> enum pipe pipe = crtc->pipe;
>
> + if (encoder->type == INTEL_OUTPUT_DP) {
> + if (dig_port->dp.compliance.test_type ==
> + DP_TEST_LINK_PHY_TEST_PATTERN) {
> + intel_dp_process_phy_request(&dig_port->dp);
> + return;
> + }
> + }
> +
> /*
> * When called from DP MST code:
> * - conn_state will be NULL
> @@ -4147,6 +4156,11 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
>
> + if (encoder->type == INTEL_OUTPUT_DP)
> + if (dig_port->dp.compliance.test_type ==
> + DP_TEST_LINK_PHY_TEST_PATTERN)
> + return;
> +
> if (is_tc_port)
> intel_tc_port_get_link(dig_port, crtc_state->lane_count);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index adf50c4b38ad..4ad11df55f07 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -545,6 +545,11 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
> static bool
> needs_modeset(const struct intel_crtc_state *state)
> {
> + struct drm_i915_private *dev_priv = to_i915(state->uapi.crtc->dev);
> +
> + if (dev_priv->dp_phy_comp)
> + return true;
Could you double check with Jani N if this is an acceptable solution to
foce a full modeset for a PHY compliance test?
> +
> return drm_atomic_crtc_needs_modeset(&state->uapi);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index df31278a1619..2c3d4b6c6036 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5010,14 +5010,91 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> }
> }
>
> +static void
> +intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + enum port port = intel_dig_port->base.port;
> + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> +
> + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> +
> + ddi_buf_ctl_value &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
> + dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
> + trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
> + DDI_PORT_WIDTH_MASK);
> +
> + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> +}
> +
> +static void
> +intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + enum port port = intel_dig_port->base.port;
> + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> +
> + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> +
> + ddi_buf_ctl_value |= DDI_BUF_CTL_ENABLE |
> + DDI_PORT_WIDTH(lane_cnt);
> + dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
> + trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
> + DDI_PORT_WIDTH(lane_cnt);
> +
> + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> +}
> +
> +void intel_dp_process_phy_request(struct intel_dp *intel_dp)
> +{
> + struct drm_dp_phy_test_params *data =
> + &intel_dp->compliance.test_data.phytest;
> + u8 link_status[DP_LINK_STATUS_SIZE];
> +
> + if (!intel_dp_get_link_status(intel_dp, link_status)) {
> + DRM_DEBUG_KMS("failed to get link status\n");
> + return;
> + }
> +
> + /* retrieve vswing & pre-emphasis setting */
> + intel_get_adjust_train(intel_dp, link_status);
> +
> + intel_dp_autotest_phy_ddi_disable(intel_dp);
> +
> + intel_dp_set_signal_levels(intel_dp);
> +
> + intel_dp_phy_pattern_update(intel_dp);
> +
> + intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
> +
> + drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
> +}
> +
> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> {
> u8 test_result = DP_TEST_NAK;
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>
> test_result = intel_dp_prepare_phytest(intel_dp);
> if (test_result != DP_TEST_ACK)
> DRM_ERROR("Phy test preparation failed\n");
>
> + /* Set test active flag here so userspace doesn't interrupt things */
> + intel_dp->compliance.test_active = 1;
> + dev_priv->dp_phy_comp = true;
> +
This should be moved to the patch 3 I think where you handle phy test request and
call prepare function. The same patch that handles test req can set the test active to 1.
> return test_result;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 0d0cb692f701..b1274ecffc7f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -120,6 +120,8 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state,
> const struct drm_connector_state *conn_state);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
> +void intel_dp_process_phy_request(struct intel_dp *intel_dp);
> +
>
> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1779f600fcfb..232e0dfb9d8e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1285,6 +1285,8 @@ struct drm_i915_private {
>
> I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
>
> + bool dp_phy_comp;
Not sure if this is the best way to handle the full mdoeset, could you double
check with Jani?
Other than that I think now the placement in pre_enable_dp is good.
The only other concern I have is changing link rate and lane count only happens
in set_phy_patterns where we write the test link rate and lane count
directly to the link bw set. But the driver's compute config is still configuring
the pipe and plls based on the link rate and lane count that could be
different than the test link rate/count. This might work now since you said
the link layer test is already configuring it at test params but in general
shouldnt we be using the phy->link rate and lane count if in phy compliance
also in compute_config()?
Jani, any thoughts here?
Regards
Manasi
> +
> /*
> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> * will be rejected. Instead look for a better place.
> --
> 2.22.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 1/7] drm/dp: get/set phy compliance pattern
@ 2019-11-18 18:39 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-18 18:39 UTC (permalink / raw)
To: Manasi Navare; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
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@intel.com>
>> ---
>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 1/7] drm/dp: get/set phy compliance pattern
@ 2019-11-18 18:39 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-18 18:39 UTC (permalink / raw)
To: Manasi Navare; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
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@intel.com>
>> ---
>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry
@ 2019-11-18 18:45 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-18 18:45 UTC (permalink / raw)
To: Manasi Navare; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On 11/18/2019 10:36 AM, Manasi Navare wrote:
> On Sun, Nov 17, 2019 at 08:58:45PM -0800, Manasi Navare wrote:
>> On Fri, Nov 15, 2019 at 08:55:46PM +0530, Animesh Manna wrote:
>>> To align with link compliance design existing intel_dp_compliance
>>> tool will be used to get the phy request in userspace through uevent.
>>>
>>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> I would prefer splitting this patch since sending a uevent is more related
>> to the PHY test prep handling and debugfs handling can all be in a separate
>> patch.
>> I prefer this because debugfs nodes might need to change in the future based
>> on more requirements or testing feedback so its better for that to be in separate
>> patch.
>>
>> you could add the hotplug event sending part to the prep patch (3/7) and mention that
>> in the commit message
Sure.
>>
>> Debugfs part looks good to me. Have you tested the debugfs nodes and validated if this
>> information is being written in the correct form?
Tested the test-type entry, planning to add a newline char, otherwise
working.
>>
>> After the split and validation of debugs nodes:
>>
>> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
Thanks.
>>
>> Manasi
>>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_dp.c | 6 ++++--
>>> drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++++
>>> 2 files changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index 338d3744c5d5..a2b860cf3b93 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -5288,8 +5288,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>>>
>>> intel_psr_short_pulse(intel_dp);
>>>
>>> - if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
>>> - DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
>>> + if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING ||
>>> + intel_dp->compliance.test_type ==
>>> + DP_TEST_LINK_PHY_TEST_PATTERN) {
>>> + DRM_DEBUG_KMS("Compliance Test requested\n");
> One more change I think here would be good IMO for debugging is that
> you should print test_type in DEBUG_KMS
Sure.
Regards,
Animesh
>
> Manasi
>
>>> /* Send a Hotplug Uevent to userspace to start modeset */
>>> drm_kms_helper_hotplug_event(&dev_priv->drm);
>>> }
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index cab632791f73..e8b1a8c1015a 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -3212,6 +3212,16 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
>>> intel_dp->compliance.test_data.vdisplay);
>>> seq_printf(m, "bpc: %u\n",
>>> intel_dp->compliance.test_data.bpc);
>>> + } else if (intel_dp->compliance.test_type ==
>>> + DP_TEST_LINK_PHY_TEST_PATTERN) {
>>> + seq_printf(m, "pattern: %d\n",
>>> + intel_dp->compliance.test_data.phytest.phy_pattern);
>>> + seq_printf(m, "Number of lanes: %d\n",
>>> + intel_dp->compliance.test_data.phytest.num_lanes);
>>> + seq_printf(m, "Link Rate: %d\n",
>>> + intel_dp->compliance.test_data.phytest.link_rate);
>>> + seq_printf(m, "level: %02x\n",
>>> + intel_dp->train_set[0]);
>>> }
>>> } else
>>> seq_puts(m, "0");
>>> --
>>> 2.22.0
>>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry
@ 2019-11-18 18:45 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-18 18:45 UTC (permalink / raw)
To: Manasi Navare; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On 11/18/2019 10:36 AM, Manasi Navare wrote:
> On Sun, Nov 17, 2019 at 08:58:45PM -0800, Manasi Navare wrote:
>> On Fri, Nov 15, 2019 at 08:55:46PM +0530, Animesh Manna wrote:
>>> To align with link compliance design existing intel_dp_compliance
>>> tool will be used to get the phy request in userspace through uevent.
>>>
>>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> I would prefer splitting this patch since sending a uevent is more related
>> to the PHY test prep handling and debugfs handling can all be in a separate
>> patch.
>> I prefer this because debugfs nodes might need to change in the future based
>> on more requirements or testing feedback so its better for that to be in separate
>> patch.
>>
>> you could add the hotplug event sending part to the prep patch (3/7) and mention that
>> in the commit message
Sure.
>>
>> Debugfs part looks good to me. Have you tested the debugfs nodes and validated if this
>> information is being written in the correct form?
Tested the test-type entry, planning to add a newline char, otherwise
working.
>>
>> After the split and validation of debugs nodes:
>>
>> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
Thanks.
>>
>> Manasi
>>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_dp.c | 6 ++++--
>>> drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++++
>>> 2 files changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index 338d3744c5d5..a2b860cf3b93 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -5288,8 +5288,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>>>
>>> intel_psr_short_pulse(intel_dp);
>>>
>>> - if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
>>> - DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
>>> + if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING ||
>>> + intel_dp->compliance.test_type ==
>>> + DP_TEST_LINK_PHY_TEST_PATTERN) {
>>> + DRM_DEBUG_KMS("Compliance Test requested\n");
> One more change I think here would be good IMO for debugging is that
> you should print test_type in DEBUG_KMS
Sure.
Regards,
Animesh
>
> Manasi
>
>>> /* Send a Hotplug Uevent to userspace to start modeset */
>>> drm_kms_helper_hotplug_event(&dev_priv->drm);
>>> }
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index cab632791f73..e8b1a8c1015a 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -3212,6 +3212,16 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
>>> intel_dp->compliance.test_data.vdisplay);
>>> seq_printf(m, "bpc: %u\n",
>>> intel_dp->compliance.test_data.bpc);
>>> + } else if (intel_dp->compliance.test_type ==
>>> + DP_TEST_LINK_PHY_TEST_PATTERN) {
>>> + seq_printf(m, "pattern: %d\n",
>>> + intel_dp->compliance.test_data.phytest.phy_pattern);
>>> + seq_printf(m, "Number of lanes: %d\n",
>>> + intel_dp->compliance.test_data.phytest.num_lanes);
>>> + seq_printf(m, "Link Rate: %d\n",
>>> + intel_dp->compliance.test_data.phytest.link_rate);
>>> + seq_printf(m, "level: %02x\n",
>>> + intel_dp->train_set[0]);
>>> }
>>> } else
>>> seq_puts(m, "0");
>>> --
>>> 2.22.0
>>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [RFC 6/7] drm/i915/dp: Update the pattern as per request
@ 2019-11-18 18:47 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-18 18:47 UTC (permalink / raw)
To: Manasi Navare; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On 11/18/2019 12:11 PM, Manasi Navare wrote:
> On Fri, Nov 15, 2019 at 08:55:48PM +0530, Animesh Manna wrote:
>> set pattern in DP_COMP_CTL.
> It would be nice to have some brief description here on
> context of setting a PHY pattern for PHY compliance and that
> this will be called during pre enable in atomic commit in case
> of phy compliance request etc..and that the requested phy pattern
> is read during test handling etc..
> something that gives a reader a better idea of the context of
> this function
>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_dp.c | 55 +++++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index a2b860cf3b93..df31278a1619 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -4955,6 +4955,61 @@ static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
>> return DP_TEST_ACK;
>> }
>>
>> +static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
>> +{
>> + struct drm_i915_private *dev_priv =
>> + to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
>> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> + struct drm_dp_phy_test_params *data =
>> + &intel_dp->compliance.test_data.phytest;
>> + u32 temp;
>> +
>> + switch (data->phy_pattern) {
>> + case DP_PHY_TEST_PATTERN_NONE:
>> + DRM_DEBUG_KMS("Disable Phy Test Pattern\n");
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port), 0x0);
>> + break;
>> + case DP_PHY_TEST_PATTERN_D10_2:
>> + DRM_DEBUG_KMS("Set D10.2 Phy Test Pattern\n");
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2);
>> + break;
>> + case DP_PHY_TEST_PATTERN_ERROR_COUNT:
>> + DRM_DEBUG_KMS("Set Error Count Phy Test Pattern\n");
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE |
>> + DDI_DP_COMP_CTL_SCRAMBLED_0);
>> + break;
>> + case DP_PHY_TEST_PATTERN_PRBS7:
>> + DRM_DEBUG_KMS("Set PRBS7 Phy Test Pattern\n");
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7);
>> + break;
>> + case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
>> + DRM_DEBUG_KMS("Set 80Bit Custom Phy Test Pattern\n");
>> + temp = ((data->custom80[0] << 24) | (data->custom80[1] << 16) |
>> + (data->custom80[2] << 8) | (data->custom80[3]));
>> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 0), temp);
>> + temp = ((data->custom80[4] << 24) | (data->custom80[5] << 16) |
>> + (data->custom80[6] << 8) | (data->custom80[7]));
>> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 1), temp);
>> + temp = ((data->custom80[8] << 8) | data->custom80[9]);
>> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 2), temp);
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_CUSTOM80);
>> + break;
>> + case DP_PHY_TEST_PATTERN_CP2520:
>> + DRM_DEBUG_KMS("Set HBR2 compliance Phy Test Pattern\n");
>> + temp = ((data->hbr2_reset[1] << 8) | data->hbr2_reset[0]);
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
>> + temp);
>> + break;
>> + default:
>> + DRM_ERROR("Invalid Phy Test PAttern\n");
> Here we would definitely want a WARN since ERROR at this point in commit is not good
> Other than looks good to me.
Thanks, will add the suggested changes.
Regards,
Animesh
>
> Manasi
>
>> + }
>> +}
>> +
>> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
>> {
>> u8 test_result = DP_TEST_NAK;
>> --
>> 2.22.0
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 6/7] drm/i915/dp: Update the pattern as per request
@ 2019-11-18 18:47 ` Animesh Manna
0 siblings, 0 replies; 44+ messages in thread
From: Animesh Manna @ 2019-11-18 18:47 UTC (permalink / raw)
To: Manasi Navare; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On 11/18/2019 12:11 PM, Manasi Navare wrote:
> On Fri, Nov 15, 2019 at 08:55:48PM +0530, Animesh Manna wrote:
>> set pattern in DP_COMP_CTL.
> It would be nice to have some brief description here on
> context of setting a PHY pattern for PHY compliance and that
> this will be called during pre enable in atomic commit in case
> of phy compliance request etc..and that the requested phy pattern
> is read during test handling etc..
> something that gives a reader a better idea of the context of
> this function
>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_dp.c | 55 +++++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index a2b860cf3b93..df31278a1619 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -4955,6 +4955,61 @@ static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
>> return DP_TEST_ACK;
>> }
>>
>> +static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
>> +{
>> + struct drm_i915_private *dev_priv =
>> + to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
>> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> + struct drm_dp_phy_test_params *data =
>> + &intel_dp->compliance.test_data.phytest;
>> + u32 temp;
>> +
>> + switch (data->phy_pattern) {
>> + case DP_PHY_TEST_PATTERN_NONE:
>> + DRM_DEBUG_KMS("Disable Phy Test Pattern\n");
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port), 0x0);
>> + break;
>> + case DP_PHY_TEST_PATTERN_D10_2:
>> + DRM_DEBUG_KMS("Set D10.2 Phy Test Pattern\n");
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2);
>> + break;
>> + case DP_PHY_TEST_PATTERN_ERROR_COUNT:
>> + DRM_DEBUG_KMS("Set Error Count Phy Test Pattern\n");
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE |
>> + DDI_DP_COMP_CTL_SCRAMBLED_0);
>> + break;
>> + case DP_PHY_TEST_PATTERN_PRBS7:
>> + DRM_DEBUG_KMS("Set PRBS7 Phy Test Pattern\n");
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7);
>> + break;
>> + case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
>> + DRM_DEBUG_KMS("Set 80Bit Custom Phy Test Pattern\n");
>> + temp = ((data->custom80[0] << 24) | (data->custom80[1] << 16) |
>> + (data->custom80[2] << 8) | (data->custom80[3]));
>> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 0), temp);
>> + temp = ((data->custom80[4] << 24) | (data->custom80[5] << 16) |
>> + (data->custom80[6] << 8) | (data->custom80[7]));
>> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 1), temp);
>> + temp = ((data->custom80[8] << 8) | data->custom80[9]);
>> + I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 2), temp);
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_CUSTOM80);
>> + break;
>> + case DP_PHY_TEST_PATTERN_CP2520:
>> + DRM_DEBUG_KMS("Set HBR2 compliance Phy Test Pattern\n");
>> + temp = ((data->hbr2_reset[1] << 8) | data->hbr2_reset[0]);
>> + I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
>> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
>> + temp);
>> + break;
>> + default:
>> + DRM_ERROR("Invalid Phy Test PAttern\n");
> Here we would definitely want a WARN since ERROR at this point in commit is not good
> Other than looks good to me.
Thanks, will add the suggested changes.
Regards,
Animesh
>
> Manasi
>
>> + }
>> +}
>> +
>> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
>> {
>> u8 test_result = DP_TEST_NAK;
>> --
>> 2.22.0
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 6/7] drm/i915/dp: Update the pattern as per request
2019-11-18 18:47 ` [Intel-gfx] " Animesh Manna
(?)
@ 2019-12-11 23:44 ` Manasi Navare
-1 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2019-12-11 23:44 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
Hi Animesh,
I dont see the patch revision with these fixes, have you sent it to the M-L?
Manasi
On Tue, Nov 19, 2019 at 12:17:03AM +0530, Animesh Manna wrote:
>
>
> On 11/18/2019 12:11 PM, Manasi Navare wrote:
> >On Fri, Nov 15, 2019 at 08:55:48PM +0530, Animesh Manna wrote:
> >>set pattern in DP_COMP_CTL.
> >It would be nice to have some brief description here on
> >context of setting a PHY pattern for PHY compliance and that
> >this will be called during pre enable in atomic commit in case
> >of phy compliance request etc..and that the requested phy pattern
> >is read during test handling etc..
> >something that gives a reader a better idea of the context of
> >this function
> >
> >>Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >>---
> >> drivers/gpu/drm/i915/display/intel_dp.c | 55 +++++++++++++++++++++++++
> >> 1 file changed, 55 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >>index a2b860cf3b93..df31278a1619 100644
> >>--- a/drivers/gpu/drm/i915/display/intel_dp.c
> >>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >>@@ -4955,6 +4955,61 @@ static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
> >> return DP_TEST_ACK;
> >> }
> >>+static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> >>+{
> >>+ struct drm_i915_private *dev_priv =
> >>+ to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> >>+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>+ struct drm_dp_phy_test_params *data =
> >>+ &intel_dp->compliance.test_data.phytest;
> >>+ u32 temp;
> >>+
> >>+ switch (data->phy_pattern) {
> >>+ case DP_PHY_TEST_PATTERN_NONE:
> >>+ DRM_DEBUG_KMS("Disable Phy Test Pattern\n");
> >>+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port), 0x0);
> >>+ break;
> >>+ case DP_PHY_TEST_PATTERN_D10_2:
> >>+ DRM_DEBUG_KMS("Set D10.2 Phy Test Pattern\n");
> >>+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> >>+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2);
> >>+ break;
> >>+ case DP_PHY_TEST_PATTERN_ERROR_COUNT:
> >>+ DRM_DEBUG_KMS("Set Error Count Phy Test Pattern\n");
> >>+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> >>+ DDI_DP_COMP_CTL_ENABLE |
> >>+ DDI_DP_COMP_CTL_SCRAMBLED_0);
> >>+ break;
> >>+ case DP_PHY_TEST_PATTERN_PRBS7:
> >>+ DRM_DEBUG_KMS("Set PRBS7 Phy Test Pattern\n");
> >>+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> >>+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7);
> >>+ break;
> >>+ case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
> >>+ DRM_DEBUG_KMS("Set 80Bit Custom Phy Test Pattern\n");
> >>+ temp = ((data->custom80[0] << 24) | (data->custom80[1] << 16) |
> >>+ (data->custom80[2] << 8) | (data->custom80[3]));
> >>+ I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 0), temp);
> >>+ temp = ((data->custom80[4] << 24) | (data->custom80[5] << 16) |
> >>+ (data->custom80[6] << 8) | (data->custom80[7]));
> >>+ I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 1), temp);
> >>+ temp = ((data->custom80[8] << 8) | data->custom80[9]);
> >>+ I915_WRITE(DDI_DP_COMP_PAT(intel_dig_port->base.port, 2), temp);
> >>+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> >>+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_CUSTOM80);
> >>+ break;
> >>+ case DP_PHY_TEST_PATTERN_CP2520:
> >>+ DRM_DEBUG_KMS("Set HBR2 compliance Phy Test Pattern\n");
> >>+ temp = ((data->hbr2_reset[1] << 8) | data->hbr2_reset[0]);
> >>+ I915_WRITE(DDI_DP_COMP_CTL(intel_dig_port->base.port),
> >>+ DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
> >>+ temp);
> >>+ break;
> >>+ default:
> >>+ DRM_ERROR("Invalid Phy Test PAttern\n");
> >Here we would definitely want a WARN since ERROR at this point in commit is not good
> >Other than looks good to me.
>
> Thanks, will add the suggested changes.
>
> Regards,
> Animesh
> >
> >Manasi
> >
> >>+ }
> >>+}
> >>+
> >> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> >> {
> >> u8 test_result = DP_TEST_NAK;
> >>--
> >>2.22.0
> >>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
2019-11-18 7:53 ` [Intel-gfx] " Manasi Navare
(?)
@ 2019-12-11 23:50 ` Manasi Navare
2019-12-13 17:24 ` Animesh Manna
-1 siblings, 1 reply; 44+ messages in thread
From: Manasi Navare @ 2019-12-11 23:50 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
Hi Animesh/Jani,
Is this the right way to force a full modeset by adding new dev_priv->dp_phy_comp?
Also its still not clear to me how this will work without actualling using the phy link rate
and lane count stored in the phy test pattern params to compute the pipe config?
Currently those only get used directly to set the link bw.
This can work if the phy comp runs after link layer comp but on standalone phy comp testing
this could be a problem.
Also I dont see my comment addressed where I had asked to move the below to patch 3:
> > + /* Set test active flag here so userspace doesn't interrupt things */
> > + intel_dp->compliance.test_active = 1;
> > + dev_priv->dp_phy_comp = true;
> > +
Manasi
On Sun, Nov 17, 2019 at 11:53:54PM -0800, Manasi Navare wrote:
> On Fri, Nov 15, 2019 at 08:55:49PM +0530, Animesh Manna wrote:
> > This patch process phy compliance request by programming requested
> > vswing, pre-emphasis and test pattern.
>
> Again here a slightly detailed description of where in the atomic modeset
> do we process the PHY com request would be good.
>
> The design overall looks good now, few comments below:
>
> >
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++
> > drivers/gpu/drm/i915/display/intel_display.c | 5 ++
> > drivers/gpu/drm/i915/display/intel_dp.c | 77 ++++++++++++++++++++
> > drivers/gpu/drm/i915/display/intel_dp.h | 2 +
> > drivers/gpu/drm/i915/i915_drv.h | 2 +
> > 5 files changed, 100 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 8f817de34460..c12d4ebbd352 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3700,8 +3700,17 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
> > {
> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> > enum pipe pipe = crtc->pipe;
> >
> > + if (encoder->type == INTEL_OUTPUT_DP) {
> > + if (dig_port->dp.compliance.test_type ==
> > + DP_TEST_LINK_PHY_TEST_PATTERN) {
> > + intel_dp_process_phy_request(&dig_port->dp);
> > + return;
> > + }
> > + }
> > +
> > /*
> > * When called from DP MST code:
> > * - conn_state will be NULL
> > @@ -4147,6 +4156,11 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> > enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> > bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
> >
> > + if (encoder->type == INTEL_OUTPUT_DP)
> > + if (dig_port->dp.compliance.test_type ==
> > + DP_TEST_LINK_PHY_TEST_PATTERN)
> > + return;
> > +
> > if (is_tc_port)
> > intel_tc_port_get_link(dig_port, crtc_state->lane_count);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index adf50c4b38ad..4ad11df55f07 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -545,6 +545,11 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
> > static bool
> > needs_modeset(const struct intel_crtc_state *state)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(state->uapi.crtc->dev);
> > +
> > + if (dev_priv->dp_phy_comp)
> > + return true;
>
> Could you double check with Jani N if this is an acceptable solution to
> foce a full modeset for a PHY compliance test?
>
> > +
> > return drm_atomic_crtc_needs_modeset(&state->uapi);
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index df31278a1619..2c3d4b6c6036 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5010,14 +5010,91 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> > }
> > }
> >
> > +static void
> > +intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
> > +{
> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > + struct drm_device *dev = intel_dig_port->base.base.dev;
> > + struct drm_i915_private *dev_priv = to_i915(dev);
> > + enum port port = intel_dig_port->base.port;
> > + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> > +
> > + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> > + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> > + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> > +
> > + ddi_buf_ctl_value &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
> > + dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
> > + trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
> > + DDI_PORT_WIDTH_MASK);
> > +
> > + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> > + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> > + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> > +}
> > +
> > +static void
> > +intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
> > +{
> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > + struct drm_device *dev = intel_dig_port->base.base.dev;
> > + struct drm_i915_private *dev_priv = to_i915(dev);
> > + enum port port = intel_dig_port->base.port;
> > + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> > +
> > + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> > + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> > + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> > +
> > + ddi_buf_ctl_value |= DDI_BUF_CTL_ENABLE |
> > + DDI_PORT_WIDTH(lane_cnt);
> > + dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
> > + trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
> > + DDI_PORT_WIDTH(lane_cnt);
> > +
> > + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> > + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> > + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> > +}
> > +
> > +void intel_dp_process_phy_request(struct intel_dp *intel_dp)
> > +{
> > + struct drm_dp_phy_test_params *data =
> > + &intel_dp->compliance.test_data.phytest;
> > + u8 link_status[DP_LINK_STATUS_SIZE];
> > +
> > + if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > + DRM_DEBUG_KMS("failed to get link status\n");
> > + return;
> > + }
> > +
> > + /* retrieve vswing & pre-emphasis setting */
> > + intel_get_adjust_train(intel_dp, link_status);
> > +
> > + intel_dp_autotest_phy_ddi_disable(intel_dp);
> > +
> > + intel_dp_set_signal_levels(intel_dp);
> > +
> > + intel_dp_phy_pattern_update(intel_dp);
> > +
> > + intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
> > +
> > + drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
> > +}
> > +
> > static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> > {
> > u8 test_result = DP_TEST_NAK;
> > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >
> > test_result = intel_dp_prepare_phytest(intel_dp);
> > if (test_result != DP_TEST_ACK)
> > DRM_ERROR("Phy test preparation failed\n");
> >
> > + /* Set test active flag here so userspace doesn't interrupt things */
> > + intel_dp->compliance.test_active = 1;
> > + dev_priv->dp_phy_comp = true;
> > +
>
> This should be moved to the patch 3 I think where you handle phy test request and
> call prepare function. The same patch that handles test req can set the test active to 1.
>
> > return test_result;
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > index 0d0cb692f701..b1274ecffc7f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -120,6 +120,8 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> > const struct intel_crtc_state *crtc_state,
> > const struct drm_connector_state *conn_state);
> > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > +void intel_dp_process_phy_request(struct intel_dp *intel_dp);
> > +
> >
> > static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> > {
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 1779f600fcfb..232e0dfb9d8e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1285,6 +1285,8 @@ struct drm_i915_private {
> >
> > I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
> >
> > + bool dp_phy_comp;
>
> Not sure if this is the best way to handle the full mdoeset, could you double
> check with Jani?
>
> Other than that I think now the placement in pre_enable_dp is good.
>
> The only other concern I have is changing link rate and lane count only happens
> in set_phy_patterns where we write the test link rate and lane count
> directly to the link bw set. But the driver's compute config is still configuring
> the pipe and plls based on the link rate and lane count that could be
> different than the test link rate/count. This might work now since you said
> the link layer test is already configuring it at test params but in general
> shouldnt we be using the phy->link rate and lane count if in phy compliance
> also in compute_config()?
>
> Jani, any thoughts here?
>
> Regards
> Manasi
>
> > +
> > /*
> > * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> > * will be rejected. Instead look for a better place.
> > --
> > 2.22.0
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
2019-12-11 23:50 ` Manasi Navare
@ 2019-12-13 17:24 ` Animesh Manna
2020-01-14 21:38 ` Manasi Navare
0 siblings, 1 reply; 44+ messages in thread
From: Animesh Manna @ 2019-12-13 17:24 UTC (permalink / raw)
To: Manasi Navare; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
Hi Manasi/Jani,
Thanks for helping phy compliance design.
Added my understanding/doubts below.
On 12/12/2019 5:20 AM, Manasi Navare wrote:
> Hi Animesh/Jani,
>
> Is this the right way to force a full modeset by adding new dev_priv->dp_phy_comp?
In modeset path we disable active crtc first then initiate the modeset
sequence which is causing trouble in updating phy compliance parameters
in modeset path.
As per phy compliance spec test-scope can request to change the
following parameters,
- Test pattern
- V-swing:
- Pre-emphasis:
- Link rate:
- Lane count:
For test-pattern/v-swing/pre-emphasis, active link may not need to cut.
Tried the same in the following patch series where I am skipping
atomic-check, atomic-commit and write these parameters in
atomic-commit-tail without touching anything else. Can you please share
your thought how to handle it properly. Maybe once we are clear about
what we should do in atomic_check/atomic_commit/atomic_commit_tail we
can move dp_phy_comp flag to some atomic-state object.
https://patchwork.freedesktop.org/series/70300/
For Link-rate/lane-count, we need to do link-training/modeset. As per my
observation the request is coming as link-compliance request, still
waiting feedback from compliance team to confirm. So not handled as part
dp-phy-compliance implementation.
> Also its still not clear to me how this will work without actualling using the phy link rate
> and lane count stored in the phy test pattern params to compute the pipe config?
> Currently those only get used directly to set the link bw.
If link-compliance request is not used for link-rate/lane-count change
then your point is valid. As mentioned above getting link-compliance
request which is confusing and waiting confirmation from compliance team.
> This can work if the phy comp runs after link layer comp but on standalone phy comp testing
> this could be a problem.
>
> Also I dont see my comment addressed where I had asked to move the below to patch 3:
>>> + /* Set test active flag here so userspace doesn't interrupt things */
>>> + intel_dp->compliance.test_active = 1;
>>> + dev_priv->dp_phy_comp = true;
>>> +
Thinking of creating a separate patch for sending uevent and setting
test_active flag true for intel_dp_compliance tool.
The patch 3/7 will take the preparation part and acknowledge the test
request. Hope it will be fine.
Regards,
Animesh
> Manasi
>
>
> On Sun, Nov 17, 2019 at 11:53:54PM -0800, Manasi Navare wrote:
>> On Fri, Nov 15, 2019 at 08:55:49PM +0530, Animesh Manna wrote:
>>> This patch process phy compliance request by programming requested
>>> vswing, pre-emphasis and test pattern.
>> Again here a slightly detailed description of where in the atomic modeset
>> do we process the PHY com request would be good.
>>
>> The design overall looks good now, few comments below:
>>
>>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++
>>> drivers/gpu/drm/i915/display/intel_display.c | 5 ++
>>> drivers/gpu/drm/i915/display/intel_dp.c | 77 ++++++++++++++++++++
>>> drivers/gpu/drm/i915/display/intel_dp.h | 2 +
>>> drivers/gpu/drm/i915/i915_drv.h | 2 +
>>> 5 files changed, 100 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>>> index 8f817de34460..c12d4ebbd352 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>>> @@ -3700,8 +3700,17 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
>>> {
>>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>> + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>>> enum pipe pipe = crtc->pipe;
>>>
>>> + if (encoder->type == INTEL_OUTPUT_DP) {
>>> + if (dig_port->dp.compliance.test_type ==
>>> + DP_TEST_LINK_PHY_TEST_PATTERN) {
>>> + intel_dp_process_phy_request(&dig_port->dp);
>>> + return;
>>> + }
>>> + }
>>> +
>>> /*
>>> * When called from DP MST code:
>>> * - conn_state will be NULL
>>> @@ -4147,6 +4156,11 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
>>> enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>>> bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
>>>
>>> + if (encoder->type == INTEL_OUTPUT_DP)
>>> + if (dig_port->dp.compliance.test_type ==
>>> + DP_TEST_LINK_PHY_TEST_PATTERN)
>>> + return;
>>> +
>>> if (is_tc_port)
>>> intel_tc_port_get_link(dig_port, crtc_state->lane_count);
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>> index adf50c4b38ad..4ad11df55f07 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -545,6 +545,11 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
>>> static bool
>>> needs_modeset(const struct intel_crtc_state *state)
>>> {
>>> + struct drm_i915_private *dev_priv = to_i915(state->uapi.crtc->dev);
>>> +
>>> + if (dev_priv->dp_phy_comp)
>>> + return true;
>> Could you double check with Jani N if this is an acceptable solution to
>> foce a full modeset for a PHY compliance test?
>>
>>> +
>>> return drm_atomic_crtc_needs_modeset(&state->uapi);
>>> }
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index df31278a1619..2c3d4b6c6036 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -5010,14 +5010,91 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
>>> }
>>> }
>>>
>>> +static void
>>> +intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
>>> +{
>>> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>> + struct drm_device *dev = intel_dig_port->base.base.dev;
>>> + struct drm_i915_private *dev_priv = to_i915(dev);
>>> + enum port port = intel_dig_port->base.port;
>>> + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
>>> +
>>> + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
>>> + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
>>> + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
>>> +
>>> + ddi_buf_ctl_value &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
>>> + dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
>>> + trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
>>> + DDI_PORT_WIDTH_MASK);
>>> +
>>> + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
>>> + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
>>> + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
>>> +}
>>> +
>>> +static void
>>> +intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
>>> +{
>>> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>> + struct drm_device *dev = intel_dig_port->base.base.dev;
>>> + struct drm_i915_private *dev_priv = to_i915(dev);
>>> + enum port port = intel_dig_port->base.port;
>>> + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
>>> +
>>> + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
>>> + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
>>> + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
>>> +
>>> + ddi_buf_ctl_value |= DDI_BUF_CTL_ENABLE |
>>> + DDI_PORT_WIDTH(lane_cnt);
>>> + dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
>>> + trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
>>> + DDI_PORT_WIDTH(lane_cnt);
>>> +
>>> + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
>>> + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
>>> + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
>>> +}
>>> +
>>> +void intel_dp_process_phy_request(struct intel_dp *intel_dp)
>>> +{
>>> + struct drm_dp_phy_test_params *data =
>>> + &intel_dp->compliance.test_data.phytest;
>>> + u8 link_status[DP_LINK_STATUS_SIZE];
>>> +
>>> + if (!intel_dp_get_link_status(intel_dp, link_status)) {
>>> + DRM_DEBUG_KMS("failed to get link status\n");
>>> + return;
>>> + }
>>> +
>>> + /* retrieve vswing & pre-emphasis setting */
>>> + intel_get_adjust_train(intel_dp, link_status);
>>> +
>>> + intel_dp_autotest_phy_ddi_disable(intel_dp);
>>> +
>>> + intel_dp_set_signal_levels(intel_dp);
>>> +
>>> + intel_dp_phy_pattern_update(intel_dp);
>>> +
>>> + intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
>>> +
>>> + drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
>>> +}
>>> +
>>> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
>>> {
>>> u8 test_result = DP_TEST_NAK;
>>> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>>>
>>> test_result = intel_dp_prepare_phytest(intel_dp);
>>> if (test_result != DP_TEST_ACK)
>>> DRM_ERROR("Phy test preparation failed\n");
>>>
>>> + /* Set test active flag here so userspace doesn't interrupt things */
>>> + intel_dp->compliance.test_active = 1;
>>> + dev_priv->dp_phy_comp = true;
>>> +
>> This should be moved to the patch 3 I think where you handle phy test request and
>> call prepare function. The same patch that handles test req can set the test active to 1.
>>
>>> return test_result;
>>> }
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>>> index 0d0cb692f701..b1274ecffc7f 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>>> @@ -120,6 +120,8 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
>>> const struct intel_crtc_state *crtc_state,
>>> const struct drm_connector_state *conn_state);
>>> bool intel_digital_port_connected(struct intel_encoder *encoder);
>>> +void intel_dp_process_phy_request(struct intel_dp *intel_dp);
>>> +
>>>
>>> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
>>> {
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index 1779f600fcfb..232e0dfb9d8e 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -1285,6 +1285,8 @@ struct drm_i915_private {
>>>
>>> I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
>>>
>>> + bool dp_phy_comp;
>> Not sure if this is the best way to handle the full mdoeset, could you double
>> check with Jani?
>>
>> Other than that I think now the placement in pre_enable_dp is good.
>>
>> The only other concern I have is changing link rate and lane count only happens
>> in set_phy_patterns where we write the test link rate and lane count
>> directly to the link bw set. But the driver's compute config is still configuring
>> the pipe and plls based on the link rate and lane count that could be
>> different than the test link rate/count. This might work now since you said
>> the link layer test is already configuring it at test params but in general
>> shouldnt we be using the phy->link rate and lane count if in phy compliance
>> also in compute_config()?
>>
>> Jani, any thoughts here?
>>
>> Regards
>> Manasi
>>
>>> +
>>> /*
>>> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>>> * will be rejected. Instead look for a better place.
>>> --
>>> 2.22.0
>>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
2019-12-13 17:24 ` Animesh Manna
@ 2020-01-14 21:38 ` Manasi Navare
2020-01-20 13:53 ` Manna, Animesh
0 siblings, 1 reply; 44+ messages in thread
From: Manasi Navare @ 2020-01-14 21:38 UTC (permalink / raw)
To: Animesh Manna; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Fri, Dec 13, 2019 at 10:54:20PM +0530, Animesh Manna wrote:
> Hi Manasi/Jani,
>
> Thanks for helping phy compliance design.
>
> Added my understanding/doubts below.
>
>
> On 12/12/2019 5:20 AM, Manasi Navare wrote:
> >Hi Animesh/Jani,
> >
> >Is this the right way to force a full modeset by adding new dev_priv->dp_phy_comp?
>
> In modeset path we disable active crtc first then initiate the modeset
> sequence which is causing trouble in updating phy compliance parameters in
> modeset path.
> As per phy compliance spec test-scope can request to change the following
> parameters,
> - Test pattern
> - V-swing:
> - Pre-emphasis:
> - Link rate:
> - Lane count:
>
> For test-pattern/v-swing/pre-emphasis, active link may not need to cut.
> Tried the same in the following patch series where I am skipping
> atomic-check, atomic-commit and write these parameters in atomic-commit-tail
> without touching anything else. Can you please share your thought how to
> handle it properly. Maybe once we are clear about what we should do in
> atomic_check/atomic_commit/atomic_commit_tail we can move dp_phy_comp flag
> to some atomic-state object.
> https://patchwork.freedesktop.org/series/70300/
So from what we had finalized on the call, was that the intel_dp_process_phy_request() would be
called in ddi_pre_enable_dp in case of dP PHY request for which you would need
to update needs_modeset() and set it to true if PHY compliance request.
That looked like a correct approach to me and AFAIR was agreed by Jani and Uma as well.
And that was decided so that this gets plugged into out atomic infrastructure.
Are you now suggesting that with that approach or the code in this patch, PHY compliance
tests for changing test-pattern/vswing/pre - emphasis are not working?
Why did you want to change the design to skip atomic check and atomic commit?
If we skip the atomic check and atomic commit and directly write these parameters in atomic commit tail
then we are essentially doing - 1. Get Short pulse, read DPCD, write the test parameters and not going
through atomic KMS ever.
And we will need to have another meeting to see if this will be accepted without going through atomic.
This might work if the PHY compliance does not expect any other crtc_state parameter change no
change in link rate/lane count or PLLs.
This brings us to our next discussion, where we need to confirm from PHY compliance Chrome team:
1. Does PHY Compliance test request require link training for requested link rate/lane count (There should be a box in
Software capabilities in DPR 100 they are using that will give this info) Clint here confimed that it does say
link training required
2. If link training required box checked, Chrome team needs to confirm if a link layer compliance request
is sent prior to every PHY request (from dmesg logs or DPR 100 logs)
3. If link layer comp request is set, then that should handle changing link rate and setting PLLs
4. In this case then we in PHY compliance do not needs to go through atomic check and commit we can
skip that and directly write the test patter/.vswing/pre-emaphasis and rather just call
the intel_dp_process_phy_request() directly on test req short pulse processing like your original design.
Animesh, please work on getting these clarifications from Chrome team and we can close
on these opens.
Meanwhile I am trying to get the DPR 100 and test equipment and do the testing myself here to
understand the flow.
Manasi
>
> For Link-rate/lane-count, we need to do link-training/modeset. As per my
> observation the request is coming as link-compliance request, still waiting
> feedback from compliance team to confirm. So not handled as part
> dp-phy-compliance implementation.
Yes we need to push the Chrome OS team to get clarification on this:
- Does PHY compliance req always send a link compliance request before requesting link rate/lane count change?
Once we get this clarification we will
> >Also its still not clear to me how this will work without actualling using the phy link rate
> >and lane count stored in the phy test pattern params to compute the pipe config?
> >Currently those only get used directly to set the link bw.
>
> If link-compliance request is not used for link-rate/lane-count change then
> your point is valid. As mentioned above getting link-compliance request
> which is confusing and waiting confirmation from compliance team.
>
> >This can work if the phy comp runs after link layer comp but on standalone phy comp testing
> >this could be a problem.
> >
> >Also I dont see my comment addressed where I had asked to move the below to patch 3:
> >>>+ /* Set test active flag here so userspace doesn't interrupt things */
> >>>+ intel_dp->compliance.test_active = 1;
> >>>+ dev_priv->dp_phy_comp = true;
> >>>+
>
> Thinking of creating a separate patch for sending uevent and setting
> test_active flag true for intel_dp_compliance tool.
> The patch 3/7 will take the preparation part and acknowledge the test
> request. Hope it will be fine.
>
> Regards,
> Animesh
>
> >Manasi
> >
> >
> >On Sun, Nov 17, 2019 at 11:53:54PM -0800, Manasi Navare wrote:
> >>On Fri, Nov 15, 2019 at 08:55:49PM +0530, Animesh Manna wrote:
> >>>This patch process phy compliance request by programming requested
> >>>vswing, pre-emphasis and test pattern.
> >>Again here a slightly detailed description of where in the atomic modeset
> >>do we process the PHY com request would be good.
> >>
> >>The design overall looks good now, few comments below:
> >>
> >>>Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >>>---
> >>> drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++
> >>> drivers/gpu/drm/i915/display/intel_display.c | 5 ++
> >>> drivers/gpu/drm/i915/display/intel_dp.c | 77 ++++++++++++++++++++
> >>> drivers/gpu/drm/i915/display/intel_dp.h | 2 +
> >>> drivers/gpu/drm/i915/i915_drv.h | 2 +
> >>> 5 files changed, 100 insertions(+)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> >>>index 8f817de34460..c12d4ebbd352 100644
> >>>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
> >>>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> >>>@@ -3700,8 +3700,17 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
> >>> {
> >>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> >>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >>>+ struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >>> enum pipe pipe = crtc->pipe;
> >>>+ if (encoder->type == INTEL_OUTPUT_DP) {
> >>>+ if (dig_port->dp.compliance.test_type ==
> >>>+ DP_TEST_LINK_PHY_TEST_PATTERN) {
> >>>+ intel_dp_process_phy_request(&dig_port->dp);
> >>>+ return;
> >>>+ }
> >>>+ }
> >>>+
> >>> /*
> >>> * When called from DP MST code:
> >>> * - conn_state will be NULL
> >>>@@ -4147,6 +4156,11 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> >>> enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> >>> bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
> >>>+ if (encoder->type == INTEL_OUTPUT_DP)
> >>>+ if (dig_port->dp.compliance.test_type ==
> >>>+ DP_TEST_LINK_PHY_TEST_PATTERN)
> >>>+ return;
> >>>+
> >>> if (is_tc_port)
> >>> intel_tc_port_get_link(dig_port, crtc_state->lane_count);
> >>>diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >>>index adf50c4b38ad..4ad11df55f07 100644
> >>>--- a/drivers/gpu/drm/i915/display/intel_display.c
> >>>+++ b/drivers/gpu/drm/i915/display/intel_display.c
> >>>@@ -545,6 +545,11 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
> >>> static bool
> >>> needs_modeset(const struct intel_crtc_state *state)
> >>> {
> >>>+ struct drm_i915_private *dev_priv = to_i915(state->uapi.crtc->dev);
> >>>+
> >>>+ if (dev_priv->dp_phy_comp)
> >>>+ return true;
> >>Could you double check with Jani N if this is an acceptable solution to
> >>foce a full modeset for a PHY compliance test?
> >>
> >>>+
> >>> return drm_atomic_crtc_needs_modeset(&state->uapi);
> >>> }
> >>>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >>>index df31278a1619..2c3d4b6c6036 100644
> >>>--- a/drivers/gpu/drm/i915/display/intel_dp.c
> >>>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >>>@@ -5010,14 +5010,91 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> >>> }
> >>> }
> >>>+static void
> >>>+intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
> >>>+{
> >>>+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>>+ struct drm_device *dev = intel_dig_port->base.base.dev;
> >>>+ struct drm_i915_private *dev_priv = to_i915(dev);
> >>>+ enum port port = intel_dig_port->base.port;
> >>>+ u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> >>>+
> >>>+ ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> >>>+ dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> >>>+ trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> >>>+
> >>>+ ddi_buf_ctl_value &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
> >>>+ dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
> >>>+ trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
> >>>+ DDI_PORT_WIDTH_MASK);
> >>>+
> >>>+ I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> >>>+ I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> >>>+ I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> >>>+}
> >>>+
> >>>+static void
> >>>+intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
> >>>+{
> >>>+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>>+ struct drm_device *dev = intel_dig_port->base.base.dev;
> >>>+ struct drm_i915_private *dev_priv = to_i915(dev);
> >>>+ enum port port = intel_dig_port->base.port;
> >>>+ u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> >>>+
> >>>+ ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> >>>+ dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> >>>+ trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> >>>+
> >>>+ ddi_buf_ctl_value |= DDI_BUF_CTL_ENABLE |
> >>>+ DDI_PORT_WIDTH(lane_cnt);
> >>>+ dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
> >>>+ trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
> >>>+ DDI_PORT_WIDTH(lane_cnt);
> >>>+
> >>>+ I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> >>>+ I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> >>>+ I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> >>>+}
> >>>+
> >>>+void intel_dp_process_phy_request(struct intel_dp *intel_dp)
> >>>+{
> >>>+ struct drm_dp_phy_test_params *data =
> >>>+ &intel_dp->compliance.test_data.phytest;
> >>>+ u8 link_status[DP_LINK_STATUS_SIZE];
> >>>+
> >>>+ if (!intel_dp_get_link_status(intel_dp, link_status)) {
> >>>+ DRM_DEBUG_KMS("failed to get link status\n");
> >>>+ return;
> >>>+ }
> >>>+
> >>>+ /* retrieve vswing & pre-emphasis setting */
> >>>+ intel_get_adjust_train(intel_dp, link_status);
> >>>+
> >>>+ intel_dp_autotest_phy_ddi_disable(intel_dp);
> >>>+
> >>>+ intel_dp_set_signal_levels(intel_dp);
> >>>+
> >>>+ intel_dp_phy_pattern_update(intel_dp);
> >>>+
> >>>+ intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
> >>>+
> >>>+ drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
> >>>+}
> >>>+
> >>> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> >>> {
> >>> u8 test_result = DP_TEST_NAK;
> >>>+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >>> test_result = intel_dp_prepare_phytest(intel_dp);
> >>> if (test_result != DP_TEST_ACK)
> >>> DRM_ERROR("Phy test preparation failed\n");
> >>>+ /* Set test active flag here so userspace doesn't interrupt things */
> >>>+ intel_dp->compliance.test_active = 1;
> >>>+ dev_priv->dp_phy_comp = true;
> >>>+
> >>This should be moved to the patch 3 I think where you handle phy test request and
> >>call prepare function. The same patch that handles test req can set the test active to 1.
> >>
> >>> return test_result;
> >>> }
> >>>diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> >>>index 0d0cb692f701..b1274ecffc7f 100644
> >>>--- a/drivers/gpu/drm/i915/display/intel_dp.h
> >>>+++ b/drivers/gpu/drm/i915/display/intel_dp.h
> >>>@@ -120,6 +120,8 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> >>> const struct intel_crtc_state *crtc_state,
> >>> const struct drm_connector_state *conn_state);
> >>> bool intel_digital_port_connected(struct intel_encoder *encoder);
> >>>+void intel_dp_process_phy_request(struct intel_dp *intel_dp);
> >>>+
> >>> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> >>> {
> >>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>index 1779f600fcfb..232e0dfb9d8e 100644
> >>>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>@@ -1285,6 +1285,8 @@ struct drm_i915_private {
> >>> I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
> >>>+ bool dp_phy_comp;
> >>Not sure if this is the best way to handle the full mdoeset, could you double
> >>check with Jani?
> >>
> >>Other than that I think now the placement in pre_enable_dp is good.
> >>
> >>The only other concern I have is changing link rate and lane count only happens
> >>in set_phy_patterns where we write the test link rate and lane count
> >>directly to the link bw set. But the driver's compute config is still configuring
> >>the pipe and plls based on the link rate and lane count that could be
> >>different than the test link rate/count. This might work now since you said
> >>the link layer test is already configuring it at test params but in general
> >>shouldnt we be using the phy->link rate and lane count if in phy compliance
> >>also in compute_config()?
> >>
> >>Jani, any thoughts here?
> >>
> >>Regards
> >>Manasi
> >>
> >>>+
> >>> /*
> >>> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> >>> * will be rejected. Instead look for a better place.
> >>>--
> >>>2.22.0
> >>>
> >>_______________________________________________
> >>Intel-gfx mailing list
> >>Intel-gfx@lists.freedesktop.org
> >>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
2020-01-14 21:38 ` Manasi Navare
@ 2020-01-20 13:53 ` Manna, Animesh
2020-01-24 0:56 ` Manasi Navare
0 siblings, 1 reply; 44+ messages in thread
From: Manna, Animesh @ 2020-01-20 13:53 UTC (permalink / raw)
To: Manasi Navare; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On 15-01-2020 03:08, Manasi Navare wrote:
> On Fri, Dec 13, 2019 at 10:54:20PM +0530, Animesh Manna wrote:
>> Hi Manasi/Jani,
>>
>> Thanks for helping phy compliance design.
>>
>> Added my understanding/doubts below.
>>
>>
>> On 12/12/2019 5:20 AM, Manasi Navare wrote:
>>> Hi Animesh/Jani,
>>>
>>> Is this the right way to force a full modeset by adding new dev_priv->dp_phy_comp?
>> In modeset path we disable active crtc first then initiate the modeset
>> sequence which is causing trouble in updating phy compliance parameters in
>> modeset path.
>> As per phy compliance spec test-scope can request to change the following
>> parameters,
>> - Test pattern
>> - V-swing:
>> - Pre-emphasis:
>> - Link rate:
>> - Lane count:
>>
>> For test-pattern/v-swing/pre-emphasis, active link may not need to cut.
>> Tried the same in the following patch series where I am skipping
>> atomic-check, atomic-commit and write these parameters in atomic-commit-tail
>> without touching anything else. Can you please share your thought how to
>> handle it properly. Maybe once we are clear about what we should do in
>> atomic_check/atomic_commit/atomic_commit_tail we can move dp_phy_comp flag
>> to some atomic-state object.
>> https://patchwork.freedesktop.org/series/70300/
> So from what we had finalized on the call, was that the intel_dp_process_phy_request() would be
> called in ddi_pre_enable_dp in case of dP PHY request for which you would need
> to update needs_modeset() and set it to true if PHY compliance request.
> That looked like a correct approach to me and AFAIR was agreed by Jani and Uma as well.
> And that was decided so that this gets plugged into out atomic infrastructure.
Agree.
>
> Are you now suggesting that with that approach or the code in this patch, PHY compliance
> tests for changing test-pattern/vswing/pre - emphasis are not working?
PHY compliance request is not a real modeset. As we are touching
multiple thing in atomic_commit_tail() so the link will be broken broken.
> Why did you want to change the design to skip atomic check and atomic commit?
For changing test-pattern/v-swing/pre-emphasis, want to avoid touching anything which may broke existing configured dp-link.
In atomic-check first am checking for dp_phy_comp flag. if set, no need to check anything else and return success.
Eventually land into atomic_commit and later commit-tail. In atomic-commit-tail check for dp_phy_comp flag. If set,
just process the phy request, not by encoder-hooks as do not want to touch any pll-code/power-well etc ..
This looks to me skipping everything what we do in atomic-check/atomic-commit.
> If we skip the atomic check and atomic commit and directly write these parameters in atomic commit tail
> then we are essentially doing - 1. Get Short pulse, read DPCD, write the test parameters and not going
> through atomic KMS ever.
> And we will need to have another meeting to see if this will be accepted without going through atomic.
> This might work if the PHY compliance does not expect any other crtc_state parameter change no
> change in link rate/lane count or PLLs.
>
> This brings us to our next discussion, where we need to confirm from PHY compliance Chrome team:
> 1. Does PHY Compliance test request require link training for requested link rate/lane count (There should be a box in
> Software capabilities in DPR 100 they are using that will give this info) Clint here confimed that it does say
> link training required
> 2. If link training required box checked, Chrome team needs to confirm if a link layer compliance request
> is sent prior to every PHY request (from dmesg logs or DPR 100 logs)
> 3. If link layer comp request is set, then that should handle changing link rate and setting PLLs
> 4. In this case then we in PHY compliance do not needs to go through atomic check and commit we can
> skip that and directly write the test patter/.vswing/pre-emaphasis and rather just call
> the intel_dp_process_phy_request() directly on test req short pulse processing like your original design.
Agree, please let me know if can go ahead with previous design.
>
> Animesh, please work on getting these clarifications from Chrome team and we can close
> on these opens.
> Meanwhile I am trying to get the DPR 100 and test equipment and do the testing myself here to
> understand the flow.
Thanks Manasi.
Regards,
Animesh
>
> Manasi
>
>
>> For Link-rate/lane-count, we need to do link-training/modeset. As per my
>> observation the request is coming as link-compliance request, still waiting
>> feedback from compliance team to confirm. So not handled as part
>> dp-phy-compliance implementation.
> Yes we need to push the Chrome OS team to get clarification on this:
> - Does PHY compliance req always send a link compliance request before requesting link rate/lane count change?
>
> Once we get this clarification we will
>>> Also its still not clear to me how this will work without actualling using the phy link rate
>>> and lane count stored in the phy test pattern params to compute the pipe config?
>>> Currently those only get used directly to set the link bw.
>> If link-compliance request is not used for link-rate/lane-count change then
>> your point is valid. As mentioned above getting link-compliance request
>> which is confusing and waiting confirmation from compliance team.
>>
>>> This can work if the phy comp runs after link layer comp but on standalone phy comp testing
>>> this could be a problem.
>>>
>>> Also I dont see my comment addressed where I had asked to move the below to patch 3:
>>>>> + /* Set test active flag here so userspace doesn't interrupt things */
>>>>> + intel_dp->compliance.test_active = 1;
>>>>> + dev_priv->dp_phy_comp = true;
>>>>> +
>> Thinking of creating a separate patch for sending uevent and setting
>> test_active flag true for intel_dp_compliance tool.
>> The patch 3/7 will take the preparation part and acknowledge the test
>> request. Hope it will be fine.
>>
>> Regards,
>> Animesh
>>
>>> Manasi
>>>
>>>
>>> On Sun, Nov 17, 2019 at 11:53:54PM -0800, Manasi Navare wrote:
>>>> On Fri, Nov 15, 2019 at 08:55:49PM +0530, Animesh Manna wrote:
>>>>> This patch process phy compliance request by programming requested
>>>>> vswing, pre-emphasis and test pattern.
>>>> Again here a slightly detailed description of where in the atomic modeset
>>>> do we process the PHY com request would be good.
>>>>
>>>> The design overall looks good now, few comments below:
>>>>
>>>>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++
>>>>> drivers/gpu/drm/i915/display/intel_display.c | 5 ++
>>>>> drivers/gpu/drm/i915/display/intel_dp.c | 77 ++++++++++++++++++++
>>>>> drivers/gpu/drm/i915/display/intel_dp.h | 2 +
>>>>> drivers/gpu/drm/i915/i915_drv.h | 2 +
>>>>> 5 files changed, 100 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>>>>> index 8f817de34460..c12d4ebbd352 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>>>>> @@ -3700,8 +3700,17 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
>>>>> {
>>>>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>>>>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>>>> + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>>>>> enum pipe pipe = crtc->pipe;
>>>>> + if (encoder->type == INTEL_OUTPUT_DP) {
>>>>> + if (dig_port->dp.compliance.test_type ==
>>>>> + DP_TEST_LINK_PHY_TEST_PATTERN) {
>>>>> + intel_dp_process_phy_request(&dig_port->dp);
>>>>> + return;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> /*
>>>>> * When called from DP MST code:
>>>>> * - conn_state will be NULL
>>>>> @@ -4147,6 +4156,11 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
>>>>> enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>>>>> bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
>>>>> + if (encoder->type == INTEL_OUTPUT_DP)
>>>>> + if (dig_port->dp.compliance.test_type ==
>>>>> + DP_TEST_LINK_PHY_TEST_PATTERN)
>>>>> + return;
>>>>> +
>>>>> if (is_tc_port)
>>>>> intel_tc_port_get_link(dig_port, crtc_state->lane_count);
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>>>> index adf50c4b38ad..4ad11df55f07 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>>> @@ -545,6 +545,11 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
>>>>> static bool
>>>>> needs_modeset(const struct intel_crtc_state *state)
>>>>> {
>>>>> + struct drm_i915_private *dev_priv = to_i915(state->uapi.crtc->dev);
>>>>> +
>>>>> + if (dev_priv->dp_phy_comp)
>>>>> + return true;
>>>> Could you double check with Jani N if this is an acceptable solution to
>>>> foce a full modeset for a PHY compliance test?
>>>>
>>>>> +
>>>>> return drm_atomic_crtc_needs_modeset(&state->uapi);
>>>>> }
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> index df31278a1619..2c3d4b6c6036 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> @@ -5010,14 +5010,91 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
>>>>> }
>>>>> }
>>>>> +static void
>>>>> +intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
>>>>> +{
>>>>> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>>>> + struct drm_device *dev = intel_dig_port->base.base.dev;
>>>>> + struct drm_i915_private *dev_priv = to_i915(dev);
>>>>> + enum port port = intel_dig_port->base.port;
>>>>> + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
>>>>> +
>>>>> + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
>>>>> + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
>>>>> + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
>>>>> +
>>>>> + ddi_buf_ctl_value &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
>>>>> + dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
>>>>> + trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
>>>>> + DDI_PORT_WIDTH_MASK);
>>>>> +
>>>>> + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
>>>>> + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
>>>>> + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
>>>>> +}
>>>>> +
>>>>> +static void
>>>>> +intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
>>>>> +{
>>>>> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>>>> + struct drm_device *dev = intel_dig_port->base.base.dev;
>>>>> + struct drm_i915_private *dev_priv = to_i915(dev);
>>>>> + enum port port = intel_dig_port->base.port;
>>>>> + u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
>>>>> +
>>>>> + ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
>>>>> + dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
>>>>> + trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
>>>>> +
>>>>> + ddi_buf_ctl_value |= DDI_BUF_CTL_ENABLE |
>>>>> + DDI_PORT_WIDTH(lane_cnt);
>>>>> + dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
>>>>> + trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
>>>>> + DDI_PORT_WIDTH(lane_cnt);
>>>>> +
>>>>> + I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
>>>>> + I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
>>>>> + I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
>>>>> +}
>>>>> +
>>>>> +void intel_dp_process_phy_request(struct intel_dp *intel_dp)
>>>>> +{
>>>>> + struct drm_dp_phy_test_params *data =
>>>>> + &intel_dp->compliance.test_data.phytest;
>>>>> + u8 link_status[DP_LINK_STATUS_SIZE];
>>>>> +
>>>>> + if (!intel_dp_get_link_status(intel_dp, link_status)) {
>>>>> + DRM_DEBUG_KMS("failed to get link status\n");
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + /* retrieve vswing & pre-emphasis setting */
>>>>> + intel_get_adjust_train(intel_dp, link_status);
>>>>> +
>>>>> + intel_dp_autotest_phy_ddi_disable(intel_dp);
>>>>> +
>>>>> + intel_dp_set_signal_levels(intel_dp);
>>>>> +
>>>>> + intel_dp_phy_pattern_update(intel_dp);
>>>>> +
>>>>> + intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
>>>>> +
>>>>> + drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
>>>>> +}
>>>>> +
>>>>> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
>>>>> {
>>>>> u8 test_result = DP_TEST_NAK;
>>>>> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>>>>> test_result = intel_dp_prepare_phytest(intel_dp);
>>>>> if (test_result != DP_TEST_ACK)
>>>>> DRM_ERROR("Phy test preparation failed\n");
>>>>> + /* Set test active flag here so userspace doesn't interrupt things */
>>>>> + intel_dp->compliance.test_active = 1;
>>>>> + dev_priv->dp_phy_comp = true;
>>>>> +
>>>> This should be moved to the patch 3 I think where you handle phy test request and
>>>> call prepare function. The same patch that handles test req can set the test active to 1.
>>>>
>>>>> return test_result;
>>>>> }
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>>>>> index 0d0cb692f701..b1274ecffc7f 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>>>>> @@ -120,6 +120,8 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
>>>>> const struct intel_crtc_state *crtc_state,
>>>>> const struct drm_connector_state *conn_state);
>>>>> bool intel_digital_port_connected(struct intel_encoder *encoder);
>>>>> +void intel_dp_process_phy_request(struct intel_dp *intel_dp);
>>>>> +
>>>>> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
>>>>> {
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>>> index 1779f600fcfb..232e0dfb9d8e 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -1285,6 +1285,8 @@ struct drm_i915_private {
>>>>> I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
>>>>> + bool dp_phy_comp;
>>>> Not sure if this is the best way to handle the full mdoeset, could you double
>>>> check with Jani?
>>>>
>>>> Other than that I think now the placement in pre_enable_dp is good.
>>>>
>>>> The only other concern I have is changing link rate and lane count only happens
>>>> in set_phy_patterns where we write the test link rate and lane count
>>>> directly to the link bw set. But the driver's compute config is still configuring
>>>> the pipe and plls based on the link rate and lane count that could be
>>>> different than the test link rate/count. This might work now since you said
>>>> the link layer test is already configuring it at test params but in general
>>>> shouldnt we be using the phy->link rate and lane count if in phy compliance
>>>> also in compute_config()?
>>>>
>>>> Jani, any thoughts here?
>>>>
>>>> Regards
>>>> Manasi
>>>>
>>>>> +
>>>>> /*
>>>>> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>>>>> * will be rejected. Instead look for a better place.
>>>>> --
>>>>> 2.22.0
>>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Intel-gfx] [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
2020-01-20 13:53 ` Manna, Animesh
@ 2020-01-24 0:56 ` Manasi Navare
0 siblings, 0 replies; 44+ messages in thread
From: Manasi Navare @ 2020-01-24 0:56 UTC (permalink / raw)
To: Manna, Animesh; +Cc: jani.nikula, nidhi1.gupta, intel-gfx
On Mon, Jan 20, 2020 at 07:23:57PM +0530, Manna, Animesh wrote:
> On 15-01-2020 03:08, Manasi Navare wrote:
> >On Fri, Dec 13, 2019 at 10:54:20PM +0530, Animesh Manna wrote:
> >>Hi Manasi/Jani,
> >>
> >>Thanks for helping phy compliance design.
> >>
> >>Added my understanding/doubts below.
> >>
> >>
> >>On 12/12/2019 5:20 AM, Manasi Navare wrote:
> >>>Hi Animesh/Jani,
> >>>
> >>>Is this the right way to force a full modeset by adding new dev_priv->dp_phy_comp?
> >>In modeset path we disable active crtc first then initiate the modeset
> >>sequence which is causing trouble in updating phy compliance parameters in
> >>modeset path.
> >>As per phy compliance spec test-scope can request to change the following
> >>parameters,
> >>- Test pattern
> >>- V-swing:
> >>- Pre-emphasis:
> >>- Link rate:
> >>- Lane count:
> >>
> >>For test-pattern/v-swing/pre-emphasis, active link may not need to cut.
> >>Tried the same in the following patch series where I am skipping
> >>atomic-check, atomic-commit and write these parameters in atomic-commit-tail
> >>without touching anything else. Can you please share your thought how to
> >>handle it properly. Maybe once we are clear about what we should do in
> >>atomic_check/atomic_commit/atomic_commit_tail we can move dp_phy_comp flag
> >>to some atomic-state object.
> >>https://patchwork.freedesktop.org/series/70300/
> >So from what we had finalized on the call, was that the intel_dp_process_phy_request() would be
> >called in ddi_pre_enable_dp in case of dP PHY request for which you would need
> >to update needs_modeset() and set it to true if PHY compliance request.
> >That looked like a correct approach to me and AFAIR was agreed by Jani and Uma as well.
> >And that was decided so that this gets plugged into out atomic infrastructure.
>
> Agree.
>
> >
> >Are you now suggesting that with that approach or the code in this patch, PHY compliance
> >tests for changing test-pattern/vswing/pre - emphasis are not working?
>
> PHY compliance request is not a real modeset. As we are touching multiple
> thing in atomic_commit_tail() so the link will be broken broken.
>
> >Why did you want to change the design to skip atomic check and atomic commit?
>
> For changing test-pattern/v-swing/pre-emphasis, want to avoid touching anything which may broke existing configured dp-link.
> In atomic-check first am checking for dp_phy_comp flag. if set, no need to check anything else and return success.
> Eventually land into atomic_commit and later commit-tail. In atomic-commit-tail check for dp_phy_comp flag. If set,
> just process the phy request, not by encoder-hooks as do not want to touch any pll-code/power-well etc ..
> This looks to me skipping everything what we do in atomic-check/atomic-commit.
>
> >If we skip the atomic check and atomic commit and directly write these parameters in atomic commit tail
> >then we are essentially doing - 1. Get Short pulse, read DPCD, write the test parameters and not going
> >through atomic KMS ever.
> >And we will need to have another meeting to see if this will be accepted without going through atomic.
> >This might work if the PHY compliance does not expect any other crtc_state parameter change no
> >change in link rate/lane count or PLLs.
> >
> >This brings us to our next discussion, where we need to confirm from PHY compliance Chrome team:
> >1. Does PHY Compliance test request require link training for requested link rate/lane count (There should be a box in
> >Software capabilities in DPR 100 they are using that will give this info) Clint here confimed that it does say
> >link training required
> >2. If link training required box checked, Chrome team needs to confirm if a link layer compliance request
> >is sent prior to every PHY request (from dmesg logs or DPR 100 logs)
> >3. If link layer comp request is set, then that should handle changing link rate and setting PLLs
> >4. In this case then we in PHY compliance do not needs to go through atomic check and commit we can
> >skip that and directly write the test patter/.vswing/pre-emaphasis and rather just call
> >the intel_dp_process_phy_request() directly on test req short pulse processing like your original design.
>
> Agree, please let me know if can go ahead with previous design.
I think that will depend on the answer we get from Chrome team as to does the PHy complianec request
need to change the PLLs according to the requested link rate/lane count that we read or
does that happen as part of link compliance link train request and PHY request does not need
to touch the PLLs at all.
I think Khaled from Chrome team also sent some responses, could you follow up with him on this?
Manasi
>
> >
> >Animesh, please work on getting these clarifications from Chrome team and we can close
> >on these opens.
> >Meanwhile I am trying to get the DPR 100 and test equipment and do the testing myself here to
> >understand the flow.
>
> Thanks Manasi.
>
> Regards,
> Animesh
>
> >
> >Manasi
> >
> >
> >>For Link-rate/lane-count, we need to do link-training/modeset. As per my
> >>observation the request is coming as link-compliance request, still waiting
> >>feedback from compliance team to confirm. So not handled as part
> >>dp-phy-compliance implementation.
> >Yes we need to push the Chrome OS team to get clarification on this:
> >- Does PHY compliance req always send a link compliance request before requesting link rate/lane count change?
> >
> >Once we get this clarification we will
> >>>Also its still not clear to me how this will work without actualling using the phy link rate
> >>>and lane count stored in the phy test pattern params to compute the pipe config?
> >>>Currently those only get used directly to set the link bw.
> >>If link-compliance request is not used for link-rate/lane-count change then
> >>your point is valid. As mentioned above getting link-compliance request
> >>which is confusing and waiting confirmation from compliance team.
> >>
> >>>This can work if the phy comp runs after link layer comp but on standalone phy comp testing
> >>>this could be a problem.
> >>>
> >>>Also I dont see my comment addressed where I had asked to move the below to patch 3:
> >>>>>+ /* Set test active flag here so userspace doesn't interrupt things */
> >>>>>+ intel_dp->compliance.test_active = 1;
> >>>>>+ dev_priv->dp_phy_comp = true;
> >>>>>+
> >>Thinking of creating a separate patch for sending uevent and setting
> >>test_active flag true for intel_dp_compliance tool.
> >>The patch 3/7 will take the preparation part and acknowledge the test
> >>request. Hope it will be fine.
> >>
> >>Regards,
> >>Animesh
> >>
> >>>Manasi
> >>>
> >>>
> >>>On Sun, Nov 17, 2019 at 11:53:54PM -0800, Manasi Navare wrote:
> >>>>On Fri, Nov 15, 2019 at 08:55:49PM +0530, Animesh Manna wrote:
> >>>>>This patch process phy compliance request by programming requested
> >>>>>vswing, pre-emphasis and test pattern.
> >>>>Again here a slightly detailed description of where in the atomic modeset
> >>>>do we process the PHY com request would be good.
> >>>>
> >>>>The design overall looks good now, few comments below:
> >>>>
> >>>>>Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >>>>>---
> >>>>> drivers/gpu/drm/i915/display/intel_ddi.c | 14 ++++
> >>>>> drivers/gpu/drm/i915/display/intel_display.c | 5 ++
> >>>>> drivers/gpu/drm/i915/display/intel_dp.c | 77 ++++++++++++++++++++
> >>>>> drivers/gpu/drm/i915/display/intel_dp.h | 2 +
> >>>>> drivers/gpu/drm/i915/i915_drv.h | 2 +
> >>>>> 5 files changed, 100 insertions(+)
> >>>>>
> >>>>>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> >>>>>index 8f817de34460..c12d4ebbd352 100644
> >>>>>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
> >>>>>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> >>>>>@@ -3700,8 +3700,17 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
> >>>>> {
> >>>>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> >>>>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >>>>>+ struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >>>>> enum pipe pipe = crtc->pipe;
> >>>>>+ if (encoder->type == INTEL_OUTPUT_DP) {
> >>>>>+ if (dig_port->dp.compliance.test_type ==
> >>>>>+ DP_TEST_LINK_PHY_TEST_PATTERN) {
> >>>>>+ intel_dp_process_phy_request(&dig_port->dp);
> >>>>>+ return;
> >>>>>+ }
> >>>>>+ }
> >>>>>+
> >>>>> /*
> >>>>> * When called from DP MST code:
> >>>>> * - conn_state will be NULL
> >>>>>@@ -4147,6 +4156,11 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> >>>>> enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> >>>>> bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
> >>>>>+ if (encoder->type == INTEL_OUTPUT_DP)
> >>>>>+ if (dig_port->dp.compliance.test_type ==
> >>>>>+ DP_TEST_LINK_PHY_TEST_PATTERN)
> >>>>>+ return;
> >>>>>+
> >>>>> if (is_tc_port)
> >>>>> intel_tc_port_get_link(dig_port, crtc_state->lane_count);
> >>>>>diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >>>>>index adf50c4b38ad..4ad11df55f07 100644
> >>>>>--- a/drivers/gpu/drm/i915/display/intel_display.c
> >>>>>+++ b/drivers/gpu/drm/i915/display/intel_display.c
> >>>>>@@ -545,6 +545,11 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
> >>>>> static bool
> >>>>> needs_modeset(const struct intel_crtc_state *state)
> >>>>> {
> >>>>>+ struct drm_i915_private *dev_priv = to_i915(state->uapi.crtc->dev);
> >>>>>+
> >>>>>+ if (dev_priv->dp_phy_comp)
> >>>>>+ return true;
> >>>>Could you double check with Jani N if this is an acceptable solution to
> >>>>foce a full modeset for a PHY compliance test?
> >>>>
> >>>>>+
> >>>>> return drm_atomic_crtc_needs_modeset(&state->uapi);
> >>>>> }
> >>>>>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >>>>>index df31278a1619..2c3d4b6c6036 100644
> >>>>>--- a/drivers/gpu/drm/i915/display/intel_dp.c
> >>>>>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >>>>>@@ -5010,14 +5010,91 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> >>>>> }
> >>>>> }
> >>>>>+static void
> >>>>>+intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
> >>>>>+{
> >>>>>+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>>>>+ struct drm_device *dev = intel_dig_port->base.base.dev;
> >>>>>+ struct drm_i915_private *dev_priv = to_i915(dev);
> >>>>>+ enum port port = intel_dig_port->base.port;
> >>>>>+ u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> >>>>>+
> >>>>>+ ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> >>>>>+ dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> >>>>>+ trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> >>>>>+
> >>>>>+ ddi_buf_ctl_value &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
> >>>>>+ dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
> >>>>>+ trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
> >>>>>+ DDI_PORT_WIDTH_MASK);
> >>>>>+
> >>>>>+ I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> >>>>>+ I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> >>>>>+ I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> >>>>>+}
> >>>>>+
> >>>>>+static void
> >>>>>+intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
> >>>>>+{
> >>>>>+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>>>>+ struct drm_device *dev = intel_dig_port->base.base.dev;
> >>>>>+ struct drm_i915_private *dev_priv = to_i915(dev);
> >>>>>+ enum port port = intel_dig_port->base.port;
> >>>>>+ u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> >>>>>+
> >>>>>+ ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> >>>>>+ dp_tp_ctl_value = I915_READ(TGL_DP_TP_CTL(port));
> >>>>>+ trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> >>>>>+
> >>>>>+ ddi_buf_ctl_value |= DDI_BUF_CTL_ENABLE |
> >>>>>+ DDI_PORT_WIDTH(lane_cnt);
> >>>>>+ dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
> >>>>>+ trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
> >>>>>+ DDI_PORT_WIDTH(lane_cnt);
> >>>>>+
> >>>>>+ I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> >>>>>+ I915_WRITE(TGL_DP_TP_CTL(port), dp_tp_ctl_value);
> >>>>>+ I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> >>>>>+}
> >>>>>+
> >>>>>+void intel_dp_process_phy_request(struct intel_dp *intel_dp)
> >>>>>+{
> >>>>>+ struct drm_dp_phy_test_params *data =
> >>>>>+ &intel_dp->compliance.test_data.phytest;
> >>>>>+ u8 link_status[DP_LINK_STATUS_SIZE];
> >>>>>+
> >>>>>+ if (!intel_dp_get_link_status(intel_dp, link_status)) {
> >>>>>+ DRM_DEBUG_KMS("failed to get link status\n");
> >>>>>+ return;
> >>>>>+ }
> >>>>>+
> >>>>>+ /* retrieve vswing & pre-emphasis setting */
> >>>>>+ intel_get_adjust_train(intel_dp, link_status);
> >>>>>+
> >>>>>+ intel_dp_autotest_phy_ddi_disable(intel_dp);
> >>>>>+
> >>>>>+ intel_dp_set_signal_levels(intel_dp);
> >>>>>+
> >>>>>+ intel_dp_phy_pattern_update(intel_dp);
> >>>>>+
> >>>>>+ intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
> >>>>>+
> >>>>>+ drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
> >>>>>+}
> >>>>>+
> >>>>> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> >>>>> {
> >>>>> u8 test_result = DP_TEST_NAK;
> >>>>>+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >>>>> test_result = intel_dp_prepare_phytest(intel_dp);
> >>>>> if (test_result != DP_TEST_ACK)
> >>>>> DRM_ERROR("Phy test preparation failed\n");
> >>>>>+ /* Set test active flag here so userspace doesn't interrupt things */
> >>>>>+ intel_dp->compliance.test_active = 1;
> >>>>>+ dev_priv->dp_phy_comp = true;
> >>>>>+
> >>>>This should be moved to the patch 3 I think where you handle phy test request and
> >>>>call prepare function. The same patch that handles test req can set the test active to 1.
> >>>>
> >>>>> return test_result;
> >>>>> }
> >>>>>diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> >>>>>index 0d0cb692f701..b1274ecffc7f 100644
> >>>>>--- a/drivers/gpu/drm/i915/display/intel_dp.h
> >>>>>+++ b/drivers/gpu/drm/i915/display/intel_dp.h
> >>>>>@@ -120,6 +120,8 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> >>>>> const struct intel_crtc_state *crtc_state,
> >>>>> const struct drm_connector_state *conn_state);
> >>>>> bool intel_digital_port_connected(struct intel_encoder *encoder);
> >>>>>+void intel_dp_process_phy_request(struct intel_dp *intel_dp);
> >>>>>+
> >>>>> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> >>>>> {
> >>>>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>>>index 1779f600fcfb..232e0dfb9d8e 100644
> >>>>>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>>>>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>>>@@ -1285,6 +1285,8 @@ struct drm_i915_private {
> >>>>> I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
> >>>>>+ bool dp_phy_comp;
> >>>>Not sure if this is the best way to handle the full mdoeset, could you double
> >>>>check with Jani?
> >>>>
> >>>>Other than that I think now the placement in pre_enable_dp is good.
> >>>>
> >>>>The only other concern I have is changing link rate and lane count only happens
> >>>>in set_phy_patterns where we write the test link rate and lane count
> >>>>directly to the link bw set. But the driver's compute config is still configuring
> >>>>the pipe and plls based on the link rate and lane count that could be
> >>>>different than the test link rate/count. This might work now since you said
> >>>>the link layer test is already configuring it at test params but in general
> >>>>shouldnt we be using the phy->link rate and lane count if in phy compliance
> >>>>also in compute_config()?
> >>>>
> >>>>Jani, any thoughts here?
> >>>>
> >>>>Regards
> >>>>Manasi
> >>>>
> >>>>>+
> >>>>> /*
> >>>>> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> >>>>> * will be rejected. Instead look for a better place.
> >>>>>--
> >>>>>2.22.0
> >>>>>
> >>>>_______________________________________________
> >>>>Intel-gfx mailing list
> >>>>Intel-gfx@lists.freedesktop.org
> >>>>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2020-01-24 0:56 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-15 15:25 [RFC 0/7] DP Phy compliace auto test Animesh Manna
2019-11-15 15:25 ` [Intel-gfx] " Animesh Manna
2019-11-15 15:25 ` [RFC 1/7] drm/dp: get/set phy compliance pattern Animesh Manna
2019-11-15 15:25 ` [Intel-gfx] " Animesh Manna
2019-11-18 4:04 ` Manasi Navare
2019-11-18 4:04 ` [Intel-gfx] " Manasi Navare
2019-11-18 18:39 ` Animesh Manna
2019-11-18 18:39 ` [Intel-gfx] " Animesh Manna
2019-11-15 15:25 ` [RFC 2/7] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation Animesh Manna
2019-11-15 15:25 ` [Intel-gfx] " Animesh Manna
2019-11-15 15:25 ` [RFC 3/7] drm/i915/dp: Preparation for DP phy compliance auto test Animesh Manna
2019-11-15 15:25 ` [Intel-gfx] " Animesh Manna
2019-11-18 4:47 ` Manasi Navare
2019-11-18 4:47 ` [Intel-gfx] " Manasi Navare
2019-11-15 15:25 ` [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry Animesh Manna
2019-11-15 15:25 ` [Intel-gfx] " Animesh Manna
2019-11-18 4:58 ` Manasi Navare
2019-11-18 4:58 ` [Intel-gfx] " Manasi Navare
2019-11-18 5:06 ` Manasi Navare
2019-11-18 5:06 ` [Intel-gfx] " Manasi Navare
2019-11-18 18:45 ` Animesh Manna
2019-11-18 18:45 ` [Intel-gfx] " Animesh Manna
2019-11-15 15:25 ` [RFC 5/7] drm/i915/dp: Register definition for DP compliance register Animesh Manna
2019-11-15 15:25 ` [Intel-gfx] " Animesh Manna
2019-11-18 5:00 ` Manasi Navare
2019-11-18 5:00 ` [Intel-gfx] " Manasi Navare
2019-11-15 15:25 ` [RFC 6/7] drm/i915/dp: Update the pattern as per request Animesh Manna
2019-11-15 15:25 ` [Intel-gfx] " Animesh Manna
2019-11-18 6:41 ` Manasi Navare
2019-11-18 6:41 ` [Intel-gfx] " Manasi Navare
2019-11-18 18:47 ` Animesh Manna
2019-11-18 18:47 ` [Intel-gfx] " Animesh Manna
2019-12-11 23:44 ` Manasi Navare
2019-11-15 15:25 ` [RFC 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern Animesh Manna
2019-11-15 15:25 ` [Intel-gfx] " Animesh Manna
2019-11-18 7:53 ` Manasi Navare
2019-11-18 7:53 ` [Intel-gfx] " Manasi Navare
2019-12-11 23:50 ` Manasi Navare
2019-12-13 17:24 ` Animesh Manna
2020-01-14 21:38 ` Manasi Navare
2020-01-20 13:53 ` Manna, Animesh
2020-01-24 0:56 ` Manasi Navare
2019-11-15 19:27 ` ✗ Fi.CI.BUILD: failure for DP Phy compliace auto test Patchwork
2019-11-15 19:27 ` [Intel-gfx] " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.