[PATCH 2/3] OMAP: DSS2: Fix FIFO Threshold calculations for dispc pipelines

[Archit Taneja]

On OMAP4, one unit in DISPC_(VIDp/GFX)_BUF_THRESHOLD and BUF_SIZE_STATUS
corrsponds to 128 bits of FIFO Size compared to 8 bits on OMAP3. Fix this.

Signed-off-by: Archit Taneja <arc...@ti.com>
---
drivers/video/omap2/dss/display.c | 25 +++++++++++++++----------
drivers/video/omap2/dss/dsi.c | 20 +++++++++++++++-----
2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
index d2b0b69..f9afe6a 100644
--- a/drivers/video/omap2/dss/display.c
+++ b/drivers/video/omap2/dss/display.c
@@ -376,17 +376,22 @@ void default_get_overlay_fifo_thresholds(enum omap_plane plane,
u32 fifo_size, enum omap_burst_size *burst_size,
u32 *fifo_low, u32 *fifo_high)
{
- unsigned burst_size_bytes;
-
- *burst_size = OMAP_DSS_BURST_16x32;
- if (cpu_is_omap44xx())
- burst_size_bytes = 8 * 128 / 8; /* OMAP4: highest
- burst size is 8x128*/
- else
- burst_size_bytes = 16 * 32 / 8;
-
+ unsigned burst_size_reg;
+
+ *burst_size = OMAP_DSS_BURST_16x32; /* 8 x 128 bits on OMAP4 */
+ /* On omap4, one unit in the BUF_SIZE_STATUS
+ * and BUF_THRESHOLD registers represent
+ * 128 bits in comparison to 8 bits on omap3
+ */
+ if (cpu_is_omap44xx()) {
+ /* Unit is 128 bits */
+ burst_size_reg = 8 * 128 / 128;
+ } else {
+ /* Unit is bytes*/
+ burst_size_reg = 16 * 32 / 8;
+ }
*fifo_high = fifo_size - 1;
- *fifo_low = fifo_size - burst_size_bytes;
+ *fifo_low = fifo_size - burst_size_reg;
}

int omapdss_default_get_recommended_bpp(struct omap_dss_device *dssdev)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 06ca944..36b7b72 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -3780,13 +3780,23 @@ void dsi_get_overlay_fifo_thresholds(enum omap_plane plane,
u32 fifo_size, enum omap_burst_size *burst_size,
u32 *fifo_low, u32 *fifo_high)
{
- unsigned burst_size_bytes;
+ unsigned burst_size_reg;

- *burst_size = OMAP_DSS_BURST_16x32;
- burst_size_bytes = 16 * 32 / 8;
+ *burst_size = OMAP_DSS_BURST_16x32; /* 8 x 128 bits on OMAP4 */
+ /* On omap4, one unit in the BUF_SIZE_STATUS
+ * and BUF_THRESHOLD registers represent
+ * 128 bits in comparison to 8 bits on omap3
+ */
+ if (cpu_is_omap44xx()) {
+ /* Unit is 128 bits */
+ burst_size_reg = 8 * 128 / 128;
+ } else {
+ /* Unit is bytes*/
+ burst_size_reg = 16 * 32 / 8;
+ }

- *fifo_high = fifo_size - burst_size_bytes;
- *fifo_low = fifo_size - burst_size_bytes * 2;
+ *fifo_high = fifo_size - burst_size_reg;
+ *fifo_low = fifo_size - burst_size_reg * 2;
}

int dsi_init_display(struct omap_dss_device *dssdev)
--
1.7.0.4

[Archit Taneja]

From: Pavel Nedev <pne...@ti.com>

Signed-off-by: Pavel Nedev <pne...@ti.com>
---
drivers/video/omap2/dss/dispc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index a53b509..a7c5620 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -283,7 +283,7 @@ struct dispc_irq_stats {
static struct {
void __iomem *base;

- u32 fifo_size[3];
+ u32 fifo_size[5];

spinlock_t irq_lock;
u32 irq_error_mask;
--
1.7.0.4

[Archit Taneja]

Use Writeback buffers to prevent GFX underflow, make a sysfs entry
which lets the user give buffers back to Writeback pipe.

Signed-off-by: Archit Taneja <arc...@ti.com>
---

arch/arm/plat-omap/include/plat/display.h | 2 +-
drivers/video/omap2/dss/dispc.c | 27 +++++++++++
drivers/video/omap2/dss/dss.h | 1 +
drivers/video/omap2/dss/wb.c | 70 +++++++++++++++++++++++++++-
4 files changed, 96 insertions(+), 4 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/display.h b/arch/arm/plat-omap/include/plat/display.h
index 122cea0..c5423c9 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -516,6 +516,7 @@ struct omap_writeback_info {
unsigned long paddr;
/* NV12 support*/
unsigned long puv_addr;
+ bool buffer_state;

};

@@ -532,7 +533,6 @@ struct omap_writeback {

int (*set_wb_info)(struct omap_writeback *wb, struct omap_writeback_info *info);
void (*get_wb_info)(struct omap_writeback *wb, struct omap_writeback_info *info);
-
};

struct omap_dss_device {
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index a7c5620..8a1ab50 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1789,6 +1789,13 @@ void dispc_setup_plane_fifo(enum omap_plane plane, u32 low, u32 high)
dispc_write_reg(ftrs_reg[plane],
FLD_VAL(high, 31, 16) | FLD_VAL(low, 15, 0));

+ /* Setting this bit to 1 gives issues on omap3,
+ * doing this only for omap4 for now

+ */
+ if (cpu_is_omap44xx()) {

+ u8 bit = plane == OMAP_DSS_GFX ? 11 : 19;
+ REG_FLD_MOD(dispc_reg_att[plane], 0x1, bit, bit);
+ }
enable_clocks(0);
}

@@ -4825,6 +4832,9 @@ static void _omap_dispc_initial_config(void)
dispc_set_loadmode(OMAP_DSS_LOAD_FRAME_ONLY);

dispc_read_plane_fifo_sizes();
+
+ if (cpu_is_omap44xx())
+ dispc_move_wb_buffers(false);
}

int dispc_init(struct platform_device *pdev)
@@ -4944,6 +4954,23 @@ void change_base_address(int plane, u32 p_uv_addr)
}

/* Writeback*/
+int dispc_move_wb_buffers(bool buffer_state)
+{
+ if (buffer_state) {
+ /* return buffers to writeback pipeline */
+ REG_FLD_MOD(DISPC_GLOBAL_BUFFER, 0x4, 26, 24);
+ REG_FLD_MOD(DISPC_GLOBAL_BUFFER, 0x4, 29, 26);
+ dispc_write_reg(DISPC_GFX_FIFO_THRESHOLD,
+ FLD_VAL(0x4FF, 31, 16) | FLD_VAL(0x4F8, 15, 0));
+ } else {
+ /* Only move buffers to GFX for now */
+ REG_FLD_MOD(DISPC_GLOBAL_BUFFER, 0x0, 29, 24);
+ dispc_write_reg(DISPC_GFX_FIFO_THRESHOLD,
+ FLD_VAL(0xCFF, 31, 16) | FLD_VAL(0xCF8, 15, 0));
+ }
+ return 0;
+}
+
int dispc_setup_wb(struct writeback_cache_data *wb)
{
unsigned long tiler_width, tiler_height;
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 569d0b2..9a880c4 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -512,6 +512,7 @@ int dispc_get_clock_div(enum omap_channel channel,
void dispc_go_wb(void);
void dispc_flush_wb(struct writeback_cache_data *wb);
int dispc_setup_wb(struct writeback_cache_data *wb);
+int dispc_move_wb_buffers(bool buffer_state);

/* VENC */
#ifdef CONFIG_OMAP2_DSS_VENC
diff --git a/drivers/video/omap2/dss/wb.c b/drivers/video/omap2/dss/wb.c
index 530ce68..b049c40 100644
--- a/drivers/video/omap2/dss/wb.c
+++ b/drivers/video/omap2/dss/wb.c
@@ -35,20 +35,85 @@

static struct list_head wb_list;

+static ssize_t wb_buffer_state_show(struct omap_writeback *wb, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "%d\n", wb->info.buffer_state);
+}
+
+static ssize_t wb_buffer_state_store(struct omap_writeback *wb, const char *buf,
+ size_t size)
+{
+ int r;
+ struct omap_writeback_info info;
+
+ if (!dss_get_mainclk_state()) {
+ DSSERR("mainclk disabled while trying"
+ "wb_buffer_state_store, returning\n");
+ return 0;
+ }
+
+ wb->get_wb_info(wb, &info);
+
+ info.buffer_state = simple_strtoul(buf, NULL, 10);
+
+ r = dispc_move_wb_buffers(info.buffer_state);
+ if (r) {
+ DSSERR("failed to move buffers\n");
+ return r;
+ }
+ r = wb->set_wb_info(wb, &info);
+ if (r)
+ return r;
+
+ return size;
+}
+
+struct wb_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct omap_writeback *, char *);
+ ssize_t (*store)(struct omap_writeback *, const char *, size_t);
+};
+
+#define WB_ATTR(_name, _mode, _show, _store) \
+ struct wb_attribute wb_attr_##_name = \
+ __ATTR(_name, _mode, _show, _store)
+
+static WB_ATTR(buffer_state, S_IRUGO|S_IWUSR,
+ wb_buffer_state_show, wb_buffer_state_store);
+
static struct attribute *writeback_sysfs_attrs[] = {
+ &wb_attr_buffer_state.attr,
NULL
};

static ssize_t writeback_attr_show(struct kobject *kobj, struct attribute *attr,
char *buf)
{
-return 0;
+ struct omap_writeback *wb;
+ struct wb_attribute *wb_attr;
+
+ wb = container_of(kobj, struct omap_writeback, kobj);
+ wb_attr = container_of(attr, struct wb_attribute, attr);
+
+ if (!wb_attr->show)
+ return -ENOENT;
+
+ return wb_attr->show(wb, buf);
}

static ssize_t writeback_attr_store(struct kobject *kobj, struct attribute *attr,
const char *buf, size_t size)
{
-return 0;
+ struct omap_writeback *wb;
+ struct wb_attribute *wb_attr;
+
+ wb = container_of(kobj, struct omap_writeback, kobj);
+ wb_attr = container_of(attr, struct wb_attribute, attr);
+
+ if (!wb_attr->store)
+ return -ENOENT;
+
+ return wb_attr->store(wb, buf, size);
}

static struct sysfs_ops writeback_sysfs_ops = {
@@ -175,5 +240,4 @@ void dss_init_writeback(struct platform_device *pdev)
if (r) {
DSSERR("failed to create sysfs file\n");
}
-
}
--
1.7.0.4

[Steve Sakoman]

On Tue, Nov 9, 2010 at 9:39 PM, Archit Taneja <arc...@ti.com> wrote:
> Use Writeback buffers to prevent GFX underflow, make a sysfs entry
> which lets the user give buffers back to Writeback pipe.

What repository/branch is this patch based on?

It will not apply to the ubuntu-maverick repo.

Steve

[Vladimir Pantelic]

Steve Sakoman wrote:
> On Tue, Nov 9, 2010 at 9:39 PM, Archit Taneja<arc...@ti.com> wrote:
>> Use Writeback buffers to prevent GFX underflow, make a sysfs entry
>> which lets the user give buffers back to Writeback pipe.
>
> What repository/branch is this patch based on?
>
> It will not apply to the ubuntu-maverick repo.

is ubuntu-maverick *the* omap4 repo?

[Vincent Stehlé]

On 11/10/2010 05:23 PM, Steve Sakoman wrote:
> It will not apply to the ubuntu-maverick repo.

It does if you remove:

>> + if (!dss_get_mainclk_state()) {
>> + DSSERR("mainclk disabled while trying"
>> + "wb_buffer_state_store, returning\n");
>> + return 0;
>> + }

And I think it makes things better; I have no fifo underflow since a few
hours.

Best regards,

--
Vincent Stehl�
Systems Engineer - TI France

[Jayabharath Goluguri]

Nope. Though in theory kernel.org is 'the' repository - we are not yet
fully there for OMAP4.

As of now http://dev.omapzoom.org/?p=integration/kernel-omap4.git (the
integration kernel tree) can be considered as the latest repo with
cutting edge & tested features.

ubuntu-kernel repo is 'derived' from intergration kernel with some ubutu
specific patches.

Gitorious kernel (gitorious.org/pandaboard) is a board validation kernel
tree with some experimental work and is derived from integration kernel
tree.

Hope it clarifies (or confuse)

--jay

--

[Vincent Stehlé]

On 11/10/2010 05:45 PM, Jayabharath Goluguri wrote:
> ubuntu-kernel repo is 'derived' from intergration kernel with some ubutu
> specific patches.

It is available here:
http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-maverick.git

For Panda you will want the ti-omap4 branch.

[Steve Sakoman]

On Tue, Nov 9, 2010 at 9:38 PM, Archit Taneja <arc...@ti.com> wrote:
> Underflows are seen for higher resolutions for GFX pipe,
> Use Writeback buffers for GFX pipeline as a temporary fix.
>
> Archit Taneja (2):
>  OMAP: DSS2: Fix FIFO Threshold calculations for dispc pipelines
>  OMAP: DSS2: Use Writeback buffers for GFX as a temporary fix
>
> Pavel Nedev (1):
>  OMAP: DSS2: Fix array size which stores dispc fifo sizes
>
>  arch/arm/plat-omap/include/plat/display.h |    2 +-
>  drivers/video/omap2/dss/dispc.c           |   29 +++++++++++-
>  drivers/video/omap2/dss/display.c         |   25 ++++++----
>  drivers/video/omap2/dss/dsi.c             |   20 ++++++--

>  drivers/video/omap2/dss/dss.h             |    1 +
>  drivers/video/omap2/dss/wb.c              |   70 +++++++++++++++++++++++++++-

>  6 files changed, 127 insertions(+), 20 deletions(-)

Tested-by: Steve Sakoman <st...@sakoman.com>

Had to modify the third patch a bit to comment out a call to a
function that doesn't exist. The repo I built from for testing was:

http://www.sakoman.com/cgi-bin/gitweb.cgi?p=ubuntu-maverick.git;a=shortlog;h=refs/heads/omap-2.6.35

[Nishanth Menon]

Archit Taneja wrote, on 11/09/2010 11:38 PM:
> From: Pavel Nedev<pne...@ti.com>
am going to be a nit pick and ask for commit message here ;) why do we
need to change from 3 to 5?

--
Regards,
Nishanth Menon

[archit]

I made the patches based on the L24.11 branch of integration/kernel-
omap4 on dev.omapzoom.org

[Taneja, Archit]

Hi,

Nishanth Menon wrote:
> Archit Taneja wrote, on 11/09/2010 11:38 PM:
>> From: Pavel Nedev<pne...@ti.com>
> am going to be a nit pick and ask for commit message here ;)
> why do we need to change from 3 to 5?

The git commit in the patch should have been more descriptive, its my bad :|
The fifo_size is a member of global dispc struct which stores the fifo sizes
for each pipeline in DISPC. On OMAP3, there were 3 pipelines(GFX, VID1, VID2)
and on OMAP4 it increased to 5 with the addition of VID3 and WB.
This is the reason for the change.

[Nishanth Menon]

Taneja, Archit wrote, on 11/11/2010 12:31 AM:
> Hi,
>
> Nishanth Menon wrote:
>> Archit Taneja wrote, on 11/09/2010 11:38 PM:
>>> From: Pavel Nedev<pne...@ti.com>
>> am going to be a nit pick and ask for commit message here ;)
>> why do we need to change from 3 to 5?
>
> The git commit in the patch should have been more descriptive, its my bad :|
> The fifo_size is a member of global dispc struct which stores the fifo sizes
> for each pipeline in DISPC. On OMAP3, there were 3 pipelines(GFX, VID1, VID2)
> and on OMAP4 it increased to 5 with the addition of VID3 and WB.
> This is the reason for the change.

Thanks. but then every time we get a new OMAP with more pipes, we'd be
hardcodign numbers.. mebbe we should at least be using #defines and
places where dereferencing is done such as fifo_size[i] - it'd be
important to use the same macro to check on the limits.

>
>>
>>>
>>> Signed-off-by: Pavel Nedev<pne...@ti.com>
>>> ---
>>> drivers/video/omap2/dss/dispc.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/dss/dispc.c
>>> b/drivers/video/omap2/dss/dispc.c index a53b509..a7c5620 100644
>>> --- a/drivers/video/omap2/dss/dispc.c
>>> +++ b/drivers/video/omap2/dss/dispc.c
>>> @@ -283,7 +283,7 @@ struct dispc_irq_stats {
>>> static struct {
>>> void __iomem *base;
>>>
>>> - u32 fifo_size[3];
>>> + u32 fifo_size[5];
>>>
>>> spinlock_t irq_lock;
>>> u32 irq_error_mask;

--
Regards,
Nishanth Menon

[Nishanth Menon]

Taneja, Archit wrote, on 11/11/2010 05:49 AM:
>> but then every time we get a new OMAP with more
>> > pipes, we'd be hardcodign numbers.. mebbe we should at least
>> > be using #defines and places where dereferencing is done such
>> > as fifo_size[i] - it'd be important to use the same macro to check on the
>> > limits.

> I agree with this, I have taken your approach with the dss_features and consequent
> patches which I am pushing to the lo-dss2 tree, we haven't backported these yet to
> our L24x kernel yet.
>
> Macros MAX_DSS_OVERLAYS and MAX_DSS_MANAGERS are in dss_features.h which will be used
> across DSS2 code.
>
Thanks... I guess the next kernel migration will get these fixes.. :)

--
Regards,
Nishanth Menon

[Ricardo Salveti]

Even after applying these patches I can easily reproduce this issue at
my ES2.1 by running "$ x11perf -copywinwin500".

At least it gets better, and I now can use PVR to run 3D applications
without easily creating a fifo underflow.
--
Ricardo Salveti de Araujo

[Vladimir Pantelic]

pardon my ignorance, but is this fifo underflow a bug in 2.0/2.1 or why
do we have to "fix" that at all? Shouldn't the OMAP4 DSS be capable to
handle these use cases without "workarounds"?