On 7/20/2026 9:14 PM, Nicolas Frattaroli wrote:
> On Wednesday, 10 June 2026 16:43:37 Central European Summer Time Nicolas 
> Frattaroli wrote:
>> 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(-)
>>>
>>> [... snip ...]
>>> --- 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,
>>> +                                      &param_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.
> 
> Aaaand this was applied without the review comment being addressed.
> 
> Along with the incredible other things in this series, like
> 
>    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",
> I'm feeling like AMD is treating mainline as a vendor BSP to just dump code
> into.
> 
Hi Nicolas,

Sorry, that's on me. I missed your review comment when I promoted the series. 
Thanks for pointing it out.

I'll take a look at your feedback and follow up with the author to make sure 
it's addressed.

Hi Alex, 

Could you please take a look at the feedback above? It also looks like pointed 
out another issue in the patch below that may need to be addressed.

Please let us know whether a follow-up fix is needed.

Apologies for the oversight on my side.

Regards,
Chenyu
>>>  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);
>>>
>>
>>
> 
> 
> 
> 

Reply via email to