Thread (28 messages) 28 messages, 5 authors, 2018-05-15

Re: [PATCH 5/7] omapfb: omapfb_dss.h: add stubs to build with COMPILE_TEST && DRM_OMAP

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2018-04-23 20:22:41
Also in: dri-devel, linux-media

On Monday, 23 April 2018 23:09:55 EEST Mauro Carvalho Chehab wrote:
Laurent Pinchart [off-list ref] escreveu:
quoted
On Monday, 23 April 2018 17:22:27 EEST Mauro Carvalho Chehab wrote:
quoted
Em Mon, 23 Apr 2018 15:56:53 +0200 Bartlomiej Zolnierkiewicz escreveu:
quoted
On Monday, April 23, 2018 02:47:28 PM Bartlomiej Zolnierkiewicz wrote:
quoted
On Friday, April 20, 2018 01:42:51 PM Mauro Carvalho Chehab wrote:
quoted
Add stubs for omapfb_dss.h, in the case it is included by
some driver when CONFIG_FB_OMAP2 is not defined, with can
happen on ARM when DRM_OMAP is not 'n'.

That allows building such driver(s) with COMPILE_TEST.

Signed-off-by: Mauro Carvalho Chehab <redacted>
This patch should be dropped (together with patch #6/7) as it was
superseded by a better solution suggested by Laurent:

https://patchwork.kernel.org/patch/10325193/

ACK-ed by Tomi:

https://www.spinics.net/lists/dri-devel/msg171918.html

and already merged by you (commit 7378f1149884 "media: omap2:
omapfb: allow building it with COMPILE_TEST")..
Hmm, I see now while this patch is still included:

menuconfig FB_OMAP2

        tristate "OMAP2+ frame buffer support"
        depends on FB
        depends on DRM_OMAP = n

Ideally we should be able to build both drivers in the same kernel
(especially as modules).

I was hoping that it could be fixed easily but then I discovered
the root source of the problem:

drivers/gpu/drm/omapdrm/dss/display.o: In function
`omapdss_unregister_display': display.c:(.text+0x2c): multiple
definition
of `omapdss_unregister_display'
drivers/video/fbdev/omap2/omapfb/dss/display.o:display.c:(.text+0x198)
:
first defined here ...
Yes, and declared on two different places:

drivers/gpu/drm/omapdrm/dss/omapdss.h:void
omapdss_unregister_display(struct omap_dss_device *dssdev);
include/video/omapfb_dss.h:void
omapdss_unregister_display(struct omap_dss_device *dssdev);

one alternative would be to give different names to it, and a common
header for both.

At such header, it could be doing something like:

static inline void omapdss_unregister_display(struct omap_dss_device
*dssdev) {
#if enabled(CONFIG_DRM_OMAP)

	omapdss_unregister_display_drm(struct omap_dss_device *dssdev);

#else

	omapdss_unregister_display_fb(struct omap_dss_device *dssdev);

##endif
}

Yet, after a very quick check, it seems that nowadays only the
media omap driver uses the symbols at FB_OMAP:

$ git grep omapfb_dss.h
drivers/media/platform/omap/omap_vout.c:#include <video/omapfb_dss.h>
drivers/media/platform/omap/omap_voutdef.h:#include <video/omapfb_dss.h>
drivers/media/platform/omap/omap_voutlib.c:#include <video/omapfb_dss.h>

So, perhaps just renaming the common symbols and changing FB_OMAP2 to:
	menuconfig FB_OMAP2
	
	         tristate "OMAP2+ frame buffer support"
	         depends on FB
	         depends on (DRM_OMAP = n) || COMPILE_TEST

would be enough to allow to build both on ARM.
I don't think it's worth it renaming the common symbols. They will change
over time as omapdrm is under heavy rework, and it's painful enough
without having to handle cross-tree changes.
It could just rename the namespace-conflicting FB_OMAP2 functions,
keeping the DRM ones as-is.
quoted
Let's just live with the fact that both drivers
can't be compiled at the same time, given that omapfb is deprecated.
IMO, a driver that it is deprecated, being in a state where it
conflicts with a non-deprecated driver that is under heavy rework
is a very good candidate to go to drivers/staging or even to /dev/null.
It's on its way, but slowly as we need to take userspace into account. Tomi 
should have more insight on a possible schedule for removal of omapfb.

-- 
Regards,

Laurent Pinchart


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help