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: Bartlomiej Zolnierkiewicz <hidden>
Date: 2018-04-25 11:13:23
Also in: dri-devel, linux-media

On Monday, April 23, 2018 05:11:14 PM Tomi Valkeinen wrote:
On 23/04/18 16:56, Bartlomiej Zolnierkiewicz wrote:
quoted
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
The main problem is that omapdrm and omapfb are two different drivers
for the same HW. You need to pick one, even if we would change those
functions and fix the link issue.
With proper resource allocation in both drivers this shouldn't be
a problem - the one which allocates resources first will be used
(+ we can initialize omapdrm first in case it is built-in). This is
how similar situations are handled in other kernel subsystems.

It seems that the real root problem is commit f76ee892a99e ("omapfb:
copy omapdss & displays for omapfb") from Dec 2015 which resulted in
duplication of ~30 KLOC of code. The code in question seems to be
both fbdev & drm independent:

"
    * omapdss, located in drivers/video/fbdev/omap2/dss/. This is a driver for the
      display subsystem IPs used on OMAP (and related) SoCs. It offers only a
      kernel internal API, and does not implement anything for fbdev or drm.
    
    * omapdss panels and encoders, located in
      drivers/video/fbdev/omap2/displays-new/. These are panel and external encoder
      drivers, which use APIs offered by omapdss driver. These also don't implement
      anything for fbdev or drm.
"

While I understand some motives behind this change I'm not overall
happy with it..
At some point in time we could compile both as modules (but not
built-in), but the only use for that was development/testing and in the
end made our life more difficult. So, now you must fully disable one of
them to enable the other. And, actually, we even have boot-time code,
not included in the module itself, which gets enabled when omapdrm is
enabled.
Do you mean some code in arch/arm/mach-omap2/ or something else?
While it's of course good to support COMPILE_TEST, if using COMPILE_TEST
with omapfb is problematic, I'm not sure if it's worth to spend time on
that. We should be moving away from omapfb to omapdrm.
Is there some approximate schedule for omapfb removal available?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help