Hi Nicolas,
Sorry for the very late reply.
I recently discovered that I missed this review comment when promoting the
series upstream. That was my mistake, and I apologize for overlooking your
feedback.
I have since reviewed the comment with the original author. After revisiting
the implementation, we do not plan to make further changes to this patch.
Regarding the concern you raised about:
link.dpcd_caps.dongle_type =
(typeof(link.dpcd_caps.dongle_type))0x7f;
in "[PATCH 21/32] drm/amd/display: Add KUnit tests for amdgpu_dm_connector",
if you'd still like to discuss that issue, could you please follow up in the
corresponding patch thread? It would be easier to keep the discussion attached
to the relevant patch and context.
Apologies again for missing your review comment, and thank you for taking the
time to review the series.
Regards,
Chenyu
On 7/21/2026 10:09 AM, Chen, Chen-Yu wrote:
> AMD General
>
> Hi @Zuo, Jerry,
>
> This is my mistake. I missed a review comment from when promoting the series,
> and the patches were applied before the feedback was addressed.
>
> Could you please review the comment below and determine whether a follow-up
> patch is required?
>
> Apologies for the oversight.
>
> Regards,
> Chenyu
>
> -----Original Message-----
> From: Nicolas Frattaroli <[email protected]>
> Sent: Wednesday, June 10, 2026 10:44 PM
> To: [email protected]; Chen, Chen-Yu <[email protected]>
> Cc: Wentland, Harry <[email protected]>; Li, Sun peng (Leo)
> <[email protected]>; Pillai, Aurabindo <[email protected]>; Li, Roman
> <[email protected]>; Lin, Wayne <[email protected]>; Chung, ChiaHsuan (Tom)
> <[email protected]>; Zuo, Jerry <[email protected]>; Wheeler, Daniel
> <[email protected]>; Wu, Ray <[email protected]>; LIPSKI, IVAN
> <[email protected]>; Hung, Alex <[email protected]>; Lin, Ping Lei
> <[email protected]>; Chen, Chen-Yu <[email protected]>; Zuo, Jerry
> <[email protected]>; Chen, Chen-Yu <[email protected]>
> Subject: Re: [PATCH 16/32] drm/amd/display: Add Support for HDMI Compliance
> Automation
>
> On Wednesday, 10 June 2026 11:45:00 Central European Summer Time Chenyu Chen
> wrote:
>> From: Fangzhi Zuo <[email protected]>
>>
>> Add support to get DUT trained at FRL link rate when working with
>> Teledyne M41h compliance automation.
>>
>> Reviewed-by: Alex Hung <[email protected]>
>> Signed-off-by: Fangzhi Zuo <[email protected]>
>> Signed-off-by: Chenyu Chen <[email protected]>
>> ---
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +
>> .../display/amdgpu_dm/amdgpu_dm_connector.c | 5 ++
>> .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 67
>> ++++++++++++++++++- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |
>> 6 ++
>> 4 files changed, 80 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>> index 2940dd5b7348..ba1e11e144f2 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>> @@ -877,6 +877,9 @@ struct amdgpu_dm_connector {
>> unsigned int hdmi_hpd_debounce_delay_ms;
>> struct delayed_work hdmi_hpd_debounce_work;
>> struct dc_sink *hdmi_prev_sink;
>> +
>> + /* HDMI compliance automation */
>> + bool hdmi_comp_auto;
>> };
>>
>> static inline void amdgpu_dm_set_mst_status(uint8_t *status, diff
>> --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
>> index 6ef257622f1a..59091ee32099 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
>> @@ -573,6 +573,11 @@ void amdgpu_dm_update_connector_after_detect(
>> amdgpu_dm_update_freesync_caps(connector,
>> aconnector->drm_edid, true);
>> amdgpu_dm_update_connector_ext_caps(aconnector);
>> dm_set_panel_type(aconnector);
>> +
>> + if (aconnector->hdmi_comp_auto) {
>> + if (sink->sink_signal != SIGNAL_TYPE_HDMI_FRL)
>> + sink->sink_signal = SIGNAL_TYPE_HDMI_FRL;
>> + }
>> } else {
>> hdmi_cec_unset_edid(aconnector);
>> drm_dp_cec_unset_edid(&aconnector->dm_dp_aux.aux);
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>> index 096a855a7304..95a56e39f452 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>> @@ -2982,6 +2982,64 @@ static ssize_t hdmi_cec_state_write(struct file *f,
>> const char __user *buf,
>> return size;
>> }
>>
>> +/**
>> + * hdmi_automation_enable - Enable/Disable HDMI automation feature
>> + * @f: file structure.
>> + * @buf: userspace buffer. set to '1' to enable; '0' to disable automation
>> feature.
>> + * @size: size of buffer from userpsace.
>> + * @pos: unused.
>> + *
>> + * Return size on success, error code on failure */ static ssize_t
>> +hdmi_automation_enable(struct file *f, const char __user *buf,
>> + size_t size, loff_t *pos)
>> +{
>> + struct amdgpu_dm_connector *aconnector = file_inode(f)->i_private;
>> + char *wr_buf = NULL;
>> + const uint32_t wr_buf_size = 40;
>> + int max_param_num = 1;
>> + uint8_t param_nums = 0;
>> + long param[2];
>> + bool hdmi_comp_auto;
>> +
>> + if (size == 0)
>> + return -EINVAL;
>> +
>> + wr_buf = kcalloc(wr_buf_size, sizeof(char), GFP_KERNEL);
>> + if (!wr_buf)
>> + return -ENOSPC;
>> +
>> + if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
>> + (long *)param, buf,
>> + max_param_num,
>> + ¶m_nums)) {
>> + kfree(wr_buf);
>> + return -EINVAL;
>> + }
>> +
>> + if (param_nums <= 0) {
>> + kfree(wr_buf);
>> + DRM_DEBUG_DRIVER("user data not be read\n");
>> + return -EINVAL;
>> + }
>> +
>> + switch (param[0]) {
>> + case 0:
>> + hdmi_comp_auto = false;
>> + break;
>> + case 1:
>> + default:
>> + hdmi_comp_auto = true;
>> + break;
>> + }
>> +
>> + /* Persist setting across sink re-detection/hotplug. */
>> + aconnector->hdmi_comp_auto = hdmi_comp_auto;
>> +
>> + kfree(wr_buf);
>> + return size;
>> +}
>> +
>> DEFINE_SHOW_ATTRIBUTE(dp_dsc_fec_support);
>> DEFINE_SHOW_ATTRIBUTE(dmub_fw_state);
>> DEFINE_SHOW_ATTRIBUTE(dmub_tracebuffer);
>> @@ -3099,6 +3157,12 @@ static const struct file_operations
>> dp_mst_link_settings_debugfs_fops = {
>> .llseek = default_llseek
>> };
>>
>> +static const struct file_operations hdmi_automation_debugfs_fops = {
>> + .owner = THIS_MODULE,
>> + .write = hdmi_automation_enable,
>> + .llseek = default_llseek
>> +};
>> +
>
> I really don't understand why this can't just be a DEFINE_DEBUGFS_ATTRIBUTE,
> and then you can replace the overcomplicated hdmi_automation_enable() with
> just simple setter and getter functions that already receive the parameter of
> the right type.
>
>> static const struct {
>> char *name;
>> const struct file_operations *fops;
>> @@ -3131,7 +3195,8 @@ static const struct {
>> const struct file_operations *fops;
>> } hdmi_debugfs_entries[] = {
>> {"hdcp_sink_capability", &hdcp_sink_capability_fops},
>> - {"hdmi_cec_state", &hdmi_cec_state_fops}
>> + {"hdmi_cec_state", &hdmi_cec_state_fops},
>> + {"hdmi_automation", &hdmi_automation_debugfs_fops}
>> };
>>
>> /*
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
>> index a2d0bb34e639..6350212b9a66 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
>> @@ -193,6 +193,12 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>> __func__, connector->name,
>> edid_caps->frl_dsc_10bpc, edid_caps->frl_dsc_12bpc, \
>> edid_caps->frl_dsc_all_bpp,
>> edid_caps->frl_dsc_native_420, edid_caps->frl_dsc_max_slices, \
>> edid_caps->frl_dsc_max_frl_rate,
>> edid_caps->frl_dsc_total_chunk_kbytes);
>> + if (aconnector->hdmi_comp_auto) {
>> + edid_caps->panel_patch.hdmi_comp_auto = true;
>> + link->ctx->dc->debug.force_frl_max = true;
>> + link->ctx->dc->debug.force_frl_dsc = true;
>> + drm_dbg_driver(connector->dev, "%s: HDMI_FRL [%s]
>> hdmi_comp_auto --> enabled\n", __func__, connector->name);
>> + }
>> }
>>
>> apply_edid_quirks(link, edid_buf, edid_caps);
>>
>
>
>
>
