Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FKMS "Broadcast RGB" property, and KMS command line parsing update #3103

Merged
merged 12 commits into from
Aug 5, 2019

Conversation

6by9
Copy link
Contributor

@6by9 6by9 commented Jul 24, 2019

"Broadcast RGB" is how Intel have added an override for the full/limited range option for HDMI.

Backports DRM command line patches from drm-misc-next, so should be in 5.3. Allows specifying of margins and rotation from the kernel command line.
eg video=HDMI-A-1:1920x1080@60,margin_right=50,rotate=90

Required a modest amount of reworking as functions have been moved between files.

reflect_x and reflect_y appear not to get actioned. That's not a use case I'm overly concerned about, but I think it's core DRM plumbing missing.

Also enables CONFIG_FRAMEBUFFER_CONSOLE_ROTATION so that the console can be rotated (in software) under DRM/KMS.

6by9 and others added 12 commits July 24, 2019 10:46
Some HDMI monitors do not abide by the full or limited
(16-235) range RGB flags in the AVI infoframe. This can
result in images looking washed out (if given limited and
interpreting as full), or detail disappearing at the extremes
(given full and interpreting as limited).

Copy the Intel i915 driver's approach of adding an override
property ("Broadcast RGB") to force one mode or the other.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
commit 772cd52 upstream.

The struct drm_cmdline_mode holds the result of the command line parsers.
However, it wasn't documented so far, so let's do that.

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Link: https://patchwork.freedesktop.org/patch/msgid/963c893c16c6a25fc469b53c726f493d99bdc578.1560783090.git-series.maxime.ripard@bootlin.com
commit e08ab74 upstream.

Rewrite the command line parser in order to get away from the state machine
parsing the video mode lines.

Hopefully, this will allow to extend it more easily to support named modes
and / or properties set directly on the command line.

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Link: https://patchwork.freedesktop.org/patch/msgid/e32cd4009153b184103554009135c7bf7c9975d7.1560783090.git-series.maxime.ripard@bootlin.com
commit 3aeeb13 upstream.
Minor conflict resolution as upstream has moved functions
from drm_fb_helper.c to a new drm_client_modeset.c

The drm subsystem also uses the video= kernel parameter, and in the
documentation refers to the fbdev documentation for that parameter.

However, that documentation also says that instead of giving the mode using
its resolution we can also give a name. However, DRM doesn't handle that
case at the moment. Even though in most case it shouldn't make any
difference, it might be useful for analog modes, where different standards
might have the same resolution, but still have a few different parameters
that are not encoded in the modes (NTSC vs NTSC-J vs PAL-M for example).

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Link: https://patchwork.freedesktop.org/patch/msgid/18443e0c3bdbbd16cea4ec63bc7f2079b820b43b.1560783090.git-series.maxime.ripard@bootlin.com
Commit 1bf4e09 upstream.
Minor conflict resolution as upstream has moved functions
from drm_fb_helper.c to a new drm_client_modeset.c

Rotations and reflections setup are needed in some scenarios to initialise
properly the initial framebuffer. Some drivers already had a bunch of
quirks to deal with this, such as either a private kernel command line
parameter (omapdss) or on the device tree (various panels).

In order to accomodate this, let's create a video mode parameter to deal
with the rotation and reflexion.

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Link: https://patchwork.freedesktop.org/patch/msgid/777da16e42db757c1f5b414b5ca34507097fed5c.1560783090.git-series.maxime.ripard@bootlin.com
Commit 22045e8 upstream.

The TV margins has been defined as a structure inside the
drm_connector_state structure so far. However, we will need it in other
structures as well, so let's move that structure definition so that it can
be reused.

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Link: https://patchwork.freedesktop.org/patch/msgid/38b773b03f15ec7a135cdf8f7db669e5ada20cf2.1560783090.git-series.maxime.ripard@bootlin.com
Commit 3d46a30 upstream.

Properly configuring the overscan properties might be needed for the
initial setup of the framebuffer for display that still have overscan.
Let's allow for more properties on the kernel command line to setup each
margin.

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Link: https://patchwork.freedesktop.org/patch/msgid/e481f1628e3768ca49226ec2115cfa4dfcbd5e4c.1560783090.git-series.maxime.ripard@bootlin.com
Commit 731514b upstream.
Reworked as functions have been moved from drm_atomic_helper.[c|h]
to drm_atomic_state_helper.[c|h].

During the connector reset, if that connector has a TV property, it needs
to be reset to the value provided on the command line.

Provide a helper to do that.

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Link: https://patchwork.freedesktop.org/patch/msgid/84a7b657f09303a2850e1cc79e68f623547f3fdd.1560783090.git-series.maxime.ripard@bootlin.com
Now that the TV margins are properly parsed and filled into
drm_cmdline_mode, we just need to initialise the first state at reset to
get those values and start using them.

Acked-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Link: https://patchwork.freedesktop.org/patch/msgid/44e24172e300be6a41578517021ef6a6e90ed682.1560783090.git-series.maxime.ripard@bootlin.com
Now that the TV margins are properly parsed and filled into
drm_cmdline_mode, we just need to initialise the first state at reset to
get those values and start using them.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
Taken from the dri-devel mailing list (11/7/2019) to fixup the cmdline
parsing, but requires changes as things have moved between 4.19 and 5.2.

From: Dmitry Osipenko <digetx@gmail.com>

The rotation mode from cmdline shouldn't be taken into account if it
wasn't specified in the cmdline. This fixes ignored default display
orientation when display mode is given using cmdline without the
rotation being specified.

Fixes: 1bf4e09 ("drm/modes: Allow to specify rotation and reflection on the commandline")
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
To allow for console rotation under DRM (where the firmware
can't lie about geometry), add FRAMEBUFFER_CONSOLE_ROTATION
so that the kernel can do it.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
@6by9
Copy link
Contributor Author

6by9 commented Aug 5, 2019

@pelwell @popcornmix Any chance of a review?

Copy link
Contributor

@pelwell pelwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One substantive issue/query and a stylistic niggle.

if (!state)
return NULL;

__drm_atomic_helper_connector_duplicate_state(connector, &state->base);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either I've missed something or the state gets copied twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kmemdup allocates and copies sizeof(struct vc4_fkms_connector_state).
__drm_atomic_helper_connector_duplicate_state copies sizeof(drm_connector_state), which is smaller.

This was reproducing the same functionality as in the Intel i915 driver, and they also copy the base state twice.
https://elixir.bootlin.com/linux/v5.2.6/source/drivers/gpu/drm/i915/intel_atomic.c#L150

To clean it up we'll end up with a funky offsetof(broadcast_rgb) for the generic solution. At the moment it'll only be copying one int. I might just assign the int across with a comment should extra members get added.

ret = -ENOMEM;
goto fail;
return ERR_PTR(-ENOMEM);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary braces, although checkpatch seems unconcerned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checkpatch not being fussy for once. I'm glad I was sitting down!

@pelwell pelwell merged commit 3e638cc into raspberrypi:rpi-4.19.y Aug 5, 2019
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Aug 6, 2019
kernel: net: bcmgenet: Workaround for Pi 4B network issue
See: raspberrypi/linux#3108

kernel: overlays: Add baudrate parameter to i2c3-i2c6

kernel: Add HDMI1 facility to the driver
See: raspberrypi/linux#3100

kernel: FKMS Broadcast RGB property, and KMS command line parsing update
See: raspberrypi/linux#3103

kernel: Improvements for bcm2835-codec
See: raspberrypi/linux#3097

kernel: hid: usb: Add device quirks for Freeway Airmouse T3 and MX3
See: raspberrypi/linux#3117

firmware: Fix to allow HDMI audio port route setting
See: raspberrypi/linux#3100

userland: dtoverlay: only run lxpanelctl hooks when lxpanel is running
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Aug 6, 2019
kernel: net: bcmgenet: Workaround for Pi 4B network issue
See: raspberrypi/linux#3108

kernel: overlays: Add baudrate parameter to i2c3-i2c6

kernel: Add HDMI1 facility to the driver
See: raspberrypi/linux#3100

kernel: FKMS Broadcast RGB property, and KMS command line parsing update
See: raspberrypi/linux#3103

kernel: Improvements for bcm2835-codec
See: raspberrypi/linux#3097

kernel: hid: usb: Add device quirks for Freeway Airmouse T3 and MX3
See: raspberrypi/linux#3117

firmware: Fix to allow HDMI audio port route setting
See: raspberrypi/linux#3100

userland: dtoverlay: only run lxpanelctl hooks when lxpanel is running
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants