RE: [RFC 0/5] Introduce drm sharpening property
On Monday, March 4th, 2024 at 15:04, Garg, Nemesa wrote: > This is generic as sharpness effect is applied post blending. Depending > on the color gamut, pixel format and other inputs the image gets > blended and once we get blended output it can be sharpened based on > strength value provided by the user. It would really help if you could provide the exact mathematical formula applied by this KMS property.
Re: [PATCH v2 1/2] drm: Introduce plane SIZE_HINTS property
On Tuesday, February 27th, 2024 at 20:35, Ville Syrjala wrote: > From: Ville Syrjälä > > Add a new immutable plane property by which a plane can advertise > a handful of recommended plane sizes. This would be mostly exposed > by cursor planes as a slightly more capable replacement for > the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare > a one size fits all limit for the whole device. > > Currently eg. amdgpu/i915/nouveau just advertize the max cursor > size via the cursor size caps. But always using the max sized > cursor can waste a surprising amount of power, so a better > stragey is desirable. Typo: strategy > Most other drivers don't specify any cursor size at all, in > which case the ioctl code just claims that 64x64 is a great > choice. Whether that is actually true is debatable. > > A poll of various compositor developers informs us that > blindly probing with setcursor/atomic ioctl to determine > suitable cursor sizes is not acceptable, thus the > introduction of the new property to supplant the cursor > size caps. The compositor will now be free to select a > more optimal cursor size from the short list of options. > > Note that the reported sizes (either via the property or the > caps) make no claims about things such as plane scaling. So > these things should only really be consulted for simple > "cursor like" use cases. > > Userspace consumer in the form of mutter seems ready: > https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3165 Do we need an IGT as well to merge this new uAPI? > v2: Try to add some docs > v3: Specify that value 0 is reserved for future use (basic idea from Jonas) > Drop the note about typical hardware (Pekka) > v4: Update the docs to indicate the list is "in order of preference" > Add a a link to the mutter MR > > Cc: Simon Ser > Cc: Jonas Ådahl > Cc: Daniel Stone > Cc: Sameer Lattannavar > Cc: Sebastian Wick > Acked-by: Harry Wentland > Acked-by: Pekka Paalanen > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_mode_config.c | 7 + > drivers/gpu/drm/drm_plane.c | 52 +++ > include/drm/drm_mode_config.h | 5 +++ > include/drm/drm_plane.h | 4 +++ > include/uapi/drm/drm_mode.h | 11 +++ > 5 files changed, 79 insertions(+) > > diff --git a/drivers/gpu/drm/drm_mode_config.c > b/drivers/gpu/drm/drm_mode_config.c > index 48fd2d67f352..568972258222 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -372,6 +372,13 @@ static int drm_mode_create_standard_properties(struct > drm_device *dev) > return -ENOMEM; > dev->mode_config.modifiers_property = prop; > > + prop = drm_property_create(dev, > +DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB, > +"SIZE_HINTS", 0); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.size_hints_property = prop; > + > return 0; > } > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 672c655c7a8e..4135ce16e608 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -140,6 +140,25 @@ > * DRM_FORMAT_MOD_LINEAR. Before linux kernel release v5.1 there have > been > * various bugs in this area with inconsistencies between the capability > * flag and per-plane properties. > + * > + * SIZE_HINTS: > + * Blob property which contains the set of recommended plane size > + * which can used for simple "cursor like" use cases (eg. no scaling). > + * Using these hints frees userspace from extensive probing of > + * supported plane sizes through atomic/setcursor ioctls. > + * > + * The blob contains an array of struct drm_plane_size_hint, in > + * order of preference. For optimal usage userspace should pick > + * the first size that satisfies its own requirements. > + * > + * Drivers should only attach this property to planes that > + * support a very limited set of sizes. > + * > + * Note that property value 0 (ie. no blob) is reserved for potential > + * future use. Current userspace is expected to ignore the property > + * if the value is 0, and fall back to some other means (eg. > + * _CAP_CURSOR_WIDTH and _CAP_CURSOR_HEIGHT) to determine > + * the appropriate plane size to use. > */ > > static unsigned int drm_num_planes(struct drm_device *dev) > @@ -1729,3 +1748,36 @@ int drm_plane_create_scaling_filter_property(struct > drm_plane *plane, > return 0; > } > EXPORT_SYMBOL(drm_
Re: [RFC 0/5] Introduce drm sharpening property
How much of this is Intel-specific? Are there any hardware vendors with a similar feature? How much is the "sharpness" knob tied to Intel hardware?
Re: [Intel-gfx] [PATCH v9 0/4] drm: Add support for atomic async page-flip
Thanks! This iteration of the first 3 patches LGTM, I've pushed them to drm-misc-next. I've made two adjustments to make checkpatch.pl happy: - s/uint64_t/u64/ - Fix indentation for a drm_dbg_atomic()
[Intel-gfx] [PATCH] drm/atomic-helper: relax unregistered connector check
The driver might pull connectors which weren't submitted by
user-space into the atomic state. For instance,
intel_dp_mst_atomic_master_trans_check() pulls in connectors
sharing the same DP-MST stream. However, if the connector is
unregistered, this later fails with:
[ 559.425658] i915 :00:02.0: [drm:drm_atomic_helper_check_modeset]
[CONNECTOR:378:DP-7] is not registered
Skip the unregistered connector check to allow user-space to turn
off connectors one-by-one.
See this wlroots issue:
https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3407
Previous discussion:
https://lore.kernel.org/intel-gfx/y6gx7z17wmdsk...@ideak-desk.fi.intel.com/
Signed-off-by: Simon Ser
Cc: Ville Syrjälä
Cc: Jani Nikula
Cc: Lyude Paul
Cc: Imre Deak
---
drivers/gpu/drm/drm_atomic_helper.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 71d399397107..c9b8343eaa20 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -290,7 +290,8 @@ static int
update_connector_routing(struct drm_atomic_state *state,
struct drm_connector *connector,
struct drm_connector_state *old_connector_state,
-struct drm_connector_state *new_connector_state)
+struct drm_connector_state *new_connector_state,
+bool added_by_user)
{
const struct drm_connector_helper_funcs *funcs;
struct drm_encoder *new_encoder;
@@ -339,9 +340,13 @@ update_connector_routing(struct drm_atomic_state *state,
* there's a chance the connector may have been destroyed during the
* process, but it's better to ignore that then cause
* drm_atomic_helper_resume() to fail.
+*
+* Last, we want to ignore connector registration when the connector
+* was not pulled in the atomic state by user-space (ie, was pulled
+* in by the driver, e.g. when updating a DP-MST stream).
*/
if (!state->duplicated && drm_connector_is_unregistered(connector) &&
- crtc_state->active) {
+ added_by_user && crtc_state->active) {
drm_dbg_atomic(connector->dev,
"[CONNECTOR:%d:%s] is not registered\n",
connector->base.id, connector->name);
@@ -620,7 +625,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
struct drm_connector *connector;
struct drm_connector_state *old_connector_state, *new_connector_state;
int i, ret;
- unsigned int connectors_mask = 0;
+ unsigned int connectors_mask = 0, user_connectors_mask = 0;
+
+ for_each_oldnew_connector_in_state(state, connector,
old_connector_state, new_connector_state, i)
+ user_connectors_mask |= BIT(i);
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
bool has_connectors =
@@ -685,7 +693,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
*/
ret = update_connector_routing(state, connector,
old_connector_state,
- new_connector_state);
+ new_connector_state,
+ BIT(i) &
user_connectors_mask);
if (ret)
return ret;
if (old_connector_state->crtc) {
--
2.42.0
Re: [Intel-gfx] [PATCH v6 3/4] drm: Expand max DRM device number to full MINORBITS
On Wednesday, August 23rd, 2023 at 12:53, Simon Ser wrote: > On Tuesday, August 8th, 2023 at 17:04, James Zhu jam...@amd.com wrote: > > > I have a MR for libdrm to support drm nodes type up to 2^MINORBITS > > nodes which can work with these patches, > > > > https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/305 > > FWIW, this MR has been merged, so in theory this kernel patch should > work fine with latest libdrm. Hm, we might want to adjust MAX_DRM_NODES still. It's set to 256 currently, which should be enough for 128 DRM devices, but not more. Not bumping this value will make drmGetDevices2() print a warning and not return all devices on systems with more than 128 DRM devices.
Re: [Intel-gfx] [PATCH v6 3/4] drm: Expand max DRM device number to full MINORBITS
On Tuesday, August 8th, 2023 at 17:04, James Zhu wrote: > I have a MR for libdrm to support drm nodes type up to 2^MINORBITS > nodes which can work with these patches, > > https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/305 FWIW, this MR has been merged, so in theory this kernel patch should work fine with latest libdrm.
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thursday, August 17th, 2023 at 21:33, Dmitry Baryshkov wrote: > We have been looking for a way to document that the corresponding DP > port is represented by the USB connector on the device. > > Consequently, I believe the best way to document it, would be to use > DisplayPort / USB, when there is no dongle connected, switching to > DisplayPort / HDMI, DisplayPort / VGA, DisplayPort / DisplayPort, etc. > when the actual dongle / display is connected and then switching back to > the DisplayPort / USB when it gets disconnected. > > If this sounds good to all parties, I'll post v2, adding this > explanation to the cover letter. But how can user-space discover that the port is USB-C when it's connected? That information is lost at this point. (In addition, this clashes with the existing semantics of the subconnector prop as discussed before: USB-C is not sub-, it's super-.)
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thursday, August 3rd, 2023 at 22:44, Laurent Pinchart wrote: > On Thu, Aug 03, 2023 at 03:31:16PM +0000, Simon Ser wrote: > > > On Thursday, August 3rd, 2023 at 17:22, Simon Ser cont...@emersion.fr wrote: > > > > > The KMS docs describe "subconnector" to be defined as "downstream port" > > > for DP. > > > Can USB-C (or USB) be seen as a DP downstream port? > > > > To expand on this a bit: I'm wondering if we're mixing apples and > > oranges here. The current values of "subconnector" typically describe > > the lower-level protocol tunneled inside DP. For instance, VGA can be > > tunneled inside the DP cable when using DP → VGA adapter. > > Doesn't this contradict the example use case you gave in your previous > e-mail, with wlroots stating "DP-3 via DVI-D" ? I understand that as DP > carried over a DVI-D physical connector, did I get it wrong ? No, this is DVI carried over DP. DP cannot be carried over VGA/DVI/HDMI, but VGA/DVI/HDMI can be carried over DP.
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thursday, August 3rd, 2023 at 17:36, Dmitry Baryshkov wrote: > On Thu, 3 Aug 2023 at 18:31, Simon Ser cont...@emersion.fr wrote: > > > On Thursday, August 3rd, 2023 at 17:22, Simon Ser cont...@emersion.fr wrote: > > > > > The KMS docs describe "subconnector" to be defined as "downstream port" > > > for DP. > > > Can USB-C (or USB) be seen as a DP downstream port? > > > > To expand on this a bit: I'm wondering if we're mixing apples and > > oranges here. The current values of "subconnector" typically describe > > the lower-level protocol tunneled inside DP. For instance, VGA can be > > tunneled inside the DP cable when using DP → VGA adapter. > > My opinion hasn't changed: I think this should be the USB connector > with proper DP / DVI / HDMI / etc. subconnector type (or lack of it). > In the end, the physical connector on the side of laptop is USB-C. - Even if the connector is USB-C, the protocol used for display is still DP. There's also the case of Thunderbolt. - This is inconsistent with existing drivers. i915 and amdgpu expose DP ports for their USB-C ports. Changing that isn't possible without causing user-space regressions (compositor config files use the connector type).
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thursday, August 3rd, 2023 at 17:22, Simon Ser wrote: > The KMS docs describe "subconnector" to be defined as "downstream port" for > DP. > Can USB-C (or USB) be seen as a DP downstream port? To expand on this a bit: I'm wondering if we're mixing apples and oranges here. The current values of "subconnector" typically describe the lower-level protocol tunneled inside DP. For instance, VGA can be tunneled inside the DP cable when using DP → VGA adapter. However, in the USB-C case, DP itself is tunneled inside USB-C. And you might use a USB-C → DP adapter. So it's not really *sub*connector, it's more of a *super*connector, right? I think [1] is somewhat related, since it also allows user-space to discover whether a connector uses USB-C. But relying on sysfs to figure this out isn't super optimal perhaps. [1]: https://lore.kernel.org/dri-devel/20221108185004.2263578-1-wonch...@google.com/
Re: [Intel-gfx] [PATCH 3/4] drm/uapi: document the USB subconnector type
On Wednesday, August 2nd, 2023 at 21:23, Dmitry Baryshkov
wrote:
> >> >> + { DRM_MODE_SUBCONNECTOR_USB, "USB" }, /* DP */
> >> >
> >> > Should this be DRM_MODE_SUBCONNECTOR_USB_C and "USB-C", in case we get
> >> > another USB type later ?
> >>
> >> Hmm, which id should I use for micro-USB then? (consider anx7808,
> >> SlimPort). I thought about using DRM_MODE_SUBCONNECTOR_USB for both of
> >> them. But maybe I should add another subtype for SlimPort.
> >
> >I suppose it depends on whether userspace needs a way to differentiate
> >those. Do you have a good visibility on the userspace use cases ?
>
> No. I'm not even sure, which userspace handles subtypes properly.
wlroots uses it for human-readable output descriptions, e.g.
> wayland-info
interface: 'wl_output', version: 4, name:
49
name: DP-3
description: Samsung Electric Company SyncMaster HS3P505873 (DP-3 via
DVI-D)
The "via DVI-D" bit comes from subconnector.
The description is displayed to the user when picking an output to screen
capture, among other things. It is helpful to users because they can better
understand why their output connected via DVI shows up as "DP".
The KMS docs describe "subconnector" to be defined as "downstream port" for DP.
Can USB-C (or USB) be seen as a DP downstream port?
Re: [Intel-gfx] [PATCH v6 3/4] drm: Expand max DRM device number to full MINORBITS
On Thursday, July 27th, 2023 at 14:01, Christian König wrote: > > We do need patches to stop trying to infer the node type from the minor > > in libdrm, though. Emil has suggested using sysfs, which we already do > > in a few places in libdrm. > > That sounds like a really good idea to me as well. > > But what do we do with DRM_MAX_MINOR? Change it or keep it and say apps > should use drmGetDevices2() like Emil suggested? DRM_MAX_MINOR has been bumped to 64 now. With the new minor allocation scheme, DRM_MAX_MINOR is meaningless because there is no "max minor per type" concept anymore: the minor no longer contains the type. So I'd suggest leaving it as-is (so that old apps still continue to work on systems with < 64 devices like they do today) and mark it as deprecated.
Re: [Intel-gfx] [PATCH v6 3/4] drm: Expand max DRM device number to full MINORBITS
On Monday, July 24th, 2023 at 23:14, Michał Winiarski wrote: > Having a limit of 64 DRM devices is not good enough for modern world > where we have multi-GPU servers, SR-IOV virtual functions and virtual > devices used for testing. > Let's utilize full minor range for DRM devices. > To avoid regressing the existing userspace, we're still maintaining the > numbering scheme where 0-63 is used for primary, 64-127 is reserved > (formerly for control) and 128-191 is used for render. > For minors >= 192, we're allocating minors dynamically on a first-come, > first-served basis. In general the approach looks good to me. Old libdrm will see the new nodes as nodes with an unknown type when it tries to infer the nod type from the minor, which is as good as it gets. We do need patches to stop trying to infer the node type from the minor in libdrm, though. Emil has suggested using sysfs, which we already do in a few places in libdrm.
Re: [Intel-gfx] [PATCH v2] i915/display/hotplug: use drm_kms_helper_connector_hotplug_event()
Any news about this patch?
[Intel-gfx] [PATCH v2] i915/display/hotplug: use drm_kms_helper_connector_hotplug_event()
This adds more information to the hotplug uevent and lets user-space
know that it's about a particular connector only.
v2: don't rely on the changed HPD pin bitmask to count changed
connectors (Jani)
Signed-off-by: Simon Ser
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: Rodrigo Vivi
Cc: Gustavo Sousa
Cc: Imre Deak
Cc: Lucas De Marchi
---
drivers/gpu/drm/i915/display/intel_hotplug.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 1160fa20433b..0ff5ed46ae1e 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -376,6 +376,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
u32 changed = 0, retry = 0;
u32 hpd_event_bits;
u32 hpd_retry_bits;
+ struct drm_connector *first_changed_connector = NULL;
+ int changed_connectors = 0;
mutex_lock(_priv->drm.mode_config.mutex);
drm_dbg_kms(_priv->drm, "running encoder hotplug functions\n");
@@ -428,6 +430,11 @@ static void i915_hotplug_work_func(struct work_struct
*work)
break;
case INTEL_HOTPLUG_CHANGED:
changed |= hpd_bit;
+ changed_connectors++;
+ if (!first_changed_connector) {
+ drm_connector_get(>base);
+ first_changed_connector =
>base;
+ }
break;
case INTEL_HOTPLUG_RETRY:
retry |= hpd_bit;
@@ -438,9 +445,14 @@ static void i915_hotplug_work_func(struct work_struct
*work)
drm_connector_list_iter_end(_iter);
mutex_unlock(_priv->drm.mode_config.mutex);
- if (changed)
+ if (changed_connectors == 1)
+ drm_kms_helper_connector_hotplug_event(first_changed_connector);
+ else if (changed_connectors > 0)
drm_kms_helper_hotplug_event(_priv->drm);
+ if (first_changed_connector)
+ drm_connector_put(first_changed_connector);
+
/* Remove shared HPD pins that have changed */
retry &= ~changed;
if (retry) {
--
2.41.0
Re: [Intel-gfx] [PATCH] i915/display/hotplug: use drm_kms_helper_connector_hotplug_event()
On Wednesday, June 21st, 2023 at 14:26, Jani Nikula wrote: > On Wed, 21 Jun 2023, Simon Ser cont...@emersion.fr wrote: > > > On Wednesday, June 21st, 2023 at 14:11, Jani Nikula jani.nik...@intel.com > > wrote: > > > > > On Wed, 21 Jun 2023, Simon Ser cont...@emersion.fr wrote: > > > > > > > On Wednesday, June 21st, 2023 at 14:05, Jani Nikula > > > > jani.nik...@intel.com wrote: > > > > > > > > > > - if (changed) > > > > > > + if (hweight32(changed) == 1) > > > > > > + drm_kms_helper_connector_hotplug_event(first_changed_connector); > > > > > > > > > > What if more than one connector share the same hpd pin? > > > > > > > > Ah, I did not believe this could happen. I'll rework the patch to > > > > count the number of changed connectors instead. > > > > > > A DP++ port is probably the prime example of this, with both DP and HDMI > > > connectors. > > > > Wouldn't that be handled by the separate DP logic though? (The > > drm_dp_mst family of helpers.) > > It's not DP MST, though. See intel_ddi_init() initializing both DP and > HDMI. One encoder, one HPD pin, two connectors. Thanks for the explanation!
Re: [Intel-gfx] [PATCH] i915/display/hotplug: use drm_kms_helper_connector_hotplug_event()
On Wednesday, June 21st, 2023 at 14:11, Jani Nikula wrote: > On Wed, 21 Jun 2023, Simon Ser cont...@emersion.fr wrote: > > > On Wednesday, June 21st, 2023 at 14:05, Jani Nikula jani.nik...@intel.com > > wrote: > > > > > > - if (changed) > > > > + if (hweight32(changed) == 1) > > > > + drm_kms_helper_connector_hotplug_event(first_changed_connector); > > > > > > What if more than one connector share the same hpd pin? > > > > Ah, I did not believe this could happen. I'll rework the patch to > > count the number of changed connectors instead. > > A DP++ port is probably the prime example of this, with both DP and HDMI > connectors. Wouldn't that be handled by the separate DP logic though? (The drm_dp_mst family of helpers.)
Re: [Intel-gfx] [PATCH] i915/display/hotplug: use drm_kms_helper_connector_hotplug_event()
On Wednesday, June 21st, 2023 at 14:05, Jani Nikula wrote: > > - if (changed) > > + if (hweight32(changed) == 1) > > + drm_kms_helper_connector_hotplug_event(first_changed_connector); > > What if more than one connector share the same hpd pin? Ah, I did not believe this could happen. I'll rework the patch to count the number of changed connectors instead.
[Intel-gfx] [PATCH] i915/display/hotplug: use drm_kms_helper_connector_hotplug_event()
This adds more information to the hotplug uevent and lets user-space
know that it's about a particular connector only.
Signed-off-by: Simon Ser
Cc: Jani Nikula
Cc: Ville Syrjälä
Cc: Rodrigo Vivi
Cc: Gustavo Sousa
Cc: Imre Deak
Cc: Lucas De Marchi
---
drivers/gpu/drm/i915/display/intel_hotplug.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 1160fa20433b..605c6e05a169 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -376,6 +376,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
u32 changed = 0, retry = 0;
u32 hpd_event_bits;
u32 hpd_retry_bits;
+ struct drm_connector *first_changed_connector = NULL;
mutex_lock(_priv->drm.mode_config.mutex);
drm_dbg_kms(_priv->drm, "running encoder hotplug functions\n");
@@ -428,6 +429,10 @@ static void i915_hotplug_work_func(struct work_struct
*work)
break;
case INTEL_HOTPLUG_CHANGED:
changed |= hpd_bit;
+ if (!first_changed_connector) {
+ drm_connector_get(>base);
+ first_changed_connector =
>base;
+ }
break;
case INTEL_HOTPLUG_RETRY:
retry |= hpd_bit;
@@ -438,9 +443,14 @@ static void i915_hotplug_work_func(struct work_struct
*work)
drm_connector_list_iter_end(_iter);
mutex_unlock(_priv->drm.mode_config.mutex);
- if (changed)
+ if (hweight32(changed) == 1)
+ drm_kms_helper_connector_hotplug_event(first_changed_connector);
+ else if (changed)
drm_kms_helper_hotplug_event(_priv->drm);
+ if (first_changed_connector)
+ drm_connector_put(first_changed_connector);
+
/* Remove shared HPD pins that have changed */
retry &= ~changed;
if (retry) {
--
2.41.0
Re: [Intel-gfx] [PATCH] drm/connector: document enum drm_connector_tv_mode DRM_MODE_TV_MODE_MAX
Reviewed-by: Simon Ser
Re: [Intel-gfx] [PATCH 1/6] drm/uapi: Document CTM matrix better
Acked-by: Simon Ser
Re: [Intel-gfx] [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
On Wednesday, January 25th, 2023 at 21:04, Thomas Zimmermann wrote: > Not having connectors indicates a driver bug. Is it? What if all connectors are of the DP-MST type, ie. they are created on-the-fly?
Re: [Intel-gfx] [PATCH 3/5] drm/i915: Remove redundant framebuffer format check
The Intel counterpart is also: Reviewed-by: Simon Ser
Re: [Intel-gfx] [PATCH 2/5] drm/amdgpu: Remove redundant framebuffer format check
Reviewed-by: Simon Ser
[Intel-gfx] [PATCH 2/2] drm/i915/dp_mst: don't pull unregistered connectors into state
In intel_dp_mst_atomic_master_trans_check(), we pull connectors sharing the same DP-MST stream into the atomic state. However, if the connector is unregistered, this later fails with: [ 559.425658] i915 :00:02.0: [drm:drm_atomic_helper_check_modeset] [CONNECTOR:378:DP-7] is not registered Skip these unregistered connectors to allow user-space to turn them off. Fixes part of this wlroots issue: https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3407 Signed-off-by: Simon Ser Cc: Ville Syrjälä Cc: Jani Nikula Cc: Lyude Paul --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index f773e117ebc4..70859a927a9d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -280,7 +280,8 @@ intel_dp_mst_atomic_master_trans_check(struct intel_connector *connector, struct intel_crtc *crtc; if (connector_iter->mst_port != connector->mst_port || - connector_iter == connector) + connector_iter == connector || + connector_iter->base.registration_state == DRM_CONNECTOR_UNREGISTERED) continue; conn_iter_state = intel_atomic_get_digital_connector_state(state, -- 2.39.0
[Intel-gfx] [PATCH 1/2] drm/i915/dp_mst: log when pulling CRTCs into atomic state
It can be surprising for user-space to see unrelated connectors,
CRTCs and planes being implicitly pulled into the atomic commit.
Log when that happens.
Signed-off-by: Simon Ser
Cc: Ville Syrjälä
Cc: Jani Nikula
Cc: Lyude Paul
---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 4077a979a924..f773e117ebc4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -293,6 +293,10 @@ intel_dp_mst_atomic_master_trans_check(struct
intel_connector *connector,
if (!conn_iter_state->base.crtc)
continue;
+ drm_dbg_kms(_priv->drm,
+ "Adding [CONNECTOR:%d:%s] which shares the same
DP-MST stream\n",
+ connector_iter->base.base.id,
connector_iter->base.name);
+
crtc = to_intel_crtc(conn_iter_state->base.crtc);
crtc_state = intel_atomic_get_crtc_state(>base, crtc);
if (IS_ERR(crtc_state)) {
--
2.39.0
Re: [Intel-gfx] [PATCH v2 15/16] drm/edid: add [CONNECTOR:%d:%s] to debug logging
Reviewed-by: Simon Ser
Re: [Intel-gfx] [PATCH 1/3] drm/amd/display: Fix merge conflict resolution in amdgpu_dm_plane.c
Acked-by: Simon Ser
CC amd-gfx
On Monday, August 1st, 2022 at 15:52, Imre Deak wrote:
> The API change introduced in
>
> commit 30c637151cfa ("drm/plane-helper: Export individual helpers")
>
> was missed in the conflict resolution of
>
> commit d93a13bd75b9 ("Merge remote-tracking branch 'drm-misc/drm-misc-next'
> into drm-tip")
>
> fix this up.
>
> Fixes: d93a13bd75b9 ("Merge remote-tracking branch 'drm-misc/drm-misc-next'
> into drm-tip")
> Cc: Simon Ser cont...@emersion.fr
>
> Cc: Thomas Zimmermann tzimmerm...@suse.de
>
> Acked-by: Thomas Zimmermann tzimmerm...@suse.de
>
> Signed-off-by: Imre Deak imre.d...@intel.com
>
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 8cd25b2ea0dca..5eb5d31e591de 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1562,7 +1562,7 @@ int dm_drm_plane_get_property(struct drm_plane *plane,
> static const struct drm_plane_funcs dm_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> - .destroy = drm_primary_helper_destroy,
> + .destroy = drm_plane_helper_destroy,
> .reset = dm_drm_plane_reset,
> .atomic_duplicate_state = dm_drm_plane_duplicate_state,
> .atomic_destroy_state = dm_drm_plane_destroy_state,
> --
> 2.37.1
Re: [Intel-gfx] [v8 3/5] drm/edid: read HF-EEODB ext block
On Wednesday, March 23rd, 2022 at 13:02, Jani Nikula wrote: > Simon and Daniel also tell me on IRC we can't just modify the base block > extension count to match HF-EEODB to dodge the problem, because the EDID > gets exposed to userspace. I'm not familiar how the EDID blob gets exposed to user-space. If the EDID data gets copied when creating the blob, and the blob is created before the kernel mutates the EDID to accomodate for HF-EEODB, then this proposal might still be workable.
Re: [Intel-gfx] [PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting
On Friday, February 18th, 2022 at 12:54, Hans de Goede wrote: > On 2/18/22 12:39, Simon Ser wrote: > > On Friday, February 18th, 2022 at 11:38, Hans de Goede > > wrote: > > > >> What I'm reading in the above is that it is being considered to allow > >> changing the panel-orientation value after the connector has been made > >> available to userspace; and let userspace know about this through a uevent. > >> > >> I believe that this is a bad idea, it is important to keep in mind here > >> what userspace (e.g. plymouth) uses this prorty for. This property is > >> used to rotate the image being rendered / shown on the framebuffer to > >> adjust for the panel orientation. > >> > >> So now lets assume we apply the correct upside-down orientation later > >> on a device with an upside-down mounted LCD panel. Then on boot the > >> following could happen: > >> > >> 1. amdgpu exports a connector for the LCD panel to userspace without > >> setting panel-orient=upside-down > >> 2. plymouth sees this and renders its splash normally, but since the > >> panel is upside-down it will now actually show upside-down > > > > At this point amdgpu hasn't probed the connector yet. So the connector > > will be marked as disconnected, and plymouth shouldn't render anything. > > If before the initial probe of the connector there is a /dev/dri/card0 > which plymouth can access, then plymouth may at this point decide > to disable any seemingly unused crtcs, which will make the screen go black... > > I'm not sure if plymouth will actually do this, but AFAICT this would > not be invalid behavior for a userspace kms consumer to do and I > believe it is likely that mutter will disable unused crtcs. > > IMHO it is just a bad idea to register /dev/dri/card0 with userspace > before the initial connector probe is done. Nothing good can come > of that. > > If all the exposed connectors initially are going to show up as > disconnected anyways what is the value in registering /dev/dri/card0 > with userspace early ? OK. I'm still unsure how I feel about this, but I think I agree with you. That said, the amdgpu architecture is quite involved with multiple abstraction levels, so I don't think I'm equipped to write a patch to fix this... cc Daniel Vetter: can you confirm probing all connectors is a good thing to do on driver module load? > >> I guess the initial modeline is inherited from the video-bios, but > >> what about the physical size? Note that you cannot just change the > >> physical size later either, that gets used to determine the hidpi > >> scaling factor in the bootsplash, and changing that after the initial > >> bootsplash dislay will also look ugly > >> > >> b) Why you need the edid for the panel-orientation property at all, > >> typically the edid prom is part of the panel and the panel does not > >> know that it is mounted e.g. upside down at all, that is a property > >> of the system as a whole not of the panel as a standalone unit so > >> in my experience getting panel-orient info is something which comes > >> from the firmware /video-bios not from edid ? > > > > This is an internal DRM thing. The orientation quirks logic uses the > > mode size advertised by the EDID. > > The DMI based quirking does, yes. But e.g. the quirk code directly > reading this from the Intel VBT does not rely on the mode. > > But if you are planning on using a DMI based quirk for the steamdeck > then yes that needs the mode. > > Thee mode check is there for 2 reasons: > > 1. To avoid also applying the quirk to external displays, but > I think that that is also solved in most drivers by only checking for > a quirk at all on the eDP connector > > 2. Some laptop models ship with different panels in different badges > some of these are portrait (so need a panel-orient) setting and others > are landscape. That makes sense. So yeah the EDID mode based matching logic needs to stay to accomodate for these cases. > > I agree that at least in the Steam > > Deck case it may not make a lot of sense to use any info from the > > EDID, but that's needed for the current status quo. > > We could extend the DMI quirk mechanism to allow quirks which don't > do the mode check, for use on devices where we can guarantee neither > 1 nor 2 happens, then amdgpu could call the quirk code early simply > passing 0x0 as resolution. Yeah. But per the above amdgpu should maybe probe connectors on module load. If/when amdgpu is fixed to do this, then we don't need to disable the mode matching logic in panel-orientation quirks anymore.
Re: [Intel-gfx] [PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting
On Friday, February 18th, 2022 at 11:38, Hans de Goede wrote: > What I'm reading in the above is that it is being considered to allow > changing the panel-orientation value after the connector has been made > available to userspace; and let userspace know about this through a uevent. > > I believe that this is a bad idea, it is important to keep in mind here > what userspace (e.g. plymouth) uses this prorty for. This property is > used to rotate the image being rendered / shown on the framebuffer to > adjust for the panel orientation. > > So now lets assume we apply the correct upside-down orientation later > on a device with an upside-down mounted LCD panel. Then on boot the > following could happen: > > 1. amdgpu exports a connector for the LCD panel to userspace without > setting panel-orient=upside-down > 2. plymouth sees this and renders its splash normally, but since the > panel is upside-down it will now actually show upside-down At this point amdgpu hasn't probed the connector yet. So the connector will be marked as disconnected, and plymouth shouldn't render anything. > 3. amdgpu adjusts the panel-orient prop to upside-down, sends out > uevents That's when amdgpu marks the connector as connected. So everything should be fine I believe, no bad frame. > 4. Lets assume plymouth handles this well (i) and now adjust its > rendering and renders the next frame of the bootsplash 180° rotated > to compensate for the panel being upside down. Then from now on > the user will see the splash normally > > So this means that the user will briefly see the bootsplash rendered > upside down which IMHO is not acceptable behavior. Also see my footnote > about how I seriously doubt plymouth will see the panel-orient change > at all. > > I'm also a bit unsure about: > > a) How you can register the panel connector with userspace before > reading the edid, don't you need the edid to give the physical size + > modeline to userspace, which you cannot just leave out ? Yup. The KMS EDID property is created before the EDID is read, and is set to zero (NULL blob). The width/height in mm and other info are also zero. You can try inspecting the state printed by drm_info on any disconnected connector to see for yourself. > I guess the initial modeline is inherited from the video-bios, but > what about the physical size? Note that you cannot just change the > physical size later either, that gets used to determine the hidpi > scaling factor in the bootsplash, and changing that after the initial > bootsplash dislay will also look ugly > > b) Why you need the edid for the panel-orientation property at all, > typically the edid prom is part of the panel and the panel does not > know that it is mounted e.g. upside down at all, that is a property > of the system as a whole not of the panel as a standalone unit so > in my experience getting panel-orient info is something which comes > from the firmware /video-bios not from edid ? This is an internal DRM thing. The orientation quirks logic uses the mode size advertised by the EDID. I agree that at least in the Steam Deck case it may not make a lot of sense to use any info from the EDID, but that's needed for the current status quo. Also note, DisplayID has a bit to indicate the panel orientation IIRC. Would be nice to support parsing this at some point.
Re: [Intel-gfx] [PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting
On Tuesday, February 15th, 2022 at 15:38, Emil Velikov wrote: > On Tue, 15 Feb 2022 at 13:55, Simon Ser wrote: > > > > On Tuesday, February 15th, 2022 at 13:04, Emil Velikov > > wrote: > > > > > Greetings everyone, > > > > > > Padron for joining in so late o/ > > > > > > On Tue, 8 Feb 2022 at 08:42, Hsin-Yi Wang wrote: > > > > > > > > drm_dev_register() sets connector->registration_state to > > > > DRM_CONNECTOR_REGISTERED and dev->registered to true. If > > > > drm_connector_set_panel_orientation() is first called after > > > > drm_dev_register(), it will fail several checks and results in following > > > > warning. > > > > > > > > Add a function to create panel orientation property and set default > > > > value > > > > to UNKNOWN, so drivers can call this function to init the property > > > > earlier > > > > , and let the panel set the real value later. > > > > > > > > > > The warning illustrates a genuine race condition, where userspace will > > > read the old/invalid property value/state. So this patch masks away > > > the WARNING without addressing the actual issue. > > > Instead can we fix the respective drivers, so that no properties are > > > created after drm_dev_register()? > > > > > > Longer version: > > > As we look into drm_dev_register() it's in charge of creating the > > > dev/sysfs nodes (et al). Note that connectors cannot disappear at > > > runtime. > > > For panel orientation, we are creating an immutable connector > > > properly, meaning that as soon as drm_dev_register() is called we must > > > ensure that the property is available (if applicable) and set to the > > > correct value. > > > > Unfortunately we can't quite do this. To apply the panel orientation quirks > > we > > need to grab the EDID of the eDP connector, and this happened too late in my > > testing. > > > > What we can do is create the prop early during module load, and update it > > when > > we read the EDID (at the place where we create it right now). User-space > > will > > receive a hotplug event after the EDID is read, so will be able to pick up > > the > > new value if any. > > Didn't quite get that, are you saying that a GETPROPERTY for the EDID, > the ioctl blocks or that we get an empty EDID? I'm not referring to GETPROPERTY, I'm referring to the driver getting the EDID from the sink (here, the eDP panel). In my experimentations with amdgpu I noticed that the driver module load finished before the EDID was available to the driver. Maybe other drivers behave differently and probe connectors when loaded, not sure. > The EDID hotplug even thing is neat - sounds like it also signals on > panel orientation, correct? > On such an event, which properties userspace should be re-fetching - > everything or guess randomly? > > Looking through the documentation, I cannot see a clear answer :-\ User-space should re-fetch *all* properties. In practice some user-space may only be fetching some properties, but that should get fixed in user-space. Also the kernel can indicate that only a single connector changed via the "CONNECTOR" uevent prop, or even a single connector property via "PROPERTY". See [1] for a user-space implementation. But all of this is purely an optional optimization. Re-fetching all properties is a bit slower (especially if some drmModeGetConnector calls force-probe connectors) but works perfectly fine. It would be nice to document, if you have the time feel free to send a patch and CC danvet, pq and me. [1]: https://gitlab.freedesktop.org/wlroots/wlroots/-/blob/252b2348bd62170d97c4e81fb2050f757b56d67e/backend/session/session.c#L144
Re: [Intel-gfx] [PATCH v8 1/3] gpu: drm: separate panel orientation property creating and value setting
On Tuesday, February 15th, 2022 at 13:04, Emil Velikov wrote: > Greetings everyone, > > Padron for joining in so late o/ > > On Tue, 8 Feb 2022 at 08:42, Hsin-Yi Wang wrote: > > > > drm_dev_register() sets connector->registration_state to > > DRM_CONNECTOR_REGISTERED and dev->registered to true. If > > drm_connector_set_panel_orientation() is first called after > > drm_dev_register(), it will fail several checks and results in following > > warning. > > > > Add a function to create panel orientation property and set default value > > to UNKNOWN, so drivers can call this function to init the property earlier > > , and let the panel set the real value later. > > > > The warning illustrates a genuine race condition, where userspace will > read the old/invalid property value/state. So this patch masks away > the WARNING without addressing the actual issue. > Instead can we fix the respective drivers, so that no properties are > created after drm_dev_register()? > > Longer version: > As we look into drm_dev_register() it's in charge of creating the > dev/sysfs nodes (et al). Note that connectors cannot disappear at > runtime. > For panel orientation, we are creating an immutable connector > properly, meaning that as soon as drm_dev_register() is called we must > ensure that the property is available (if applicable) and set to the > correct value. Unfortunately we can't quite do this. To apply the panel orientation quirks we need to grab the EDID of the eDP connector, and this happened too late in my testing. What we can do is create the prop early during module load, and update it when we read the EDID (at the place where we create it right now). User-space will receive a hotplug event after the EDID is read, so will be able to pick up the new value if any.
Re: [Intel-gfx] [PATCH v3 2/6] drm/plane: Fix typo in format_mod_supported documentation
On Friday, January 7th, 2022 at 18:26, José Expósito wrote: > Is there something that needs to improve in the other 4 patches? > Or just waiting on maintainers input? Yeah, these patches look good to me. Feel free to add my R-b. Maintainers for these drivers still need to review/ack/apply them.
Re: [Intel-gfx] [PATCH v3 2/6] drm/plane: Fix typo in format_mod_supported documentation
Pushed patches 1 & 2 to drm-misc-next. Thanks for your contribution!
Re: [Intel-gfx] [PATCH v2 1/6] drm/plane: Make format_mod_supported truly optional
On Thursday, December 23rd, 2021 at 12:56, Ville Syrjälä
wrote:
> > - /* If we can't determine support, just bail */
> > - if (!plane->funcs->format_mod_supported)
> > - goto done;
> > -
> > mod = modifiers_ptr(blob_data);
> > for (i = 0; i < plane->modifier_count; i++) {
> > for (j = 0; j < plane->format_count; j++) {
> > - if (plane->funcs->format_mod_supported(plane,
> > + if (!plane->funcs->format_mod_supported ||
> > + plane->funcs->format_mod_supported(plane,
> >
> > plane->format_types[j],
> >
> > plane->modifiers[i])) {
>
> So instead of skipping the whole loop you just skip doing anything
> inside the loop? Can't see how that achieves anything at all.
No, the check is skipped when the function isn't populated by the driver.
Re: [Intel-gfx] [PATCH v2 1/6] drm/plane: Make format_mod_supported truly optional
On Wednesday, December 22nd, 2021 at 10:05, José Expósito wrote: > Make "create_in_format_blob" behave as documented. LGTM, nice! Reviewed-by: Simon Ser CC Ville just in case
Re: [Intel-gfx] [PATCH v2 2/6] drm/plane: Fix typo in format_mod_supported documentation
Reviewed-by: Simon Ser
Re: [Intel-gfx] [PATCH v10 08/10] dyndbg: add print-to-tracefs, selftest with it - RFC
First off, let me reiterate that this feature would be invaluable as user-space
developers. It's often pretty difficult to figure out the cause of an EINVAL,
we have to ask users to follow complicated instructions [1] to grab DRM logs.
Then have to skim through several megabytes of logs to find the error.
I have a hack [2] which just calls system("sudo dmesg") after a failed atomic
commit, it's been pretty handy. But it's really just a hack, a proper solution
would be awesome.
[1]: https://gitlab.freedesktop.org/wlroots/wlroots/-/wikis/DRM-Debugging
[2]: https://gitlab.freedesktop.org/emersion/libliftoff/-/merge_requests/61
> > > > Having a subsystem specific trace buffer would allow subsystem specific
> > > > trace log permissions depending on the sensitivity of the data. But
> > > > doesn't drm output today go to the system log which is typically world
> > > > readable today?
dmesg isn't world-readable these days, it's been changed recently-ish (last
year?) at least on my distribution (Arch). I need root to grab dmesg.
(Maybe we can we just let the DRM master process grab the logs?)
> > > Yes, and that is exactly the problem. The DRM debug output is so high
> > > traffic it would make the system log both unusable due to cruft and
> > > slow down the whole machine. The debug output is only useful when
> > > something went wrong, and at that point it is too late to enable
> > > debugging. That's why a flight recorder with an over-written circular
> > > in-memory buffer is needed.
> >
> > Seans patch reuses enum drm_debug_category to split the tracing
> > stream into 10 sub-streams
> > - how much traffic from each ?
> > - are some sub-streams more valuable for post-mortem ?
> > - any value from further refinement of categories ?
> > - drop irrelevant callsites individually to reduce clutter, extend
> > buffer time/space ?
>
> I think it's hard to predict which sub-streams you are going to need
> before you have a bug to debug. Hence I would err on the side of
> enabling too much. This also means that better or more refined
> categorisation might not be that much of help - or if it is, then are
> the excluded debug messages worth having in the kernel to begin with.
> Well, we're probably not that interested in GPU debugs but just
> everything related to the KMS side, which on the existing categories
> is... everything except half of CORE and DRIVER, maybe? Not sure.
We've been recommending drm.debug=0x19F so far (see wiki linked above).
KMS + PRIME + ATOMIC + LEASE is definitely something we want in, and
CORE + DRIVER contains other useful info. We definitely don't want VBL.
> My feeling is that that could mean in the order of hundreds of log
> events at framerate (e.g. 60 times per second) per each enabled output
> individually. And per DRM device, of course. This is with the
> uninteresting GPU debugs already excluded.
Indeed, successful KMS atomic commits already generate a lot of noise. On my
machine, setting drm.debug=0x19F and running the following command:
sudo dmesg -w | pv >/dev/null
I get 400KiB/s when idling, and 850KiB/s when wiggling the cursor.
> Still, I don't think the flight recorder buffer would need to be
> massive. I suspect it would be enough to hold a few frames' worth which
> is just a split second under active operation. When something happens,
> the userspace stack is likely going to stop on its tracks immediately
> to collect the debug information, which means the flooding should pause
> and the relevant messages don't get overwritten before we get them. In
> a multi-seat system where each device is controlled by a separate
> display server instance, per-device logs would help with this. OTOH,
> multi-seat is not a very common use case I suppose.
There's also the case of multi-GPU where GPU B's logs could clutter GPU A's,
making it harder to understand the cause of an atomic commit failure on GPU A.
So per-device logs would be useful, but not a hard requirement for me, having
*anything* at all would already be a big win.
In my experiments linked above [2], system("sudo dmesg") after atomic commit
failure worked pretty well, and the bottom of the log contained the cause of
the failure. It was pretty useful to system("sudo dmesg -C") before performing
an atomic commit, to be able to only collect the extract of the log relevant to
the atomic commit.
Having some kind of "marker" mechanism could be pretty cool. "Mark" the log
stream before performing an atomic commit (ideally that'd just return e.g. an
uint64 offset), then on failure request the logs collected after that mark.
Re: [Intel-gfx] [PATCH v2 14/17] uapi/drm/dg2: Format modifier for DG2 unified compression and clear color
For the include/uapi/drm/drm_fourcc.h changes: Acked-by: Simon Ser
Re: [Intel-gfx] [RFC v2 01/22] drm: RFC for Plane Color Hardware Pipeline
On Tuesday, October 12th, 2021 at 12:30, Pekka Paalanen wrote: > is there a practise of landing proposal documents in the kernel? How > does that work, will a kernel tree carry the patch files? > Or should this document be worded like documentation for an accepted > feature, and then the patches either land or don't? Once everyone agrees, the RFC can land. I don't think a kernel tree is necessary. See: https://dri.freedesktop.org/docs/drm/gpu/rfc/index.html
Re: [Intel-gfx] [PATCH v2 1/7] drm/sysfs: introduce drm_sysfs_connector_hotplug_event
Ping
Re: [Intel-gfx] [PATCH v2 0/7] drm: add per-connector hotplug events
Ping, anyone up for a review?
Re: [Intel-gfx] [PATCH v2 7/7] drm/connector: add ref to drm_connector_get in iter docs
Pushed this one doc patch with Daniel's R-b on IRC.
Re: [Intel-gfx] [PATCH] include/uapi/drm: fix spelling mistakes in header files
Reviewed-by: Simon Ser But this this touches a lot of different drivers, not sure we can just merge this via drm-misc-next? In any case, please ping me again in a week if this hasn't been merged by then. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen wrote: > yes, I think this makes sense, even if it is a property that one can't > tell for sure what it does before hand. > > Using a pair of properties, preference and active, to ask for something > and then check what actually worked is good for reducing the > combinatorial explosion caused by needing to "atomic TEST_ONLY commit" > test different KMS configurations. Userspace has a better chance of > finding a configuration that is possible. > > OTOH, this has the problem than in UI one cannot tell the user in > advance which options are truly possible. Given that KMS properties are > rarely completely independent, and in this case known to depend on > several other KMS properties, I think it is good enough to know after > the fact. > > If a driver does not use what userspace prefers, there is no way to > understand why, or what else to change to make it happen. That problem > exists anyway, because TEST_ONLY commits do not give useful feedback > but only a yes/no. By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing one property at a time), user-space can discover which property makes the atomic commit fail. I'm not a fan of this "preference" property approach. The only way to find out whether it's possible to change the color format is to perform a user-visible change (with a regular atomic commit) and check whether it worked after-the-fact. This is unlike all other existing KMS properties. I'd much rather see a more general approach to fix this combinatorial explosion than to add special-cases like this. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 09/17] drm/uAPI: Add "active color range" drm property as feedback for userspace
On Tuesday, June 22nd, 2021 at 11:50, Werner Sembach wrote: > Unknown is when no monitor is connected or is when the > connector/monitor is disabled. I think the other connector props (link-status, non-desktop, etc) don't have a special "unset" value, and instead the value is set to a random enum entry. User-space should ignore the prop on these disconnected connectors anyways. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/7] drm/probe-helper: add drm_kms_helper_connector_hotplug_event
This function is the same as drm_kms_helper_hotplug_event, but takes
a connector instead of a device.
Signed-off-by: Simon Ser
---
drivers/gpu/drm/drm_probe_helper.c | 23 +++
include/drm/drm_probe_helper.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index e7e1ee2aa352..8cc673267cba 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -604,6 +604,9 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
*
* This function must be called from process context with no mode
* setting locks held.
+ *
+ * If only a single connector has changed, consider calling
+ * drm_kms_helper_connector_hotplug_event() instead.
*/
void drm_kms_helper_hotplug_event(struct drm_device *dev)
{
@@ -616,6 +619,26 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
+/**
+ * drm_kms_helper_connector_hotplug_event - fire off a KMS connector hotplug
event
+ * @connector: drm_connector which has changed
+ *
+ * This is the same as drm_kms_helper_hotplug_event(), except it fires a more
+ * fine-grained uevent for a single connector.
+ */
+void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+
+ /* send a uevent + call fbdev */
+ drm_sysfs_connector_hotplug_event(connector);
+ if (dev->mode_config.funcs->output_poll_changed)
+ dev->mode_config.funcs->output_poll_changed(dev);
+
+ drm_client_dev_hotplug(dev);
+}
+EXPORT_SYMBOL(drm_kms_helper_connector_hotplug_event);
+
static void output_poll_execute(struct work_struct *work)
{
struct delayed_work *delayed_work = to_delayed_work(work);
diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h
index 8d3ed2834d34..733147ea89be 100644
--- a/include/drm/drm_probe_helper.h
+++ b/include/drm/drm_probe_helper.h
@@ -19,6 +19,7 @@ void drm_kms_helper_poll_init(struct drm_device *dev);
void drm_kms_helper_poll_fini(struct drm_device *dev);
bool drm_helper_hpd_irq_event(struct drm_device *dev);
void drm_kms_helper_hotplug_event(struct drm_device *dev);
+void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector);
void drm_kms_helper_poll_disable(struct drm_device *dev);
void drm_kms_helper_poll_enable(struct drm_device *dev);
--
2.31.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 6/7] i915/display/dp: send a more fine-grained link-status uevent
When link-status changes, send a hotplug uevent which contains the connector and property ID. That way, user-space can more easily figure out that only the link-status property of this connector has been updated. Signed-off-by: Simon Ser --- drivers/gpu/drm/i915/display/intel_dp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 5c983044..0ce44a97dd14 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5276,6 +5276,8 @@ static void intel_dp_modeset_retry_work_fn(struct work_struct *work) mutex_unlock(>dev->mode_config.mutex); /* Send Hotplug uevent so userspace can reprobe */ drm_kms_helper_hotplug_event(connector->dev); + drm_sysfs_connector_status_event(connector, + connector->dev->mode_config.link_status_property); } bool -- 2.31.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 7/7] drm/connector: add ref to drm_connector_get in iter docs
Mention that connectors need to be referenced manually if they are
to be accessed after the iteration has progressed or ended.
Signed-off-by: Simon Ser
---
include/drm/drm_connector.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 714d1a01c065..c1af1e4ca560 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1735,6 +1735,11 @@ void drm_mode_put_tile_group(struct drm_device *dev,
* drm_connector_list_iter_begin(), drm_connector_list_iter_end() and
* drm_connector_list_iter_next() respectively the convenience macro
* drm_for_each_connector_iter().
+ *
+ * Note that the return value of drm_connector_list_iter_next() is only valid
+ * up to the next drm_connector_list_iter_next() or
+ * drm_connector_list_iter_end() call. If you want to use the connector later,
+ * then you need to grab your own reference first using drm_connector_get().
*/
struct drm_connector_list_iter {
/* private: */
--
2.31.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 5/7] drm/probe-helper: use drm_kms_helper_connector_hotplug_event
If an hotplug event only updates a single connector, use
drm_kms_helper_connector_hotplug_event instead of
drm_kms_helper_hotplug_event.
Signed-off-by: Simon Ser
---
drivers/gpu/drm/drm_probe_helper.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index 8cc673267cba..f4130c1a90e2 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -843,7 +843,7 @@ EXPORT_SYMBOL(drm_kms_helper_poll_fini);
*/
bool drm_helper_hpd_irq_event(struct drm_device *dev)
{
- struct drm_connector *connector;
+ struct drm_connector *connector, *changed_connector = NULL;
struct drm_connector_list_iter conn_iter;
enum drm_connector_status old_status;
bool changed = false;
@@ -883,16 +883,27 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
* Check if epoch counter had changed, meaning that we need
* to send a uevent.
*/
- if (old_epoch_counter != connector->epoch_counter)
+ if (old_epoch_counter != connector->epoch_counter) {
+ if (changed) {
+ if (changed_connector)
+ drm_connector_put(changed_connector);
+ changed_connector = NULL;
+ } else {
+ drm_connector_get(connector);
+ changed_connector = connector;
+ }
changed = true;
+ }
}
drm_connector_list_iter_end(_iter);
mutex_unlock(>mode_config.mutex);
- if (changed) {
+ if (changed_connector) {
+ drm_kms_helper_connector_hotplug_event(changed_connector);
+ drm_connector_put(changed_connector);
+ } else if (changed) {
drm_kms_helper_hotplug_event(dev);
- DRM_DEBUG_KMS("Sent hotplug event\n");
}
return changed;
--
2.31.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/7] amdgpu: use drm_kms_helper_connector_hotplug_event
When updating a single connector, use
drm_kms_helper_connector_hotplug_event instead of
drm_kms_helper_hotplug_event.
Signed-off-by: Simon Ser
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 4 ++--
2 files changed, 6 insertions(+), 6 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 3267eb2e35dd..4b91534ff324 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2638,7 +2638,7 @@ static void handle_hpd_irq(void *param)
drm_modeset_unlock_all(dev);
if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_connector_hotplug_event(connector);
} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
if (new_connection_type == dc_connection_none &&
@@ -2652,7 +2652,7 @@ static void handle_hpd_irq(void *param)
drm_modeset_unlock_all(dev);
if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_connector_hotplug_event(connector);
}
mutex_unlock(>hpd_lock);
@@ -2805,7 +2805,7 @@ static void handle_hpd_rx_irq(void *param)
dm_restore_drm_connector_state(dev, connector);
drm_modeset_unlock_all(dev);
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_connector_hotplug_event(connector);
} else if (dc_link_detect(dc_link, DETECT_REASON_HPDRX)) {
if (aconnector->fake_enable)
@@ -2818,7 +2818,7 @@ static void handle_hpd_rx_irq(void *param)
dm_restore_drm_connector_state(dev, connector);
drm_modeset_unlock_all(dev);
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_connector_hotplug_event(connector);
}
}
#ifdef CONFIG_DRM_AMD_DC_HDCP
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 9fbbd0159119..221242b6e528 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1200,7 +1200,7 @@ static ssize_t trigger_hotplug(struct file *f, const char
__user *buf,
dm_restore_drm_connector_state(dev, connector);
drm_modeset_unlock_all(dev);
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_connector_hotplug_event(connector);
} else if (param[0] == 0) {
if (!aconnector->dc_link)
goto unlock;
@@ -1222,7 +1222,7 @@ static ssize_t trigger_hotplug(struct file *f, const char
__user *buf,
dm_restore_drm_connector_state(dev, connector);
drm_modeset_unlock_all(dev);
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_connector_hotplug_event(connector);
}
unlock:
--
2.31.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/7] drm/connector: use drm_sysfs_connector_hotplug_event
In drm_connector_register, use drm_sysfs_connector_hotplug_event instead of drm_sysfs_hotplug_event, because the hotplug event only updates a single connector. Signed-off-by: Simon Ser --- drivers/gpu/drm/drm_connector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index da39e7ff6965..76930e0b8949 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -530,7 +530,7 @@ int drm_connector_register(struct drm_connector *connector) connector->registration_state = DRM_CONNECTOR_REGISTERED; /* Let userspace know we have a new connector */ - drm_sysfs_hotplug_event(connector->dev); + drm_sysfs_connector_hotplug_event(connector); goto unlock; -- 2.31.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/7] drm/sysfs: introduce drm_sysfs_connector_hotplug_event
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);
+
/**
* drm_sysfs_connector_status_event - generate a DRM uevent for connector
* property status change
diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
index d454ef617b2c..6273cac44e47 100644
--- a/include/drm/drm_sysfs.h
+++ b/include/drm/drm_sysfs.h
@@ -11,6 +11,7 @@ int drm_class_device_register(struct device *dev);
void drm_class_device_unregister(struct device *dev);
void drm_sysfs_hotplug_event(struct drm_device *dev);
+void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
void drm_sysfs_connector_status_event(struct drm_connector *connector,
struct drm_property *property);
#endif
--
2.31.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/7] drm: add per-connector hotplug events
When a uevent only updates a single connector, add a CONNECTOR property to the uevent. This allows user-space to ignore other connectors when handling the uevent. This is purely an optimization, drivers can still send a uevent without the CONNECTOR property. The CONNECTOR property is already set when sending HDCP property update uevents, see drm_sysfs_connector_status_event. This has been tested with a wlroots patch [1]. amdgpu and the probe-helper has been updated to use these new fine-grained uevents. [1]: https://github.com/swaywm/wlroots/pull/2959 Simon Ser (7): drm/sysfs: introduce drm_sysfs_connector_hotplug_event drm/probe-helper: add drm_kms_helper_connector_hotplug_event drm/connector: use drm_sysfs_connector_hotplug_event amdgpu: use drm_kms_helper_connector_hotplug_event drm/probe-helper: use drm_kms_helper_connector_hotplug_event i915/display/dp: send a more fine-grained link-status uevent drm/connector: add ref to drm_connector_get in iter docs .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++-- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 4 +- drivers/gpu/drm/drm_connector.c | 2 +- drivers/gpu/drm/drm_probe_helper.c| 42 +-- drivers/gpu/drm/drm_sysfs.c | 25 +++ drivers/gpu/drm/i915/display/intel_dp.c | 2 + include/drm/drm_connector.h | 5 +++ include/drm/drm_probe_helper.h| 1 + include/drm/drm_sysfs.h | 1 + 9 files changed, 79 insertions(+), 11 deletions(-) -- 2.31.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] dma-buf: Add an API for exporting sync files (v11)
On Thursday, May 27th, 2021 at 12:33 PM, Daniel Vetter wrote: > > The sync_file is also import/exportable to a certain drm_syncobj timeline > > point (or as binary signal). So no big deal, we are all compatible here :) > > I just thought that it might be more appropriate to return a drm_syncobj > > directly instead of a sync_file. > > I think another big potential user for this is a vk-based compositor which > needs to interact/support implicit synced clients. And compositor world I > think is right now still more sync_file (because that's where we started > with atomic kms ioctl). Yeah, right now compositors can only deal with sync_file. I have a Vulkan pull request for wlroots [1] that would benefit from this, I think. Also it seems like drm_syncobj isn't necessarily going to be the the sync primitive that ends them all with all that user-space fence discussion, so long-term I guess we'll need a conversion anyways. [1]: https://github.com/swaywm/wlroots/pull/2771 > Plus you can convert them to the other form anyway, so really doesn't > matter much. But for the above reasons I'm leaning slightly towards > sync_file. Except if compositor folks tell me I'm a fool and why :-) sync_file is fine from my PoV. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/7] Per client engine busyness
On Monday, May 17th, 2021 at 8:16 PM, Nieto, David M wrote: > Btw is DRM_MAJOR 226 consider uapi? I don't see it in uapi headers. It's not in the headers, but it's de facto uAPI, as seen in libdrm: > git grep 226 xf86drm.c 99:#define DRM_MAJOR 226 /* Linux */ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] New uAPI for color management proposal and feedback request
On Wednesday, May 12th, 2021 at 3:04 PM, Ville Syrjälä wrote: > > Adoption: > > > > A KDE dev wants to implement the settings in the KDE settings GUI: > > > > https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_912370 > > > > Tuxedo Computers (my employer) wants to implement the settings desktop > > environment agnostic in Tuxedo Control Center. I > > will start work on this in parallel to implementing the new kernel code. > > I suspect everyone would be happier to accept new uapi if we had > multiple compositors signed up to implement it. Sign me up. We already have a patch blocked by "Broadcast RGB" standardization: https://github.com/swaywm/wlroots/pull/2310 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/modifiers: Enforce consistency between the cap an IN_FORMATS
Continuing on that idea to push for enabling the cap in more cases: do we have a policy to require new drivers to always support modifiers? That would be nice, even if it's just about enabling LINEAR. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/modifiers: Enforce consistency between the cap an IN_FORMATS
Reviewed-by: Simon Ser ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/9] drm: Add privacy-screen class and connector properties
On Thursday, April 22nd, 2021 at 10:54 AM, Hans de Goede wrote: > I guess Marco was waiting for the kernel bits too land before submitting > these, > but I agree that it would probably be good to have these submitted now, we > can mark them as WIP to avoid them getting merged before the kernel side > is finalized. Yes, this is how it should be done, see [1]. In particular: > The userspace side must be fully reviewed and tested to the standards > of that userspace project. And yeah, the user-space side still can't be merged before the kernel side: > The kernel patch can only be merged after all the above requirements > are met, but it must be merged to either drm-next or drm-misc-next > before the userspace patches land. [1]: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/9] drm: Add privacy-screen class and connector properties
Hi, On Wednesday, April 21st, 2021 at 10:47 PM, Hans de Goede wrote: > There now is GNOME userspace code using the new properties: > https://hackmd.io/@3v1n0/rkyIy3BOw Thanks for working on this. Can these patches be submitted as merge requests against the upstream projects? It would be nice to get some feedback from the maintainers, and be able to easily leave some comments there as well. Thanks, Simon ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/modifiers: Enforce consistency between the cap an IN_FORMATS
Reviewed-by: Simon Ser ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/12] drm/modifiers: Enforce consistency between the cap an IN_FORMATS
On Tuesday, April 13th, 2021 at 11:49 AM, Daniel Vetter wrote: > + * Note that userspace can check the DRM_CAP_ADDFB2_MODIFIERS driver Prepend an ampersand like so and a hyperlink will be rendered: Note that userspace can check the _CAP_ADDFB2_MODIFIERS driver ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/doc: Add RFC section
Acked-by: Simon Ser ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/doc: Add RFC section
Side note: I feel like we could have better lines of communication between kernel devs and user-space devs. The new uAPI requirements seem to be a high barrier to entry for kernel devs. However sometimes user-space devs might be interested in helping out with the user-space part… Maybe it would be good to CC e.g. wayland-devel for new RFCs, so that user-space devs can jump in if they're interested. And also provide feedback, since new uAPI is hard to spot in the sea of messages in dri-devel. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] kcmp: Support selection of SYS_kcmp without CHECKPOINT_RESTORE
On Friday, February 12th, 2021 at 1:57 PM, Emil Velikov wrote: > On Fri, 5 Feb 2021 at 22:01, Chris Wilson wrote: > > > > Userspace has discovered the functionality offered by SYS_kcmp and has > > started to depend upon it. In particular, Mesa uses SYS_kcmp for > > os_same_file_description() in order to identify when two fd (e.g. device > > or dmabuf) > > As you rightfully point out, SYS_kcmp is a bit of a two edged sword. > While you mention the CONFIG issue, there is also a portability aspect > (mesa runs on more than just linux) and as well as sandbox filtering > of the extra syscall. > > Last time I looked, the latter was still an issue and mesa was using > SYS_kcmp to compare device node fds. > A far shorter and more portable solution is possible, so let me > prepare a Mesa patch. Comparing two DMA-BUFs can be done with their inode number, I think. Comparing two device FDs is more subtle, because of GEM handle ref'counting. You sometimes really want to check whether two FDs are backed by the same file *description*. See [1] for details. [1]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY"
On Wednesday, February 10th, 2021 at 2:16 PM, Daniel Vetter wrote: > On Tue, Feb 09, 2021 at 04:14:01PM -0800, Manasi Navare wrote: > > > These additional checks added to avoid EBUSY give unnecessary WARN_ON > > in case of big joiner used in i915 in which case even if the modeset > > is requested on a single pipe, internally another consecutive > > pipe is stolen and used to drive half of the transcoder timings. > > So in this case it is expected that requested crtc and affected crtcs > > do not match. Hence the added WARN ON becomes irrelevant. The WARN_ON only happens if allow_modeset == false. If allow_modeset == true, then the driver is allowed to steal an unrelated pipe. Maybe i915 is stealing a pipe without allow_modeset? > Nope. We can maybe rework this so that i915 can do stuff under the hood, > but wrt uapi this was the thing we discussed with compositors. Without > such a guarantee atomic is defacto broken from a compositor pov. Agreed. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 1/6] drm/damage_helper: Check if damage clips has valid values
> Userspace can set a damage clip with a negative coordinate, negative > width or height or larger than the plane. > This invalid values could cause issues in some HW or even worst enable > security flaws. > > v2: > - add debug messages to let userspace know why atomic commit failed > due invalid damage clips > > Cc: Simon Ser > Cc: Gwan-gyeong Mun > Cc: Sean Paul > Cc: Fabio Estevam > Cc: Deepak Rawat > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: José Roberto de Souza After looking at the kernel code, it seems like the kernel already checks for all of that in drm_atomic_plane_check. Are you aware of this? > + w = drm_rect_width(_state->src) >> 16; > + h = drm_rect_height(_state->src) >> 16; The docs say this should be in FB coordinates, not in SRC_* coordinates. So we shouldn't need to check any SRC_* prop here. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/damage_helper: Check if damage clips has valid values
Can you add some drm_dbg_atomic logs when the damage is invalid, to make it easier for user-space to understand why an atomic commit failed? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [drm-tip:drm-tip 1117/1129] drivers/gpu/drm/drm_atomic_uapi.c:342 drm_atomic_set_crtc_for_connector() error: we previously assumed 'crtc' could be null (see line 326)
Already fixed in 0003b687ee6d ("drm: fix oops in
drm_atomic_set_crtc_for_connector"). Thanks.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Include fb modidier in state dumps
Thanks! Acked-by: Simon Ser ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Don't create the IN_FORMATS blob when the driver does not provide .format_mod_supported()
On Friday, October 23, 2020 10:39 PM, Ville Syrjala wrote: > From: Ville Syrjälä ville.syrj...@linux.intel.com > > The code responsible for creating the IN_FORMATS > blob is broken when the driver doesn't provide a > .format_mod_supported() hook. It just copies in > the format list, but leaves all the modifier information > zeroed. That would indicate (in a very silly way) that > there are in fact no supported format+modifier combinations. > That is utter nonsense. Should we WARN_ON when the driver enables allow_fb_modifiers but doesn't populate format_mod_supported? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm_modes: signed integer overflow
On Friday, October 23, 2020 5:27 PM, Ville Syrjälä wrote: > These are two extreme cases: Thanks! > > I'm trying to get > > a fix for my user-space 1, and I'm wondering if int32_t is enough > > after dividing by mode->htotal. > > CC Pekka, just FYI (I think Weston has similar code). > > What's with those 100LL constants? Are you storing > clock in Hz units? We're storing the vertical refresh rate in mHz (milli-Hertz). ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm_modes: signed integer overflow
On Thursday, October 22, 2020 12:14 PM, Ville Syrjälä wrote: > On Wed, Oct 21, 2020 at 08:13:43PM -0700, Randy Dunlap wrote: > > > Hi, > > With linux-next 20201021, when booting up, I am seeing this: > > [ 0.560896] UBSAN: signed-integer-overflow in > > ../drivers/gpu/drm/drm_modes.c:765:20 > > [ 0.560903] 2376000 * 1000 cannot be represented in type 'int' > > Dang. Didn't realize these new crazy >8k modes have dotclocks reaching > almost 6 GHz, which would overflow even u32. I guess we'll switch to > 64bit maths. Now I wonder how many other places can hit this overflow > in practice... Can you provide an example of a full crazy >8k mode? I'm trying to get a fix for my user-space [1], and I'm wondering if int32_t is enough after dividing by mode->htotal. CC Pekka, just FYI (I think Weston has similar code). [1]: https://github.com/swaywm/wlroots/pull/2450 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 1/4] drm: Introduce plane and CRTC scaling filter properties
For the docs: Acked-by: Simon Ser ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 0/5] Introduce drm scaling filter property
On Tuesday, October 13, 2020 4:26 PM, Simon Ser wrote: > On Monday, October 12, 2020 8:41 PM, Pankaj Bharadiya > pankaj.laxminarayan.bharad...@intel.com wrote: > > > Now, Sameer has implemented Integer scaling in Kodi Retro gaming > > framework which demonstrate how Integer scaling gives distinctive > > look to pixel art games when played on higher resolution monitors. > > Kodi patches are reviewed and accepted for merge now. > > Here is the userspace patch series link: > > https://github.com/xbmc/xbmc/pull/18194 > > As a side note, these user-space patches hard-code the kernel enum > values [1]. This is something which we discussed some time ago [2], > the result of the discussion is that user-space shouldn't do that. Sameer has submitted a pull request [1] to fix this. Thanks, this looks good to me from a uAPI usage point-of-view! [1]: https://github.com/xbmc/xbmc/pull/18567 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 0/5] Introduce drm scaling filter property
On Monday, October 12, 2020 8:41 PM, Pankaj Bharadiya wrote: > Now, Sameer has implemented Integer scaling in Kodi Retro gaming > framework which demonstrate how Integer scaling gives distinctive > look to pixel art games when played on higher resolution monitors. > > Kodi patches are reviewed and accepted for merge now. > > Here is the userspace patch series link: > https://github.com/xbmc/xbmc/pull/18194 As a side note, these user-space patches hard-code the kernel enum values [1]. This is something which we discussed some time ago [2], the result of the discussion is that user-space shouldn't do that. [1]: https://github.com/xbmc/xbmc/pull/18194/files#diff-94967b31726326769b31635c3dd7fc9b50d003057b49306a136b6b702795dd96R30 [2]: https://lists.freedesktop.org/archives/dri-devel/2020-April/261055.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 1/5] drm: Introduce plane and CRTC scaling filter properties
On Monday, October 12, 2020 8:41 PM, Pankaj Bharadiya wrote: > +/** > + * DOC: CRTC scaling filter property > + * > + * SCALING_FILTER: > + * > + * Indicates scaling filter to be used for CRTC scaler > + * > + * The value of this property can be one of the following: > + * Default: > + * Driver's default scaling filter > + * Nearest Neighbor: > + * Nearest Neighbor scaling filter > + * > + * Drivers can set up this property for a CRTC by calling > + * drm_crtc_create_scaling_filter_property > + */ Can we put this under the existing "Standard CRTC Properties" doc comment instead? I'd prefer to avoid having a lot of different headings for properties. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 1/5] drm: Introduce plane and CRTC scaling filter properties
> +/** > + * DOC: Plane scaling filter property > + * > + * SCALING_FILTER: > + * > + * Indicates scaling filter to be used for plane scaler > + * > + * The value of this property can be one of the following: > + * Default: > + * Driver's default scaling filter > + * Nearest Neighbor: > + * Nearest Neighbor scaling filter > + * > + * Drivers can set up this property for a plane by calling > + * drm_plane_create_scaling_filter_property > + */ Similarily, this can be moved in "Plane Composition Properties". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 i-g-t 2/4] kms_writeback: Add initial writeback tests
> > + igt_output_set_prop_value(output, > > IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR, (uint64_t)out_fence_ptr); > > On ARM (32bits), this cast creates a compilation error since the > pointer size isn't an uint64_t. Does casting to uintptr_t before uint64_t fix it? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 0/5] Introduce drm scaling filter property
Hi, On Monday, March 30, 2020 8:38 PM, Pankaj Bharadiya wrote: > Userspace patch series link: https://github.com/lrusak/xbmc/pull/24 This pull request is against a fork, not the official Kodi repository. Are there any plans to upstream the change so that users can benefit from it? Is there a reason why this pull request hasn't been opened against the official repo? Thanks, Simon ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/5] drm/drm-kms.rst: Add plane and CRTC scaling filter property documentation
On Monday, March 30, 2020 8:38 PM, Pankaj Bharadiya wrote: > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > index 906771e03103..b0335e9d887c 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -509,6 +509,18 @@ Variable Refresh Properties > .. kernel-doc:: drivers/gpu/drm/drm_connector.c > :doc: Variable refresh properties > > +Plane Scaling Filter Property > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_plane.c > > - :doc: Plane scaling filter property > - > > +CRTC Scaling Filter Property > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_crtc.c > > - :doc: CRTC scaling filter property > - > This adds whole new sections just for the scaling filter property. Shouldn't we use the existing "Plane properties" section defined in drm_blend.c for plane props? (Same for CRTC props.) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm core/helpers and MIT license
Adding zeis...@freebsd.org for FreeBSD. I'll try to see if I can ping some NetBSD/OpenBSD folks too. On Tuesday, November 12, 2019 4:03 PM, Daniel Vetter wrote: > Hi all, > > Dave and me chatted about this last week on irc. Essentially we have: > > $ git grep SPDX.*GPL -- ':(glob)drivers/gpu/drm/c' > drivers/gpu/drm/drm_client.c:// SPDX-License-Identifier: GPL-2.0 > drivers/gpu/drm/drm_damage_helper.c:// SPDX-License-Identifier: GPL-2.0 OR MIT > drivers/gpu/drm/drm_dp_cec.c:// SPDX-License-Identifier: GPL-2.0 > drivers/gpu/drm/drm_edid_load.c:// SPDX-License-Identifier: GPL-2.0-or-later > drivers/gpu/drm/drm_fb_cma_helper.c:// SPDX-License-Identifier: > GPL-2.0-or-later > drivers/gpu/drm/drm_format_helper.c:/ SPDX-License-Identifier: GPL-2.0 > */drivers/gpu/drm/drm_gem_cma_helper.c:// SPDX-License-Identifier: > GPL-2.0-or-later > drivers/gpu/drm/drm_gem_framebuffer_helper.c:// > SPDX-License-Identifier: GPL-2.0-or-later > drivers/gpu/drm/drm_gem_shmem_helper.c:// SPDX-License-Identifier: GPL-2.0 > drivers/gpu/drm/drm_gem_ttm_helper.c:// SPDX-License-Identifier: > GPL-2.0-or-later > drivers/gpu/drm/drm_gem_vram_helper.c:// SPDX-License-Identifier: > GPL-2.0-or-later > drivers/gpu/drm/drm_hdcp.c:// SPDX-License-Identifier: GPL-2.0 > drivers/gpu/drm/drm_lease.c:// SPDX-License-Identifier: GPL-2.0-or-later > drivers/gpu/drm/drm_mipi_dbi.c:// SPDX-License-Identifier: GPL-2.0-or-later > drivers/gpu/drm/drm_of.c:// SPDX-License-Identifier: GPL-2.0-only > drivers/gpu/drm/drm_simple_kms_helper.c:// SPDX-License-Identifier: > GPL-2.0-or-later > drivers/gpu/drm/drm_sysfs.c:// SPDX-License-Identifier: GPL-2.0-only > drivers/gpu/drm/drm_vma_manager.c:// SPDX-License-Identifier: GPL-2.0 OR MIT > drivers/gpu/drm/drm_vram_helper_common.c:// SPDX-License-Identifier: > GPL-2.0-or-later > drivers/gpu/drm/drm_writeback.c:// SPDX-License-Identifier: GPL-2.0 > > One is GPL+MIT, so ok, and one is a default GPL-only header from > Greg's infamous patch (so could probably be changed to MIT license > header). I only looked at .c sources, since headers are worse wrt > having questionable default headers. So about 18 files with clear GPL > licenses thus far in drm core/helpers. > > Looking at where that code came from, it is mostly from GPL-only > drivers (we have a lot of those nowadays), so seems legit non-MIT > licensed. Question is now what do we do: > > - Nothing, which means GPL will slowly encroach on drm core/helpers, > which is roughly the same as ... > > - Throw in the towel on MIT drm core officially. Same as above, except > lets just make it official. > > - Try to counter this, which means at least a) relicensing a bunch of > stuff b) rewriting a bunch of stuff c) making sure that's ok with > everyone, there's a lot of GPL-by-default for the kernel (that's how > we got most of the above code through merged drivers I think). I > suspect that whomever cares will need to put in the work to make this > happen (since it will need a pile of active resistance at least). > > Cc maintainers/driver teams who might care most about this. > > Also if people could cc *bsd, they probably care and I don't know best > contacts for graphics stuff (or anything else really at all). > > Cheers, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: two planes with the same zpos have undefined ordering
Currently the property docs don't specify whether it's okay for two planes to have the same zpos value and what user-space should expect in this case. The rule mentionned in the past was to disambiguate with object IDs. However some drivers break this rule (that's why the ordering is documented as unspecified in case the zpos property is missing). Additionally it doesn't really make sense for a driver to user identical zpos values if it knows their relative position: the driver can just pick different values instead. So two solutions would make sense: either disallow completely identical zpos values for two different planes, either make the ordering unspecified. To allow drivers that don't know the relative ordering between two planes to still expose the zpos property, choose the latter solution. Signed-off-by: Simon Ser --- drivers/gpu/drm/drm_blend.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@ * planes. Without this property the primary plane is always below the cursor * plane, and ordering between all other planes is undefined. The positive * Z axis points towards the user, i.e. planes with lower Z position values - * are underneath planes with higher Z position values. Note that the Z - * position value can also be immutable, to inform userspace about the - * hard-coded stacking of overlay planes, see - * drm_plane_create_zpos_immutable_property(). + * are underneath planes with higher Z position values. Two planes with the + * same Z position value have undefined ordering. Note that the Z position + * value can also be immutable, to inform userspace about the hard-coded + * stacking of overlay planes, see drm_plane_create_zpos_immutable_property(). * * pixel blend mode: * Pixel blend mode is set up with drm_plane_create_blend_mode_property(). -- 2.23.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: add Type-C port to encoder name
This patch adds the Type-C port number to the encoder name. This is an alternative to [1]. [1]: https://patchwork.freedesktop.org/series/65695/ Signed-off-by: Simon Ser Cc: Imre Deak Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_ddi.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 8eb2b3ec01ed..e543d44a7105 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -4278,6 +4278,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) bool init_hdmi, init_dp, init_lspcon = false; enum pipe pipe; enum phy phy = intel_port_to_phy(dev_priv, port); + enum tc_port tc_port; + char tc_suffix[128]; init_hdmi = port_info->supports_dvi || port_info->supports_hdmi; init_dp = port_info->supports_dp; @@ -4307,8 +4309,15 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) intel_encoder = _dig_port->base; encoder = _encoder->base; + tc_port = intel_port_to_tc(dev_priv, port); + if (tc_port != PORT_TC_NONE) + snprintf(tc_suffix, sizeof(tc_suffix), "/TC#%d", tc_port + 1); + else + tc_suffix[0] = '\0'; + drm_encoder_init(_priv->drm, encoder, _ddi_funcs, -DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port)); +DRM_MODE_ENCODER_TMDS, "DDI %c%s", port_name(port), +tc_suffix); intel_encoder->hotplug = intel_ddi_hotplug; intel_encoder->compute_output_type = intel_ddi_compute_output_type; -- 2.23.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: add port info to debugfs
This patch adds a line with the port name to connectors in
debugfs/i915_debug_info. If the port is Type-C, the Type-C port number is
printed too.
Signed-off-by: Simon Ser
Cc: Imre Deak
Cc: Manasi Navare
Reviewed-by: Imre Deak
---
Resending v2 to the correct mailing list.
drivers/gpu/drm/i915/i915_debugfs.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index b39226d7f8d2..4ba508c954f8 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2557,9 +2557,11 @@ static void intel_lvds_info(struct seq_file *m,
static void intel_connector_info(struct seq_file *m,
struct drm_connector *connector)
{
+ struct drm_i915_private *i915 = to_i915(connector->dev);
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_encoder *intel_encoder = intel_connector->encoder;
struct drm_display_mode *mode;
+ enum tc_port tc_port;
seq_printf(m, "connector %d: type %s, status: %s\n",
connector->base.id, connector->name,
@@ -2578,6 +2580,14 @@ static void intel_connector_info(struct seq_file *m,
if (!intel_encoder)
return;
+ if (intel_encoder->port != PORT_NONE) {
+ seq_printf(m, "\tport: %c", port_name(intel_encoder->port));
+ tc_port = intel_port_to_tc(i915, intel_encoder->port);
+ if (tc_port != PORT_TC_NONE)
+ seq_printf(m, "/TC#%d", tc_port + 1);
+ seq_printf(m, "\n");
+ }
+
switch (connector->connector_type) {
case DRM_MODE_CONNECTOR_DisplayPort:
case DRM_MODE_CONNECTOR_eDP:
--
2.22.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: add immutable zpos plane properties
On Thursday, April 18, 2019 8:20 AM, Simon Ser wrote: > On Wednesday, April 17, 2019 11:35 PM, Simon Ser cont...@emersion.fr wrote: > > > In terms of graphs, if a plane is a node and a two-plane overlap is an > > edge, it means we want a complete graph (each node has an edge to all > > other nodes). If we only have square planes, it's already impossible to > > get a solution for 4 planes (the graph contains an edge that crosses > > another edge [1]). > > The following is only true if all planes have an identical size (the > transformation from planes to the graph is lossy otherwise). It's > possible to choose different plane sizes and still be able to find a > solution without an alpha channel for 4 planes [1]. Thanks Léo Andrès > for pointing this out! > > Will investigate a little bit more, to see if I can find a general > formula for n planes. > > [1]: https://www.zapashcanon.fr/~leo/lohi2.png Discussed with Ville, and this solution won't work as older hardware doesn't have an alpha channel for overlay planes. Discussed with Martin, and the most reasonable solution seems to keep it simple and draw as many frames as there are planes. Frame i would show plane i as fullscreen and all planes but plane i as small squares on top or under it. This simple approach makes it easier to debug and still uses just a few frames. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: add immutable zpos plane properties
On Wednesday, April 17, 2019 11:35 PM, Simon Ser wrote: > In terms of graphs, if a plane is a node and a two-plane overlap is an > edge, it means we want a complete graph (each node has an edge to all > other nodes). If we only have square planes, it's already impossible to > get a solution for 4 planes (the graph contains an edge that crosses > another edge [1]). The following is only true if all planes have an identical size (the transformation from planes to the graph is lossy otherwise). It's possible to choose different plane sizes and still be able to find a solution without an alpha channel for 4 planes [1]. Thanks Léo Andrès for pointing this out! Will investigate a little bit more, to see if I can find a general formula for n planes. [1]: https://www.zapashcanon.fr/~leo/lohi2.png ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: add immutable zpos plane properties
On Tuesday, April 16, 2019 10:04 PM, Ville Syrjälä wrote: > On Tue, Apr 16, 2019 at 08:13:12PM +0200, Maarten Lankhorst wrote: > > > Op 16-04-2019 om 15:42 schreef Ville Syrjälä: > > > > > On Tue, Apr 16, 2019 at 03:28:15PM +0200, Maarten Lankhorst wrote: > > > > > > > Op 16-04-2019 om 15:20 schreef Ville Syrjälä: > > > > > > > > > On Sat, Apr 13, 2019 at 11:13:27AM +, Simon Ser wrote: > > > > > > > > > > > From: Ville Syrjälä ville.syrj...@linux.intel.com > > > > > > This adds basic immutable support for the zpos property. The zpos > > > > > > increases > > > > > > from bottom to top: primary, sprites, cursor. > > > > > > I was thinking a bit about how we might go about testing this. > > > > > > > > > > We probably want a basic test that just checks that if any > > > > > plane has a zpos prop then all planes should have it. > > > > > This would be a good test for BAT. > > > > > A functional test would stack the planes up in some way and > > > > > compare against a software rendered reference. IIRC there was > > > > > a zpos test case floating around but that depended on alpha > > > > > blending which we don't necessarily have. > > > > > But with semi-overlapping planes you would accomplish the same, > > > > > without alpha dependency. > > > > > > > > Something like this? > > > > [BG] [Sprite 1] [Cursor] > > > > [Primary] [Sprite 2] > > > > Should probably be good enough. Though I was pondering is there a > > > > way to position an arbitraty number of planes such that the > > > > resulting picture has a visible region for every possible > > > > combination of planes? > > > > n planes, width = width / (n + 1) > > position = n * 3/4 * plane_width ? or something > > If each plane has its own color, then it would work.. > > That's not going to hit all the combinations. > > Three is easy, you just position the planes in a triangular sort > of shape. But four already seems hard. Or maybe I'm just not smart > enough. Probably needs some graph theory math proof or something. > > I suppose you could do thatr staircase type approach you suggested, > and them swap the planes around a bit. I think that should cover > everything eventually. But I was hoping for a cool way to check > everything from a single frame :) That's an interesting problem. Goal: an image which contains, for each pair of planes (P1, P2), a region with these two planes overlapping (and only these two planes). In terms of graphs, if a plane is a node and a two-plane overlap is an edge, it means we want a complete graph (each node has an edge to all other nodes). If we only have square planes, it's already impossible to get a solution for 4 planes (the graph contains an edge that crosses another edge [1]). However if we get non-square planes (ie. we have an alpha channel) it becomes possible. If n is the number of planes, I can imagine to draw (n - 1) little squares on each plane (the rest being totally transparent), each of them intersecting with one of the others planes' squares. This would mean we'd have n * (n - 1) little squares total, and n * (n - 1) / 2 couple of squares that intersect (this matches the number of edges of a complete graph). Thoughts? Is this clear enough? Is it acceptable to require an alpha channel? Am I mistaken? [1]: https://en.wikipedia.org/wiki/File:3-simplex_graph.svg ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: add immutable zpos plane properties
From: Ville Syrjälä
This adds basic immutable support for the zpos property. The zpos increases
from bottom to top: primary, sprites, cursor.
Signed-off-by: Ville Syrjälä
[cont...@emersion.fr: adapted for latest drm-tip]
Signed-off-by: Simon Ser
---
Maarten, could your review this patch?
Changes from v2 to v3: add the zpos property in skl_universal_plane_create too.
drivers/gpu/drm/i915/intel_display.c | 10 --
drivers/gpu/drm/i915/intel_sprite.c | 7 ++-
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8576a7f799..f0a85a75bd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14323,7 +14323,7 @@ intel_primary_plane_create(struct drm_i915_private
*dev_priv, enum pipe pipe)
const u64 *modifiers;
const u32 *formats;
int num_formats;
- int ret;
+ int ret, zpos;
if (INTEL_GEN(dev_priv) >= 9)
return skl_universal_plane_create(dev_priv, pipe,
@@ -14412,6 +14412,9 @@ intel_primary_plane_create(struct drm_i915_private
*dev_priv, enum pipe pipe)
DRM_MODE_ROTATE_0,
supported_rotations);
+ zpos = 0;
+ drm_plane_create_zpos_immutable_property(>base, zpos);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return plane;
@@ -14428,7 +14431,7 @@ intel_cursor_plane_create(struct drm_i915_private
*dev_priv,
{
unsigned int possible_crtcs;
struct intel_plane *cursor;
- int ret;
+ int ret, zpos;
cursor = intel_plane_alloc();
if (IS_ERR(cursor))
@@ -14477,6 +14480,9 @@ intel_cursor_plane_create(struct drm_i915_private
*dev_priv,
DRM_MODE_ROTATE_0 |
DRM_MODE_ROTATE_180);
+ zpos = RUNTIME_INFO(dev_priv)->num_sprites[pipe] + 1;
+ drm_plane_create_zpos_immutable_property(>base, zpos);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return cursor;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 65de7387bf..40b7bcd720 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -2333,6 +2333,8 @@ skl_universal_plane_create(struct drm_i915_private
*dev_priv,
BIT(DRM_MODE_BLEND_PREMULTI) |
BIT(DRM_MODE_BLEND_COVERAGE));
+ drm_plane_create_zpos_immutable_property(>base, plane_id);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return plane;
@@ -2354,7 +2356,7 @@ intel_sprite_plane_create(struct drm_i915_private
*dev_priv,
const u64 *modifiers;
const u32 *formats;
int num_formats;
- int ret;
+ int ret, zpos;
if (INTEL_GEN(dev_priv) >= 9)
return skl_universal_plane_create(dev_priv, pipe,
@@ -2444,6 +2446,9 @@ intel_sprite_plane_create(struct drm_i915_private
*dev_priv,
DRM_COLOR_YCBCR_BT709,
DRM_COLOR_YCBCR_LIMITED_RANGE);
+ zpos = sprite + 1;
+ drm_plane_create_zpos_immutable_property(>base, zpos);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return plane;
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: add immutable zpos plane properties
Hi Jani,
git blame says you are familiar with intel_primary_plane_create! Would
you have some time to review this patch?
Thanks!
--
Simon Ser
https://emersion.fr
> From: Ville Syrjälä
>
> This adds basic immutable support for the zpos property. The zpos increases
> from bottom to top: primary, sprites, cursor.
>
> Signed-off-by: Ville Syrjälä
> [cont...@emersion.fr: adapted for latest drm-tip]
> Signed-off-by: Simon Ser
> ---
>
> Changes in v2: set correct author and S-o-b tags
>
> drivers/gpu/drm/i915/intel_display.c | 10 --
> drivers/gpu/drm/i915/intel_sprite.c | 5 -
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 8576a7f799..f0a85a75bd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14323,7 +14323,7 @@ intel_primary_plane_create(struct drm_i915_private
> *dev_priv, enum pipe pipe)
> const u64 *modifiers;
> const u32 *formats;
> int num_formats;
> - int ret;
> + int ret, zpos;
>
> if (INTEL_GEN(dev_priv) >= 9)
> return skl_universal_plane_create(dev_priv, pipe,
> @@ -14412,6 +14412,9 @@ intel_primary_plane_create(struct drm_i915_private
> *dev_priv, enum pipe pipe)
> DRM_MODE_ROTATE_0,
> supported_rotations);
>
> + zpos = 0;
> + drm_plane_create_zpos_immutable_property(>base, zpos);
> +
> drm_plane_helper_add(>base, _plane_helper_funcs);
>
> return plane;
> @@ -14428,7 +14431,7 @@ intel_cursor_plane_create(struct drm_i915_private
> *dev_priv,
> {
> unsigned int possible_crtcs;
> struct intel_plane *cursor;
> - int ret;
> + int ret, zpos;
>
> cursor = intel_plane_alloc();
> if (IS_ERR(cursor))
> @@ -14477,6 +14480,9 @@ intel_cursor_plane_create(struct drm_i915_private
> *dev_priv,
> DRM_MODE_ROTATE_0 |
> DRM_MODE_ROTATE_180);
>
> + zpos = RUNTIME_INFO(dev_priv)->num_sprites[pipe] + 1;
> + drm_plane_create_zpos_immutable_property(>base, zpos);
> +
> drm_plane_helper_add(>base, _plane_helper_funcs);
>
> return cursor;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 65de7387bf..48bd8f9079 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -2354,7 +2354,7 @@ intel_sprite_plane_create(struct drm_i915_private
> *dev_priv,
> const u64 *modifiers;
> const u32 *formats;
> int num_formats;
> - int ret;
> + int ret, zpos;
>
> if (INTEL_GEN(dev_priv) >= 9)
> return skl_universal_plane_create(dev_priv, pipe,
> @@ -2444,6 +2444,9 @@ intel_sprite_plane_create(struct drm_i915_private
> *dev_priv,
> DRM_COLOR_YCBCR_BT709,
> DRM_COLOR_YCBCR_LIMITED_RANGE);
>
> + zpos = sprite + 1;
> + drm_plane_create_zpos_immutable_property(>base, zpos);
> +
> drm_plane_helper_add(>base, _plane_helper_funcs);
>
> return plane;
> --
> 2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: add immutable zpos plane properties
From: Ville Syrjälä
This adds basic immutable support for the zpos property. The zpos increases
from bottom to top: primary, sprites, cursor.
Signed-off-by: Ville Syrjälä
[cont...@emersion.fr: adapted for latest drm-tip]
Signed-off-by: Simon Ser
---
Changes in v2: set correct author and S-o-b tags
drivers/gpu/drm/i915/intel_display.c | 10 --
drivers/gpu/drm/i915/intel_sprite.c | 5 -
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8576a7f799..f0a85a75bd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14323,7 +14323,7 @@ intel_primary_plane_create(struct drm_i915_private
*dev_priv, enum pipe pipe)
const u64 *modifiers;
const u32 *formats;
int num_formats;
- int ret;
+ int ret, zpos;
if (INTEL_GEN(dev_priv) >= 9)
return skl_universal_plane_create(dev_priv, pipe,
@@ -14412,6 +14412,9 @@ intel_primary_plane_create(struct drm_i915_private
*dev_priv, enum pipe pipe)
DRM_MODE_ROTATE_0,
supported_rotations);
+ zpos = 0;
+ drm_plane_create_zpos_immutable_property(>base, zpos);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return plane;
@@ -14428,7 +14431,7 @@ intel_cursor_plane_create(struct drm_i915_private
*dev_priv,
{
unsigned int possible_crtcs;
struct intel_plane *cursor;
- int ret;
+ int ret, zpos;
cursor = intel_plane_alloc();
if (IS_ERR(cursor))
@@ -14477,6 +14480,9 @@ intel_cursor_plane_create(struct drm_i915_private
*dev_priv,
DRM_MODE_ROTATE_0 |
DRM_MODE_ROTATE_180);
+ zpos = RUNTIME_INFO(dev_priv)->num_sprites[pipe] + 1;
+ drm_plane_create_zpos_immutable_property(>base, zpos);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return cursor;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 65de7387bf..48bd8f9079 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -2354,7 +2354,7 @@ intel_sprite_plane_create(struct drm_i915_private
*dev_priv,
const u64 *modifiers;
const u32 *formats;
int num_formats;
- int ret;
+ int ret, zpos;
if (INTEL_GEN(dev_priv) >= 9)
return skl_universal_plane_create(dev_priv, pipe,
@@ -2444,6 +2444,9 @@ intel_sprite_plane_create(struct drm_i915_private
*dev_priv,
DRM_COLOR_YCBCR_BT709,
DRM_COLOR_YCBCR_LIMITED_RANGE);
+ zpos = sprite + 1;
+ drm_plane_create_zpos_immutable_property(>base, zpos);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return plane;
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add immutable zpos plane properties
On Tuesday, April 2, 2019 3:35 PM, Joonas Lahtinen wrote: > Quoting Simon Ser (2019-03-30 00:19:25) > > > From: emersion cont...@emersion.fr > > Please fix your From: field. Gah. > > This adds basic immutable support for the zpos property. The zpos increases > > from bottom to top: primary, sprites, cursor. > > Signed-off-by: Simon Ser cont...@emersion.fr > > This is just Ville's patch rebased, so it's incorrect to strip the S-o-b, > please read: > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin Honestly I wasn't sure what to do, especially considering that I've edited most of the lines. I tried to ask on IRC but I got no reply. Thanks for clearing that up! So just to be sure I don't mess up v2: - This is Ville's patch so he definitely should be the author. - Ville's S-o-b should definitely be kept as-is. - Now, should I add my own S-o-b? It seems like I should. - Co-Authored-By probably doesn't make sense here. - I originally wanted to add my Reviewed-by tag, but it probably wouldn't make sense if I have a S-o-b tag. Is this correct? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: add immutable zpos plane properties
From: emersion
This adds basic immutable support for the zpos property. The zpos increases
from bottom to top: primary, sprites, cursor.
Signed-off-by: Simon Ser
---
This is based on a previous patch by Ville [1] that I wanted to review.
Unfortunately the patch no longer applies, so here is a new one.
[1]: https://patchwork.freedesktop.org/patch/225887/?series=43902=1
drivers/gpu/drm/i915/intel_display.c | 10 --
drivers/gpu/drm/i915/intel_sprite.c | 5 -
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8576a7f799..f0a85a75bd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14323,7 +14323,7 @@ intel_primary_plane_create(struct drm_i915_private
*dev_priv, enum pipe pipe)
const u64 *modifiers;
const u32 *formats;
int num_formats;
- int ret;
+ int ret, zpos;
if (INTEL_GEN(dev_priv) >= 9)
return skl_universal_plane_create(dev_priv, pipe,
@@ -14412,6 +14412,9 @@ intel_primary_plane_create(struct drm_i915_private
*dev_priv, enum pipe pipe)
DRM_MODE_ROTATE_0,
supported_rotations);
+ zpos = 0;
+ drm_plane_create_zpos_immutable_property(>base, zpos);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return plane;
@@ -14428,7 +14431,7 @@ intel_cursor_plane_create(struct drm_i915_private
*dev_priv,
{
unsigned int possible_crtcs;
struct intel_plane *cursor;
- int ret;
+ int ret, zpos;
cursor = intel_plane_alloc();
if (IS_ERR(cursor))
@@ -14477,6 +14480,9 @@ intel_cursor_plane_create(struct drm_i915_private
*dev_priv,
DRM_MODE_ROTATE_0 |
DRM_MODE_ROTATE_180);
+ zpos = RUNTIME_INFO(dev_priv)->num_sprites[pipe] + 1;
+ drm_plane_create_zpos_immutable_property(>base, zpos);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return cursor;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 65de7387bf..48bd8f9079 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -2354,7 +2354,7 @@ intel_sprite_plane_create(struct drm_i915_private
*dev_priv,
const u64 *modifiers;
const u32 *formats;
int num_formats;
- int ret;
+ int ret, zpos;
if (INTEL_GEN(dev_priv) >= 9)
return skl_universal_plane_create(dev_priv, pipe,
@@ -2444,6 +2444,9 @@ intel_sprite_plane_create(struct drm_i915_private
*dev_priv,
DRM_COLOR_YCBCR_BT709,
DRM_COLOR_YCBCR_LIMITED_RANGE);
+ zpos = sprite + 1;
+ drm_plane_create_zpos_immutable_property(>base, zpos);
+
drm_plane_helper_add(>base, _plane_helper_funcs);
return plane;
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
