On 5/24/23 13:58, Mario Limonciello wrote: > There is already access to the DPCD from userspace through > `CONFIG_DRM_DP_AUX_CHARDEV`, so it's unnecessary to reinvent the wheel > with a set of extra debugfs nodes specific to amdgpu. > > The character device interface behaves more like you would expect in that > you can seek/read/write all from the same file. > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx> Harry > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 - > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 107 ------------------ > 2 files changed, 111 deletions(-) > > 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 2e2413fd73a4..4561f55afa99 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > @@ -661,10 +661,6 @@ struct amdgpu_dm_connector { > struct mutex hpd_lock; > > bool fake_enable; > -#ifdef CONFIG_DEBUG_FS > - uint32_t debugfs_dpcd_address; > - uint32_t debugfs_dpcd_size; > -#endif > bool force_yuv420_output; > struct dsc_preferred_settings dsc_settings; > union dp_downstream_port_present mst_downstream_port_present; > 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 827fcb4fb3b3..82234397dd44 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 > @@ -1039,88 +1039,6 @@ static ssize_t dp_sdp_message_debugfs_write(struct file *f, const char __user *b > return write_size; > } > > -static ssize_t dp_dpcd_address_write(struct file *f, const char __user *buf, > - size_t size, loff_t *pos) > -{ > - int r; > - struct amdgpu_dm_connector *connector = file_inode(f)->i_private; > - > - if (size < sizeof(connector->debugfs_dpcd_address)) > - return -EINVAL; > - > - r = copy_from_user(&connector->debugfs_dpcd_address, > - buf, sizeof(connector->debugfs_dpcd_address)); > - > - return size - r; > -} > - > -static ssize_t dp_dpcd_size_write(struct file *f, const char __user *buf, > - size_t size, loff_t *pos) > -{ > - int r; > - struct amdgpu_dm_connector *connector = file_inode(f)->i_private; > - > - if (size < sizeof(connector->debugfs_dpcd_size)) > - return -EINVAL; > - > - r = copy_from_user(&connector->debugfs_dpcd_size, > - buf, sizeof(connector->debugfs_dpcd_size)); > - > - if (connector->debugfs_dpcd_size > 256) > - connector->debugfs_dpcd_size = 0; > - > - return size - r; > -} > - > -static ssize_t dp_dpcd_data_write(struct file *f, const char __user *buf, > - size_t size, loff_t *pos) > -{ > - int r; > - char *data; > - struct amdgpu_dm_connector *connector = file_inode(f)->i_private; > - struct dc_link *link = connector->dc_link; > - uint32_t write_size = connector->debugfs_dpcd_size; > - > - if (!write_size || size < write_size) > - return -EINVAL; > - > - data = kzalloc(write_size, GFP_KERNEL); > - if (!data) > - return 0; > - > - r = copy_from_user(data, buf, write_size); > - > - dm_helpers_dp_write_dpcd(link->ctx, link, > - connector->debugfs_dpcd_address, data, write_size - r); > - kfree(data); > - return write_size - r; > -} > - > -static ssize_t dp_dpcd_data_read(struct file *f, char __user *buf, > - size_t size, loff_t *pos) > -{ > - int r; > - char *data; > - struct amdgpu_dm_connector *connector = file_inode(f)->i_private; > - struct dc_link *link = connector->dc_link; > - uint32_t read_size = connector->debugfs_dpcd_size; > - > - if (!read_size || size < read_size) > - return 0; > - > - data = kzalloc(read_size, GFP_KERNEL); > - if (!data) > - return 0; > - > - dm_helpers_dp_read_dpcd(link->ctx, link, > - connector->debugfs_dpcd_address, data, read_size); > - > - r = copy_to_user(buf, data, read_size); > - > - kfree(data); > - return read_size - r; > -} > - > /* function: Read link's DSC & FEC capabilities > * > * > @@ -2682,25 +2600,6 @@ static const struct file_operations sdp_message_fops = { > .llseek = default_llseek > }; > > -static const struct file_operations dp_dpcd_address_debugfs_fops = { > - .owner = THIS_MODULE, > - .write = dp_dpcd_address_write, > - .llseek = default_llseek > -}; > - > -static const struct file_operations dp_dpcd_size_debugfs_fops = { > - .owner = THIS_MODULE, > - .write = dp_dpcd_size_write, > - .llseek = default_llseek > -}; > - > -static const struct file_operations dp_dpcd_data_debugfs_fops = { > - .owner = THIS_MODULE, > - .read = dp_dpcd_data_read, > - .write = dp_dpcd_data_write, > - .llseek = default_llseek > -}; > - > static const struct file_operations dp_max_bpc_debugfs_fops = { > .owner = THIS_MODULE, > .read = dp_max_bpc_read, > @@ -2724,9 +2623,6 @@ static const struct { > {"test_pattern", &dp_phy_test_pattern_fops}, > {"hdcp_sink_capability", &hdcp_sink_capability_fops}, > {"sdp_message", &sdp_message_fops}, > - {"aux_dpcd_address", &dp_dpcd_address_debugfs_fops}, > - {"aux_dpcd_size", &dp_dpcd_size_debugfs_fops}, > - {"aux_dpcd_data", &dp_dpcd_data_debugfs_fops}, > {"dsc_clock_en", &dp_dsc_clock_en_debugfs_fops}, > {"dsc_slice_width", &dp_dsc_slice_width_debugfs_fops}, > {"dsc_slice_height", &dp_dsc_slice_height_debugfs_fops}, > @@ -3025,9 +2921,6 @@ void connector_debugfs_init(struct amdgpu_dm_connector *connector) > connector_debugfs_entries[i].fops); > } > > - connector->debugfs_dpcd_address = 0; > - connector->debugfs_dpcd_size = 0; > - > if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA) { > for (i = 0; i < ARRAY_SIZE(hdmi_debugfs_entries); i++) { > debugfs_create_file(hdmi_debugfs_entries[i].name,