2024-03-18 19:18:18

by Mehdi Djait

[permalink] [raw]
Subject: drm/tiny: QUESTION: What to use instead of drm_simple_display_pipe ?

Hello everyone :)

I am implementing a tiny drm driver and I am currently working on the
V2: https://lore.kernel.org/dri-devel/[email protected]/

I got a review on the v1 telling me not to use the
drm_simple_display_pipe. Can someone please explain this further ? Or
give me an example drm driver that does it the right way ?

--
Kind Regards
Mehdi Djait


2024-03-19 08:36:06

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: drm/tiny: QUESTION: What to use instead of drm_simple_display_pipe ?

Hi

Am 18.03.24 um 20:18 schrieb Mehdi Djait:
> Hello everyone :)
>
> I am implementing a tiny drm driver and I am currently working on the
> V2: https://lore.kernel.org/dri-devel/[email protected]/
>
> I got a review on the v1 telling me not to use the
> drm_simple_display_pipe. Can someone please explain this further ? Or
> give me an example drm driver that does it the right way ?

You can copy the code from drm_simple_kms_helper.c into your driver file
and start inlining everything. For example

 1) Your driver calls drm_simple_display_pipe_init(), so you copy that
code into your source file
 2) drm_simple_display_pipe_init() uses drm_simple_kms_plane_funcs and
drm_simple_kms_crtc_funcs, so you copy these into your source file;
together with the drm_simple_kms_*() helpers that they use for their
callback pointers.
 3) Mayb do this for other drm_simple_kms_*() code.
 4) Then start inlining: inline your copy of
drm_simple_display_pipe_iit(). Instead of using
sharp_ls027b7dh01_pipe_funcs, inline its functions into your copy of the
callers. And so on.
 5) Rename the resulting code, so that it fits you driver.

With careful changes, you 'll end up with the same functionality as
before, but without the intermediate layer of the simple-KMS code.

Best regards
Thomas

>
> --
> Kind Regards
> Mehdi Djait

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


2024-03-19 08:52:49

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: drm/tiny: QUESTION: What to use instead of drm_simple_display_pipe ?

Thomas Zimmermann <[email protected]> writes:

Hello Mehdi,

> Hi
>
> Am 18.03.24 um 20:18 schrieb Mehdi Djait:
>> Hello everyone :)
>>
>> I am implementing a tiny drm driver and I am currently working on the
>> V2: https://lore.kernel.org/dri-devel/[email protected]/
>>
>> I got a review on the v1 telling me not to use the
>> drm_simple_display_pipe. Can someone please explain this further ? Or
>> give me an example drm driver that does it the right way ?
>
> You can copy the code from drm_simple_kms_helper.c into your driver file
> and start inlining everything. For example
>
>  1) Your driver calls drm_simple_display_pipe_init(), so you copy that
> code into your source file
>  2) drm_simple_display_pipe_init() uses drm_simple_kms_plane_funcs and
> drm_simple_kms_crtc_funcs, so you copy these into your source file;
> together with the drm_simple_kms_*() helpers that they use for their
> callback pointers.
>  3) Mayb do this for other drm_simple_kms_*() code.
>  4) Then start inlining: inline your copy of
> drm_simple_display_pipe_iit(). Instead of using
> sharp_ls027b7dh01_pipe_funcs, inline its functions into your copy of the
> callers. And so on.
>  5) Rename the resulting code, so that it fits you driver.
>
> With careful changes, you 'll end up with the same functionality as
> before, but without the intermediate layer of the simple-KMS code.
>

On top of what Thomas said, you can check 622113b9f11f ("drm/ssd130x:
Replace simple display helpers with the atomic helpers") that did this
change for the drivers/gpu/drm/solomon/ssd130x.c driver.

The driver is also for a monochrome panel controller and it does support
SPI as transport, which means the controller is similar to yours in many
aspects. You could use that driver code as a reference for your driver.

> Best regards
> Thomas
>
>>
>> --
>> Kind Regards
>> Mehdi Djait
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2024-03-20 18:59:28

by Mehdi Djait

[permalink] [raw]
Subject: Re: drm/tiny: QUESTION: What to use instead of drm_simple_display_pipe ?

Hello Javier, Hello Thomas,

On Tue, Mar 19, 2024 at 09:52:34AM +0100, Javier Martinez Canillas wrote:
> Thomas Zimmermann <[email protected]> writes:
>
> Hello Mehdi,
>
> > Hi
> >
> > Am 18.03.24 um 20:18 schrieb Mehdi Djait:
> >> Hello everyone :)
> >>
> >> I am implementing a tiny drm driver and I am currently working on the
> >> V2: https://lore.kernel.org/dri-devel/[email protected]/
> >>
> >> I got a review on the v1 telling me not to use the
> >> drm_simple_display_pipe. Can someone please explain this further ? Or
> >> give me an example drm driver that does it the right way ?
> >
> > You can copy the code from drm_simple_kms_helper.c into your driver file
> > and start inlining everything. For example
> >
> > ?1) Your driver calls drm_simple_display_pipe_init(), so you copy that
> > code into your source file
> > ?2) drm_simple_display_pipe_init() uses drm_simple_kms_plane_funcs and
> > drm_simple_kms_crtc_funcs, so you copy these into your source file;
> > together with the drm_simple_kms_*() helpers that they use for their
> > callback pointers.
> > ?3) Mayb do this for other drm_simple_kms_*() code.
> > ?4) Then start inlining: inline your copy of
> > drm_simple_display_pipe_iit(). Instead of using
> > sharp_ls027b7dh01_pipe_funcs, inline its functions into your copy of the
> > callers. And so on.
> > ?5) Rename the resulting code, so that it fits you driver.
> >
> > With careful changes, you 'll end up with the same functionality as
> > before, but without the intermediate layer of the simple-KMS code.
> >
>
> On top of what Thomas said, you can check 622113b9f11f ("drm/ssd130x:
> Replace simple display helpers with the atomic helpers") that did this
> change for the drivers/gpu/drm/solomon/ssd130x.c driver.
>
> The driver is also for a monochrome panel controller and it does support
> SPI as transport, which means the controller is similar to yours in many
> aspects. You could use that driver code as a reference for your driver.

Thank you both very much!

--
Kind Regards
Mehdi Djait