[0/7] drm/vc4: Allow for more boot-time configuration

Message

Maxime Ripard March 4, 2019, 2:52 p.m. UTC

Hi,

The proprietary stack for the RaspberryPi allows for a number of video
parameters widely used by their users, but yet don't have any equivalents
in the mainline kernel.

Those options are detailed here:
https://www.raspberrypi.org/documentation/configuration/config-txt/video.md

While not all of them are desirable to have in the mainline kernel, some of
them still have value, such as properties to initialise the overscan or
rotation parameters, or the one to deal with broken displays.

This series is an attempt to support those, and is based on a rewrite of
the command line parser I did a couple of years ago that never reached
upstream (due to a lack of time on my side). While this parser was
initially made to deal with named modes (in order to support TV modes), it
also allowed to extend it more easily, which is why it's resurrected.

Since a change of the command line parser can pretty easily get things
wrong and introduce regressions, I also worked with a number of unit tests
that you can find here: http://code.bulix.org/tpo7dg-607264?raw

Eventually, I guess those tests should be part of the kernel somewhere, but
I haven't found a suitable place for them to be included yet.

Let me know what you think,
Maxime

Maxime Ripard (7):
  drm/vc4: hdmi: Check that the monitor supports HDMI audio
  drm/edid: Allow to ignore the audio EDID data
  drm/edid: Allow to ignore the HDMI monitor mode
  drm/modes: Rewrite the command line parser
  drm/modes: Support modes names on the command line
  drm/modes: Allow to specify rotation and reflection on the commandline
  drm/modes: Parse overscan properties

 drivers/gpu/drm/drm_connector.c |   3 +-
 drivers/gpu/drm/drm_edid.c      |  16 +-
 drivers/gpu/drm/drm_fb_helper.c |  55 ++++-
 drivers/gpu/drm/drm_modes.c     | 441 ++++++++++++++++++++++++---------
 drivers/gpu/drm/vc4/vc4_hdmi.c  |   6 +-
 include/drm/drm_connector.h     |   3 +-
 6 files changed, 408 insertions(+), 116 deletions(-)

base-commit: e179d8e074e05a913a0915ae3c4b82f1434d9f4e

Comments

Peter Stuge March 4, 2019, 3:21 p.m. UTC | #1

Hi,

Maxime Ripard wrote:
> properties to initialise the overscan or rotation parameters, or the
> one to deal with broken displays.

How does that work on systems with multiple connectors?

On SBCs with only one output I guess it's fine to have a global option,
but it may be important for new options to also be usable on more
complex systems. And maybe even SPI displays are relevant?

Also, some of the forward-ported patches still have your old email
address, maybe you want to change that.


Thanks

//Peter

Stefan Wahren March 4, 2019, 3:44 p.m. UTC | #2

Hi Maxime,

Am 04.03.2019 um 15:52 schrieb Maxime Ripard:
> Hi,
>
> The proprietary stack for the RaspberryPi allows for a number of video
> parameters widely used by their users, but yet don't have any equivalents
> in the mainline kernel.
>
> Those options are detailed here:
> https://www.raspberrypi.org/documentation/configuration/config-txt/video.md
>
> While not all of them are desirable to have in the mainline kernel, some of
> them still have value, such as properties to initialise the overscan or
> rotation parameters, or the one to deal with broken displays.
>
> This series is an attempt to support those, and is based on a rewrite of
> the command line parser I did a couple of years ago that never reached
> upstream (due to a lack of time on my side). While this parser was
> initially made to deal with named modes (in order to support TV modes), it
> also allowed to extend it more easily, which is why it's resurrected.
>
> Since a change of the command line parser can pretty easily get things
> wrong and introduce regressions, I also worked with a number of unit tests
> that you can find here: http://code.bulix.org/tpo7dg-607264?raw
>
> Eventually, I guess those tests should be part of the kernel somewhere, but
> I haven't found a suitable place for them to be included yet.
>
> Let me know what you think,
> Maxime
>
thanks for this series. In case you want some feedback from the 
Foundation, please add Phil Elwell instead of Eben to CC.

Stefan

Maxime Ripard March 4, 2019, 3:56 p.m. UTC | #3

Hi Peter,

On Mon, Mar 04, 2019 at 03:21:35PM +0000, Peter Stuge wrote:
> Hi,
> 
> Maxime Ripard wrote:
> > properties to initialise the overscan or rotation parameters, or the
> > one to deal with broken displays.
> 
> How does that work on systems with multiple connectors?
> 
> On SBCs with only one output I guess it's fine to have a global option,
> but it may be important for new options to also be usable on more
> complex systems. And maybe even SPI displays are relevant?

These properties are per-connector, so only the display attached to
that connector will be affected, and you can have multiple connectors
with various properties and values defined if you want to

> Also, some of the forward-ported patches still have your old email
> address, maybe you want to change that.

Good catch, thanks!
Maxime

Eric Anholt March 4, 2019, 8:06 p.m. UTC | #4

Maxime Ripard <[e-mail]> writes:

> Hi,
>
> The proprietary stack for the RaspberryPi allows for a number of video
> parameters widely used by their users, but yet don't have any equivalents
> in the mainline kernel.
>
> Those options are detailed here:
> https://www.raspberrypi.org/documentation/configuration/config-txt/video.md
>
> While not all of them are desirable to have in the mainline kernel, some of
> them still have value, such as properties to initialise the overscan or
> rotation parameters, or the one to deal with broken displays.
>
> This series is an attempt to support those, and is based on a rewrite of
> the command line parser I did a couple of years ago that never reached
> upstream (due to a lack of time on my side). While this parser was
> initially made to deal with named modes (in order to support TV modes), it
> also allowed to extend it more easily, which is why it's resurrected.

FWIW for other reviewers, the overscan and rotation are the really
important parts of this series.  Since Raspberry Pi ends up connected to
TVs so frequently, there are many users of the overscan workaround.
Rotation is important for supporting the official DSI touchscreen panel,
which is unfortunately mounted upside down in most mounts you'll find.

> Since a change of the command line parser can pretty easily get things
> wrong and introduce regressions, I also worked with a number of unit tests
> that you can find here: http://code.bulix.org/tpo7dg-607264?raw

Would kselftests be an appropriate way to include these, maybe?

Maxime Ripard March 5, 2019, 9:14 a.m. UTC | #5

Hi,

On Mon, Mar 04, 2019 at 12:06:01PM -0800, Eric Anholt wrote:
> > Since a change of the command line parser can pretty easily get things
> > wrong and introduce regressions, I also worked with a number of unit tests
> > that you can find here: http://code.bulix.org/tpo7dg-607264?raw
> 
> Would kselftests be an appropriate way to include these, maybe?

That looks like a good way to do this indeed. I'll try it and let you know

Thanks!
Maxime

Daniel Vetter March 11, 2019, 1 p.m. UTC | #6

On Tue, Mar 05, 2019 at 10:14:16AM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Mar 04, 2019 at 12:06:01PM -0800, Eric Anholt wrote:
> > > Since a change of the command line parser can pretty easily get things
> > > wrong and introduce regressions, I also worked with a number of unit tests
> > > that you can find here: http://code.bulix.org/tpo7dg-607264?raw
> > 
> > Would kselftests be an appropriate way to include these, maybe?
> 
> That looks like a good way to do this indeed. I'll try it and let you know

drm/selftests already has a pile of infrastructure, together with igt and
will be run in intel-gfx-ci to make sure stuff keeps working.

And yeah parser like this is imo the perfect selftest use-case.

If you wonder about kselftest and kunit and all that: We have some compat
glue, but I do hope that longterm we can all unify these internal
self/unit tests under one framework. For now I recommend the drm one,
since that's CI'ed.

Cheers, Daniel