Re: [PATCH v6] drm/display: split DSC helpers from DP helpers
On Thu, 4 Jul 2024 22:17:08 +0300, Dmitry Baryshkov wrote: > Currently the DRM DSC functions are selected by the > DRM_DISPLAY_DP_HELPER Kconfig symbol. This is not optimal, since the DSI > code (both panel and host drivers) end up selecting the seemingly > irrelevant DP helpers. Split the DSC code to be guarded by the separate > DRM_DISPLAY_DSC_HELPER Kconfig symbol. > > [ ... ] Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH v2 8/8] drm/amdgpu: Call drm_atomic_helper_shutdown() at shutdown time
On Thu, Jun 20, 2024 at 09:00:23AM GMT, Alex Deucher wrote: > On Thu, Jun 20, 2024 at 3:10 AM Maxime Ripard wrote: > > > > Hi, > > > > On Wed, Jun 19, 2024 at 09:53:12AM GMT, Alex Deucher wrote: > > > On Wed, Jun 19, 2024 at 9:50 AM Alex Deucher > > > wrote: > > > > > > > > On Tue, Jun 18, 2024 at 7:53 PM Doug Anderson > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Tue, Jun 18, 2024 at 3:00 PM Alex Deucher > > > > > wrote: > > > > > > > > > > > > On Tue, Jun 18, 2024 at 5:40 PM Doug Anderson > > > > > > wrote: > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > > On Mon, Jun 17, 2024 at 8:01 AM Alex Deucher > > > > > > > wrote: > > > > > > > > > > > > > > > > On Wed, Jun 12, 2024 at 6:37 PM Douglas Anderson > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Based on grepping through the source code this driver appears > > > > > > > > > to be > > > > > > > > > missing a call to drm_atomic_helper_shutdown() at system > > > > > > > > > shutdown > > > > > > > > > time. Among other things, this means that if a panel is in > > > > > > > > > use that it > > > > > > > > > won't be cleanly powered off at system shutdown time. > > > > > > > > > > > > > > > > > > The fact that we should call drm_atomic_helper_shutdown() in > > > > > > > > > the case > > > > > > > > > of OS shutdown/restart comes straight out of the kernel doc > > > > > > > > > "driver > > > > > > > > > instance overview" in drm_drv.c. > > > > > > > > > > > > > > > > > > Suggested-by: Maxime Ripard > > > > > > > > > Cc: Alex Deucher > > > > > > > > > Cc: Christian König > > > > > > > > > Cc: Xinhui Pan > > > > > > > > > Signed-off-by: Douglas Anderson > > > > > > > > > --- > > > > > > > > > This commit is only compile-time tested. > > > > > > > > > > > > > > > > > > ...and further, I'd say that this patch is more of a plea for > > > > > > > > > help > > > > > > > > > than a patch I think is actually right. I'm _fairly_ certain > > > > > > > > > that > > > > > > > > > drm/amdgpu needs this call at shutdown time but the logic is > > > > > > > > > a bit > > > > > > > > > hard for me to follow. I'd appreciate if anyone who actually > > > > > > > > > knows > > > > > > > > > what this should look like could illuminate me, or perhaps > > > > > > > > > even just > > > > > > > > > post a patch themselves! > > > > > > > > > > > > > > > > I'm not sure this patch makes sense or not. The driver doesn't > > > > > > > > really > > > > > > > > do a formal tear down in its shutdown routine, it just quiesces > > > > > > > > the > > > > > > > > hardware. What are the actual requirements of the shutdown > > > > > > > > function? > > > > > > > > In the past when we did a full driver tear down in shutdown, it > > > > > > > > delayed the shutdown sequence and users complained. > > > > > > > > > > > > > > The "inspiration" for this patch is to handle panels properly. > > > > > > > Specifically, panels often have several power/enable signals > > > > > > > going to > > > > > > > them and often have requirements that these signals are powered > > > > > > > off in > > > > > > > the proper order with the proper delays between them. While we > > > > > > > can't > > > > > > >
Re: [PATCH v2 8/8] drm/amdgpu: Call drm_atomic_helper_shutdown() at shutdown time
Hi, On Wed, Jun 19, 2024 at 09:53:12AM GMT, Alex Deucher wrote: > On Wed, Jun 19, 2024 at 9:50 AM Alex Deucher wrote: > > > > On Tue, Jun 18, 2024 at 7:53 PM Doug Anderson wrote: > > > > > > Hi, > > > > > > On Tue, Jun 18, 2024 at 3:00 PM Alex Deucher > > > wrote: > > > > > > > > On Tue, Jun 18, 2024 at 5:40 PM Doug Anderson > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > On Mon, Jun 17, 2024 at 8:01 AM Alex Deucher > > > > > wrote: > > > > > > > > > > > > On Wed, Jun 12, 2024 at 6:37 PM Douglas Anderson > > > > > > wrote: > > > > > > > > > > > > > > Based on grepping through the source code this driver appears to > > > > > > > be > > > > > > > missing a call to drm_atomic_helper_shutdown() at system shutdown > > > > > > > time. Among other things, this means that if a panel is in use > > > > > > > that it > > > > > > > won't be cleanly powered off at system shutdown time. > > > > > > > > > > > > > > The fact that we should call drm_atomic_helper_shutdown() in the > > > > > > > case > > > > > > > of OS shutdown/restart comes straight out of the kernel doc > > > > > > > "driver > > > > > > > instance overview" in drm_drv.c. > > > > > > > > > > > > > > Suggested-by: Maxime Ripard > > > > > > > Cc: Alex Deucher > > > > > > > Cc: Christian König > > > > > > > Cc: Xinhui Pan > > > > > > > Signed-off-by: Douglas Anderson > > > > > > > --- > > > > > > > This commit is only compile-time tested. > > > > > > > > > > > > > > ...and further, I'd say that this patch is more of a plea for help > > > > > > > than a patch I think is actually right. I'm _fairly_ certain that > > > > > > > drm/amdgpu needs this call at shutdown time but the logic is a bit > > > > > > > hard for me to follow. I'd appreciate if anyone who actually knows > > > > > > > what this should look like could illuminate me, or perhaps even > > > > > > > just > > > > > > > post a patch themselves! > > > > > > > > > > > > I'm not sure this patch makes sense or not. The driver doesn't > > > > > > really > > > > > > do a formal tear down in its shutdown routine, it just quiesces the > > > > > > hardware. What are the actual requirements of the shutdown > > > > > > function? > > > > > > In the past when we did a full driver tear down in shutdown, it > > > > > > delayed the shutdown sequence and users complained. > > > > > > > > > > The "inspiration" for this patch is to handle panels properly. > > > > > Specifically, panels often have several power/enable signals going to > > > > > them and often have requirements that these signals are powered off in > > > > > the proper order with the proper delays between them. While we can't > > > > > always do so when the system crashes / reboots in an uncontrolled way, > > > > > panel manufacturers / HW Engineers get upset if we don't power things > > > > > off properly during an orderly shutdown/reboot. When panels are > > > > > powered off badly it can cause garbage on the screen and, so I've been > > > > > told, can even cause long term damage to the panels over time. > > > > > > > > > > In Linux, some panel drivers have tried to ensure a proper poweroff of > > > > > the panel by handling the shutdown() call themselves. However, this is > > > > > ugly and panel maintainers want panel drivers to stop doing it. We > > > > > have removed the code doing this from most panels now [1]. Instead the > > > > > assumption is that the DRM modeset drivers should be calling > > > > > drm_atomic_helper_shutdown() which will make sure panels get an > > > > > orderly shutdown. > > > > > > > > > > For a lot more details, see the cover letter [2] which then contains > > > > &g
Re: Re: [PATCH 1/2] drm/print: drop include debugfs.h and include where needed
On Mon, 15 Apr 2024 16:09:22 +0300, Jani Nikula wrote: > On Wed, 10 Apr 2024, Jani Nikula wrote: > > Surprisingly many places depend on debugfs.h to be included via > > drm_print.h. Fix them. > > While all of this is trivial, merely adding some includes, please > > [ ... ] Acked-by: Maxime Ripard Thanks! Maxime
Re: Re: [PATCH v4 1/3] drm: Add drm_get_acpi_edid() helper
On Thu, Feb 08, 2024 at 11:57:11AM +0200, Jani Nikula wrote:
> On Wed, 07 Feb 2024, Mario Limonciello wrote:
> > Some manufacturers have intentionally put an EDID that differs from
> > the EDID on the internal panel on laptops. Drivers can call this
> > helper to attempt to fetch the EDID from the BIOS's ACPI _DDC method.
> >
> > Signed-off-by: Mario Limonciello
> > ---
> > drivers/gpu/drm/Kconfig| 5 +++
> > drivers/gpu/drm/drm_edid.c | 77 ++
> > include/drm/drm_edid.h | 1 +
> > 3 files changed, 83 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 6ec33d36f3a4..ec2bb71e8b36 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -21,6 +21,11 @@ menuconfig DRM
> > select KCMP
> > select VIDEO_CMDLINE
> > select VIDEO_NOMODESET
> > + select ACPI_VIDEO if ACPI
> > + select BACKLIGHT_CLASS_DEVICE if ACPI
> > + select INPUT if ACPI
> > + select X86_PLATFORM_DEVICES if ACPI && X86
> > + select ACPI_WMI if ACPI && X86
>
> I think I'll defer to drm maintainers on whether this is okay or
> something to be avoided.
>
>
> > help
> > Kernel-level support for the Direct Rendering Infrastructure (DRI)
> > introduced in XFree86 4.0. If you say Y here, you need to select
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 923c4423151c..c649b4f9fd8e 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -28,6 +28,7 @@
> > * DEALINGS IN THE SOFTWARE.
> > */
> >
> > +#include
> > #include
> > #include
> > #include
> > @@ -2188,6 +2189,49 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned
> > int block, size_t len)
> > return ret == xfers ? 0 : -1;
> > }
> >
> > +/**
> > + * drm_do_probe_acpi_edid() - get EDID information via ACPI _DDC
> > + * @data: struct drm_device
> > + * @buf: EDID data buffer to be filled
> > + * @block: 128 byte EDID block to start fetching from
> > + * @len: EDID data buffer length to fetch
> > + *
> > + * Try to fetch EDID information by calling acpi_video_get_edid() function.
> > + *
> > + * Return: 0 on success or error code on failure.
> > + */
> > +static int
> > +drm_do_probe_acpi_edid(void *data, u8 *buf, unsigned int block, size_t len)
> > +{
> > + struct drm_device *ddev = data;
> > + struct acpi_device *acpidev = ACPI_COMPANION(ddev->dev);
> > + unsigned char start = block * EDID_LENGTH;
> > + void *edid;
> > + int r;
> > +
> > + if (!acpidev)
> > + return -ENODEV;
> > +
> > + /* fetch the entire edid from BIOS */
> > + r = acpi_video_get_edid(acpidev, ACPI_VIDEO_DISPLAY_LCD, -1, );
> > + if (r < 0) {
> > + DRM_DEBUG_KMS("Failed to get EDID from ACPI: %d\n", r);
> > + return -EINVAL;
> > + }
> > + if (len > r || start > r || start + len > r) {
> > + r = -EINVAL;
> > + goto cleanup;
> > + }
> > +
> > + memcpy(buf, edid + start, len);
> > + r = 0;
> > +
> > +cleanup:
> > + kfree(edid);
> > +
> > + return r;
> > +}
> > +
> > static void connector_bad_edid(struct drm_connector *connector,
> >const struct edid *edid, int num_blocks)
> > {
> > @@ -2643,6 +2687,39 @@ struct edid *drm_get_edid(struct drm_connector
> > *connector,
> > }
> > EXPORT_SYMBOL(drm_get_edid);
> >
> > +/**
> > + * drm_get_acpi_edid - get EDID data, if available
>
> I'd prefer all the new EDID API to be named drm_edid_*. Makes a clean
> break from the old API, and is more consistent.
>
> So perhaps drm_edid_read_acpi() to be in line with all the other struct
> drm_edid based EDID reading functions.
>
> > + * @connector: connector we're probing
> > + *
> > + * Use the BIOS to attempt to grab EDID data if possible.
> > + *
> > + * The returned pointer must be freed using drm_edid_free().
> > + *
> > + * Return: Pointer to valid EDID or NULL if we couldn't find any.
> > + */
> > +const struct drm_edid *drm_get_acpi_edid(struct drm_connector *connector)
> > +{
> > + const struct drm_edid *drm_edid;
> > +
> > + switch (connector->connector_type) {
> > + case DRM_MODE_CONNECTOR_LVDS:
> > + case DRM_MODE_CONNECTOR_eDP:
> > + break;
> > + default:
> > + return NULL;
> > + }
> > +
> > + if (connector->force == DRM_FORCE_OFF)
> > + return NULL;
> > +
> > + drm_edid = drm_edid_read_custom(connector, drm_do_probe_acpi_edid,
> > connector->dev);
> > +
> > + /* Note: Do *not* call connector updates here. */
> > +
> > + return drm_edid;
> > +}
> > +EXPORT_SYMBOL(drm_get_acpi_edid);
Why shouldn't we use the BIOS/UEFI to retrieve them if it's available?
I guess what I'm asking is why should we make this an exported function
that drivers would have to call explicitly, instead of just making it
part of the usual EDID retrieval interface.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 5/7] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Hi,
On Tue, Jan 09, 2024 at 06:11:02PM +, Andri Yngvason wrote:
> From: Werner Sembach
>
> Add a new general drm property "preferred color format" which can be used
> by userspace to tell the graphic drivers to which color format to use.
>
> Possible options are:
> - auto (default/current behaviour)
> - rgb
> - ycbcr444
> - ycbcr422 (not supported by both amdgpu and i915)
> - ycbcr420
>
> In theory the auto option should choose the best available option for the
> current setup, but because of bad internal conversion some monitors look
> better with rgb and some with ycbcr444.
I looked at the patch and I couldn't find what is supposed to happen if
you set it to something else than auto, and the driver can't match that.
Are we supposed to fallback to the "auto" behaviour, or are we suppose
to reject the mode entirely?
The combination with the active output format property suggests the
former, but we should document it explicitly.
> Also, because of bad shielded connectors and/or cables, it might be
> preferable to use the less bandwidth heavy ycbcr422 and ycbcr420 formats
> for a signal that is less deceptible to interference.
>
> In the future, automatic color calibration for screens might also depend on
> this option being available.
>
> Signed-off-by: Werner Sembach
> Reported-by: Dan Carpenter
> Signed-off-by: Andri Yngvason
> Tested-by: Andri Yngvason
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 4 +++
> drivers/gpu/drm/drm_atomic_uapi.c | 4 +++
> drivers/gpu/drm/drm_connector.c | 50 -
> include/drm/drm_connector.h | 17 ++
> 4 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 68ffcc0b00dca..745a43d9c5da3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -707,6 +707,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> if (old_connector_state->max_requested_bpc !=
> new_connector_state->max_requested_bpc)
> new_crtc_state->connectors_changed = true;
> +
> + if (old_connector_state->preferred_color_format !=
> + new_connector_state->preferred_color_format)
> + new_crtc_state->connectors_changed = true;
> }
>
> if (funcs->atomic_check)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 98d3b10c08ae1..eba5dea1249e5 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -798,6 +798,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> state->max_requested_bpc = val;
> } else if (property == connector->privacy_screen_sw_state_property) {
> state->privacy_screen_sw_state = val;
> + } else if (property == connector->preferred_color_format_property) {
> + state->preferred_color_format = val;
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -881,6 +883,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = state->max_requested_bpc;
> } else if (property == connector->privacy_screen_sw_state_property) {
> *val = state->privacy_screen_sw_state;
> + } else if (property == connector->preferred_color_format_property) {
> + *val = state->preferred_color_format;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 30d62e505d188..4de48a38792cf 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1061,6 +1061,14 @@ static const struct drm_prop_enum_list
> drm_dp_subconnector_enum_list[] = {
> { DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */
> };
>
> +static const struct drm_prop_enum_list
> drm_preferred_color_format_enum_list[] = {
> + { 0, "auto" },
> + { DRM_COLOR_FORMAT_RGB444, "rgb" },
> + { DRM_COLOR_FORMAT_YCBCR444, "ycbcr444" },
> + { DRM_COLOR_FORMAT_YCBCR422, "ycbcr422" },
> + { DRM_COLOR_FORMAT_YCBCR420, "ycbcr420" },
> +};
> +
> static const struct drm_prop_enum_list drm_active_color_format_enum_list[] =
> {
> { 0, "not applicable" },
> { DRM_COLOR_FORMAT_RGB444, "rgb" },
> @@ -1398,11 +1406,20 @@ static const u32 dp_colorspaces =
> * drm_connector_attach_max_bpc_property() to create and attach the
> * property to the connector during initialization.
> *
> + * preferred
Re: [PATCH v5 00/32] drm/amd/display: add AMD driver-specific properties for color mgmt
On Fri, Dec 01, 2023 at 10:20:40AM -0500, Harry Wentland wrote: > > > On 2023-11-30 06:34, Daniel Vetter wrote: > > On Tue, 28 Nov 2023 at 23:11, Harry Wentland wrote: > >> > >> On 2023-11-16 14:57, Melissa Wen wrote: > >>> Hello, > >>> > >>> This series extends the current KMS color management API with AMD > >>> driver-specific properties to enhance the color management support on > >>> AMD Steam Deck. The key additions to the color pipeline include: > >>> > >> > >> snip > >> > >>> Melissa Wen (18): > >>> drm/drm_mode_object: increase max objects to accommodate new color > >>> props > >>> drm/drm_property: make replace_property_blob_from_id a DRM helper > >>> drm/drm_plane: track color mgmt changes per plane > >> > >> If all patches are merged through amd-staging-drm-next I worry that > >> conflicts creep in if any code around replace_property_blob_from_id > >> changes in DRM. > >> > >> My plan is to merge DRM patches through drm-misc-next, as well > >> as include them in the amd-staging-drm-next merge. They should then > >> fall out at the next amd-staging-drm-next pull and (hopefully) > >> ensure that there is no conflict. > >> > >> If no objections I'll go ahead with that later this week. > > > > Double-merging tends to be the worst because git doesn't realize the > > commits match, which actually makes the conflicts worse when they > > happen (because the 3-way merge diff gets absolute confused by all the > > changed context and misplaces everything to the max). So please don't, > > _only_ every cherry-pick when a patch in -next is also needed in > > -fixes, and we didn't put it into the right tree. But even that is a > > bit tricky and should only be done by maintainers (using dim > > cherry-pick if it's drm-misc) because the conflicts tend to be bad and > > need to be sorted out with backmerges sooner than later. > > > > For this case merge everything through one tree with the right acks, > > pull to drm-next asap and then backmerge into the other tree. Here > > probably amdgpu-next with drm-misc maintainer acks for the 3 core > > patches. Or if amdgpu-next pull won't come for a while, put them into > > drm-misc-next and just wait a week until it's in drm-next, then > > forward amdgpu-next. > > > > Maxime, Maarten, Thomas, could I get an ACK from you for the three > DRM core patches and an ACK for pulling them through the AMD tree? Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH] MAINTAINERS: drm/ci: add entries for xfail files
On Tue, 19 Sep 2023 15:22:49 -0300, Helen Koike wrote: > DRM CI keeps track of which tests are failing, flaking or being skipped > by the ci in the expectations files. Add entries for those files to the > corresponding driver maintainer, so they can be notified when they > change. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH v2] MAINTAINERS: Update drm-misc entry to match all drivers
Hi, On Thu, Sep 21, 2023 at 12:57:43PM +0200, Maxime Ripard wrote: > We've had a number of times when a patch slipped through and we couldn't > pick them up either because our MAINTAINERS entry only covers the > framework and thus we weren't Cc'd. > > Let's take another approach where we match everything, and remove all > the drivers that are not maintained through drm-misc. > > Acked-by: Jani Nikula > Signed-off-by: Maxime Ripard Applied with Dave's Acked-by given on IRC. This was conflicting with https://lore.kernel.org/r/20230925154929.1.I3287e895ce8e68d41b458494a49a1b5ec5c71013@changeid So I removed the imx exclusion from that list while applying. Maxime signature.asc Description: PGP signature
[PATCH v2] MAINTAINERS: Update drm-misc entry to match all drivers
We've had a number of times when a patch slipped through and we couldn't pick them up either because our MAINTAINERS entry only covers the framework and thus we weren't Cc'd. Let's take another approach where we match everything, and remove all the drivers that are not maintained through drm-misc. Acked-by: Jani Nikula Signed-off-by: Maxime Ripard --- Cc: Alex Deucher Cc: Christian König Cc: "Pan, Xinhui" Cc: Russell King Cc: Lucas Stach Cc: Christian Gmeiner Cc: Inki Dae Cc: Seung-Woo Kim Cc: Kyungmin Park Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: Tvrtko Ursulin Cc: Philipp Zabel Cc: Laurentiu Palcu Cc: Anitha Chrisanthus Cc: Edmund Dea Cc: Chun-Kuang Hu Cc: Rob Clark Cc: Abhinav Kumar Cc: Dmitry Baryshkov Cc: Sean Paul Cc: Marijn Suijten Cc: Ben Skeggs Cc: Karol Herbst Cc: Lyude Paul Cc: Laurent Pinchart Cc: Kieran Bingham Cc: Thierry Reding Cc: Mikko Perttunen Cc: dri-de...@lists.freedesktop.org Cc: amd-gfx@lists.freedesktop.org Cc: etna...@lists.freedesktop.org Cc: intel-...@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: linux-renesas-...@vger.kernel.org Cc: linux-te...@vger.kernel.org Changes from v1: - Remove ingenic and gma500 from the excluded list --- MAINTAINERS | 21 ++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 90f13281d297..1012402dada5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6860,12 +6860,27 @@ M: Thomas Zimmermann S: Maintained W: https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/ +F: Documentation/devicetree/bindings/gpu/ F: Documentation/gpu/ -F: drivers/gpu/drm/* +F: drivers/gpu/drm/ F: drivers/gpu/vga/ -F: include/drm/drm* +F: include/drm/drm F: include/linux/vga* -F: include/uapi/drm/drm* +F: include/uapi/drm/ +X: drivers/gpu/drm/amd/ +X: drivers/gpu/drm/armada/ +X: drivers/gpu/drm/etnaviv/ +X: drivers/gpu/drm/exynos/ +X: drivers/gpu/drm/i915/ +X: drivers/gpu/drm/imx/ +X: drivers/gpu/drm/kmb/ +X: drivers/gpu/drm/mediatek/ +X: drivers/gpu/drm/msm/ +X: drivers/gpu/drm/nouveau/ +X: drivers/gpu/drm/radeon/ +X: drivers/gpu/drm/renesas/ +X: drivers/gpu/drm/tegra/ DRM DRIVERS FOR ALLWINNER A10 M: Maxime Ripard -- 2.41.0
Re: [PATCH] MAINTAINERS: drm/ci: add entries for xfail files
Hi, On Tue, Sep 19, 2023 at 03:22:49PM -0300, Helen Koike wrote: > DRM CI keeps track of which tests are failing, flaking or being skipped > by the ci in the expectations files. Add entries for those files to the > corresponding driver maintainer, so they can be notified when they > change. > > Signed-off-by: Helen Koike Thanks for sending this Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH v3 4/5] drm/vc4: add trailing newlines to drm_dbg msgs
On Wed, 6 Sep 2023 13:02:22 -0600, Jim Cromie wrote: > By at least strong convention, a print-buffer's trailing newline says > "message complete, send it". The exception (no TNL, followed by a call > to pr_cont) proves the general rule. > > Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG, > > [ ... ] Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH v3 1/5] drm/connector: add trailing newlines to drm_dbg msgs
On Wed, Sep 06, 2023 at 01:02:19PM -0600, Jim Cromie wrote: > By at least strong convention, a print-buffer's trailing newline says > "message complete, send it". The exception (no TNL, followed by a call > to pr_cont) proves the general rule. > > Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG, > 1288 drm_dbg. Clean up the remainders, in maintainer sized chunks. > > No functional changes. > > Signed-off-by: Jim Cromie > --- > drivers/gpu/drm/drm_connector.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index f28725736237..14020585bdc0 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -2925,7 +2925,9 @@ int drm_mode_getconnector(struct drm_device *dev, void > *data, >dev->mode_config.max_width, > > dev->mode_config.max_height); > else > - drm_dbg_kms(dev, "User-space requested a forced probe > on [CONNECTOR:%d:%s] but is not the DRM master, demoting to read-only probe", > + drm_dbg_kms(dev, > + "User-space requested a forced probe on > [CONNECTOR:%d:%s] " > + "but is not the DRM master, demoting to > read-only probe\n", > connector->base.id, connector->name); I'm fine with the general idea behind this patch, but we shouldn't break the message itself. See https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings Maxime signature.asc Description: PGP signature
Re: [RFT PATCH 14/15] drm/radeon: Call drm_helper_force_disable_all() at shutdown/remove time
On Fri, 1 Sep 2023 16:41:25 -0700, Douglas Anderson wrote: > Based on grepping through the source code, this driver appears to be > missing a call to drm_atomic_helper_shutdown(), or in this case the > non-atomic equivalent drm_helper_force_disable_all(), at system > shutdown time and at driver remove time. This is important because > drm_helper_force_disable_all() will cause panels to get disabled > > [ ... ] Reviewed-by: Maxime Ripard Thanks! Maxime
Re: [PATCH (set 1) 00/20] Rid W=1 warnings from GPU
On Thu, Aug 24, 2023 at 12:03:20PM +0300, Jani Nikula wrote: > On Thu, 24 Aug 2023, Lee Jones wrote: > > This set is part of a larger effort attempting to clean-up W=1 > > kernel builds, which are currently overwhelmingly riddled with > > niggly little warnings. > > The next question is, how do we keep it W=1 clean going forward? > > Most people don't use W=1 because it's too noisy, so it's a bit of a > catch-22. > > In i915, we enable a lot of W=1 warnings using subdir-ccflags-y in our > Makefile. For CI/developer use we also enable kernel-doc warnings by > default. > > Should we start enabling some of those warning flags in drm/Makefile to > to keep the entire subsystem warning free? I think that's a good idea. At least the documentation fixes I just merged could have been easily caught before submission. Maxime signature.asc Description: PGP signature
Re: [PATCH (set 1) 00/20] Rid W=1 warnings from GPU
Hi, On Thu, Aug 24, 2023 at 10:59:54AM +0200, Maxime Ripard wrote: > On Thu, 24 Aug 2023 08:36:45 +0100, Lee Jones wrote: > > This set is part of a larger effort attempting to clean-up W=1 > > kernel builds, which are currently overwhelmingly riddled with > > niggly little warnings. > > > > Cc: Alex Deucher > > Cc: amd-gfx@lists.freedesktop.org > > Cc: Ben Skeggs > > Cc: "Christian König" > > Cc: Daniel Vetter > > Cc: Danilo Krummrich > > Cc: David Airlie > > Cc: dri-de...@lists.freedesktop.org > > Cc: Fabio Estevam > > Cc: Gourav Samaiya > > Cc: Hawking Zhang > > Cc: Hyun Kwon > > Cc: Jerome Glisse > > Cc: Jonathan Hunter > > Cc: Karol Herbst > > Cc: Laurent Pinchart > > Cc: linaro-mm-...@lists.linaro.org > > Cc: linux-arm-ker...@lists.infradead.org > > Cc: linux-me...@vger.kernel.org > > Cc: linux-te...@vger.kernel.org > > Cc: Luben Tuikov > > Cc: Lyude Paul > > Cc: Maarten Lankhorst > > Cc: "Maíra Canal" > > Cc: Mario Limonciello > > Cc: Maxime Ripard > > Cc: Michal Simek > > Cc: Mikko Perttunen > > Cc: nouv...@lists.freedesktop.org > > Cc: NXP Linux Team > > Cc: "Pan, Xinhui" > > Cc: Pengutronix Kernel Team > > Cc: Philipp Zabel > > Cc: Sascha Hauer > > Cc: Shashank Sharma > > Cc: Shawn Guo > > Cc: Stanley Yang > > Cc: Sumit Semwal > > Cc: Thierry Reding > > Cc: Thomas Zimmermann > > > > [...] > > Applied to drm/drm-misc (drm-misc-fixes). I got confused with b4 usage, but that wasn't actually applied. Only the three patches I explicitly mentioned were, sorry for the confusion. Maxime signature.asc Description: PGP signature
Re: [PATCH (set 1) 00/20] Rid W=1 warnings from GPU
On Thu, 24 Aug 2023 08:36:45 +0100, Lee Jones wrote: > This set is part of a larger effort attempting to clean-up W=1 > kernel builds, which are currently overwhelmingly riddled with > niggly little warnings. > > Cc: Alex Deucher > Cc: amd-gfx@lists.freedesktop.org > Cc: Ben Skeggs > Cc: "Christian König" > Cc: Daniel Vetter > Cc: Danilo Krummrich > Cc: David Airlie > Cc: dri-de...@lists.freedesktop.org > Cc: Fabio Estevam > Cc: Gourav Samaiya > Cc: Hawking Zhang > Cc: Hyun Kwon > Cc: Jerome Glisse > Cc: Jonathan Hunter > Cc: Karol Herbst > Cc: Laurent Pinchart > Cc: linaro-mm-...@lists.linaro.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-me...@vger.kernel.org > Cc: linux-te...@vger.kernel.org > Cc: Luben Tuikov > Cc: Lyude Paul > Cc: Maarten Lankhorst > Cc: "Maíra Canal" > Cc: Mario Limonciello > Cc: Maxime Ripard > Cc: Michal Simek > Cc: Mikko Perttunen > Cc: nouv...@lists.freedesktop.org > Cc: NXP Linux Team > Cc: "Pan, Xinhui" > Cc: Pengutronix Kernel Team > Cc: Philipp Zabel > Cc: Sascha Hauer > Cc: Shashank Sharma > Cc: Shawn Guo > Cc: Stanley Yang > Cc: Sumit Semwal > Cc: Thierry Reding > Cc: Thomas Zimmermann > > [...] Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
Re: [PATCH] video/hdmi: convert *_infoframe_init() functions to void
Hi,
On Tue, Aug 08, 2023 at 11:02:45AM -0700, Nikita Zhandarovich wrote:
> Four hdmi_*_infoframe_init() functions that initialize different
> types of hdmi infoframes only return the default 0 value, contrary to
> their descriptions. Yet these functions are still unnecessarily checked
> against possible errors in case of failure.
>
> Remove redundant error checks in calls to following functions:
> - hdmi_spd_infoframe_init
> - hdmi_audio_infoframe_init
> - hdmi_vendor_infoframe_init
> - hdmi_drm_infoframe_init
> Also, convert these functions to 'void' and fix their descriptions.
I'm not sure what value it actually adds. None of them return any
errors, but very well might if we started to be a bit serious about it.
Since the error handling is already there, then I'd rather leave it
there.
> Fixes: 2c676f378edb ("[media] hdmi: added unpack and logging functions for
> InfoFrames")
I'm confused about that part. What does it fix exactly?
Maxime
signature.asc
Description: PGP signature
Re: [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Thu, Jul 13, 2023 at 04:14:55PM +0100, Tvrtko Ursulin wrote: > > On 13/07/2023 16:09, Thomas Zimmermann wrote: > > Hi > > > > Am 13.07.23 um 16:41 schrieb Sean Paul: > > > On Thu, Jul 13, 2023 at 9:04 AM Uwe Kleine-König > > > wrote: > > > > > > > > hello Sean, > > > > > > > > On Wed, Jul 12, 2023 at 02:31:02PM -0400, Sean Paul wrote: > > > > > I'd really prefer this patch (series or single) is not accepted. This > > > > > will cause problems for everyone cherry-picking patches to a > > > > > downstream kernel (LTS or distro tree). I usually wouldn't expect > > > > > sympathy here, but the questionable benefit does not outweigh the cost > > > > > IM[biased]O. > > > > > > > > I agree that for backports this isn't so nice. However with the split > > > > approach (that was argumented against here) it's not soo bad. Patch #1 > > > > (and similar changes for the other affected structures) could be > > > > trivially backported and with that it doesn't matter if you write dev or > > > > drm (or whatever name is chosen in the end); both work in the same way. > > > > > > Patch #1 avoids the need to backport the entire set, however every > > > change occuring after the rename patches will cause conflicts on > > > future cherry-picks. Downstream kernels will have to backport the > > > whole set. Backporting the entire set will create an epoch in > > > downstream kernels where cherry-picking patches preceding this set > > > will need to undergo conflict resolution as well. As mentioned in my > > > previous email, I don't expect sympathy here, it's part of maintaining > > > a downstream kernel, but there is a real cost to kernel consumers. > > > > > > > > > > > But even with the one-patch-per-rename approach I'd consider the > > > > renaming a net win, because ease of understanding code has a big value. > > > > It's value is not so easy measurable as "conflicts when backporting", > > > > but it also matters in say two years from now, while backporting > > > > shouldn't be an issue then any more. > > > > > > You've rightly identified the conjecture in your statement. I've been > > > on both sides of the argument, having written/maintained drm code > > > upstream and cherry-picked changes to a downstream kernel. Perhaps > > > it's because drm's definition of dev is ingrained in my muscle memory, > > > or maybe it's because I don't do a lot of upstream development these > > > days, but I just have a hard time seeing the benefit here. > > > > I can only second what Sean writes. I've done quite a bit of backporting > > of DRM code. It's hard already. And this kind of change is going to to > > affect almost every backported DRM patch in the coming years. Not just > > for distribution kernels, but also for upstream's stable series. It's > > really only possible to do this change over many releases while keeping > > compatible with the old name. So the more I think about it, the less I > > like this change. > > I've done my share of backporting, and still am doing it, so I can say I > dislike it as much as anyone, however.. Is this an argument which the kernel > as a wider entity typically accepts? If not could it be a slippery slope to > start a precedent? > > It is a honest question - I am not familiar if there were or were not any > similar discussions in the past. Eventually, it's a trade-off. There's always pros and cons to merging every patch, and "backporting pains" is indeed not a very strong con. But it's definitely the kind of patch where everyone and their mother will have their opinion, without every reaching a clear consensus, and there's no clear benefit either (but I might be biaised on that one). So imo, while that downside is fairly weak, the pros are certainly weaker. > My gut feeling is that *if* there is a consensus that something _improves_ > the code base significantly, backporting pains should probably not be > weighted very heavily as a contra argument. 100% agreed here, but I'm afraid we're far from that point. Maxime signature.asc Description: PGP signature
Re: [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Wed, Jul 12, 2023 at 03:38:03PM +0200, Uwe Kleine-König wrote: > Hello Maxime, > > On Wed, Jul 12, 2023 at 02:52:38PM +0200, Maxime Ripard wrote: > > On Wed, Jul 12, 2023 at 01:02:53PM +0200, Uwe Kleine-König wrote: > > > > Background is that this makes merge conflicts easier to handle and > > > > detect. > > > > > > Really? > > > > FWIW, I agree with Christian here. > > > > > Each file (apart from include/drm/drm_crtc.h) is only touched once. So > > > unless I'm missing something you don't get less or easier conflicts by > > > doing it all in a single patch. But you gain the freedom to drop a > > > patch for one driver without having to drop the rest with it. > > > > Not really, because the last patch removed the union anyway. So you have > > to revert both the last patch, plus that driver one. And then you need > > to add a TODO to remove that union eventually. > > Yes, with a single patch you have only one revert (but 194 files changed, > 1264 insertions(+), 1296 deletions(-)) instead of two (one of them: 1 > file changed, 9 insertions(+), 1 deletion(-); the other maybe a bit > bigger). (And maybe you get away with just reverting the last patch.) > > With a single patch the TODO after a revert is "redo it all again (and > prepare for a different set of conflicts)" while with the split series > it's only "fix that one driver that was forgotten/borked" + reapply that > 10 line patch. As the one who gets that TODO, I prefer the latter. > > So in sum: If your metric is "small count of reverted commits", you're > right. If however your metric is: Better get 95% of this series' change > in than maybe 0%, the split series is the way to do it. I guess that's where we disagree: I don't see the point of having 95% of it, either 0 or 100. > With me having spend ~3h on this series' changes, it's maybe > understandable that I did it the way I did. I'm sorry, but that's never been an argument? I'm sure you and I both have had series that took much longer dropped because it wasn't the right approach. > FTR: This series was created on top of v6.5-rc1. If you apply it to > drm-misc-next you get a (trivial) conflict in patch #2. If I consider to > be the responsible maintainer who applies this series, I like being able > to just do git am --skip then. Or we can ask that the driver is based on drm-misc-next ... > FTR#2: In drm-misc-next is a new driver > (drivers/gpu/drm/loongson/lsdc_crtc.c) so skipping the last patch for > now might indeed be a good idea. ... which is going to fix that one too. Maxime signature.asc Description: PGP signature
Re: [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Wed, Jul 12, 2023 at 01:02:53PM +0200, Uwe Kleine-König wrote: > > Background is that this makes merge conflicts easier to handle and detect. > > Really? FWIW, I agree with Christian here. > Each file (apart from include/drm/drm_crtc.h) is only touched once. So > unless I'm missing something you don't get less or easier conflicts by > doing it all in a single patch. But you gain the freedom to drop a > patch for one driver without having to drop the rest with it. Not really, because the last patch removed the union anyway. So you have to revert both the last patch, plus that driver one. And then you need to add a TODO to remove that union eventually. > So I still like the split version better, but I'm open to a more > verbose reasoning from your side. You're doing only one thing here, really: you change the name of a structure field. If it was shared between multiple maintainers, then sure, splitting that up is easier for everyone, but this will go through drm-misc, so I can't see the benefit it brings. Maxime signature.asc Description: PGP signature
Re: [PATCH 1/2] drm/vc4: Fix build error with undefined label
On Wed, Mar 08, 2023 at 04:27:01PM +, Zhuo, Qingqing (Lillian) wrote: > [AMD Official Use Only - General] > > > Hi, > > On Wed, Mar 08, 2023 at 11:11:22AM -0500, Hamza Mahfooz wrote: > > + vc4 maintainers > > > > On 3/8/23 04:34, Qingqing Zhuo wrote: > > > [Why] > > > drivers/gpu/drm/vc4/vc4_hdmi.c: In function ‘vc4_hdmi_bind’: > > > drivers/gpu/drm/vc4/vc4_hdmi.c:3448:17: error: label > > > ‘err_disable_runtime_pm’ used but not defined > > > > > > [How] > > > update err_disable_runtime_pm to err_put_runtime_pm. > > > > > > Signed-off-by: Qingqing Zhuo > > > --- > > > drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c > > > b/drivers/gpu/drm/vc4/vc4_hdmi.c index 9e145690c480..edf882360d24 > > > 100644 > > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > > > @@ -3445,7 +3445,7 @@ static int vc4_hdmi_bind(struct device *dev, struct > > > device *master, void *data) > > >*/ > > > ret = pm_runtime_resume_and_get(dev); > > > if (ret) > > > - goto err_disable_runtime_pm; > > > + goto err_put_runtime_pm; > > > if ((of_device_is_compatible(dev->of_node, > > > "brcm,bcm2711-hdmi0") || > > >of_device_is_compatible(dev->of_node, > > > "brcm,bcm2711-hdmi1")) > > > && > > > The current drm-misc-next branch doesn't have that context at all. What > > tree is this based on? > > This is for amd-staging-drm-next. I don't get it, why is there a vc4 patch in an AMD tree? Maxime signature.asc Description: PGP signature
Re: [PATCH 1/2] drm/vc4: Fix build error with undefined label
Hi, On Wed, Mar 08, 2023 at 11:11:22AM -0500, Hamza Mahfooz wrote: > + vc4 maintainers > > On 3/8/23 04:34, Qingqing Zhuo wrote: > > [Why] > > drivers/gpu/drm/vc4/vc4_hdmi.c: In function ‘vc4_hdmi_bind’: > > drivers/gpu/drm/vc4/vc4_hdmi.c:3448:17: error: label > > ‘err_disable_runtime_pm’ used but not defined > > > > [How] > > update err_disable_runtime_pm to err_put_runtime_pm. > > > > Signed-off-by: Qingqing Zhuo > > --- > > drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > > index 9e145690c480..edf882360d24 100644 > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > > @@ -3445,7 +3445,7 @@ static int vc4_hdmi_bind(struct device *dev, struct > > device *master, void *data) > > */ > > ret = pm_runtime_resume_and_get(dev); > > if (ret) > > - goto err_disable_runtime_pm; > > + goto err_put_runtime_pm; > > if ((of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi0") || > > of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi1")) && The current drm-misc-next branch doesn't have that context at all. What tree is this based on? Maxime
Re: [PATCH 0/5] drm: Do not include unnecessarily
On Mon, Jan 09, 2023 at 11:12:38AM +0100, Thomas Zimmermann wrote: > Remove unnecessary include statements for . I recently > changed this header and had to rebuild a good part of DRM. So avoid > this by removing the dependency. > > Some source files require the OF or backlight headers. Include those > instead. For the series: Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [GIT PULL] Immutable backlight-detect-refactor branch between acpi, drm-* and pdx86
Hi Hans, On Mon, Sep 05, 2022 at 10:35:47AM +0200, Hans de Goede wrote: > Hi All, > > Now that all patches have been reviewed/acked here is an immutable > backlight-detect-refactor > branch with 6.0-rc1 + the v5 patch-set, for merging into the relevant (acpi, > drm-* and pdx86) > subsystems. > > Please pull this branch into the relevant subsystems. > > I will merge this into the review-hans branch of the pdx86 git tree today and > from there it will move to for-next once the builders have successfully > build-tested > the merge. I merged it into drm-misc-next, thanks! Maxime signature.asc Description: PGP signature
[PATCH v2 05/22] drm/amd/display: Fix color encoding mismatch
The amdgpu KMS driver calls drm_plane_create_color_properties() with a default encoding set to BT709. However, the core will ignore it and the driver doesn't force it in its plane state reset hook, so the initial value will be 0, which represents BT601. Fix the mismatch by using an initial value of BT601 in drm_plane_create_color_properties(). Cc: amd-gfx@lists.freedesktop.org Cc: Alex Deucher Cc: "Christian König" Cc: Harry Wentland Cc: Leo Li Cc: "Pan, Xinhui" Cc: Rodrigo Siqueira Signed-off-by: Maxime Ripard --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index feccf2b555d2..86b27a355e90 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, BIT(DRM_COLOR_YCBCR_BT2020), BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) | BIT(DRM_COLOR_YCBCR_FULL_RANGE), - DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE); + DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE); } supported_rotations = -- 2.35.1
Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch
Hi Harry, On Thu, Feb 10, 2022 at 09:38:24AM -0500, Harry Wentland wrote: > On 2022-02-10 03:42, Maxime Ripard wrote: > > On Mon, Feb 07, 2022 at 01:59:38PM -0500, Harry Wentland wrote: > >> On 2022-02-07 13:57, Harry Wentland wrote: > >>> On 2022-02-07 11:34, Maxime Ripard wrote: > >>>> The amdgpu KMS driver calls drm_plane_create_color_properties() with a > >>>> default encoding set to BT709. > >>>> > >>>> However, the core will ignore it and the driver doesn't force it in its > >>>> plane state reset hook, so the initial value will be 0, which represents > >>>> BT601. > >>>> > >>> > >>> Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset > >>> reset all plane_state members to their properties' default values? > >>> > >> > >> Ah, looks like that's exactly what you do in the later patches, which is > >> perfect. With that, I don't think you'll need this patch anymore. > > > > Ok, I'll squash it into the patch that removes the reset code. > > > > I don't think that's right. I think we can just drop this patch. > The amdgpu display driver is not doing BT601 by default. My understanding from the code currently in tree is that: 1) amdgpu_dm_plane_init() will call drm_plane_create_color_properties() with an initial value set to BT709. 2) dm_drm_plane_reset() will use kzalloc and then just rely on __drm_atomic_helper_plane_reset(), which will not set the color encoding at all. It's thus 0 in the initial state. 3) the drm_color_encoding enum will have BT601 associated to 0 So it does look like the default for amdgpu at the moment is BT601? Maxime signature.asc Description: PGP signature
Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch
Hi Harry, On Mon, Feb 07, 2022 at 01:59:38PM -0500, Harry Wentland wrote: > On 2022-02-07 13:57, Harry Wentland wrote: > > On 2022-02-07 11:34, Maxime Ripard wrote: > >> The amdgpu KMS driver calls drm_plane_create_color_properties() with a > >> default encoding set to BT709. > >> > >> However, the core will ignore it and the driver doesn't force it in its > >> plane state reset hook, so the initial value will be 0, which represents > >> BT601. > >> > > > > Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset > > reset all plane_state members to their properties' default values? > > > > Ah, looks like that's exactly what you do in the later patches, which is > perfect. With that, I don't think you'll need this patch anymore. Ok, I'll squash it into the patch that removes the reset code. Thanks! Maxime signature.asc Description: PGP signature
[PATCH 05/23] drm/amd/display: Fix color encoding mismatch
The amdgpu KMS driver calls drm_plane_create_color_properties() with a default encoding set to BT709. However, the core will ignore it and the driver doesn't force it in its plane state reset hook, so the initial value will be 0, which represents BT601. Fix the mismatch by using an initial value of BT601 in drm_plane_create_color_properties(). Cc: amd-gfx@lists.freedesktop.org Cc: Alex Deucher Cc: "Christian König" Cc: Harry Wentland Cc: Leo Li Cc: "Pan, Xinhui" Cc: Rodrigo Siqueira Signed-off-by: Maxime Ripard --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index feccf2b555d2..86b27a355e90 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, BIT(DRM_COLOR_YCBCR_BT2020), BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) | BIT(DRM_COLOR_YCBCR_FULL_RANGE), - DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE); + DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE); } supported_rotations = -- 2.34.1
Re: [PATCH 01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate
On Thu, Nov 04, 2021 at 05:41:13PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 04, 2021 at 09:48:41AM +0100, Maxime Ripard wrote:
> > Hi Ville,
> >
> > On Wed, Nov 03, 2021 at 08:05:16PM +0200, Ville Syrjälä wrote:
> > > On Wed, Nov 03, 2021 at 01:02:11PM +0200, Ville Syrjälä wrote:
> > > > On Tue, Nov 02, 2021 at 03:59:32PM +0100, Maxime Ripard wrote:
> > > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > > @@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct
> > > > > drm_connector *connector,
> > > > > u32 max_tmds_clock = hf_vsdb[5] * 5000;
> > > > > struct drm_scdc *scdc = >scdc;
> > > > >
> > > > > - if (max_tmds_clock > 34) {
> > > > > + if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> > > > > display->max_tmds_clock = max_tmds_clock;
> > > > > DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d
> > > > > kHz\n",
> > > > > display->max_tmds_clock);
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > > > b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > > > index d2e61f6c6e08..0666203d52b7 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > > > @@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct
> > > > > intel_encoder *encoder,
> > > > > if (scdc->scrambling.low_rates)
> > > > > pipe_config->hdmi_scrambling = true;
> > > > >
> > > > > - if (pipe_config->port_clock > 34) {
> > > > > + if (pipe_config->port_clock >
> > > > > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> > > > > pipe_config->hdmi_scrambling = true;
> > > > > pipe_config->hdmi_high_tmds_clock_ratio = true;
> > > > > }
> > > >
> > > > All of that is HDMI 2.0 stuff. So this just makes it all super
> > > > confusing IMO. Nak.
> > >
> > > So reading throgh HDMI 1.4 again it does specify 340 MHz as some kind
> > > of upper limit for the physical cable. But nowhere else is that number
> > > really mentioned AFAICS. HDMI 2.0 does talk quite a bit about the 340
> > > Mcsc limit in various places.
> > >
> > > I wonder what people would think of a couple of helpers like:
> > > - drm_hdmi_{can,must}_use_scrambling()
> > > - drm_hdmi_is_high_tmds_clock_ratio()
> > > or something along those lines? At least with those the code would
> > > read decently and I wouldn't have to wonder what this HDMI 1.4 TMDS
> > > clock limit really is.
> >
> > Patch 2 introduces something along those lines.
> >
> > It doesn't cover everything though, we're using this define in vc4 to
> > limit the available modes in mode_valid on HDMI controllers not
> > 4k-capable
>
> I wouldn't want to use this kind of define for those kinds of checks
> anyway. If the hardware has specific limits in what kind of clocks it
> can generate (or what it was validated for) IMO you should spell
> those out explicitly instead of assuming they happen to match
> some standard defined max value.
AFAIK, in the vc4 case, this is the hardware limit.
And there's other cases where it still seems to make sense to have that
define:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_edid.c#n4978
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_encoders.c#n385
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c#n1174
etc..
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate
Hi Ville,
On Wed, Nov 03, 2021 at 08:05:16PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 03, 2021 at 01:02:11PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 02, 2021 at 03:59:32PM +0100, Maxime Ripard wrote:
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct
> > > drm_connector *connector,
> > > u32 max_tmds_clock = hf_vsdb[5] * 5000;
> > > struct drm_scdc *scdc = >scdc;
> > >
> > > - if (max_tmds_clock > 34) {
> > > + if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> > > display->max_tmds_clock = max_tmds_clock;
> > > DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
> > > display->max_tmds_clock);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > index d2e61f6c6e08..0666203d52b7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > @@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct intel_encoder
> > > *encoder,
> > > if (scdc->scrambling.low_rates)
> > > pipe_config->hdmi_scrambling = true;
> > >
> > > - if (pipe_config->port_clock > 34) {
> > > + if (pipe_config->port_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> > > pipe_config->hdmi_scrambling = true;
> > > pipe_config->hdmi_high_tmds_clock_ratio = true;
> > > }
> >
> > All of that is HDMI 2.0 stuff. So this just makes it all super
> > confusing IMO. Nak.
>
> So reading throgh HDMI 1.4 again it does specify 340 MHz as some kind
> of upper limit for the physical cable. But nowhere else is that number
> really mentioned AFAICS. HDMI 2.0 does talk quite a bit about the 340
> Mcsc limit in various places.
>
> I wonder what people would think of a couple of helpers like:
> - drm_hdmi_{can,must}_use_scrambling()
> - drm_hdmi_is_high_tmds_clock_ratio()
> or something along those lines? At least with those the code would
> read decently and I wouldn't have to wonder what this HDMI 1.4 TMDS
> clock limit really is.
Patch 2 introduces something along those lines.
It doesn't cover everything though, we're using this define in vc4 to
limit the available modes in mode_valid on HDMI controllers not
4k-capable
We could probably do better on the name, but I still believe a define
like this would be valuable.
Maxime
signature.asc
Description: PGP signature
[PATCH 01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate
A lot of drivers open-code the HDMI 1.4 maximum pixel rate in their
driver to test whether the resolutions are supported or if the
scrambling needs to be enabled.
Let's create a common define for everyone to use it.
Cc: Alex Deucher
Cc: amd-gfx@lists.freedesktop.org
Cc: Andrzej Hajda
Cc: Benjamin Gaignard
Cc: "Christian König"
Cc: Emma Anholt
Cc: intel-...@lists.freedesktop.org
Cc: Jani Nikula
Cc: Jernej Skrabec
Cc: Jerome Brunet
Cc: Jonas Karlman
Cc: Jonathan Hunter
Cc: Joonas Lahtinen
Cc: Kevin Hilman
Cc: Laurent Pinchart
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-te...@vger.kernel.org
Cc: Martin Blumenstingl
Cc: Neil Armstrong
Cc: "Pan, Xinhui"
Cc: Robert Foss
Cc: Rodrigo Vivi
Cc: Thierry Reding
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++--
drivers/gpu/drm/drm_edid.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/meson/meson_dw_hdmi.c | 4 ++--
drivers/gpu/drm/radeon/radeon_encoders.c | 2 +-
drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.c | 2 +-
drivers/gpu/drm/tegra/sor.c| 8
drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ++--
include/drm/drm_connector.h| 2 ++
9 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 62ae63565d3a..3a58db357be0 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -46,7 +46,7 @@
/* DW-HDMI Controller >= 0x200a are at least compliant with SCDC version 1 */
#define SCDC_MIN_SOURCE_VERSION0x1
-#define HDMI14_MAX_TMDSCLK 34000
+#define HDMI14_MAX_TMDSCLK (DRM_HDMI_14_MAX_TMDS_CLK_KHZ * 1000)
enum hdmi_datamap {
RGB444_8B = 0x01,
@@ -1264,7 +1264,7 @@ static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi,
* for low rates is not supported either
*/
if (!display->hdmi.scdc.scrambling.low_rates &&
- display->max_tmds_clock <= 34)
+ display->max_tmds_clock <= DRM_HDMI_14_MAX_TMDS_CLK_KHZ)
return false;
return true;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7aa2a56a71c8..ec8fb2d098ae 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct
drm_connector *connector,
u32 max_tmds_clock = hf_vsdb[5] * 5000;
struct drm_scdc *scdc = >scdc;
- if (max_tmds_clock > 34) {
+ if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
display->max_tmds_clock = max_tmds_clock;
DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
display->max_tmds_clock);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index d2e61f6c6e08..0666203d52b7 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct intel_encoder
*encoder,
if (scdc->scrambling.low_rates)
pipe_config->hdmi_scrambling = true;
- if (pipe_config->port_clock > 34) {
+ if (pipe_config->port_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
pipe_config->hdmi_scrambling = true;
pipe_config->hdmi_high_tmds_clock_ratio = true;
}
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 0afbd1e70bfc..8078667aea0e 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -434,7 +434,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void
*data,
readl_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING));
DRM_DEBUG_DRIVER("\"%s\" div%d\n", mode->name,
-mode->clock > 34 ? 40 : 10);
+mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ ? 40 : 10);
/* Enable clocks */
regmap_update_bits(priv->hhi, HHI_HDMI_CLK_CNTL, 0x, 0x100);
@@ -457,7 +457,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void
*data,
dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12));
/* TMDS pattern setup */
- if (mode->clock > 34 &&
+ if (mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ &&
dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_YUV8_1X24) {
dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01,
0);
diff --git a/drivers/gpu/drm/radeo
Re: New uAPI for color management proposal and feedback request
Hi Pekka, On Mon, Jun 07, 2021 at 11:06:32AM +0300, Pekka Paalanen wrote: > On Mon, 7 Jun 2021 09:48:05 +0200 > Maxime Ripard wrote: > > > I've started to implement this for the raspberrypi some time ago. > > > > https://github.com/raspberrypi/linux/pull/4201 > > > > It's basically two properties: a bitmask of the available output pixel > > encoding to report both what the display and the controller supports, > > and one to actually set what the userspace wants to get enforced (and > > that would return the active one when read). > > Hi Maxime, > > I would like to point out that I think it is a bad design to create a > read/write property that returns not what was written to it. It can > cause headaches to userspace that wants to save and restore property > values it does not understand. Userspace would want to do that to > mitigate damage from switching to another KMS client and then back. The > other KMS client could change properties the first KMS client does not > understand, causing the first KMS client to show incorrectly after > switching back. > > Please, consider whether this use-case will work before designing a > property where read-back may not necessarily return the written value. Thanks for bringing that up. I guess the work being done currently by Werner and his active color format property addresses that concern :) Maxime ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 1/7] drm/sysfs: introduce drm_sysfs_connector_hotplug_event
Hi,
On Wed, Jun 09, 2021 at 09:23:27PM +, Simon Ser wrote:
> This function sends a hotplug uevent with a CONNECTOR property.
>
> Signed-off-by: Simon Ser
> ---
> drivers/gpu/drm/drm_sysfs.c | 25 +
> include/drm/drm_sysfs.h | 1 +
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 968a9560b4aa..8423e44c3035 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -343,6 +343,31 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_sysfs_hotplug_event);
>
> +/**
> + * drm_sysfs_connector_hotplug_event - generate a DRM uevent for any
> connector
> + * change
> + * @connector: connector which has changed
> + *
> + * Send a uevent for the DRM connector specified by @connector. This will
> send
> + * a uevent with the properties HOTPLUG=1 and CONNECTOR.
> + */
> +void drm_sysfs_connector_hotplug_event(struct drm_connector *connector)
> +{
> + struct drm_device *dev = connector->dev;
> + char hotplug_str[] = "HOTPLUG=1", conn_id[21];
> + char *envp[] = { hotplug_str, conn_id, NULL };
> +
> + snprintf(conn_id, sizeof(conn_id),
> + "CONNECTOR=%u", connector->base.id);
> +
> + drm_dbg_kms(connector->dev,
> + "[CONNECTOR:%d:%s] generating connector hotplug event\n",
> + connector->base.id, connector->name);
> +
> + kobject_uevent_env(>primary->kdev->kobj, KOBJ_CHANGE, envp);
> +}
> +EXPORT_SYMBOL(drm_sysfs_connector_hotplug_event);
Would it make sense to call sysfs_notify on the status file?
It would allow to call poll() on the status file in sysfs and skipping
udev in simple cases?
Maxime
signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 4/7] drm/i915/display: Add handling for new "active bpc" property
Hi
On Tue, Jun 08, 2021 at 07:43:17PM +0200, Werner Sembach wrote:
> This commits implements the "active bpc" drm property for the Intel GPU
> driver.
>
> Signed-off-by: Werner Sembach
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 14 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 8 ++--
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++-
> drivers/gpu/drm/i915/display/intel_hdmi.c| 4 +++-
> 4 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 64e9107d70f7..50c11b8770a7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -10388,6 +10388,9 @@ static int intel_atomic_commit(struct drm_device *dev,
> {
> struct intel_atomic_state *state = to_intel_atomic_state(_state);
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_connector *connector;
> + struct drm_connector_state *new_conn_state;
> + int i;
> int ret = 0;
>
> state->wakeref = intel_runtime_pm_get(_priv->runtime_pm);
> @@ -10456,6 +10459,17 @@ static int intel_atomic_commit(struct drm_device
> *dev,
> intel_shared_dpll_swap_state(state);
> intel_atomic_track_fbs(state);
>
> + /* Extract information from crtc to communicate it to userspace as
> connector properties */
> + for_each_new_connector_in_state(>base, connector,
> new_conn_state, i) {
> + struct intel_crtc *crtc = to_intel_crtc(new_conn_state->crtc);
> + if (crtc) {
> + struct intel_crtc_state *new_crtc_state =
> intel_atomic_get_new_crtc_state(state, crtc);
> + new_conn_state->active_bpc = new_crtc_state->pipe_bpp /
> 3;
> + }
> + else
> + new_conn_state->active_bpc = 0;
> + }
> +
This seems fairly intrusive, but also commit / commit_tail might not be
the best place to put this, we want to support it at the connector
level.
Indeed, this will cause some issue if your HDMI output is a bridge for
example, where the commit will be in an entirely different driver that
has no dependency on the HDMI controller one.
I think it would be best to do that assignment in atomic_check. That
way, if the userspace does a commit with DRM_MODE_ATOMIC_TEST_ONLY it
would know what the output state would have been like.
Also, all of your patches don't follow the kernel coding style. Make
sure you fix the issues reported by checkpatch --strict
Maxime
signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/4] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property
Hi,
On Fri, Jun 04, 2021 at 07:17:21PM +0200, Werner Sembach wrote:
> Add a new general drm property "active bpc" which can be used by graphic
> drivers
> to report the applied bit depth per pixel back to userspace.
Just a heads up, we'll need an open source project that has accepted it
before merging it.
See
https://www.kernel.org/doc/html/latest/gpu/drm-uapi.html#open-source-userspace-requirements
> While "max bpc" can be used to change the color depth, there was no way to
> check
> which one actually got used. While in theory the driver chooses the
> best/highest
> color depth within the max bpc setting a user might not be fully aware what
> his
> hardware is or isn't capable off. This is meant as a quick way to double check
> the setup.
>
> In the future, automatic color calibration for screens might also depend on
> this
> information available.
>
> Signed-off-by: Werner Sembach
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 2 ++
> drivers/gpu/drm/drm_connector.c | 40 +++
> include/drm/drm_connector.h | 15
> 3 files changed, 57 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 268bb69c2e2f..7ae4e40936b5 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -873,6 +873,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = 0;
> } else if (property == connector->max_bpc_property) {
> *val = state->max_requested_bpc;
> + } else if (property == connector->active_bpc_property) {
> + *val = state->active_bpc;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 7631f76e7f34..5f42a5be5822 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1195,6 +1195,13 @@ static const struct drm_prop_enum_list
> dp_colorspaces[] = {
> * drm_connector_attach_max_bpc_property() to create and attach the
> * property to the connector during initialization.
> *
> + * active bpc:
> + * This read-only range property is used by userspace check the bit depth
> + * actually applied by the GPU driver after evaluation all hardware
^ display
Depending on the system, the display component might have a GPU attached
or not, and the GPU might have a display component or not.
> + * capabilities and max bpc. Drivers to use the function
> + * drm_connector_attach_active_bpc_property() to create and attach the
> + * property to the connector during initialization.
> + *
> * Connectors also have one standardized atomic property:
> *
> * CRTC_ID:
> @@ -2150,6 +2157,39 @@ int drm_connector_attach_max_bpc_property(struct
> drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
>
> +/**
> + * drm_connector_attach_active_bpc_property - attach "active bpc" property
> + * @connector: connector to attach active bpc property on.
> + * @min: The minimum bit depth supported by the connector.
> + * @max: The maximum bit depth supported by the connector.
> + *
> + * This is used to check the applied bit depth on a connector.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_active_bpc_property(struct drm_connector *connector,
> + int min, int max)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_property *prop;
> +
> + prop = connector->active_bpc_property;
> + if (!prop) {
> + prop = drm_property_create_range(dev, 0, "active bpc", min,
> max);
> + if (!prop)
> + return -ENOMEM;
> +
> + connector->active_bpc_property = prop;
> + }
> +
> + drm_object_attach_property(>base, prop, 0);
> + connector->state->active_bpc = 0;
I guess we want to default to 8?
Maxime
signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: New uAPI for color management proposal and feedback request
Hi, On Wed, May 12, 2021 at 02:06:56PM +0200, Werner Sembach wrote: > Hello, > > In addition to the existing "max bpc", and "Broadcast RGB/output_csc" > drm properties I propose 4 new properties: "preferred pixel encoding", > "active color depth", "active color range", and "active pixel > encoding" > > > Motivation: > > Current monitors have a variety pixel encodings available: RGB, YCbCr > 4:4:4, YCbCr 4:2:2, YCbCr 4:2:0. > > In addition they might be full or limited RGB range and the monitors > accept different bit depths. > > Currently the kernel driver for AMD and Intel GPUs automatically > configure the color settings automatically with little to no influence > of the user. However there are several real world scenarios where the > user might disagree with the default chosen by the drivers and wants > to set his or her own preference. > > Some examples: > > 1. While RGB and YCbCr 4:4:4 in theory carry the same amount of color > information, some screens might look better on one than the other > because of bad internal conversion. The driver currently however has a > fixed default that is chosen if available (RGB for Intel and YCbCr > 4:4:4 for AMD). The only way to change this currently is by editing > and overloading the edid reported by the monitor to the kernel. > > 2. RGB and YCbCr 4:4:4 need a higher port clock then YCbCr 4:2:0. Some > hardware might report that it supports the higher port clock, but > because of bad shielding on the PC, the cable, or the monitor the > screen cuts out every few seconds when RGB or YCbCr 4:4:4 encoding is > used, while YCbCr 4:2:0 might just work fine without changing > hardware. The drivers currently however always default to the "best > available" option even if it might be broken. > > 3. Some screens natively only supporting 8-bit color, simulate 10-Bit > color by rapidly switching between 2 adjacent colors. They advertise > themselves to the kernel as 10-bit monitors but the user might not > like the "fake" 10-bit effect and prefer running at the native 8-bit > per color. > > 4. Some screens are falsely classified as full RGB range wile they > actually use limited RGB range. This results in washed out colors in > dark and bright scenes. A user override can be helpful to manually fix > this issue when it occurs. > > There already exist several requests, discussion, and patches > regarding the thematic: > > - https://gitlab.freedesktop.org/drm/amd/-/issues/476 > > - https://gitlab.freedesktop.org/drm/amd/-/issues/1548 > > - https://lkml.org/lkml/2021/5/7/695 > > - https://lkml.org/lkml/2021/5/11/416 > > > Current State: > > I only know bits about the Intel i915 and AMD amdgpu driver. I don't > know how other driver handle color management > > - "max bpc", global setting applied by both i915 (only on dp i think?) > and amdgpu. Default value is "8". For every resolution + frequency > combination the highest possible even number between 6 and max_bpc is > chosen. If the range doesn't contain a valid mode the resolution + > frequency combination is discarded (but I guess that would be a very > special edge case, if existent at all, when 6 doesn't work but 10 > would work). Intel HDMI code always checks 8, 12, and 10 and does not > check the max_bpc setting. > > - "Broadcast RGB" for i915 and "output_csc" for the old radeon driver > (not amdgpu), overwrites the kernel chosen color range setting (full > or limited). If I recall correctly Intel HDMI code defaults to full > unless this property is set, Intel dp code tries to probe the monitor > to find out what to use. amdgpu has no corresponding setting (I don't > know how it's decided there). > > - RGB pixel encoding can be forced by overloading a Monitors edid with > one that tells the kernel that only RGB is possible. That doesn't work > for YCbCr 4:4:4 however because of the edid specification. Forcing > YCbCr 4:2:0 would theoretically also be possible this way. amdgpu has > a debugfs switch "force_ycbcr_420" which makes the driver default to > YCbCr 4:2:0 on all monitors if possible. > > > Proposed Solution: > > 1. Add a new uAPI property "preferred pixel encoding", as a per port >setting. > > - An amdgpu specific implementation was already shared here: > https://gitlab.freedesktop.org/drm/amd/-/issues/476 > > - It also writes back the actually used encoding if the one > requested was not possible, overwriting the requested value in > the process. I think it would be better to have this feedback > channel as a different, read-only property. > > - Make this solution vendor agnostic by putting it in the > drm_connector_state struct next do max_bpc > > https://elixir.bootlin.com/linux/v5.13-rc1/source/include/drm/drm_connector.h#L654 > and add patches to amdgpu and i915 to respect this setting > > 2. Convert "Broadcast RGB" to a vendor agnostic setting/replace with a >vendor agnostic setting. > > - Imho the name is not
Re: [PATCH v3 1/5] drm/connector: Create a helper to attach the hdr_output_metadata property
Hi, On Fri, Apr 30, 2021 at 11:44:47AM +0200, Maxime Ripard wrote: > All the drivers that implement HDR output call pretty much the same > function to initialise the hdr_output_metadata property, and while the > creation of that property is in a helper, every driver uses the same > code to attach it. > > Provide a helper for it as well > > Reviewed-by: Harry Wentland > Reviewed-by: Jernej Skrabec > Signed-off-by: Maxime Ripard I pushed all 5 patches on friday Maxime signature.asc Description: PGP signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 2/5] drm/connector: Add helper to compare HDR metadata
All the drivers that support the HDR metadata property have a similar
function to compare the metadata from one connector state to the next,
and force a mode change if they differ.
All these functions run pretty much the same code, so let's turn it into
an helper that can be shared across those drivers.
Reviewed-by: Harry Wentland
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
Changes from v1:
- Rebased on latest drm-misc-next tag
- Added the tags
- Fix build breakage on amdgpu
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +--
drivers/gpu/drm/drm_connector.c | 28 +++
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +
include/drm/drm_connector.h | 2 ++
5 files changed, 34 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c8d7e7dbc05e..296704ce3768 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6275,25 +6275,6 @@ static int fill_hdr_info_packet(const struct
drm_connector_state *state,
return 0;
}
-static bool
-is_hdr_metadata_different(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (old_blob != new_blob) {
- if (old_blob && new_blob &&
- old_blob->length == new_blob->length)
- return memcmp(old_blob->data, new_blob->data,
- old_blob->length);
-
- return true;
- }
-
- return false;
-}
-
static int
amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
struct drm_atomic_state *state)
@@ -6311,7 +6292,7 @@ amdgpu_dm_connector_atomic_check(struct drm_connector
*conn,
if (!crtc)
return 0;
- if (is_hdr_metadata_different(old_con_state, new_con_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_con_state,
new_con_state)) {
struct dc_info_packet hdr_infopacket;
ret = fill_hdr_info_packet(new_con_state, _infopacket);
@@ -8803,7 +8784,7 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
dm_old_crtc_state->abm_level;
hdr_changed =
- is_hdr_metadata_different(old_con_state, new_con_state);
+ !drm_connector_atomic_hdr_metadata_equal(old_con_state,
new_con_state);
if (!scaling_changed && !abm_changed && !hdr_changed)
continue;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index dd7f6eda2ce2..e7c7c9b9c646 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2395,21 +2395,6 @@ static int dw_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
-static bool hdr_metadata_equal(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (!old_blob || !new_blob)
- return old_blob == new_blob;
-
- if (old_blob->length != new_blob->length)
- return false;
-
- return !memcmp(old_blob->data, new_blob->data, old_blob->length);
-}
-
static int dw_hdmi_connector_atomic_check(struct drm_connector *connector,
struct drm_atomic_state *state)
{
@@ -2423,7 +2408,7 @@ static int dw_hdmi_connector_atomic_check(struct
drm_connector *connector,
if (!crtc)
return 0;
- if (!hdr_metadata_equal(old_state, new_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
crtc_state = drm_atomic_get_crtc_state(state, crtc);
if (IS_ERR(crtc_state))
return PTR_ERR(crtc_state);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index c5e2f642acd9..eed9cd05c94e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2172,6 +2172,34 @@ int
drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connector_atomi
[PATCH v3 1/5] drm/connector: Create a helper to attach the hdr_output_metadata property
All the drivers that implement HDR output call pretty much the same
function to initialise the hdr_output_metadata property, and while the
creation of that property is in a helper, every driver uses the same
code to attach it.
Provide a helper for it as well
Reviewed-by: Harry Wentland
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
- Fixed a merge conflict with i915
Changes from v1:
- Rebased on latest drm-misc-next tag
- Added the tags
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +--
drivers/gpu/drm/drm_connector.c | 21 +++
drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +--
include/drm/drm_connector.h | 1 +
5 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index a0c8c41e4e57..c8d7e7dbc05e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7498,9 +7498,7 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
- drm_object_attach_property(
- >base.base,
- dm->ddev->mode_config.hdr_output_metadata_property, 0);
+
drm_connector_attach_hdr_output_metadata_property(>base);
if (!aconnector->mst_port)
drm_connector_attach_vrr_capable_property(>base);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ae97513ef886..dd7f6eda2ce2 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2492,8 +2492,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
drm_connector_attach_max_bpc_property(connector, 8, 16);
if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe)
- drm_object_attach_property(>base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
drm_connector_attach_encoder(connector, hdmi->bridge.encoder);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index eab8c0b82de2..c5e2f642acd9 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2151,6 +2151,27 @@ int drm_connector_attach_max_bpc_property(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
+/**
+ * drm_connector_attach_hdr_output_metadata_property - attach
"HDR_OUTPUT_METADA" property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to send HDR Metadata to the
+ * driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_property *prop =
dev->mode_config.hdr_output_metadata_property;
+
+ drm_object_attach_property(>base, prop, 0);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+
/**
* drm_connector_set_vrr_capable_property - sets the variable refresh rate
* capable property for a connector
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9c172dd6fb5b..3c767bcc47b1 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2459,8 +2459,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi,
struct drm_connector *c
drm_connector_attach_content_type_property(connector);
if (DISPLAY_VER(dev_priv) >= 10)
- drm_object_attach_property(>base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
if (!HAS_GMCH(dev_priv))
drm_connector_attach_max_bpc_property(connector, 8, 12);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 1922b278ffad..32172dab8427 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct
drm_connector *connector,
u32 scaling_mode_mask);
int drm_connector_attach_vrr_capable_property(
struct drm_connector *connector);
+int drm_conn
[PATCH v3 3/5] drm/vc4: Add HDR metadata property to the VC5 HDMI connectors
From: Dave Stevenson
Now that we can export deeper colour depths, add in the signalling
for HDR metadata.
Signed-off-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
Changes from v1:
- Rebased on latest drm-misc-next tag
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 53 ++
drivers/gpu/drm/vc4/vc4_hdmi.h | 3 ++
2 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 1fda574579af..a33fa1662588 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -214,6 +214,31 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
+static int vc4_hdmi_connector_atomic_check(struct drm_connector *connector,
+ struct drm_atomic_state *state)
+{
+ struct drm_connector_state *old_state =
+ drm_atomic_get_old_connector_state(state, connector);
+ struct drm_connector_state *new_state =
+ drm_atomic_get_new_connector_state(state, connector);
+ struct drm_crtc *crtc = new_state->crtc;
+
+ if (!crtc)
+ return 0;
+
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
+ struct drm_crtc_state *crtc_state;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ crtc_state->mode_changed = true;
+ }
+
+ return 0;
+}
+
static void vc4_hdmi_connector_reset(struct drm_connector *connector)
{
struct vc4_hdmi_connector_state *old_state =
@@ -263,6 +288,7 @@ static const struct drm_connector_funcs
vc4_hdmi_connector_funcs = {
static const struct drm_connector_helper_funcs vc4_hdmi_connector_helper_funcs
= {
.get_modes = vc4_hdmi_connector_get_modes,
+ .atomic_check = vc4_hdmi_connector_atomic_check,
};
static int vc4_hdmi_connector_init(struct drm_device *dev,
@@ -299,6 +325,9 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
connector->interlace_allowed = 1;
connector->doublescan_allowed = 0;
+ if (vc4_hdmi->variant->supports_hdr)
+ drm_connector_attach_hdr_output_metadata_property(connector);
+
drm_connector_attach_encoder(connector, encoder);
return 0;
@@ -432,6 +461,25 @@ static void vc4_hdmi_set_audio_infoframe(struct
drm_encoder *encoder)
vc4_hdmi_write_infoframe(encoder, );
}
+static void vc4_hdmi_set_hdr_infoframe(struct drm_encoder *encoder)
+{
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_connector *connector = _hdmi->connector;
+ struct drm_connector_state *conn_state = connector->state;
+ union hdmi_infoframe frame;
+
+ if (!vc4_hdmi->variant->supports_hdr)
+ return;
+
+ if (!conn_state->hdr_output_metadata)
+ return;
+
+ if (drm_hdmi_infoframe_set_hdr_metadata(, conn_state))
+ return;
+
+ vc4_hdmi_write_infoframe(encoder, );
+}
+
static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -444,6 +492,8 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
*/
if (vc4_hdmi->audio.streaming)
vc4_hdmi_set_audio_infoframe(encoder);
+
+ vc4_hdmi_set_hdr_infoframe(encoder);
}
static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
@@ -2102,6 +2152,7 @@ static const struct vc4_hdmi_variant bcm2835_variant = {
.phy_rng_enable = vc4_hdmi_phy_rng_enable,
.phy_rng_disable= vc4_hdmi_phy_rng_disable,
.channel_map= vc4_hdmi_channel_map,
+ .supports_hdr = false,
};
static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = {
@@ -2129,6 +2180,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi0_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = {
@@ -2156,6 +2208,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi1_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct of_device_id vc4_hdmi_dt_match[] = {
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 3cebd1fd00fc..060bcaefbeb5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -9
[PATCH v3 5/5] drm/vc4: hdmi: Signal the proper colorimetry info in the infoframe
Our driver while supporting HDR didn't send the proper colorimetry info
in the AVI infoframe.
Let's add the property needed so that the userspace can let us know what
the colorspace is supposed to be.
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
Changes from v1:
- New patch
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index a33fa1662588..a22e17788074 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -226,7 +226,8 @@ static int vc4_hdmi_connector_atomic_check(struct
drm_connector *connector,
if (!crtc)
return 0;
- if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
+ if (old_state->colorspace != new_state->colorspace ||
+ !drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
struct drm_crtc_state *crtc_state;
crtc_state = drm_atomic_get_crtc_state(state, crtc);
@@ -316,6 +317,11 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
if (ret)
return ret;
+ ret = drm_mode_create_hdmi_colorspace_property(connector);
+ if (ret)
+ return ret;
+
+ drm_connector_attach_colorspace_property(connector);
drm_connector_attach_tv_margin_properties(connector);
drm_connector_attach_max_bpc_property(connector, 8, 12);
@@ -424,7 +430,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder
*encoder)
vc4_encoder->limited_rgb_range ?
HDMI_QUANTIZATION_RANGE_LIMITED :
HDMI_QUANTIZATION_RANGE_FULL);
-
+ drm_hdmi_avi_infoframe_colorspace(, cstate);
drm_hdmi_avi_infoframe_bars(, cstate);
vc4_hdmi_write_infoframe(encoder, );
--
2.31.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 4/5] drm/connector: Add a helper to attach the colorspace property
The intel driver uses the same logic to attach the Colorspace property
in multiple places and we'll need it in vc4 too. Let's move that common
code in a helper.
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
Changes from v1:
- New patch
---
drivers/gpu/drm/drm_connector.c | 20 +++
.../gpu/drm/i915/display/intel_connector.c| 6 ++
include/drm/drm_connector.h | 1 +
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index eed9cd05c94e..733da42cfd31 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2172,6 +2172,26 @@ int
drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connector_attach_colorspace_property - attach "Colorspace" property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to signal the output colorspace
+ * to the driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_colorspace_property(struct drm_connector *connector)
+{
+ struct drm_property *prop = connector->colorspace_property;
+
+ drm_object_attach_property(>base, prop,
DRM_MODE_COLORIMETRY_DEFAULT);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_colorspace_property);
+
/**
* drm_connector_atomic_hdr_metadata_equal - checks if the hdr metadata changed
* @old_state: old connector state to compare
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index d5ceb7bdc14b..9bed1ccecea0 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -282,14 +282,12 @@ void
intel_attach_hdmi_colorspace_property(struct drm_connector *connector)
{
if (!drm_mode_create_hdmi_colorspace_property(connector))
- drm_object_attach_property(>base,
- connector->colorspace_property, 0);
+ drm_connector_attach_colorspace_property(connector);
}
void
intel_attach_dp_colorspace_property(struct drm_connector *connector)
{
if (!drm_mode_create_dp_colorspace_property(connector))
- drm_object_attach_property(>base,
- connector->colorspace_property, 0);
+ drm_connector_attach_colorspace_property(connector);
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 1f51d73ca715..714d1a01c065 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct
drm_connector *connector,
u32 scaling_mode_mask);
int drm_connector_attach_vrr_capable_property(
struct drm_connector *connector);
+int drm_connector_attach_colorspace_property(struct drm_connector *connector);
int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector);
bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state
*old_state,
struct drm_connector_state
*new_state);
--
2.31.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 1/5] drm/connector: Create a helper to attach the hdr_output_metadata property
All the drivers that implement HDR output call pretty much the same
function to initialise the hdr_output_metadata property, and while the
creation of that property is in a helper, every driver uses the same
code to attach it.
Provide a helper for it as well
Reviewed-by: Harry Wentland
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Rebased on latest drm-misc-next tag
- Added the tags
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +--
drivers/gpu/drm/drm_connector.c | 21 +++
drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +--
include/drm/drm_connector.h | 1 +
5 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 55e39b462a5e..1e22ce718010 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7078,9 +7078,7 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
- drm_object_attach_property(
- >base.base,
- dm->ddev->mode_config.hdr_output_metadata_property, 0);
+
drm_connector_attach_hdr_output_metadata_property(>base);
if (!aconnector->mst_port)
drm_connector_attach_vrr_capable_property(>base);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index dda4fa9a1a08..f24bbb840dbf 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2492,8 +2492,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
drm_connector_attach_max_bpc_property(connector, 8, 16);
if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe)
- drm_object_attach_property(>base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
drm_connector_attach_encoder(connector, hdmi->bridge.encoder);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 7631f76e7f34..a4aa2d87af35 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2150,6 +2150,27 @@ int drm_connector_attach_max_bpc_property(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
+/**
+ * drm_connector_attach_hdr_output_metadata_property - attach
"HDR_OUTPUT_METADA" property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to send HDR Metadata to the
+ * driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_property *prop =
dev->mode_config.hdr_output_metadata_property;
+
+ drm_object_attach_property(>base, prop, 0);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+
/**
* drm_connector_set_vrr_capable_property - sets the variable refresh rate
* capable property for a connector
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 95919d325b0b..f2f1b025e6ba 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2965,8 +2965,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi,
struct drm_connector *c
drm_connector_attach_content_type_property(connector);
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
- drm_object_attach_property(>base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
if (!HAS_GMCH(dev_priv))
drm_connector_attach_max_bpc_property(connector, 8, 12);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 1922b278ffad..32172dab8427 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct
drm_connector *connector,
u32 scaling_mode_mask);
int drm_connector_attach_vrr_capable_property(
struct drm_connector *connector);
+int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*con
[PATCH v2 5/5] drm/vc4: hdmi: Signal the proper colorimetry info in the infoframe
Our driver while supporting HDR didn't send the proper colorimetry info
in the AVI infoframe.
Let's add the property needed so that the userspace can let us know what
the colorspace is supposed to be.
Signed-off-by: Maxime Ripard
---
Changes from v1:
- New patch
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index a33fa1662588..a22e17788074 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -226,7 +226,8 @@ static int vc4_hdmi_connector_atomic_check(struct
drm_connector *connector,
if (!crtc)
return 0;
- if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
+ if (old_state->colorspace != new_state->colorspace ||
+ !drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
struct drm_crtc_state *crtc_state;
crtc_state = drm_atomic_get_crtc_state(state, crtc);
@@ -316,6 +317,11 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
if (ret)
return ret;
+ ret = drm_mode_create_hdmi_colorspace_property(connector);
+ if (ret)
+ return ret;
+
+ drm_connector_attach_colorspace_property(connector);
drm_connector_attach_tv_margin_properties(connector);
drm_connector_attach_max_bpc_property(connector, 8, 12);
@@ -424,7 +430,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder
*encoder)
vc4_encoder->limited_rgb_range ?
HDMI_QUANTIZATION_RANGE_LIMITED :
HDMI_QUANTIZATION_RANGE_FULL);
-
+ drm_hdmi_avi_infoframe_colorspace(, cstate);
drm_hdmi_avi_infoframe_bars(, cstate);
vc4_hdmi_write_infoframe(encoder, );
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 4/5] drm/connector: Add a helper to attach the colorspace property
The intel driver uses the same logic to attach the Colorspace property
in multiple places and we'll need it in vc4 too. Let's move that common
code in a helper.
Signed-off-by: Maxime Ripard
---
Changes from v1:
- New patch
---
drivers/gpu/drm/drm_connector.c | 20 +++
.../gpu/drm/i915/display/intel_connector.c| 6 ++
include/drm/drm_connector.h | 1 +
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b13021b1b128..6a20b249e533 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2171,6 +2171,26 @@ int
drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connector_attach_colorspace_property - attach "Colorspace" property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to signal the output colorspace
+ * to the driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_colorspace_property(struct drm_connector *connector)
+{
+ struct drm_property *prop = connector->colorspace_property;
+
+ drm_object_attach_property(>base, prop,
DRM_MODE_COLORIMETRY_DEFAULT);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_colorspace_property);
+
/**
* drm_connector_atomic_hdr_metadata_equal - checks if the hdr metadata changed
* @old_state: old connector state to compare
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index d5ceb7bdc14b..9bed1ccecea0 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -282,14 +282,12 @@ void
intel_attach_hdmi_colorspace_property(struct drm_connector *connector)
{
if (!drm_mode_create_hdmi_colorspace_property(connector))
- drm_object_attach_property(>base,
- connector->colorspace_property, 0);
+ drm_connector_attach_colorspace_property(connector);
}
void
intel_attach_dp_colorspace_property(struct drm_connector *connector)
{
if (!drm_mode_create_dp_colorspace_property(connector))
- drm_object_attach_property(>base,
- connector->colorspace_property, 0);
+ drm_connector_attach_colorspace_property(connector);
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 1f51d73ca715..714d1a01c065 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct
drm_connector *connector,
u32 scaling_mode_mask);
int drm_connector_attach_vrr_capable_property(
struct drm_connector *connector);
+int drm_connector_attach_colorspace_property(struct drm_connector *connector);
int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector);
bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state
*old_state,
struct drm_connector_state
*new_state);
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 3/5] drm/vc4: Add HDR metadata property to the VC5 HDMI connectors
From: Dave Stevenson
Now that we can export deeper colour depths, add in the signalling
for HDR metadata.
Signed-off-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Rebased on latest drm-misc-next tag
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 53 ++
drivers/gpu/drm/vc4/vc4_hdmi.h | 3 ++
2 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 1fda574579af..a33fa1662588 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -214,6 +214,31 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
+static int vc4_hdmi_connector_atomic_check(struct drm_connector *connector,
+ struct drm_atomic_state *state)
+{
+ struct drm_connector_state *old_state =
+ drm_atomic_get_old_connector_state(state, connector);
+ struct drm_connector_state *new_state =
+ drm_atomic_get_new_connector_state(state, connector);
+ struct drm_crtc *crtc = new_state->crtc;
+
+ if (!crtc)
+ return 0;
+
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
+ struct drm_crtc_state *crtc_state;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ crtc_state->mode_changed = true;
+ }
+
+ return 0;
+}
+
static void vc4_hdmi_connector_reset(struct drm_connector *connector)
{
struct vc4_hdmi_connector_state *old_state =
@@ -263,6 +288,7 @@ static const struct drm_connector_funcs
vc4_hdmi_connector_funcs = {
static const struct drm_connector_helper_funcs vc4_hdmi_connector_helper_funcs
= {
.get_modes = vc4_hdmi_connector_get_modes,
+ .atomic_check = vc4_hdmi_connector_atomic_check,
};
static int vc4_hdmi_connector_init(struct drm_device *dev,
@@ -299,6 +325,9 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
connector->interlace_allowed = 1;
connector->doublescan_allowed = 0;
+ if (vc4_hdmi->variant->supports_hdr)
+ drm_connector_attach_hdr_output_metadata_property(connector);
+
drm_connector_attach_encoder(connector, encoder);
return 0;
@@ -432,6 +461,25 @@ static void vc4_hdmi_set_audio_infoframe(struct
drm_encoder *encoder)
vc4_hdmi_write_infoframe(encoder, );
}
+static void vc4_hdmi_set_hdr_infoframe(struct drm_encoder *encoder)
+{
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_connector *connector = _hdmi->connector;
+ struct drm_connector_state *conn_state = connector->state;
+ union hdmi_infoframe frame;
+
+ if (!vc4_hdmi->variant->supports_hdr)
+ return;
+
+ if (!conn_state->hdr_output_metadata)
+ return;
+
+ if (drm_hdmi_infoframe_set_hdr_metadata(, conn_state))
+ return;
+
+ vc4_hdmi_write_infoframe(encoder, );
+}
+
static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -444,6 +492,8 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
*/
if (vc4_hdmi->audio.streaming)
vc4_hdmi_set_audio_infoframe(encoder);
+
+ vc4_hdmi_set_hdr_infoframe(encoder);
}
static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
@@ -2102,6 +2152,7 @@ static const struct vc4_hdmi_variant bcm2835_variant = {
.phy_rng_enable = vc4_hdmi_phy_rng_enable,
.phy_rng_disable= vc4_hdmi_phy_rng_disable,
.channel_map= vc4_hdmi_channel_map,
+ .supports_hdr = false,
};
static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = {
@@ -2129,6 +2180,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi0_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = {
@@ -2156,6 +2208,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi1_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct of_device_id vc4_hdmi_dt_match[] = {
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 3cebd1fd00fc..060bcaefbeb5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -99,6 +99,9 @@ struct vc4_hdmi_variant {
/* Callback to get chan
[PATCH v2 2/5] drm/connector: Add helper to compare HDR metadata
All the drivers that support the HDR metadata property have a similar
function to compare the metadata from one connector state to the next,
and force a mode change if they differ.
All these functions run pretty much the same code, so let's turn it into
an helper that can be shared across those drivers.
Reviewed-by: Harry Wentland
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Rebased on latest drm-misc-next tag
- Added the tags
- Fix build breakage on amdgpu
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +--
drivers/gpu/drm/drm_connector.c | 28 +++
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +
include/drm/drm_connector.h | 2 ++
5 files changed, 34 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1e22ce718010..ed1972fb531d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5967,25 +5967,6 @@ static int fill_hdr_info_packet(const struct
drm_connector_state *state,
return 0;
}
-static bool
-is_hdr_metadata_different(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (old_blob != new_blob) {
- if (old_blob && new_blob &&
- old_blob->length == new_blob->length)
- return memcmp(old_blob->data, new_blob->data,
- old_blob->length);
-
- return true;
- }
-
- return false;
-}
-
static int
amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
struct drm_atomic_state *state)
@@ -6003,7 +5984,7 @@ amdgpu_dm_connector_atomic_check(struct drm_connector
*conn,
if (!crtc)
return 0;
- if (is_hdr_metadata_different(old_con_state, new_con_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_con_state,
new_con_state)) {
struct dc_info_packet hdr_infopacket;
ret = fill_hdr_info_packet(new_con_state, _infopacket);
@@ -8357,7 +8338,7 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
dm_old_crtc_state->abm_level;
hdr_changed =
- is_hdr_metadata_different(old_con_state, new_con_state);
+ !drm_connector_atomic_hdr_metadata_equal(old_con_state,
new_con_state);
if (!scaling_changed && !abm_changed && !hdr_changed)
continue;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f24bbb840dbf..f871e33c2fc9 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2395,21 +2395,6 @@ static int dw_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
-static bool hdr_metadata_equal(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (!old_blob || !new_blob)
- return old_blob == new_blob;
-
- if (old_blob->length != new_blob->length)
- return false;
-
- return !memcmp(old_blob->data, new_blob->data, old_blob->length);
-}
-
static int dw_hdmi_connector_atomic_check(struct drm_connector *connector,
struct drm_atomic_state *state)
{
@@ -2423,7 +2408,7 @@ static int dw_hdmi_connector_atomic_check(struct
drm_connector *connector,
if (!crtc)
return 0;
- if (!hdr_metadata_equal(old_state, new_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
crtc_state = drm_atomic_get_crtc_state(state, crtc);
if (IS_ERR(crtc_state))
return PTR_ERR(crtc_state);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index a4aa2d87af35..b13021b1b128 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2171,6 +2171,34 @@ int
drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connector_atomic_hdr_metadata_equal - checks if the hdr metadata changed
[PATCH 1/3] drm/connector: Create a helper to attach the hdr_output_metadata property
All the drivers that implement HDR output call pretty much the same
function to initialise the hdr_output_metadata property, and while the
creation of that property is in a helper, every driver uses the same
code to attach it.
Provide a helper for it as well
Signed-off-by: Maxime Ripard
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +--
drivers/gpu/drm/drm_connector.c | 21 +++
drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +--
include/drm/drm_connector.h | 1 +
5 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 22124f76d0b5..06908a3cee0f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7017,9 +7017,7 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
- drm_object_attach_property(
- >base.base,
- dm->ddev->mode_config.hdr_output_metadata_property, 0);
+
drm_connector_attach_hdr_output_metadata_property(>base);
if (!aconnector->mst_port)
drm_connector_attach_vrr_capable_property(>base);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index dda4fa9a1a08..f24bbb840dbf 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2492,8 +2492,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
drm_connector_attach_max_bpc_property(connector, 8, 16);
if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe)
- drm_object_attach_property(>base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
drm_connector_attach_encoder(connector, hdmi->bridge.encoder);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 98b6ec45ef96..e25248e23e18 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2149,6 +2149,27 @@ int drm_connector_attach_max_bpc_property(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
+/**
+ * drm_connector_attach_hdr_output_metadata_property - attach
"HDR_OUTPUT_METADA" property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to send HDR Metadata to the
+ * driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_property *prop =
dev->mode_config.hdr_output_metadata_property;
+
+ drm_object_attach_property(>base, prop, 0);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+
/**
* drm_connector_set_vrr_capable_property - sets the variable refresh rate
* capable property for a connector
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index c5959590562b..52c051efb7b7 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2958,8 +2958,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi,
struct drm_connector *c
drm_connector_attach_content_type_property(connector);
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
- drm_object_attach_property(>base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
if (!HAS_GMCH(dev_priv))
drm_connector_attach_max_bpc_property(connector, 8, 12);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 1922b278ffad..32172dab8427 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct
drm_connector *connector,
u32 scaling_mode_mask);
int drm_connector_attach_vrr_capable_property(
struct drm_connector *connector);
+int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector);
int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
int drm_mode_create_hdmi_colorspace_property(struct drm_connector *connec
[PATCH 2/3] drm/connector: Add helper to compare HDR metadata
All the drivers that support the HDR metadata property have a similar
function to compare the metadata from one connector state to the next,
and force a mode change if they differ.
All these functions run pretty much the same code, so let's turn it into
an helper that can be shared across those drivers.
Signed-off-by: Maxime Ripard
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 21 +-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +--
drivers/gpu/drm/drm_connector.c | 28 +++
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +
include/drm/drm_connector.h | 2 ++
5 files changed, 33 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 06908a3cee0f..4eb5201e566a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5924,25 +5924,6 @@ static int fill_hdr_info_packet(const struct
drm_connector_state *state,
return 0;
}
-static bool
-is_hdr_metadata_different(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (old_blob != new_blob) {
- if (old_blob && new_blob &&
- old_blob->length == new_blob->length)
- return memcmp(old_blob->data, new_blob->data,
- old_blob->length);
-
- return true;
- }
-
- return false;
-}
-
static int
amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
struct drm_atomic_state *state)
@@ -5960,7 +5941,7 @@ amdgpu_dm_connector_atomic_check(struct drm_connector
*conn,
if (!crtc)
return 0;
- if (is_hdr_metadata_different(old_con_state, new_con_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_con_state,
new_con_state)) {
struct dc_info_packet hdr_infopacket;
ret = fill_hdr_info_packet(new_con_state, _infopacket);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f24bbb840dbf..f871e33c2fc9 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2395,21 +2395,6 @@ static int dw_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
-static bool hdr_metadata_equal(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (!old_blob || !new_blob)
- return old_blob == new_blob;
-
- if (old_blob->length != new_blob->length)
- return false;
-
- return !memcmp(old_blob->data, new_blob->data, old_blob->length);
-}
-
static int dw_hdmi_connector_atomic_check(struct drm_connector *connector,
struct drm_atomic_state *state)
{
@@ -2423,7 +2408,7 @@ static int dw_hdmi_connector_atomic_check(struct
drm_connector *connector,
if (!crtc)
return 0;
- if (!hdr_metadata_equal(old_state, new_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
crtc_state = drm_atomic_get_crtc_state(state, crtc);
if (IS_ERR(crtc_state))
return PTR_ERR(crtc_state);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index e25248e23e18..d781a3a1e9bf 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2170,6 +2170,34 @@ int
drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connector_atomic_hdr_metadata_equal - checks if the hdr metadata changed
+ * @old_state: old connector state to compare
+ * @new_state: new connector state to compare
+ *
+ * This is used by HDR-enabled drivers to test whether the HDR metadata
+ * have changed between two different connector state (and thus probably
+ * requires a full blown mode change).
+ *
+ * Returns:
+ * True if the metadata are equal, False otherwise
+ */
+bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state
*old_state,
+struct drm_connector_state
*new_state)
+{
+ struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
+ struct drm_property_blob *new_blob = new_sta
[PATCH 3/3] drm/vc4: Add HDR metadata property to the VC5 HDMI connectors
From: Dave Stevenson
Now that we can export deeper colour depths, add in the signalling
for HDR metadata.
Signed-off-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 53 ++
drivers/gpu/drm/vc4/vc4_hdmi.h | 3 ++
2 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index eee9751009c2..c8846cf9dd24 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -214,6 +214,31 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
+static int vc4_hdmi_connector_atomic_check(struct drm_connector *connector,
+ struct drm_atomic_state *state)
+{
+ struct drm_connector_state *old_state =
+ drm_atomic_get_old_connector_state(state, connector);
+ struct drm_connector_state *new_state =
+ drm_atomic_get_new_connector_state(state, connector);
+ struct drm_crtc *crtc = new_state->crtc;
+
+ if (!crtc)
+ return 0;
+
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
+ struct drm_crtc_state *crtc_state;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ crtc_state->mode_changed = true;
+ }
+
+ return 0;
+}
+
static void vc4_hdmi_connector_reset(struct drm_connector *connector)
{
struct vc4_hdmi_connector_state *old_state =
@@ -263,6 +288,7 @@ static const struct drm_connector_funcs
vc4_hdmi_connector_funcs = {
static const struct drm_connector_helper_funcs vc4_hdmi_connector_helper_funcs
= {
.get_modes = vc4_hdmi_connector_get_modes,
+ .atomic_check = vc4_hdmi_connector_atomic_check,
};
static int vc4_hdmi_connector_init(struct drm_device *dev,
@@ -299,6 +325,9 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
connector->interlace_allowed = 1;
connector->doublescan_allowed = 0;
+ if (vc4_hdmi->variant->supports_hdr)
+ drm_connector_attach_hdr_output_metadata_property(connector);
+
drm_connector_attach_encoder(connector, encoder);
return 0;
@@ -432,6 +461,25 @@ static void vc4_hdmi_set_audio_infoframe(struct
drm_encoder *encoder)
vc4_hdmi_write_infoframe(encoder, );
}
+static void vc4_hdmi_set_hdr_infoframe(struct drm_encoder *encoder)
+{
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_connector *connector = _hdmi->connector;
+ struct drm_connector_state *conn_state = connector->state;
+ union hdmi_infoframe frame;
+
+ if (!vc4_hdmi->variant->supports_hdr)
+ return;
+
+ if (!conn_state->hdr_output_metadata)
+ return;
+
+ if (drm_hdmi_infoframe_set_hdr_metadata(, conn_state))
+ return;
+
+ vc4_hdmi_write_infoframe(encoder, );
+}
+
static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -444,6 +492,8 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
*/
if (vc4_hdmi->audio.streaming)
vc4_hdmi_set_audio_infoframe(encoder);
+
+ vc4_hdmi_set_hdr_infoframe(encoder);
}
static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
@@ -2101,6 +2151,7 @@ static const struct vc4_hdmi_variant bcm2835_variant = {
.phy_rng_enable = vc4_hdmi_phy_rng_enable,
.phy_rng_disable= vc4_hdmi_phy_rng_disable,
.channel_map= vc4_hdmi_channel_map,
+ .supports_hdr = false,
};
static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = {
@@ -2128,6 +2179,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi0_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = {
@@ -2155,6 +2207,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi1_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct of_device_id vc4_hdmi_dt_match[] = {
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 3cebd1fd00fc..060bcaefbeb5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -99,6 +99,9 @@ struct vc4_hdmi_variant {
/* Callback to get channel map */
u32 (*channel_map)(struct
Re: [PATCH v3 01/11] drm/atomic: Pass the full state to planes async atomic check and update
Hi, On Wed, Feb 24, 2021 at 12:33:45PM +0100, Thomas Zimmermann wrote: > Hi Maxime, > > for the whole series: > > Acked-by: Thomas Zimmermann Applied the whole series, thanks to everyone involved in the review, it's been a pretty daunting one :) Maxime signature.asc Description: PGP signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 02/11] drm: Rename plane atomic_check state names
Hi Thomas,
Thanks for your review!
On Fri, Feb 19, 2021 at 03:49:22PM +0100, Thomas Zimmermann wrote:
> > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
> > b/drivers/gpu/drm/imx/ipuv3-plane.c
> > index 075508051b5f..1873a155bb26 100644
> > --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> > @@ -337,12 +337,12 @@ static const struct drm_plane_funcs ipu_plane_funcs =
> > {
> > };
> > static int ipu_plane_atomic_check(struct drm_plane *plane,
> > - struct drm_plane_state *state)
> > + struct drm_plane_state *new_state)
>
> This function uses a different naming convention then the others?
>
> > {
> > struct drm_plane_state *old_state = plane->state;
So, the function already had a variable named old_state, so I was
actually trying to make the drivers consistent here: having one variable
with old_state and new_plane_state felt weird.
The heuristic is thus to use the convention of the driver if one exists
already, and if there's none pick new_plane_state.
It makes it indeed inconsistent across drivers, but it felt more natural
to be consistent within a single driver.
Maxime
signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 01/11] drm/atomic: Pass the full state to planes async atomic check and update
The current atomic helpers have either their object state being passed as
an argument or the full atomic state.
The former is the pattern that was done at first, before switching to the
latter for new hooks or when it was needed.
Let's start convert all the remaining helpers to provide a consistent
interface, starting with the planes atomic_async_check and
atomic_async_update.
The conversion was done using the coccinelle script below, built tested on
all the drivers.
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
int (*atomic_async_check)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
void (*atomic_async_update)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
(
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_async_check = func,
...,
};
|
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_async_update = func,
...,
};
)
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_async_check(plane, plane_state)
+ FUNCS->atomic_async_check(plane, state)
...+>
}
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_async_update(plane, plane_state)
+ FUNCS->atomic_async_update(plane, state)
...+>
}
@@
identifier mtk_plane_atomic_async_update;
identifier plane;
symbol new_state, state;
expression e;
@@
void mtk_plane_atomic_async_update(struct drm_plane *plane, struct
drm_plane_state *new_state)
{
...
- struct mtk_plane_state *state = e;
+ struct mtk_plane_state *new_plane_state = e;
<+...
- state
+ new_plane_state
...+>
}
@@
identifier plane_atomic_func.func;
identifier plane;
symbol state;
@@
func(struct drm_plane *plane,
-struct drm_plane_state *state)
+struct drm_plane_state *new_plane_state)
{
<...
- state
+ new_plane_state
...>
}
@ ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
... when != new_plane_state
}
@ adds_new_state depends on plane_atomic_func && !ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
+ struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state, plane);
...
}
@ depends on plane_atomic_func @
identifier plane_atomic_func.func;
identifier plane, plane_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *plane_state
+ struct drm_atomic_state *state
)
{ ... }
@ include depends on adds_new_state @
@@
#include
@ no_include depends on !include && adds_new_state @
@@
+ #include
#include
@@
identifier plane_atomic_func.func;
identifier plane, state;
identifier plane_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state) {
...
struct drm_plane_state *plane_state =
drm_atomic_get_new_plane_state(state, plane);
<+...
- plane_state->state
+ state
...+>
}
Acked-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Updated the comment according to Thomas suggestions
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++-
drivers/gpu/drm/drm_atomic_helper.c | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 26 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 33 ++-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 16 --
drivers/gpu/drm/vc4/vc4_plane.c | 56 ++-
include/drm/drm_modeset_helper_vtables.h | 18 +++---
7 files changed, 89 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 6ed96633425f..63f839679a0a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6468,7 +6468,7 @@ static int dm_plane_atomic_check(struct drm_plane *plane,
}
static int dm_plane_atomic_async_check(struct drm_plane *plane,
[PATCH v3 04/11] drm/atomic: Pass the full state to planes atomic_check
The current atomic helpers have either their object state being passed as
an argument or the full atomic state.
The former is the pattern that was done at first, before switching to the
latter for new hooks or when it was needed.
Let's convert all the remaining helpers to provide a consistent
interface, starting with the planes atomic_check.
The conversion was done using the coccinelle script below plus some
manual changes for vmwgfx, built tested on all the drivers.
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
int (*atomic_check)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_check = func,
...,
};
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_check(plane, plane_state)
+ FUNCS->atomic_check(plane, state)
...+>
}
@ ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
... when != new_plane_state
}
@ adds_new_state depends on plane_atomic_func && !ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
+ struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state, plane);
...
}
@ depends on plane_atomic_func @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *new_plane_state
+ struct drm_atomic_state *state
)
{ ... }
@ include depends on adds_new_state @
@@
#include
@ no_include depends on !include && adds_new_state @
@@
+ #include
#include
Reviewed-by: Laurent Pinchart
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Rewording and removal of a coccinelle rule suggested by Laurent
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++-
drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 4 +++-
drivers/gpu/drm/arm/hdlcd_crtc.c | 4 +++-
drivers/gpu/drm/arm/malidp_planes.c | 4 +++-
drivers/gpu/drm/armada/armada_plane.c | 4 +++-
drivers/gpu/drm/armada/armada_plane.h | 2 +-
drivers/gpu/drm/ast/ast_mode.c| 8 ++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 ++-
drivers/gpu/drm/drm_atomic_helper.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 4 +++-
drivers/gpu/drm/exynos/exynos_drm_plane.c | 4 +++-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 5 -
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 4 +++-
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 4 +++-
drivers/gpu/drm/imx/dcss/dcss-plane.c | 4 +++-
drivers/gpu/drm/imx/ipuv3-plane.c | 4 +++-
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 4 +++-
drivers/gpu/drm/ingenic/ingenic-ipu.c | 4 +++-
drivers/gpu/drm/kmb/kmb_plane.c | 4 +++-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 4 +++-
drivers/gpu/drm/meson/meson_overlay.c | 4 +++-
drivers/gpu/drm/meson/meson_plane.c | 4 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 -
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c| 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 4 +++-
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 4 +++-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 5 -
drivers/gpu/drm/omapdrm/omap_plane.c | 4 +++-
drivers/gpu/drm/qxl/qxl_display.c | 4 +++-
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +++-
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 -
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +++-
drivers/gpu/drm/sti/sti_cursor.c | 4 +++-
drivers/gpu/drm/sti/sti_gdp.c | 4 +++-
drivers/gpu/drm/sti/sti_hqvdp.c | 4 +++-
drivers/gpu/drm/stm/ltdc.c| 4 +++-
drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 4 +++-
drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 4 +++-
drivers/gpu/drm/tegra/dc.c| 8 ++--
drivers/gpu/drm/tegra/hub.c | 4 +++-
drivers/gpu/drm/tidss/tidss_plane.c | 4 +++-
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 4 +++-
drivers/gpu/drm/vboxvideo/vbox_mode.c | 8 ++--
drivers/gpu/drm/vc4/vc4_plane.c | 4
[PATCH v3 02/11] drm: Rename plane atomic_check state names
Most drivers call the argument to the plane atomic_check hook simply
state, which is going to conflict with the global atomic state in a
later rework. Let's rename it to new_plane_state (or new_state depending
on the convention used in the driver).
This was done using the coccinelle script below, and built tested:
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static const struct drm_plane_helper_funcs helpers = {
.atomic_check = func,
};
@ has_old_state @
identifier plane_atomic_func.func;
identifier plane;
expression e;
symbol old_state;
symbol state;
@@
func(struct drm_plane *plane, struct drm_plane_state *state)
{
...
struct drm_plane_state *old_state = e;
...
}
@ depends on has_old_state @
identifier plane_atomic_func.func;
identifier plane;
symbol old_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *state
+ struct drm_plane_state *new_state
)
{
<+...
- state
+ new_state
...+>
}
@ has_state @
identifier plane_atomic_func.func;
identifier plane;
symbol state;
@@
func(struct drm_plane *plane, struct drm_plane_state *state)
{
...
}
@ depends on has_state @
identifier plane_atomic_func.func;
identifier plane;
symbol old_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *state
+ struct drm_plane_state *new_plane_state
)
{
<+...
- state
+ new_plane_state
...+>
}
Reviewed-by: Laurent Pinchart
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Updated the variable name in the comment in omapdrm
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 +++---
.../gpu/drm/arm/display/komeda/komeda_plane.c | 11 ++---
drivers/gpu/drm/arm/hdlcd_crtc.c | 18
drivers/gpu/drm/arm/malidp_planes.c | 36
drivers/gpu/drm/armada/armada_plane.c | 41 ++-
drivers/gpu/drm/ast/ast_mode.c| 26 ++--
drivers/gpu/drm/exynos/exynos_drm_plane.c | 6 +--
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 6 +--
.../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 22 +-
.../gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 24 +--
drivers/gpu/drm/imx/dcss/dcss-plane.c | 26 ++--
drivers/gpu/drm/imx/ipuv3-plane.c | 31 +++---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 27 ++--
drivers/gpu/drm/ingenic/ingenic-ipu.c | 30 +++---
drivers/gpu/drm/kmb/kmb_plane.c | 22 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 16
drivers/gpu/drm/meson/meson_overlay.c | 10 +++--
drivers/gpu/drm/meson/meson_plane.c | 10 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 35
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 9 ++--
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 5 ++-
drivers/gpu/drm/omapdrm/omap_plane.c | 21 +-
drivers/gpu/drm/qxl/qxl_display.c | 6 +--
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++--
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 7 ++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 27 ++--
drivers/gpu/drm/sti/sti_cursor.c | 22 +-
drivers/gpu/drm/sti/sti_gdp.c | 26 ++--
drivers/gpu/drm/sti/sti_hqvdp.c | 24 +--
drivers/gpu/drm/stm/ltdc.c| 10 ++---
drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 10 +++--
drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 10 +++--
drivers/gpu/drm/tegra/dc.c| 38 -
drivers/gpu/drm/tegra/hub.c | 18
drivers/gpu/drm/tidss/tidss_plane.c | 34 ---
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 24 +--
drivers/gpu/drm/vc4/vc4_plane.c | 10 ++---
drivers/gpu/drm/virtio/virtgpu_plane.c| 9 ++--
drivers/gpu/drm/vkms/vkms_plane.c | 11 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 13 +++---
drivers/gpu/drm/xlnx/zynqmp_disp.c| 10 +++--
41 files changed, 403 insertions(+), 358 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 63f839679a0a..906fa4ae25c9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6432,7 +6432,7 @@ static int dm_plane_helper_check_state(struct
drm_plane_state *state,
}
static int dm_plane_atomic_check(struct drm_plane *plane,
-struct drm_plane_state *state)
+struct drm_plane_state *new_plane_state)
{
struct amdgpu_device *adev = drm_to_adev(plane->dev);
struct dc *dc = adev->dm.dc;
@@ -6441,23 +6441,24 @@ static int dm_plane_atomic_check(struct drm_plane
*plane,
str
[PATCH v3 05/11] drm: Use the state pointer directly in planes atomic_check
Now that atomic_check takes the global atomic state as a parameter, we
don't need to go through the pointer in the plane state.
This was done using the following coccinelle script:
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static struct drm_plane_helper_funcs helpers = {
...,
.atomic_check = func,
...,
};
@@
identifier plane_atomic_func.func;
identifier plane, state;
identifier plane_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state) {
...
- struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state,
plane);
<... when != plane_state
- plane_state->state
+ state
...>
}
@@
identifier plane_atomic_func.func;
identifier plane, state;
identifier plane_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state) {
...
struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state,
plane);
<...
- plane_state->state
+ state
...>
}
Reviewed-by: Laurent Pinchart
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Fixed the formatting in zynqmp_disp
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 2 +-
drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
drivers/gpu/drm/armada/armada_plane.c | 4 ++--
drivers/gpu/drm/ast/ast_mode.c| 4 ++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 2 +-
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +-
drivers/gpu/drm/imx/dcss/dcss-plane.c | 2 +-
drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +-
drivers/gpu/drm/ingenic/ingenic-ipu.c | 2 +-
drivers/gpu/drm/kmb/kmb_plane.c | 2 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +-
drivers/gpu/drm/meson/meson_overlay.c | 2 +-
drivers/gpu/drm/meson/meson_plane.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 2 +-
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 2 +-
drivers/gpu/drm/omapdrm/omap_plane.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/sti/sti_cursor.c | 2 +-
drivers/gpu/drm/sti/sti_gdp.c | 2 +-
drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 2 +-
drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 2 +-
drivers/gpu/drm/tidss/tidss_plane.c | 2 +-
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 2 +-
drivers/gpu/drm/vboxvideo/vbox_mode.c | 8
drivers/gpu/drm/virtio/virtgpu_plane.c| 2 +-
drivers/gpu/drm/vkms/vkms_plane.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
drivers/gpu/drm/xlnx/zynqmp_disp.c| 3 +--
drivers/gpu/drm/zte/zx_plane.c| 4 ++--
35 files changed, 41 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1cdff048b0c0..22124f76d0b5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6451,7 +6451,7 @@ static int dm_plane_atomic_check(struct drm_plane *plane,
return 0;
new_crtc_state =
- drm_atomic_get_new_crtc_state(new_plane_state->state,
+ drm_atomic_get_new_crtc_state(state,
new_plane_state->crtc);
if (!new_crtc_state)
return -EINVAL;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index 96a6fe95a4e7..13582c174bbb 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -84,7 +84,7 @@ komeda_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->crtc || !new_plane_state->fb)
return 0;
- crtc_st = drm_atomic_get_crtc_state(new_plane_state->state,
+ crtc_st = drm_atomic_get_crtc_state(state,
new_plane_state->crtc);
if (IS_ERR(crtc_st) || !crtc_st->enable) {
DRM_DEBUG_ATOMIC("Cannot update plane on a disabled CRTC.\n");
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 9da9d0581ce9..028ec39c8484 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -244,7 +244,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}
- for_each_new_crtc_
[PATCH v2 01/11] drm/atomic: Pass the full state to planes async atomic check and update
The current atomic helpers have either their object state being passed as
an argument or the full atomic state.
The former is the pattern that was done at first, before switching to the
latter for new hooks or when it was needed.
Let's start convert all the remaining helpers to provide a consistent
interface, starting with the planes atomic_async_check and
atomic_async_update.
The conversion was done using the coccinelle script below, built tested on
all the drivers.
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
int (*atomic_async_check)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
void (*atomic_async_update)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
(
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_async_check = func,
...,
};
|
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_async_update = func,
...,
};
)
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_async_check(plane, plane_state)
+ FUNCS->atomic_async_check(plane, state)
...+>
}
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_async_update(plane, plane_state)
+ FUNCS->atomic_async_update(plane, state)
...+>
}
@@
identifier mtk_plane_atomic_async_update;
identifier plane;
symbol new_state, state;
expression e;
@@
void mtk_plane_atomic_async_update(struct drm_plane *plane, struct
drm_plane_state *new_state)
{
...
- struct mtk_plane_state *state = e;
+ struct mtk_plane_state *new_plane_state = e;
<+...
- state
+ new_plane_state
...+>
}
@@
identifier plane_atomic_func.func;
identifier plane;
symbol state;
@@
func(struct drm_plane *plane,
-struct drm_plane_state *state)
+struct drm_plane_state *new_plane_state)
{
<...
- state
+ new_plane_state
...>
}
@ ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
... when != new_plane_state
}
@ adds_new_state depends on plane_atomic_func && !ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
+ struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state, plane);
...
}
@ depends on plane_atomic_func @
identifier plane_atomic_func.func;
identifier plane, plane_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *plane_state
+ struct drm_atomic_state *state
)
{ ... }
@ include depends on adds_new_state @
@@
#include
@ no_include depends on !include && adds_new_state @
@@
+ #include
#include
@@
identifier plane_atomic_func.func;
identifier plane, state;
identifier plane_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state) {
...
struct drm_plane_state *plane_state =
drm_atomic_get_new_plane_state(state, plane);
<+...
- plane_state->state
+ state
...+>
}
Acked-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Updated the comment according to Thomas suggestions
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++-
drivers/gpu/drm/drm_atomic_helper.c | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 26 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 33 ++-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 16 --
drivers/gpu/drm/vc4/vc4_plane.c | 56 ++-
include/drm/drm_modeset_helper_vtables.h | 18 +++---
7 files changed, 89 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 86c2b2c897bb..7caebb1a475a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6469,7 +6469,7 @@ static int dm_plane_atomic_check(struct drm_plane *plane,
}
static int dm_plane_atomic_async_check(struct drm_plane *plane,
[PATCH v2 02/11] drm: Rename plane atomic_check state names
Most drivers call the argument to the plane atomic_check hook simply
state, which is going to conflict with the global atomic state in a
later rework. Let's rename it to new_plane_state (or new_state depending
on the convention used in the driver).
This was done using the coccinelle script below, and built tested:
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static const struct drm_plane_helper_funcs helpers = {
.atomic_check = func,
};
@ has_old_state @
identifier plane_atomic_func.func;
identifier plane;
expression e;
symbol old_state;
symbol state;
@@
func(struct drm_plane *plane, struct drm_plane_state *state)
{
...
struct drm_plane_state *old_state = e;
...
}
@ depends on has_old_state @
identifier plane_atomic_func.func;
identifier plane;
symbol old_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *state
+ struct drm_plane_state *new_state
)
{
<+...
- state
+ new_state
...+>
}
@ has_state @
identifier plane_atomic_func.func;
identifier plane;
symbol state;
@@
func(struct drm_plane *plane, struct drm_plane_state *state)
{
...
}
@ depends on has_state @
identifier plane_atomic_func.func;
identifier plane;
symbol old_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *state
+ struct drm_plane_state *new_plane_state
)
{
<+...
- state
+ new_plane_state
...+>
}
Reviewed-by: Laurent Pinchart
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Updated the variable name in the comment in omapdrm
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 +++---
.../gpu/drm/arm/display/komeda/komeda_plane.c | 11 ++---
drivers/gpu/drm/arm/hdlcd_crtc.c | 18
drivers/gpu/drm/arm/malidp_planes.c | 36
drivers/gpu/drm/armada/armada_plane.c | 41 ++-
drivers/gpu/drm/ast/ast_mode.c| 26 ++--
drivers/gpu/drm/exynos/exynos_drm_plane.c | 6 +--
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 6 +--
.../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 22 +-
.../gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 24 +--
drivers/gpu/drm/imx/dcss/dcss-plane.c | 26 ++--
drivers/gpu/drm/imx/ipuv3-plane.c | 31 +++---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 27 ++--
drivers/gpu/drm/ingenic/ingenic-ipu.c | 30 +++---
drivers/gpu/drm/kmb/kmb_plane.c | 22 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 16
drivers/gpu/drm/meson/meson_overlay.c | 10 +++--
drivers/gpu/drm/meson/meson_plane.c | 10 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 35
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 9 ++--
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 5 ++-
drivers/gpu/drm/omapdrm/omap_plane.c | 21 +-
drivers/gpu/drm/qxl/qxl_display.c | 6 +--
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++--
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 7 ++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 27 ++--
drivers/gpu/drm/sti/sti_cursor.c | 22 +-
drivers/gpu/drm/sti/sti_gdp.c | 26 ++--
drivers/gpu/drm/sti/sti_hqvdp.c | 24 +--
drivers/gpu/drm/stm/ltdc.c| 10 ++---
drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 10 +++--
drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 10 +++--
drivers/gpu/drm/tegra/dc.c| 38 -
drivers/gpu/drm/tegra/hub.c | 18
drivers/gpu/drm/tidss/tidss_plane.c | 34 ---
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 24 +--
drivers/gpu/drm/vc4/vc4_plane.c | 10 ++---
drivers/gpu/drm/virtio/virtgpu_plane.c| 9 ++--
drivers/gpu/drm/vkms/vkms_plane.c | 11 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 13 +++---
drivers/gpu/drm/xlnx/zynqmp_disp.c| 10 +++--
41 files changed, 403 insertions(+), 358 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 7caebb1a475a..dcf970454121 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6433,7 +6433,7 @@ static int dm_plane_helper_check_state(struct
drm_plane_state *state,
}
static int dm_plane_atomic_check(struct drm_plane *plane,
-struct drm_plane_state *state)
+struct drm_plane_state *new_plane_state)
{
struct amdgpu_device *adev = drm_to_adev(plane->dev);
struct dc *dc = adev->dm.dc;
@@ -6442,23 +6442,24 @@ static int dm_plane_atomic_check(struct drm_plane
*plane,
str
[PATCH v2 04/11] drm/atomic: Pass the full state to planes atomic_check
The current atomic helpers have either their object state being passed as
an argument or the full atomic state.
The former is the pattern that was done at first, before switching to the
latter for new hooks or when it was needed.
Let's convert all the remaining helpers to provide a consistent
interface, starting with the planes atomic_check.
The conversion was done using the coccinelle script below plus some
manual changes for vmwgfx, built tested on all the drivers.
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
int (*atomic_check)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_check = func,
...,
};
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_check(plane, plane_state)
+ FUNCS->atomic_check(plane, state)
...+>
}
@ ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
... when != new_plane_state
}
@ adds_new_state depends on plane_atomic_func && !ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
+ struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state, plane);
...
}
@ depends on plane_atomic_func @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *new_plane_state
+ struct drm_atomic_state *state
)
{ ... }
@ include depends on adds_new_state @
@@
#include
@ no_include depends on !include && adds_new_state @
@@
+ #include
#include
Reviewed-by: Laurent Pinchart
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Rewording and removal of a coccinelle rule suggested by Laurent
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++-
drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 4 +++-
drivers/gpu/drm/arm/hdlcd_crtc.c | 4 +++-
drivers/gpu/drm/arm/malidp_planes.c | 4 +++-
drivers/gpu/drm/armada/armada_plane.c | 4 +++-
drivers/gpu/drm/armada/armada_plane.h | 2 +-
drivers/gpu/drm/ast/ast_mode.c| 8 ++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 ++-
drivers/gpu/drm/drm_atomic_helper.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 4 +++-
drivers/gpu/drm/exynos/exynos_drm_plane.c | 4 +++-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 5 -
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 4 +++-
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 4 +++-
drivers/gpu/drm/imx/dcss/dcss-plane.c | 4 +++-
drivers/gpu/drm/imx/ipuv3-plane.c | 4 +++-
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 4 +++-
drivers/gpu/drm/ingenic/ingenic-ipu.c | 4 +++-
drivers/gpu/drm/kmb/kmb_plane.c | 4 +++-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 4 +++-
drivers/gpu/drm/meson/meson_overlay.c | 4 +++-
drivers/gpu/drm/meson/meson_plane.c | 4 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 -
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c| 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 4 +++-
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 4 +++-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 5 -
drivers/gpu/drm/omapdrm/omap_plane.c | 4 +++-
drivers/gpu/drm/qxl/qxl_display.c | 4 +++-
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +++-
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 -
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +++-
drivers/gpu/drm/sti/sti_cursor.c | 4 +++-
drivers/gpu/drm/sti/sti_gdp.c | 4 +++-
drivers/gpu/drm/sti/sti_hqvdp.c | 4 +++-
drivers/gpu/drm/stm/ltdc.c| 4 +++-
drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 4 +++-
drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 4 +++-
drivers/gpu/drm/tegra/dc.c| 8 ++--
drivers/gpu/drm/tegra/hub.c | 4 +++-
drivers/gpu/drm/tidss/tidss_plane.c | 4 +++-
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 4 +++-
drivers/gpu/drm/vboxvideo/vbox_mode.c | 8 ++--
drivers/gpu/drm/vc4/vc4_plane.c | 4
[PATCH v2 05/11] drm: Use the state pointer directly in planes atomic_check
Now that atomic_check takes the global atomic state as a parameter, we
don't need to go through the pointer in the plane state.
This was done using the following coccinelle script:
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static struct drm_plane_helper_funcs helpers = {
...,
.atomic_check = func,
...,
};
@@
identifier plane_atomic_func.func;
identifier plane, state;
identifier plane_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state) {
...
- struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state,
plane);
<... when != plane_state
- plane_state->state
+ state
...>
}
@@
identifier plane_atomic_func.func;
identifier plane, state;
identifier plane_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state) {
...
struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state,
plane);
<...
- plane_state->state
+ state
...>
}
Reviewed-by: Laurent Pinchart
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Fixed the formatting in zynqmp_disp
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 2 +-
drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
drivers/gpu/drm/armada/armada_plane.c | 4 ++--
drivers/gpu/drm/ast/ast_mode.c| 4 ++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 2 +-
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +-
drivers/gpu/drm/imx/dcss/dcss-plane.c | 2 +-
drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +-
drivers/gpu/drm/ingenic/ingenic-ipu.c | 2 +-
drivers/gpu/drm/kmb/kmb_plane.c | 2 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +-
drivers/gpu/drm/meson/meson_overlay.c | 2 +-
drivers/gpu/drm/meson/meson_plane.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 2 +-
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 2 +-
drivers/gpu/drm/omapdrm/omap_plane.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/sti/sti_cursor.c | 2 +-
drivers/gpu/drm/sti/sti_gdp.c | 2 +-
drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 2 +-
drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 2 +-
drivers/gpu/drm/tidss/tidss_plane.c | 2 +-
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 2 +-
drivers/gpu/drm/vboxvideo/vbox_mode.c | 8
drivers/gpu/drm/virtio/virtgpu_plane.c| 2 +-
drivers/gpu/drm/vkms/vkms_plane.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
drivers/gpu/drm/xlnx/zynqmp_disp.c| 3 +--
drivers/gpu/drm/zte/zx_plane.c| 4 ++--
35 files changed, 41 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index dc340adba098..63284fe2bc22 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6452,7 +6452,7 @@ static int dm_plane_atomic_check(struct drm_plane *plane,
return 0;
new_crtc_state =
- drm_atomic_get_new_crtc_state(new_plane_state->state,
+ drm_atomic_get_new_crtc_state(state,
new_plane_state->crtc);
if (!new_crtc_state)
return -EINVAL;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index 2b67b6b9a6b5..4cc4800f0ae5 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -86,7 +86,7 @@ komeda_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->crtc || !new_plane_state->fb)
return 0;
- crtc_st = drm_atomic_get_crtc_state(new_plane_state->state,
+ crtc_st = drm_atomic_get_crtc_state(state,
new_plane_state->crtc);
if (IS_ERR(crtc_st) || !crtc_st->enable) {
DRM_DEBUG_ATOMIC("Cannot update plane on a disabled CRTC.\n");
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 9da9d0581ce9..028ec39c8484 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -244,7 +244,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}
- for_each_new_crtc_
[PATCH 01/10] drm/atomic: Pass the full state to planes async atomic check and update
The current atomic helpers have either their object state being passed as
an argument or the full atomic state.
The former is the pattern that was done at first, before switching to the
latter for new hooks or when it was needed.
Let's start convert all the remaining helpers to provide a consistent
interface, starting with the planes atomic_async_check and
atomic_async_update.
The conversion was done using the coccinelle script below, built tested on
all the drivers.
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
int (*atomic_async_check)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
void (*atomic_async_update)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
(
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_async_check = func,
...,
};
|
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_async_update = func,
...,
};
)
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_async_check(plane, plane_state)
+ FUNCS->atomic_async_check(plane, state)
...+>
}
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_async_update(plane, plane_state)
+ FUNCS->atomic_async_update(plane, state)
...+>
}
@@
identifier mtk_plane_atomic_async_update;
identifier plane;
symbol new_state, state;
expression e;
@@
void mtk_plane_atomic_async_update(struct drm_plane *plane, struct
drm_plane_state *new_state)
{
...
- struct mtk_plane_state *state = e;
+ struct mtk_plane_state *new_plane_state = e;
<+...
- state
+ new_plane_state
...+>
}
@@
identifier plane_atomic_func.func;
identifier plane;
symbol state;
@@
func(struct drm_plane *plane,
-struct drm_plane_state *state)
+struct drm_plane_state *new_plane_state)
{
<...
- state
+ new_plane_state
...>
}
@ ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
... when != new_plane_state
}
@ adds_new_state depends on plane_atomic_func && !ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
+ struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state, plane);
...
}
@ depends on plane_atomic_func @
identifier plane_atomic_func.func;
identifier plane, plane_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *plane_state
+ struct drm_atomic_state *state
)
{ ... }
@ include depends on adds_new_state @
@@
#include
@ no_include depends on !include && adds_new_state @
@@
+ #include
#include
@@
identifier plane_atomic_func.func;
identifier plane, state;
identifier plane_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state) {
...
struct drm_plane_state *plane_state =
drm_atomic_get_new_plane_state(state, plane);
<+...
- plane_state->state
+ state
...+>
}
Signed-off-by: Maxime Ripard
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++-
drivers/gpu/drm/drm_atomic_helper.c | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 26 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 33 ++-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 16 --
drivers/gpu/drm/vc4/vc4_plane.c | 56 ++-
include/drm/drm_modeset_helper_vtables.h | 14 ++---
7 files changed, 87 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5675c1f9368a..476bf2e6a4f4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6475,7 +6475,7 @@ static int dm_plane_atomic_check(struct drm_plane *plane,
}
static int dm_plane_atomic_async_check(struct drm_plane *plane,
- struct drm_plane_state *new_plane_state)
+
[PATCH 02/10] drm: Rename plane atomic_check state names
Most drivers call the argument to the plane atomic_check hook simply
state, which is going to conflict with the global atomic state in a
later rework. Let's rename it to new_plane_state (or new_state depending
on the convention used in the driver).
This was done using the coccinelle script below, and built tested:
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static const struct drm_plane_helper_funcs helpers = {
.atomic_check = func,
};
@ has_old_state @
identifier plane_atomic_func.func;
identifier plane;
expression e;
symbol old_state;
symbol state;
@@
func(struct drm_plane *plane, struct drm_plane_state *state)
{
...
struct drm_plane_state *old_state = e;
...
}
@ depends on has_old_state @
identifier plane_atomic_func.func;
identifier plane;
symbol old_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *state
+ struct drm_plane_state *new_state
)
{
<+...
- state
+ new_state
...+>
}
@ has_state @
identifier plane_atomic_func.func;
identifier plane;
symbol state;
@@
func(struct drm_plane *plane, struct drm_plane_state *state)
{
...
}
@ depends on has_state @
identifier plane_atomic_func.func;
identifier plane;
symbol old_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *state
+ struct drm_plane_state *new_plane_state
)
{
<+...
- state
+ new_plane_state
...+>
}
Signed-off-by: Maxime Ripard
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 +++---
.../gpu/drm/arm/display/komeda/komeda_plane.c | 11 ++---
drivers/gpu/drm/arm/hdlcd_crtc.c | 18
drivers/gpu/drm/arm/malidp_planes.c | 36
drivers/gpu/drm/armada/armada_plane.c | 41 ++-
drivers/gpu/drm/ast/ast_mode.c| 26 ++--
drivers/gpu/drm/exynos/exynos_drm_plane.c | 6 +--
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 6 +--
.../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 22 +-
.../gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 24 +--
drivers/gpu/drm/imx/dcss/dcss-plane.c | 26 ++--
drivers/gpu/drm/imx/ipuv3-plane.c | 31 +++---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 27 ++--
drivers/gpu/drm/ingenic/ingenic-ipu.c | 30 +++---
drivers/gpu/drm/kmb/kmb_plane.c | 22 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 16
drivers/gpu/drm/meson/meson_overlay.c | 10 +++--
drivers/gpu/drm/meson/meson_plane.c | 10 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 35
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 9 ++--
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 5 ++-
drivers/gpu/drm/omapdrm/omap_plane.c | 19 +
drivers/gpu/drm/qxl/qxl_display.c | 6 +--
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++--
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 7 ++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 27 ++--
drivers/gpu/drm/sti/sti_cursor.c | 22 +-
drivers/gpu/drm/sti/sti_gdp.c | 26 ++--
drivers/gpu/drm/sti/sti_hqvdp.c | 24 +--
drivers/gpu/drm/stm/ltdc.c| 10 ++---
drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 10 +++--
drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 10 +++--
drivers/gpu/drm/tegra/dc.c| 38 -
drivers/gpu/drm/tegra/hub.c | 18
drivers/gpu/drm/tidss/tidss_plane.c | 34 ---
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 24 +--
drivers/gpu/drm/vc4/vc4_plane.c | 10 ++---
drivers/gpu/drm/virtio/virtgpu_plane.c| 9 ++--
drivers/gpu/drm/vkms/vkms_plane.c | 11 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 13 +++---
drivers/gpu/drm/xlnx/zynqmp_disp.c| 10 +++--
41 files changed, 402 insertions(+), 357 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 476bf2e6a4f4..d85336c43e5e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6439,7 +6439,7 @@ static int dm_plane_helper_check_state(struct
drm_plane_state *state,
}
static int dm_plane_atomic_check(struct drm_plane *plane,
-struct drm_plane_state *state)
+struct drm_plane_state *new_plane_state)
{
struct amdgpu_device *adev = drm_to_adev(plane->dev);
struct dc *dc = adev->dm.dc;
@@ -6448,23 +6448,24 @@ static int dm_plane_atomic_check(struct drm_plane
*plane,
struct drm_crtc_state *new_crtc_state;
int ret;
- trace_amdgpu_dm_plane_
Re: [PATCH 02/10] drm: Rename plane atomic_check state names
Hi,
On Fri, Jan 15, 2021 at 02:46:36PM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 15.01.21 um 13:56 schrieb Maxime Ripard:
> > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
> > b/drivers/gpu/drm/imx/ipuv3-plane.c
> > index 8a4235d9d9f1..2cb09e9d9306 100644
> > --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> > @@ -344,12 +344,12 @@ static const struct drm_plane_funcs ipu_plane_funcs =
> > {
> > };
> > static int ipu_plane_atomic_check(struct drm_plane *plane,
> > - struct drm_plane_state *state)
> > + struct drm_plane_state *new_state)
>
> It's not 'new_plane_state' ?
That function is using old_state for plane->state:
> > {
> > struct drm_plane_state *old_state = plane->state;
Here ^
So it felt more natural to keep the convention in use in that driver
Maxime
signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 05/10] drm: Use the state pointer directly in planes atomic_check
Now that atomic_check takes the global atomic state as a parameter, we
don't need to go through the pointer in the plane state.
This was done using the following coccinelle script:
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static struct drm_plane_helper_funcs helpers = {
...,
.atomic_check = func,
...,
};
@@
identifier plane_atomic_func.func;
identifier plane, state;
identifier plane_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state) {
...
- struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state,
plane);
<... when != plane_state
- plane_state->state
+ state
...>
}
@@
identifier plane_atomic_func.func;
identifier plane, state;
identifier plane_state;
@@
func(struct drm_plane *plane, struct drm_atomic_state *state) {
...
struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state,
plane);
<...
- plane_state->state
+ state
...>
}
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 2 +-
drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
drivers/gpu/drm/armada/armada_plane.c | 4 ++--
drivers/gpu/drm/ast/ast_mode.c| 4 ++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 2 +-
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +-
drivers/gpu/drm/imx/dcss/dcss-plane.c | 2 +-
drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +-
drivers/gpu/drm/ingenic/ingenic-ipu.c | 2 +-
drivers/gpu/drm/kmb/kmb_plane.c | 2 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +-
drivers/gpu/drm/meson/meson_overlay.c | 2 +-
drivers/gpu/drm/meson/meson_plane.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 2 +-
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 2 +-
drivers/gpu/drm/omapdrm/omap_plane.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/sti/sti_cursor.c | 2 +-
drivers/gpu/drm/sti/sti_gdp.c | 2 +-
drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 2 +-
drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 2 +-
drivers/gpu/drm/tidss/tidss_plane.c | 2 +-
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 2 +-
drivers/gpu/drm/vboxvideo/vbox_mode.c | 8
drivers/gpu/drm/virtio/virtgpu_plane.c| 2 +-
drivers/gpu/drm/vkms/vkms_plane.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
drivers/gpu/drm/xlnx/zynqmp_disp.c| 2 +-
drivers/gpu/drm/zte/zx_plane.c| 4 ++--
35 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 84e35021d9bc..3e056a1fb11a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6458,7 +6458,7 @@ static int dm_plane_atomic_check(struct drm_plane *plane,
return 0;
new_crtc_state =
- drm_atomic_get_new_crtc_state(new_plane_state->state,
+ drm_atomic_get_new_crtc_state(state,
new_plane_state->crtc);
if (!new_crtc_state)
return -EINVAL;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index 2b67b6b9a6b5..4cc4800f0ae5 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -86,7 +86,7 @@ komeda_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->crtc || !new_plane_state->fb)
return 0;
- crtc_st = drm_atomic_get_crtc_state(new_plane_state->state,
+ crtc_st = drm_atomic_get_crtc_state(state,
new_plane_state->crtc);
if (IS_ERR(crtc_st) || !crtc_st->enable) {
DRM_DEBUG_ATOMIC("Cannot update plane on a disabled CRTC.\n");
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 9da9d0581ce9..028ec39c8484 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -244,7 +244,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}
- for_each_new_crtc_in_state(new_plane_state->state, crtc, crtc_state,
+ for_each_new_crtc_in_state(state, cr
[PATCH 04/10] drm/atomic: Pass the full state to planes atomic_check
The current atomic helpers have either their object state being passed as
an argument or the full atomic state.
The former is the pattern that was done at first, before switching to the
latter for new hooks or when it was needed.
Let's start convert all the remaining helpers to provide a consistent
interface, starting with the planes atomic_check.
The conversion was done using the coccinelle script below plus some
manual changes for vmwgfx, built tested on all the drivers.
@@
identifier plane, plane_state;
symbol state;
@@
struct drm_plane_helper_funcs {
...
int (*atomic_check)(struct drm_plane *plane,
- struct drm_plane_state *plane_state);
+ struct drm_atomic_state *state);
...
}
@ plane_atomic_func @
identifier helpers;
identifier func;
@@
static const struct drm_plane_helper_funcs helpers = {
...,
.atomic_check = func,
...,
};
@@
struct drm_plane_helper_funcs *FUNCS;
identifier f;
identifier dev;
identifier plane, plane_state, state;
@@
f(struct drm_device *dev, struct drm_atomic_state *state)
{
<+...
- FUNCS->atomic_check(plane, plane_state)
+ FUNCS->atomic_check(plane, state)
...+>
}
@@
identifier plane_atomic_func.func;
identifier plane;
symbol state;
@@
func(struct drm_plane *plane,
-struct drm_plane_state *state)
+struct drm_plane_state *new_plane_state)
{
<...
- state
+ new_plane_state
...>
}
@ ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
... when != new_plane_state
}
@ adds_new_state depends on plane_atomic_func && !ignores_new_state @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
{
+ struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state, plane);
...
}
@ depends on plane_atomic_func @
identifier plane_atomic_func.func;
identifier plane, new_plane_state;
@@
func(struct drm_plane *plane,
- struct drm_plane_state *new_plane_state
+ struct drm_atomic_state *state
)
{ ... }
@ include depends on adds_new_state @
@@
#include
@ no_include depends on !include && adds_new_state @
@@
+ #include
#include
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++-
drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 4 +++-
drivers/gpu/drm/arm/hdlcd_crtc.c | 4 +++-
drivers/gpu/drm/arm/malidp_planes.c | 4 +++-
drivers/gpu/drm/armada/armada_plane.c | 4 +++-
drivers/gpu/drm/armada/armada_plane.h | 2 +-
drivers/gpu/drm/ast/ast_mode.c| 8 ++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 ++-
drivers/gpu/drm/drm_atomic_helper.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 4 +++-
drivers/gpu/drm/exynos/exynos_drm_plane.c | 4 +++-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 5 -
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 4 +++-
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 4 +++-
drivers/gpu/drm/imx/dcss/dcss-plane.c | 4 +++-
drivers/gpu/drm/imx/ipuv3-plane.c | 4 +++-
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 4 +++-
drivers/gpu/drm/ingenic/ingenic-ipu.c | 4 +++-
drivers/gpu/drm/kmb/kmb_plane.c | 4 +++-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 4 +++-
drivers/gpu/drm/meson/meson_overlay.c | 4 +++-
drivers/gpu/drm/meson/meson_plane.c | 4 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 -
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c| 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 4 +++-
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 4 +++-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 5 -
drivers/gpu/drm/omapdrm/omap_plane.c | 4 +++-
drivers/gpu/drm/qxl/qxl_display.c | 4 +++-
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +++-
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 -
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +++-
drivers/gpu/drm/sti/sti_cursor.c | 4 +++-
drivers/gpu/drm/sti/sti_gdp.c | 4 +++-
drivers/gpu/drm/sti/sti_hqvdp.c | 4 +++-
drivers/gpu/drm/stm/ltdc.c| 4 +++-
drivers/gpu/drm/sun4i/sun8i_ui_layer.c| 4 +++-
drivers/gpu/drm/sun4i/sun8i_vi_layer.c| 4 +++-
drivers/gpu/drm/tegra/dc.c| 8 ++--
drivers/gpu/drm/tegra/hub.c | 4 +++-
drivers/gpu/drm/tidss/tidss_plane.c | 4 +++-
drivers/gpu/drm/tilcdc/tilcdc_plane.c
Re: [PATCH v6 07/24] drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs directory
On Fri, Jul 26, 2019 at 07:23:01PM +0200, Andrzej Pietrasiewicz wrote: > Use the ddc pointer provided by the generic connector. > > Signed-off-by: Andrzej Pietrasiewicz Acked-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
