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

[Sam Ravnborg <sam@ravnborg.org>]

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.
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);


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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel