Hi Nicholas.
On Mon, Aug 13, 2018 at 05:54:48PM +0200, Nicolas Ferre wrote:
On 12/08/2018 at 20:41, Sam Ravnborg wrote:
New DRM based driver for at91sam9 SOC's that uses the Atmel LCDC IP core.
I'm delighted to see this work: Thanks a lot Sam!
Glad to hear. I was a bit worried that the response would be "this is waste of time as we have a working driver already".
This is first version of a patch set that adds drivers for the Atmel LCDC IP core. Posted for review as the basics works now.
The LCDC IP core contains two devices:
- a PWM often used for backlight
- a LCD display controller
Both devices are supported today by the atmel_lcdfb driver. For this new set of drivers the compatible strings was selected to avoid clash with the existing compatible strings used for the atmel_lcdfb driver to allow them to co-exist.
Would be good to have a plan to phase-out the old atmel_lcdfb fbdev driver when this one addresses some TODO items that make sense.
Agree on this. One approach could be to say that when all in-kernel users of atmel_lcdfb are ported, then the old driver could be dropped after a kernel release.
The mfd suffix seems strange to me. What about "atmel,at91sam9263-lcdc-mfd" => "atmel,at91sam9263-lcd" (or "microchip,at91sam9263-lcdc").
The "-mfd" suffix was added to avoid clashing with the current compatible string used by the atmel_lcdfb driver.
I susggest we do the following: - use the microchip prefix, as this is now owned by microchip - and add the driver to a drm/microchip/ directory (Then we can only hope that microchip do not change name or are purchased by someone else).
The DRM implementation has a few shortcomings compared to the existing fbdev based driver: - STN displays are not supported Binding support is missing but most of the STN specific functionality is otherwise ported from the fbdev driver. I assume the info should come from the panel but as I lack HW I have not looked too much into what is required.
Yes, I'm not even sure if STN displays are still available those days... Might not be worth it spending time on this.
Then I will delete all the STN bits that was ported over. If we need them later they can be found on the mailign list.
- gamma support is missing The driver utilises drm_simple_kms_helper and this helper lacks support for setting up gamma. If this is useful please let me know and I will extend drm_simple_kms_helper to support this and update the driver. - modesetting is not checked (see TODO in file) Is this required for such a simple setup? - support for extra modes as applicable (and lcd-wiring-mode) - support for AVR32 (is it relevant?)No, AVR32 is not relevant anymore as the core and SoC were removed some years ago from Linux. All mention of AT32 or AVR32 can be remove then.
Ok, I will delete these.
The drivers _works_ on a proprietary at91sam9263 based board and I will eventually migrate the at91sam9263ek over to use it as well.
We'll be able to test it on other SoCs and boards.
Thanks!
Works is maybe a stretch - the following was tested: modetest shows reasonable output modetest -s shows some nice colored squares
You must see something like this (without the overlay additional black square): http://www.at91.com/linux4sam/bin/view/Linux4SAM/UsingAtmelDRMDriver#Some_mo...
I posted a picture to G+ here: https://plus.google.com/+SamRavnborg/posts/YBt4jUGLgWa
They look similar but are not equal. For now I assume this is OK. I will do some testing with a Qt app, and will test colors with this too.
All feedback (from spelling errors to general structure) appreciated!
On my side, it's mostly Nitpicking ;-) Now that we're Microchip for a little bit more than 2 years, I tend to make this name prevalent compared to Atmel for new work around our products... But I know this driver is for older SoCs and that it got inspired by two former drivers that have this "atmel" naming everywhere. MAINTAINERS and Kconfig changes could include Microchip name, so that it's obvious for the newcomers...
Will fix, as described above.
Thanks for the inputs!
Sam