Re: [PATCH v5] drm/pl111: Initial drm/kms driver for pl111

[Eric Anholt]

Linus Walleij <linus.walleij@xxxxxxxxxx> writes:

> On Tue, Apr 11, 2017 at 3:18 AM, Eric Anholt <eric@xxxxxxxxxx> wrote:
>
>> From: Tom Cooksey <tom.cooksey@xxxxxxx>
>
> Well that can be debated at this point. I think it should have
> your Author: tag and just Tom in the Signed-off-by, then mention
> the history in the commit message.

I'm happy giving credit to the original author, unless he wants to duck
responsibility at this point.  I probably wouldn't have done this work
if I didn't have his to start from.

>> This is a modesetting driver for the pl111 CLCD display controller
>> found on various ARM platforms such as the Versatile Express. The
>> driver has only been tested on the bcm911360_entphn platform so far,
>> with PRIME-based buffer sharing between vc4 and clcd.
>>
>> It reuses the existing devicetree binding, while not using quite as
>> many of its properties as the fbdev driver does (those are left for
>> future work).
>>
>> v2: Nearly complete rewrite by anholt, cutting 2/3 of the code thanks
>>     to DRM core's excellent new helpers.
>> v3: Don't match pl110 any more, don't attach if we don't have a DRM
>>     panel, use DRM_GEM_CMA_FOPS, update MAINTAINERS, use the simple
>>     display helper, use drm_gem_cma_dumb_create (same as our wrapper).
>> v4: Change the driver's .name to not clash with fbdev in sysfs, drop
>>     platform alias, drop redundant "drm" in DRM driver name, hook up
>>     .prepare_fb to the CMA helper so that DMA fences should work.
>> v5: Move register definitions inside the driver directory, fix build
>>     in COMPILE_TEST and !AMBA mode.
>>
>> Signed-off-by: Tom Cooksey <tom.cooksey@xxxxxxx>
>> Signed-off-by: Eric Anholt <eric@xxxxxxxxxx>
>
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>
> This is as good starting point as any. We need to get moving with
> this. Some minor things below that can just as well be fixed later.

Thanks!

>>  create mode 100644 drivers/gpu/drm/pl111/Kconfig
>
> Maybe someone would want to place this under
> drivers/gpu/drm/arm since it is obviously an ARM block.
> But since ARM has pretty much dropped the ball on this
> IP it is probably best to keep it separate like this.
>
>> +++ b/drivers/gpu/drm/pl111/Kconfig
>> @@ -0,0 +1,12 @@
>> +config DRM_PL111
>> +       tristate "DRM Support for PL111 CLCD Controller"
>> +       depends on DRM
>> +       depends on ARM || ARM64 || COMPILE_TEST
>> +       select DRM_KMS_HELPER
>> +       select DRM_KMS_CMA_HELPER
>> +       select DRM_GEM_CMA_HELPER
>> +       select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE
>> +       help
>> +         Choose this option for DRM support for the PL111 CLCD controller.
>> +         If M is selected the module will be called pl111_drm.
>
> I must say the driver is *really* slim and readable with all the new
> helpers from DRM, good job all who refactored the DRM support
> for simple framebuffer systems.
>
>> +       panel = of_drm_find_panel(panel_node);
>> +       of_node_put(panel_node);
>
> Was meaning to ask whether the panel bindings used by the
> fbdev drivers are 100% compatible with what DRM is using
> and from your code it seems like yes, they are?

Yeah, the panel bindings seem fine.  We'll need to build equivalent
panel drivers within DRM for anyone that wants to switch over, but
that's not hard.

Attachment: signature.asc
Description: PGP signature