linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/bridge/synopsys: dsi: Allow VPG to be enabled via debugfs
@ 2019-04-24 14:22 Matt Redfearn
  2019-04-25  9:31 ` Laurent Pinchart
  0 siblings, 1 reply; 3+ messages in thread
From: Matt Redfearn @ 2019-04-24 14:22 UTC (permalink / raw)
  To: Andrzej Hajda, Laurent Pinchart, Philippe Cornu
  Cc: dri-devel, Matthew Redfearn, Nickey Yang, Heiko Stuebner,
	Archit Taneja, linux-kernel, David Airlie, Daniel Vetter

The Synopsys MIPI DSI IP contains a video test pattern generator which
is helpful in debugging video timing with connected displays.
Add a debugfs directory containing files which allow the VPG to be
enabled and disabled, and it's orientation to be changed.

Signed-off-by: Matt Redfearn <matt.redfearn@thinci.com>

---

 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 0ee440216b8..a1ee2306382 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -10,6 +10,7 @@
 
 #include <linux/clk.h>
 #include <linux/component.h>
+#include <linux/debugfs.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -86,6 +87,8 @@
 #define VID_MODE_TYPE_NON_BURST_SYNC_EVENTS	0x1
 #define VID_MODE_TYPE_BURST			0x2
 #define VID_MODE_TYPE_MASK			0x3
+#define VID_MODE_VPG_ENABLE		BIT(16)
+#define VID_MODE_VPG_HORIZONTAL		BIT(24)
 
 #define DSI_VID_PKT_SIZE		0x3c
 #define VID_PKT_SIZE(p)			((p) & 0x3fff)
@@ -234,6 +237,15 @@ struct dw_mipi_dsi {
 	u32 format;
 	unsigned long mode_flags;
 
+#ifdef CONFIG_DEBUG_FS
+	struct dentry *debugfs_root;
+	struct dentry *debugfs_vpg;
+	struct dentry *debugfs_vpg_horizontal;
+
+	bool vpg;
+	bool vpg_horizontal;
+#endif /* CONFIG_DEBUG_FS */
+
 	struct dw_mipi_dsi *master; /* dual-dsi master ptr */
 	struct dw_mipi_dsi *slave; /* dual-dsi slave ptr */
 
@@ -525,6 +537,11 @@ static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
 	else
 		val |= VID_MODE_TYPE_NON_BURST_SYNC_EVENTS;
 
+	if (dsi->vpg) {
+		val |= VID_MODE_VPG_ENABLE;
+		val |= dsi->vpg_horizontal ? VID_MODE_VPG_HORIZONTAL : 0;
+	}
+
 	dsi_write(dsi, DSI_VID_MODE_CFG, val);
 }
 
@@ -935,6 +952,41 @@ static const struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = {
 	.attach	      = dw_mipi_dsi_bridge_attach,
 };
 
+#ifdef CONFIG_DEBUG_FS
+
+static void dw_mipi_dsi_debugfs_init(struct dw_mipi_dsi *dsi)
+{
+	struct dentry *d;
+
+	d = debugfs_create_dir(dev_name(dsi->dev), NULL);
+	if (IS_ERR(d)) {
+		dev_err(dsi->dev, "failed to create debugfs root\n");
+		return;
+	}
+	dsi->debugfs_root = d;
+
+	d = debugfs_create_bool("vpg", 0660, dsi->debugfs_root, &dsi->vpg);
+	dsi->debugfs_vpg = d;
+
+	d = debugfs_create_bool("vpg_horizontal", 0660, dsi->debugfs_root,
+				&dsi->vpg_horizontal);
+	dsi->debugfs_vpg_horizontal = d;
+}
+
+static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi)
+{
+	debugfs_remove(dsi->debugfs_vpg_horizontal);
+	debugfs_remove(dsi->debugfs_vpg);
+	debugfs_remove(dsi->debugfs_root);
+}
+
+#else
+
+static void dw_mipi_dsi_debugfs_init(struct dw_mipi_dsi *dsi) { }
+static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi) { }
+
+#endif /* CONFIG_DEBUG_FS */
+
 static struct dw_mipi_dsi *
 __dw_mipi_dsi_probe(struct platform_device *pdev,
 		    const struct dw_mipi_dsi_plat_data *plat_data)
@@ -1005,6 +1057,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
 		clk_disable_unprepare(dsi->pclk);
 	}
 
+	dw_mipi_dsi_debugfs_init(dsi);
 	pm_runtime_enable(dev);
 
 	dsi->dsi_host.ops = &dw_mipi_dsi_host_ops;
@@ -1012,6 +1065,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
 	ret = mipi_dsi_host_register(&dsi->dsi_host);
 	if (ret) {
 		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
+		dw_mipi_dsi_debugfs_remove(dsi);
 		return ERR_PTR(ret);
 	}
 
@@ -1029,6 +1083,7 @@ static void __dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi)
 	mipi_dsi_host_unregister(&dsi->dsi_host);
 
 	pm_runtime_disable(dsi->dev);
+	dw_mipi_dsi_debugfs_remove(dsi);
 }
 
 void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi *slave)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/bridge/synopsys: dsi: Allow VPG to be enabled via debugfs
  2019-04-24 14:22 [PATCH] drm/bridge/synopsys: dsi: Allow VPG to be enabled via debugfs Matt Redfearn
@ 2019-04-25  9:31 ` Laurent Pinchart
  2019-04-25 12:42   ` Matt Redfearn
  0 siblings, 1 reply; 3+ messages in thread
From: Laurent Pinchart @ 2019-04-25  9:31 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: Andrzej Hajda, Philippe Cornu, dri-devel, Matthew Redfearn,
	Nickey Yang, Heiko Stuebner, Archit Taneja, linux-kernel,
	David Airlie, Daniel Vetter

On Wed, Apr 24, 2019 at 02:22:18PM +0000, Matt Redfearn wrote:
> The Synopsys MIPI DSI IP contains a video test pattern generator which
> is helpful in debugging video timing with connected displays.
> Add a debugfs directory containing files which allow the VPG to be
> enabled and disabled, and it's orientation to be changed.

s/it's/its/
> 
> Signed-off-by: Matt Redfearn <matt.redfearn@thinci.com>
> 
> ---
> 
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 0ee440216b8..a1ee2306382 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -10,6 +10,7 @@
>  
>  #include <linux/clk.h>
>  #include <linux/component.h>
> +#include <linux/debugfs.h>
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> @@ -86,6 +87,8 @@
>  #define VID_MODE_TYPE_NON_BURST_SYNC_EVENTS	0x1
>  #define VID_MODE_TYPE_BURST			0x2
>  #define VID_MODE_TYPE_MASK			0x3
> +#define VID_MODE_VPG_ENABLE		BIT(16)
> +#define VID_MODE_VPG_HORIZONTAL		BIT(24)
>  
>  #define DSI_VID_PKT_SIZE		0x3c
>  #define VID_PKT_SIZE(p)			((p) & 0x3fff)
> @@ -234,6 +237,15 @@ struct dw_mipi_dsi {
>  	u32 format;
>  	unsigned long mode_flags;
>  
> +#ifdef CONFIG_DEBUG_FS
> +	struct dentry *debugfs_root;
> +	struct dentry *debugfs_vpg;
> +	struct dentry *debugfs_vpg_horizontal;
> +
> +	bool vpg;
> +	bool vpg_horizontal;
> +#endif /* CONFIG_DEBUG_FS */
> +
>  	struct dw_mipi_dsi *master; /* dual-dsi master ptr */
>  	struct dw_mipi_dsi *slave; /* dual-dsi slave ptr */
>  
> @@ -525,6 +537,11 @@ static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
>  	else
>  		val |= VID_MODE_TYPE_NON_BURST_SYNC_EVENTS;
>  
> +	if (dsi->vpg) {

Won't this break if CONFIG_DDEBUG_FS is not defined ?

> +		val |= VID_MODE_VPG_ENABLE;
> +		val |= dsi->vpg_horizontal ? VID_MODE_VPG_HORIZONTAL : 0;
> +	}
> +
>  	dsi_write(dsi, DSI_VID_MODE_CFG, val);
>  }
>  
> @@ -935,6 +952,41 @@ static const struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = {
>  	.attach	      = dw_mipi_dsi_bridge_attach,
>  };
>  
> +#ifdef CONFIG_DEBUG_FS
> +
> +static void dw_mipi_dsi_debugfs_init(struct dw_mipi_dsi *dsi)
> +{
> +	struct dentry *d;
> +
> +	d = debugfs_create_dir(dev_name(dsi->dev), NULL);
> +	if (IS_ERR(d)) {
> +		dev_err(dsi->dev, "failed to create debugfs root\n");
> +		return;
> +	}
> +	dsi->debugfs_root = d;
> +
> +	d = debugfs_create_bool("vpg", 0660, dsi->debugfs_root, &dsi->vpg);
> +	dsi->debugfs_vpg = d;

If you don't handle failures here you don't need the intermediate d
variable, you can just assign dsi->debugfs_vpg directly.

> +
> +	d = debugfs_create_bool("vpg_horizontal", 0660, dsi->debugfs_root,
> +				&dsi->vpg_horizontal);
> +	dsi->debugfs_vpg_horizontal = d;

Same here.

> +}
> +
> +static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi)
> +{
> +	debugfs_remove(dsi->debugfs_vpg_horizontal);
> +	debugfs_remove(dsi->debugfs_vpg);
> +	debugfs_remove(dsi->debugfs_root);
> +}
> +
> +#else
> +
> +static void dw_mipi_dsi_debugfs_init(struct dw_mipi_dsi *dsi) { }
> +static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi) { }
> +
> +#endif /* CONFIG_DEBUG_FS */
> +
>  static struct dw_mipi_dsi *
>  __dw_mipi_dsi_probe(struct platform_device *pdev,
>  		    const struct dw_mipi_dsi_plat_data *plat_data)
> @@ -1005,6 +1057,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>  		clk_disable_unprepare(dsi->pclk);
>  	}
>  
> +	dw_mipi_dsi_debugfs_init(dsi);
>  	pm_runtime_enable(dev);
>  
>  	dsi->dsi_host.ops = &dw_mipi_dsi_host_ops;
> @@ -1012,6 +1065,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>  	ret = mipi_dsi_host_register(&dsi->dsi_host);
>  	if (ret) {
>  		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
> +		dw_mipi_dsi_debugfs_remove(dsi);
>  		return ERR_PTR(ret);
>  	}
>  
> @@ -1029,6 +1083,7 @@ static void __dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi)
>  	mipi_dsi_host_unregister(&dsi->dsi_host);
>  
>  	pm_runtime_disable(dsi->dev);
> +	dw_mipi_dsi_debugfs_remove(dsi);
>  }
>  
>  void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi *slave)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/bridge/synopsys: dsi: Allow VPG to be enabled via debugfs
  2019-04-25  9:31 ` Laurent Pinchart
@ 2019-04-25 12:42   ` Matt Redfearn
  0 siblings, 0 replies; 3+ messages in thread
From: Matt Redfearn @ 2019-04-25 12:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Archit Taneja, David Airlie, Philippe Cornu, Matthew Redfearn,
	linux-kernel, Nickey Yang, dri-devel

Hi Laurent,

Thanks for the review!


On 25/04/2019 10:31, Laurent Pinchart wrote:
> On Wed, Apr 24, 2019 at 02:22:18PM +0000, Matt Redfearn wrote:
>> The Synopsys MIPI DSI IP contains a video test pattern generator which
>> is helpful in debugging video timing with connected displays.
>> Add a debugfs directory containing files which allow the VPG to be
>> enabled and disabled, and it's orientation to be changed.
> 
> s/it's/its/
>>
>> Signed-off-by: Matt Redfearn <matt.redfearn@thinci.com>
>>
>> ---
>>
>>   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 55 +++++++++++++++++++
>>   1 file changed, 55 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> index 0ee440216b8..a1ee2306382 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> @@ -10,6 +10,7 @@
>>   
>>   #include <linux/clk.h>
>>   #include <linux/component.h>
>> +#include <linux/debugfs.h>
>>   #include <linux/iopoll.h>
>>   #include <linux/module.h>
>>   #include <linux/of_device.h>
>> @@ -86,6 +87,8 @@
>>   #define VID_MODE_TYPE_NON_BURST_SYNC_EVENTS	0x1
>>   #define VID_MODE_TYPE_BURST			0x2
>>   #define VID_MODE_TYPE_MASK			0x3
>> +#define VID_MODE_VPG_ENABLE		BIT(16)
>> +#define VID_MODE_VPG_HORIZONTAL		BIT(24)
>>   
>>   #define DSI_VID_PKT_SIZE		0x3c
>>   #define VID_PKT_SIZE(p)			((p) & 0x3fff)
>> @@ -234,6 +237,15 @@ struct dw_mipi_dsi {
>>   	u32 format;
>>   	unsigned long mode_flags;
>>   
>> +#ifdef CONFIG_DEBUG_FS
>> +	struct dentry *debugfs_root;
>> +	struct dentry *debugfs_vpg;
>> +	struct dentry *debugfs_vpg_horizontal;
>> +
>> +	bool vpg;
>> +	bool vpg_horizontal;
>> +#endif /* CONFIG_DEBUG_FS */
>> +
>>   	struct dw_mipi_dsi *master; /* dual-dsi master ptr */
>>   	struct dw_mipi_dsi *slave; /* dual-dsi slave ptr */
>>   
>> @@ -525,6 +537,11 @@ static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
>>   	else
>>   		val |= VID_MODE_TYPE_NON_BURST_SYNC_EVENTS;
>>   
>> +	if (dsi->vpg) {
> 
> Won't this break if CONFIG_DDEBUG_FS is not defined ?

Ah yes - quite right.

> 
>> +		val |= VID_MODE_VPG_ENABLE;
>> +		val |= dsi->vpg_horizontal ? VID_MODE_VPG_HORIZONTAL : 0;
>> +	}
>> +
>>   	dsi_write(dsi, DSI_VID_MODE_CFG, val);
>>   }
>>   
>> @@ -935,6 +952,41 @@ static const struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = {
>>   	.attach	      = dw_mipi_dsi_bridge_attach,
>>   };
>>   
>> +#ifdef CONFIG_DEBUG_FS
>> +
>> +static void dw_mipi_dsi_debugfs_init(struct dw_mipi_dsi *dsi)
>> +{
>> +	struct dentry *d;
>> +
>> +	d = debugfs_create_dir(dev_name(dsi->dev), NULL);
>> +	if (IS_ERR(d)) {
>> +		dev_err(dsi->dev, "failed to create debugfs root\n");
>> +		return;
>> +	}
>> +	dsi->debugfs_root = d;
>> +
>> +	d = debugfs_create_bool("vpg", 0660, dsi->debugfs_root, &dsi->vpg);
>> +	dsi->debugfs_vpg = d;
> 
> If you don't handle failures here you don't need the intermediate d
> variable, you can just assign dsi->debugfs_vpg directly.

True - I will update it.

> 
>> +
>> +	d = debugfs_create_bool("vpg_horizontal", 0660, dsi->debugfs_root,
>> +				&dsi->vpg_horizontal);
>> +	dsi->debugfs_vpg_horizontal = d;
> 
> Same here.


And here - thanks!

Matt

> 
>> +}
>> +
>> +static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi)
>> +{
>> +	debugfs_remove(dsi->debugfs_vpg_horizontal);
>> +	debugfs_remove(dsi->debugfs_vpg);
>> +	debugfs_remove(dsi->debugfs_root);
>> +}
>> +
>> +#else
>> +
>> +static void dw_mipi_dsi_debugfs_init(struct dw_mipi_dsi *dsi) { }
>> +static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi) { }
>> +
>> +#endif /* CONFIG_DEBUG_FS */
>> +
>>   static struct dw_mipi_dsi *
>>   __dw_mipi_dsi_probe(struct platform_device *pdev,
>>   		    const struct dw_mipi_dsi_plat_data *plat_data)
>> @@ -1005,6 +1057,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>>   		clk_disable_unprepare(dsi->pclk);
>>   	}
>>   
>> +	dw_mipi_dsi_debugfs_init(dsi);
>>   	pm_runtime_enable(dev);
>>   
>>   	dsi->dsi_host.ops = &dw_mipi_dsi_host_ops;
>> @@ -1012,6 +1065,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>>   	ret = mipi_dsi_host_register(&dsi->dsi_host);
>>   	if (ret) {
>>   		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
>> +		dw_mipi_dsi_debugfs_remove(dsi);
>>   		return ERR_PTR(ret);
>>   	}
>>   
>> @@ -1029,6 +1083,7 @@ static void __dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi)
>>   	mipi_dsi_host_unregister(&dsi->dsi_host);
>>   
>>   	pm_runtime_disable(dsi->dev);
>> +	dw_mipi_dsi_debugfs_remove(dsi);
>>   }
>>   
>>   void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi *slave)
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-25 12:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24 14:22 [PATCH] drm/bridge/synopsys: dsi: Allow VPG to be enabled via debugfs Matt Redfearn
2019-04-25  9:31 ` Laurent Pinchart
2019-04-25 12:42   ` Matt Redfearn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).