* Re: [PATCH V2 0/2] OMAPDSS: Enable dynamic debug printing
From: Tomi Valkeinen @ 2012-09-26 14:29 UTC (permalink / raw)
To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <cover.1348636132.git.cmahapatra@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]
Hi,
On Wed, 2012-09-26 at 10:45 +0530, Chandrabhanu Mahapatra wrote:
> Hi everyone,
> this patch series aims at cleaning up of DSS of printk()'s enabled with
> dss_debug and replace them with generic dynamic debug printing.
>
> The 1st patch
> * replaces printk() in DSSDBG definition with pr_debug()
> * removes DSSDBGF definition and replaces its instances with DSSDBG()
> The 2nd patch
> * cleans up printk()'s in omap_dispc_unregister_isr() and
> _dsi_print_reset_status() with pr_debug()
> * removes dss_debug variable
>
> Changes with respect to V1:
> * added debug messages to DSSDBG calls replacing DSSDBGF
> * added patch "OMAPDSS: Remove dss_debug variable"
>
> All your comments and suggestions are welcome.
This doesn't work quite correctly. The problem is in dss.h, where we
define DEBUG if CONFIG_OMAP2_DSS_DEBUG_SUPPORT is set. The thing is,
DEBUG should be defined before including the kernel headers where the
pr_debug etc are defined.
So if you try the patches without dynamic debugging enabled, you won't
get any debug outputs at all, even if CONFIG_OMAP2_DSS_DEBUG_SUPPORT is
set.
And for dynamic debug, the Kconfig help says:
If a source file is compiled with DEBUG flag set, any
pr_debug() calls in it are enabled by default, but can be
disabled at runtime as below. Note that DEBUG flag is
turned on by many CONFIG_*DEBUG* options.
So if we have CONFIG_OMAP2_DSS_DEBUG_SUPPORT set, all the pr_debugs
should be enabled by default, which is not the case, again because DEBUG
is defined too late.
I think setting DEBUG in dss.h should be removed, and instead DEBUG
should be set in the makefile if CONFIG_OMAP2_DSS_DEBUG_SUPPORT is set.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2 10/18] OMAPDSS: DISPC: Configure input and output sizes for writeback
From: Tomi Valkeinen @ 2012-09-26 6:34 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <50629F0B.1080904@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2903 bytes --]
On Wed, 2012-09-26 at 11:52 +0530, Archit Taneja wrote:
> On Tuesday 25 September 2012 08:03 PM, Tomi Valkeinen wrote:
> > On Tue, 2012-09-25 at 11:49 +0530, Archit Taneja wrote:
> >> Writeback uses the WB_PICTURE_SIZE register to define the size of the content
> >> written to memory, this is the output of the scaler. It uses the WB_SIZE
> >> register to define the size of the content coming from the overlay/manager to
> >> which it is connected, this is the input to the scaler. This naming is different
> >> as compared to overlays.
> >>
> >> Add checks for writeback in dispc_ovl_set_input_size() and
> >> dispc_ovl_set_output_size() to write to the correct registers.
> >>
> >> Signed-off-by: Archit Taneja <archit@ti.com>
> >> ---
> >> drivers/video/omap2/dss/dispc.c | 7 +++++--
> >> 1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> >> index e42e902..04fdd33 100644
> >> --- a/drivers/video/omap2/dss/dispc.c
> >> +++ b/drivers/video/omap2/dss/dispc.c
> >> @@ -719,7 +719,7 @@ static void dispc_ovl_set_input_size(enum omap_plane plane, int width,
> >> {
> >> u32 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
> >>
> >> - if (plane == OMAP_DSS_GFX)
> >> + if (plane == OMAP_DSS_GFX || plane == OMAP_DSS_WB)
> >> dispc_write_reg(DISPC_OVL_SIZE(plane), val);
> >> else
> >> dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
> >> @@ -734,7 +734,10 @@ static void dispc_ovl_set_output_size(enum omap_plane plane, int width,
> >>
> >> val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
> >>
> >> - dispc_write_reg(DISPC_OVL_SIZE(plane), val);
> >> + if (plane == OMAP_DSS_WB)
> >> + dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
> >> + else
> >> + dispc_write_reg(DISPC_OVL_SIZE(plane), val);
> >> }
> >
> > Should we just rename the dispc registers to DISPC_OVL_IN_SIZE and
> > DISPC_OVL_OUT_SIZE, and then we could do without the ifs? The registers
> > have always confused me a bit, I don't know why they are named so in the
> > TRM.
>
> It'll be hard for someone who's referring to the TRM in that case. It'll
> also be hard to track reg dumps if these names change. I think we should
> stick to the TRM names only.
Generally speaking I agree.
However, with each new OMAP we get register name changes. We already
have at least a few of those, and I think it'll just get worse. So at
some point we'll have a mishmash of register names from different omap
versions, or we'll rename the registers according to one particular omap
version (probably the latest one), or we'll name the registers as seems
best for the driver.
But yes, perhaps it's still best to keep the reg names as they are.
Perhaps there's even a logic with the names, I just can't see it =).
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2 10/18] OMAPDSS: DISPC: Configure input and output sizes for writeback
From: Archit Taneja @ 2012-09-26 6:34 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1348583595.21717.2.camel@lappyti>
On Tuesday 25 September 2012 08:03 PM, Tomi Valkeinen wrote:
> On Tue, 2012-09-25 at 11:49 +0530, Archit Taneja wrote:
>> Writeback uses the WB_PICTURE_SIZE register to define the size of the content
>> written to memory, this is the output of the scaler. It uses the WB_SIZE
>> register to define the size of the content coming from the overlay/manager to
>> which it is connected, this is the input to the scaler. This naming is different
>> as compared to overlays.
>>
>> Add checks for writeback in dispc_ovl_set_input_size() and
>> dispc_ovl_set_output_size() to write to the correct registers.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>> drivers/video/omap2/dss/dispc.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index e42e902..04fdd33 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -719,7 +719,7 @@ static void dispc_ovl_set_input_size(enum omap_plane plane, int width,
>> {
>> u32 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
>>
>> - if (plane = OMAP_DSS_GFX)
>> + if (plane = OMAP_DSS_GFX || plane = OMAP_DSS_WB)
>> dispc_write_reg(DISPC_OVL_SIZE(plane), val);
>> else
>> dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
>> @@ -734,7 +734,10 @@ static void dispc_ovl_set_output_size(enum omap_plane plane, int width,
>>
>> val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
>>
>> - dispc_write_reg(DISPC_OVL_SIZE(plane), val);
>> + if (plane = OMAP_DSS_WB)
>> + dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
>> + else
>> + dispc_write_reg(DISPC_OVL_SIZE(plane), val);
>> }
>
> Should we just rename the dispc registers to DISPC_OVL_IN_SIZE and
> DISPC_OVL_OUT_SIZE, and then we could do without the ifs? The registers
> have always confused me a bit, I don't know why they are named so in the
> TRM.
It'll be hard for someone who's referring to the TRM in that case. It'll
also be hard to track reg dumps if these names change. I think we should
stick to the TRM names only.
Archit
^ permalink raw reply
* Re: [PATCH v2 16/18] OMAPDSS: DISPC: Configure writeback FIFOs
From: Archit Taneja @ 2012-09-26 5:43 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1348584652.21717.7.camel@lappyti>
On Tuesday 25 September 2012 08:20 PM, Tomi Valkeinen wrote:
> On Tue, 2012-09-25 at 11:49 +0530, Archit Taneja wrote:
>> Extend the DISPC fifo functions to also configure the writeback FIFO thresholds.
>>
>> The most optimal configuration for writeback is to push out data to the
>> interconnect the moment writeback pushes enough pixels in the FIFO to form a
>> burst. This reduces the chance of writeback overflowing it's FIFO.
>
> Hmm, why is this optimal?
>
> The FIFO for WB is the output fifo, right? In mem-to-mem mode the whole
> WB pipeline can stall, so the fifo can't overflow? If so, isn't it
> better to collect more data and flush all that to the memory, instead of
> sending each burst-size piece one by one?
I guess this configuration is optimal for capture mode. Where the input
side will push out data at the rate of pixel clock, and writeback has to
collect this data and flush to memory. Writeback can't stall here.
About writeback FIFO overflowing in mem-to-mem mode, I'm not totally
sure about this. But what you are saying seems valid, writeback would
stall the input. So it should be safe to accumulate content in the FIFO,
and push out large number of bursts together.
>
> Then again, if the input side is reading pixels from the memory all the
> time, even if the output fifo helps to keep the output side idle for
> longer periods, it probably doesn't help as the input side keeps the
> memory bus awake.
Maybe if the FIFOs are adjusted such that reading of the pixels from
input side and writeback flushing out can be done at the same time, then
we might idle the memory bus more. I don't know if this is possible though.
Archit
^ permalink raw reply
* [PATCH V2 2/2] OMAPDSS: Remove dss_debug variable
From: Chandrabhanu Mahapatra @ 2012-09-26 5:28 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1348636132.git.cmahapatra@ti.com>
The debug prints in omap_dispc_unregister_isr() and _dsi_print_reset_status()
are replaced with dynamic debug enabled pr_debug(). So, as the final dependency
on dss_debug variable is replaced with dyndbg, the dss_debug variable is
removed.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/core.c | 5 -----
drivers/video/omap2/dss/dispc.c | 39 ++++++++++++++-------------------------
drivers/video/omap2/dss/dsi.c | 37 ++++++++++++++-----------------------
drivers/video/omap2/dss/dss.h | 4 ----
4 files changed, 28 insertions(+), 57 deletions(-)
diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index 58bd9c2..5d77c86 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -52,11 +52,6 @@ static char *def_disp_name;
module_param_named(def_disp, def_disp_name, charp, 0);
MODULE_PARM_DESC(def_disp, "default display name");
-#ifdef DEBUG
-bool dss_debug;
-module_param_named(debug, dss_debug, bool, 0644);
-#endif
-
/* REGULATORS */
struct regulator *dss_get_vdds_dsi(void)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 1121823..6c813a6 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3447,34 +3447,26 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
}
EXPORT_SYMBOL(omap_dispc_unregister_isr);
-#ifdef DEBUG
static void print_irq_status(u32 status)
{
if ((status & dispc.irq_error_mask) = 0)
return;
- printk(KERN_DEBUG "DISPC IRQ: 0x%x: ", status);
-
-#define PIS(x) \
- if (status & DISPC_IRQ_##x) \
- printk(#x " ");
- PIS(GFX_FIFO_UNDERFLOW);
- PIS(OCP_ERR);
- PIS(VID1_FIFO_UNDERFLOW);
- PIS(VID2_FIFO_UNDERFLOW);
- if (dss_feat_get_num_ovls() > 3)
- PIS(VID3_FIFO_UNDERFLOW);
- PIS(SYNC_LOST);
- PIS(SYNC_LOST_DIGIT);
- if (dss_has_feature(FEAT_MGR_LCD2))
- PIS(SYNC_LOST2);
- if (dss_has_feature(FEAT_MGR_LCD3))
- PIS(SYNC_LOST3);
+#define PIS(x) (status & DISPC_IRQ_##x) ? (#x " ") : ""
+
+ pr_debug("DISPC IRQ: 0x%x: %s%s%s%s%s%s%s%s%s\n",
+ status,
+ PIS(OCP_ERR),
+ PIS(GFX_FIFO_UNDERFLOW),
+ PIS(VID1_FIFO_UNDERFLOW),
+ PIS(VID2_FIFO_UNDERFLOW),
+ dss_feat_get_num_ovls() > 3 ? PIS(VID3_FIFO_UNDERFLOW) : "",
+ PIS(SYNC_LOST),
+ PIS(SYNC_LOST_DIGIT),
+ dss_has_feature(FEAT_MGR_LCD2) ? PIS(SYNC_LOST2) : "",
+ dss_has_feature(FEAT_MGR_LCD3) ? PIS(SYNC_LOST3) : "");
#undef PIS
-
- printk("\n");
}
-#endif
/* Called from dss.c. Note that we don't touch clocks here,
* but we presume they are on because we got an IRQ. However,
@@ -3507,10 +3499,7 @@ static irqreturn_t omap_dispc_irq_handler(int irq, void *arg)
spin_unlock(&dispc.irq_stats_lock);
#endif
-#ifdef DEBUG
- if (dss_debug)
- print_irq_status(irqstatus);
-#endif
+ print_irq_status(irqstatus);
/* Ack the interrupt. Do it here before clocks are possibly turned
* off */
dispc_write_reg(DISPC_IRQSTATUS, irqstatus);
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 18834f3..ed7ff72 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1100,28 +1100,16 @@ static inline void dsi_enable_pll_clock(struct platform_device *dsidev,
}
}
-#ifdef DEBUG
static void _dsi_print_reset_status(struct platform_device *dsidev)
{
u32 l;
int b0, b1, b2;
- if (!dss_debug)
- return;
-
/* A dummy read using the SCP interface to any DSIPHY register is
* required after DSIPHY reset to complete the reset of the DSI complex
* I/O. */
l = dsi_read_reg(dsidev, DSI_DSIPHY_CFG5);
- printk(KERN_DEBUG "DSI resets: ");
-
- l = dsi_read_reg(dsidev, DSI_PLL_STATUS);
- printk("PLL (%d) ", FLD_GET(l, 0, 0));
-
- l = dsi_read_reg(dsidev, DSI_COMPLEXIO_CFG1);
- printk("CIO (%d) ", FLD_GET(l, 29, 29));
-
if (dss_has_feature(FEAT_DSI_REVERSE_TXCLKESC)) {
b0 = 28;
b1 = 27;
@@ -1132,18 +1120,21 @@ static void _dsi_print_reset_status(struct platform_device *dsidev)
b2 = 26;
}
- l = dsi_read_reg(dsidev, DSI_DSIPHY_CFG5);
- printk("PHY (%x%x%x, %d, %d, %d)\n",
- FLD_GET(l, b0, b0),
- FLD_GET(l, b1, b1),
- FLD_GET(l, b2, b2),
- FLD_GET(l, 29, 29),
- FLD_GET(l, 30, 30),
- FLD_GET(l, 31, 31));
+#define DSI_FLD_GET(fld, start, end)\
+ FLD_GET(dsi_read_reg(dsidev, DSI_##fld), start, end)
+
+ pr_debug("DSI resets: PLL (%d) CIO (%d) PHY (%x%x%x, %d, %d, %d)\n",
+ DSI_FLD_GET(PLL_STATUS, 0, 0),
+ DSI_FLD_GET(COMPLEXIO_CFG1, 29, 29),
+ DSI_FLD_GET(DSIPHY_CFG5, b0, b0),
+ DSI_FLD_GET(DSIPHY_CFG5, b1, b1),
+ DSI_FLD_GET(DSIPHY_CFG5, b2, b2),
+ DSI_FLD_GET(DSIPHY_CFG5, 29, 29),
+ DSI_FLD_GET(DSIPHY_CFG5, 30, 30),
+ DSI_FLD_GET(DSIPHY_CFG5, 31, 31));
+
+#undef DSI_FLD_GET
}
-#else
-#define _dsi_print_reset_status(x)
-#endif
static inline int dsi_if_enable(struct platform_device *dsidev, bool enable)
{
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 3a2caab..0015803 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -27,10 +27,6 @@
#define DEBUG
#endif
-#ifdef DEBUG
-extern bool dss_debug;
-#endif
-
#ifdef pr_fmt
#undef pr_fmt
#endif
--
1.7.10
^ permalink raw reply related
* [PATCH V2 1/2] OMAPDSS: Cleanup DSSDBG with dynamic pr_debug function
From: Chandrabhanu Mahapatra @ 2012-09-26 5:27 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1348636132.git.cmahapatra@ti.com>
The printk in DSSDBG function definition is replaced with dynamic debug enabled
pr_debug(). The use of dynamic debugging provides more flexibility as each debug
statement can be enabled or disabled dynamically on basis of source filename,
line number, module name etc. by writing to a control file in debugfs
filesystem. For better understanding please refer to
Documentation/dynamic-debug-howto.txt.
The DSSDBGF() differs from DSSDBG() by providing function name. However,
function name, line number, module name and thread ID can be printed through
dynamic debug by setting appropriate flags 'f','l','m' and 't' in the debugfs
control file. So, DSSDBGF instances are replaced with DSSDBG.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/apply.c | 8 ++++----
drivers/video/omap2/dss/dsi.c | 12 ++++++------
drivers/video/omap2/dss/dss.h | 34 ++++++++--------------------------
3 files changed, 18 insertions(+), 36 deletions(-)
diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 6354bb8..e92f5fb 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -559,7 +559,7 @@ static void dss_ovl_write_regs(struct omap_overlay *ovl)
struct mgr_priv_data *mp;
int r;
- DSSDBGF("%d", ovl->id);
+ DSSDBG("writing ovl %d regs", ovl->id);
if (!op->enabled || !op->info_dirty)
return;
@@ -594,7 +594,7 @@ static void dss_ovl_write_regs_extra(struct omap_overlay *ovl)
struct ovl_priv_data *op = get_ovl_priv(ovl);
struct mgr_priv_data *mp;
- DSSDBGF("%d", ovl->id);
+ DSSDBG("writing ovl %d regs extra", ovl->id);
if (!op->extra_info_dirty)
return;
@@ -618,7 +618,7 @@ static void dss_mgr_write_regs(struct omap_overlay_manager *mgr)
struct mgr_priv_data *mp = get_mgr_priv(mgr);
struct omap_overlay *ovl;
- DSSDBGF("%d", mgr->id);
+ DSSDBG("writing mgr %d regs", mgr->id);
if (!mp->enabled)
return;
@@ -644,7 +644,7 @@ 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);
+ DSSDBG("writing mgr %d regs extra", mgr->id);
if (!mp->extra_info_dirty)
return;
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 8d815e3..18834f3 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1525,7 +1525,7 @@ int dsi_pll_set_clock_div(struct platform_device *dsidev,
u8 regn_start, regn_end, regm_start, regm_end;
u8 regm_dispc_start, regm_dispc_end, regm_dsi_start, regm_dsi_end;
- DSSDBGF();
+ DSSDBG("DSI PLL clock config starts");
dsi->current_cinfo.clkin = cinfo->clkin;
dsi->current_cinfo.fint = cinfo->fint;
@@ -2334,7 +2334,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
int r;
u32 l;
- DSSDBGF();
+ DSSDBG("DSI CIO init starts");
r = dss_dsi_enable_pads(dsi->module_id, dsi_get_lane_mask(dssdev));
if (r)
@@ -2686,7 +2686,7 @@ static void dsi_vc_initial_config(struct platform_device *dsidev, int channel)
{
u32 r;
- DSSDBGF("%d", channel);
+ DSSDBG("Initial Config of Virtual Channel %d", channel);
r = dsi_read_reg(dsidev, DSI_VC_CTRL(channel));
@@ -2718,7 +2718,7 @@ static int dsi_vc_config_source(struct platform_device *dsidev, int channel,
if (dsi->vc[channel].source = source)
return 0;
- DSSDBGF("%d", channel);
+ DSSDBG("Source Config of Virtual Channel %d", channel);
dsi_sync_vc(dsidev, channel);
@@ -3475,7 +3475,7 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
int r, i;
unsigned mask;
- DSSDBGF();
+ DSSDBG("Entering ULPS");
WARN_ON(!dsi_bus_is_locked(dsidev));
@@ -4184,7 +4184,7 @@ int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
unsigned long pck;
int r;
- DSSDBGF("ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
+ DSSDBG("Setting DSI clocks: ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
mutex_lock(&dsi->lock);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 5e9fd769..3a2caab 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -29,38 +29,20 @@
#ifdef DEBUG
extern bool dss_debug;
-#ifdef DSS_SUBSYS_NAME
-#define DSSDBG(format, ...) \
- if (dss_debug) \
- printk(KERN_DEBUG "omapdss " DSS_SUBSYS_NAME ": " format, \
- ## __VA_ARGS__)
-#else
-#define DSSDBG(format, ...) \
- if (dss_debug) \
- printk(KERN_DEBUG "omapdss: " format, ## __VA_ARGS__)
#endif
-#ifdef DSS_SUBSYS_NAME
-#define DSSDBGF(format, ...) \
- if (dss_debug) \
- printk(KERN_DEBUG "omapdss " DSS_SUBSYS_NAME \
- ": %s(" format ")\n", \
- __func__, \
- ## __VA_ARGS__)
-#else
-#define DSSDBGF(format, ...) \
- if (dss_debug) \
- printk(KERN_DEBUG "omapdss: " \
- ": %s(" format ")\n", \
- __func__, \
- ## __VA_ARGS__)
+#ifdef pr_fmt
+#undef pr_fmt
#endif
-#else /* DEBUG */
-#define DSSDBG(format, ...)
-#define DSSDBGF(format, ...)
+#ifdef DSS_SUBSYS_NAME
+#define pr_fmt(fmt) DSS_SUBSYS_NAME ": " fmt
+#else
+#define pr_fmt(fmt) fmt
#endif
+#define DSSDBG(format, ...) \
+ pr_debug(format, ## __VA_ARGS__)
#ifdef DSS_SUBSYS_NAME
#define DSSERR(format, ...) \
--
1.7.10
^ permalink raw reply related
* [PATCH V2 0/2] OMAPDSS: Enable dynamic debug printing
From: Chandrabhanu Mahapatra @ 2012-09-26 5:27 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <1348552998-6596-1-git-send-email-cmahapatra@ti.com>
Hi everyone,
this patch series aims at cleaning up of DSS of printk()'s enabled with
dss_debug and replace them with generic dynamic debug printing.
The 1st patch
* replaces printk() in DSSDBG definition with pr_debug()
* removes DSSDBGF definition and replaces its instances with DSSDBG()
The 2nd patch
* cleans up printk()'s in omap_dispc_unregister_isr() and
_dsi_print_reset_status() with pr_debug()
* removes dss_debug variable
Changes with respect to V1:
* added debug messages to DSSDBG calls replacing DSSDBGF
* added patch "OMAPDSS: Remove dss_debug variable"
All your comments and suggestions are welcome.
Regards,
Chandrabhanu
Chandrabhanu Mahapatra (2):
OMAPDSS: Cleanup DSSDBG with dynamic pr_debug function
OMAPDSS: Remove dss_debug variable
drivers/video/omap2/dss/apply.c | 8 +++----
drivers/video/omap2/dss/core.c | 5 ----
drivers/video/omap2/dss/dispc.c | 39 +++++++++++--------------------
drivers/video/omap2/dss/dsi.c | 49 ++++++++++++++++-----------------------
drivers/video/omap2/dss/dss.h | 34 +++++----------------------
5 files changed, 44 insertions(+), 91 deletions(-)
--
1.7.10
^ permalink raw reply
* Re: [PATCH v2 16/18] OMAPDSS: DISPC: Configure writeback FIFOs
From: Tomi Valkeinen @ 2012-09-25 14:50 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1348553993-8083-17-git-send-email-archit@ti.com>
[-- Attachment #1: Type: text/plain, Size: 904 bytes --]
On Tue, 2012-09-25 at 11:49 +0530, Archit Taneja wrote:
> Extend the DISPC fifo functions to also configure the writeback FIFO thresholds.
>
> The most optimal configuration for writeback is to push out data to the
> interconnect the moment writeback pushes enough pixels in the FIFO to form a
> burst. This reduces the chance of writeback overflowing it's FIFO.
Hmm, why is this optimal?
The FIFO for WB is the output fifo, right? In mem-to-mem mode the whole
WB pipeline can stall, so the fifo can't overflow? If so, isn't it
better to collect more data and flush all that to the memory, instead of
sending each burst-size piece one by one?
Then again, if the input side is reading pixels from the memory all the
time, even if the output fifo helps to keep the output side idle for
longer periods, it probably doesn't help as the input side keeps the
memory bus awake.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2 10/18] OMAPDSS: DISPC: Configure input and output sizes for writeback
From: Tomi Valkeinen @ 2012-09-25 14:33 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1348553993-8083-11-git-send-email-archit@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]
On Tue, 2012-09-25 at 11:49 +0530, Archit Taneja wrote:
> Writeback uses the WB_PICTURE_SIZE register to define the size of the content
> written to memory, this is the output of the scaler. It uses the WB_SIZE
> register to define the size of the content coming from the overlay/manager to
> which it is connected, this is the input to the scaler. This naming is different
> as compared to overlays.
>
> Add checks for writeback in dispc_ovl_set_input_size() and
> dispc_ovl_set_output_size() to write to the correct registers.
>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> drivers/video/omap2/dss/dispc.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index e42e902..04fdd33 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -719,7 +719,7 @@ static void dispc_ovl_set_input_size(enum omap_plane plane, int width,
> {
> u32 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
>
> - if (plane == OMAP_DSS_GFX)
> + if (plane == OMAP_DSS_GFX || plane == OMAP_DSS_WB)
> dispc_write_reg(DISPC_OVL_SIZE(plane), val);
> else
> dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
> @@ -734,7 +734,10 @@ static void dispc_ovl_set_output_size(enum omap_plane plane, int width,
>
> val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
>
> - dispc_write_reg(DISPC_OVL_SIZE(plane), val);
> + if (plane == OMAP_DSS_WB)
> + dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
> + else
> + dispc_write_reg(DISPC_OVL_SIZE(plane), val);
> }
Should we just rename the dispc registers to DISPC_OVL_IN_SIZE and
DISPC_OVL_OUT_SIZE, and then we could do without the ifs? The registers
have always confused me a bit, I don't know why they are named so in the
TRM.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] da8xx-fb: save and restore LCDC context across suspend/resume cycle
From: Manjunathappa, Prakash @ 2012-09-25 14:23 UTC (permalink / raw)
To: linux-fbdev
Save and restore register context of LCDC respectively
before suspend and after resume.
Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Applies on top of below patch under review:
"da8xx-fb: add pm_runtime support"
drivers/video/da8xx-fb.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 4280d23..65a11ef 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1491,6 +1491,69 @@ err_request_mem:
}
#ifdef CONFIG_PM
+struct lcdc_context {
+ u32 clk_enable;
+ u32 ctrl;
+ u32 dma_ctrl;
+ u32 raster_timing_0;
+ u32 raster_timing_1;
+ u32 raster_timing_2;
+ u32 int_enable_set;
+ u32 dma_frm_buf_base_addr_0;
+ u32 dma_frm_buf_ceiling_addr_0;
+ u32 dma_frm_buf_base_addr_1;
+ u32 dma_frm_buf_ceiling_addr_1;
+ u32 raster_ctrl;
+} reg_context;
+
+static void lcd_context_save(void)
+{
+ if (lcd_revision = LCD_VERSION_2) {
+ reg_context.clk_enable = lcdc_read(LCD_CLK_ENABLE_REG);
+ reg_context.int_enable_set = lcdc_read(LCD_INT_ENABLE_SET_REG);
+ }
+
+ reg_context.ctrl = lcdc_read(LCD_CTRL_REG);
+ reg_context.dma_ctrl = lcdc_read(LCD_DMA_CTRL_REG);
+ reg_context.raster_timing_0 = lcdc_read(LCD_RASTER_TIMING_0_REG);
+ reg_context.raster_timing_1 = lcdc_read(LCD_RASTER_TIMING_1_REG);
+ reg_context.raster_timing_2 = lcdc_read(LCD_RASTER_TIMING_2_REG);
+ reg_context.dma_frm_buf_base_addr_0 + lcdc_read(LCD_DMA_FRM_BUF_BASE_ADDR_0_REG);
+ reg_context.dma_frm_buf_ceiling_addr_0 + lcdc_read(LCD_DMA_FRM_BUF_CEILING_ADDR_0_REG);
+ reg_context.dma_frm_buf_base_addr_1 + lcdc_read(LCD_DMA_FRM_BUF_BASE_ADDR_1_REG);
+ reg_context.dma_frm_buf_ceiling_addr_1 + lcdc_read(LCD_DMA_FRM_BUF_CEILING_ADDR_1_REG);
+ reg_context.raster_ctrl = lcdc_read(LCD_RASTER_CTRL_REG);
+ return;
+}
+
+static void lcd_context_restore(void)
+{
+ if (lcd_revision = LCD_VERSION_2) {
+ lcdc_write(reg_context.clk_enable, LCD_CLK_ENABLE_REG);
+ lcdc_write(reg_context.int_enable_set, LCD_INT_ENABLE_SET_REG);
+ }
+
+ lcdc_write(reg_context.ctrl, LCD_CTRL_REG);
+ lcdc_write(reg_context.dma_ctrl, LCD_DMA_CTRL_REG);
+ lcdc_write(reg_context.raster_timing_0, LCD_RASTER_TIMING_0_REG);
+ lcdc_write(reg_context.raster_timing_1, LCD_RASTER_TIMING_1_REG);
+ lcdc_write(reg_context.raster_timing_2, LCD_RASTER_TIMING_2_REG);
+ lcdc_write(reg_context.dma_frm_buf_base_addr_0,
+ LCD_DMA_FRM_BUF_BASE_ADDR_0_REG);
+ lcdc_write(reg_context.dma_frm_buf_ceiling_addr_0,
+ LCD_DMA_FRM_BUF_CEILING_ADDR_0_REG);
+ lcdc_write(reg_context.dma_frm_buf_base_addr_1,
+ LCD_DMA_FRM_BUF_BASE_ADDR_1_REG);
+ lcdc_write(reg_context.dma_frm_buf_ceiling_addr_1,
+ LCD_DMA_FRM_BUF_CEILING_ADDR_1_REG);
+ lcdc_write(reg_context.raster_ctrl, LCD_RASTER_CTRL_REG);
+ return;
+}
+
static int fb_suspend(struct platform_device *dev, pm_message_t state)
{
struct fb_info *info = platform_get_drvdata(dev);
@@ -1502,6 +1565,7 @@ static int fb_suspend(struct platform_device *dev, pm_message_t state)
fb_set_suspend(info, 1);
lcd_disable_raster(true);
+ lcd_context_save();
pm_runtime_put_sync(&dev->dev);
console_unlock();
@@ -1514,6 +1578,7 @@ static int fb_resume(struct platform_device *dev)
console_lock();
pm_runtime_get_sync(&dev->dev);
+ lcd_context_restore();
if (par->blank = FB_BLANK_UNBLANK) {
lcd_enable_raster();
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] OMAPDSS: DISPC: Add predecimation limit for TILER based rotations
From: Tomi Valkeinen @ 2012-09-25 13:40 UTC (permalink / raw)
To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1348468707-5142-1-git-send-email-cmahapatra@ti.com>
[-- Attachment #1: Type: text/plain, Size: 748 bytes --]
On Mon, 2012-09-24 at 12:08 +0530, Chandrabhanu Mahapatra wrote:
> In OMAP4 and OMAP5 when TILER 2D burst mode is used, a maximum of one line can
> be skipped as per the respective TRMs. The MBlockStride OCP signal, which is
> sum of ROWINC and image width in memory, is only 17 bits wide. In 2D mode TILER
> supports 8192, 16384, 32768 and 65536 values of MBlockStride. In case when 2 or
> more lines are skipped the ROWINC value exceeds 65536 resulting in OCP errors.
> So, maximum vertical predecimation achievable is 2.
>
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
> drivers/video/omap2/dss/dispc.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
Thanks, looks fine to me.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2] of: Add videomode helper
From: Laurent Pinchart @ 2012-09-25 12:59 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20120913111954.GH6180-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Sascha,
On Thursday 13 September 2012 13:19:54 Sascha Hauer wrote:
> On Thu, Sep 13, 2012 at 01:54:07PM +0300, Tomi Valkeinen wrote:
> > On Wed, 2012-07-04 at 09:56 +0200, Sascha Hauer wrote:
> > > This patch adds a helper function for parsing videomodes from the
> > > devicetree. The videomode can be either converted to a struct
> > > drm_display_mode or a struct fb_videomode.
> >
> > I have more or less the same generic comment for this as for the power
> > sequences series discussed: this would add panel specific information
> > into DT data, instead of the driver handling it. But, as also discussed
> > in the thread, there are differing opinions on which way is better.
>
> With the panel timings I think the approach of moving them into DT is
> the best we can do. There are so many displays out there, patching the
> kernel each time a customer comes with some new display is no fun.
For generic panels, sure, but for panels that require a specific driver (for
instance because the panel implements vendor-specific comments) the mode(s)
could be hardcoded in the panel driver.
> > > +int of_get_video_mode(struct device_node *np, struct drm_display_mode
> > > *dmode,
> > > + struct fb_videomode *fbmode);
> >
> > From caller's point of view I think it'd make more sense to have
> > separate functions to get drm mode or fb mode. I don't think there's a
> > case where the caller would want both.
>
> Ok, that makes sense.
>
> > Then again, even better would be to have a common video mode struct used
> > by both fb and drm... But I think that's been on the todo list of
> > Laurent for a long time already =).
>
> Yes, indeed. We should go into that direction. We already realized that
> we want to have ranges instead of fixed values for the timings.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v4] of: Add videomode helper
From: Laurent Pinchart @ 2012-09-25 11:23 UTC (permalink / raw)
To: Rob Herring
Cc: linux-fbdev, devicetree-discuss, dri-devel, kernel,
Steffen Trumtrar
In-Reply-To: <50606334.7030902@gmail.com>
Hi Rob,
On Monday 24 September 2012 08:42:12 Rob Herring wrote:
> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> > This patch adds a helper function for parsing videomodes from the
> > devicetree. The videomode can be either converted to a struct
> > drm_display_mode or a struct fb_videomode.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > ---
> >
> > Hi!
> >
> > changes since v3:
> > - print error messages
> > - free alloced memory
> > - general cleanup
> >
> > Regards
> > Steffen
> >
> > .../devicetree/bindings/video/displaymode | 74 +++++
> > drivers/of/Kconfig | 5 +
> > drivers/of/Makefile | 1 +
> > drivers/of/of_videomode.c | 283
> > ++++++++++++++++++++ include/linux/of_videomode.h
> > | 56 ++++
> > 5 files changed, 419 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/video/displaymode
> > create mode 100644 drivers/of/of_videomode.c
> > create mode 100644 include/linux/of_videomode.h
> >
> > diff --git a/Documentation/devicetree/bindings/video/displaymode
> > b/Documentation/devicetree/bindings/video/displaymode new file mode
> > 100644
> > index 0000000..990ca52
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/displaymode
> > @@ -0,0 +1,74 @@
> > +videomode bindings
> > +=========
> > +
> > +Required properties:
> > + - hactive, vactive: Display resolution
> > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing
> > parameters + in pixels
> > + vfront-porch, vback-porch, vsync-len: Vertical display timing
> > parameters in + lines
> > + - clock: displayclock in Hz
>
> A major piece missing is the LCD controller to display interface width
> and component ordering.
>
> > +
> > +Optional properties:
> > + - width-mm, height-mm: Display dimensions in mm
> > + - hsync-active-high (bool): Hsync pulse is active high
> > + - vsync-active-high (bool): Vsync pulse is active high
> > + - interlaced (bool): This is an interlaced mode
> > + - doublescan (bool): This is a doublescan mode
> > +
> > +There are different ways of describing a display mode. The devicetree
> > representation
> > +corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its mode is split in two parts: first
> > the display
> > +properties like size in mm and (optionally) multiple subnodes with the
> > supported modes.
> > +
> > +Example:
> > +
> > + display@0 {
>
> It would be useful to have a compatible string here. We may not always
> know the panel type or have a fixed panel though. We could define
> "generic-lcd" or something for cases where the panel type is unknown.
I'm working on a generic panel framework (see
http://lwn.net/Articles/512363/). DT bindings are not there yet, but they're
certainly a hot topic. A compatible string here will definitely be needed
here.
The exact properties required in the display node will likely be panel-
dependent. width-mm, height-mm and modes sound like a good baseline to me.
> > + width-mm = <800>;
> > + height-mm = <480>;
> > + modes {
> > + mode0: mode@0 {
> > + /* 1920x1080p24 */
> > + clock = <52000000>;
> > + hactive = <1920>;
> > + vactive = <1080>;
> > + hfront-porch = <25>;
> > + hback-porch = <25>;
> > + hsync-len = <25>;
> > + vback-porch = <2>;
> > + vfront-porch = <2>;
> > + vsync-len = <2>;
> > + hsync-active-high;
> > + };
> > + };
> > + };
> > +
> > +Every property also supports the use of ranges, so the commonly used
> > datasheet +description with <min typ max>-tuples can be used.
> > +
> > +Example:
> > +
> > + mode1: mode@1 {
> > + /* 1920x1080p24 */
> > + clock = <148500000>;
> > + hactive = <1920>;
> > + vactive = <1080>;
> > + hsync-len = <0 44 60>;
> > + hfront-porch = <80 88 95>;
> > + hback-porch = <100 148 160>;
> > + vfront-porch = <0 4 6>;
> > + vback-porch = <0 36 50>;
> > + vsync-len = <0 5 6>;
> > + };
> > +
> > +The videomode can be linked to a connector via phandles. The connector
> > has to
> > +support the display- and default-mode-property to link to and select a
> > mode.
>
> Could also be phandle in the lcd controller node? What are the '-' for?
> Is "display-blah" a valid name or something?
>
> "default-mode" is pretty generic. How about display-mode or
> display-default-mode?
>
> Rob
>
> > +
> > +Example:
> > +
> > + hdmi@00120000 {
> > + status = "okay";
> > + display = <&benq>;
> > + default-mode = <&mode1>;
> > + };
> > +
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 1/2] video: Add support for the Solomon SSD1307 OLED Controller
From: Maxime Ripard @ 2012-09-25 10:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <505C73D1.7030800@free-electrons.com>
Hello Florian,
Le 21/09/2012 16:04, Maxime Ripard a écrit :
> Le 20/09/2012 18:33, Maxime Ripard a écrit :
>> This patch adds support for the Solomon SSD1307 OLED
>> controller found on the Crystalfontz CFA10036 board.
>>
>> This controller can drive a display with a resolution up
>> to 128x39 and can operate over I2C or SPI.
>>
>> The current driver has only been tested on the CFA-10036,
>> that is using this controller over I2C to driver a 96x16
>> OLED screen.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> Cc: Brian Lilly <brian@crystalfontz.com>
>
> Would you consider merging this patch or review it?
> It has been around for quite some time now and has not received any
> comments for at least the 2 last versions, so I guess there is not much
> left holding it back (unless, of course, if you have some comments to
> address), and I really would like to have it in 3.7.
I guess we missed the 3.7 merge here. That's no big deal, but I sent the
patch series on a regular basis, starting 07/17 (sent new versions on
07/31, 08/23, 09/05 and 09/20), addressed all the issues that were
raised, and never got any feedback from you, either positive or
negative. The patches affecting the MX28 device trees have already been
merged by Shawn Guo into its tree, so only the driver and the bindings
remain to be merged. So I'm kind of lost on what should I do here?
Is it because I have an obviously broken driver or you don't have enough
time, or you don't want to merge drivers for such type of low resolution
controllers, or any other reason?
Thanks for your answer,
Maxime
--
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Cleanup DSSDBG with dynamic pr_debug function
From: Tomi Valkeinen @ 2012-09-25 9:57 UTC (permalink / raw)
To: Mahapatra, Chandrabhanu; +Cc: linux-omap, linux-fbdev
In-Reply-To: <CAF0AtAukV_5FXXc79yuXr50+1XXWNiFKdN5MWLgRy2N6Ux-adg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3425 bytes --]
On Tue, 2012-09-25 at 15:00 +0530, Mahapatra, Chandrabhanu wrote:
> > This debug message is even less meaningful than the overlay number =).
> > Again, I think either keep the DSSDBGF, or print something sensible,
> > like "entering ULPS".
> >
>
> I dont think it would be wise enough to update code for one and keep
> the older version for another when both DSSDBG and DSSDBGF are almost
> one and the same. Its better to add something meaningful to the prints
> as you have mentioned like "writing ovl %d regs" and "DSI entering
> ULPS".
I didn't mean to leave DSSDBGF unchanged. I meant that it should work as
it works now, printing the func name, but using pr_debug.
> >>
> >> WARN_ON(!dsi_bus_is_locked(dsidev));
> >>
> >> @@ -4184,7 +4180,7 @@ int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
> >> unsigned long pck;
> >> int r;
> >>
> >> - DSSDBGF("ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
> >> + DSSDBG("ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
> >>
> >> mutex_lock(&dsi->lock);
> >>
> >> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> >> index 5e9fd769..3a2caab 100644
> >> --- a/drivers/video/omap2/dss/dss.h
> >> +++ b/drivers/video/omap2/dss/dss.h
> >> @@ -29,38 +29,20 @@
> >>
> >> #ifdef DEBUG
> >> extern bool dss_debug;
> >
> > You still left the dss_debug option here, even if it's not used by the
> > DSSDBG anymore. What's your plan about this?
> >
> > Tomi
> >
>
> dss_debug and DEBUG need to remain here as it is being used by
> functions omap_dispc_irq_handler() and _dsi_print_reset_status() in
It would be good to clean all this in one patch series.
> dispc.c and dsi.c. I am little bit unsure of how to deal with it.
> There could be a single print in omap_dispc_irq_handler() but it is a
> bit tricky in _dsi_print_reset_status().
>
> May be a macro like this one can be used in _dsi_print_reset_status()
>
> #define DSI_FLD_GET(fld, start, end)\
> FLD_GET(dsi_read_reg(dsidev, DSI_##fld), start, end);
>
> pr_debug("PLL (%d) CIO (%d) \n PHY (%x%x%x, %d, %d, %d) \n",
> DSI_FLD_GET(PLL_STATUS, 0, 0),
> DSI_FLD_GET(COMPLEXIO_CFG1, 29, 29),
> DSI_FLD_GET(DSIPHY_CFG5, bo, bo),
> DSI_FLD_GET(DSIPHY_CFG5, b1, b1),
> ..................................................);
> This could be defined at the beginning of the function and later at its end.
Yes, I think something like that could work. I don't see any problem
with having temporary helper macros to help creating the debug message.
> As you had previously mentioned a print like
>
> #define PIS(x) (status & DSI_IRQ_##x) ? (#x " ") : ""
>
> pr_debug("DSI IRQ: 0x%x: %s%s%s",
> status,
> PIS(WAKEUP),
> PIS(RESYNC),
> PIS(PLL_LOCK));
>
> could help in print_irq_status() but I am still unsure how to deal
> with conditional statements in print_irq_status() like
> if (dss_has_feature(FEAT_MGR_LCD3))
> PIS(SYNC_LOST3);
> Should we use approach like
>
> pr_debug("DSI IRQ: 0x%x: %s%s%s%s...",
> status,
> PIS(WAKEUP),
> PIS(RESYNC),
> PIS(PLL_LOCK)
> dss_has_feature(FEAT_MGR_LCD3) ? PIS(SYNC_LOST3) : ""
> ...................................... );
Yes, that looks fine to me.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: Cleanup DSSDBG with dynamic pr_debug function
From: Mahapatra, Chandrabhanu @ 2012-09-25 9:42 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1348554291.2342.8.camel@deskari>
>> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
>> index 6354bb8..cb86d94 100644
>> --- a/drivers/video/omap2/dss/apply.c
>> +++ b/drivers/video/omap2/dss/apply.c
>> @@ -559,7 +559,7 @@ static void dss_ovl_write_regs(struct omap_overlay *ovl)
>> struct mgr_priv_data *mp;
>> int r;
>>
>> - DSSDBGF("%d", ovl->id);
>> + DSSDBG("%d", ovl->id);
>
> I don't think this is good. It's true that dyn-debug can print the
> function name, but that's optional. The debug message should be somehow
> sensible independently, but in this case only a number is printed which
> is totally meaningless.
>
> Either the messages should be modified to give a hint what's going on,
> or the DSSDBGF could be kept for now. In the above case the debug
> message could be something like "writing ovl %d regs".
>
> However, I think it'd be easier just to keep the DSSDBGF for now, and
> remove it gradually.
>
>> if (!op->enabled || !op->info_dirty)
>> return;
>> @@ -594,7 +594,7 @@ static void dss_ovl_write_regs_extra(struct omap_overlay *ovl)
>> struct ovl_priv_data *op = get_ovl_priv(ovl);
>> struct mgr_priv_data *mp;
>>
>> - DSSDBGF("%d", ovl->id);
>> + DSSDBG("%d", ovl->id);
>>
>> if (!op->extra_info_dirty)
>> return;
>> @@ -618,7 +618,7 @@ static void dss_mgr_write_regs(struct omap_overlay_manager *mgr)
>> struct mgr_priv_data *mp = get_mgr_priv(mgr);
>> struct omap_overlay *ovl;
>>
>> - DSSDBGF("%d", mgr->id);
>> + DSSDBG("%d", mgr->id);
>>
>> if (!mp->enabled)
>> return;
>> @@ -644,7 +644,7 @@ 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);
>> + DSSDBG("%d", mgr->id);
>>
>> if (!mp->extra_info_dirty)
>> return;
>> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
>> index 8d815e3..8304cc6b 100644
>> --- a/drivers/video/omap2/dss/dsi.c
>> +++ b/drivers/video/omap2/dss/dsi.c
>> @@ -1525,8 +1525,6 @@ int dsi_pll_set_clock_div(struct platform_device *dsidev,
>> u8 regn_start, regn_end, regm_start, regm_end;
>> u8 regm_dispc_start, regm_dispc_end, regm_dsi_start, regm_dsi_end;
>>
>> - DSSDBGF();
>> -
>> dsi->current_cinfo.clkin = cinfo->clkin;
>> dsi->current_cinfo.fint = cinfo->fint;
>> dsi->current_cinfo.clkin4ddr = cinfo->clkin4ddr;
>> @@ -2334,8 +2332,6 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
>> int r;
>> u32 l;
>>
>> - DSSDBGF();
>> -
>> r = dss_dsi_enable_pads(dsi->module_id, dsi_get_lane_mask(dssdev));
>> if (r)
>> return r;
>> @@ -2686,7 +2682,7 @@ static void dsi_vc_initial_config(struct platform_device *dsidev, int channel)
>> {
>> u32 r;
>>
>> - DSSDBGF("%d", channel);
>> + DSSDBG("%d", channel);
>>
>> r = dsi_read_reg(dsidev, DSI_VC_CTRL(channel));
>>
>> @@ -2718,7 +2714,7 @@ static int dsi_vc_config_source(struct platform_device *dsidev, int channel,
>> if (dsi->vc[channel].source = source)
>> return 0;
>>
>> - DSSDBGF("%d", channel);
>> + DSSDBG("%d", channel);
>>
>> dsi_sync_vc(dsidev, channel);
>>
>> @@ -3475,7 +3471,7 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
>> int r, i;
>> unsigned mask;
>>
>> - DSSDBGF();
>> + DSSDBG("");
>
> This debug message is even less meaningful than the overlay number =).
> Again, I think either keep the DSSDBGF, or print something sensible,
> like "entering ULPS".
>
I dont think it would be wise enough to update code for one and keep
the older version for another when both DSSDBG and DSSDBGF are almost
one and the same. Its better to add something meaningful to the prints
as you have mentioned like "writing ovl %d regs" and "DSI entering
ULPS".
>>
>> WARN_ON(!dsi_bus_is_locked(dsidev));
>>
>> @@ -4184,7 +4180,7 @@ int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
>> unsigned long pck;
>> int r;
>>
>> - DSSDBGF("ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
>> + DSSDBG("ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
>>
>> mutex_lock(&dsi->lock);
>>
>> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
>> index 5e9fd769..3a2caab 100644
>> --- a/drivers/video/omap2/dss/dss.h
>> +++ b/drivers/video/omap2/dss/dss.h
>> @@ -29,38 +29,20 @@
>>
>> #ifdef DEBUG
>> extern bool dss_debug;
>
> You still left the dss_debug option here, even if it's not used by the
> DSSDBG anymore. What's your plan about this?
>
> Tomi
>
dss_debug and DEBUG need to remain here as it is being used by
functions omap_dispc_irq_handler() and _dsi_print_reset_status() in
dispc.c and dsi.c. I am little bit unsure of how to deal with it.
There could be a single print in omap_dispc_irq_handler() but it is a
bit tricky in _dsi_print_reset_status().
May be a macro like this one can be used in _dsi_print_reset_status()
#define DSI_FLD_GET(fld, start, end)\
FLD_GET(dsi_read_reg(dsidev, DSI_##fld), start, end);
pr_debug("PLL (%d) CIO (%d) \n PHY (%x%x%x, %d, %d, %d) \n",
DSI_FLD_GET(PLL_STATUS, 0, 0),
DSI_FLD_GET(COMPLEXIO_CFG1, 29, 29),
DSI_FLD_GET(DSIPHY_CFG5, bo, bo),
DSI_FLD_GET(DSIPHY_CFG5, b1, b1),
..................................................);
This could be defined at the beginning of the function and later at its end.
As you had previously mentioned a print like
#define PIS(x) (status & DSI_IRQ_##x) ? (#x " ") : ""
pr_debug("DSI IRQ: 0x%x: %s%s%s",
status,
PIS(WAKEUP),
PIS(RESYNC),
PIS(PLL_LOCK));
could help in print_irq_status() but I am still unsure how to deal
with conditional statements in print_irq_status() like
if (dss_has_feature(FEAT_MGR_LCD3))
PIS(SYNC_LOST3);
Should we use approach like
pr_debug("DSI IRQ: 0x%x: %s%s%s%s...",
status,
PIS(WAKEUP),
PIS(RESYNC),
PIS(PLL_LOCK)
dss_has_feature(FEAT_MGR_LCD3) ? PIS(SYNC_LOST3) : ""
...................................... );
--
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.
^ permalink raw reply
* Re: [PATCH v4] of: Add videomode helper
From: Steffen Trumtrar @ 2012-09-25 8:03 UTC (permalink / raw)
To: Stephen Warren
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Sascha Hauer
In-Reply-To: <5060E82A.3030404-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
On Mon, Sep 24, 2012 at 05:09:30PM -0600, Stephen Warren wrote:
> On 09/24/2012 12:26 PM, Rob Herring wrote:
> > On 09/24/2012 10:45 AM, Stephen Warren wrote:
> >> On 09/24/2012 07:42 AM, Rob Herring wrote:
> >>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> >>>> This patch adds a helper function for parsing videomodes from the devicetree.
> >>>> The videomode can be either converted to a struct drm_display_mode or a
> >>>> struct fb_videomode.
> >>
> >>>> +++ b/Documentation/devicetree/bindings/video/displaymode
> >>>> @@ -0,0 +1,74 @@
> >>>> +videomode bindings
> >>>> +=========
> >>>> +
> >>>> +Required properties:
> >>>> + - hactive, vactive: Display resolution
> >>>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
> >>>> + in pixels
> >>>> + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
> >>>> + lines
> >>>> + - clock: displayclock in Hz
> >>>
> >>> A major piece missing is the LCD controller to display interface width
> >>> and component ordering.
> >>
> >> I thought this binding was solely defining the timing of the video
> >> signal (hence "video mode"). Any definition of the physical interface to
> >> the LCD/display-connector is something entirely orthogonal, so it seems
> >> entirely reasonable to represent that separately.
> >
> > It is not orthogonal because in many cases the LCD panel defines the
> > mode.
>
> The LCD panel itself defines both the mode and the physical interface
> properties. The mode does not imply anything about the physical
> interface, nor does the physical interface imply anything about the
> mode. So, they are in fact orthogonal. In other words, just because you
> need both sets of information, doesn't make the two pieces of
> information correlated.
>
> >>>> +Example:
> >>>> +
> >>>> + display@0 {
> >>>
> >>> It would be useful to have a compatible string here. We may not always
> >>> know the panel type or have a fixed panel though. We could define
> >>> "generic-lcd" or something for cases where the panel type is unknown.
> >>>
> >>>> + width-mm = <800>;
> >>>> + height-mm = <480>;
> >>
> >> I would hope that everything in the example above this point is just
> >> that - an example, and this binding only covers the display mode
> >> definition - i.e. that part of the example below.
> >>
> >
> > It's fairly clear this binding is being defined based on what Linux
> > supports vs. what the h/w looks like.
> >
> >> If that's not the intent, as Rob says, there's a /ton/ of stuff missing.
> >
> > Assuming not, what all is missing?
>
> Everything related to the physical interface:
>
> * For DSI, whatever it needs to be configured.
> * For LVDS, e.g. number of lanes of R, G, B.
> * Perhaps multi-pumping rates (# of clock signals to send each data
> value for, to satisfy any minimum clock rates)
> * Built-in displays typically need to be coupled with a backlight and
> all the associated control of that.
> * Pinctrl interaction.
>
> and probably a bunch of other things I haven't thought about.
I already added some of those things in my v5. For those who missed it
<1348500924-8551-1-git-send-email-s.trumtrar@pengutronix.de>
I renamed from "of videomode helper" to "of display helper", seemed to be more
clear what it is supposed to accomplish.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v4] of: Add videomode helper
From: Steffen Trumtrar @ 2012-09-25 7:51 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20120924191628.GR1322-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Mon, Sep 24, 2012 at 09:16:28PM +0200, Sascha Hauer wrote:
> On Mon, Sep 24, 2012 at 10:18:39AM -0500, Rob Herring wrote:
> > On 09/24/2012 09:12 AM, Sascha Hauer wrote:
> > >>
> > >> A major piece missing is the LCD controller to display interface width
> > >> and component ordering.
> > >
> > > Psst. We silently skipped this for now...
> > >
> > > I think having such a videomode helper is useful without having the
> > > data order part. We are aware that this should be added later, but
> > > I think currently it makes sense to concentrate on the basics.
> >
> > Evolving bindings is not a good thing. We know this is needed, so we
> > should address it now. If Linux had a standard way to describe the
> > interface (it didn't a few years ago and I haven't kept up), then I
> > would bet it would already be part of this binding. Defining the
> > bindings in terms of what an OS uses or not is the wrong way around.
>
> I see your point. I'm just afraid that if we start a discussion about
> this now, it will take a long time we get something merged. In the
> meantime we will get a lot of ad-hoc bindings like this one
> suggested for the exynos:
>
> > + lcd_fimd0: lcd_panel0 {
> > + lcd-htiming = <4 4 4 480>;
> > + lcd-vtiming = <4 4 4 320>;
> > + supports-mipi-panel;
> > + };
>
> Or this one for the AMBA LCD controller;
>
> > + panel.mode.refresh = get_val(node, "refresh");
> > + panel.mode.xres = get_val(node, "xres");
> > + panel.mode.yres = get_val(node, "yres");
> > + panel.mode.pixclock = get_val(node, "pixclock");
> > + panel.mode.left_margin = get_val(node, "left_margin");
> > + panel.mode.right_margin = get_val(node, "right_margin");
> > + panel.mode.upper_margin = get_val(node, "upper_margin");
> > + panel.mode.lower_margin = get_val(node, "lower_margin");
> > + panel.mode.hsync_len = get_val(node, "hsync_len");
> > + panel.mode.vsync_len = get_val(node, "vsync_len");
> > + panel.mode.sync = get_val(node, "sync");
> > + panel.bpp = get_val(node, "bpp");
> > + panel.width = (signed short) get_val(node, "width");
> > + panel.height = (signed short) get_val(node, "height");
>
> (get_val() is a wrapper around of_property_read_u32)
>
> BTW the SoC-camera guys will need a wire format description for their
> cameras aswell.
>
> > >
> > > We expect the display nodes being subnodes of a driver (which of course
> > > has a compatible), or maybe referred to from a driver using phandles. So
> > > I don't see why the display node itself should have a compatible
> > > property. The display information is only ever useful in the context of
> > > a driver.
> >
> > A subnode or phandle will describe the h/w connection, but you need a
> > name to describe what is at each end of the connection.
> >
> > Where would the model number of an lcd panel be captured then? The
> > timing parameters are a property of a specific panel, so both should be
> > described together. You may not have any use for the compatible string
> > now, but more information is better than less and adding it would not
> > hurt anything. For pretty much any other device sitting on a board, we
> > describe the manufacturer and type of device. LCD panels should be no
> > different.
>
> You convinced me. Lets add a compatible property, it won't hurt.
Okay. That doesn't seem to be a big problem. I can add that.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* linux-next: manual merge of the arm-soc tree with the fbdev tree
From: Stephen Rothwell @ 2012-09-25 6:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120924191508.6100b0603d2642c1e7a36679@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 836 bytes --]
Hi all,
Today's linux-next merge of the arm-soc tree got a conflict in
drivers/video/msm/mdp_hw.h between commit 8abf0b31e161 ("video: msm:
Remove useless mach/* includes") from the fbdev tree and commit
1ef21f6343ff ("ARM: msm: move platform_data definitions") from the
arm-soc tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/video/msm/mdp_hw.h
index 2a84137,a0bacf5..0000000
--- a/drivers/video/msm/mdp_hw.h
+++ b/drivers/video/msm/mdp_hw.h
@@@ -15,7 -15,8 +15,7 @@@
#ifndef _MDP_HW_H_
#define _MDP_HW_H_
- #include <mach/msm_fb.h>
-#include <mach/msm_iomap.h>
+ #include <linux/platform_data/video-msm_fb.h>
struct mdp_info {
struct mdp_device mdp_dev;
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v2 18/18] OMAPDSS: DISPC: Configure color conversion coefficients for writeback
From: Archit Taneja @ 2012-09-25 6:31 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1348553993-8083-1-git-send-email-archit@ti.com>
Writeback pipeline receives RGB data from one of the overlays or one of the
overlay managers. If the target color mode is YUV422 or NV12, we need to convert
the RGB pixels to YUV. The scaler in WB then converts it to the target color
mode.
Hence, the color conversion coefficients that need to be programmed are the ones
which convert a RGB24 pixel to YUV444. Program these coefficients for writeback
pipeline.
Rearrange the code a bit to configure different coefficients for overlays and
writeback.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 61 +++++++++++++++++++++------------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 034ecb5..1f03690 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -236,6 +236,11 @@ static const struct {
},
};
+struct color_conv_coef {
+ int ry, rcr, rcb, gy, gcr, gcb, by, bcr, bcb;
+ int full_range;
+};
+
static void _omap_dispc_set_irqs(void);
static unsigned long dispc_plane_pclk_rate(enum omap_plane plane);
static unsigned long dispc_plane_lclk_rate(enum omap_plane plane);
@@ -674,41 +679,41 @@ static void dispc_ovl_set_scale_coef(enum omap_plane plane, int fir_hinc,
}
}
-static void _dispc_setup_color_conv_coef(void)
-{
- int i;
- const struct color_conv_coef {
- int ry, rcr, rcb, gy, gcr, gcb, by, bcr, bcb;
- int full_range;
- } ctbl_bt601_5 = {
- 298, 409, 0, 298, -208, -100, 298, 0, 517, 0,
- };
-
- const struct color_conv_coef *ct;
+static void dispc_ovl_write_color_conv_coef(enum omap_plane plane,
+ const struct color_conv_coef *ct)
+{
#define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0))
- ct = &ctbl_bt601_5;
+ dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 0), CVAL(ct->rcr, ct->ry));
+ dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 1), CVAL(ct->gy, ct->rcb));
+ dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 2), CVAL(ct->gcb, ct->gcr));
+ dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 3), CVAL(ct->bcr, ct->by));
+ dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 4), CVAL(0, ct->bcb));
- for (i = 1; i < dss_feat_get_num_ovls(); i++) {
- dispc_write_reg(DISPC_OVL_CONV_COEF(i, 0),
- CVAL(ct->rcr, ct->ry));
- dispc_write_reg(DISPC_OVL_CONV_COEF(i, 1),
- CVAL(ct->gy, ct->rcb));
- dispc_write_reg(DISPC_OVL_CONV_COEF(i, 2),
- CVAL(ct->gcb, ct->gcr));
- dispc_write_reg(DISPC_OVL_CONV_COEF(i, 3),
- CVAL(ct->bcr, ct->by));
- dispc_write_reg(DISPC_OVL_CONV_COEF(i, 4),
- CVAL(0, ct->bcb));
-
- REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(i), ct->full_range,
- 11, 11);
- }
+ REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), ct->full_range, 11, 11);
#undef CVAL
}
+static void dispc_setup_color_conv_coef(void)
+{
+ int i;
+ int num_ovl = dss_feat_get_num_ovls();
+ int num_wb = dss_feat_get_num_wbs();
+ const struct color_conv_coef ctbl_bt601_5_ovl = {
+ 298, 409, 0, 298, -208, -100, 298, 0, 517, 0,
+ };
+ const struct color_conv_coef ctbl_bt601_5_wb = {
+ 66, 112, -38, 129, -94, -74, 25, -18, 112, 0,
+ };
+
+ for (i = 1; i < num_ovl; i++)
+ dispc_ovl_write_color_conv_coef(i, &ctbl_bt601_5_ovl);
+
+ for (; i < num_wb; i++)
+ dispc_ovl_write_color_conv_coef(i, &ctbl_bt601_5_wb);
+}
static void dispc_ovl_set_ba0(enum omap_plane plane, u32 paddr)
{
@@ -3973,7 +3978,7 @@ static void _omap_dispc_initial_config(void)
if (dss_has_feature(FEAT_FUNCGATED))
REG_FLD_MOD(DISPC_CONFIG, 1, 9, 9);
- _dispc_setup_color_conv_coef();
+ dispc_setup_color_conv_coef();
dispc_set_loadmode(OMAP_DSS_LOAD_FRAME_ONLY);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 17/18] OMAPDSS: DISPC: Add manager like functions for writeback
From: Archit Taneja @ 2012-09-25 6:31 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1348553993-8083-1-git-send-email-archit@ti.com>
Add functions to enable writeback, and set/check state of GO bit. These bits are
identical in behaviour with the corresponding overlay manager bits. Configure
them in a similar way to mgr_enable() and mgr_go_* functions. Add a helper to
get the FRAMEDONE irq corresponding to writeback.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 70 +++++++++++++++++++++++++++++++++++++++
drivers/video/omap2/dss/dss.h | 5 +++
2 files changed, 75 insertions(+)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index d6f120d..034ecb5 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -536,6 +536,11 @@ u32 dispc_mgr_get_framedone_irq(enum omap_channel channel)
return mgr_desc[channel].framedone_irq;
}
+u32 dispc_wb_get_framedone_irq(void)
+{
+ return DISPC_IRQ_FRAMEDONEWB;
+}
+
bool dispc_mgr_go_busy(enum omap_channel channel)
{
return mgr_fld_read(channel, DISPC_MGR_FLD_GO) = 1;
@@ -563,6 +568,30 @@ void dispc_mgr_go(enum omap_channel channel)
mgr_fld_write(channel, DISPC_MGR_FLD_GO, 1);
}
+bool dispc_wb_go_busy(void)
+{
+ return REG_GET(DISPC_CONTROL2, 6, 6) = 1;
+}
+
+void dispc_wb_go(void)
+{
+ enum omap_plane plane = OMAP_DSS_WB;
+ bool enable, go;
+
+ enable = REG_GET(DISPC_OVL_ATTRIBUTES(plane), 0, 0) = 1;
+
+ if (!enable)
+ return;
+
+ go = REG_GET(DISPC_CONTROL2, 6, 6) = 1;
+ if (go) {
+ DSSERR("GO bit not down for WB\n");
+ return;
+ }
+
+ REG_FLD_MOD(DISPC_CONTROL2, 1, 6, 6);
+}
+
static void dispc_ovl_write_firh_reg(enum omap_plane plane, int reg, u32 value)
{
dispc_write_reg(DISPC_OVL_FIR_COEF_H(plane, reg), value);
@@ -2690,6 +2719,47 @@ void dispc_mgr_enable(enum omap_channel channel, bool enable)
BUG();
}
+void dispc_wb_enable(bool enable)
+{
+ enum omap_plane plane = OMAP_DSS_WB;
+ struct completion frame_done_completion;
+ bool is_on;
+ int r;
+ u32 irq;
+
+ is_on = REG_GET(DISPC_OVL_ATTRIBUTES(plane), 0, 0);
+ irq = DISPC_IRQ_FRAMEDONEWB;
+
+ if (!enable && is_on) {
+ init_completion(&frame_done_completion);
+
+ r = omap_dispc_register_isr(dispc_disable_isr,
+ &frame_done_completion, irq);
+ if (r)
+ DSSERR("failed to register FRAMEDONEWB isr\n");
+ }
+
+ REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), enable ? 1 : 0, 0, 0);
+
+ if (!enable && is_on) {
+ if (!wait_for_completion_timeout(&frame_done_completion,
+ msecs_to_jiffies(100)))
+ DSSERR("timeout waiting for FRAMEDONEWB\n");
+
+ r = omap_dispc_unregister_isr(dispc_disable_isr,
+ &frame_done_completion, irq);
+ if (r)
+ DSSERR("failed to unregister FRAMEDONEWB isr\n");
+ }
+}
+
+bool dispc_wb_is_enabled(void)
+{
+ enum omap_plane plane = OMAP_DSS_WB;
+
+ return REG_GET(DISPC_OVL_ATTRIBUTES(plane), 0, 0);
+}
+
void dispc_lcd_enable_signal_polarity(bool act_high)
{
if (!dss_has_feature(FEAT_LCDENABLEPOL))
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 442c06a..a71cc88 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -482,6 +482,11 @@ int dispc_mgr_get_clock_div(enum omap_channel channel,
void dispc_mgr_setup(enum omap_channel channel,
struct omap_overlay_manager_info *info);
+u32 dispc_wb_get_framedone_irq(void);
+bool dispc_wb_go_busy(void);
+void dispc_wb_go(void);
+void dispc_wb_enable(bool enable);
+bool dispc_wb_is_enabled(void);
void dispc_wb_set_channel_in(enum dss_writeback_channel channel);
int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
bool mem_to_mem, const struct omap_video_timings *timings);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 16/18] OMAPDSS: DISPC: Configure writeback FIFOs
From: Archit Taneja @ 2012-09-25 6:31 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1348553993-8083-1-git-send-email-archit@ti.com>
Extend the DISPC fifo functions to also configure the writeback FIFO thresholds.
The most optimal configuration for writeback is to push out data to the
interconnect the moment writeback pushes enough pixels in the FIFO to form a
burst. This reduces the chance of writeback overflowing it's FIFO.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index ca28a88..d6f120d 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -993,7 +993,7 @@ void dispc_wb_set_channel_in(enum dss_writeback_channel channel)
static void dispc_ovl_set_burst_size(enum omap_plane plane,
enum omap_burst_size burst_size)
{
- static const unsigned shifts[] = { 6, 14, 14, 14, };
+ static const unsigned shifts[] = { 6, 14, 14, 14, 14, };
int shift;
shift = shifts[plane];
@@ -1225,6 +1225,14 @@ void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
if (manual_update && dss_has_feature(FEAT_OMAP3_DSI_FIFO_BUG)) {
*fifo_low = ovl_fifo_size - burst_size * 2;
*fifo_high = total_fifo_size - burst_size;
+ } else if (plane = OMAP_DSS_WB) {
+ /*
+ * Most optimal configuration for writeback is to push out data
+ * to the interconnect the moment writeback pushes enough pixels
+ * in the FIFO to form a burst
+ */
+ *fifo_low = 0;
+ *fifo_high = burst_size;
} else {
*fifo_low = ovl_fifo_size - burst_size;
*fifo_high = total_fifo_size - buf_unit;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 15/18] OMAPDSS: DISPC: Configure writeback specific parameters in dispc_wb_setup()
From: Archit Taneja @ 2012-09-25 6:31 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1348553993-8083-1-git-send-email-archit@ti.com>
Configure some of the writeback specific parameters in dispc_wb_setup(). The
writeback parameters configured are:
truncation: This needs to be set if the color depth input to writeback is more
than the color depth of the color mode we want to store in memory.
writeback mode: This configures whether we want to use writeback in mem to mem
or capture mode. This information will be directly passed by APPLY later.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 28 ++++++++++++++++++++++++++--
drivers/video/omap2/dss/dss.h | 2 +-
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 2e5f033..ca28a88 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2480,13 +2480,15 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
}
int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
- const struct omap_video_timings *mgr_timings)
+ bool mem_to_mem, const struct omap_video_timings *mgr_timings)
{
int r;
+ u32 l;
enum omap_plane plane = OMAP_DSS_WB;
const int pos_x = 0, pos_y = 0;
const u8 zorder = 0, global_alpha = 0;
const bool replication = false;
+ bool truncation;
int in_width = mgr_timings->x_res;
int in_height = mgr_timings->y_res;
enum omap_overlay_caps caps @@ -2501,7 +2503,29 @@ int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
wi->buf_width, pos_x, pos_y, in_width, in_height, wi->width,
wi->height, wi->color_mode, wi->rotation, wi->mirror, zorder,
wi->pre_mult_alpha, global_alpha, wi->rotation_type,
- replication, mgr_timings, false);
+ replication, mgr_timings, mem_to_mem);
+
+ switch (wi->color_mode) {
+ case OMAP_DSS_COLOR_RGB16:
+ case OMAP_DSS_COLOR_RGB24P:
+ case OMAP_DSS_COLOR_ARGB16:
+ case OMAP_DSS_COLOR_RGBA16:
+ case OMAP_DSS_COLOR_RGB12U:
+ case OMAP_DSS_COLOR_ARGB16_1555:
+ case OMAP_DSS_COLOR_XRGB16_1555:
+ case OMAP_DSS_COLOR_RGBX16:
+ truncation = true;
+ break;
+ default:
+ truncation = false;
+ break;
+ }
+
+ /* setup extra DISPC_WB_ATTRIBUTES */
+ l = dispc_read_reg(DISPC_OVL_ATTRIBUTES(plane));
+ l = FLD_MOD(l, truncation, 10, 10); /* TRUNCATIONENABLE */
+ l = FLD_MOD(l, mem_to_mem, 19, 19); /* WRITEBACKMODE */
+ dispc_write_reg(DISPC_OVL_ATTRIBUTES(plane), l);
return r;
}
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 1a09f00..442c06a 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -484,7 +484,7 @@ void dispc_mgr_setup(enum omap_channel channel,
void dispc_wb_set_channel_in(enum dss_writeback_channel channel);
int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
- const struct omap_video_timings *timings);
+ bool mem_to_mem, const struct omap_video_timings *timings);
/* VENC */
#ifdef CONFIG_OMAP2_DSS_VENC
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 14/18] OMAPDSS: DISPC: Configure overlay-like parameters in dispc_wb_setup
From: Archit Taneja @ 2012-09-25 6:31 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1348553993-8083-1-git-send-email-archit@ti.com>
Create struct omap_dss_writeback_info, this is similar to omap_overlay_info,
the major difference is that there is no parameter which describes the input
size to writeback, this is because this is always fixed, and decided by the
connected overlay or overlay manager. One more difference is that screen_width
is renamed to buf_width, to give the value of stride the writeback buffer has.
Call dispc_ovl_setup_common() through dispc_wb_setup() to configure overlay-like
parameters. The parameters in dispc_ovl_setup_common() which do not hold for
writeback are filled passed as zeroes or false, the code takes care of not
configuring them as they won't possess the needed overlay caps.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 27 +++++++++++++++++++++++++++
drivers/video/omap2/dss/dss.h | 2 ++
include/video/omapdss.h | 13 +++++++++++++
3 files changed, 42 insertions(+)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 6d482f5..2e5f033 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2479,6 +2479,33 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
return r;
}
+int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
+ const struct omap_video_timings *mgr_timings)
+{
+ int r;
+ enum omap_plane plane = OMAP_DSS_WB;
+ const int pos_x = 0, pos_y = 0;
+ const u8 zorder = 0, global_alpha = 0;
+ const bool replication = false;
+ int in_width = mgr_timings->x_res;
+ int in_height = mgr_timings->y_res;
+ enum omap_overlay_caps caps + OMAP_DSS_OVL_CAP_SCALE | OMAP_DSS_OVL_CAP_PRE_MULT_ALPHA;
+
+ DSSDBG("dispc_wb_setup, pa %x, pa_uv %x, %d,%d -> %dx%d, cmode %x, "
+ "rot %d, mir %d\n", wi->paddr, wi->p_uv_addr, in_width,
+ in_height, wi->width, wi->height, wi->color_mode, wi->rotation,
+ wi->mirror);
+
+ r = dispc_ovl_setup_common(plane, caps, wi->paddr, wi->p_uv_addr,
+ wi->buf_width, pos_x, pos_y, in_width, in_height, wi->width,
+ wi->height, wi->color_mode, wi->rotation, wi->mirror, zorder,
+ wi->pre_mult_alpha, global_alpha, wi->rotation_type,
+ replication, mgr_timings, false);
+
+ return r;
+}
+
int dispc_ovl_enable(enum omap_plane plane, bool enable)
{
DSSDBG("dispc_enable_plane %d, %d\n", plane, enable);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 872e420..1a09f00 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -483,6 +483,8 @@ void dispc_mgr_setup(enum omap_channel channel,
struct omap_overlay_manager_info *info);
void dispc_wb_set_channel_in(enum dss_writeback_channel channel);
+int dispc_wb_setup(const struct omap_dss_writeback_info *wi,
+ const struct omap_video_timings *timings);
/* VENC */
#ifdef CONFIG_OMAP2_DSS_VENC
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 46097bd..3729173 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -510,6 +510,19 @@ struct omap_dsi_pin_config {
int pins[OMAP_DSS_MAX_DSI_PINS];
};
+struct omap_dss_writeback_info {
+ u32 paddr;
+ u32 p_uv_addr;
+ u16 buf_width;
+ u16 width;
+ u16 height;
+ enum omap_color_mode color_mode;
+ u8 rotation;
+ enum omap_dss_rotation_type rotation_type;
+ bool mirror;
+ u8 pre_mult_alpha;
+};
+
struct omap_dss_output {
struct list_head list;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 13/18] OMAPDSS: DISPC: Add function to set channel in for writeback
From: Archit Taneja @ 2012-09-25 6:31 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1348553993-8083-1-git-send-email-archit@ti.com>
Writeback can take input from either one of the overlays, or one of the overlay
managers. Add an enum which represents the channel_in for writeback, and maps
to the register field programming.
Add a function to configure channel in for writeback. This will be used later in
APPLY.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 7 +++++++
drivers/video/omap2/dss/dss.h | 13 +++++++++++++
2 files changed, 20 insertions(+)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index d9ca7bf..6d482f5 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -983,6 +983,13 @@ static enum omap_channel dispc_ovl_get_channel_out(enum omap_plane plane)
return channel;
}
+void dispc_wb_set_channel_in(enum dss_writeback_channel channel)
+{
+ enum omap_plane plane = OMAP_DSS_WB;
+
+ REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), channel, 18, 16);
+}
+
static void dispc_ovl_set_burst_size(enum omap_plane plane,
enum omap_burst_size burst_size)
{
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 04a1eda..872e420 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -113,6 +113,17 @@ enum dss_dsi_content_type {
DSS_DSI_CONTENT_GENERIC,
};
+enum dss_writeback_channel {
+ DSS_WB_LCD1_MGR = 0,
+ DSS_WB_LCD2_MGR = 1,
+ DSS_WB_TV_MGR = 2,
+ DSS_WB_OVL0 = 3,
+ DSS_WB_OVL1 = 4,
+ DSS_WB_OVL2 = 5,
+ DSS_WB_OVL3 = 6,
+ DSS_WB_LCD3_MGR = 7,
+};
+
struct dss_clock_info {
/* rates that we get with dividers below */
unsigned long fck;
@@ -471,6 +482,8 @@ int dispc_mgr_get_clock_div(enum omap_channel channel,
void dispc_mgr_setup(enum omap_channel channel,
struct omap_overlay_manager_info *info);
+void dispc_wb_set_channel_in(enum dss_writeback_channel channel);
+
/* VENC */
#ifdef CONFIG_OMAP2_DSS_VENC
int venc_init_platform_driver(void) __init;
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox