[RFC PATCH 0/3] drm/panel: Pull some code out into common helpers

[Emil Velikov]

On 22 March 2017 at 15:06, Sean Paul <seanpaul at chromium.org> wrote:
> On Wed, Mar 22, 2017 at 02:36:27PM +0000, Emil Velikov wrote:
>> Hi Sean,
>>
>> On 16 March 2017 at 22:08, Sean Paul <seanpaul at chromium.org> wrote:
>> > This series pulls out the power-sequencing code from panel-simple into a
>> > panel-common helper library. This allows drivers that cannot leverage
>> > panel-simple to share some code.
>> >
>> > I've converted the 2 sharp mipi drivers, and Chris Zhong's driver on the
>> > list can also be converted. I haven't checked any other drivers, but I
>> > suspect we'll see the same code blocks there too.
>> >
>> > I'm sure there's more we can pull out of the various drivers, but this
>> > seems like a good place to start talking about how to share common panel
>> > code across drivers.
>> >
>> Fwiw I think that the idea is good, but I'm wondering on the following
>> architectural questions:
>
> Hey Emil,
> Thanks for your feedback!
>
>
>>  - Shouldn't prepared and enabled be part of struct drm_panel ?
>
> I don't think so. Not all panels need to worry about keeping track of the
> prepared/enabled state.
>
You're correct - I did not notice those before. Yet, seems like adding
the trivial prepared/enabled check will be beneficial to them ?

Unrelated tidbits/ideas, while skimming through:
 - lg-lg4573
struct lg4573::vm - never set/used.
Might want a prepare/unprepare hook to manage power on/off
 - panasonic-vvx10f034n00
struct wuxga_nt_panel::mode - set but unused
 - sharp-ls043t1le01
struct sharp_nt_panel::mode - set but unused


>>  - Would it be better to subclass struct panel_common around struct drm_panel ?
>>
>> I might be threading the thin line of "midlayer vs helpers" here, so
>> please let me know if I've got it wrong.
>
> Yeah, you could do either. I was going for something more akin to the helpers we
> already have. If you went the subclass route, the drivers would need to subclass
> panel_common, which would subclass drm_panel. I figured it was too much
> unraveling to get at the important bits in the hooks that provide a drm_panel
> pointer. Nothing that a few to_* helpers couldn't solve, though.
>
> I was also thinking we could subclass panel_common if there's an opportunity to
> get something like mipi_dcs_panel_common to reduce the copypasta amongst mipi
> panels which execute a common dcs recipe.
>
Indeed - wrapping panel_common around drm_panel might not be that good of idea.

Thanks
Emil