On Mon, Dec 02, 2024 at 12:42:00PM -0800, Abhinav Kumar wrote:
> DP test pattern generator is a very useful tool to debug issues
> where monitor is showing incorrect output as it helps to isolate
> whether the issue is due to rest of DPU pipeline or in the DP
> controller itself. Expose a debugfs to use the TPG configuration
> to help debug DP issues.
>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/dp/dp_debug.c | 61 +++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/dp/dp_panel.h | 2 ++
> 2 files changed, 63 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c b/drivers/gpu/drm/msm/dp/dp_debug.c
> index 22fd946ee201..843fe77268f8 100644
> --- a/drivers/gpu/drm/msm/dp/dp_debug.c
> +++ b/drivers/gpu/drm/msm/dp/dp_debug.c
> @@ -197,6 +197,65 @@ static const struct file_operations test_active_fops = {
> .write = msm_dp_test_active_write
> };
>
> +static ssize_t msm_dp_tpg_write(struct file *file, const char __user *ubuf,
> + size_t len, loff_t *offp)
> +{
> + const struct msm_dp_debug_private *debug;
> + char *input_buffer;
> + int val;
> + int status = 0;
> + struct msm_dp_panel *dp_panel;
> +
> + debug = ((struct seq_file *)file->private_data)->private;
> + dp_panel = debug->panel;
> +
> + input_buffer = memdup_user_nul(ubuf, len);
> + if (IS_ERR(input_buffer))
> + return PTR_ERR(input_buffer);
> +
> + status = kstrtoint(input_buffer, 10, &val);
> + if (status < 0) {
> + kfree(input_buffer);
> + return status;
> + }
> +
> + msm_dp_panel_tpg_config(dp_panel, val);
> +
> + dp_panel->tpg_enabled = val;
Does this need any kind of locking? The driver performs some actions,
then we write the global state. What if the user in parallel writes
different values to the file?
> +
> + kfree(input_buffer);
> +
> + *offp += len;
> + return len;
> +}
> +
> +static int msm_dp_tpg_show(struct seq_file *f, void *data)
> +{
> + struct msm_dp_debug_private *debug = f->private;
> + struct msm_dp_panel *dp_panel = debug->panel;
> +
> + if (dp_panel->tpg_enabled)
> + seq_puts(f, "1");
> + else
> + seq_puts(f, "0");
> +
> + return 0;
> +}
> +
> +static int msm_dp_tpg_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, msm_dp_tpg_show, inode->i_private);
> +}
> +
> +static const struct file_operations msm_dp_tpg_fops = {
> + .owner = THIS_MODULE,
> + .open = msm_dp_tpg_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> + .write = msm_dp_tpg_write
> +};
> +
> int msm_dp_debug_init(struct device *dev, struct msm_dp_panel *panel,
> struct msm_dp_link *link,
> struct drm_connector *connector,
> @@ -231,6 +290,8 @@ int msm_dp_debug_init(struct device *dev, struct msm_dp_panel *panel,
> debugfs_create_file("dp_test_type", 0444,
> root,
> debug, &msm_dp_test_type_fops);
> +
> + debugfs_create_file("dp_tpg", 0444, root, debug, &msm_dp_tpg_fops);
I'd say, skip the dp_ part of the name, everything in that dir is
DP-related.
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
> index 0e944db3adf2..7910b11fd685 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -50,6 +50,8 @@ struct msm_dp_panel {
> u32 max_dp_link_rate;
>
> u32 max_bw_code;
> +
> + bool tpg_enabled;
> };
>
> int msm_dp_panel_init_panel_info(struct msm_dp_panel *msm_dp_panel);
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
