Re: [PATCH V5 0/6] OMAPDSS: Cleanup cpu_is checks

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

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

On Thu, 2012-08-30 at 10:19 -0700, Tony Lindgren wrote:
> Hi,
> 
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [120830 00:35]:
> > On Wed, 2012-08-29 at 17:20 -0700, Tony Lindgren wrote:
> > > 
> > > Good to see this, we need this badly to avoid blocking
> > > single zImage effort on omaps. Can you also please take
> > 
> > What is the issue with single zImage? How do cpu_is_ check affect it?
> 
> The usage for that should only be limited to arch/arm/mach-omap2
> so we can make cpu.h local as we can't include mach and plat
> header files from the drivers with single zImage.

Ok.

> > $ git grep -E "<plat|<mach" drivers/video/omap*
> > drivers/video/omap/lcd_ams_delta.c:#include <plat/board-ams-delta.h>
> > * Needs to be moved
> 
> Yes, that should be either mach/board-ams-delta.h, or separate driver
> specific headers in include/linux/platform_data. For omap1 we are not
> planning common zImage support, so let's just make sure we're not
> breaking anything there as people are still using it.

Hmm, so did I understand right, for omap1 stuff we can still include
from arch/arm/mach-omap1/include/mach?

If so, that makes things easier. I can manage the omap2+ stuff fine, but
I have no experience with omap1, nor do I have any omap1 devices. So I'd
rather keep the omap1 code as it is, in fear that I'd just break it
totally, and I'd rather spend my time on omap2+ code.

> > drivers/video/omap2/dss/dss.c:#include <plat/cpu.h>
> > * Uses cpu_is_* to find out the DSS version. dispc.c also uses cpu_is_*
> > functions, but doesn't include plat/cpu.h. I know cpu_is_* checks should
> > be removed, but is there some other file to include for the time being
> > than plat/cpu.h?
> 
> We could make it mach/cpu.h for omap1, but ideally we would just
> pass DSS_VERSION_XYZ type flag in platform data on omap1.

Ok. The omap1 fb driver seems to contain a few cpu_is_omap15xx() checks,
nothing else.

> > drivers/video/omap2/dss/dss_features.c:#include <plat/cpu.h>
> > * Uses cpu_is_* to find out the DSS version
> 
> Here too.

Yep, for omap2+ I'll create a version ID that we'll pass via plat data.
 
> > drivers/video/omap2/omapfb/omapfb-ioctl.c:#include <plat/vrfb.h>
> > drivers/video/omap2/omapfb/omapfb-ioctl.c:#include <plat/vram.h>
> > drivers/video/omap2/omapfb/omapfb-main.c:#include <plat/vram.h>
> > drivers/video/omap2/omapfb/omapfb-main.c:#include <plat/vrfb.h>
> > drivers/video/omap2/omapfb/omapfb-sysfs.c:#include <plat/vrfb.h>
> > drivers/video/omap2/vram.c:#include <plat/vram.h>
> > drivers/video/omap2/vram.c:#include <plat/dma.h>
> > drivers/video/omap2/vrfb.c:#include <plat/vrfb.h>
> > drivers/video/omap2/vrfb.c:#include <plat/sdrc.h>
> > 
> > Of these, I'm not sure how to handle.
> 
> These should eventually be in platform_data as driver specific headers.
>  
> > Grep shows that vram.c is only used by (the newer) omapfb, so it could
> > be considered a part of that driver. It still needs to be built-in, as
> > it needs to reserve memory early in the boot process (done with a call
> > from arch/arm/plat-omap/common.c).
> 
> Hmm it sounds like omap_vram_reserve_sdram_memblock() should be in
> plat-omap and just pass the pointer for later use for vram.c.
>  
> > Also board files can use a func call to define the amount of memory to
> > allocate, but only rx51 seems to do this in the mainline.
> > 
> > Anyway, I believe vram.c is going away when we start to use CMA.
> 
> OK cool. 
>  
> > As for vrfb... I'm not really sure where it belongs. It is used by the
> > newer omapfb and OMAP V4L2 driver. VRFB is a part of the OMAP's memory
> > controller, so I'm not sure if it's really a normal driver.
> 
> Maybe just split it to platform code for the memblock stuff and pass
> the necessary information to the driver in platform_data?

I'll try to figure out something for vrfb.

 Tomi


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