[Intel-gfx] [RFC 1/7] drm/dp: get/set phy compliance pattern
Animesh Manna
animesh.manna at intel.com
Mon Nov 18 18:39:53 UTC 2019
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 at 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
>>
More information about the Intel-gfx
mailing list