Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 4/4] fbdev: sh_mobile_hdmi: add HDMI Control Register support
From: Kuninori Morimoto @ 2012-05-08  4:08 UTC (permalink / raw)
  To: linux-fbdev

Latest SuperH HDMI uses not only HDMI Core Register (HTOP0)
but also HDMI Control Register (HTOP1).
This patch adds HDMI Control Register support.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/video/sh_mobile_hdmi.c |  152 +++++++++++++++++++++++++++++++++++++++-
 include/video/sh_mobile_hdmi.h |    1 +
 2 files changed, 152 insertions(+), 1 deletions(-)

diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index 4d48a80..930e550 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -31,6 +31,7 @@
 
 #include "sh_mobile_lcdcfb.h"
 
+/* HDMI Core Control Register (HTOP0) */
 #define HDMI_SYSTEM_CTRL			0x00 /* System control */
 #define HDMI_L_R_DATA_SWAP_CTRL_RPKT		0x01 /* L/R data swap control,
 							bits 19..16 of 20-bit N for Audio Clock Regeneration packet */
@@ -201,6 +202,68 @@
 #define HDMI_REVISION_ID			0xF1 /* Revision ID */
 #define HDMI_TEST_MODE				0xFE /* Test mode */
 
+/* HDMI Control Register (HTOP1) */
+#define HDMI_HTOP1_TEST_MODE			0x0000 /* Test mode */
+#define HDMI_HTOP1_VIDEO_INPUT			0x0008 /* VideoInput */
+#define HDMI_HTOP1_CORE_RSTN			0x000C /* CoreResetn */
+#define HDMI_HTOP1_PLLBW			0x0018 /* PLLBW */
+#define HDMI_HTOP1_CLK_TO_PHY			0x001C /* Clk to Phy */
+#define HDMI_HTOP1_VIDEO_INPUT2			0x0020 /* VideoInput2 */
+#define HDMI_HTOP1_TISEMP0_1			0x0024 /* tisemp0-1 */
+#define HDMI_HTOP1_TISEMP2_C			0x0028 /* tisemp2-c */
+#define HDMI_HTOP1_TISIDRV			0x002C /* tisidrv */
+#define HDMI_HTOP1_TISEN			0x0034 /* tisen */
+#define HDMI_HTOP1_TISDREN			0x0038 /* tisdren  */
+#define HDMI_HTOP1_CISRANGE			0x003C /* cisrange  */
+#define HDMI_HTOP1_ENABLE_SELECTOR		0x0040 /* Enable Selector */
+#define HDMI_HTOP1_MACRO_RESET			0x0044 /* Macro reset */
+#define HDMI_HTOP1_PLL_CALIBRATION		0x0048 /* PLL calibration */
+#define HDMI_HTOP1_RE_CALIBRATION		0x004C /* Re-calibration */
+#define HDMI_HTOP1_CURRENT			0x0050 /* Current */
+#define HDMI_HTOP1_PLL_LOCK_DETECT		0x0054 /* PLL lock detect */
+#define HDMI_HTOP1_PHY_TEST_MODE		0x0058 /* PHY Test Mode */
+#define HDMI_HTOP1_CLK_SET			0x0080 /* Clock Set */
+#define HDMI_HTOP1_DDC_FAIL_SAFE		0x0084 /* DDC fail safe */
+#define HDMI_HTOP1_PRBS				0x0088 /* PRBS */
+#define HDMI_HTOP1_EDID_AINC_CONTROL		0x008C /* EDID ainc Control */
+#define HDMI_HTOP1_HTOP_DCL_MODE		0x00FC /* Deep Coloer Mode */
+#define HDMI_HTOP1_HTOP_DCL_FRC_COEF0		0x0100 /* Deep Color:FRC COEF0 */
+#define HDMI_HTOP1_HTOP_DCL_FRC_COEF1		0x0104 /* Deep Color:FRC COEF1 */
+#define HDMI_HTOP1_HTOP_DCL_FRC_COEF2		0x0108 /* Deep Color:FRC COEF2 */
+#define HDMI_HTOP1_HTOP_DCL_FRC_COEF3		0x010C /* Deep Color:FRC COEF3 */
+#define HDMI_HTOP1_HTOP_DCL_FRC_COEF0_C		0x0110 /* Deep Color:FRC COEF0C */
+#define HDMI_HTOP1_HTOP_DCL_FRC_COEF1_C		0x0114 /* Deep Color:FRC COEF1C */
+#define HDMI_HTOP1_HTOP_DCL_FRC_COEF2_C		0x0118 /* Deep Color:FRC COEF2C */
+#define HDMI_HTOP1_HTOP_DCL_FRC_COEF3_C		0x011C /* Deep Color:FRC COEF3C */
+#define HDMI_HTOP1_HTOP_DCL_FRC_MODE		0x0120 /* Deep Color:FRC Mode */
+#define HDMI_HTOP1_HTOP_DCL_RECT_START1		0x0124 /* Deep Color:Rect Start1 */
+#define HDMI_HTOP1_HTOP_DCL_RECT_SIZE1		0x0128 /* Deep Color:Rect Size1 */
+#define HDMI_HTOP1_HTOP_DCL_RECT_START2		0x012C /* Deep Color:Rect Start2 */
+#define HDMI_HTOP1_HTOP_DCL_RECT_SIZE2		0x0130 /* Deep Color:Rect Size2 */
+#define HDMI_HTOP1_HTOP_DCL_RECT_START3		0x0134 /* Deep Color:Rect Start3 */
+#define HDMI_HTOP1_HTOP_DCL_RECT_SIZE3		0x0138 /* Deep Color:Rect Size3 */
+#define HDMI_HTOP1_HTOP_DCL_RECT_START4		0x013C /* Deep Color:Rect Start4 */
+#define HDMI_HTOP1_HTOP_DCL_RECT_SIZE4		0x0140 /* Deep Color:Rect Size4 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_Y1_1	0x0144 /* Deep Color:Fil Para Y1_1 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_Y1_2	0x0148 /* Deep Color:Fil Para Y1_2 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_CB1_1	0x014C /* Deep Color:Fil Para CB1_1 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_CB1_2	0x0150 /* Deep Color:Fil Para CB1_2 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_CR1_1	0x0154 /* Deep Color:Fil Para CR1_1 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_CR1_2	0x0158 /* Deep Color:Fil Para CR1_2 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_Y2_1	0x015C /* Deep Color:Fil Para Y2_1 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_Y2_2	0x0160 /* Deep Color:Fil Para Y2_2 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_CB2_1	0x0164 /* Deep Color:Fil Para CB2_1 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_CB2_2	0x0168 /* Deep Color:Fil Para CB2_2 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_CR2_1	0x016C /* Deep Color:Fil Para CR2_1 */
+#define HDMI_HTOP1_HTOP_DCL_FIL_PARA_CR2_2	0x0170 /* Deep Color:Fil Para CR2_2 */
+#define HDMI_HTOP1_HTOP_DCL_COR_PARA_Y1		0x0174 /* Deep Color:Cor Para Y1 */
+#define HDMI_HTOP1_HTOP_DCL_COR_PARA_CB1	0x0178 /* Deep Color:Cor Para CB1 */
+#define HDMI_HTOP1_HTOP_DCL_COR_PARA_CR1	0x017C /* Deep Color:Cor Para CR1 */
+#define HDMI_HTOP1_HTOP_DCL_COR_PARA_Y2		0x0180 /* Deep Color:Cor Para Y2 */
+#define HDMI_HTOP1_HTOP_DCL_COR_PARA_CB2	0x0184 /* Deep Color:Cor Para CB2 */
+#define HDMI_HTOP1_HTOP_DCL_COR_PARA_CR2	0x0188 /* Deep Color:Cor Para CR2 */
+#define HDMI_HTOP1_EDID_DATA_READ		0x0200 /* EDID Data Read 128Byte:0x03FC */
+
 enum hotplug_state {
 	HDMI_HOTPLUG_DISCONNECTED,
 	HDMI_HOTPLUG_CONNECTED,
@@ -211,6 +274,7 @@ struct sh_hdmi {
 	struct sh_mobile_lcdc_entity entity;
 
 	void __iomem *base;
+	void __iomem *htop1;
 	enum hotplug_state hp_state;	/* hot-plug status */
 	u8 preprogrammed_vic;		/* use a pre-programmed VIC or
 					   the external mode */
@@ -271,6 +335,17 @@ static void hdmi_bit_set(struct sh_hdmi *hdmi, u8 mask, u8 data, u8 reg)
 	hdmi_write(hdmi, val, reg);
 }
 
+static void hdmi_htop1_write(struct sh_hdmi *hdmi, u32 data, u32 reg)
+{
+	iowrite32(data, hdmi->htop1 + reg);
+	udelay(100);
+}
+
+static u32 hdmi_htop1_read(struct sh_hdmi *hdmi, u32 reg)
+{
+	return ioread32(hdmi->htop1 + reg);
+}
+
 /*
  *	HDMI sound
  */
@@ -781,7 +856,9 @@ static int sh_hdmi_read_edid(struct sh_hdmi *hdmi, unsigned long *hdmi_rate,
 	/* Read EDID */
 	dev_dbg(hdmi->dev, "Read back EDID code:");
 	for (i = 0; i < 128; i++) {
-		edid[i] = hdmi_read(hdmi, HDMI_EDID_KSV_FIFO_ACCESS_WINDOW);
+		edid[i] = (hdmi->htop1) ?
+			(u8)hdmi_htop1_read(hdmi, HDMI_HTOP1_EDID_DATA_READ + (i * 4)) :
+			hdmi_read(hdmi, HDMI_EDID_KSV_FIFO_ACCESS_WINDOW);
 #ifdef DEBUG
 		if ((i % 16) = 0) {
 			printk(KERN_CONT "\n");
@@ -1145,10 +1222,58 @@ out:
 	dev_dbg(hdmi->dev, "%s(%p): end\n", __func__, hdmi);
 }
 
+static void sh_hdmi_htop1_init(struct sh_hdmi *hdmi)
+{
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_HTOP_DCL_MODE);
+	hdmi_htop1_write(hdmi, 0x0000000b, 0x0010);
+	hdmi_htop1_write(hdmi, 0x00006710, HDMI_HTOP1_HTOP_DCL_FRC_MODE);
+	hdmi_htop1_write(hdmi, 0x01020406, HDMI_HTOP1_HTOP_DCL_FIL_PARA_Y1_1);
+	hdmi_htop1_write(hdmi, 0x07080806, HDMI_HTOP1_HTOP_DCL_FIL_PARA_Y1_2);
+	hdmi_htop1_write(hdmi, 0x01020406, HDMI_HTOP1_HTOP_DCL_FIL_PARA_CB1_1);
+	hdmi_htop1_write(hdmi, 0x07080806, HDMI_HTOP1_HTOP_DCL_FIL_PARA_CB1_2);
+	hdmi_htop1_write(hdmi, 0x01020406, HDMI_HTOP1_HTOP_DCL_FIL_PARA_CR1_1);
+	hdmi_htop1_write(hdmi, 0x07080806, HDMI_HTOP1_HTOP_DCL_FIL_PARA_CR1_2);
+	hdmi_htop1_write(hdmi, 0x01020406, HDMI_HTOP1_HTOP_DCL_FIL_PARA_Y2_1);
+	hdmi_htop1_write(hdmi, 0x07080806, HDMI_HTOP1_HTOP_DCL_FIL_PARA_Y2_2);
+	hdmi_htop1_write(hdmi, 0x01020406, HDMI_HTOP1_HTOP_DCL_FIL_PARA_CB2_1);
+	hdmi_htop1_write(hdmi, 0x07080806, HDMI_HTOP1_HTOP_DCL_FIL_PARA_CB2_2);
+	hdmi_htop1_write(hdmi, 0x01020406, HDMI_HTOP1_HTOP_DCL_FIL_PARA_CR2_1);
+	hdmi_htop1_write(hdmi, 0x07080806, HDMI_HTOP1_HTOP_DCL_FIL_PARA_CR2_2);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_HTOP_DCL_COR_PARA_Y1);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_HTOP_DCL_COR_PARA_CB1);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_HTOP_DCL_COR_PARA_CR1);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_HTOP_DCL_COR_PARA_Y2);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_HTOP_DCL_COR_PARA_CB2);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_HTOP_DCL_COR_PARA_CR2);
+	hdmi_htop1_write(hdmi, 0x00000008, HDMI_HTOP1_CURRENT);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_TISEMP0_1);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_TISEMP2_C);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_PHY_TEST_MODE);
+	hdmi_htop1_write(hdmi, 0x00000081, HDMI_HTOP1_TISIDRV);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_PLLBW);
+	hdmi_htop1_write(hdmi, 0x0000000f, HDMI_HTOP1_TISEN);
+	hdmi_htop1_write(hdmi, 0x0000000f, HDMI_HTOP1_TISDREN);
+	hdmi_htop1_write(hdmi, 0x00000003, HDMI_HTOP1_ENABLE_SELECTOR);
+	hdmi_htop1_write(hdmi, 0x00000001, HDMI_HTOP1_MACRO_RESET);
+	hdmi_htop1_write(hdmi, 0x00000016, HDMI_HTOP1_CISRANGE);
+	msleep(100);
+	hdmi_htop1_write(hdmi, 0x00000001, HDMI_HTOP1_ENABLE_SELECTOR);
+	msleep(100);
+	hdmi_htop1_write(hdmi, 0x00000003, HDMI_HTOP1_ENABLE_SELECTOR);
+	hdmi_htop1_write(hdmi, 0x00000001, HDMI_HTOP1_MACRO_RESET);
+	hdmi_htop1_write(hdmi, 0x0000000f, HDMI_HTOP1_TISEN);
+	hdmi_htop1_write(hdmi, 0x0000000f, HDMI_HTOP1_TISDREN);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_VIDEO_INPUT);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_CLK_TO_PHY);
+	hdmi_htop1_write(hdmi, 0x00000000, HDMI_HTOP1_VIDEO_INPUT2);
+	hdmi_htop1_write(hdmi, 0x0000000a, HDMI_HTOP1_CLK_SET);
+}
+
 static int __init sh_hdmi_probe(struct platform_device *pdev)
 {
 	struct sh_mobile_hdmi_info *pdata = pdev->dev.platform_data;
 	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct resource *htop1_res;
 	int irq = platform_get_irq(pdev, 0), ret;
 	struct sh_hdmi *hdmi;
 	long rate;
@@ -1156,6 +1281,15 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
 	if (!res || !pdata || irq < 0)
 		return -ENODEV;
 
+	htop1_res = NULL;
+	if (pdata->flags & HDMI_HAS_HTOP1) {
+		htop1_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		if (!htop1_res) {
+			dev_err(&pdev->dev, "htop1 needs register base\n");
+			return -EINVAL;
+		}
+	}
+
 	hdmi = kzalloc(sizeof(*hdmi), GFP_KERNEL);
 	if (!hdmi) {
 		dev_err(&pdev->dev, "Cannot allocate device data\n");
@@ -1227,6 +1361,17 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
 	if (pdata->flags & HDMI_OUTPUT_POLARITY_HI)
 		hdmi_bit_set(hdmi, 0x01, 0x01, HDMI_SYSTEM_CTRL);
 
+	/* enable htop1 register if needed */
+	if (htop1_res) {
+		hdmi->htop1 = ioremap(htop1_res->start, resource_size(htop1_res));
+		if (!hdmi->htop1) {
+			dev_err(&pdev->dev, "control register region already claimed\n");
+			ret = -ENOMEM;
+			goto emap_htop1;
+		}
+		sh_hdmi_htop1_init(hdmi);
+	}
+
 	/* Product and revision IDs are 0 in sh-mobile version */
 	dev_info(&pdev->dev, "Detected HDMI controller 0x%x:0x%x\n",
 		 hdmi_read(hdmi, HDMI_PRODUCT_ID), hdmi_read(hdmi, HDMI_REVISION_ID));
@@ -1250,6 +1395,9 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
 ecodec:
 	free_irq(irq, hdmi);
 ereqirq:
+	if (hdmi->htop1)
+		iounmap(hdmi->htop1);
+emap_htop1:
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	iounmap(hdmi->base);
@@ -1281,6 +1429,8 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	clk_disable(hdmi->hdmi_clk);
 	clk_put(hdmi->hdmi_clk);
+	if (hdmi->htop1)
+		iounmap(hdmi->htop1);
 	iounmap(hdmi->base);
 	release_mem_region(res->start, resource_size(res));
 	kfree(hdmi);
diff --git a/include/video/sh_mobile_hdmi.h b/include/video/sh_mobile_hdmi.h
index ce8a540..63d20ef 100644
--- a/include/video/sh_mobile_hdmi.h
+++ b/include/video/sh_mobile_hdmi.h
@@ -38,6 +38,7 @@ struct clk;
 
 /* Chip specific option */
 #define HDMI_32BIT_REG		(1 << 8)
+#define HDMI_HAS_HTOP1		(1 << 9)
 
 struct sh_mobile_hdmi_info {
 	unsigned int			 flags;
-- 
1.7.5.4


^ permalink raw reply related

* Re: [PATCH v2 1/4] OMAPDSS: APPLY: Add manager timings as extra_info in private data
From: Archit Taneja @ 2012-05-08  4:36 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336402071.2667.16.camel@deskari>

Hi,

On Monday 07 May 2012 08:17 PM, Tomi Valkeinen wrote:
> Hi,
>
> On Thu, 2012-05-03 at 12:37 +0530, Archit Taneja wrote:
>> DISPC manager size and DISPC manager blanking parameters(for LCD managers)
>> follow the shadow register programming model. Currently, they are programmed
>> directly by the interface drivers.
>>
>> To configure manager timings using APPLY, there is a need to introduce extra
>> info flags for managers, similar to what is done for overlays. This is needed
>> because timings aren't a part of overlay_manager_info struct configured by a
>> user of DSS, they are configured internally by the interface or panel drivers.
>>
>> Add dirty and shadow_dirty extra_info flags for managers, update these flags
>> at the appropriate places. Rewrite the function extra_info_update_ongoing()
>> slightly as checking for manager's extra_info flags can simplify the code a bit.
>>
>> Create function dss_mgr_set_timings() which applies the new manager timings to
>> extra_info.
>>
>> Signed-off-by: Archit Taneja<archit@ti.com>
>
> <snip>
>
>> +static void dss_apply_mgr_timings(struct omap_overlay_manager *mgr,
>> +		struct omap_video_timings *timings)
>> +{
>> +	struct mgr_priv_data *mp = get_mgr_priv(mgr);
>> +
>> +	mp->timings = *timings;
>> +	mp->extra_info_dirty = true;
>> +}
>> +
>> +void dss_mgr_set_timings(struct omap_overlay_manager *mgr,
>> +		struct omap_video_timings *timings)
>> +{
>> +	unsigned long flags;
>> +
>> +	mutex_lock(&apply_lock);
>> +
>> +	spin_lock_irqsave(&data_lock, flags);
>> +
>> +	dss_apply_mgr_timings(mgr, timings);
>> +
>> +	spin_unlock_irqrestore(&data_lock, flags);
>> +
>> +	mutex_unlock(&apply_lock);
>> +}
>
> With this, dpi.c&  others still need to use dispc_mgr_go(), which is
> something that should be removed and done only from apply.c.

Ah ok, so with this set, dss_mgr_set_timings() doesn't ensure that the 
configuration is taken in, the configuration may go in the next 
overlay/manager enable or the next mgr_apply, but that may happen much 
later.

>
> dss_<ovl|mgr>_<enable|disable>  functions handle GO, so you could have a
> look at them. However, setting the timings is a bit different, so I'm
> not sure how it should be done.
>
> I think we have a few different options:
>
> - Separate omapdss internal function (in apply.c) that can be used to
> set GO after set_timings
>
> - set GO in dss_mgr_set_timings(), but don't block
>
> - set GO in dss_mgr_set_timings(), and block until the changes are in HW
> (this is more or less what the dss_<ovl|mgr>_<enable|disable>  functions
> do).
>
> The first one would be good if the interface drivers would need to set
> multiple configurations, and we don't want to block after each set call.
> But we don't have anything like that, at least currently.
>
> The second one avoids blocking, but could perhaps cause problems because
> the timings are not actually used yet when the function returns.
>
> I don't see any problem with the last option, so I'm slightly leaning
> towards it.

The 3rd option looks good to me too, but I'm wondering if we would need 
to do the same things with all manager parameters which are in shadow 
registers. Like in dpi.c, in dpi_set_mode() we set the DISPC_POL_FREQ 
and DISPC_DIVISORo registers, writing GO for each parameter would be in 
efficient, it's good that it doesn't happen much often. Maybe we could 
group the rest of these parameters.

Archit

>
>   Tomi
>


^ permalink raw reply

* Re: [PATCH v2 4/4] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Archit Taneja @ 2012-05-08  5:15 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Archit Taneja, linux-omap, linux-fbdev
In-Reply-To: <1336403017.3522.2.camel@lappyti>

On Monday 07 May 2012 08:33 PM, Tomi Valkeinen wrote:
> On Thu, 2012-05-03 at 12:37 +0530, Archit Taneja wrote:
>> In order to check the validity of overlay and manager info, there was a need to
>> use the omap_dss_device struct to get the panel resolution. The manager's
>> private data in APPLY now contains the manager timings. Hence, we don't need to
>> rely on the display resolution any more.
>>
>> Create a function dss_mgr_get_timings() which returns the timings in manager's
>> private data. Remove the need of passing omap_dss_device structs in the
>> functions which check for overlay and managers.
>>
>> Have some initial values for manager timings in apply_init(), these would ensure
>> that manager checks don't fail if an interface driver or a panel driver hasn't
>> set the manager timings yet.
>>
>> Signed-off-by: Archit Taneja<archit@ti.com>
>
>> +struct omap_video_timings *dss_mgr_get_timings(struct omap_overlay_manager *mgr)
>> +{
>> +	struct mgr_priv_data *mp = get_mgr_priv(mgr);
>> +
>> +	return&mp->timings;
>> +}
>
> This one returns a pointer into apply.c's internal data structures. The
> safest way would be to return a copy, but as it's an omapdss internal
> function, I think it's enough to return a pointer to a const struct.

Okay I'll fix that, I was a bit concerned about the locking here, I use 
this function in the later series to remove some dssdev references in 
dispc.c. I traced the paths and saw that in all cases this function 
would be protected by the data_lock spinlock, but not the apply_lock 
mutex in all cases. Any thoughts on this?

Archit

>
>   Tomi
>


^ permalink raw reply

* Re: [PATCH v2 1/4] OMAPDSS: APPLY: Add manager timings as extra_info in private data
From: Tomi Valkeinen @ 2012-05-08  7:01 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4FA8A00B.4040600@ti.com>

[-- Attachment #1: Type: text/plain, Size: 3059 bytes --]

On Tue, 2012-05-08 at 09:54 +0530, Archit Taneja wrote:
> Hi,
> 
> On Monday 07 May 2012 08:17 PM, Tomi Valkeinen wrote:
> > Hi,

> > dss_<ovl|mgr>_<enable|disable>  functions handle GO, so you could have a
> > look at them. However, setting the timings is a bit different, so I'm
> > not sure how it should be done.
> >
> > I think we have a few different options:
> >
> > - Separate omapdss internal function (in apply.c) that can be used to
> > set GO after set_timings
> >
> > - set GO in dss_mgr_set_timings(), but don't block
> >
> > - set GO in dss_mgr_set_timings(), and block until the changes are in HW
> > (this is more or less what the dss_<ovl|mgr>_<enable|disable>  functions
> > do).
> >
> > The first one would be good if the interface drivers would need to set
> > multiple configurations, and we don't want to block after each set call.
> > But we don't have anything like that, at least currently.
> >
> > The second one avoids blocking, but could perhaps cause problems because
> > the timings are not actually used yet when the function returns.
> >
> > I don't see any problem with the last option, so I'm slightly leaning
> > towards it.
> 
> The 3rd option looks good to me too, but I'm wondering if we would need 
> to do the same things with all manager parameters which are in shadow 
> registers. Like in dpi.c, in dpi_set_mode() we set the DISPC_POL_FREQ 
> and DISPC_DIVISORo registers, writing GO for each parameter would be in 
> efficient, it's good that it doesn't happen much often. Maybe we could 
> group the rest of these parameters.

Hmm, that's true.

We could make shortcuts with settings that are only changed while the
display is off. For example, currently DISPC_POL_FREQ cannot be changed
dynamically, so it could only be set when before enabling the output.
However, it seems we currently do call dispc_mgr_set_pol_freq() in
dpi_set_mode(), which could be called when the output is enabled. But
even then we always set the same values, so it doesn't matter.

For DISPC_DIVISORo, I guess we could make a shortcut with that also.
While it is a shadow register, when we are changing the dss clocks we
also set non-shadowed registers. So I think the best way to change clock
frequencies is to turn off the output temporarily.

Perhaps all the shadow registers currently being set directly from
interface drivers are such settings? The code should still be changed so
that we only touch the registers when enabling the output.

But, of course, a better design and also more future proof would be to
handle all shadow registers properly, even if we currently only set them
while the output is off. That may be left for future, though.

In any case, I think we should mark clearly the places where we set
shadow registers directly, without using the apply.c. Perhaps we should
even mark all the dispc functions that set shadow registers, like
dispc_mgr_set_pol_freq_sh(). But let's not do that now either, as we're
quite close to merge window.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2 4/4] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Tomi Valkeinen @ 2012-05-08  7:16 UTC (permalink / raw)
  To: Archit Taneja; +Cc: Archit Taneja, linux-omap, linux-fbdev
In-Reply-To: <4FA8A927.9080705@ti.com>

[-- Attachment #1: Type: text/plain, Size: 2872 bytes --]

On Tue, 2012-05-08 at 10:33 +0530, Archit Taneja wrote:
> On Monday 07 May 2012 08:33 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-05-03 at 12:37 +0530, Archit Taneja wrote:
> >> In order to check the validity of overlay and manager info, there was a need to
> >> use the omap_dss_device struct to get the panel resolution. The manager's
> >> private data in APPLY now contains the manager timings. Hence, we don't need to
> >> rely on the display resolution any more.
> >>
> >> Create a function dss_mgr_get_timings() which returns the timings in manager's
> >> private data. Remove the need of passing omap_dss_device structs in the
> >> functions which check for overlay and managers.
> >>
> >> Have some initial values for manager timings in apply_init(), these would ensure
> >> that manager checks don't fail if an interface driver or a panel driver hasn't
> >> set the manager timings yet.
> >>
> >> Signed-off-by: Archit Taneja<archit@ti.com>
> >
> >> +struct omap_video_timings *dss_mgr_get_timings(struct omap_overlay_manager *mgr)
> >> +{
> >> +	struct mgr_priv_data *mp = get_mgr_priv(mgr);
> >> +
> >> +	return&mp->timings;
> >> +}
> >
> > This one returns a pointer into apply.c's internal data structures. The
> > safest way would be to return a copy, but as it's an omapdss internal
> > function, I think it's enough to return a pointer to a const struct.
> 
> Okay I'll fix that, I was a bit concerned about the locking here, I use 
> this function in the later series to remove some dssdev references in 
> dispc.c. I traced the paths and saw that in all cases this function 
> would be protected by the data_lock spinlock, but not the apply_lock 
> mutex in all cases. Any thoughts on this?

Hmm, you're right, locking here gets a bit confusing. set_timings has
locks, so logically get_timings should also. But I guess all the uses of
get_timings happens via apply.c, and apply.c already holds the
data_lock, as you said?

Making the get_timings function public (inside omapdss) is a bit nasty,
as it's quite easy to call it without having the appropriate locks. And
actually there's no way to acquire the locks outside apply.c

I'm not sure if it applies here, but as a general strategy, I suggest
doing things in apply.c that require data from apply.c's internal data.
When that's not possible, apply.c should call the functions outside
apply.c, and pass the internal data as parameters (like calls to dispc).

In your case, I for example see dss_mgr_check() calling
dss_mgr_get_timings(). It would be quite easy to pass the timings to
dss_mgr_check() from apply.c, thus removing the need to call the
function. And, as you see, dss_mgr_check() already has a bunch of
parameters, and the idea is the same with those: give the params, so
that dss_mgr_check() doesn't need to ask for them.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2 4/4] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Archit Taneja @ 2012-05-08  7:50 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336461377.1896.23.camel@lappyti>

On Tuesday 08 May 2012 12:46 PM, Tomi Valkeinen wrote:
> On Tue, 2012-05-08 at 10:33 +0530, Archit Taneja wrote:
>> On Monday 07 May 2012 08:33 PM, Tomi Valkeinen wrote:
>>> On Thu, 2012-05-03 at 12:37 +0530, Archit Taneja wrote:
>>>> In order to check the validity of overlay and manager info, there was a need to
>>>> use the omap_dss_device struct to get the panel resolution. The manager's
>>>> private data in APPLY now contains the manager timings. Hence, we don't need to
>>>> rely on the display resolution any more.
>>>>
>>>> Create a function dss_mgr_get_timings() which returns the timings in manager's
>>>> private data. Remove the need of passing omap_dss_device structs in the
>>>> functions which check for overlay and managers.
>>>>
>>>> Have some initial values for manager timings in apply_init(), these would ensure
>>>> that manager checks don't fail if an interface driver or a panel driver hasn't
>>>> set the manager timings yet.
>>>>
>>>> Signed-off-by: Archit Taneja<archit@ti.com>
>>>
>>>> +struct omap_video_timings *dss_mgr_get_timings(struct omap_overlay_manager *mgr)
>>>> +{
>>>> +	struct mgr_priv_data *mp = get_mgr_priv(mgr);
>>>> +
>>>> +	return&mp->timings;
>>>> +}
>>>
>>> This one returns a pointer into apply.c's internal data structures. The
>>> safest way would be to return a copy, but as it's an omapdss internal
>>> function, I think it's enough to return a pointer to a const struct.
>>
>> Okay I'll fix that, I was a bit concerned about the locking here, I use
>> this function in the later series to remove some dssdev references in
>> dispc.c. I traced the paths and saw that in all cases this function
>> would be protected by the data_lock spinlock, but not the apply_lock
>> mutex in all cases. Any thoughts on this?
>
> Hmm, you're right, locking here gets a bit confusing. set_timings has
> locks, so logically get_timings should also. But I guess all the uses of
> get_timings happens via apply.c, and apply.c already holds the
> data_lock, as you said?

Yes.

>
> Making the get_timings function public (inside omapdss) is a bit nasty,
> as it's quite easy to call it without having the appropriate locks. And
> actually there's no way to acquire the locks outside apply.c
>
> I'm not sure if it applies here, but as a general strategy, I suggest
> doing things in apply.c that require data from apply.c's internal data.
> When that's not possible, apply.c should call the functions outside
> apply.c, and pass the internal data as parameters (like calls to dispc).
>
> In your case, I for example see dss_mgr_check() calling
> dss_mgr_get_timings(). It would be quite easy to pass the timings to
> dss_mgr_check() from apply.c, thus removing the need to call the
> function. And, as you see, dss_mgr_check() already has a bunch of
> parameters, and the idea is the same with those: give the params, so
> that dss_mgr_check() doesn't need to ask for them.

Right, I can do this for dss_mgr_check() and dss_ovl_check(), but if I 
you see the misc cleanup patch series I posted, I use 
dss_mgr_get_timings()  in dispc.c functions to remove some 
omap_dss_device references. These DISPC functions are called in 
dispc_ovl_setup(), I could pass manager timings in dispc_ovl_setup() so 
that it comes directly from apply.c. What do you say?

I guess we can get away from exposing private data in apply to DSS, but 
it might get a bit harder when we try to remove other omap_dss_device 
references in the future. I'm just guessing though.

Archit

>
>   Tomi
>


^ permalink raw reply

* Re: [PATCH 00/25] OMAPDSS: DT preparation patches v2
From: Tomi Valkeinen @ 2012-05-08  8:44 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <20120507174657.GF5088@atomide.com>

[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]

On Mon, 2012-05-07 at 10:46 -0700, Tony Lindgren wrote:
> Hi,
> 
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [120503 07:01]:
> > Hi,
> > 
> > I started cleaning up and restructuring omapdss for device tree, and here's the
> > first set of patches from that ordeal. There's nothing DT specific in these
> > patches, but they are mostly generic cleanups that make sense even without DT.
> > 
> > This is the second version of these patches, the previous version can be found
> > from: http://www.spinics.net/lists/linux-fbdev/msg05667.html
> > 
> > The first 21 patches, which were in the previous version, have only gotten
> > minor cleanups (and, of course, more testing). The last 4 patches are new. The
> > most important of those patches is the DSI pin config patch, which makes it
> > possible for the panel driver to configure the DSI pins it needs.
> > 
> > This series can also be found from:
> > git://gitorious.org/linux-omap-dss2/linux.git work/devtree-base
> 
> Nice clean up. Can you please put the first 12 arch/arm/*omap*/* touching
> patches (and the drivers/video dependencies needed) into a separate branch
> and send me a pull request. That is assuming those patches are now immutable.
> 
> Then I can pull it into cleanup-dss branch that we both can merge as
> needed.

Ok, I'll see how it goes. Do I have your ack for the board file changes?

Do you want to have patches that touch only
arch/arm/mach-omap2/display.c, but not the board files? That's a dss
specific file, and I don't expect anyone else to make changes to it, so
chances for conflicts should be quite minimal.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2 4/4] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Tomi Valkeinen @ 2012-05-08  8:52 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4FA8CD6D.3010503@ti.com>

[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]

On Tue, 2012-05-08 at 13:08 +0530, Archit Taneja wrote:
> On Tuesday 08 May 2012 12:46 PM, Tomi Valkeinen wrote:

> > I'm not sure if it applies here, but as a general strategy, I suggest
> > doing things in apply.c that require data from apply.c's internal data.
> > When that's not possible, apply.c should call the functions outside
> > apply.c, and pass the internal data as parameters (like calls to dispc).
> >
> > In your case, I for example see dss_mgr_check() calling
> > dss_mgr_get_timings(). It would be quite easy to pass the timings to
> > dss_mgr_check() from apply.c, thus removing the need to call the
> > function. And, as you see, dss_mgr_check() already has a bunch of
> > parameters, and the idea is the same with those: give the params, so
> > that dss_mgr_check() doesn't need to ask for them.
> 
> Right, I can do this for dss_mgr_check() and dss_ovl_check(), but if I 
> you see the misc cleanup patch series I posted, I use 
> dss_mgr_get_timings()  in dispc.c functions to remove some 
> omap_dss_device references. These DISPC functions are called in 
> dispc_ovl_setup(), I could pass manager timings in dispc_ovl_setup() so 
> that it comes directly from apply.c. What do you say?

Yes, I guess dispc_ovl_setup should get the timings pointer also. It's
getting a bit complex, but I don't see any simpler way. Especially dispc
functions should be quite self contained, dispc.c should not call
elsewhere, but it should get all the needed data as parameters.

> I guess we can get away from exposing private data in apply to DSS, but 
> it might get a bit harder when we try to remove other omap_dss_device 
> references in the future. I'm just guessing though.

We have two cases here: 1) using timings when doing apply.c things, as
your patches do. In these cases the timings can be passed from apply.c
as parameters. 2) using timings "outside" apply.c. I guess we currently
don't have these, but for those we could have a get_timings() function
that does take the locks. And similarly for other configs.

Of course, that's still not perfect. Even if get_timings() is protected
properly, there's no guarantee that the timings won't change right after
get_timings has returned. But hopefully all writes and reads to timings
would go via the same place, for example dpi.c. And dpi.c's own lock
will protect the change of timings while another thread is using them.

Btw, there's a typo in "OMAPDSS: DISPC: Remove usage of
dispc_mgr_get_device()"'s description, dss_mgr_get_device() should be
get_timings in one place.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2 4/4] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Archit Taneja @ 2012-05-08  9:19 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336467151.5761.20.camel@deskari>

On Tuesday 08 May 2012 02:22 PM, Tomi Valkeinen wrote:
> On Tue, 2012-05-08 at 13:08 +0530, Archit Taneja wrote:
>> On Tuesday 08 May 2012 12:46 PM, Tomi Valkeinen wrote:
>
>>> I'm not sure if it applies here, but as a general strategy, I suggest
>>> doing things in apply.c that require data from apply.c's internal data.
>>> When that's not possible, apply.c should call the functions outside
>>> apply.c, and pass the internal data as parameters (like calls to dispc).
>>>
>>> In your case, I for example see dss_mgr_check() calling
>>> dss_mgr_get_timings(). It would be quite easy to pass the timings to
>>> dss_mgr_check() from apply.c, thus removing the need to call the
>>> function. And, as you see, dss_mgr_check() already has a bunch of
>>> parameters, and the idea is the same with those: give the params, so
>>> that dss_mgr_check() doesn't need to ask for them.
>>
>> Right, I can do this for dss_mgr_check() and dss_ovl_check(), but if I
>> you see the misc cleanup patch series I posted, I use
>> dss_mgr_get_timings()  in dispc.c functions to remove some
>> omap_dss_device references. These DISPC functions are called in
>> dispc_ovl_setup(), I could pass manager timings in dispc_ovl_setup() so
>> that it comes directly from apply.c. What do you say?
>
> Yes, I guess dispc_ovl_setup should get the timings pointer also. It's
> getting a bit complex, but I don't see any simpler way. Especially dispc
> functions should be quite self contained, dispc.c should not call
> elsewhere, but it should get all the needed data as parameters.

Ok, you are right about dispc.c querying stuff from apply, it should all 
be one way from apply/interface drivers to the dispc, and if that's the 
case, then it has to get more complex. We could just pass the data in a 
nicer way to make the number of arguments passed to dispc functions seem 
small.

>
>> I guess we can get away from exposing private data in apply to DSS, but
>> it might get a bit harder when we try to remove other omap_dss_device
>> references in the future. I'm just guessing though.
>
> We have two cases here: 1) using timings when doing apply.c things, as
> your patches do. In these cases the timings can be passed from apply.c
> as parameters. 2) using timings "outside" apply.c. I guess we currently
> don't have these, but for those we could have a get_timings() function
> that does take the locks. And similarly for other configs.
>
> Of course, that's still not perfect. Even if get_timings() is protected
> properly, there's no guarantee that the timings won't change right after
> get_timings has returned. But hopefully all writes and reads to timings
> would go via the same place, for example dpi.c. And dpi.c's own lock
> will protect the change of timings while another thread is using them.
>
> Btw, there's a typo in "OMAPDSS: DISPC: Remove usage of
> dispc_mgr_get_device()"'s description, dss_mgr_get_device() should be
> get_timings in one place.

Ah ok, I am removing the dss_mgr_get_timings() usage anyway, I'll pass 
timings through dispc_ovl_setup().

I am also going to make the first patch of the newer set 'OMAPDSS: 
DPI/HDMI: Apply manager timings even if panel is disabled' a part of 
this series since it's more related to applying timings. I posted it in 
the next series since I had already posted out this series.

Archit

>
>   Tomi
>


^ permalink raw reply

* [PATCH v3 0/5] OMAPDSS: APPLY: Treat overlay manager timings as shadow registers
From: Archit Taneja @ 2012-05-08 10:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1334561027-28569-1-git-send-email-archit@ti.com>

An overlay manager's timings (the manager size, and blanking parameters if an
LCD manager) are DISPC shadow registers, and they should hence follow the
correct programming model.

This set makes the timings an extra_info parameter in manager's private data .
The interface drivers now apply the timings in instead of directly writing to
registers.

This change also prevents the need to use display resolution for overlay
checks, hence making some of the APPLY functions less dependent on the display.

Changes since v3:

- Make sure that we write to the timing registers and set go bits when calling
  dss_mgr_set_timings
- Don't create dss_mgr_get_timings(), apply should pass the timings rather than
  outside DSS dunctions querying form it.

These patches apply over:

git://gitorious.org/linux-omap-dss2/linux.git dev

Archit Taneja (5):
  OMAPDSS: APPLY: Add manager timings as extra_info in private data
  OMAPDSS: Apply manager timings instead of direct DISPC writes
  OMAPDSS: MANAGER: Create a function to check manager timings
  OMAPDSS: APPLY: Remove display dependency from overlay and manager
    checks
  OMAPDSS: DPI/HDMI: Apply manager timings even if panel is disabled

 drivers/video/omap2/dss/apply.c   |  140 ++++++++++++++++++++++++++++++-------
 drivers/video/omap2/dss/dispc.c   |    2 +-
 drivers/video/omap2/dss/dpi.c     |    6 +-
 drivers/video/omap2/dss/dsi.c     |    5 +-
 drivers/video/omap2/dss/dss.h     |   13 +++-
 drivers/video/omap2/dss/hdmi.c    |    4 +-
 drivers/video/omap2/dss/manager.c |   19 +++++-
 drivers/video/omap2/dss/overlay.c |   20 +++---
 drivers/video/omap2/dss/rfbi.c    |    4 +-
 drivers/video/omap2/dss/sdi.c     |    2 +-
 drivers/video/omap2/dss/venc.c    |    2 +-
 11 files changed, 163 insertions(+), 54 deletions(-)

-- 
1.7.5.4


^ permalink raw reply

* [PATCH v3 1/5] OMAPDSS: APPLY: Add manager timings as extra_info in private data
From: Archit Taneja @ 2012-05-08 10:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1336471096-21096-1-git-send-email-archit@ti.com>

DISPC manager size and DISPC manager blanking parameters(for LCD managers)
follow the shadow register programming model. Currently, they are programmed
directly by the interface drivers.

To configure manager timings using APPLY, there is a need to introduce extra
info flags for managers, similar to what is done for overlays. This is needed
because timings aren't a part of overlay_manager_info struct configured by a
user of DSS, they are configured internally by the interface or panel drivers.

Add dirty and shadow_dirty extra_info flags for managers, update these flags
at the appropriate places. Rewrite the function extra_info_update_ongoing()
slightly as checking for manager's extra_info flags can simplify the code a bit.

Create function dss_mgr_set_timings() which applies the new manager timings to
extra_info and writes it to the registers and set the go bits.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c |   96 +++++++++++++++++++++++++++++++++-----
 drivers/video/omap2/dss/dss.h   |    2 +
 2 files changed, 85 insertions(+), 13 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index b10b3bc..57686f6 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -99,6 +99,11 @@ struct mgr_priv_data {
 
 	/* If true, a display is enabled using this manager */
 	bool enabled;
+
+	bool extra_info_dirty;
+	bool shadow_extra_info_dirty;
+
+	struct omap_video_timings timings;
 };
 
 static struct {
@@ -261,6 +266,20 @@ static bool need_isr(void)
 			if (mp->shadow_info_dirty)
 				return true;
 
+			/*
+			 * NOTE: we don't check extra_info flags for disabled
+			 * managers, once the manager is enabled, the extra_info
+			 * related manager changes will be taken in by HW.
+			 */
+
+			/* to write new values to registers */
+			if (mp->extra_info_dirty)
+				return true;
+
+			/* to set GO bit */
+			if (mp->shadow_extra_info_dirty)
+				return true;
+
 			list_for_each_entry(ovl, &mgr->overlays, list) {
 				struct ovl_priv_data *op;
 
@@ -305,7 +324,7 @@ static bool need_go(struct omap_overlay_manager *mgr)
 
 	mp = get_mgr_priv(mgr);
 
-	if (mp->shadow_info_dirty)
+	if (mp->shadow_info_dirty || mp->shadow_extra_info_dirty)
 		return true;
 
 	list_for_each_entry(ovl, &mgr->overlays, list) {
@@ -320,20 +339,16 @@ static bool need_go(struct omap_overlay_manager *mgr)
 /* returns true if an extra_info field is currently being updated */
 static bool extra_info_update_ongoing(void)
 {
-	const int num_ovls = omap_dss_get_num_overlays();
-	struct ovl_priv_data *op;
-	struct omap_overlay *ovl;
-	struct mgr_priv_data *mp;
+	const int num_mgrs = dss_feat_get_num_mgrs();
 	int i;
 
-	for (i = 0; i < num_ovls; ++i) {
-		ovl = omap_dss_get_overlay(i);
-		op = get_ovl_priv(ovl);
-
-		if (!ovl->manager)
-			continue;
+	for (i = 0; i < num_mgrs; ++i) {
+		struct omap_overlay_manager *mgr;
+		struct omap_overlay *ovl;
+		struct mgr_priv_data *mp;
 
-		mp = get_mgr_priv(ovl->manager);
+		mgr = omap_dss_get_overlay_manager(i);
+		mp = get_mgr_priv(mgr);
 
 		if (!mp->enabled)
 			continue;
@@ -341,8 +356,15 @@ static bool extra_info_update_ongoing(void)
 		if (!mp->updating)
 			continue;
 
-		if (op->extra_info_dirty || op->shadow_extra_info_dirty)
+		if (mp->extra_info_dirty || mp->shadow_extra_info_dirty)
 			return true;
+
+		list_for_each_entry(ovl, &mgr->overlays, list) {
+			struct ovl_priv_data *op = get_ovl_priv(ovl);
+
+			if (op->extra_info_dirty || op->shadow_extra_info_dirty)
+				return true;
+		}
 	}
 
 	return false;
@@ -601,6 +623,22 @@ static void dss_mgr_write_regs(struct omap_overlay_manager *mgr)
 	}
 }
 
+static void dss_mgr_write_regs_extra(struct omap_overlay_manager *mgr)
+{
+	struct mgr_priv_data *mp = get_mgr_priv(mgr);
+
+	DSSDBGF("%d", mgr->id);
+
+	if (!mp->extra_info_dirty)
+		return;
+
+	dispc_mgr_set_timings(mgr->id, &mp->timings);
+
+	mp->extra_info_dirty = false;
+	if (mp->updating)
+		mp->shadow_extra_info_dirty = true;
+}
+
 static void dss_write_regs_common(void)
 {
 	const int num_mgrs = omap_dss_get_num_overlay_managers();
@@ -654,6 +692,7 @@ static void dss_write_regs(void)
 		}
 
 		dss_mgr_write_regs(mgr);
+		dss_mgr_write_regs_extra(mgr);
 	}
 }
 
@@ -693,6 +732,7 @@ static void mgr_clear_shadow_dirty(struct omap_overlay_manager *mgr)
 
 	mp = get_mgr_priv(mgr);
 	mp->shadow_info_dirty = false;
+	mp->shadow_extra_info_dirty = false;
 
 	list_for_each_entry(ovl, &mgr->overlays, list) {
 		op = get_ovl_priv(ovl);
@@ -719,6 +759,7 @@ void dss_mgr_start_update(struct omap_overlay_manager *mgr)
 	}
 
 	dss_mgr_write_regs(mgr);
+	dss_mgr_write_regs_extra(mgr);
 
 	dss_write_regs_common();
 
@@ -1225,6 +1266,35 @@ err:
 	return r;
 }
 
+static void dss_apply_mgr_timings(struct omap_overlay_manager *mgr,
+		struct omap_video_timings *timings)
+{
+	struct mgr_priv_data *mp = get_mgr_priv(mgr);
+
+	mp->timings = *timings;
+	mp->extra_info_dirty = true;
+}
+
+void dss_mgr_set_timings(struct omap_overlay_manager *mgr,
+		struct omap_video_timings *timings)
+{
+	unsigned long flags;
+
+	mutex_lock(&apply_lock);
+
+	spin_lock_irqsave(&data_lock, flags);
+
+	dss_apply_mgr_timings(mgr, timings);
+
+	dss_write_regs();
+	dss_set_go_bits();
+
+	spin_unlock_irqrestore(&data_lock, flags);
+
+	wait_pending_extra_info_updates();
+
+	mutex_unlock(&apply_lock);
+}
 
 int dss_ovl_set_info(struct omap_overlay *ovl,
 		struct omap_overlay_info *info)
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 5ca67f1..ca59481 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -177,6 +177,8 @@ void dss_mgr_get_info(struct omap_overlay_manager *mgr,
 int dss_mgr_set_device(struct omap_overlay_manager *mgr,
 		struct omap_dss_device *dssdev);
 int dss_mgr_unset_device(struct omap_overlay_manager *mgr);
+void dss_mgr_set_timings(struct omap_overlay_manager *mgr,
+		struct omap_video_timings *timings);
 
 bool dss_ovl_is_enabled(struct omap_overlay *ovl);
 int dss_ovl_enable(struct omap_overlay *ovl);
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH v3 2/5] OMAPDSS: Apply manager timings instead of direct DISPC writes
From: Archit Taneja @ 2012-05-08 10:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1336471096-21096-1-git-send-email-archit@ti.com>

Replace the function dispc_mgr_set_timings() with dss_mgr_set_timings() in the
interface drivers. The latter function ensures that the timing related DISPC
registers are configured according to the shadow register programming model.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dpi.c  |    2 +-
 drivers/video/omap2/dss/dsi.c  |    5 ++---
 drivers/video/omap2/dss/hdmi.c |    2 +-
 drivers/video/omap2/dss/rfbi.c |    4 ++--
 drivers/video/omap2/dss/sdi.c  |    2 +-
 drivers/video/omap2/dss/venc.c |    2 +-
 6 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index cec1166..5d84ab0 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -156,7 +156,7 @@ static int dpi_set_mode(struct omap_dss_device *dssdev)
 		t->pixel_clock = pck;
 	}
 
-	dispc_mgr_set_timings(dssdev->manager->id, t);
+	dss_mgr_set_timings(dssdev->manager, t);
 
 	return 0;
 }
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index b6cf03c..db73598 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -4219,13 +4219,12 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 		dispc_mgr_enable_stallmode(dssdev->manager->id, true);
 		dispc_mgr_enable_fifohandcheck(dssdev->manager->id, 1);
 
-		dispc_mgr_set_timings(dssdev->manager->id, &timings);
+		dss_mgr_set_timings(dssdev->manager, &timings);
 	} else {
 		dispc_mgr_enable_stallmode(dssdev->manager->id, false);
 		dispc_mgr_enable_fifohandcheck(dssdev->manager->id, 0);
 
-		dispc_mgr_set_timings(dssdev->manager->id,
-			&dssdev->panel.timings);
+		dss_mgr_set_timings(dssdev->manager, &dssdev->panel.timings);
 	}
 
 		dispc_mgr_set_lcd_display_type(dssdev->manager->id,
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 56f6e9c..8d4ff8c 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -376,7 +376,7 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)
 	dispc_enable_gamma_table(0);
 
 	/* tv size */
-	dispc_mgr_set_timings(dssdev->manager->id, &dssdev->panel.timings);
+	dss_mgr_set_timings(dssdev->manager, &dssdev->panel.timings);
 
 	hdmi.ip_data.ops->video_enable(&hdmi.ip_data, 1);
 
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index a81ffcb..feadfab 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -320,7 +320,7 @@ static void rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
 
 	DSSDBG("rfbi_transfer_area %dx%d\n", width, height);
 
-	dispc_mgr_set_timings(dssdev->manager->id, &timings);
+	dss_mgr_set_timings(dssdev->manager, &timings);
 
 	dispc_mgr_enable(dssdev->manager->id, true);
 
@@ -804,7 +804,7 @@ int omap_rfbi_prepare_update(struct omap_dss_device *dssdev,
 	if (*w = 0 || *h = 0)
 		return -EINVAL;
 
-	dispc_mgr_set_timings(dssdev->manager->id, &timings);
+	dss_mgr_set_timings(dssdev->manager, &timings);
 
 	return 0;
 }
diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
index 741b834..67fbe7c 100644
--- a/drivers/video/omap2/dss/sdi.c
+++ b/drivers/video/omap2/dss/sdi.c
@@ -107,7 +107,7 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
 	}
 
 
-	dispc_mgr_set_timings(dssdev->manager->id, t);
+	dss_mgr_set_timings(dssdev->manager, t);
 
 	r = dss_set_clock_div(&dss_cinfo);
 	if (r)
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 30bbb63..e237464 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -444,7 +444,7 @@ static int venc_power_on(struct omap_dss_device *dssdev)
 	timings = dssdev->panel.timings;
 	timings.y_res /= 2;
 
-	dispc_mgr_set_timings(dssdev->manager->id, &timings);
+	dss_mgr_set_timings(dssdev->manager, &timings);
 
 	r = regulator_enable(venc.vdda_dac_reg);
 	if (r)
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH v3 3/5] OMAPDSS: MANAGER: Create a function to check manager timings
From: Archit Taneja @ 2012-05-08 10:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1336471096-21096-1-git-send-email-archit@ti.com>

Create a function dss_mgr_check_timings() which wraps around the function
dispc_mgr_timings_ok(). This is mainly a clean up to hide dispc functions
from interface drivers.

dss_mgr_check_timings() is added in the function dss_mgr_check(), it currently
takes the timings maintained in the omap_dss_device struct. This would be later
replaced by the timings stored in the manager's private data.

Make dss_mgr_check_timings() and dispc_mgr_timings_ok() take a const
omap_video_timings pointer since these functions just check the timings.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c   |    2 +-
 drivers/video/omap2/dss/dpi.c     |    2 +-
 drivers/video/omap2/dss/dss.h     |    4 +++-
 drivers/video/omap2/dss/manager.c |   15 +++++++++++++++
 4 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 49015b8..2829a92 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2479,7 +2479,7 @@ static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
 }
 
 bool dispc_mgr_timings_ok(enum omap_channel channel,
-		struct omap_video_timings *timings)
+		const struct omap_video_timings *timings)
 {
 	bool timings_ok;
 
diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index 5d84ab0..8127f46 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -312,7 +312,7 @@ int dpi_check_timings(struct omap_dss_device *dssdev,
 	unsigned long pck;
 	struct dispc_clock_info dispc_cinfo;
 
-	if (!dispc_mgr_timings_ok(dssdev->manager->id, timings))
+	if (!dss_mgr_check_timings(dssdev->manager, timings))
 		return -EINVAL;
 
 	if (timings->pixel_clock = 0)
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index ca59481..7c9b9bc 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -208,6 +208,8 @@ int dss_init_overlay_managers(struct platform_device *pdev);
 void dss_uninit_overlay_managers(struct platform_device *pdev);
 int dss_mgr_simple_check(struct omap_overlay_manager *mgr,
 		const struct omap_overlay_manager_info *info);
+int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
+		const struct omap_video_timings *timings);
 int dss_mgr_check(struct omap_overlay_manager *mgr,
 		struct omap_dss_device *dssdev,
 		struct omap_overlay_manager_info *info,
@@ -414,7 +416,7 @@ void dispc_enable_gamma_table(bool enable);
 void dispc_set_loadmode(enum omap_dss_load_mode mode);
 
 bool dispc_mgr_timings_ok(enum omap_channel channel,
-		struct omap_video_timings *timings);
+		const struct omap_video_timings *timings);
 unsigned long dispc_fclk_rate(void);
 void dispc_find_clk_divs(bool is_tft, unsigned long req_pck, unsigned long fck,
 		struct dispc_clock_info *cinfo);
diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index e736460..566fbba 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -654,6 +654,17 @@ static int dss_mgr_check_zorder(struct omap_overlay_manager *mgr,
 	return 0;
 }
 
+int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
+		const struct omap_video_timings *timings)
+{
+	if (!dispc_mgr_timings_ok(mgr->id, timings)) {
+		DSSERR("check_manager: invalid timings\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int dss_mgr_check(struct omap_overlay_manager *mgr,
 		struct omap_dss_device *dssdev,
 		struct omap_overlay_manager_info *info,
@@ -668,6 +679,10 @@ int dss_mgr_check(struct omap_overlay_manager *mgr,
 			return r;
 	}
 
+	r = dss_mgr_check_timings(mgr, &dssdev->panel.timings);
+	if (r)
+		return r;
+
 	list_for_each_entry(ovl, &mgr->overlays, list) {
 		struct omap_overlay_info *oi;
 		int r;
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH v3 4/5] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Archit Taneja @ 2012-05-08 10:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1336471096-21096-1-git-send-email-archit@ti.com>

In order to check the validity of overlay and manager info, there was a need to
use the omap_dss_device struct to get the panel resolution. The manager's
private data in APPLY now contains the manager timings. Hence, we don't need to
rely on the display resolution any more.

Pass the manager's timings(in private data) to dss_mgr_check(). Remove the need
of passing omap_dss_device structs in the functions which check for overlay and
managers.

Have some initial values for manager timings in apply_init(), these would ensure
that manager checks don't fail if an interface driver or a panel driver hasn't
set the manager timings yet.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c   |   44 ++++++++++++++++++++++++++-----------
 drivers/video/omap2/dss/dss.h     |    7 +++--
 drivers/video/omap2/dss/manager.c |    6 ++--
 drivers/video/omap2/dss/overlay.c |   20 +++++++---------
 4 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 57686f6..7d59caf 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -137,10 +137,30 @@ static struct mgr_priv_data *get_mgr_priv(struct omap_overlay_manager *mgr)
 void dss_apply_init(void)
 {
 	const int num_ovls = dss_feat_get_num_ovls();
+	const int num_mgrs = dss_feat_get_num_ovls();
 	int i;
+	/* Use dummy manager timings during initialization */
+	struct omap_video_timings timings = {
+		.hsw		= 1,
+		.hfp		= 1,
+		.hbp		= 1,
+		.vsw		= 1,
+		.vfp		= 0,
+		.vbp		= 0,
+		.x_res	= dss_feat_get_param_max(FEAT_PARAM_MGR_WIDTH),
+		.y_res	= dss_feat_get_param_max(FEAT_PARAM_MGR_HEIGHT),
+	};
 
 	spin_lock_init(&data_lock);
 
+	for (i = 0; i < num_mgrs; i++) {
+		struct mgr_priv_data *mp;
+
+		mp = &dss_data.mgr_priv_data_array[i];
+
+		mp->timings = timings;
+	}
+
 	for (i = 0; i < num_ovls; ++i) {
 		struct ovl_priv_data *op;
 
@@ -181,7 +201,7 @@ static bool mgr_manual_update(struct omap_overlay_manager *mgr)
 }
 
 static int dss_check_settings_low(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev, bool applying)
+		bool applying)
 {
 	struct omap_overlay_info *oi;
 	struct omap_overlay_manager_info *mi;
@@ -211,26 +231,24 @@ static int dss_check_settings_low(struct omap_overlay_manager *mgr,
 		ois[ovl->id] = oi;
 	}
 
-	return dss_mgr_check(mgr, dssdev, mi, ois);
+	return dss_mgr_check(mgr, mi, &mp->timings, ois);
 }
 
 /*
  * check manager and overlay settings using overlay_info from data->info
  */
-static int dss_check_settings(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev)
+static int dss_check_settings(struct omap_overlay_manager *mgr)
 {
-	return dss_check_settings_low(mgr, dssdev, false);
+	return dss_check_settings_low(mgr, false);
 }
 
 /*
  * check manager and overlay settings using overlay_info from ovl->info if
  * dirty and from data->info otherwise
  */
-static int dss_check_settings_apply(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev)
+static int dss_check_settings_apply(struct omap_overlay_manager *mgr)
 {
-	return dss_check_settings_low(mgr, dssdev, true);
+	return dss_check_settings_low(mgr, true);
 }
 
 static bool need_isr(void)
@@ -684,7 +702,7 @@ static void dss_write_regs(void)
 		if (!mp->enabled || mgr_manual_update(mgr) || mp->busy)
 			continue;
 
-		r = dss_check_settings(mgr, mgr->device);
+		r = dss_check_settings(mgr);
 		if (r) {
 			DSSERR("cannot write registers for manager %s: "
 					"illegal configuration\n", mgr->name);
@@ -751,7 +769,7 @@ void dss_mgr_start_update(struct omap_overlay_manager *mgr)
 
 	WARN_ON(mp->updating);
 
-	r = dss_check_settings(mgr, mgr->device);
+	r = dss_check_settings(mgr);
 	if (r) {
 		DSSERR("cannot start manual update: illegal configuration\n");
 		spin_unlock_irqrestore(&data_lock, flags);
@@ -898,7 +916,7 @@ int omap_dss_mgr_apply(struct omap_overlay_manager *mgr)
 
 	spin_lock_irqsave(&data_lock, flags);
 
-	r = dss_check_settings_apply(mgr, mgr->device);
+	r = dss_check_settings_apply(mgr);
 	if (r) {
 		spin_unlock_irqrestore(&data_lock, flags);
 		DSSERR("failed to apply settings: illegal configuration.\n");
@@ -1091,7 +1109,7 @@ int dss_mgr_enable(struct omap_overlay_manager *mgr)
 
 	mp->enabled = true;
 
-	r = dss_check_settings(mgr, mgr->device);
+	r = dss_check_settings(mgr);
 	if (r) {
 		DSSERR("failed to enable manager %d: check_settings failed\n",
 				mgr->id);
@@ -1463,7 +1481,7 @@ int dss_ovl_enable(struct omap_overlay *ovl)
 
 	op->enabling = true;
 
-	r = dss_check_settings(ovl->manager, ovl->manager->device);
+	r = dss_check_settings(ovl->manager);
 	if (r) {
 		DSSERR("failed to enable overlay %d: check_settings failed\n",
 				ovl->id);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 7c9b9bc..eff3d7f 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -179,6 +179,7 @@ int dss_mgr_set_device(struct omap_overlay_manager *mgr,
 int dss_mgr_unset_device(struct omap_overlay_manager *mgr);
 void dss_mgr_set_timings(struct omap_overlay_manager *mgr,
 		struct omap_video_timings *timings);
+const struct omap_video_timings *dss_mgr_get_timings(struct omap_overlay_manager *mgr);
 
 bool dss_ovl_is_enabled(struct omap_overlay *ovl);
 int dss_ovl_enable(struct omap_overlay *ovl);
@@ -211,8 +212,8 @@ int dss_mgr_simple_check(struct omap_overlay_manager *mgr,
 int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
 		const struct omap_video_timings *timings);
 int dss_mgr_check(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev,
 		struct omap_overlay_manager_info *info,
+		const struct omap_video_timings *mgr_timings,
 		struct omap_overlay_info **overlay_infos);
 
 /* overlay */
@@ -222,8 +223,8 @@ void dss_overlay_setup_dispc_manager(struct omap_overlay_manager *mgr);
 void dss_recheck_connections(struct omap_dss_device *dssdev, bool force);
 int dss_ovl_simple_check(struct omap_overlay *ovl,
 		const struct omap_overlay_info *info);
-int dss_ovl_check(struct omap_overlay *ovl,
-		struct omap_overlay_info *info, struct omap_dss_device *dssdev);
+int dss_ovl_check(struct omap_overlay *ovl, struct omap_overlay_info *info,
+		const struct omap_video_timings *mgr_timings);
 
 /* DSS */
 int dss_init_platform_driver(void);
diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index 566fbba..0cbcde4 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -666,8 +666,8 @@ int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
 }
 
 int dss_mgr_check(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev,
 		struct omap_overlay_manager_info *info,
+		const struct omap_video_timings *mgr_timings,
 		struct omap_overlay_info **overlay_infos)
 {
 	struct omap_overlay *ovl;
@@ -679,7 +679,7 @@ int dss_mgr_check(struct omap_overlay_manager *mgr,
 			return r;
 	}
 
-	r = dss_mgr_check_timings(mgr, &dssdev->panel.timings);
+	r = dss_mgr_check_timings(mgr, mgr_timings);
 	if (r)
 		return r;
 
@@ -692,7 +692,7 @@ int dss_mgr_check(struct omap_overlay_manager *mgr,
 		if (oi = NULL)
 			continue;
 
-		r = dss_ovl_check(ovl, oi, dssdev);
+		r = dss_ovl_check(ovl, oi, mgr_timings);
 		if (r)
 			return r;
 	}
diff --git a/drivers/video/omap2/dss/overlay.c b/drivers/video/omap2/dss/overlay.c
index 6e82181..74db90c 100644
--- a/drivers/video/omap2/dss/overlay.c
+++ b/drivers/video/omap2/dss/overlay.c
@@ -631,16 +631,14 @@ int dss_ovl_simple_check(struct omap_overlay *ovl,
 	return 0;
 }
 
-int dss_ovl_check(struct omap_overlay *ovl,
-		struct omap_overlay_info *info, struct omap_dss_device *dssdev)
+int dss_ovl_check(struct omap_overlay *ovl, struct omap_overlay_info *info,
+		const struct omap_video_timings *mgr_timings)
 {
 	u16 outw, outh;
-	u16 dw, dh;
+	u16 mgr_width, mgr_height;
 
-	if (dssdev = NULL)
-		return 0;
-
-	dssdev->driver->get_resolution(dssdev, &dw, &dh);
+	mgr_width = mgr_timings->x_res;
+	mgr_height = mgr_timings->y_res;
 
 	if ((ovl->caps & OMAP_DSS_OVL_CAP_SCALE) = 0) {
 		outw = info->width;
@@ -657,17 +655,17 @@ int dss_ovl_check(struct omap_overlay *ovl,
 			outh = info->out_height;
 	}
 
-	if (dw < info->pos_x + outw) {
+	if (mgr_width < info->pos_x + outw) {
 		DSSERR("overlay %d horizontally not inside the display area "
 				"(%d + %d >= %d)\n",
-				ovl->id, info->pos_x, outw, dw);
+				ovl->id, info->pos_x, outw, mgr_width);
 		return -EINVAL;
 	}
 
-	if (dh < info->pos_y + outh) {
+	if (mgr_height < info->pos_y + outh) {
 		DSSERR("overlay %d vertically not inside the display area "
 				"(%d + %d >= %d)\n",
-				ovl->id, info->pos_y, outh, dh);
+				ovl->id, info->pos_y, outh, mgr_height);
 		return -EINVAL;
 	}
 
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH v3 5/5] OMAPDSS: DPI/HDMI: Apply manager timings even if panel is disabled
From: Archit Taneja @ 2012-05-08 10:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1336471096-21096-1-git-send-email-archit@ti.com>

The DPI and HDMI interfaces use their 'set_timing' functions to take in a new
set of timings. If the panel is disabled, they do not disable and re-enable
the interface. Currently, the manager timings are applied in hdmi_power_on()
and dpi_set_mode() respectively, these are not called by set_timings if the
panel is disabled.

When checking overlay and manager data, the DSS driver uses the last applied
manager timings, and not the timings held by omap_dss_device struct. Hence,
there is a need to apply the new manager timings even if the panel is disabled.

Apply the manager timings if the panel is disabled. Eventually, there should be
one common place where the timings are applied independent of the state of the
panel.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dpi.c  |    2 ++
 drivers/video/omap2/dss/hdmi.c |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index 8127f46..1650050 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -298,6 +298,8 @@ void dpi_set_timings(struct omap_dss_device *dssdev,
 
 		dispc_runtime_put();
 		dss_runtime_put();
+	} else {
+		dss_mgr_set_timings(dssdev->manager, timings);
 	}
 }
 EXPORT_SYMBOL(dpi_set_timings);
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 8d4ff8c..32ad712 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -435,6 +435,8 @@ void omapdss_hdmi_display_set_timing(struct omap_dss_device *dssdev)
 		r = hdmi_power_on(dssdev);
 		if (r)
 			DSSERR("failed to power on device\n");
+	} else {
+		dss_mgr_set_timings(dssdev->manager, &dssdev->panel.timings);
 	}
 }
 
-- 
1.7.5.4


^ permalink raw reply related

* Re: ASoC: wm9712: Microphone doesn't work, "Capture Volume" inverted
From: Christoph Fritz @ 2012-05-08 10:29 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, Hans J. Koch, Sascha Hauer, Liam Girdwood,
	linux-fbdev
In-Reply-To: <20120507215506.GA5063@lovely.krouter>

On Mon, May 07, 2012 at 11:55:06PM +0200, Christoph Fritz wrote:
> On Fri, May 04, 2012 at 03:28:45PM +0200, Christoph Fritz wrote:
> > On Fri, Apr 27, 2012 at 02:46:39PM +0100, Mark Brown wrote:
> > > On Fri, Apr 27, 2012 at 10:00:02AM +0200, Christoph Fritz wrote:
> > > > On Thu, 2012-04-26 at 22:37 +0100, Mark Brown wrote:
> > > 
> > > > >   The write will be suppresed if the register contents don't
> > > > > change which looks like what you're seeing here - 
> > > 
> > > > Can you imagine why the registers don't change?
> > > 
> > > I can't immediately think of any reason, no - I'd step through or
> > > annotate the code and have a look.
> > 
> > I'm still on.
> > 
> > And while testing WM9712 its touchpad interface, connecting a 800x600
> > display (instead of the default 640x480 one) results in a gone sound and
> > input-device - pretty queer:
> > 
> > WM9711/WM9712 SoC Audio Codec 0.4
> > asoc: platform pcm constructor failed
> > asoc: can't create pcm HiFi
> > asoc: failed to instantiate card PhyCORE-ac97-audio: -12
> > 
> > I have to admit that I used this time a 3.2 kernel. I'll test with
> > current later these days.
> 
> Same behaviour with 3.4.0-rc6:
> 
> Framebuffer driver mx3fb configured for a 800x600 display:
> 
>   soc-audio soc-audio: ASoC machine PhyCORE-ac97-audio should use snd_soc_register_card()
>   asoc: platform pcm constructor failed
>   asoc: can't create pcm HiFi :-12
>   asoc: failed to instantiate card PhyCORE-ac97-audio: -12

When I do decrease from 800x600 to 800x594, wm9712 works. 

Any ideas?

> 
> 
> mx3fb configured for a 640x480 display:
>   soc-audio soc-audio: ASoC machine PhyCORE-ac97-audio should use
>   snd_soc_register_card()
>   asoc: wm9712-hifi <-> imx-ssi.0 mapping ok
> 
>  
> Thanks,
>  -- Christoph

^ permalink raw reply

* Re: [PATCH v3 4/5] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Tomi Valkeinen @ 2012-05-08 10:50 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336471096-21096-5-git-send-email-archit@ti.com>

[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]

On Tue, 2012-05-08 at 15:28 +0530, Archit Taneja wrote:
> In order to check the validity of overlay and manager info, there was a need to
> use the omap_dss_device struct to get the panel resolution. The manager's
> private data in APPLY now contains the manager timings. Hence, we don't need to
> rely on the display resolution any more.
> 
> Pass the manager's timings(in private data) to dss_mgr_check(). Remove the need
> of passing omap_dss_device structs in the functions which check for overlay and
> managers.
> 
> Have some initial values for manager timings in apply_init(), these would ensure
> that manager checks don't fail if an interface driver or a panel driver hasn't
> set the manager timings yet.

In what code patch were the initial values needed?

Checking the validity of all the settings is a bit tricky, but currently
I think, as a rule of thumb, we should accept any settings when things
are disabled. So, until the interface driver sets the timings before
enabling the output, all ovl/mgr settings should be allowed. And we
shouldn't even write any shadow registers until the moment the output is
enabled.

Except settings that do not depend on anything, like, if max ovl width
is 2048, it's fine to reject 2049+ width anytime.

> -int dss_ovl_check(struct omap_overlay *ovl,
> -		struct omap_overlay_info *info, struct omap_dss_device *dssdev)
> +int dss_ovl_check(struct omap_overlay *ovl, struct omap_overlay_info *info,
> +		const struct omap_video_timings *mgr_timings)
>  {
>  	u16 outw, outh;
> -	u16 dw, dh;
> +	u16 mgr_width, mgr_height;
>  
> -	if (dssdev == NULL)
> -		return 0;
> -
> -	dssdev->driver->get_resolution(dssdev, &dw, &dh);
> +	mgr_width = mgr_timings->x_res;
> +	mgr_height = mgr_timings->y_res;

I think you could've just used the existing dw and dh variables, thus
avoiding the need to change the rest of the function. The 'd' refers to
display, which is more or less the same as mgr width.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v3 2/5] OMAPDSS: Apply manager timings instead of direct DISPC writes
From: Tomi Valkeinen @ 2012-05-08 10:59 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336471096-21096-3-git-send-email-archit@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1768 bytes --]

On Tue, 2012-05-08 at 15:28 +0530, Archit Taneja wrote:
> Replace the function dispc_mgr_set_timings() with dss_mgr_set_timings() in the
> interface drivers. The latter function ensures that the timing related DISPC
> registers are configured according to the shadow register programming model.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/video/omap2/dss/dpi.c  |    2 +-
>  drivers/video/omap2/dss/dsi.c  |    5 ++---
>  drivers/video/omap2/dss/hdmi.c |    2 +-
>  drivers/video/omap2/dss/rfbi.c |    4 ++--
>  drivers/video/omap2/dss/sdi.c  |    2 +-
>  drivers/video/omap2/dss/venc.c |    2 +-
>  6 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
> index cec1166..5d84ab0 100644
> --- a/drivers/video/omap2/dss/dpi.c
> +++ b/drivers/video/omap2/dss/dpi.c
> @@ -156,7 +156,7 @@ static int dpi_set_mode(struct omap_dss_device *dssdev)
>  		t->pixel_clock = pck;
>  	}
>  
> -	dispc_mgr_set_timings(dssdev->manager->id, t);
> +	dss_mgr_set_timings(dssdev->manager, t);
>  
>  	return 0;
>  }

I think you can now remove the dispc_mgr_go() from dpi.c in this patch.

And something else, which doesn't need to be fixed now, but just to
point out: dpi_set_timings() currently uses runtime_get to enable the
HW. If everything was in proper shape, this wouldn't be needed.
dpi_set_timings() would call apply.c's functions, and if the output is
disabled, the settings would just be stored in ram. So there wouldn't be
any need to enable the HW with runtime_get().

Then again, if we do change clock settings or non-shadow registers in
dpi_set_timings(), then that's not possible (at least via apply.c's
caching system).

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
From: Jean Delvare @ 2012-05-08 11:31 UTC (permalink / raw)
  To: Mark Brown
  Cc: Florian Tobias Schandinat, Dmitry Torokhov, Wolfram Sang,
	linux-i2c, linux-input, linux-fbdev
In-Reply-To: <1336127208-17951-1-git-send-email-broonie@opensource.wolfsonmicro.com>

On Fri,  4 May 2012 11:26:48 +0100, Mark Brown wrote:
> Since there are uses for I2C_M_NOSTART which are much more sensible and
> standard than most of the protocol mangling functionality (the main one
> being gather writes to devices where something like a register address
> needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> for this feature and update all the users to use it.
> 
> Also strengthen the disrecommendation of the protocol mangling while we're
> at it.
> 
> In the case of regmap-i2c we remove the requirement for mangling as
> I2C_M_NOSTART is the only mangling feature which is being used.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  Documentation/i2c/functionality       |    8 +++++---
>  Documentation/i2c/i2c-protocol        |    9 ++++++++-
>  drivers/base/regmap/regmap-i2c.c      |    2 +-
>  drivers/i2c/algos/i2c-algo-bit.c      |    2 +-
>  drivers/i2c/busses/i2c-nuc900.c       |    3 ++-
>  drivers/i2c/busses/i2c-s3c2410.c      |    3 ++-
>  drivers/input/joystick/as5011.c       |    1 +
>  drivers/video/matrox/matroxfb_maven.c |    1 +
>  include/linux/i2c.h                   |    5 +++--
>  9 files changed, 24 insertions(+), 10 deletions(-)
> (...)

Applied with minor cleanups, thanks.

-- 
Jean Delvare

^ permalink raw reply

* Re: [PATCH v3 4/5] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Archit Taneja @ 2012-05-08 11:34 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336474223.1821.6.camel@lappyti>

On Tuesday 08 May 2012 04:20 PM, Tomi Valkeinen wrote:
> On Tue, 2012-05-08 at 15:28 +0530, Archit Taneja wrote:
>> In order to check the validity of overlay and manager info, there was a need to
>> use the omap_dss_device struct to get the panel resolution. The manager's
>> private data in APPLY now contains the manager timings. Hence, we don't need to
>> rely on the display resolution any more.
>>
>> Pass the manager's timings(in private data) to dss_mgr_check(). Remove the need
>> of passing omap_dss_device structs in the functions which check for overlay and
>> managers.
>>
>> Have some initial values for manager timings in apply_init(), these would ensure
>> that manager checks don't fail if an interface driver or a panel driver hasn't
>> set the manager timings yet.
>
> In what code patch were the initial values needed?

I guess you meant code path. If a panel driver doesn't have set_timings 
op, then we get into trouble with omapfb. The following steps happen:

- try to set timings of display
- create buffers, configure overlays according to omap_dss_device
- call mgr->apply()
- enable the panel

For panels which don't have set_timings populated, if apply is called, 
the manager timings are still zero, and the mgr->apply fails when 
dss_mgr_check() is called.

When the panel driver's enable is called, the timings are configured in 
the interface drivers 'enable'. The enables(like 
dpi_display_enable,dsi_display_enable) of all interface driver's call 
dss_mgr_set_timings(), so we are sure that the timings are configured by 
then. But there is no such guarantee at set timings.

Things used to work previously because we used to simply get the 
connected panel and use its dimensions, if there was no panel connected, 
we used to skip the check.

>
> Checking the validity of all the settings is a bit tricky, but currently
> I think, as a rule of thumb, we should accept any settings when things
> are disabled. So, until the interface driver sets the timings before
> enabling the output, all ovl/mgr settings should be allowed. And we

We have 2 ways to go about this, one is to have an initial set of 
'always valid' values like I have done, the other option is to ignore 
manager timing related checks if the manager is disabled, i.e all 
configs are okay. To implement the second option, I think our function 
dss_check_settings_low() would get more complicated. We would now have 
to pass mp->enabled outside of apply, and pass it to dss_mgr_check() 
which would further need to pass this to dss_ovl_check(). Hence I used 
the first approach.

> shouldn't even write any shadow registers until the moment the output is
> enabled.

That's being done correctly even now I guess, with the checks for 
mp->enabled in wrtie_regs() and set_go_bits().

>
> Except settings that do not depend on anything, like, if max ovl width
> is 2048, it's fine to reject 2049+ width anytime.
>
>> -int dss_ovl_check(struct omap_overlay *ovl,
>> -		struct omap_overlay_info *info, struct omap_dss_device *dssdev)
>> +int dss_ovl_check(struct omap_overlay *ovl, struct omap_overlay_info *info,
>> +		const struct omap_video_timings *mgr_timings)
>>   {
>>   	u16 outw, outh;
>> -	u16 dw, dh;
>> +	u16 mgr_width, mgr_height;
>>
>> -	if (dssdev = NULL)
>> -		return 0;
>> -
>> -	dssdev->driver->get_resolution(dssdev,&dw,&dh);
>> +	mgr_width = mgr_timings->x_res;
>> +	mgr_height = mgr_timings->y_res;
>
> I think you could've just used the existing dw and dh variables, thus
> avoiding the need to change the rest of the function. The 'd' refers to
> display, which is more or less the same as mgr width.

I'll fix this.

Archit

>
>   Tomi
>


^ permalink raw reply

* Re: [PATCH v3 4/5] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Tomi Valkeinen @ 2012-05-08 11:55 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4FA901EC.8020002@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]

On Tue, 2012-05-08 at 16:52 +0530, Archit Taneja wrote:
> On Tuesday 08 May 2012 04:20 PM, Tomi Valkeinen wrote:

> > Checking the validity of all the settings is a bit tricky, but currently
> > I think, as a rule of thumb, we should accept any settings when things
> > are disabled. So, until the interface driver sets the timings before
> > enabling the output, all ovl/mgr settings should be allowed. And we
> 
> We have 2 ways to go about this, one is to have an initial set of 
> 'always valid' values like I have done, the other option is to ignore 
> manager timing related checks if the manager is disabled, i.e all 
> configs are okay. To implement the second option, I think our function 
> dss_check_settings_low() would get more complicated. We would now have 
> to pass mp->enabled outside of apply, and pass it to dss_mgr_check() 
> which would further need to pass this to dss_ovl_check(). Hence I used 
> the first approach.

I'm not sure about that. We already do it for overlay. In ovl.c we have
dss_ovl_simple_check() and dss_ovl_check(). The simple check sees if the
settings pass basic sanity check. The check sees if all the ovl & mgr
settings are compatible with each other.

Simple check is done when setting the config, called from
dss_ovl_set_info(). The proper check is done later when the actual
config is about to be taken into use.

If mp->enabled == false, can't we just skip dss_check_settings_low()
totally for that manager? We skip the check for ovls the same way.

> > shouldn't even write any shadow registers until the moment the output is
> > enabled.
> 
> That's being done correctly even now I guess, with the checks for 
> mp->enabled in wrtie_regs() and set_go_bits().

Yes, for timings. I was thinking more about the other settings done in
dpi.c currently, like dispc_mgr_set_pol_freq(). That writes directly to
registers, so we need runtime_get for that. 

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 0/6] OMAPDSS: Misc fixes and cleanups
From: Tomi Valkeinen @ 2012-05-08 11:58 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336389696-21636-1-git-send-email-archit@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

On Mon, 2012-05-07 at 16:51 +0530, Archit Taneja wrote:
> The first patch in this series is a follow up on the previously posted series
> 'OMAPDSS: APPLY: Treat overlay manager timings as shadow registers'. It is
> required for HDMI and DPI interfaces to work properly, it ensures manager
> timings are applied in the set_timing() ops for these interfaces.
> 
> The next 3 patches remove some unnecessary usage of omap_dss_device pointer in
> DISPC and APPLY.
> 
> The last 2 patches are miscellaneous fixes and are self explanatory.
> 
> Reference tree containing this series:
> 
> git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git mgr_timing_and_fixes
> 
> Tested on OMAP4 SDP.
> 
> Archit Taneja (6):
>   OMAPDSS: DPI/HDMI: Apply manager timings even if panel is disabled
>   OMAPDSS: APPLY: Remove an unnecessary omap_dss_device pointer
>   OMAPDSS: DISPC: Remove omap_dss_device pointer usage from
>     dispc_mgr_pclk_rate()
>   OMAPDSS: DISPC: Remove usage of dispc_mgr_get_device()
>   OMAPDSS: Fix DSI_FCLK clock source selection
>   OMAPDSS: DISPC: Remove Fake VSYNC support

The first four patches seem to be related (or at least based) to the
set_timings series (and the first patch you already had included in
later version).

Should I take the two last patches and apply them already, as they are
fine and don't depend on anything? You could add the first 4 patches
into the set_timings series.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 0/2] OMAPDSS: HDMI: Fix register dump of CORE registers
From: Tomi Valkeinen @ 2012-05-08 12:04 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336396828-29311-1-git-send-email-archit@ti.com>

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Mon, 2012-05-07 at 18:50 +0530, Archit Taneja wrote:
> The HDMI CORE registers are dumped incorrectly due to incorrect register offset
> calculations. They are also dumped in a random order, with some of the registers
> repeated. This series fixes these issues.
> 
> The patches apply over:
> 
> git://gitorious.org/linux-omap-dss2/linux.git dev
> 
> Tested on OMAP4 SDP.
> 
> Archit Taneja (2):
>   OMAPDSS: HDMI: Fix ti_hdmi_4xxx_core_dump
>   OMAPDSS: HDMI: define and dump CORE registers in correct order
> 
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |  163 +++++++++++++++--------------
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h |   57 +++++------
>  2 files changed, 109 insertions(+), 111 deletions(-)

Thanks, applying these.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 0/2] OMAPDSS: HDMI: Fix register dump of CORE registers
From: Archit Taneja @ 2012-05-08 12:21 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336478684.5761.30.camel@deskari>

Hi,

On Tuesday 08 May 2012 05:34 PM, Tomi Valkeinen wrote:
> On Mon, 2012-05-07 at 18:50 +0530, Archit Taneja wrote:
>> The HDMI CORE registers are dumped incorrectly due to incorrect register offset
>> calculations. They are also dumped in a random order, with some of the registers
>> repeated. This series fixes these issues.
>>
>> The patches apply over:
>>
>> git://gitorious.org/linux-omap-dss2/linux.git dev
>>
>> Tested on OMAP4 SDP.
>>
>> Archit Taneja (2):
>>    OMAPDSS: HDMI: Fix ti_hdmi_4xxx_core_dump
>>    OMAPDSS: HDMI: define and dump CORE registers in correct order
>>
>>   drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |  163 +++++++++++++++--------------
>>   drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h |   57 +++++------
>>   2 files changed, 109 insertions(+), 111 deletions(-)
>
> Thanks, applying these.

In the second patch, there are some unnecessary lines left in the 
core_dump function():

-	DUMPCOREAV(HDMI_CORE_AV_AVI_CHSUM);

-	for (i = 0; i < HDMI_CORE_AV_AVI_DBYTE_NELEMS; i++)
-		DUMPCOREAV2(i, HDMI_CORE_AV_AVI_DBYTE);

-	for (i = 0; i < HDMI_CORE_AV_SPD_DBYTE_NELEMS; i++)
-		DUMPCOREAV2(i, HDMI_CORE_AV_SPD_DBYTE);

-	for (i = 0; i < HDMI_CORE_AV_AUD_DBYTE_NELEMS; i++)
-		DUMPCOREAV2(i, HDMI_CORE_AV_AUD_DBYTE);
-
-	for (i = 0; i < HDMI_CORE_AV_MPEG_DBYTE_NELEMS; i++)
-		DUMPCOREAV2(i, HDMI_CORE_AV_MPEG_DBYTE);

-	for (i = 0; i < HDMI_CORE_AV_GEN_DBYTE_NELEMS; i++)
-		DUMPCOREAV2(i, HDMI_CORE_AV_GEN_DBYTE);
-
-	for (i = 0; i < HDMI_CORE_AV_GEN2_DBYTE_NELEMS; i++)
-		DUMPCOREAV2(i, HDMI_CORE_AV_GEN2_DBYTE);

So you will see 5 annoying empty lines, could you remove those when you 
apply, i totally missed this out..

Thanks,
Archit

>
>   Tomi
>


^ permalink raw reply

* Re: [PATCH 0/6] OMAPDSS: Misc fixes and cleanups
From: Archit Taneja @ 2012-05-08 12:24 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336478337.5761.29.camel@deskari>

On Tuesday 08 May 2012 05:28 PM, Tomi Valkeinen wrote:
> On Mon, 2012-05-07 at 16:51 +0530, Archit Taneja wrote:
>> The first patch in this series is a follow up on the previously posted series
>> 'OMAPDSS: APPLY: Treat overlay manager timings as shadow registers'. It is
>> required for HDMI and DPI interfaces to work properly, it ensures manager
>> timings are applied in the set_timing() ops for these interfaces.
>>
>> The next 3 patches remove some unnecessary usage of omap_dss_device pointer in
>> DISPC and APPLY.
>>
>> The last 2 patches are miscellaneous fixes and are self explanatory.
>>
>> Reference tree containing this series:
>>
>> git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git mgr_timing_and_fixes
>>
>> Tested on OMAP4 SDP.
>>
>> Archit Taneja (6):
>>    OMAPDSS: DPI/HDMI: Apply manager timings even if panel is disabled
>>    OMAPDSS: APPLY: Remove an unnecessary omap_dss_device pointer
>>    OMAPDSS: DISPC: Remove omap_dss_device pointer usage from
>>      dispc_mgr_pclk_rate()
>>    OMAPDSS: DISPC: Remove usage of dispc_mgr_get_device()
>>    OMAPDSS: Fix DSI_FCLK clock source selection
>>    OMAPDSS: DISPC: Remove Fake VSYNC support
>
> The first four patches seem to be related (or at least based) to the
> set_timings series (and the first patch you already had included in
> later version).
>
> Should I take the two last patches and apply them already, as they are
> fine and don't depend on anything? You could add the first 4 patches
> into the set_timings series.

Yes, you can apply the last 2 patches.

Thanks,
Archit

>
>   Tomi
>


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox