[Intel-gfx] [PATCH] drm/managed: Cleanup of unused functions and polishing docs
Following functions are only used internally, not by drivers:
- devm_drm_dev_init
Also, now that we have a very slick and polished way to allocate a
drm_device with devm_drm_dev_alloc, update all the docs to reflect the
new reality. Mostly this consists of deleting old and misleading
hints. Two main ones:
- it is no longer required that the drm_device base class is first in
the structure. devm_drm_dev_alloc can cope with it being anywhere
- obviously embedded now strongly recommends using devm_drm_dev_alloc
v2: Fix typos (Noralf)
v3: Split out the removal of drm_dev_init, that's blocked on some
discussions on how to convert vgem/vkms/i915-selftests. Adjust commit
message to reflect that.
Cc: Noralf Trønnes
Acked-by: Noralf Trønnes (v2)
Acked-by: Sam Ravnborg
Cc: Luben Tuikov
Cc: amd-...@lists.freedesktop.org
Signed-off-by: Daniel Vetter
---
.../driver-api/driver-model/devres.rst| 2 +-
drivers/gpu/drm/drm_drv.c | 78 +--
drivers/gpu/drm/drm_managed.c | 2 +-
include/drm/drm_device.h | 2 +-
include/drm/drm_drv.h | 16 ++--
5 files changed, 30 insertions(+), 70 deletions(-)
diff --git a/Documentation/driver-api/driver-model/devres.rst
b/Documentation/driver-api/driver-model/devres.rst
index efc21134..aa4d2420f79e 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -263,7 +263,7 @@ DMA
dmam_pool_destroy()
DRM
- devm_drm_dev_init()
+ devm_drm_dev_alloc()
GPIO
devm_gpiod_get()
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index d4506f7a234e..7c1689842ec0 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -240,13 +240,13 @@ void drm_minor_release(struct drm_minor *minor)
* DOC: driver instance overview
*
* A device instance for a drm driver is represented by &struct drm_device.
This
- * is initialized with drm_dev_init(), usually from bus-specific ->probe()
- * callbacks implemented by the driver. The driver then needs to initialize all
- * the various subsystems for the drm device like memory management, vblank
- * handling, modesetting support and intial output configuration plus obviously
- * initialize all the corresponding hardware bits. Finally when everything is
up
- * and running and ready for userspace the device instance can be published
- * using drm_dev_register().
+ * is allocated and initialized with devm_drm_dev_alloc(), usually from
+ * bus-specific ->probe() callbacks implemented by the driver. The driver then
+ * needs to initialize all the various subsystems for the drm device like
memory
+ * management, vblank handling, modesetting support and initial output
+ * configuration plus obviously initialize all the corresponding hardware bits.
+ * Finally when everything is up and running and ready for userspace the device
+ * instance can be published using drm_dev_register().
*
* There is also deprecated support for initalizing device instances using
* bus-specific helpers and the &drm_driver.load callback. But due to
@@ -274,7 +274,7 @@ void drm_minor_release(struct drm_minor *minor)
*
* The following example shows a typical structure of a DRM display driver.
* The example focus on the probe() function and the other functions that is
- * almost always present and serves as a demonstration of devm_drm_dev_init().
+ * almost always present and serves as a demonstration of devm_drm_dev_alloc().
*
* .. code-block:: c
*
@@ -294,22 +294,12 @@ void drm_minor_release(struct drm_minor *minor)
* struct drm_device *drm;
* int ret;
*
- * // devm_kzalloc() can't be used here because the drm_device '
- * // lifetime can exceed the device lifetime if driver unbind
- * // happens when userspace still has open file descriptors.
- * priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- * if (!priv)
- * return -ENOMEM;
- *
+ * priv = devm_drm_dev_alloc(&pdev->dev, &driver_drm_driver,
+ * struct driver_device, drm);
+ * if (IS_ERR(priv))
+ * return PTR_ERR(priv);
* drm = &priv->drm;
*
- * ret = devm_drm_dev_init(&pdev->dev, drm, &driver_drm_driver);
- * if (ret) {
- * kfree(priv);
- * return ret;
- * }
- * drmm_add_final_kfree(drm, priv);
- *
* ret = drmm_mode_config_init(drm);
* if (ret)
* return ret;
@@ -550,9 +540,9 @@ static void drm_fs_inode_free(struct inode *inode)
* following guidelines apply:
*
* - The entire device initialization procedure should be run from the
- *&component_master_ops.master_bind callback, starting with drm_dev_init(),
- *then binding all components with component_
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/managed: Cleanup of unused functions and polishing docs
== Series Details == Series: drm/managed: Cleanup of unused functions and polishing docs URL : https://patchwork.freedesktop.org/series/81253/ State : warning == Summary == $ dim checkpatch origin/drm-tip aabae0318a42 drm/managed: Cleanup of unused functions and polishing docs -:240: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter ' total: 0 errors, 1 warnings, 0 checks, 174 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST
On 2020-09-01 at 09:57:35 -0400, Sean Paul wrote:
> On Tue, Sep 1, 2020 at 8:22 AM Anshuman Gupta
> wrote:
> >
>
> Hi Anshuman,
> Thank you for sending this along! I have a few comments below.
Thanks sean for your comment.
I have some doubts over ENCRYPT_STATUS_CHANGE_TIMEOUT_MS may be ram can
provide some insight over it.
Thanks,
Anshuman Gupta.
>
> > Gen12 has measure changes with respect to HDCP display
> > engine instaces lies in Trascoder insead of DDI as in Gen11.
>
> *instances
> *transcoder
> *instead
>
> >
> > This requires hdcp driver to use mst_master_transcoder for link
> > authentication and stream trascoder for stream encryption
>
> *transcoder
>
> > separately.
> >
> > It also requires to validate the stream encryption status
> > in HDCP_STATUS_{TRANSCODER,PORT} driving that link register.
> >
> > There is also some changes over existing HDCP 1.4 DP MST Gen11
> > implementation, related to Multistream HDCP Select bit in
> > TRANS_DDI_FUNC_CTL need to be required with respect to B.Spec
> > Documentation.
> >
> > Cc: Ramalingam C
> > Signed-off-by: Anshuman Gupta
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 12 +--
> > drivers/gpu/drm/i915/display/intel_ddi.h | 6 +-
> > .../drm/i915/display/intel_display_types.h| 9 +++
> > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 73 ---
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +-
> > drivers/gpu/drm/i915/display/intel_hdcp.c | 35 ++---
> > drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> > drivers/gpu/drm/i915/display/intel_hdmi.c | 16 ++--
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > 9 files changed, 121 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index a2b7dcf84430..5d6e4fd7bccd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1849,9 +1849,9 @@ void intel_ddi_disable_transcoder_func(const struct
> > intel_crtc_state *crtc_state
> > }
> > }
> >
> > -int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > -enum transcoder cpu_transcoder,
> > -bool enable)
> > +int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
> > + enum transcoder cpu_transcoder,
> > + bool enable, u32 hdcp_mask)
> > {
> > struct drm_device *dev = intel_encoder->base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > @@ -1866,9 +1866,9 @@ int intel_ddi_toggle_hdcp_signalling(struct
> > intel_encoder *intel_encoder,
> >
> > tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > if (enable)
> > - tmp |= TRANS_DDI_HDCP_SIGNALLING;
> > + tmp |= hdcp_mask;
> > else
> > - tmp &= ~TRANS_DDI_HDCP_SIGNALLING;
> > + tmp &= ~hdcp_mask;
> > intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder), tmp);
> > intel_display_power_put(dev_priv, intel_encoder->power_domain,
> > wakeref);
> > return ret;
> > @@ -3967,7 +3967,7 @@ static void intel_enable_ddi(struct
> > intel_atomic_state *state,
> > if (conn_state->content_protection ==
> > DRM_MODE_CONTENT_PROTECTION_DESIRED)
> > intel_hdcp_enable(to_intel_connector(conn_state->connector),
> > - crtc_state->cpu_transcoder,
> > + crtc_state,
> > (u8)conn_state->hdcp_content_type);
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h
> > b/drivers/gpu/drm/i915/display/intel_ddi.h
> > index f5fb62fc9400..69d9e495992c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.h
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.h
> > @@ -43,9 +43,9 @@ void intel_ddi_compute_min_voltage_level(struct
> > drm_i915_private *dev_priv,
> > struct intel_crtc_state
> > *crtc_state);
> > u32 bxt_signal_levels(struct intel_dp *intel_dp);
> > u32 ddi_signal_levels(struct intel_dp *intel_dp);
> > -int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > -enum transcoder cpu_transcoder,
> > -bool enable);
> > +int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
> > + enum transcoder cpu_transcoder,
> > + bool enable, u32 hdcp_mask);
> > void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
> >
> > #endif /* __INTEL_DDI_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 413b60337a0b..dc71ee4d314a 100644
> > -
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/managed: Cleanup of unused functions and polishing docs
== Series Details ==
Series: drm/managed: Cleanup of unused functions and polishing docs
URL : https://patchwork.freedesktop.org/series/81253/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_8953 -> Patchwork_18431
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18431/index.html
Known issues
Here are the changes found in Patchwork_18431 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_module_load@reload:
- fi-bxt-dsi: [PASS][1] -> [DMESG-WARN][2] ([i915#1635] /
[i915#1982])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8953/fi-bxt-dsi/igt@i915_module_l...@reload.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18431/fi-bxt-dsi/igt@i915_module_l...@reload.html
* igt@i915_pm_rpm@module-reload:
- fi-bsw-n3050: [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8953/fi-bsw-n3050/igt@i915_pm_...@module-reload.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18431/fi-bsw-n3050/igt@i915_pm_...@module-reload.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-bsw-kefka: [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8953/fi-bsw-kefka/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18431/fi-bsw-kefka/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html
Possible fixes
* igt@gem_exec_parallel@engines@contexts:
- fi-skl-lmem:[INCOMPLETE][7] ([i915#2398]) -> [PASS][8]
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8953/fi-skl-lmem/igt@gem_exec_parallel@engi...@contexts.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18431/fi-skl-lmem/igt@gem_exec_parallel@engi...@contexts.html
* igt@i915_module_load@reload:
- {fi-ehl-1}: [DMESG-WARN][9] ([i915#1982]) -> [PASS][10]
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8953/fi-ehl-1/igt@i915_module_l...@reload.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18431/fi-ehl-1/igt@i915_module_l...@reload.html
* igt@i915_selftest@live@gem_contexts:
- fi-tgl-u2: [INCOMPLETE][11] ([i915#2045]) -> [PASS][12]
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8953/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18431/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- fi-icl-u2: [DMESG-WARN][13] ([i915#1982]) -> [PASS][14]
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8953/fi-icl-u2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18431/fi-icl-u2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2045]: https://gitlab.freedesktop.org/drm/intel/issues/2045
[i915#2398]: https://gitlab.freedesktop.org/drm/intel/issues/2398
Participating hosts (38 -> 33)
--
Missing(5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan
fi-byt-clapper
Build changes
-
* Linux: CI_DRM_8953 -> Patchwork_18431
CI-20190529: 20190529
CI_DRM_8953: 9a68455dca436ea4f8fc230fdfc4309a9ba65ddb @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_5776: 46e4315096bcaa2465c82c547274627365b1a69e @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18431: aabae0318a423a3af71f0279eceff1598200270c @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
aabae0318a42 drm/managed: Cleanup of unused functions and polishing docs
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18431/index.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] [RFC] drm/kms: Add separate hotplug event call for drm connector
Add separate hotplug event call for drm connector.
Currently from uevent, user space doesn't know
which connector got disconnected when multiple
connectors are connected to DRM.
Signed-off-by: Mohammed Khajapasha
---
drivers/gpu/drm/drm_probe_helper.c | 30 ++
drivers/gpu/drm/drm_sysfs.c| 26 ++
2 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index d6017726cc2a..d9a38569cfe1 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -588,6 +588,36 @@ int drm_helper_probe_single_connector_modes(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
+/**
+ * drm_kms_connector_hotplug_helper - fire off KMS connector hotplug event
+ * @dev: drm_device whose connector state changed
+ * @connector: drm_connector which connected/disconnected
+ *
+ * This function fires off the uevent for userspace and also calls the
+ * output_poll_changed function, which is most commonly used to inform the
fbdev
+ * emulation code and allow it to update the fbcon output configuration.
+ *
+ * Drivers should call this from their hotplug handling code when a change is
+ * detected. Note that this function does not do any output detection of its
+ * own, like drm_helper_hpd_irq_event() does - this is assumed to be done by
the
+ * driver already.
+ *
+ * This function must be called from process context with no mode
+ * setting locks held.
+ *
+ */
+void drm_kms_connector_hotplug_helper(struct drm_device *dev,
+ struct drm_connector *connector)
+{
+ drm_sysfs_connector_hotplug_event(dev, 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_connector_hotplug_helper);
+
/**
* drm_kms_helper_hotplug_event - fire off KMS hotplug events
* @dev: drm_device whose connector state changed
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index f0336c804639..31bd08e1f77d 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -344,6 +344,32 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_sysfs_hotplug_event);
+/**
+ * drm_sysfs_connector_hotplug_event - generate a connector hotplug uevent
+ * @dev: DRM device
+ *
+ * Send a uevent for the DRM device specified by @dev. Currently we only
+ * set HOTPLUG=1 and connector id in the uevent environment, but this could be
+ * expanded to deal with other types of events.
+ *
+ * Any new uapi should be using the drm_sysfs_connector_status_event()
+ * for uevents on connector status change.
+ */
+void drm_sysfs_connector_hotplug_event(struct drm_device *dev,
+ struct drm_connector *connector)
+{
+ char hotplug_str[] = "HOTPLUG=1", conn_id[21];
+ char *envp[] = { hotplug_str, conn_id, NULL };
+
+ snprintf(conn_id, ARRAY_SIZE(conn_id),
+ "CONNECTOR=%u", connector->base.id);
+
+ DRM_DEBUG("generating connector hotplug event\n");
+
+ kobject_uevent_env(&dev->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
--
2.24.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/kms: Add separate hotplug event call for drm connector
== Series Details == Series: drm/kms: Add separate hotplug event call for drm connector URL : https://patchwork.freedesktop.org/series/81257/ State : failure == Summary == CALLscripts/checksyscalls.sh CALLscripts/atomic/check-atomics.sh DESCEND objtool CHK include/generated/compile.h CC drivers/gpu/drm/drm_probe_helper.o drivers/gpu/drm/drm_probe_helper.c: In function ‘drm_kms_connector_hotplug_helper’: drivers/gpu/drm/drm_probe_helper.c:612:2: error: implicit declaration of function ‘drm_sysfs_connector_hotplug_event’; did you mean ‘drm_sysfs_connector_status_event’? [-Werror=implicit-function-declaration] drm_sysfs_connector_hotplug_event(dev, connector); ^ drm_sysfs_connector_status_event cc1: some warnings being treated as errors scripts/Makefile.build:283: recipe for target 'drivers/gpu/drm/drm_probe_helper.o' failed make[3]: *** [drivers/gpu/drm/drm_probe_helper.o] Error 1 scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2 scripts/Makefile.build:500: recipe for target 'drivers/gpu' failed make[1]: *** [drivers/gpu] Error 2 Makefile:1788: recipe for target 'drivers' failed make: *** [drivers] Error 2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID
On Tue, Sep 01, 2020 at 04:18:19PM +0300, Jani Nikula wrote: > On Mon, 31 Aug 2020, Jani Nikula wrote: > > On Mon, 31 Aug 2020, Ville Syrjälä wrote: > >> On Fri, Aug 28, 2020 at 09:19:40AM +0300, Jani Nikula wrote: > >>> The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be > >>> used for the embedded display. Add support for using it via the EDID > >>> override mechanism. > >> > >> Abusing the override for this feels a bit odd. > > > > It's the least intrusive way to make this work across the drm and driver > > EDID code that I could think of. > > > > BR, > > Jani. > > > > > >> > >> Also I have a vague recollection that there was perhaps some > >> linkage between the mailbox and the ACPI _DDC stuff: > >> git://github.com/vsyrjala/linux.git acpi_edid > > Only looked at this now. The patch at hand is for actually overriding > the EDID from the panel, because the panel EDID is readable but bogus. Do we have an actual use case for this? The commit msg doesn't say so. > I > have no idea how the ACPI _DDC stuff would work in this case. Would it > return the EDID from the panel or from mailbox #5 or something > completely different? Who knows. > > Using drm_do_get_edid() would of course be doable for reading mailbox #5 > directly as well, but you'd have to make that the "primary" method and > fall back to the usual drm_get_edid(). Note that this completely > prevents you from ever reading the actual panel EDID. Using > edid_override lets you get at the panel EDID too. Might be nice to make .get_edid() a connector vfunc and let each driver implement it however they want. That way the driver would be in total control over the priority of different EDID sources. But haven't really looked at what that would take. Not even sure if a vfunc is totally required as I think most EDID reads should be in some connector specific driver code. -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK
On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> to separate function") the order of force_min_cdclk_changed check and
> intel_modeset_checks(), was reversed. This broke the mechanism to
> immediately force a new CDCLK minimum, and lead to driver probe
> errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> the issue by moving intel_modeset_checks() call later.
Yep. I eyeed this same code recently and noticed the same bug.
The one thing I didn't yet figure out is whether there is some
subtle ordering requirement that was the reason for the change.
But considering intel_modeset_checks() doesn't really do much
anymore I think it should be safe.
Sadly CI has been lumping all underrun errors under some ancient
bugs, so no one noticed that things started to fail when this
regression was introduced :(
Reviewed-by: Ville Syrjälä
>
> Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to
> separate function)"
> BugLink: https://github.com/thesofproject/linux/issues/2410
> Signed-off-by: Kai Vehmanen
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 7d50b7177d40..8caeed23037c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device *dev,
> if (dev_priv->wm.distrust_bios_wm)
> any_ms = true;
>
> - if (any_ms) {
> - ret = intel_modeset_checks(state);
> - if (ret)
> - goto fail;
> - }
> -
> intel_fbc_choose_crtc(dev_priv, state);
> ret = calc_watermark_data(state);
> if (ret)
> @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device *dev,
> goto fail;
>
> if (any_ms) {
> + ret = intel_modeset_checks(state);
> + if (ret)
> + goto fail;
> +
> ret = intel_modeset_calc_cdclk(state);
> if (ret)
> return ret;
> --
> 2.27.0
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] [RFC] drm/kms: Add separate hotplug event call for drm connector
Add separate hotplug event call for drm connector.
Currently from uevent, user space doesn't know
which connector got disconnected when multiple
connectors are connected to DRM.
Signed-off-by: Mohammed Khajapasha
---
drivers/gpu/drm/drm_probe_helper.c | 30 ++
drivers/gpu/drm/drm_sysfs.c| 24
include/drm/drm_probe_helper.h | 2 ++
include/drm/drm_sysfs.h| 2 ++
4 files changed, 58 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index d6017726cc2a..d9a38569cfe1 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -588,6 +588,36 @@ int drm_helper_probe_single_connector_modes(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
+/**
+ * drm_kms_connector_hotplug_helper - fire off KMS connector hotplug event
+ * @dev: drm_device whose connector state changed
+ * @connector: drm_connector which connected/disconnected
+ *
+ * This function fires off the uevent for userspace and also calls the
+ * output_poll_changed function, which is most commonly used to inform the
fbdev
+ * emulation code and allow it to update the fbcon output configuration.
+ *
+ * Drivers should call this from their hotplug handling code when a change is
+ * detected. Note that this function does not do any output detection of its
+ * own, like drm_helper_hpd_irq_event() does - this is assumed to be done by
the
+ * driver already.
+ *
+ * This function must be called from process context with no mode
+ * setting locks held.
+ *
+ */
+void drm_kms_connector_hotplug_helper(struct drm_device *dev,
+ struct drm_connector *connector)
+{
+ drm_sysfs_connector_hotplug_event(dev, 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_connector_hotplug_helper);
+
/**
* drm_kms_helper_hotplug_event - fire off KMS hotplug events
* @dev: drm_device whose connector state changed
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index f0336c804639..10016a61d468 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -344,6 +344,30 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_sysfs_hotplug_event);
+/**
+ * drm_sysfs_connector_hotplug_event - generate a connector hotplug uevent
+ * @dev: DRM device
+ * @connector: drm_connector which connect/disconnected for hotplug
+ *
+ * Send a uevent for the DRM device specified by @dev. Currently we only
+ * set HOTPLUG=1 and connector id in the uevent environment, but this could be
+ * expanded to deal with other types of events.
+ */
+void drm_sysfs_connector_hotplug_event(struct drm_device *dev,
+ struct drm_connector *connector)
+{
+ char hotplug_str[] = "HOTPLUG=1", conn_id[21];
+ char *envp[] = { hotplug_str, conn_id, NULL };
+
+ snprintf(conn_id, ARRAY_SIZE(conn_id),
+ "CONNECTOR=%u", connector->base.id);
+
+ DRM_DEBUG("generating connector hotplug event\n");
+
+ kobject_uevent_env(&dev->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_probe_helper.h b/include/drm/drm_probe_helper.h
index 8d3ed2834d34..813475adaf01 100644
--- a/include/drm/drm_probe_helper.h
+++ b/include/drm/drm_probe_helper.h
@@ -19,6 +19,8 @@ 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_connector_hotplug_helper(struct drm_device *dev,
+ struct drm_connector *connector);
void drm_kms_helper_poll_disable(struct drm_device *dev);
void drm_kms_helper_poll_enable(struct drm_device *dev);
diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
index d454ef617b2c..1be92541df84 100644
--- a/include/drm/drm_sysfs.h
+++ b/include/drm/drm_sysfs.h
@@ -11,6 +11,8 @@ 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_device *dev,
+ struct drm_connector *connector);
void drm_sysfs_connector_status_event(struct drm_connector *connector,
struct drm_property *property);
#endif
--
2.24.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID
On Wed, 02 Sep 2020, Ville Syrjälä wrote: > On Tue, Sep 01, 2020 at 04:18:19PM +0300, Jani Nikula wrote: >> On Mon, 31 Aug 2020, Jani Nikula wrote: >> > On Mon, 31 Aug 2020, Ville Syrjälä wrote: >> >> On Fri, Aug 28, 2020 at 09:19:40AM +0300, Jani Nikula wrote: >> >>> The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be >> >>> used for the embedded display. Add support for using it via the EDID >> >>> override mechanism. >> >> >> >> Abusing the override for this feels a bit odd. >> > >> > It's the least intrusive way to make this work across the drm and driver >> > EDID code that I could think of. >> > >> > BR, >> > Jani. >> > >> > >> >> >> >> Also I have a vague recollection that there was perhaps some >> >> linkage between the mailbox and the ACPI _DDC stuff: >> >> git://github.com/vsyrjala/linux.git acpi_edid >> >> Only looked at this now. The patch at hand is for actually overriding >> the EDID from the panel, because the panel EDID is readable but bogus. > > Do we have an actual use case for this? The commit msg doesn't say so. It's a bit hazy still, but potentially yes, with a need to backport to stable as well. >> I >> have no idea how the ACPI _DDC stuff would work in this case. Would it >> return the EDID from the panel or from mailbox #5 or something >> completely different? Who knows. >> >> Using drm_do_get_edid() would of course be doable for reading mailbox #5 >> directly as well, but you'd have to make that the "primary" method and >> fall back to the usual drm_get_edid(). Note that this completely >> prevents you from ever reading the actual panel EDID. Using >> edid_override lets you get at the panel EDID too. > > Might be nice to make .get_edid() a connector vfunc and let each driver > implement it however they want. That way the driver would be in total > control over the priority of different EDID sources. But haven't really > looked at what that would take. Not even sure if a vfunc is totally > required as I think most EDID reads should be in some connector specific > driver code. -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/kms: Add separate hotplug event call for drm connector (rev2)
== Series Details ==
Series: drm/kms: Add separate hotplug event call for drm connector (rev2)
URL : https://patchwork.freedesktop.org/series/81257/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
e24fd2c19263 drm/kms: Add separate hotplug event call for drm connector
-:40: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#40: FILE: drivers/gpu/drm/drm_probe_helper.c:610:
+void drm_kms_connector_hotplug_helper(struct drm_device *dev,
+ struct drm_connector *connector)
-:72: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#72: FILE: drivers/gpu/drm/drm_sysfs.c:357:
+void drm_sysfs_connector_hotplug_event(struct drm_device *dev,
+ struct drm_connector *connector)
-:75: WARNING:STATIC_CONST_CHAR_ARRAY: char * array declaration might be better
as static const
#75: FILE: drivers/gpu/drm/drm_sysfs.c:360:
+ char *envp[] = { hotplug_str, conn_id, NULL };
-:78: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#78: FILE: drivers/gpu/drm/drm_sysfs.c:363:
+ snprintf(conn_id, ARRAY_SIZE(conn_id),
+ "CONNECTOR=%u", connector->base.id);
-:98: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#98: FILE: include/drm/drm_probe_helper.h:23:
+void drm_kms_connector_hotplug_helper(struct drm_device *dev,
+ struct drm_connector *connector);
-:111: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#111: FILE: include/drm/drm_sysfs.h:15:
+void drm_sysfs_connector_hotplug_event(struct drm_device *dev,
+ struct drm_connector *connector);
total: 0 errors, 1 warnings, 5 checks, 82 lines checked
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/kms: Add separate hotplug event call for drm connector (rev2)
== Series Details == Series: drm/kms: Add separate hotplug event call for drm connector (rev2) URL : https://patchwork.freedesktop.org/series/81257/ State : success == Summary == CI Bug Log - changes from CI_DRM_8954 -> Patchwork_18433 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/index.html Known issues Here are the changes found in Patchwork_18433 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_suspend@basic-s0: - fi-tgl-u2: [PASS][1] -> [FAIL][2] ([i915#1888]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-tgl-u2/igt@gem_exec_susp...@basic-s0.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-tgl-u2/igt@gem_exec_susp...@basic-s0.html * igt@i915_pm_rpm@basic-pci-d3-state: - fi-byt-j1900: [PASS][3] -> [DMESG-WARN][4] ([i915#1982]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-byt-j1900/igt@i915_pm_...@basic-pci-d3-state.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-byt-j1900/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_selftest@live@execlists: - fi-icl-y: [PASS][5] -> [INCOMPLETE][6] ([i915#2276]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-icl-y/igt@i915_selftest@l...@execlists.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-icl-y/igt@i915_selftest@l...@execlists.html * igt@i915_selftest@live@gem_contexts: - fi-tgl-u2: [PASS][7] -> [INCOMPLETE][8] ([i915#2045]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-bsw-n3050: [PASS][9] -> [DMESG-WARN][10] ([i915#1982]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-bsw-n3050/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-bsw-n3050/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html Possible fixes * igt@gem_exec_parallel@engines@contexts: - fi-icl-y: [INCOMPLETE][11] ([i915#2398]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-icl-y/igt@gem_exec_parallel@engi...@contexts.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-icl-y/igt@gem_exec_parallel@engi...@contexts.html * igt@i915_pm_rpm@module-reload: - fi-bsw-n3050: [DMESG-WARN][13] ([i915#1982]) -> [PASS][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-bsw-n3050/igt@i915_pm_...@module-reload.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-bsw-n3050/igt@i915_pm_...@module-reload.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-bsw-kefka: [DMESG-WARN][15] ([i915#1982]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-bsw-kefka/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-bsw-kefka/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html * igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1: - fi-icl-u2: [DMESG-WARN][17] ([i915#1982]) -> [PASS][18] +2 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vbl...@c-edp1.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vbl...@c-edp1.html Warnings * igt@amdgpu/amd_prime@i915-to-amd: - fi-cfl-8109u: [FAIL][19] -> [SKIP][20] ([fdo#109271]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-cfl-8109u/igt@amdgpu/amd_pr...@i915-to-amd.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18433/fi-cfl-8109u/igt@amdgpu/amd_pr...@i915-to-amd.html [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888 [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2045]: https://gitlab.freedesktop.org/drm/intel/issues/2045 [i915#2276]: https://gitlab.freedesktop.org/drm/intel/issues/2276 [i915#2398]: https://gitlab.freedesktop.org/drm/intel/issues/2398 Participating hosts (38 -> 33) -- Missing(5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper Build changes - * Linux: CI_DRM_8954 -> Patchwork_18433 CI-20190529: 20190529 CI_DRM_8954: 3d79444abaf4a8a0036a944c0cfd308a8b844ced @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5776: 46e431
Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK
On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > to separate function") the order of force_min_cdclk_changed check and
> > intel_modeset_checks(), was reversed. This broke the mechanism to
> > immediately force a new CDCLK minimum, and lead to driver probe
> > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > the issue by moving intel_modeset_checks() call later.
>
> Yep. I eyeed this same code recently and noticed the same bug.
> The one thing I didn't yet figure out is whether there is some
> subtle ordering requirement that was the reason for the change.
> But considering intel_modeset_checks() doesn't really do much
> anymore I think it should be safe.
>
> Sadly CI has been lumping all underrun errors under some ancient
> bugs, so no one noticed that things started to fail when this
> regression was introduced :(
>
> Reviewed-by: Ville Syrjälä
What surprises me here, is that the actual patch has been sent
and merged during late spring I think and we figure out that there was
a regression only by now.
For example I figured out this only today. When I was doing that change,
was actually aware that the change is actually quite significant as
it changes the way how we deal with CDCLK, however those were necessary
as we had a massive FIFO underrun issues at the moment. However CI didn't
show any problems, so we went ahead with this.
>
> >
> > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to
> > separate function)"
> > BugLink: https://github.com/thesofproject/linux/issues/2410
> > Signed-off-by: Kai Vehmanen
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 10 --
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 7d50b7177d40..8caeed23037c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device
> > *dev,
> > if (dev_priv->wm.distrust_bios_wm)
> > any_ms = true;
> >
> > - if (any_ms) {
> > - ret = intel_modeset_checks(state);
> > - if (ret)
> > - goto fail;
> > - }
> > -
> > intel_fbc_choose_crtc(dev_priv, state);
> > ret = calc_watermark_data(state);
> > if (ret)
> > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device
> > *dev,
> > goto fail;
> >
> > if (any_ms) {
> > + ret = intel_modeset_checks(state);
> > + if (ret)
> > + goto fail;
> > +
> > ret = intel_modeset_calc_cdclk(state);
> > if (ret)
> > return ret;
> > --
> > 2.27.0
>
> --
> Ville Syrjälä
> Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK
On Wed, Sep 02, 2020 at 03:12:01PM +0300, Lisovskiy, Stanislav wrote:
> On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > > to separate function") the order of force_min_cdclk_changed check and
> > > intel_modeset_checks(), was reversed. This broke the mechanism to
> > > immediately force a new CDCLK minimum, and lead to driver probe
> > > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > > the issue by moving intel_modeset_checks() call later.
> >
> > Yep. I eyeed this same code recently and noticed the same bug.
> > The one thing I didn't yet figure out is whether there is some
> > subtle ordering requirement that was the reason for the change.
> > But considering intel_modeset_checks() doesn't really do much
> > anymore I think it should be safe.
> >
> > Sadly CI has been lumping all underrun errors under some ancient
> > bugs, so no one noticed that things started to fail when this
> > regression was introduced :(
> >
> > Reviewed-by: Ville Syrjälä
>
> What surprises me here, is that the actual patch has been sent
> and merged during late spring I think and we figure out that there was
> a regression only by now.
> For example I figured out this only today. When I was doing that change,
> was actually aware that the change is actually quite significant as
> it changes the way how we deal with CDCLK, however those were necessary
> as we had a massive FIFO underrun issues at the moment. However CI didn't
> show any problems, so we went ahead with this.
I spotted some CI logs that show underruns due to this regression,
but the results just got lumped in with other older underrun bugs,
and thus CI results were always "success" :/
I think we need to kill off all underrun related CI filters and
start from scratch. Otherwise new bugs will keep slipping through.
>
> >
> > >
> > > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to
> > > separate function)"
> > > BugLink: https://github.com/thesofproject/linux/issues/2410
> > > Signed-off-by: Kai Vehmanen
> > > ---
> > > drivers/gpu/drm/i915/display/intel_display.c | 10 --
> > > 1 file changed, 4 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 7d50b7177d40..8caeed23037c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device
> > > *dev,
> > > if (dev_priv->wm.distrust_bios_wm)
> > > any_ms = true;
> > >
> > > - if (any_ms) {
> > > - ret = intel_modeset_checks(state);
> > > - if (ret)
> > > - goto fail;
> > > - }
> > > -
> > > intel_fbc_choose_crtc(dev_priv, state);
> > > ret = calc_watermark_data(state);
> > > if (ret)
> > > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device
> > > *dev,
> > > goto fail;
> > >
> > > if (any_ms) {
> > > + ret = intel_modeset_checks(state);
> > > + if (ret)
> > > + goto fail;
> > > +
> > > ret = intel_modeset_calc_cdclk(state);
> > > if (ret)
> > > return ret;
> > > --
> > > 2.27.0
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Remove the old global state stuff
From: Ville Syrjälä
With the dbuf code mostly converted over to the new global state
handling we can remove the leftovers of the old global state
stuff.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_atomic.c | 39 ---
drivers/gpu/drm/i915/display/intel_atomic.h | 4 --
drivers/gpu/drm/i915/display/intel_display.c | 22 ---
.../drm/i915/display/intel_display_types.h| 7
4 files changed, 72 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index 630f49b7aa01..86be032bcf96 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -527,8 +527,6 @@ void intel_atomic_state_clear(struct drm_atomic_state *s)
intel_atomic_clear_global_state(state);
state->dpll_set = state->modeset = false;
- state->global_state_changed = false;
- state->active_pipes = 0;
}
struct intel_crtc_state *
@@ -542,40 +540,3 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
return to_intel_crtc_state(crtc_state);
}
-
-int _intel_atomic_lock_global_state(struct intel_atomic_state *state)
-{
- struct drm_i915_private *dev_priv = to_i915(state->base.dev);
- struct intel_crtc *crtc;
-
- state->global_state_changed = true;
-
- for_each_intel_crtc(&dev_priv->drm, crtc) {
- int ret;
-
- ret = drm_modeset_lock(&crtc->base.mutex,
- state->base.acquire_ctx);
- if (ret)
- return ret;
- }
-
- return 0;
-}
-
-int _intel_atomic_serialize_global_state(struct intel_atomic_state *state)
-{
- struct drm_i915_private *dev_priv = to_i915(state->base.dev);
- struct intel_crtc *crtc;
-
- state->global_state_changed = true;
-
- for_each_intel_crtc(&dev_priv->drm, crtc) {
- struct intel_crtc_state *crtc_state;
-
- crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
- if (IS_ERR(crtc_state))
- return PTR_ERR(crtc_state);
- }
-
- return 0;
-}
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h
b/drivers/gpu/drm/i915/display/intel_atomic.h
index 11146292b06f..285de07011dc 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic.h
@@ -56,8 +56,4 @@ int intel_atomic_setup_scalers(struct drm_i915_private
*dev_priv,
struct intel_crtc *intel_crtc,
struct intel_crtc_state *crtc_state);
-int _intel_atomic_lock_global_state(struct intel_atomic_state *state);
-
-int _intel_atomic_serialize_global_state(struct intel_atomic_state *state);
-
#endif /* __INTEL_ATOMIC_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index dc622af8695c..553e4440442a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14636,16 +14636,8 @@ u8 intel_calc_active_pipes(struct intel_atomic_state
*state,
static int intel_modeset_checks(struct intel_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
- int ret;
state->modeset = true;
- state->active_pipes = intel_calc_active_pipes(state,
dev_priv->active_pipes);
-
- if (state->active_pipes != dev_priv->active_pipes) {
- ret = _intel_atomic_lock_global_state(state);
- if (ret)
- return ret;
- }
if (IS_HASWELL(dev_priv))
return hsw_mode_set_planes_workaround(state);
@@ -15759,14 +15751,6 @@ static void intel_atomic_track_fbs(struct
intel_atomic_state *state)
plane->frontbuffer_bit);
}
-static void assert_global_state_locked(struct drm_i915_private *dev_priv)
-{
- struct intel_crtc *crtc;
-
- for_each_intel_crtc(&dev_priv->drm, crtc)
- drm_modeset_lock_assert_held(&crtc->base.mutex);
-}
-
static int intel_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *_state,
bool nonblock)
@@ -15842,12 +15826,6 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_shared_dpll_swap_state(state);
intel_atomic_track_fbs(state);
- if (state->global_state_changed) {
- assert_global_state_locked(dev_priv);
-
- dev_priv->active_pipes = state->active_pipes;
- }
-
drm_atomic_state_get(&state->base);
INIT_WORK(&state->base.commit_work, intel_atomic_commit_work);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 413b60337a0b..60f66013e513 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/
Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK
On Wed, Sep 02, 2020 at 03:17:08PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 02, 2020 at 03:12:01PM +0300, Lisovskiy, Stanislav wrote:
> > On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > > > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > > > to separate function") the order of force_min_cdclk_changed check and
> > > > intel_modeset_checks(), was reversed. This broke the mechanism to
> > > > immediately force a new CDCLK minimum, and lead to driver probe
> > > > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > > > the issue by moving intel_modeset_checks() call later.
> > >
> > > Yep. I eyeed this same code recently and noticed the same bug.
> > > The one thing I didn't yet figure out is whether there is some
> > > subtle ordering requirement that was the reason for the change.
> > > But considering intel_modeset_checks() doesn't really do much
> > > anymore I think it should be safe.
> > >
> > > Sadly CI has been lumping all underrun errors under some ancient
> > > bugs, so no one noticed that things started to fail when this
> > > regression was introduced :(
> > >
> > > Reviewed-by: Ville Syrjälä
> >
> > What surprises me here, is that the actual patch has been sent
> > and merged during late spring I think and we figure out that there was
> > a regression only by now.
> > For example I figured out this only today. When I was doing that change,
> > was actually aware that the change is actually quite significant as
> > it changes the way how we deal with CDCLK, however those were necessary
> > as we had a massive FIFO underrun issues at the moment. However CI didn't
> > show any problems, so we went ahead with this.
>
> I spotted some CI logs that show underruns due to this regression,
> but the results just got lumped in with other older underrun bugs,
> and thus CI results were always "success" :/
>
> I think we need to kill off all underrun related CI filters and
> start from scratch. Otherwise new bugs will keep slipping through.
Another concern I have here, as I understand now intel_modeset_checks
will be put again after wm/ddb and bw calculations - won't this
cause any additional issues?
Also you now have modeset checks still before that force_min_cdclk
condition check which is now in intel_modeset_calc_cdclk.
My idea was to put all CDCLK related calculations and checks into
same function. However this could be a bad idea, so should you may
be just extract force_min_cdclk check condition back from
intel_modeset_calc_cdclk
to the original place where it was?
I'm just now thinking in terms of not breaking anything else now...
Stan
>
> >
> > >
> > > >
> > > > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to
> > > > separate function)"
> > > > BugLink: https://github.com/thesofproject/linux/issues/2410
> > > > Signed-off-by: Kai Vehmanen
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_display.c | 10 --
> > > > 1 file changed, 4 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 7d50b7177d40..8caeed23037c 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct
> > > > drm_device *dev,
> > > > if (dev_priv->wm.distrust_bios_wm)
> > > > any_ms = true;
> > > >
> > > > - if (any_ms) {
> > > > - ret = intel_modeset_checks(state);
> > > > - if (ret)
> > > > - goto fail;
> > > > - }
> > > > -
> > > > intel_fbc_choose_crtc(dev_priv, state);
> > > > ret = calc_watermark_data(state);
> > > > if (ret)
> > > > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct
> > > > drm_device *dev,
> > > > goto fail;
> > > >
> > > > if (any_ms) {
> > > > + ret = intel_modeset_checks(state);
> > > > + if (ret)
> > > > + goto fail;
> > > > +
> > > > ret = intel_modeset_calc_cdclk(state);
> > > > if (ret)
> > > > return ret;
> > > > --
> > > > 2.27.0
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
>
> --
> Ville Syrjälä
> Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK
On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > to separate function") the order of force_min_cdclk_changed check and
> > intel_modeset_checks(), was reversed. This broke the mechanism to
> > immediately force a new CDCLK minimum, and lead to driver probe
> > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > the issue by moving intel_modeset_checks() call later.
>
> Yep. I eyeed this same code recently and noticed the same bug.
> The one thing I didn't yet figure out is whether there is some
> subtle ordering requirement that was the reason for the change.
> But considering intel_modeset_checks() doesn't really do much
> anymore I think it should be safe.
>
> Sadly CI has been lumping all underrun errors under some ancient
> bugs, so no one noticed that things started to fail when this
> regression was introduced :(
>
> Reviewed-by: Ville Syrjälä
Replying to that thread as well:
Another concern I have here, as I understand now intel_modeset_checks
will be put again after wm/ddb and bw calculations - won't this
cause any additional issues?
Also you now have modeset checks still before that force_min_cdclk
condition check which is now in intel_modeset_calc_cdclk.
My idea was to put all CDCLK related calculations and checks into
same function. However this could be a bad idea, so should you may
be just extract force_min_cdclk check condition back from
intel_modeset_calc_cdclk
to the original place where it was?
I'm just now thinking in terms of not breaking anything else now...
Stan
>
> >
> > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to
> > separate function)"
> > BugLink: https://github.com/thesofproject/linux/issues/2410
> > Signed-off-by: Kai Vehmanen
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 10 --
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 7d50b7177d40..8caeed23037c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device
> > *dev,
> > if (dev_priv->wm.distrust_bios_wm)
> > any_ms = true;
> >
> > - if (any_ms) {
> > - ret = intel_modeset_checks(state);
> > - if (ret)
> > - goto fail;
> > - }
> > -
> > intel_fbc_choose_crtc(dev_priv, state);
> > ret = calc_watermark_data(state);
> > if (ret)
> > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device
> > *dev,
> > goto fail;
> >
> > if (any_ms) {
> > + ret = intel_modeset_checks(state);
> > + if (ret)
> > + goto fail;
> > +
> > ret = intel_modeset_calc_cdclk(state);
> > if (ret)
> > return ret;
> > --
> > 2.27.0
>
> --
> Ville Syrjälä
> Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Remove the old global state stuff
== Series Details == Series: drm/i915: Remove the old global state stuff URL : https://patchwork.freedesktop.org/series/81265/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8954 -> Patchwork_18434 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18434 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18434, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/index.html Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18434: ### IGT changes ### Possible regressions * igt@gem_exec_gttfill@basic: - fi-cfl-8109u: [PASS][1] -> [TIMEOUT][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-cfl-8109u/igt@gem_exec_gttf...@basic.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-cfl-8109u/igt@gem_exec_gttf...@basic.html Known issues Here are the changes found in Patchwork_18434 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_parallel@engines@basic: - fi-tgl-u2: [PASS][3] -> [INCOMPLETE][4] ([i915#2398]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-tgl-u2/igt@gem_exec_parallel@engi...@basic.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-tgl-u2/igt@gem_exec_parallel@engi...@basic.html * igt@i915_pm_rpm@basic-pci-d3-state: - fi-byt-j1900: [PASS][5] -> [DMESG-WARN][6] ([i915#1982]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-byt-j1900/igt@i915_pm_...@basic-pci-d3-state.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-byt-j1900/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_selftest@live@execlists: - fi-icl-y: [PASS][7] -> [INCOMPLETE][8] ([i915#2276]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-icl-y/igt@i915_selftest@l...@execlists.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-icl-y/igt@i915_selftest@l...@execlists.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-bsw-n3050: [PASS][9] -> [DMESG-WARN][10] ([i915#1982]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-bsw-n3050/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-bsw-n3050/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html * igt@kms_cursor_legacy@basic-flip-before-cursor-atomic: - fi-icl-u2: [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) +2 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-icl-u2/igt@kms_cursor_leg...@basic-flip-before-cursor-atomic.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-icl-u2/igt@kms_cursor_leg...@basic-flip-before-cursor-atomic.html Possible fixes * igt@gem_exec_parallel@engines@contexts: - fi-icl-y: [INCOMPLETE][13] ([i915#2398]) -> [PASS][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-icl-y/igt@gem_exec_parallel@engi...@contexts.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-icl-y/igt@gem_exec_parallel@engi...@contexts.html * igt@i915_pm_rpm@basic-pci-d3-state: - fi-bsw-kefka: [DMESG-WARN][15] ([i915#1982]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_pm_rpm@module-reload: - fi-bsw-n3050: [DMESG-WARN][17] ([i915#1982]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-bsw-n3050/igt@i915_pm_...@module-reload.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-bsw-n3050/igt@i915_pm_...@module-reload.html * igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1: - fi-icl-u2: [DMESG-WARN][19] ([i915#1982]) -> [PASS][20] +2 similar issues [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vbl...@c-edp1.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vbl...@c-edp1.html Warnings * igt@amdgpu/amd_prime@i915-to-amd: - fi-cfl-8109u: [FAIL][21] -> [SKIP][22] ([fdo#109271]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8954/fi-cfl-8109u/igt@amdgpu/amd_pr...@i915-to-amd.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18434/fi-cfl-8109u/igt@amdgpu/amd_pr...@i915-to-a
Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK
On Wed, Sep 02, 2020 at 03:34:50PM +0300, Lisovskiy, Stanislav wrote:
> On Wed, Sep 02, 2020 at 03:17:08PM +0300, Ville Syrjälä wrote:
> > On Wed, Sep 02, 2020 at 03:12:01PM +0300, Lisovskiy, Stanislav wrote:
> > > On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > > > > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > > > > to separate function") the order of force_min_cdclk_changed check and
> > > > > intel_modeset_checks(), was reversed. This broke the mechanism to
> > > > > immediately force a new CDCLK minimum, and lead to driver probe
> > > > > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > > > > the issue by moving intel_modeset_checks() call later.
> > > >
> > > > Yep. I eyeed this same code recently and noticed the same bug.
> > > > The one thing I didn't yet figure out is whether there is some
> > > > subtle ordering requirement that was the reason for the change.
> > > > But considering intel_modeset_checks() doesn't really do much
> > > > anymore I think it should be safe.
> > > >
> > > > Sadly CI has been lumping all underrun errors under some ancient
> > > > bugs, so no one noticed that things started to fail when this
> > > > regression was introduced :(
> > > >
> > > > Reviewed-by: Ville Syrjälä
> > >
> > > What surprises me here, is that the actual patch has been sent
> > > and merged during late spring I think and we figure out that there was
> > > a regression only by now.
> > > For example I figured out this only today. When I was doing that change,
> > > was actually aware that the change is actually quite significant as
> > > it changes the way how we deal with CDCLK, however those were necessary
> > > as we had a massive FIFO underrun issues at the moment. However CI didn't
> > > show any problems, so we went ahead with this.
> >
> > I spotted some CI logs that show underruns due to this regression,
> > but the results just got lumped in with other older underrun bugs,
> > and thus CI results were always "success" :/
> >
> > I think we need to kill off all underrun related CI filters and
> > start from scratch. Otherwise new bugs will keep slipping through.
>
> Another concern I have here, as I understand now intel_modeset_checks
> will be put again after wm/ddb and bw calculations - won't this
> cause any additional issues?
intel_modeset_checks() doesn't really do much anymore.
I think the old global state mess was the reason there was
some ordering requirement between the two. But if you see the
patch I just posted that old global state stuff is now dead
code that can be ripped out.
Oh, the other linkage was the cdclk vs. linetime watermark, but
I moved the linetime wm stuff elsewhere already a while ago. So
also not an issue.
>
> Also you now have modeset checks still before that force_min_cdclk
> condition check which is now in intel_modeset_calc_cdclk.
> My idea was to put all CDCLK related calculations and checks into
> same function. However this could be a bad idea, so should you may
> be just extract force_min_cdclk check condition back from
> intel_modeset_calc_cdclk
> to the original place where it was?
The force cdclk stuff looks fine to me. Though there is
https://patchwork.freedesktop.org/patch/377191/?series=79480&rev=1
still unreviewed which would clean it up a bit further.
>
> I'm just now thinking in terms of not breaking anything else now...
>
> Stan
>
> >
> > >
> > > >
> > > > >
> > > > > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > > > > to separate function)"
> > > > > BugLink: https://github.com/thesofproject/linux/issues/2410
> > > > > Signed-off-by: Kai Vehmanen
> > > > > ---
> > > > > drivers/gpu/drm/i915/display/intel_display.c | 10 --
> > > > > 1 file changed, 4 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 7d50b7177d40..8caeed23037c 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct
> > > > > drm_device *dev,
> > > > > if (dev_priv->wm.distrust_bios_wm)
> > > > > any_ms = true;
> > > > >
> > > > > - if (any_ms) {
> > > > > - ret = intel_modeset_checks(state);
> > > > > - if (ret)
> > > > > - goto fail;
> > > > > - }
> > > > > -
> > > > > intel_fbc_choose_crtc(dev_priv, state);
> > > > > ret = calc_watermark_data(state);
> > > > > if (ret)
> > > > > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct
> > > > > drm_device *dev,
> > > > > goto fail;
> > > > >
> > > > > if (any_ms) {
> > > > > + ret = intel_modeset_checks(state);
> > > > > +
Re: [Intel-gfx] Various problems for the Xen for XenGT code and guide.
Hi, It is mainly due to the business priority change. XenGT project was originally created for data center usages with XEON E3 servers which have integrated processor graphics. After SkyLake E3, there are no new servers capable of running GVT-g, and Intel future graphics for data center will have different approaches for GPU sharing. Another reason is the XenGT upstream difficulty. Different from KVMGT which has been fully merged to upstream, Xen part of GVT-g still has technical opens that are hard to close quickly. Sorry that we did not sync up with community in time the XenGT ramping down plan. Internally we have been testing XenGT until November 2019. We will update our setup guide for XenGT part to reflect the information. Going forward we will try our best to do XenGT consultant, but will not be able to do code rebase/test or debugging. Meanwhile, we are still maintaining KVMGT mainly for client integrated GPU usages. We will continue to fix issues that can be reproduced with KVMGT. https://github.com/intel/gvt-linux/tree/topic/gvt-xengt https://github.com/intel/Igvtg-xen/tree/xengt-stable-4.10 Hi Marietto, we appreciate your efforts trying GVT-g (XenGT). Hope Terrence's reply helps. By switching back to old gcc those compile errors should be gone. Meanwhile please be kindly noticed that we can only provide limited support on XenGT with old versions. Thanks! Regards, -Zhiyuan -Original Message- From: Jason Long Date: Wednesday, September 2, 2020 at 1:07 AM To: Mario Marietto , "igv...@lists.01.org" , "xen-de...@lists.xenproject.org" , "xen-de...@lists.xen.org" , "intel-gfx@lists.freedesktop.org" , "linux-ker...@vger.kernel.org" , Susie Li , "Tian, Kevin" , Zhiyuan Lv , "Li, Weinan Z" , "Downs, Mike" , "Xu, Terrence" Subject: Re: Various problems for the Xen for XenGT code and guide. Hello, Why XenGT doesn't have any new version? On Tuesday, September 1, 2020, 09:21:27 PM GMT+4:30, Xu, Terrence wrote: Hi Mario, Sorry to make you feel uncomfortable. I think it is not setup guide problem, the main reason is the Xen code is very old (We are upgrading GVT-g code on Linux kernel side and we haven’t upgraded the Xen and Qemu source for XenGT for at least 2 years) but your GCC is new (You are using Ubuntu 20.4, the gcc version is 9+). I have a way to workaround it, as below: 1. apt-get install gcc-72. ln -fs gcc-7 /usr/bin/gcc Any more problem just let us know! Thanks Terrence From: Mario Marietto Sent: Thursday, August 27, 2020 9:52 PM To: Xu, Terrence ; igv...@lists.01.org; xen-de...@lists.xenproject.org; xen-de...@lists.xen.org; intel-gfx@lists.freedesktop.org; linux-ker...@vger.kernel.org; Li, Susie ; Tian, Kevin ; Lv, Zhiyuan ; Li, Weinan Z ; Downs, Mike Subject: Various problems for the Xen for XenGT code and guide. Hello. I would like to pass the integrated gpu from the host os (ubuntu 20.04) to the windows 10 guest os with xen. This is because xen works great for me,better than qemu-kvm for my specific needs and because I have only two graphic cards. The nvidia rtx 2080 ti that I have already passed to the guest,and the intel UHD 630,that can be duplicated from the host to the guest so that it can be used in both places without interruptions. So I'm trying to build this repository : https://github.com/intel/gvt-linux/wiki/GVTg_Setup_Guide#332-build-qemu--xen-for-xengt I have to say that this guide is totally not very well written. And the code is full of unpatched bugs. It's a month that I'm working on that,trying to fix the bugs that are came out from the 2015 until today. This is not my job. This is my hobby. But,I need to activate the pass through for my integrated GPU so I don't to give up. I'm also very angry with those coders who do not do their job well and with those coders who do not respond to help messages. It is not enough to write good code to be a good programmer. It is also important to keep the documentation updated, to help those who cannot get the code to work. Anyway,I've documented every step that I did to make it work here : https://github.com/intel/gvt-linux/issues/168 right now I'm trying to fix the bug n. 434544,that you can see below. CC util/qemu-error.o/etc/xen/igvtg-xen/tools/qemu-xen-dir/util/qemu-error.c: In function ‘vreport’:/etc/xen/igvtg-xen/tools/qemu-xen-dir/util/qemu-error.c:201:5: error: ‘GTimeVal’ is deprecated: Use 'GDateTime' instead [-Werror=deprecated-declarations]201 | GTimeVal tv;| ^~~~In file included from /usr/include/glib-2.0/glib/galloca.h:32,from /usr/include/glib-2.0/glib.h:30,from /etc/xen/igvtg-xen/tools/qemu-xen-dir/include/glib-compat.h:19,from /etc/xen/igvtg-xen/tools/qemu-xen-dir/include/qemu/osdep.h:107,from /etc/xen/igvtg-xen/tools/qemu-xen-dir/util/qemu-error.c:13:/usr/include/glib-2.0/glib/gtypes.h:547:8: note: declared here547 | struct GTimeVal| ^/etc/xen/igvtg-xen/tools/qemu-xen-dir/util/qem
Re: [Intel-gfx] [PATCH v6 1/7] drm/i915: Add enable/disable flip done and flip done handler
On 9/1/2020 4:42 PM, Ville Syrjälä wrote:
On Fri, Aug 07, 2020 at 03:05:45PM +0530, Karthik B S wrote:
Add enable/disable flip done functions and the flip done handler
function which handles the flip done interrupt.
Enable the flip done interrupt in IER.
Enable flip done function is called before writing the
surface address register as the write to this register triggers
the flip done interrupt
Flip done handler is used to send the page flip event as soon as the
surface address is written as per the requirement of async flips.
The interrupt is disabled after the event is sent.
v2: -Change function name from icl_* to skl_* (Paulo)
-Move flip handler to this patch (Paulo)
-Remove vblank_put() (Paulo)
-Enable flip done interrupt for gen9+ only (Paulo)
-Enable flip done interrupt in power_well_post_enable hook (Paulo)
-Removed the event check in flip done handler to handle async
flips without pageflip events.
v3: -Move skl_disable_flip_done out of interrupt handler (Paulo)
-Make the pending vblank event NULL in the beginning of
flip_done_handler to remove sporadic WARN_ON that is seen.
v4: -Calculate timestamps using flip done time stamp and current
timestamp for async flips (Ville)
v5: -Fix the sparse warning by making the function 'g4x_get_flip_counter'
static.(Reported-by: kernel test robot )
-Fix the typo in commit message.
v6: -Revert back to old time stamping code.
-Remove the break while calling skl_enable_flip_done. (Paulo)
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_display.c | 8 +++
drivers/gpu/drm/i915/i915_irq.c | 52
drivers/gpu/drm/i915/i915_irq.h | 2 +
3 files changed, 62 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 522c772a2111..1ac2e6f27597 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15562,6 +15562,11 @@ static void intel_atomic_commit_tail(struct
intel_atomic_state *state)
intel_dbuf_pre_plane_update(state);
+ for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+ if (new_crtc_state->uapi.async_flip)
+ skl_enable_flip_done(&crtc->base);
+ }
+
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
dev_priv->display.commit_modeset_enables(state);
@@ -15583,6 +15588,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
drm_atomic_helper_wait_for_flip_done(dev, &state->base);
for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+ if (new_crtc_state->uapi.async_flip)
+ skl_disable_flip_done(&crtc->base);
Where do we wait for the flip done? Can't see such code anywhere.
Thanks for the review.
My understanding is that drm_atomic_helper_wait_for_flip_done should
take care of waiting for flip done.
Am I missing something? should there be a separate wait?
Thanks,
Karthik.B.S
+
if (new_crtc_state->hw.active &&
!needs_modeset(new_crtc_state) &&
!new_crtc_state->preload_luts &&
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f113fe44572b..6cc129b031d3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1296,6 +1296,23 @@ display_pipe_crc_irq_handler(struct drm_i915_private
*dev_priv,
u32 crc4) {}
#endif
+static void flip_done_handler(struct drm_i915_private *dev_priv,
+ unsigned int pipe)
+{
+ struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+ struct drm_crtc_state *crtc_state = crtc->base.state;
+ struct drm_pending_vblank_event *e = crtc_state->event;
+ struct drm_device *dev = &dev_priv->drm;
+ unsigned long irqflags;
+
+ spin_lock_irqsave(&dev->event_lock, irqflags);
+
+ crtc_state->event = NULL;
+
+ drm_crtc_send_vblank_event(&crtc->base, e);
+
+ spin_unlock_irqrestore(&dev->event_lock, irqflags);
+}
static void hsw_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
enum pipe pipe)
@@ -2390,6 +2407,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv,
u32 master_ctl)
if (iir & GEN8_PIPE_VBLANK)
intel_handle_vblank(dev_priv, pipe);
+ if (iir & GEN9_PIPE_PLANE1_FLIP_DONE)
+ flip_done_handler(dev_priv, pipe);
+
if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
hsw_pipe_crc_irq_handler(dev_priv, pipe);
@@ -2711,6 +2731,19 @@ int bdw_enable_vblank(struct drm_crtc *crtc)
return 0;
}
+void skl_enable_flip_done(struct drm_crtc *crtc)
+{
+ struct drm
Re: [Intel-gfx] [PATCH v6 2/7] drm/i915: Add support for async flips in I915
On 9/1/2020 4:45 PM, Ville Syrjälä wrote:
On Fri, Aug 07, 2020 at 03:05:46PM +0530, Karthik B S wrote:
Set the Async Address Update Enable bit in plane ctl
when async flip is requested.
v2: -Move the Async flip enablement to individual patch (Paulo)
v3: -Rebased.
v4: -Add separate plane hook for async flip case (Ville)
v5: -Rebased.
v6: -Move the plane hook to separate patch. (Paulo)
-Remove the early return in skl_plane_ctl. (Paulo)
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_display.c | 3 +++
drivers/gpu/drm/i915/i915_reg.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 1ac2e6f27597..ce2b0c14a073 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4768,6 +4768,9 @@ u32 skl_plane_ctl(const struct intel_crtc_state
*crtc_state,
plane_ctl = PLANE_CTL_ENABLE;
+ if (crtc_state->uapi.async_flip)
+ plane_ctl |= PLANE_CTL_ASYNC_FLIP;
Hmm. We might want to put that into skl_plane_ctl_crtc() since it's
a crtc-wide thing,
Thanks for the review.
Sure, I'll move this.
Thanks,
Karthik.B.S
+
if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) {
plane_ctl |= skl_plane_ctl_alpha(plane_state);
plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e85c6fc1f3cb..3f88d9ac90a8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6924,6 +6924,7 @@ enum {
#define PLANE_CTL_TILED_X (1 << 10)
#define PLANE_CTL_TILED_Y (4 << 10)
#define PLANE_CTL_TILED_YF (5 << 10)
+#define PLANE_CTL_ASYNC_FLIP (1 << 9)
#define PLANE_CTL_FLIP_HORIZONTAL (1 << 8)
#define PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE(1 << 4) /* TGL+ */
#define PLANE_CTL_ALPHA_MASK(0x3 << 4) /* Pre-GLK */
--
2.22.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 3/7] drm/i915: Add checks specific to async flips
On 9/1/2020 4:51 PM, Ville Syrjälä wrote:
On Fri, Aug 07, 2020 at 03:05:47PM +0530, Karthik B S wrote:
If flip is requested on any other plane, reject it.
Make sure there is no change in fbc, offset and framebuffer modifiers
when async flip is requested.
If any of these are modified, reject async flip.
v2: -Replace DRM_ERROR (Paulo)
-Add check for changes in OFFSET, FBC, RC(Paulo)
v3: -Removed TODO as benchmarking tests have been run now.
v4: -Added more state checks for async flip (Ville)
-Moved intel_atomic_check_async to the end of intel_atomic_check
as the plane checks needs to pass before this. (Ville)
-Removed crtc_state->enable_fbc check. (Ville)
-Set the I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP flag for async
flip case as scanline counter is not reliable here.
v5: -Fix typo and other check patch errors seen in CI
in 'intel_atomic_check_async' function.
v6: -Don't call intel_atomic_check_async multiple times. (Ville)
-Remove the check for n_planes in intel_atomic_check_async
-Added documentation for async flips. (Paulo)
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_display.c | 113 +++
1 file changed, 113 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index ce2b0c14a073..9629c751d2af 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14832,6 +14832,110 @@ static bool intel_cpu_transcoders_need_modeset(struct
intel_atomic_state *state,
return false;
}
+/**
+ * DOC: asynchronous flip implementation
+ *
+ * Asynchronous page flip is the implementation for the
DRM_MODE_PAGE_FLIP_ASYNC
+ * flag. Currently async flip is only supported via the drmModePageFlip IOCTL.
+ * Correspondingly, support is currently added for primary plane only.
+ *
+ * Async flip can only change the plane surface address, so anything else
+ * changing is rejected from the intel_atomic_check_async() function.
+ * Once this check is cleared, flip done interrupt is enabled using
+ * the skl_enable_flip_done() function.
+ *
+ * As soon as the surface address register is written, flip done interrupt is
+ * generated and the requested events are sent to the usersapce in the
interrupt
+ * handler itself. The timestamp and sequence sent during the flip done event
+ * correspond to the last vblank and have no relation to the actual time when
+ * the flip done event was sent.
+ */
+
+static int intel_atomic_check_async(struct intel_atomic_state *state)
+{
+ struct intel_crtc_state *old_crtc_state, *new_crtc_state;
+ struct intel_plane_state *new_plane_state, *old_plane_state;
+ struct intel_crtc *crtc;
+ struct intel_plane *intel_plane;
s/intel_plane/plane/
Thanks for the review.
I will update this.
+ int i;
+
+ for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+ new_crtc_state, i) {
+ if (needs_modeset(new_crtc_state)) {
+ DRM_DEBUG_KMS("Modeset Required. Async flip not
supported\n");
+ return -EINVAL;
+ }
+
+ if (!new_crtc_state->uapi.active) {
+ DRM_DEBUG_KMS("CRTC inactive\n");
+ return -EINVAL;
+ }
All the uapi.foo stuff should be hw.foo most likely.
Will update this.
+ if (old_crtc_state->active_planes !=
new_crtc_state->active_planes) {
+ DRM_DEBUG_KMS("Active planes cannot be changed during async
flip\n");
+ return -EINVAL;
+ }
+ }
+
+ for_each_oldnew_intel_plane_in_state(state, intel_plane,
old_plane_state,
+new_plane_state, i) {
+ /*TODO: Async flip is only supported through the page flip IOCTL
+* as of now. So support currently added for primary plane only.
+* Support for other planes should be added when async flip is
+* enabled in the atomic IOCTL path.
+*/
+ if (intel_plane->id != PLANE_PRIMARY)
+ return -EINVAL;
+
+ if (old_plane_state->color_plane[0].x !=
+ new_plane_state->color_plane[0].x ||
+ old_plane_state->color_plane[0].y !=
+ new_plane_state->color_plane[0].y) {
+ DRM_DEBUG_KMS("Offsets cannot be changed in async
flip\n");
+ return -EINVAL;
+ }
+
+ if (old_plane_state->uapi.fb->modifier !=
+ new_plane_state->uapi.fb->modifier) {
+ DRM_DEBUG_KMS("Framebuffer modifiers cannot be changed in
async flip\n");
+ return -EINVAL;
+ }
+
+
Re: [Intel-gfx] [PATCH v6 4/7] drm/i915: Do not call drm_crtc_arm_vblank_event in async flips
On 9/1/2020 4:53 PM, Ville Syrjälä wrote:
On Fri, Aug 07, 2020 at 03:05:48PM +0530, Karthik B S wrote:
Since the flip done event will be sent in the flip_done_handler,
no need to add the event to the list and delay it for later.
v2: -Moved the async check above vblank_get as it
was causing issues for PSR.
v3: -No need to wait for vblank to pass, as this wait was causing a
16ms delay once every few flips.
v4: -Rebased.
v5: -Rebased.
v6: -Rebased.
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_sprite.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
b/drivers/gpu/drm/i915/display/intel_sprite.c
index c26ca029fc0a..2b2d96c59d7f 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -93,6 +93,9 @@ void intel_pipe_update_start(const struct intel_crtc_state
*new_crtc_state)
DEFINE_WAIT(wait);
u32 psr_status;
+ if (new_crtc_state->uapi.async_flip)
+ goto irq_disable;
We shouldn't really need the irq disable at all if we don't do the
vblank evade. And if we only write ctl+surf then atomicity is already
guaranteed by the hw.
Thanks for the review.
Sure, will return directly after this check.
Thanks,
Karthik.B.S
+
vblank_start = adjusted_mode->crtc_vblank_start;
if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
vblank_start = DIV_ROUND_UP(vblank_start, 2);
@@ -206,7 +209,7 @@ void intel_pipe_update_end(struct intel_crtc_state
*new_crtc_state)
* Would be slightly nice to just grab the vblank count and arm the
* event outside of the critical section - the spinlock might spin for a
* while ... */
- if (new_crtc_state->uapi.event) {
+ if (new_crtc_state->uapi.event && !new_crtc_state->uapi.async_flip) {
drm_WARN_ON(&dev_priv->drm,
drm_crtc_vblank_get(&crtc->base) != 0);
@@ -220,6 +223,9 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
local_irq_enable();
+ if (new_crtc_state->uapi.async_flip)
+ return;
+
if (intel_vgpu_active(dev_priv))
return;
--
2.22.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 5/7] drm/i915: Add dedicated plane hook for async flip case
On 9/1/2020 4:57 PM, Ville Syrjälä wrote:
On Fri, Aug 07, 2020 at 03:05:49PM +0530, Karthik B S wrote:
This hook is added to avoid writing other plane registers in case of
async flips, so that we do not write the double buffered registers
during async surface address update.
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_sprite.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
b/drivers/gpu/drm/i915/display/intel_sprite.c
index 2b2d96c59d7f..1c03546a4d2a 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -609,6 +609,24 @@ icl_program_input_csc(struct intel_plane *plane,
PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 2), 0x0);
}
+static void
+skl_program_async_surface_address(struct drm_i915_private *dev_priv,
+ const struct intel_plane_state *plane_state,
+ enum pipe pipe, enum plane_id plane_id,
+ u32 surf_addr)
+{
+ unsigned long irqflags;
+ u32 plane_ctl = plane_state->ctl;
Need the bits from skl_plane_ctl_crtc() too.
Thanks for the review.
Sure, I'll update this.
+
+ spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+ intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
+ intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
+ intel_plane_ggtt_offset(plane_state) + surf_addr);
+
+ spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
static void
skl_program_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
@@ -637,6 +655,13 @@ skl_program_plane(struct intel_plane *plane,
u32 keymsk, keymax;
u32 plane_ctl = plane_state->ctl;
+ /* During Async flip, no other updates are allowed */
+ if (crtc_state->uapi.async_flip) {
+ skl_program_async_surface_address(dev_priv, plane_state,
+ pipe, plane_id, surf_addr);
+ return;
+ }
I'd suggest adding a vfunc for this. Should be able to call it from
intel_update_plane(). That way we don't need to patch it into each
and every .update_plane() implementation.
Sure. I will add a vfunc for this in intel_plane and call it directly
from intel_update_plane()
Thanks,
Karthik.B.S
+
plane_ctl |= skl_plane_ctl_crtc(crtc_state);
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
--
2.22.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 35/39] drm/i915: Encode fence specific waitqueue behaviour into the wait.flags
Hi, Chris,
On 8/26/20 3:28 PM, Chris Wilson wrote:
Use the wait_queue_entry.flags to denote the special fence behaviour
(flattening continuations along fence chains, and for propagating
errors) rather than trying to detect ordinary waiters by their
functions.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_sw_fence.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c
b/drivers/gpu/drm/i915/i915_sw_fence.c
index 4cd2038cbe35..4e557d1c4644 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -18,10 +18,15 @@
#define I915_SW_FENCE_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
#endif
-#define I915_SW_FENCE_FLAG_ALLOC BIT(3) /* after WQ_FLAG_* for safety */
-
static DEFINE_SPINLOCK(i915_sw_fence_lock);
+#define WQ_FLAG_BITS \
+ BITS_PER_TYPE(typeof_member(struct wait_queue_entry, flags))
+
+/* after WQ_FLAG_* for safety */
+#define I915_SW_FENCE_FLAG_FENCE BIT(WQ_FLAG_BITS - 1)
+#define I915_SW_FENCE_FLAG_ALLOC BIT(WQ_FLAG_BITS - 2)
Not sure if sharing the flags field with the wait.c implementation is
fully OK either. Is the @key parameter to the wake function useable? I
mean rather than passing just a list head could we pass something like
struct i915_sw_fence_key {
bool no_recursion; /* Makes the wait function just put its entry on
@continuation and return */
int error;
struct list_head continuation;
}
/Thomas
+
enum {
DEBUG_FENCE_IDLE = 0,
DEBUG_FENCE_NOTIFY,
@@ -154,10 +159,10 @@ static void __i915_sw_fence_wake_up_all(struct
i915_sw_fence *fence,
spin_lock_irqsave_nested(&x->lock, flags, 1 + !!continuation);
if (continuation) {
list_for_each_entry_safe(pos, next, &x->head, entry) {
- if (pos->func == autoremove_wake_function)
- pos->func(pos, TASK_NORMAL, 0, continuation);
- else
+ if (pos->flags & I915_SW_FENCE_FLAG_FENCE)
list_move_tail(&pos->entry, continuation);
+ else
+ pos->func(pos, TASK_NORMAL, 0, continuation);
}
} else {
LIST_HEAD(extra);
@@ -166,9 +171,9 @@ static void __i915_sw_fence_wake_up_all(struct
i915_sw_fence *fence,
list_for_each_entry_safe(pos, next, &x->head, entry) {
int wake_flags;
-wake_flags = fence->error;
- if (pos->func == autoremove_wake_function)
- wake_flags = 0;
+ wake_flags = 0;
+ if (pos->flags & I915_SW_FENCE_FLAG_FENCE)
+ wake_flags = fence->error;
pos->func(pos, TASK_NORMAL, wake_flags, &extra);
}
@@ -332,8 +337,8 @@ static int __i915_sw_fence_await_sw_fence(struct
i915_sw_fence *fence,
struct i915_sw_fence *signaler,
wait_queue_entry_t *wq, gfp_t gfp)
{
+ unsigned int pending;
unsigned long flags;
- int pending;
debug_fence_assert(fence);
might_sleep_if(gfpflags_allow_blocking(gfp));
@@ -349,7 +354,7 @@ static int __i915_sw_fence_await_sw_fence(struct
i915_sw_fence *fence,
if (unlikely(i915_sw_fence_check_if_after(fence, signaler)))
return -EINVAL;
- pending = 0;
+ pending = I915_SW_FENCE_FLAG_FENCE;
if (!wq) {
wq = kmalloc(sizeof(*wq), gfp);
if (!wq) {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 1/7] drm/i915: Add enable/disable flip done and flip done handler
On 9/1/2020 5:10 PM, Ville Syrjälä wrote:
On Fri, Aug 07, 2020 at 03:05:45PM +0530, Karthik B S wrote:
Add enable/disable flip done functions and the flip done handler
function which handles the flip done interrupt.
Enable the flip done interrupt in IER.
Enable flip done function is called before writing the
surface address register as the write to this register triggers
the flip done interrupt
Flip done handler is used to send the page flip event as soon as the
surface address is written as per the requirement of async flips.
The interrupt is disabled after the event is sent.
v2: -Change function name from icl_* to skl_* (Paulo)
-Move flip handler to this patch (Paulo)
-Remove vblank_put() (Paulo)
-Enable flip done interrupt for gen9+ only (Paulo)
-Enable flip done interrupt in power_well_post_enable hook (Paulo)
-Removed the event check in flip done handler to handle async
flips without pageflip events.
v3: -Move skl_disable_flip_done out of interrupt handler (Paulo)
-Make the pending vblank event NULL in the beginning of
flip_done_handler to remove sporadic WARN_ON that is seen.
v4: -Calculate timestamps using flip done time stamp and current
timestamp for async flips (Ville)
v5: -Fix the sparse warning by making the function 'g4x_get_flip_counter'
static.(Reported-by: kernel test robot )
-Fix the typo in commit message.
v6: -Revert back to old time stamping code.
-Remove the break while calling skl_enable_flip_done. (Paulo)
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_display.c | 8 +++
drivers/gpu/drm/i915/i915_irq.c | 52
drivers/gpu/drm/i915/i915_irq.h | 2 +
3 files changed, 62 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 522c772a2111..1ac2e6f27597 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15562,6 +15562,11 @@ static void intel_atomic_commit_tail(struct
intel_atomic_state *state)
intel_dbuf_pre_plane_update(state);
+ for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+ if (new_crtc_state->uapi.async_flip)
+ skl_enable_flip_done(&crtc->base);
+ }
+
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
dev_priv->display.commit_modeset_enables(state);
@@ -15583,6 +15588,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
drm_atomic_helper_wait_for_flip_done(dev, &state->base);
for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+ if (new_crtc_state->uapi.async_flip)
+ skl_disable_flip_done(&crtc->base);
+
if (new_crtc_state->hw.active &&
!needs_modeset(new_crtc_state) &&
!new_crtc_state->preload_luts &&
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f113fe44572b..6cc129b031d3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1296,6 +1296,23 @@ display_pipe_crc_irq_handler(struct drm_i915_private
*dev_priv,
u32 crc4) {}
#endif
+static void flip_done_handler(struct drm_i915_private *dev_priv,
+ unsigned int pipe)
+{
+ struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+ struct drm_crtc_state *crtc_state = crtc->base.state;
+ struct drm_pending_vblank_event *e = crtc_state->event;
+ struct drm_device *dev = &dev_priv->drm;
+ unsigned long irqflags;
+
+ spin_lock_irqsave(&dev->event_lock, irqflags);
+
+ crtc_state->event = NULL;
+
+ drm_crtc_send_vblank_event(&crtc->base, e);
The timestamp is going to be wrong here. We should perhaps just sample
the current time+frame counter here.
Thanks for the review.
Tried updating the proper timestamp in the previous version(V5) of the
series by using flip timestamp. Feeback received was that userspace
doesn't use this.Also trying to align to the current AMD implementation,
where the timestamp corresponds to the previous vblank timestamp.
In V5, we had tried by updating the sequence(by using flip counter), but
the feedback received was that the sequence shouldn't change and it
should always reflect the frame counter.Thus, in our current
implementation we do not make any changes to the sequence.So, even if
there is any update in time stamp it doesn't get reflected, even after
calling drm_update_vblank_count.
As userspace is not interested in this (as per the previous mail
discussions, I've updated the kernel documentation also with this
detail), should we implement this?
Thanks,
Karthik.B.S
+
+ spin_unlock_irqrestore(&dev->event_lock, irqflags);
+}
stati
[Intel-gfx] [PATCH 1/4] drm/i915: split intel_modeset_init() pre/post gem init
Turn current intel_modeset_init() to a pre-gem init function, and add a
new intel_modeset_init() function and move all post-gem modeset init
there, in the correct layer. No functional changes.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/display/intel_display.c | 28 ++--
drivers/gpu/drm/i915/display/intel_display.h | 1 +
drivers/gpu/drm/i915/i915_drv.c | 17 ++--
3 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index dc622af8695c..1279a50ade35 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -17909,8 +17909,8 @@ int intel_modeset_init_noirq(struct drm_i915_private
*i915)
return 0;
}
-/* part #2: call after irq install */
-int intel_modeset_init(struct drm_i915_private *i915)
+/* part #2: call after irq install, but before gem init */
+int intel_modeset_init_nogem(struct drm_i915_private *i915)
{
struct drm_device *dev = &i915->drm;
enum pipe pipe;
@@ -18009,6 +18009,30 @@ int intel_modeset_init(struct drm_i915_private *i915)
return 0;
}
+/* part #3: call after gem init */
+int intel_modeset_init(struct drm_i915_private *i915)
+{
+ int ret;
+
+ intel_overlay_setup(i915);
+
+ if (!HAS_DISPLAY(i915) || !INTEL_DISPLAY_ENABLED(i915))
+ return 0;
+
+ ret = intel_fbdev_init(&i915->drm);
+ if (ret)
+ return ret;
+
+ /* Only enable hotplug handling once the fbdev is fully set up. */
+ intel_hpd_init(i915);
+
+ intel_init_ipc(i915);
+
+ intel_psr_set_force_mode_changed(i915->psr.dp);
+
+ return 0;
+}
+
void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
{
struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h
b/drivers/gpu/drm/i915/display/intel_display.h
index e890c8fb779b..63130e1a9eff 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -629,6 +629,7 @@ intel_format_info_is_yuv_semiplanar(const struct
drm_format_info *info,
/* modesetting */
void intel_modeset_init_hw(struct drm_i915_private *i915);
int intel_modeset_init_noirq(struct drm_i915_private *i915);
+int intel_modeset_init_nogem(struct drm_i915_private *i915);
int intel_modeset_init(struct drm_i915_private *i915);
void intel_modeset_driver_remove(struct drm_i915_private *i915);
void intel_modeset_driver_remove_noirq(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 00292a849c34..70632119c8a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -58,7 +58,6 @@
#include "display/intel_hotplug.h"
#include "display/intel_overlay.h"
#include "display/intel_pipe_crc.h"
-#include "display/intel_psr.h"
#include "display/intel_sprite.h"
#include "display/intel_vga.h"
@@ -263,7 +262,7 @@ static int i915_driver_modeset_probe(struct
drm_i915_private *i915)
/* Important: The output setup functions called by modeset_init need
* working irqs for e.g. gmbus and dp aux transfers. */
- ret = intel_modeset_init(i915);
+ ret = intel_modeset_init_nogem(i915);
if (ret)
goto out;
@@ -271,22 +270,10 @@ static int i915_driver_modeset_probe(struct
drm_i915_private *i915)
if (ret)
goto cleanup_modeset;
- intel_overlay_setup(i915);
-
- if (!HAS_DISPLAY(i915) || !INTEL_DISPLAY_ENABLED(i915))
- return 0;
-
- ret = intel_fbdev_init(&i915->drm);
+ ret = intel_modeset_init(i915);
if (ret)
goto cleanup_gem;
- /* Only enable hotplug handling once the fbdev is fully set up. */
- intel_hpd_init(i915);
-
- intel_init_ipc(i915);
-
- intel_psr_set_force_mode_changed(i915->psr.dp);
-
return 0;
cleanup_gem:
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] drm/i915: modeset probe cleanup
Try to cleanup modeset probe and driver remove paths. Based on old patches, removed old reviewed-by's due to changes. BR, Jani. Jani Nikula (4): drm/i915: split intel_modeset_init() pre/post gem init drm/i915: move more display related probe to intel_modeset_init_noirq() drm/i915: split out intel_modeset_driver_remove_nogem() and simplify drm/i915: remove the extra modeset init layer drivers/gpu/drm/i915/display/intel_display.c | 77 ++- drivers/gpu/drm/i915/display/intel_display.h | 2 + drivers/gpu/drm/i915/i915_drv.c | 134 --- 3 files changed, 97 insertions(+), 116 deletions(-) -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: split out intel_modeset_driver_remove_nogem() and simplify
Split out a separate display function for driver remove after gem
deinitialization. Note that the sequence is not symmetric with
init. However use similar naming as that reflects the deinit sequence.
No functional changes.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/display/intel_display.c | 12 ++
drivers/gpu/drm/i915/display/intel_display.h | 1 +
drivers/gpu/drm/i915/i915_drv.c | 24 +++-
3 files changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 79c74b072a15..81ea609a5fa0 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -18948,6 +18948,18 @@ void intel_modeset_driver_remove_noirq(struct
drm_i915_private *i915)
intel_fbc_cleanup_cfb(i915);
}
+/* part #3: call after gem init */
+void intel_modeset_driver_remove_nogem(struct drm_i915_private *i915)
+{
+ intel_csr_ucode_fini(i915);
+
+ intel_power_domains_driver_remove(i915);
+
+ intel_vga_unregister(i915);
+
+ intel_bios_driver_remove(i915);
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
struct intel_display_error_state {
diff --git a/drivers/gpu/drm/i915/display/intel_display.h
b/drivers/gpu/drm/i915/display/intel_display.h
index 63130e1a9eff..3670cabeb3cd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -633,6 +633,7 @@ int intel_modeset_init_nogem(struct drm_i915_private *i915);
int intel_modeset_init(struct drm_i915_private *i915);
void intel_modeset_driver_remove(struct drm_i915_private *i915);
void intel_modeset_driver_remove_noirq(struct drm_i915_private *i915);
+void intel_modeset_driver_remove_nogem(struct drm_i915_private *i915);
void intel_display_resume(struct drm_device *dev);
void intel_init_pch_refclk(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6cf1330d3b8e..e332b6fd701d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -255,24 +255,6 @@ static int i915_driver_modeset_probe(struct
drm_i915_private *i915)
return ret;
}
-/* part #1: call before irq uninstall */
-static void i915_driver_modeset_remove(struct drm_i915_private *i915)
-{
- intel_modeset_driver_remove(i915);
-}
-
-/* part #2: call after irq uninstall */
-static void i915_driver_modeset_remove_noirq(struct drm_i915_private *i915)
-{
- intel_csr_ucode_fini(i915);
-
- intel_power_domains_driver_remove(i915);
-
- intel_vga_unregister(i915);
-
- intel_bios_driver_remove(i915);
-}
-
static void intel_init_dpio(struct drm_i915_private *dev_priv)
{
/*
@@ -966,7 +948,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
out_cleanup_irq:
intel_irq_uninstall(i915);
out_cleanup_modeset:
- i915_driver_modeset_remove_noirq(i915);
+ intel_modeset_driver_remove_nogem(i915);
out_cleanup_hw:
i915_driver_hw_remove(i915);
intel_memory_regions_driver_release(i915);
@@ -998,7 +980,7 @@ void i915_driver_remove(struct drm_i915_private *i915)
intel_gvt_driver_remove(i915);
- i915_driver_modeset_remove(i915);
+ intel_modeset_driver_remove(i915);
intel_irq_uninstall(i915);
@@ -1007,7 +989,7 @@ void i915_driver_remove(struct drm_i915_private *i915)
i915_reset_error_state(i915);
i915_gem_driver_remove(i915);
- i915_driver_modeset_remove_noirq(i915);
+ intel_modeset_driver_remove_nogem(i915);
i915_driver_hw_remove(i915);
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: remove the extra modeset init layer
Streamline the modeset init by removing the extra init layer.
No functional changes, which means the cleanup path looks hideous.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.c | 63 +++--
1 file changed, 20 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e332b6fd701d..4d9b61b1a115 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -215,46 +215,6 @@ intel_teardown_mchbar(struct drm_i915_private *dev_priv)
release_resource(&dev_priv->mch_res);
}
-/* part #1: call before irq install */
-static int i915_driver_modeset_probe_noirq(struct drm_i915_private *i915)
-{
- return intel_modeset_init_noirq(i915);
-}
-
-/* part #2: call after irq install */
-static int i915_driver_modeset_probe(struct drm_i915_private *i915)
-{
- int ret;
-
- /* Important: The output setup functions called by modeset_init need
-* working irqs for e.g. gmbus and dp aux transfers. */
- ret = intel_modeset_init_nogem(i915);
- if (ret)
- goto out;
-
- ret = i915_gem_init(i915);
- if (ret)
- goto cleanup_modeset;
-
- ret = intel_modeset_init(i915);
- if (ret)
- goto cleanup_gem;
-
- return 0;
-
-cleanup_gem:
- i915_gem_suspend(i915);
- i915_gem_driver_remove(i915);
- i915_gem_driver_release(i915);
-cleanup_modeset:
- /* FIXME */
- intel_modeset_driver_remove(i915);
- intel_irq_uninstall(i915);
- intel_modeset_driver_remove_noirq(i915);
-out:
- return ret;
-}
-
static void intel_init_dpio(struct drm_i915_private *dev_priv)
{
/*
@@ -923,7 +883,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
if (ret < 0)
goto out_cleanup_mmio;
- ret = i915_driver_modeset_probe_noirq(i915);
+ ret = intel_modeset_init_noirq(i915);
if (ret < 0)
goto out_cleanup_hw;
@@ -931,10 +891,18 @@ int i915_driver_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
if (ret)
goto out_cleanup_modeset;
- ret = i915_driver_modeset_probe(i915);
- if (ret < 0)
+ ret = intel_modeset_init_nogem(i915);
+ if (ret)
goto out_cleanup_irq;
+ ret = i915_gem_init(i915);
+ if (ret)
+ goto out_cleanup_modeset2;
+
+ ret = intel_modeset_init(i915);
+ if (ret)
+ goto out_cleanup_gem;
+
i915_driver_register(i915);
enable_rpm_wakeref_asserts(&i915->runtime_pm);
@@ -945,6 +913,15 @@ int i915_driver_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
return 0;
+out_cleanup_gem:
+ i915_gem_suspend(i915);
+ i915_gem_driver_remove(i915);
+ i915_gem_driver_release(i915);
+out_cleanup_modeset2:
+ intel_modeset_driver_remove(i915);
+ intel_irq_uninstall(i915);
+ intel_modeset_driver_remove_noirq(i915);
+ goto out_cleanup_modeset;
out_cleanup_irq:
intel_irq_uninstall(i915);
out_cleanup_modeset:
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: move more display related probe to intel_modeset_init_noirq()
With the intel_modeset_* probe functions clarified, we can continue with
moving more related calls to the right layer:
- drm_vblank_init()
- intel_bios_init()
- intel_vga_register()
- intel_csr_ucode_init()
Unfortunately, for the time being, we also need to move a call to the
*wrong* layer: the power domain init.
No functional changes.
v2: move probe failure while at it, power domain init
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/display/intel_display.c | 37 ++--
drivers/gpu/drm/i915/i915_drv.c | 36 +--
2 files changed, 35 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 1279a50ade35..79c74b072a15 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -67,6 +67,7 @@
#include "intel_bw.h"
#include "intel_cdclk.h"
#include "intel_color.h"
+#include "intel_csr.h"
#include "intel_display_types.h"
#include "intel_dp_link_training.h"
#include "intel_fbc.h"
@@ -17880,6 +17881,27 @@ int intel_modeset_init_noirq(struct drm_i915_private
*i915)
{
int ret;
+ if (i915_inject_probe_failure(i915))
+ return -ENODEV;
+
+ if (HAS_DISPLAY(i915) && INTEL_DISPLAY_ENABLED(i915)) {
+ ret = drm_vblank_init(&i915->drm,
+ INTEL_NUM_PIPES(i915));
+ if (ret)
+ return ret;
+ }
+
+ intel_bios_init(i915);
+
+ ret = intel_vga_register(i915);
+ if (ret)
+ goto cleanup_bios;
+
+ /* FIXME: completely on the wrong abstraction layer */
+ intel_power_domains_init_hw(i915, false);
+
+ intel_csr_ucode_init(i915);
+
i915->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0);
i915->flip_wq = alloc_workqueue("i915_flip", WQ_HIGHPRI |
WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
@@ -17888,15 +17910,15 @@ int intel_modeset_init_noirq(struct drm_i915_private
*i915)
ret = intel_cdclk_init(i915);
if (ret)
- return ret;
+ goto cleanup_vga_client_pw_domain_csr;
ret = intel_dbuf_init(i915);
if (ret)
- return ret;
+ goto cleanup_vga_client_pw_domain_csr;
ret = intel_bw_init(i915);
if (ret)
- return ret;
+ goto cleanup_vga_client_pw_domain_csr;
init_llist_head(&i915->atomic_helper.free_list);
INIT_WORK(&i915->atomic_helper.free_work,
@@ -17907,6 +17929,15 @@ int intel_modeset_init_noirq(struct drm_i915_private
*i915)
intel_fbc_init(i915);
return 0;
+
+cleanup_vga_client_pw_domain_csr:
+ intel_csr_ucode_fini(i915);
+ intel_power_domains_driver_remove(i915);
+ intel_vga_unregister(i915);
+cleanup_bios:
+ intel_bios_driver_remove(i915);
+
+ return ret;
}
/* part #2: call after irq install, but before gem init */
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 70632119c8a6..6cf1330d3b8e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -218,41 +218,7 @@ intel_teardown_mchbar(struct drm_i915_private *dev_priv)
/* part #1: call before irq install */
static int i915_driver_modeset_probe_noirq(struct drm_i915_private *i915)
{
- int ret;
-
- if (i915_inject_probe_failure(i915))
- return -ENODEV;
-
- if (HAS_DISPLAY(i915) && INTEL_DISPLAY_ENABLED(i915)) {
- ret = drm_vblank_init(&i915->drm,
- INTEL_NUM_PIPES(i915));
- if (ret)
- return ret;
- }
-
- intel_bios_init(i915);
-
- ret = intel_vga_register(i915);
- if (ret)
- goto cleanup_bios;
-
- intel_power_domains_init_hw(i915, false);
-
- intel_csr_ucode_init(i915);
-
- ret = intel_modeset_init_noirq(i915);
- if (ret)
- goto cleanup_vga_client_pw_domain_csr;
-
- return 0;
-
-cleanup_vga_client_pw_domain_csr:
- intel_csr_ucode_fini(i915);
- intel_power_domains_driver_remove(i915);
- intel_vga_unregister(i915);
-cleanup_bios:
- intel_bios_driver_remove(i915);
- return ret;
+ return intel_modeset_init_noirq(i915);
}
/* part #2: call after irq install */
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: modeset probe cleanup
== Series Details == Series: drm/i915: modeset probe cleanup URL : https://patchwork.freedesktop.org/series/81267/ State : warning == Summary == $ dim sparse --fast origin/drm-tip Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. - +drivers/gpu/drm/i915/gt/intel_reset.c:1311:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block +drivers/gpu/drm/i915/gvt/mmio.c:287:23: warning: memcpy with byte count of 279040 +drivers/gpu/drm/i915/i915_perf.c:1440:15: warning: memset with byte count of 16777216 +drivers/gpu/drm/i915/i915_perf.c:1494:15: warning: memset with byte count of 16777216 +drivers/gpu/drm/i915/intel_wakeref.c:137:19: warning: context imbalance in 'wakeref_auto_timeout' - unexpected unlock +./include/linux/seqlock.h:752:24: warning: trying to copy expression type 31 +./include/linux/seqlock.h:778:16: warning: trying to copy expression type 31 +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lis
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: modeset probe cleanup
== Series Details == Series: drm/i915: modeset probe cleanup URL : https://patchwork.freedesktop.org/series/81267/ State : success == Summary == CI Bug Log - changes from CI_DRM_8955 -> Patchwork_18435 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/index.html Known issues Here are the changes found in Patchwork_18435 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_parallel@engines@contexts: - fi-cfl-8109u: [PASS][1] -> [INCOMPLETE][2] ([i915#2398]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8955/fi-cfl-8109u/igt@gem_exec_parallel@engi...@contexts.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/fi-cfl-8109u/igt@gem_exec_parallel@engi...@contexts.html * igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1: - fi-icl-u2: [PASS][3] -> [DMESG-WARN][4] ([i915#1982]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8955/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vbl...@c-edp1.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vbl...@c-edp1.html Possible fixes * igt@i915_module_load@reload: - fi-bxt-dsi: [DMESG-WARN][5] ([i915#1635] / [i915#1982]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8955/fi-bxt-dsi/igt@i915_module_l...@reload.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/fi-bxt-dsi/igt@i915_module_l...@reload.html - fi-tgl-u2: [TIMEOUT][7] ([i915#1418]) -> [PASS][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8955/fi-tgl-u2/igt@i915_module_l...@reload.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/fi-tgl-u2/igt@i915_module_l...@reload.html * igt@i915_pm_rpm@basic-pci-d3-state: - fi-byt-j1900: [DMESG-WARN][9] ([i915#1982]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8955/fi-byt-j1900/igt@i915_pm_...@basic-pci-d3-state.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/fi-byt-j1900/igt@i915_pm_...@basic-pci-d3-state.html - fi-bsw-kefka: [DMESG-WARN][11] ([i915#1982]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8955/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_pm_rpm@module-reload: - fi-tgl-u2: [SKIP][13] ([i915#579]) -> [PASS][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8955/fi-tgl-u2/igt@i915_pm_...@module-reload.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/fi-tgl-u2/igt@i915_pm_...@module-reload.html * igt@i915_selftest@live@execlists: - fi-icl-y: [INCOMPLETE][15] ([i915#2276]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8955/fi-icl-y/igt@i915_selftest@l...@execlists.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/fi-icl-y/igt@i915_selftest@l...@execlists.html [i915#1418]: https://gitlab.freedesktop.org/drm/intel/issues/1418 [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635 [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2276]: https://gitlab.freedesktop.org/drm/intel/issues/2276 [i915#2398]: https://gitlab.freedesktop.org/drm/intel/issues/2398 [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579 Participating hosts (38 -> 33) -- Missing(5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper Build changes - * Linux: CI_DRM_8955 -> Patchwork_18435 CI-20190529: 20190529 CI_DRM_8955: 744c2c7c60ac2fbe7fae0c25c01a451d41d207b2 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5776: 46e4315096bcaa2465c82c547274627365b1a69e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18435: 0ff068b7cefd2ce745f26f578b6a46bfada75358 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 0ff068b7cefd drm/i915: remove the extra modeset init layer 7cd1162fcd5f drm/i915: split out intel_modeset_driver_remove_nogem() and simplify c08fc8f7c7ed drm/i915: move more display related probe to intel_modeset_init_noirq() af603150eba7 drm/i915: split intel_modeset_init() pre/post gem init == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18435/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: remove the extra modeset init layer
On Wed, Sep 02, 2020 at 05:30:23PM +0300, Jani Nikula wrote:
> Streamline the modeset init by removing the extra init layer.
>
> No functional changes, which means the cleanup path looks hideous.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/i915_drv.c | 63 +++--
> 1 file changed, 20 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e332b6fd701d..4d9b61b1a115 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -215,46 +215,6 @@ intel_teardown_mchbar(struct drm_i915_private *dev_priv)
> release_resource(&dev_priv->mch_res);
> }
>
> -/* part #1: call before irq install */
> -static int i915_driver_modeset_probe_noirq(struct drm_i915_private *i915)
> -{
> - return intel_modeset_init_noirq(i915);
> -}
> -
> -/* part #2: call after irq install */
> -static int i915_driver_modeset_probe(struct drm_i915_private *i915)
> -{
> - int ret;
> -
> - /* Important: The output setup functions called by modeset_init need
> - * working irqs for e.g. gmbus and dp aux transfers. */
> - ret = intel_modeset_init_nogem(i915);
> - if (ret)
> - goto out;
> -
> - ret = i915_gem_init(i915);
> - if (ret)
> - goto cleanup_modeset;
> -
> - ret = intel_modeset_init(i915);
> - if (ret)
> - goto cleanup_gem;
> -
> - return 0;
> -
> -cleanup_gem:
> - i915_gem_suspend(i915);
> - i915_gem_driver_remove(i915);
> - i915_gem_driver_release(i915);
> -cleanup_modeset:
> - /* FIXME */
> - intel_modeset_driver_remove(i915);
> - intel_irq_uninstall(i915);
> - intel_modeset_driver_remove_noirq(i915);
> -out:
> - return ret;
> -}
> -
> static void intel_init_dpio(struct drm_i915_private *dev_priv)
> {
> /*
> @@ -923,7 +883,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> if (ret < 0)
> goto out_cleanup_mmio;
>
> - ret = i915_driver_modeset_probe_noirq(i915);
> + ret = intel_modeset_init_noirq(i915);
> if (ret < 0)
> goto out_cleanup_hw;
>
> @@ -931,10 +891,18 @@ int i915_driver_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
> if (ret)
> goto out_cleanup_modeset;
>
> - ret = i915_driver_modeset_probe(i915);
> - if (ret < 0)
> + ret = intel_modeset_init_nogem(i915);
> + if (ret)
> goto out_cleanup_irq;
>
> + ret = i915_gem_init(i915);
> + if (ret)
> + goto out_cleanup_modeset2;
> +
> + ret = intel_modeset_init(i915);
> + if (ret)
> + goto out_cleanup_gem;
> +
> i915_driver_register(i915);
>
> enable_rpm_wakeref_asserts(&i915->runtime_pm);
> @@ -945,6 +913,15 @@ int i915_driver_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
>
> return 0;
>
> +out_cleanup_gem:
> + i915_gem_suspend(i915);
> + i915_gem_driver_remove(i915);
> + i915_gem_driver_release(i915);
> +out_cleanup_modeset2:
> + intel_modeset_driver_remove(i915);
> + intel_irq_uninstall(i915);
> + intel_modeset_driver_remove_noirq(i915);
> + goto out_cleanup_modeset;
Looks like we used to do the intel_irq_uninstall() twice? We even
have a FIXME in there stating as much. With this goto we only do
it the once I guess. So seems like a slight change in behaviour.
Though the comment says it gets called twice during driver remove
as well, which does not seem to be true (at least anymore).
Anyways, fixing that properly likely requires some axctual thought
wrt. hpd vs. irq vs. other stuff.
Series is
Reviewed-by: Ville Syrjälä
> out_cleanup_irq:
> intel_irq_uninstall(i915);
> out_cleanup_modeset:
> --
> 2.20.1
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: fix regression leading to display audio probe failure on GLK
== Series Details == Series: drm/i915: fix regression leading to display audio probe failure on GLK URL : https://patchwork.freedesktop.org/series/81227/ State : success == Summary == CI Bug Log - changes from CI_DRM_8951_full -> Patchwork_18429_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_18429_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_busy@close-race: - shard-skl: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) +10 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl9/igt@gem_b...@close-race.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl6/igt@gem_b...@close-race.html * igt@gem_exec_nop@basic-sequential: - shard-kbl: [PASS][3] -> [TIMEOUT][4] ([i915#1958]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-kbl7/igt@gem_exec_...@basic-sequential.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-kbl4/igt@gem_exec_...@basic-sequential.html * igt@gem_exec_reloc@basic-many-active@vcs0: - shard-glk: [PASS][5] -> [FAIL][6] ([i915#2389]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk8/igt@gem_exec_reloc@basic-many-act...@vcs0.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk9/igt@gem_exec_reloc@basic-many-act...@vcs0.html * igt@gem_exec_whisper@basic-contexts-forked: - shard-glk: [PASS][7] -> [DMESG-WARN][8] ([i915#118] / [i915#95]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk4/igt@gem_exec_whis...@basic-contexts-forked.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk4/igt@gem_exec_whis...@basic-contexts-forked.html * igt@gem_exec_whisper@basic-fds-forked: - shard-apl: [PASS][9] -> [TIMEOUT][10] ([i915#1635] / [i915#1958]) +1 similar issue [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-apl3/igt@gem_exec_whis...@basic-fds-forked.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-apl4/igt@gem_exec_whis...@basic-fds-forked.html * igt@gem_exec_whisper@basic-forked: - shard-iclb: [PASS][11] -> [TIMEOUT][12] ([i915#1958]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-iclb8/igt@gem_exec_whis...@basic-forked.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-iclb1/igt@gem_exec_whis...@basic-forked.html - shard-skl: [PASS][13] -> [TIMEOUT][14] ([i915#1958]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl9/igt@gem_exec_whis...@basic-forked.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl6/igt@gem_exec_whis...@basic-forked.html - shard-tglb: [PASS][15] -> [TIMEOUT][16] ([i915#1958]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-tglb5/igt@gem_exec_whis...@basic-forked.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-tglb7/igt@gem_exec_whis...@basic-forked.html * igt@gem_exec_whisper@basic-queues-forked-all: - shard-glk: [PASS][17] -> [TIMEOUT][18] ([i915#1958]) +3 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk5/igt@gem_exec_whis...@basic-queues-forked-all.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk8/igt@gem_exec_whis...@basic-queues-forked-all.html * igt@gem_sync@basic-store-all: - shard-glk: [PASS][19] -> [FAIL][20] ([i915#2356]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk7/igt@gem_s...@basic-store-all.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk1/igt@gem_s...@basic-store-all.html - shard-tglb: [PASS][21] -> [FAIL][22] ([i915#2356]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-tglb8/igt@gem_s...@basic-store-all.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-tglb5/igt@gem_s...@basic-store-all.html * igt@kms_flip@2x-plain-flip-ts-check@bc-hdmi-a1-hdmi-a2: - shard-glk: [PASS][23] -> [FAIL][24] ([i915#2122]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk8/igt@kms_flip@2x-plain-flip-ts-ch...@bc-hdmi-a1-hdmi-a2.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk2/igt@kms_flip@2x-plain-flip-ts-ch...@bc-hdmi-a1-hdmi-a2.html * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a1: - shard-glk: [PASS][25] -> [FAIL][26] ([i915#79]) [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interrupti...@c-hdmi-a1.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk1/igt@kms_flip@flip-vs-expi
Re: [Intel-gfx] Various problems for the Xen for XenGT code and guide.
Why not new XenGT for new GPU? Intel migrated to the KVM? On Wednesday, September 2, 2020, 05:19:18 PM GMT+4:30, Lv, Zhiyuan wrote: Hi, It is mainly due to the business priority change. XenGT project was originally created for data center usages with XEON E3 servers which have integrated processor graphics. After SkyLake E3, there are no new servers capable of running GVT-g, and Intel future graphics for data center will have different approaches for GPU sharing. Another reason is the XenGT upstream difficulty. Different from KVMGT which has been fully merged to upstream, Xen part of GVT-g still has technical opens that are hard to close quickly. Sorry that we did not sync up with community in time the XenGT ramping down plan. Internally we have been testing XenGT until November 2019. We will update our setup guide for XenGT part to reflect the information. Going forward we will try our best to do XenGT consultant, but will not be able to do code rebase/test or debugging. Meanwhile, we are still maintaining KVMGT mainly for client integrated GPU usages. We will continue to fix issues that can be reproduced with KVMGT. https://github.com/intel/gvt-linux/tree/topic/gvt-xengt https://github.com/intel/Igvtg-xen/tree/xengt-stable-4.10 Hi Marietto, we appreciate your efforts trying GVT-g (XenGT). Hope Terrence's reply helps. By switching back to old gcc those compile errors should be gone. Meanwhile please be kindly noticed that we can only provide limited support on XenGT with old versions. Thanks! Regards, -Zhiyuan -Original Message- From: Jason Long Date: Wednesday, September 2, 2020 at 1:07 AM To: Mario Marietto , "igv...@lists.01.org" , "xen-de...@lists.xenproject.org" , "xen-de...@lists.xen.org" , "intel-gfx@lists.freedesktop.org" , "linux-ker...@vger.kernel.org" , Susie Li , "Tian, Kevin" , Zhiyuan Lv , "Li, Weinan Z" , "Downs, Mike" , "Xu, Terrence" Subject: Re: Various problems for the Xen for XenGT code and guide. Hello, Why XenGT doesn't have any new version? On Tuesday, September 1, 2020, 09:21:27 PM GMT+4:30, Xu, Terrence wrote: Hi Mario, Sorry to make you feel uncomfortable. I think it is not setup guide problem, the main reason is the Xen code is very old (We are upgrading GVT-g code on Linux kernel side and we haven’t upgraded the Xen and Qemu source for XenGT for at least 2 years) but your GCC is new (You are using Ubuntu 20.4, the gcc version is 9+). I have a way to workaround it, as below: 1. apt-get install gcc-72. ln -fs gcc-7 /usr/bin/gcc Any more problem just let us know! Thanks Terrence From: Mario Marietto Sent: Thursday, August 27, 2020 9:52 PM To: Xu, Terrence ; igv...@lists.01.org; xen-de...@lists.xenproject.org; xen-de...@lists.xen.org; intel-gfx@lists.freedesktop.org; linux-ker...@vger.kernel.org; Li, Susie ; Tian, Kevin ; Lv, Zhiyuan ; Li, Weinan Z ; Downs, Mike Subject: Various problems for the Xen for XenGT code and guide. Hello. I would like to pass the integrated gpu from the host os (ubuntu 20.04) to the windows 10 guest os with xen. This is because xen works great for me,better than qemu-kvm for my specific needs and because I have only two graphic cards. The nvidia rtx 2080 ti that I have already passed to the guest,and the intel UHD 630,that can be duplicated from the host to the guest so that it can be used in both places without interruptions. So I'm trying to build this repository : https://github.com/intel/gvt-linux/wiki/GVTg_Setup_Guide#332-build-qemu--xen-for-xengt I have to say that this guide is totally not very well written. And the code is full of unpatched bugs. It's a month that I'm working on that,trying to fix the bugs that are came out from the 2015 until today. This is not my job. This is my hobby. But,I need to activate the pass through for my integrated GPU so I don't to give up. I'm also very angry with those coders who do not do their job well and with those coders who do not respond to help messages. It is not enough to write good code to be a good programmer. It is also important to keep the documentation updated, to help those who cannot get the code to work. Anyway,I've documented every step that I did to make it work here : https://github.com/intel/gvt-linux/issues/168 right now I'm trying to fix the bug n. 434544,that you can see below. CC util/qemu-error.o/etc/xen/igvtg-xen/tools/qemu-xen-dir/util/qemu-error.c: In function ‘vreport’:/etc/xen/igvtg-xen/tools/qemu-xen-dir/util/qemu-error.c:201:5: error: ‘GTimeVal’ is deprecated: Use 'GDateTime' instead [-Werror=deprecated-declarations]201 | GTimeVal tv;| ^~~~In file included from /usr/include/glib-2.0/glib/galloca.h:32,from /usr/include/glib-2.0/glib.h:30,from /etc/xen/igvtg-xen/tools/qemu-xen-dir/include/glib-compat.h:19,from /etc/xen/igvtg-xen/tools/qemu-xen-dir/include/qemu/osdep.h:107,from /etc/xen/igvtg-xen/tools/qemu-xen-dir/util/qemu-error.
Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
> -Original Message-
> From: Rodrigo Vivi
> Sent: Tuesday, September 1, 2020 12:30 PM
> To: Srivatsa, Anusha
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
> lookup
>
> On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote:
> > We currenty check for platform at multiple parts in the driver to grab
> > the correct PLL. Let us begin to centralize it through a helper
> > function.
> >
> > v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)
> >
> > Suggested-by: Matt Roper
> > Cc: Ville Syrjälä
> > Cc: Matt Roper
> > Signed-off-by: Anusha Srivatsa
> > ---
> > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25
> > +++
> > 1 file changed, 15 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index c9013f8f766f..7440836c5e44 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private
> *dev_priv,
> > pll->info->name, onoff(state), onoff(cur_state)); }
> >
> > +static
> > +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private
> *dev_priv,
> > + struct intel_shared_dpll *pll) {
> > +
> > + if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id ==
> DPLL_ID_EHL_DPLL4))
> > + return MG_PLL_ENABLE(0);
> > +
> > + return CNL_DPLL_ENABLE(pll->info->id);
> > +
> > +
> > +}
> > /**
> > * intel_prepare_shared_dpll - call a dpll's prepare hook
> > * @crtc_state: CRTC, and its state, which has a shared dpll @@
> > -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct
> drm_i915_private *dev_priv,
> >struct intel_shared_dpll *pll,
> >struct intel_dpll_hw_state *hw_state) {
> > - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > -
> > - if (IS_ELKHARTLAKE(dev_priv) &&
> > - pll->info->id == DPLL_ID_EHL_DPLL4) {
> > - enable_reg = MG_PLL_ENABLE(0);
> > - }
> > + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >
> > return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg); }
> > @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct
> > drm_i915_private *dev_priv, static void combo_pll_enable(struct
> drm_i915_private *dev_priv,
> > struct intel_shared_dpll *pll) {
> > - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >
> > if (IS_ELKHARTLAKE(dev_priv) &&
> > pll->info->id == DPLL_ID_EHL_DPLL4) {
>
> there's probably something else that we can do now with the
> power_{put,get} to get rid of the, now, doubled if checks...
Don't follow you here Rodrigo. Are you suggesting using power_{put/get} to
somehow get rid of doubled if?
> > - enable_reg = MG_PLL_ENABLE(0);
> >
> > /*
> > * We need to disable DC states when this DPLL is enabled.
> > @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct
> > drm_i915_private *dev_priv, static void combo_pll_disable(struct
> drm_i915_private *dev_priv,
> > struct intel_shared_dpll *pll) {
> > - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
> >
> > if (IS_ELKHARTLAKE(dev_priv) &&
> > pll->info->id == DPLL_ID_EHL_DPLL4) {
> > - enable_reg = MG_PLL_ENABLE(0);
> > icl_pll_disable(dev_priv, pll, enable_reg);
>
> but here, at least, let's clean this function now...
> move this call above and out of the if and delete the one below and keep
> just the power_put inside the if...
Good change. Thanks!
Will change that.
Anusha
> >
> > intel_display_power_put(dev_priv,
> POWER_DOMAIN_DPLL_DC_OFF,
> > --
> > 2.25.0
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register lookup
> On Sep 2, 2020, at 12:30 PM, Srivatsa, Anusha
> wrote:
>
>
>
>> -Original Message-
>> From: Rodrigo Vivi
>> Sent: Tuesday, September 1, 2020 12:30 PM
>> To: Srivatsa, Anusha
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
>> lookup
>>
>> On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa wrote:
>>> We currenty check for platform at multiple parts in the driver to grab
>>> the correct PLL. Let us begin to centralize it through a helper
>>> function.
>>>
>>> v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville)
>>>
>>> Suggested-by: Matt Roper
>>> Cc: Ville Syrjälä
>>> Cc: Matt Roper
>>> Signed-off-by: Anusha Srivatsa
>>> ---
>>> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25
>>> +++
>>> 1 file changed, 15 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>> index c9013f8f766f..7440836c5e44 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>>> @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private
>> *dev_priv,
>>> pll->info->name, onoff(state), onoff(cur_state)); }
>>>
>>> +static
>>> +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private
>> *dev_priv,
>>> + struct intel_shared_dpll *pll) {
>>> +
>>> + if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id ==
>> DPLL_ID_EHL_DPLL4))
>>> + return MG_PLL_ENABLE(0);
>>> +
>>> + return CNL_DPLL_ENABLE(pll->info->id);
>>> +
>>> +
>>> +}
>>> /**
>>> * intel_prepare_shared_dpll - call a dpll's prepare hook
>>> * @crtc_state: CRTC, and its state, which has a shared dpll @@
>>> -3842,12 +3854,7 @@ static bool combo_pll_get_hw_state(struct
>> drm_i915_private *dev_priv,
>>>struct intel_shared_dpll *pll,
>>>struct intel_dpll_hw_state *hw_state) {
>>> - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>>> -
>>> - if (IS_ELKHARTLAKE(dev_priv) &&
>>> - pll->info->id == DPLL_ID_EHL_DPLL4) {
>>> - enable_reg = MG_PLL_ENABLE(0);
>>> - }
>>> + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>>>
>>> return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg); }
>>> @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct
>>> drm_i915_private *dev_priv, static void combo_pll_enable(struct
>> drm_i915_private *dev_priv,
>>> struct intel_shared_dpll *pll) {
>>> - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>>> + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>>>
>>> if (IS_ELKHARTLAKE(dev_priv) &&
>>> pll->info->id == DPLL_ID_EHL_DPLL4) {
>>
>> there's probably something else that we can do now with the
>> power_{put,get} to get rid of the, now, doubled if checks...
>
> Don't follow you here Rodrigo.
me neither ;)
I'm just brainstorming... thinking out lout.
> Are you suggesting using power_{put/get} to somehow get rid of doubled if?
after this patch, on this path we will do this if check twice.
not a big deal, but we can probably do something better.
However I don't understand why we had this get/put here at first place.
Only for this platform and only for this pll4. So, what I am wondering is
that we have something better to do with the power_well infrastructure
in general that would allow us to avoid the if (platform && pll4) in favor
of something more generic.
but definitely not a blocker for this patch itself.
>
>>> - enable_reg = MG_PLL_ENABLE(0);
>>>
>>> /*
>>> * We need to disable DC states when this DPLL is enabled.
>>> @@ -4157,11 +4163,10 @@ static void icl_pll_disable(struct
>>> drm_i915_private *dev_priv, static void combo_pll_disable(struct
>> drm_i915_private *dev_priv,
>>> struct intel_shared_dpll *pll) {
>>> - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>>> + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll);
>>>
>>> if (IS_ELKHARTLAKE(dev_priv) &&
>>> pll->info->id == DPLL_ID_EHL_DPLL4) {
>>> - enable_reg = MG_PLL_ENABLE(0);
>>> icl_pll_disable(dev_priv, pll, enable_reg);
>>
>> but here, at least, let's clean this function now...
>> move this call above and out of the if and delete the one below and keep
>> just the power_put inside the if...
>
> Good change. Thanks!
> Will change that.
>
> Anusha
>
>>>
>>> intel_display_power_put(dev_priv,
>> POWER_DOMAIN_DPLL_DC_OFF,
>>> --
>>> 2.25.0
>>>
>>> ___
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
I
Re: [Intel-gfx] [Regression] "drm/i915: Implement display w/a #1143" breaks HDMI on ASUS GL552VW
> On Aug 26, 2020, at 21:05, Ville Syrjälä > wrote: > > On Wed, Aug 26, 2020 at 12:40:15PM +0800, Kai-Heng Feng wrote: >> Hi, >> >>> On Aug 25, 2020, at 02:46, Runyan, Arthur J >>> wrote: >>> >>> I remember some strangeness about the blnclegdisbl. I'll see if I can dig >>> up some more. >> >> >> The register read can be found at [1] and [2]. >> >> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1871721/comments/119 >> [2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1871721/comments/120 > > Looks like it's using the 400mV/0dB setting. Can we get the same dumps > with the driver loaded just to confirm whether we're using different > settings? > > Also a dump of /sys/kernel/debug/dri/0/i915_vbt would be good > to have. https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1871721/comments/124 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1871721/comments/125 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1871721/comments/126 Kai-Heng > >> >> Kai-Heng >> >>> >>> -Original Message- >>> From: Ville Syrjälä >>> Sent: Monday, August 24, 2020 11:05 AM >>> To: Kai-Heng Feng >>> Cc: Runyan, Arthur J ; Vivi, Rodrigo >>> ; intel-gfx >>> Subject: Re: [Regression] "drm/i915: Implement display w/a #1143" breaks >>> HDMI on ASUS GL552VW >>> >>> On Mon, Aug 17, 2020 at 02:17:49PM +0800, Kai-Heng Feng wrote: > On Aug 17, 2020, at 00:22, Runyan, Arthur J > wrote: > > You'll need to read out the DDI_BUF_TRANS_* and DISPIO_CR_TX_BMU_CR0 > registers at boot before i915 programs them and compare with what driver > programs. > Rodrigo can probably show you how. Right, I'll wait for a patch then :) >>> >>> To grab the BIOS reg values we just have to make sure the driver doesn't >>> load. Eg. pass something like "modprobe.blacklist=i915,snd_hda_intel 3" to >>> the kernel cmdline (+ whatever other magic ubuntu might require). Confirm >>> with something like "lsmod | grep i915" to make sure the driver didn't >>> sneak in despite our best efforts. >>> >>> Then we can dump the registers with intel_reg from igt-gpu-tools: >>> intel_reg read --count 20 0x64E00 0x64E60 0x64EC0 0x64F20 0x64F80 intel_reg >>> read 0x64000 0x64100 0x64200 0x64300 0x64400 0x6C00C >>> >>> The only somewhat suspicious thing I noticed is that we treat >>> DISPIO_CR_TX_BMU_CR0:tx_blnclegdisbl as a bitmask (bit 23 -> DDI A, bit 24 >>> -> DDI B, etc.) whereas the spec seems to be saying that we should just >>> zero out all the bits of tx_blnclegdisbl when any DDI needs iboost. Art, is >>> our interpretation of the bits correct or just a fairy tale? >>> Kai-Heng > > -Original Message- > From: Kai-Heng Feng > Sent: Thursday, August 13, 2020 10:14 PM > To: Runyan, Arthur J > Cc: Vivi, Rodrigo ; Ville Syrjälä > ; intel-gfx > > Subject: Re: [Regression] "drm/i915: Implement display w/a #1143" > breaks HDMI on ASUS GL552VW > > Hi, > >> On Aug 14, 2020, at 01:56, Runyan, Arthur J >> wrote: >> >> The workaround is freeing up stuck vswing values to let new vswing >> programming kick in. Maybe the new vswing values are wrong. >> Try checking the vswing that driver programs against what BIOS/GOP >> programs. > > Do you mean to print out value of I915_READ()? > val = I915_READ(CHICKEN_TRANS(transcoder)); > > Kai-Heng > >> >> -Original Message- >> From: Vivi, Rodrigo >> Sent: Thursday, August 13, 2020 9:50 AM >> To: Kai-Heng Feng ; Runyan, Arthur J >> >> Cc: Ville Syrjälä ; intel-gfx >> >> Subject: Re: [Regression] "drm/i915: Implement display w/a #1143" >> breaks HDMI on ASUS GL552VW >> >> Art, any comment here? >> >> I just checked and the W/a 1143 is implemented as described, but it is >> failing HDMI on this hybrid system. >> >>> On Aug 12, 2020, at 9:07 PM, Kai-Heng Feng >>> wrote: >>> >>> Hi, >>> >>> There's a regression reported that HDMI output stops working after os >>> upgrade: >>> https://bugs.launchpad.net/bugs/1871721 >>> >>> Here's the bisect result: >>> 0519c102f5285476d7868a387bdb6c58385e4074 is the first bad commit >>> commit 0519c102f5285476d7868a387bdb6c58385e4074 >>> Author: Ville Syrjälä >>> Date: Mon Jan 22 19:41:31 2018 +0200 >>> >>> drm/i915: Implement display w/a #1143 >>> >>> Apparently SKL/KBL/CFL need some manual help to get the >>> programmed HDMI vswing to stick. Implement the relevant >>> workaround (display w/a #1143). >>> >>> Note that the relevant chicken bits live in a transcoder register >>> even though the bits affect a specific DDI port rather than a >>> specific transcoder. Hence we must pick the correct transcoder >>> register instance based on the port r
