dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Harry Wentland <hwentlan@amd.com>
To: "Manna, Animesh" <animesh.manna@intel.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: jani.nikula@intel.com, nidhi1.gupta@intel.com,
	manasi.d.navare@intel.com, uma.shankar@intel.com,
	anshuman.gupta@intel.com
Subject: Re: [PATCH v3 2/9] drm/dp: get/set phy compliance pattern
Date: Mon, 30 Dec 2019 11:11:06 -0500	[thread overview]
Message-ID: <d733d496-108a-d0c0-ce5b-04cfb03416f2@amd.com> (raw)
In-Reply-To: <93fd1b3e-2c97-2cfb-0e1a-9de86b327451@intel.com>



On 2019-12-30 11:05 a.m., Manna, Animesh wrote:
> On 24-12-2019 01:23, Harry Wentland wrote:
>>
>> On 2019-12-23 12:03 p.m., Animesh Manna wrote:
>>> During phy compliance 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.
>>>
>>> v2: As per review feedback from Manasi on RFC version,
>>> - added dp revision as function argument in set_phy_pattern api.
>>> - used int for link_rate and u8 for lane_count to align with
>>> existing code.
>>>
>>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>>> ---
>>>   drivers/gpu/drm/drm_dp_helper.c | 93
>>> +++++++++++++++++++++++++++++++++
>>>   include/drm/drm_dp_helper.h     | 31 +++++++++++
>>>   2 files changed, 124 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>>> b/drivers/gpu/drm/drm_dp_helper.c
>>> index 2c7870aef469..91c80973aa83 100644
>>> --- a/drivers/gpu/drm/drm_dp_helper.c
>>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>>> @@ -1371,3 +1371,96 @@ 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_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);
>> Using sizeof(data->custom80) might be safer.
>>
>>> +        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);
>> Same here, using sizeof(data->hbr2_reset).
>>
>>> +        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, u8 dp_rev)
>>> +{
>>> +    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 (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 d6e560870fb1..42a364748308 100644
>>> --- a/include/drm/drm_dp_helper.h
>>> +++ b/include/drm/drm_dp_helper.h
>>> @@ -700,6 +700,15 @@
>>>   # define DP_TEST_COUNT_MASK            0xf
>>>     #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
>>> @@ -1570,4 +1579,26 @@ 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.
>> @link seems to be from a previous patch version. Please describe
>> link_rate and num_lanes instead.
>>
>>> + * @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.
>> Missing doc for enhanced_frame_cap.
>>
>>> + */
>>> +struct drm_dp_phy_test_params {
>>> +    int link_rate;
>> Is there a reason you're not using u8 here? Would be nice to keep it
>> consistent with num_lanes.
>
> link_rate variable is used here to store actual clock value after
> multiplying with 27000 (0.27 gbps) with link-code using
> drm_dp_bw_code_to_link_rate().
> Followed the same and u8 maybe not be sufficient. Added other
> suggested changes. Thanks for review.
>
That makes sense. I missed that. In that case feel free to add my
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> Regrads,
> Animesh
>
>>
>>> +    u8 num_lanes;
>>> +    u8 phy_pattern;
>>> +    u8 hbr2_reset[2];
>>> +    u8 custom80[10];
>>> +    bool enahanced_frame_cap;
>> Typo. Should be enhanved_frame_cap.
>>
>> Harry
>>
>>> +};
>>> +
>>> +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, u8 dp_rev);
>>>   #endif /* _DRM_DP_HELPER_H_ */
>>>

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

  reply	other threads:[~2019-12-30 16:11 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-18 15:13 [PATCH v2 0/9] DP Phy compliance auto test Animesh Manna
2019-12-18 15:13 ` [PATCH v2 1/9] drm/dp: get/set phy compliance pattern Animesh Manna
2019-12-18 15:13 ` [PATCH v2 2/9] drm/amd/display: Fix compilation issue Animesh Manna
2019-12-18 15:42   ` Harry Wentland
2019-12-18 16:13     ` Manna, Animesh
2019-12-18 16:22       ` Harry Wentland
2019-12-18 23:53       ` Manasi Navare
2019-12-19 13:36         ` Manna, Animesh
2019-12-23 17:03         ` [PATCH v3 1/9] drm/amd/display: Align macro name as per DP spec Animesh Manna
2019-12-23 17:03           ` [PATCH v3 2/9] drm/dp: get/set phy compliance pattern Animesh Manna
2019-12-23 19:53             ` Harry Wentland
2019-12-30 16:05               ` Manna, Animesh
2019-12-30 16:11                 ` Harry Wentland [this message]
2019-12-30 16:30                   ` Manna, Animesh
2019-12-23 19:53           ` [PATCH v3 1/9] drm/amd/display: Align macro name as per DP spec Harry Wentland
2019-12-18 15:13 ` [PATCH v2 3/9] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation Animesh Manna
2019-12-19 10:51   ` Jani Nikula
2019-12-19 13:27     ` Manna, Animesh
2019-12-19 12:33   ` Ville Syrjälä
2019-12-19 13:31     ` Manna, Animesh
2019-12-23 16:45     ` [PATCH v3 " Animesh Manna
2019-12-18 15:13 ` [PATCH v2 4/9] drm/i915/dp: Preparation for DP phy compliance auto test Animesh Manna
2019-12-18 15:13 ` [PATCH v2 5/9] drm/i915/dsb: Send uevent to testapp Animesh Manna
2019-12-18 15:13 ` [PATCH v2 6/9] drm/i915/dp: Add debugfs entry for DP phy compliance Animesh Manna
2019-12-18 15:13 ` [PATCH v2 7/9] drm/i915/dp: Register definition for DP compliance register Animesh Manna
2019-12-18 15:13 ` [PATCH v2 8/9] drm/i915/dp: Update the pattern as per request Animesh Manna
2019-12-18 15:13 ` [PATCH v2 9/9] drm/i915/dp: [FIXME] Program vswing, pre-emphasis, test-pattern Animesh Manna
2019-12-30 16:15 [PATCH v3 0/9] DP Phy compliance auto test Animesh Manna
2019-12-30 16:15 ` [PATCH v3 2/9] drm/dp: get/set phy compliance pattern Animesh Manna

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d733d496-108a-d0c0-ce5b-04cfb03416f2@amd.com \
    --to=hwentlan@amd.com \
    --cc=animesh.manna@intel.com \
    --cc=anshuman.gupta@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=manasi.d.navare@intel.com \
    --cc=nidhi1.gupta@intel.com \
    --cc=uma.shankar@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).