Re: MMP display subsystem questions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Daniel,

Thank you for using our soc/drivers and thank you for your suggestions.
Please see our comments below.

On 06/20/2013 01:26 AM, Daniel Drake wrote:
Is there a design document available somewhere? There are very few
explanatory comments in the code. I am interested in definitions and
explanations of:
  - Paths
  - Overlays
  - dmafetch (part of overlay configuration)
  - path_config (part of path configuration)
  - link_config (part of path configuration)

Looking at the code for answers, I am a little confused.
I admit it's an issue of no documents. We would submit a patch to add description under Documentation soon later.

A path is an output device (e.g. a panel or TV). There can be multiple
outputs connected to the display controller. At the moment I am just
working with a single dumb panel.

A path can have many overlays, and according to the original commit
message, an overlay is a buffer displayed on the path's output device.
However, I simply cannot see how multiple overlays would be supported
on a path. For example let's look at this function:

static int overlay_set_addr(struct mmp_overlay *overlay, struct mmp_addr *addr)
{
struct lcd_regs *regs = path_regs(overlay->path);

/* FIXME: assert addr supported */
memcpy(&overlay->addr, addr, sizeof(struct mmp_win));
writel(addr->phys[0], &regs->g_0);

return overlay->addr.phys[0];
}

The overlay's address is written to a register that is specific to
it's parent path. If we are dealing with two overlays, how does this
work? We write both overlay addresses to the same register and the
second one "wins"? I checked all of the other overlay-related
functions and they all defer to the parent path as well.

As an experiment I modified platform data to suggest that my path has
98 overlays and the framebuffer should run on overlay 33. Nonsense,
but the framebuffer booted up as normal. It seems like there is
something missing in this path/overlay relationship.

What is the plan going forward for this overlay management? At the
moment there is only one consumer of overlays in the kernel - the
framebuffer driver. And the framebuffer will only ever use one. Who
are the other intended in-kernel users of overlays?
We have already pushed patches to fix this issue and support overlays. Please see patches below:
[V2 7/7] video: mmp: add video layer set win/addr operations support
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/175071.html

Moving onto dmafetch_id. Can't find any documentation here. It looks
like the only meaningful values are 0 and 1, and the only difference
is that overlay_is_vid() returns 1 if dmafetch_id is 1, and 0
otherwise. This causes some minor differences in the way registers get
programmed, but I don't understand exactly what the consequences are
here. My framebuffer works with dmafetch_id as both 0 and 1.
Yes it's a legacy config there. Please ignore it. We would submit a new patch totally remove it soon later.

path_config: this value gets written to registers LCD_SCLK *and*
IOPAD_CONTROL. The bit definitions in these registers are totally
different. It seems like nonsense to share the same configuration
value between two diversely different registers - what is going on
here?

link_config: Seems to have a dual meaning. Upper 4 bits can be used to
specify dumb panel mode in registers like LCD_DUMB_CTRL. Bit 0 is
totally unrelated and triggers "swap RB" i.e. use BGR instead of RGB,
when dealing with totally different registers. Other bits are ignored.
If "disorganised" bitfields are to be haphazardly invented in this way
it would at least be nice to have documentation.
Actually we are using mixed configs here and it is not so clear.
We may update these config to separated configs with more clearly meanings.
Currently 4 fields is here:
SCLK source: select source clock for sclk. We have plan to remove it and move the source selection into common clock driver.
IOPAD CONTROL: for iopad control
DUMB CONTROL: for dumb panel mode
RB SWAP: for rb swaps in link. Actually it's do required as for some panels would swap r/b links. We have 2 swaps there, one is after dma fetch for each overlay and another is in output after overlay mixing. We could not simply use bgr format as input might be yuv for which yvu would not simple converted to bgr.
Also for input/output swap distinguish, a fix patch is there:
[V2 1/7] video: mmp: rb swap setting update for new LCD driver
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/175070.html
We would push patches to make these configs more clear soon later and also these description would be added into adding Documentation patch.

Moving onto the devicetree definition. It is clear that representing
the display controller and the panel in the device tree makes sense;
this can cause the appropriate probes to happen. But we additionally
need to trigger the probe of the framebuffer driver. I am not sure if
the framebuffer that Linux will setup is something that should be
represented in the device tree (remember that we already have display
controller and panel). Looking at the configuration items needed to
setup the framebuffer:

1. Name - doesn't really matter.

2. Pixel format - surely this question applies to all framebuffer
drivers. I guess there is a default, plus a standard framebuffer
interface that allows it to be changed?
Yes you are right, it's just a default value and would be changed after system boot.

3. Which path to use - how do other framebuffer drivers deal with
this? The question here is basically do we output to the laptop's
inbuilt LCD panel, or to the externally connected HDMI TV? Again,
maybe we can choose a sensible default and allow runtime
configuration.
Actually in our usage mode, we'd rather prefer to make one fb connected to one path. In many usage mode, we may need 2 or more devices (panel, external tv...) turned on together so 2 or more fb devices are required to connect to separate path.

4. Which overlay to use - according to my investigation above, this
doesn't actually have any meaningful effect on the driver.
As overlay enabled in patch above now, this config is required to make fb show on some overlay and other interface(v4l2, private interface) to use others.

5. dmafetch_id - its not clear to me what this is.


So, I don't think the framebuffer is something that should be defined
by the platform (or the device tree). Instead, maybe it should just be
initialized with good defaults when the first path becomes ready for
use.
So according to comments above, fb buffer is still need to be configure in dts for path/overlay configs. Also there still might be some settings in fb for some further settings like vsync usage or yres_virtual size in coming patches.

Any clarifications appreciated.

Thanks
Daniel


--
Thanks,
-Zhou
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux