On Mon, Sep 10, 2018 at 09:27:24AM +0200, Boris Brezillon wrote:
Hi Sam,
On Sat, 8 Sep 2018 22:17:55 +0200 Sam Ravnborg sam@ravnborg.org wrote:
Hi all.
When working on the DRM driver for Atmel LCDC the first approach was to use a MFD driver, that had two sub-drivers:
- PWM dirver
- DRM driver
Feedback was that the PWM feature was too small to warrant a MFD driver. (There was no consencus on this, but I anyway went ahead).
So the new approch is much simpler (from a code point of view): DRM Driver that has one sub-driver
PWM driver The PWM driver uses registers in the same memory range as the DRM driver, so the two drivers uses the same regmap.
The PWM driver is located in pwm/ The DRM driver is located in gpu/drm/atmel The DRM driver uses platform_device_register_data() to register the sub-device/driver. I have yet to see it work, but I think this is the right way to do it.
This all looked fine until reality kicked in.
There is the following dependency chain (=> depends on): DRM Driver => Panel => Backlight => PWM => DRM Driver
The DRM Driver rely indirectly on the DRM driver, which it not OK.
So the open question is how to fix this dependency challenge?
- Drop the generic backlight driver and implement all pwm/backlight handling in the driver.
- Re-introduce the MFD driver.
- ?
Any good ideas?
I keep thinking #2 is the cleanest option. The fact is, the LCDC IP is actually providing 2 functionalities: a PWM (most of the time driving a backlight) and a display controller. Really, I don't see why representing this IP as an MFD is not appropriate, especially since the HLCDC MFD driver is already in place and can easily be re-used.
But can you enable the PWM functionality without the DRM mode? If yes, then I do agree that a PWM driver should be re-introduced (MFD is a bit of a stretch, to me that means "the driver there can do several things on its own", rather than "this is the remaining functionality out of a different driver").
Best regards, Liviu
What should be adjusted though, is the need for 2 sub-nodes: I think you can just use a single node and declare #pwm-cells at the top level.
Regards,
Boris
Regards,
Boris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel