[PATCH 1/4] drm/mipi-dbi: Support command mode panel drivers

[Noralf Trønnes]

Den 12.08.2019 20.49, skrev Sam Ravnborg:
> Hi Noralf.
> 
>>> - drm_panel has proper support for modes.
>>>   This is today duplicated in mipi_dbi.
>>>   Could we make it so that when a panel is used then the panel
>>>   has the mode info - as we then use the panel more in the way we do
>>>   in other cases?
>>>   As it is now the mode is specified in mipi_dbi_dev_init()
>>>   The drm_connector would then, if a panel is used, ask the panel for
>>>   the mode.
>>>   I did not really think to the end of this, but it seems wrong that
>>>   we introduce drm_panel and then keep modes in mipi_dbi.
>>>
>>
>> I considered that, but it would would just generate duplicate code for
>> the connector. It would make sense to refactor this when/if all mipi dbi
>> drivers are turned into panel drivers.
> 
> The objective should be that all mipi dbi drivers could be refactored.

Not all can be converted, easily at least. ili9225 and st7586 are not
true MIPI DBI controllers, they are just similar enough to benefit from
the helper.

> And so it makes sense to do it right from the beginning.
> It will be some duplicated code until all are migrated.
> But as the number of mipi dbi drivers are low it is doable if a few
> people throw some time after it.
> I volunteer to assist.
> 
> In drm_mipi_dbi.c we could just add:
> 
> static int mipi_dbi_connector_get_modes(struct drm_connector *connector)
> {
> 	...
>         if (dbidev->panel)
>                 return drm_panel_get_modes(dbidev->panel);
> 
> 

This would make sense if the display mode was used as-is in the panel
driver. But the mode can be rotated so that would neeed to be taken care
of somehow.

I have started to work on a driver now so I can spend time on complex
refactorings. I'm not against changing this, but maybe a change like
this should happen after drivers have been converted and the picture is
more clear.

And Thierry hasn't voiced his opinion on this either, so no point in
spending more time on this if he disagrees with this "panel driver as
full drm driver" move.

Noralf.

> Then if there is a panel we would use the mode specified by the panel.
> To make this work we would need a drm_panel_attach() in
> drm_mipi_dbi_panel_register() to give the panel access to the connector.
> I have patches that makes connector an argument to drm_panel_get_modes()
> but they need a bit more baking, so you cannot benefit from them yet.
> 
> Maybe this is the same argument as backlight?
> We can introduce this when the drm_panel core is better prepared.
> 
>>>> +int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev,
>>>> +				struct drm_driver *driver, const struct drm_display_mode *mode,
>>>> +				u32 rotation)
>>> Can we make this use enum drm_panel_orientation - so we can use
>>> of_drm_get_panel_orientation() in the callers?
>>> of_drm_get_panel_orientation() is not merged yet, but we could do so if
>>> this patch-set needs it.
>>>
>>> I know that this would require mipi_dbi_dev_init() and all users to be
>>> updated. But it is a simpler interface so worth it.
>>>
>>
>> That would break rotation on userspace that doesn't know about the panel
>> orientation property which is a recent addition. These panels are mostly
>> used in the embedded world not desktop. It also would break fbdev 90/270
>> rotation (see drm_client_rotation()).
> 
> I think it is possible to move most of drm over to one way to spicify
> rotation.
> But let's wait with that battle until another day.
> It should not hinder this series.
> 
> 	Sam
>