[Intel-gfx] [RFC 1/7] drm/dp: get/set phy compliance pattern
Manasi Navare
manasi.d.navare at intel.com
Mon Nov 18 04:04:29 UTC 2019
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 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