linux-kernel - Re: [PATCH 3/3] drm/msm/dp: add a debugfs node for using tpg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <uv2phgi72mmw5c462ijsqlqedeiv4gahrwi26i2luqkhgwmfxc@ycj7jrujdj6w>
Date: Wed, 4 Dec 2024 01:38:10 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Abhinav Kumar <quic_abhinavk@...cinc.com>
Cc: Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>, 
	Marijn Suijten <marijn.suijten@...ainline.org>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Stephen Boyd <swboyd@...omium.org>, 
	Kuogee Hsieh <quic_khsieh@...cinc.com>, linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	Jessica Zhang <quic_jesszhan@...cinc.com>
Subject: Re: [PATCH 3/3] drm/msm/dp: add a debugfs node for using tpg

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@...cinc.com>
> ---
>  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