Bug 3123

Summary:	Patch to kernel to allow DSP framebuffer sharing on N8x0 devices
Product:	[Maemo Official Platform] Core	Reporter:	Simon Pickering <S.G.Pickering>
Component:	Kernel	Assignee:	unassigned <nobody>
Status:	RESOLVED WONTFIX	QA Contact:	linux-kernel-bugs
Severity:	normal
Priority:	Medium	CC:	andre_klapper, carsten.munk, dominikowski, maemo, S.G.Pickering, siarhei.siamashka
Version:	4.0	Keywords:	community-diablo, patch
Target Milestone:	---
Hardware:	All
OS:	Linux
Attachments:	Patch to dsp_mem.c (post Nokia patches) to allow DSP fb sharing

Description Simon Pickering (reporter) maemo.org

2008-04-29 17:22:29 UTC

Kernel version: kernel-source-rx-34_2.6.21.0-osso71

The attached patch changes two parts of the code in dsp_mem.c

First part is a patch to mbox_fbctl_upd() to pass the correct structure to
omapfb_update_window_async(). In the current form the code passes a
omapfb_update_window structure (defined in omapfb.h) but does not fill in the
last 4 elements (out_x, out_y, out_width, out_height). This is presumably as
these elements were not required for the HWA742 device found in the 770, which
was the last device to enable DSP framebuffer sharing. The patch fills in these
elements from the data passed across from the DSP.

Obviously if these elements are filled with random data it screws up the screen
updates from the DSP.

The second part of the patch is to dsp_fbexport(), also in dsp_mem.c. This
change rounds the size of the framebuffer up to 0x100000 (if smaller than that)
as otherwise there are too few TLBs available to map the exact framebuffer size
(whereas a size of 0x100000 consumes a single TLB). I note that in the 770's
kernel, the framebuffer is reported as being 0x100000 in size. I thought this
change would be less intrusive as it only affects the page size when the
framebuffer is being shared.

These changes won't make any odds to most people, but if they can be included
it will allow those of us who'd like to play with the DSP to provide code that
works out of the box (other than needing to add a FRAMEBUFFER section to
/lib/dsp/avs_kernelcfg.cmd, but that's more trivial than flashing a new
kernel).

Comment 1 Simon Pickering (reporter) maemo.org

2008-04-29 17:23:47 UTC

Created an attachment (id=758) [details]
Patch to dsp_mem.c (post Nokia patches) to allow DSP fb sharing

Comment 2 Andre Klapper maemo.org

2008-05-05 15:02:21 UTC

(In general, please add the "patch" keyword when adding a patch. Thanks.)

Comment 3 Siarhei Siamashka 2008-06-20 13:38:29 UTC

Just out of curiosity. Why is the following change needed?

     }
     padr_sys = registered_fb[0]->fix.smem_start;
     fbsz_sys = registered_fb[0]->fix.smem_len;
+    
+    if(fbsz_sys<=0x100000) fbsz_sys=0x100000; // added patch here
+    
     if (fbsz_sys == 0) {
         printk(KERN_ERR
                "omapdsp: framebuffer doesn't seem to be configured "

Because with this line added, the next 'if' statement would be never evaluated
as true.

Comment 4 Simon Pickering (reporter) maemo.org

2008-06-20 14:00:43 UTC

Good point, I should probably change it to:

if(fbsz_sys<=0x100000 & fbsz_sys>0x0)

This shouldn't be a problem for the Nokia devices though (and this is a patch
to their patches) as they all (currently) have framebuffers.

Comment 5 Simon Pickering (reporter) maemo.org

2008-06-20 14:03:40 UTC

Actually, I seem to remember that the kernel on the N8x0 lies about the
framebuffer size (and says its size = 0) to avoid sharing it.

I'll have to take another look at the code to remember.

Sorry.

Comment 6 Andre Klapper maemo.org

2008-09-23 14:27:50 UTC

Simon: Interested in coming up with an updated patch (see comments 4 and 5) so
this can be handled?

Comment 7 Andre Klapper maemo.org

2009-02-04 17:43:38 UTC

Anybody able to update/whether this still makes sense/applies for Fremantle
pre-alpha2 code?

Comment 8 Andre Klapper maemo.org

2009-05-28 13:33:30 UTC

Anybody able to update/whether this still makes sense/applies for Fremantle SDK
beta code?
Or is this Chinook/Diablo only?

Comment 9 Simon Pickering (reporter) maemo.org

2009-05-31 20:06:23 UTC

Sorry for the slow response.

This is specific to the DSP Gateway and the LCD controller used in the N8x0
machines. I hope the same LCD controller won't be used, and the DSP Gateway
certainly won't be used for the Fremantle devices, so no this has no bearing on
the Fremantle stuff.

Comment 10 Andre Klapper maemo.org

2009-06-04 20:26:55 UTC

> so no this has no bearing on the Fremantle stuff.

Okay, I know this is disappointing (and hopefully not also demotivating) but
WONTFIX for Diablo as Diablo is in maintenance mode and Nokia will only provide
bugfixes for critical issues if at all.
CC'ing Carsten as the Mer project aims to provide a community backport of
Fremantle for N8x0 devices and we have a nice patch here that could be
interesting for Mer. See http://wiki.maemo.org/Mer for more information.

Comment 11 Simon Pickering (reporter) maemo.org

2009-06-04 21:25:24 UTC

No, that's fine Andre, I expected that and am quite happy about it.

I'll have a chat with Carsten about adding this sort of patch to the Mer kernel
once I'm back from honeymoon.

Main priority re. DSP would be ARM-side audio codec drivers mind you, so we can
cut the DSP free and use it for something without requiring the Nokia DSP
kernel binary, but that's for another bug (though a quiet word might not go
amiss ;)).

Comment 12 Tomasz Dominikowski 2009-07-12 13:30:19 UTC

Patch added to kernel-source-diablo, in N8x0:Testing, should be a part of the
next release of Mer (0.15).

Comment 13 Lucas Maneos 2009-10-22 07:57:14 UTC

Marking patches of interest to Diablo (Maemo4) community updates, please excuse
the noise.