[Intel-gfx] ✓ Fi.CI.IGT: success for kvm/vfio: fix potential deadlock on vfio group lock
== Series Details == Series: kvm/vfio: fix potential deadlock on vfio group lock URL : https://patchwork.freedesktop.org/series/112568/ State : success == Summary == CI Bug Log - changes from CI_DRM_12560_full -> Patchwork_112568v1_full Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/index.html Participating hosts (13 -> 10) -- Missing(3): pig-skl-6260u pig-kbl-iris pig-glk-j5005 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_112568v1_full: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_selftest@perf@engine_cs: - {shard-rkl}:[PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-rkl-4/igt@i915_selftest@perf@engine_cs.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-rkl-5/igt@i915_selftest@perf@engine_cs.html * igt@kms_cursor_crc@cursor-sliding-128x42@pipe-a-hdmi-a-4: - {shard-dg1}:NOTRUN -> [DMESG-WARN][3] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-dg1-15/igt@kms_cursor_crc@cursor-sliding-128...@pipe-a-hdmi-a-4.html Known issues Here are the changes found in Patchwork_112568v1_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_fair@basic-none-rrul@rcs0: - shard-glk: [PASS][4] -> [FAIL][5] ([i915#2842]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-glk5/igt@gem_exec_fair@basic-none-r...@rcs0.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-glk3/igt@gem_exec_fair@basic-none-r...@rcs0.html * igt@kms_flip@flip-vs-expired-vblank-interruptible@b-hdmi-a1: - shard-glk: [PASS][6] -> [FAIL][7] ([i915#79]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-glk8/igt@kms_flip@flip-vs-expired-vblank-interrupti...@b-hdmi-a1.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-glk4/igt@kms_flip@flip-vs-expired-vblank-interrupti...@b-hdmi-a1.html Possible fixes * igt@drm_fdinfo@idle@rcs0: - {shard-rkl}:[FAIL][8] ([i915#7742]) -> [PASS][9] +2 similar issues [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-rkl-1/igt@drm_fdinfo@i...@rcs0.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-rkl-6/igt@drm_fdinfo@i...@rcs0.html * igt@fbdev@read: - {shard-rkl}:[SKIP][10] ([i915#2582]) -> [PASS][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-rkl-1/igt@fb...@read.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-rkl-6/igt@fb...@read.html * igt@feature_discovery@psr2: - {shard-rkl}:[SKIP][12] ([i915#658]) -> [PASS][13] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-rkl-4/igt@feature_discov...@psr2.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-rkl-6/igt@feature_discov...@psr2.html * igt@gem_ctx_exec@basic-nohangcheck: - {shard-tglu}: [FAIL][14] ([i915#6268]) -> [PASS][15] [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-tglu-5/igt@gem_ctx_e...@basic-nohangcheck.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-tglu-7/igt@gem_ctx_e...@basic-nohangcheck.html * igt@gem_eio@in-flight-suspend: - {shard-rkl}:[FAIL][16] ([fdo#103375]) -> [PASS][17] [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-rkl-3/igt@gem_...@in-flight-suspend.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-rkl-2/igt@gem_...@in-flight-suspend.html * igt@gem_exec_fair@basic-throttle@rcs0: - shard-glk: [FAIL][18] ([i915#2842]) -> [PASS][19] [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-glk7/igt@gem_exec_fair@basic-throt...@rcs0.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-glk2/igt@gem_exec_fair@basic-throt...@rcs0.html * igt@gem_exec_flush@basic-batch-kernel-default-cmd: - {shard-rkl}:[SKIP][20] ([fdo#109313]) -> [PASS][21] [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-rkl-1/igt@gem_exec_fl...@basic-batch-kernel-default-cmd.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/shard-rkl-5/igt@gem_exec_fl...@basic-batch-kernel-default-cmd.html * igt@gem_exec_reloc@basic-gtt-read-noreloc: - {shard-rkl}:[SKIP][22] ([i915#3281]) -> [PASS][23] +16 similar issues [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/shard-rkl-1/igt@gem_exec_re...@basic-gtt-read-noreloc.html
Re: [Intel-gfx] [PATCH] drm/i915/fbc: Avoid full proxy f_ops for FBC debug attributes
On Mon, Jan 09, 2023 at 02:06:13PM -0500, Rodrigo Vivi wrote: > On Sun, Jan 08, 2023 at 01:33:41AM +0530, Deepak R Varma wrote: > > On Thu, Jan 05, 2023 at 09:13:35AM +0100, Julia Lawall wrote: > > > > Hi Julia, thanks for helping here. > > > > > > > > So, my question is why this > > > > > > > > make coccicheck M=drivers/gpu/drm/i915/ MODE=context > > > > COCCI=./scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > > > > > > > > didn't catch this chunck: > > > > > > > > - debugfs_create_file("i915_fbc_false_color", 0644, > > > > parent, > > > > - fbc, > > > > _fbc_debugfs_false_color_fops); > > > > + debugfs_create_file_unsafe("i915_fbc_false_color", > > > > 0644, parent, > > > > + fbc, > > > > _fbc_debugfs_false_color_fops); > > > > > > > > When I run it it only catches and replaces this: > > > > > > > > - DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > > > > + DEFINE_DEBUGFS_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > > > > > > There is something strange in your question. You have MODE=context but > > > you show the output for MODE=patch. The rule dcf matches a call to > > > debugfs_create_file, and the context rule matching DEFINE_SIMPLE_ATTRIBUTE > > > is only activated if dcf succeeds. So when the context rule gives a > > > report, there is always a corresponding call to debugfs_create_file in the > > > same file, it is just not highlighted. So the request is that it should > > > be highlighted as well? > > > > Hello Rodrigo, > > Not trying to speak for you, but I think Julia's comment appears to be the > > correct interpretation of your observation. Would you mind > > confirming/clarifying > > and suggest next steps for this proposal? > > doh! newby coccinelle user detected! My bad, sorry! > > make coccicheck M=drivers/gpu/drm/i915/ MODE=patch > COCCI=./scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > > do shows everything. > > So, could you please mention this line in the commit message so we don't > forget that? Sure, I will do that. > > Also could you please provide patches for the other cases? > 1 patch for each file is desirable in this case since it touches different > areas. Sounds good. I will separate patches one per file and send in a series as appropriate. Thank you, ./drv > > > > > Thank you, > > ./drv > > > > > > > > julia > > > > > > >
[Intel-gfx] ✓ Fi.CI.BAT: success for Enable HDCP2.x via GSC CS (rev6)
== Series Details == Series: Enable HDCP2.x via GSC CS (rev6) URL : https://patchwork.freedesktop.org/series/111876/ State : success == Summary == CI Bug Log - changes from CI_DRM_12561 -> Patchwork_111876v6 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111876v6/index.html Participating hosts (39 -> 38) -- Missing(1): fi-snb-2520m Known issues Here are the changes found in Patchwork_111876v6 that come from known issues: ### IGT changes ### Possible fixes * igt@i915_pm_rpm@basic-rte: - {bat-adlp-6}: [DMESG-WARN][1] ([i915#7077]) -> [PASS][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12561/bat-adlp-6/igt@i915_pm_...@basic-rte.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111876v6/bat-adlp-6/igt@i915_pm_...@basic-rte.html Warnings * igt@i915_suspend@basic-s3-without-i915: - fi-rkl-11600: [FAIL][3] ([fdo#103375]) -> [INCOMPLETE][4] ([i915#4817]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12561/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111876v6/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375 [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295 [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291 [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301 [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546 [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817 [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983 [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621 [i915#7077]: https://gitlab.freedesktop.org/drm/intel/issues/7077 Build changes - * Linux: CI_DRM_12561 -> Patchwork_111876v6 CI-20190529: 20190529 CI_DRM_12561: 73221da9da21a841ab8a837facb7e58e63e87166 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7114: 2fd839599a200c089a5c9dbf5048609faf9b8104 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_111876v6: 73221da9da21a841ab8a837facb7e58e63e87166 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 615cbbf08825 drm/i915/mtl: Add HDCP GSC interface e1586fc4e35c drm/i915/mtl: Add function to send command to GSC CS 6a3cb7f82c79 drm/i915/hdcp: Refactor HDCP API structures 948273621243 i915/hdcp: HDCP2.x Refactoring to agnostic hdcp 6a872bb7d591 drm/i915/hdcp: Keep hdcp agonstic naming convention adf22dddaf31 drm/i915/gsc: Create GSC request submission mechanism == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111876v6/index.html
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Enable HDCP2.x via GSC CS (rev6)
== Series Details == Series: Enable HDCP2.x via GSC CS (rev6) URL : https://patchwork.freedesktop.org/series/111876/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
[Intel-gfx] [PATCH v6 6/6] drm/i915/mtl: Add HDCP GSC interface
MTL uses GSC command streamer i.e gsc cs to send HDCP/PXP commands to GSC f/w. It requires to keep hdcp display driver agnostic to content protection f/w (ME/GSC fw) in the form of i915_hdcp_fw_ops generic ops. Adding HDCP GSC CS interface by leveraging the i915_hdcp_fw_ops generic ops instead of I915_HDCP_COMPONENT as integral part of i915. Adding checks to see if GSC is loaded and proxy is setup --v6 -dont change the license date in same patch series [Jani] -fix the license year {Jani] Cc: Tomas Winkler Cc: Rodrigo Vivi Cc: Uma Shankar Cc: Ankit Nautiyal Signed-off-by: Anshuman Gupta Signed-off-by: Suraj Kandpal Reviewed-by: Ankit Nautiyal --- drivers/gpu/drm/i915/display/intel_hdcp.c | 28 +- drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 631 +- drivers/gpu/drm/i915/display/intel_hdcp_gsc.h | 3 + 3 files changed, 655 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 0d6aed1eb171..0824c186a31f 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -25,6 +25,8 @@ #include "intel_hdcp.h" #include "intel_hdcp_regs.h" #include "intel_pcode.h" +#include "intel_connector.h" +#include "intel_hdcp_gsc.h" #define KEY_LOAD_TRIES 5 #define HDCP2_LC_RETRY_CNT 3 @@ -203,13 +205,20 @@ bool intel_hdcp2_capable(struct intel_connector *connector) struct intel_digital_port *dig_port = intel_attached_dig_port(connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_hdcp *hdcp = >hdcp; + struct intel_gt *gt = dev_priv->media_gt; + struct intel_gsc_uc *gsc = >uc.gsc; bool capable = false; /* I915 support for HDCP2.2 */ if (!hdcp->hdcp2_supported) return false; - /* MEI interface is solid */ + /* If MTL+ make sure gsc is loaded and proxy is setup */ + if (DISPLAY_VER(dev_priv) >= 14) + if (!intel_uc_fw_is_running(>fw)) + return false; + + /* MEI/GSC interface is solid depending on which is used */ mutex_lock(_priv->display.hdcp.comp_mutex); if (!dev_priv->display.hdcp.comp_added || !dev_priv->display.hdcp.master) { mutex_unlock(_priv->display.hdcp.comp_mutex); @@ -2235,7 +2244,7 @@ static int initialize_hdcp_port_data(struct intel_connector *connector, static bool is_hdcp2_supported(struct drm_i915_private *dev_priv) { - if (!IS_ENABLED(CONFIG_INTEL_MEI_HDCP)) + if (intel_hdcp_gsc_cs_required(dev_priv) && !IS_ENABLED(CONFIG_INTEL_MEI_HDCP)) return false; return (DISPLAY_VER(dev_priv) >= 10 || @@ -2256,10 +2265,14 @@ void intel_hdcp_component_init(struct drm_i915_private *dev_priv) dev_priv->display.hdcp.comp_added = true; mutex_unlock(_priv->display.hdcp.comp_mutex); - ret = component_add_typed(dev_priv->drm.dev, _hdcp_ops, - I915_COMPONENT_HDCP); + if (intel_hdcp_gsc_cs_required(dev_priv)) + ret = intel_hdcp_gsc_init(dev_priv); + else + ret = component_add_typed(dev_priv->drm.dev, _hdcp_ops, + I915_COMPONENT_HDCP); + if (ret < 0) { - drm_dbg_kms(_priv->drm, "Failed at component add(%d)\n", + drm_dbg_kms(_priv->drm, "Failed at fw component add(%d)\n", ret); mutex_lock(_priv->display.hdcp.comp_mutex); dev_priv->display.hdcp.comp_added = false; @@ -2486,7 +2499,10 @@ void intel_hdcp_component_fini(struct drm_i915_private *dev_priv) dev_priv->display.hdcp.comp_added = false; mutex_unlock(_priv->display.hdcp.comp_mutex); - component_del(dev_priv->drm.dev, _hdcp_ops); + if (intel_hdcp_gsc_cs_required(dev_priv)) + intel_hdcp_gsc_fini(dev_priv); + else + component_del(dev_priv->drm.dev, _hdcp_ops); } void intel_hdcp_cleanup(struct intel_connector *connector) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c index ca7bfeeb1768..838417e08610 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c @@ -3,6 +3,8 @@ * Copyright 2023, Intel Corporation. */ +#include + #include "display/intel_hdcp_gsc.h" #include "gem/i915_gem_region.h" #include "gt/uc/intel_gsc_uc_heci_cmd_submit.h" @@ -15,6 +17,632 @@ struct intel_hdcp_gsc_message { void *hdcp_cmd; }; +bool intel_hdcp_gsc_cs_required(struct drm_i915_private *i915) +{ + if (DISPLAY_VER(i915) >= 14) + return true; + return false; +} + +static int +gsc_hdcp_initiate_session(struct device *dev, struct hdcp_port_data *data, + struct hdcp2_ake_init *ake_data) +{ +
[Intel-gfx] [PATCH v6 5/6] drm/i915/mtl: Add function to send command to GSC CS
Add function that takes care of sending command to gsc cs. We start of with allocation of memory for our command intel_hdcp_gsc_message that contains gsc cs memory header as directed in specs followed by the actual payload hdcp message that we want to send. Spec states that we need to poll pending bit of response header around 20 times each try being 50ms apart hence adding that to current gsc_msg_send function Also we use the same function to take care of both sending and receiving hence no separate function to get the response. --v4 -Create common function to fill in gsc_mtl_header [Alan] -define host session bitmask [Alan] --v5 -use i915 directly instead of gt->i915 [Alan] -No need to make fields NULL as we are already using kzalloc [Alan] Cc: Ankit Nautiyal Cc: Daniele Ceraolo Spurio Cc: Uma Shankar Cc: Anshuman Gupta Signed-off-by: Suraj Kandpal --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 202 ++ drivers/gpu/drm/i915/display/intel_hdcp_gsc.h | 18 ++ .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 15 ++ .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h | 6 + 5 files changed, 242 insertions(+) create mode 100644 drivers/gpu/drm/i915/display/intel_hdcp_gsc.c create mode 100644 drivers/gpu/drm/i915/display/intel_hdcp_gsc.h diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 461d6b40656d..194aae92c354 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -254,6 +254,7 @@ i915-y += \ display/intel_frontbuffer.o \ display/intel_global_state.o \ display/intel_hdcp.o \ + display/intel_hdcp_gsc.o \ display/intel_hotplug.o \ display/intel_hti.o \ display/intel_lpe_audio.o \ diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c new file mode 100644 index ..ca7bfeeb1768 --- /dev/null +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c @@ -0,0 +1,202 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright 2023, Intel Corporation. + */ + +#include "display/intel_hdcp_gsc.h" +#include "gem/i915_gem_region.h" +#include "gt/uc/intel_gsc_uc_heci_cmd_submit.h" +#include "i915_drv.h" +#include "i915_utils.h" + +struct intel_hdcp_gsc_message { + struct drm_i915_gem_object *obj; + struct i915_vma *vma; + void *hdcp_cmd; +}; + +/*This function helps allocate memory for the command that we will send to gsc cs */ +static int intel_initialize_hdcp_gsc_message(struct drm_i915_private *i915, +struct intel_hdcp_gsc_message *hdcp_message) +{ + struct intel_gt *gt = i915->media_gt; + struct drm_i915_gem_object *obj = NULL; + struct i915_vma *vma = NULL; + void *cmd; + int err; + + /* allocate object of one page for HDCP command memory and store it */ + obj = i915_gem_object_create_shmem(i915, PAGE_SIZE); + + if (IS_ERR(obj)) { + drm_err(>drm, "Failed to allocate HDCP streaming command!\n"); + return PTR_ERR(obj); + } + + cmd = i915_gem_object_pin_map_unlocked(obj, i915_coherent_map_type(i915, obj, true)); + if (IS_ERR(cmd)) { + drm_err(>drm, "Failed to map gsc message page!\n"); + err = PTR_ERR(cmd); + goto out_unpin; + } + + vma = i915_vma_instance(obj, >ggtt->vm, NULL); + if (IS_ERR(vma)) { + err = PTR_ERR(vma); + goto out_unmap; + } + + err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL); + if (err) + goto out_unmap; + + memset(cmd, 0, obj->base.size); + + hdcp_message->obj = obj; + hdcp_message->hdcp_cmd = cmd; + hdcp_message->vma = vma; + + return 0; + +out_unmap: + i915_gem_object_unpin_map(obj); +out_unpin: + i915_gem_object_put(obj); + return err; +} + +static void intel_free_hdcp_gsc_message(struct intel_hdcp_gsc_message *hdcp_message) +{ + struct drm_i915_gem_object *obj = fetch_and_zero(_message->obj); + + if (!obj) + return; + + if (hdcp_message->vma) + i915_vma_unpin(fetch_and_zero(_message->vma)); + + i915_gem_object_unpin_map(obj); + i915_gem_object_put(obj); + kfree(hdcp_message); +} + +static int intel_gsc_send_sync(struct drm_i915_private *i915, + struct intel_gsc_mtl_header *header, u64 addr, + size_t msg_out_len) +{ + struct intel_gt *gt = i915->media_gt; + int ret; + + header->flags = 0; + ret = intel_gsc_uc_heci_cmd_submit_packet(>uc.gsc, addr, + header->message_size, + addr, + msg_out_len + sizeof(*header)); + if (ret) { +
[Intel-gfx] [PATCH v6 4/6] drm/i915/hdcp: Refactor HDCP API structures
It requires to move intel specific HDCP API structures to i915_cp_fw_hdcp_interface.h from driver/misc/mei/hdcp/mei_hdcp.h so that any content protection fw interfaces can use these structures. Cc: Tomas Winkler Cc: Rodrigo Vivi Cc: Uma Shankar Cc: Ankit Nautiyal Signed-off-by: Anshuman Gupta Signed-off-by: Suraj Kandpal --- drivers/misc/mei/hdcp/mei_hdcp.c | 44 ++-- drivers/misc/mei/hdcp/mei_hdcp.h | 354 - include/drm/i915_hdcp_interface.h | 355 ++ 3 files changed, 377 insertions(+), 376 deletions(-) diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c index 0ff0bd07e385..d4faecbbbe76 100644 --- a/drivers/misc/mei/hdcp/mei_hdcp.c +++ b/drivers/misc/mei/hdcp/mei_hdcp.c @@ -52,7 +52,7 @@ mei_hdcp_initiate_session(struct device *dev, struct hdcp_port_data *data, session_init_in.header.api_version = HDCP_API_VERSION; session_init_in.header.command_id = WIRED_INITIATE_HDCP2_SESSION; - session_init_in.header.status = ME_HDCP_STATUS_SUCCESS; + session_init_in.header.status = FW_HDCP_STATUS_SUCCESS; session_init_in.header.buffer_len = WIRED_CMD_BUF_LEN_INITIATE_HDCP2_SESSION_IN; @@ -75,7 +75,7 @@ mei_hdcp_initiate_session(struct device *dev, struct hdcp_port_data *data, return byte; } - if (session_init_out.header.status != ME_HDCP_STATUS_SUCCESS) { + if (session_init_out.header.status != FW_HDCP_STATUS_SUCCESS) { dev_dbg(dev, "ME cmd 0x%08X Failed. Status: 0x%X\n", WIRED_INITIATE_HDCP2_SESSION, session_init_out.header.status); @@ -122,7 +122,7 @@ mei_hdcp_verify_receiver_cert_prepare_km(struct device *dev, verify_rxcert_in.header.api_version = HDCP_API_VERSION; verify_rxcert_in.header.command_id = WIRED_VERIFY_RECEIVER_CERT; - verify_rxcert_in.header.status = ME_HDCP_STATUS_SUCCESS; + verify_rxcert_in.header.status = FW_HDCP_STATUS_SUCCESS; verify_rxcert_in.header.buffer_len = WIRED_CMD_BUF_LEN_VERIFY_RECEIVER_CERT_IN; @@ -148,7 +148,7 @@ mei_hdcp_verify_receiver_cert_prepare_km(struct device *dev, return byte; } - if (verify_rxcert_out.header.status != ME_HDCP_STATUS_SUCCESS) { + if (verify_rxcert_out.header.status != FW_HDCP_STATUS_SUCCESS) { dev_dbg(dev, "ME cmd 0x%08X Failed. Status: 0x%X\n", WIRED_VERIFY_RECEIVER_CERT, verify_rxcert_out.header.status); @@ -194,7 +194,7 @@ mei_hdcp_verify_hprime(struct device *dev, struct hdcp_port_data *data, send_hprime_in.header.api_version = HDCP_API_VERSION; send_hprime_in.header.command_id = WIRED_AKE_SEND_HPRIME; - send_hprime_in.header.status = ME_HDCP_STATUS_SUCCESS; + send_hprime_in.header.status = FW_HDCP_STATUS_SUCCESS; send_hprime_in.header.buffer_len = WIRED_CMD_BUF_LEN_AKE_SEND_HPRIME_IN; send_hprime_in.port.integrated_port_type = data->port_type; @@ -218,7 +218,7 @@ mei_hdcp_verify_hprime(struct device *dev, struct hdcp_port_data *data, return byte; } - if (send_hprime_out.header.status != ME_HDCP_STATUS_SUCCESS) { + if (send_hprime_out.header.status != FW_HDCP_STATUS_SUCCESS) { dev_dbg(dev, "ME cmd 0x%08X Failed. Status: 0x%X\n", WIRED_AKE_SEND_HPRIME, send_hprime_out.header.status); return -EIO; @@ -251,7 +251,7 @@ mei_hdcp_store_pairing_info(struct device *dev, struct hdcp_port_data *data, pairing_info_in.header.api_version = HDCP_API_VERSION; pairing_info_in.header.command_id = WIRED_AKE_SEND_PAIRING_INFO; - pairing_info_in.header.status = ME_HDCP_STATUS_SUCCESS; + pairing_info_in.header.status = FW_HDCP_STATUS_SUCCESS; pairing_info_in.header.buffer_len = WIRED_CMD_BUF_LEN_SEND_PAIRING_INFO_IN; @@ -276,7 +276,7 @@ mei_hdcp_store_pairing_info(struct device *dev, struct hdcp_port_data *data, return byte; } - if (pairing_info_out.header.status != ME_HDCP_STATUS_SUCCESS) { + if (pairing_info_out.header.status != FW_HDCP_STATUS_SUCCESS) { dev_dbg(dev, "ME cmd 0x%08X failed. Status: 0x%X\n", WIRED_AKE_SEND_PAIRING_INFO, pairing_info_out.header.status); @@ -311,7 +311,7 @@ mei_hdcp_initiate_locality_check(struct device *dev, lc_init_in.header.api_version = HDCP_API_VERSION; lc_init_in.header.command_id = WIRED_INIT_LOCALITY_CHECK; - lc_init_in.header.status = ME_HDCP_STATUS_SUCCESS; + lc_init_in.header.status = FW_HDCP_STATUS_SUCCESS; lc_init_in.header.buffer_len = WIRED_CMD_BUF_LEN_INIT_LOCALITY_CHECK_IN; lc_init_in.port.integrated_port_type =
[Intel-gfx] [PATCH v6 2/6] drm/i915/hdcp: Keep hdcp agonstic naming convention
From: Anshuman Gupta Change the include/drm/i915_mei_hdcp_interface.h to include/drm/i915_hdcp_interface.h --v6 -make each patch build individually [Jani] Cc: Tomas Winkler Cc: Rodrigo Vivi Cc: Uma Shankar Cc: Ankit Nautiyal Signed-off-by: Anshuman Gupta Signed-off-by: Suraj Kandpal Acked-by: Tomas Winkler --- .../gpu/drm/i915/display/intel_display_core.h | 2 +- .../drm/i915/display/intel_display_types.h| 2 +- drivers/gpu/drm/i915/display/intel_hdcp.c | 81 - drivers/misc/mei/hdcp/mei_hdcp.c | 58 ++--- ...hdcp_interface.h => i915_hdcp_interface.h} | 86 +-- 5 files changed, 115 insertions(+), 114 deletions(-) rename include/drm/{i915_mei_hdcp_interface.h => i915_hdcp_interface.h} (75%) diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index 57ddce3ba02b..de71ff6ad80a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -368,7 +368,7 @@ struct intel_display { } gmbus; struct { - struct i915_hdcp_comp_master *master; + struct i915_hdcp_master *master; bool comp_added; /* Mutex to protect the above hdcp component related values. */ diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 32e8b2fc3cc6..81d195ef5e57 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -43,7 +43,7 @@ #include #include #include -#include +#include #include #include "i915_vma.h" diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 6406fd487ee5..262c76f21801 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1143,7 +1143,7 @@ hdcp2_prepare_ake_init(struct intel_connector *connector, struct intel_digital_port *dig_port = intel_attached_dig_port(connector); struct hdcp_port_data *data = _port->hdcp_port_data; struct drm_i915_private *dev_priv = to_i915(connector->base.dev); - struct i915_hdcp_comp_master *comp; + struct i915_hdcp_master *comp; int ret; mutex_lock(_priv->display.hdcp.comp_mutex); @@ -1154,7 +1154,7 @@ hdcp2_prepare_ake_init(struct intel_connector *connector, return -EINVAL; } - ret = comp->ops->initiate_hdcp2_session(comp->mei_dev, data, ake_data); + ret = comp->ops->initiate_hdcp2_session(comp->hdcp_dev, data, ake_data); if (ret) drm_dbg_kms(_priv->drm, "Prepare_ake_init failed. %d\n", ret); @@ -1173,7 +1173,7 @@ hdcp2_verify_rx_cert_prepare_km(struct intel_connector *connector, struct intel_digital_port *dig_port = intel_attached_dig_port(connector); struct hdcp_port_data *data = _port->hdcp_port_data; struct drm_i915_private *dev_priv = to_i915(connector->base.dev); - struct i915_hdcp_comp_master *comp; + struct i915_hdcp_master *comp; int ret; mutex_lock(_priv->display.hdcp.comp_mutex); @@ -1184,7 +1184,7 @@ hdcp2_verify_rx_cert_prepare_km(struct intel_connector *connector, return -EINVAL; } - ret = comp->ops->verify_receiver_cert_prepare_km(comp->mei_dev, data, + ret = comp->ops->verify_receiver_cert_prepare_km(comp->hdcp_dev, data, rx_cert, paired, ek_pub_km, msg_sz); if (ret < 0) @@ -1201,7 +1201,7 @@ static int hdcp2_verify_hprime(struct intel_connector *connector, struct intel_digital_port *dig_port = intel_attached_dig_port(connector); struct hdcp_port_data *data = _port->hdcp_port_data; struct drm_i915_private *dev_priv = to_i915(connector->base.dev); - struct i915_hdcp_comp_master *comp; + struct i915_hdcp_master *comp; int ret; mutex_lock(_priv->display.hdcp.comp_mutex); @@ -1212,7 +1212,7 @@ static int hdcp2_verify_hprime(struct intel_connector *connector, return -EINVAL; } - ret = comp->ops->verify_hprime(comp->mei_dev, data, rx_hprime); + ret = comp->ops->verify_hprime(comp->hdcp_dev, data, rx_hprime); if (ret < 0) drm_dbg_kms(_priv->drm, "Verify hprime failed. %d\n", ret); mutex_unlock(_priv->display.hdcp.comp_mutex); @@ -1227,7 +1227,7 @@ hdcp2_store_pairing_info(struct intel_connector *connector, struct intel_digital_port *dig_port = intel_attached_dig_port(connector); struct hdcp_port_data *data = _port->hdcp_port_data; struct drm_i915_private *dev_priv = to_i915(connector->base.dev); - struct i915_hdcp_comp_master *comp; + struct
[Intel-gfx] [PATCH v6 3/6] i915/hdcp: HDCP2.x Refactoring to agnostic hdcp
As now we have more then one type of content protection secrity firmware. Let change the i915_cp_fw_hdcp_interface.h header naming convention to suit generic f/w type. %s/MEI_/HDCP_ %s/mei_dev/hdcp_dev As interface to CP FW can be either a non i915 component or i915 intergral component, change structure name Accordingly. %s/i915_hdcp_comp_master/i915_hdcp_master %s/i915_hdcp_component_ops/i915_hdcp_ops --v3 -Changing names to drop cp_fw to make naming more agnostic[Jani] Cc: Tomas Winkler Cc: Rodrigo Vivi Cc: Uma Shankar Cc: Ankit Nautiyal Signed-off-by: Anshuman Gupta Signed-off-by: Suraj Kandpal --- drivers/gpu/drm/i915/display/intel_display_core.h | 1 + drivers/gpu/drm/i915/display/intel_hdcp.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index de71ff6ad80a..132e9134ba05 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -15,6 +15,7 @@ #include #include +#include #include "intel_cdclk.h" #include "intel_display.h" diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 262c76f21801..0d6aed1eb171 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1409,7 +1409,7 @@ static int hdcp2_authenticate_port(struct intel_connector *connector) return ret; } -static int hdcp2_close_mei_session(struct intel_connector *connector) +static int hdcp2_close_session(struct intel_connector *connector) { struct intel_digital_port *dig_port = intel_attached_dig_port(connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -1433,7 +1433,7 @@ static int hdcp2_close_mei_session(struct intel_connector *connector) static int hdcp2_deauthenticate_port(struct intel_connector *connector) { - return hdcp2_close_mei_session(connector); + return hdcp2_close_session(connector); } /* Authentication flow starts from here */ -- 2.25.1
[Intel-gfx] [PATCH v6 1/6] drm/i915/gsc: Create GSC request submission mechanism
HDCP and PXP will require a common function to allow it to submit commands to the gsc cs. Also adding the gsc mtl header that needs to be added on to the existing payloads of HDCP and PXP. --v4 -Seprate gsc load and heci cmd submission into different functions in different files for better scalability [Alan] -Rename gsc address field [Alan] Cc: Daniele Ceraolo Spurio Cc: Alan Previn Signed-off-by: Suraj Kandpal Reviewed-by: Alan Previn --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 2 + drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h | 1 + .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 94 +++ .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h | 45 + 5 files changed, 143 insertions(+) create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index f47f00b162a4..461d6b40656d 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -194,6 +194,7 @@ i915-y += \ i915-y += \ gt/uc/intel_gsc_fw.o \ gt/uc/intel_gsc_uc.o \ + gt/uc/intel_gsc_uc_heci_cmd_submit.o\ gt/uc/intel_guc.o \ gt/uc/intel_guc_ads.o \ gt/uc/intel_guc_capture.o \ diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h index 2af1ae3831df..454179884801 100644 --- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h +++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h @@ -439,6 +439,8 @@ #define GSC_FW_LOAD GSC_INSTR(1, 0, 2) #define HECI1_FW_LIMIT_VALID (1 << 31) +#define GSC_HECI_CMD_PKT GSC_INSTR(0, 0, 6) + /* * Used to convert any address to canonical form. * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS, diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h index 4b5dbb44afb4..146ac0128f69 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h @@ -12,4 +12,5 @@ struct intel_gsc_uc; int intel_gsc_uc_fw_upload(struct intel_gsc_uc *gsc); bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc); + #endif diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c new file mode 100644 index ..be2424af521d --- /dev/null +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2023 Intel Corporation + */ + +#include "gt/intel_engine_pm.h" +#include "gt/intel_gpu_commands.h" +#include "gt/intel_gt.h" +#include "gt/intel_ring.h" +#include "intel_gsc_uc_heci_cmd_submit.h" + +struct gsc_heci_pkt { + u64 addr_in; + u32 size_in; + u64 addr_out; + u32 size_out; +}; + +static int emit_gsc_heci_pkt(struct i915_request *rq, struct gsc_heci_pkt *pkt) +{ + u32 *cs; + + cs = intel_ring_begin(rq, 8); + if (IS_ERR(cs)) + return PTR_ERR(cs); + + *cs++ = GSC_HECI_CMD_PKT; + *cs++ = lower_32_bits(pkt->addr_in); + *cs++ = upper_32_bits(pkt->addr_in); + *cs++ = pkt->size_in; + *cs++ = lower_32_bits(pkt->addr_out); + *cs++ = upper_32_bits(pkt->addr_out); + *cs++ = pkt->size_out; + *cs++ = 0; + + intel_ring_advance(rq, cs); + + return 0; +} + +int intel_gsc_uc_heci_cmd_submit_packet(struct intel_gsc_uc *gsc, u64 addr_in, + u32 size_in, u64 addr_out, + u32 size_out) +{ + struct intel_context *ce = gsc->ce; + struct i915_request *rq; + struct gsc_heci_pkt pkt = { + .addr_in = addr_in, + .size_in = size_in, + .addr_out = addr_out, + .size_out = size_out + }; + int err; + + if (!ce) + return -ENODEV; + + rq = i915_request_create(ce); + if (IS_ERR(rq)) + return PTR_ERR(rq); + + if (ce->engine->emit_init_breadcrumb) { + err = ce->engine->emit_init_breadcrumb(rq); + if (err) + goto out_rq; + } + + err = emit_gsc_heci_pkt(rq, ); + + if (err) + goto out_rq; + + err = ce->engine->emit_flush(rq, 0); + +out_rq: + i915_request_get(rq); + + if (unlikely(err)) + i915_request_set_error_once(rq, err); + + i915_request_add(rq); + + if (!err && i915_request_wait(rq, 0, msecs_to_jiffies(500)) < 0) + err = -ETIME; + + i915_request_put(rq); + + if (err) + drm_err(_uc_to_gt(gsc)->i915->drm, + "Request submission for GSC heci cmd failed (%d)\n", + err); + + return err; +} diff --git
[Intel-gfx] [PATCH v6 0/7] Enable HDCP2.x via GSC CS
These patches enable HDCP2.x on machines MTL and above. >From MTL onwards CSME is spilt into GSC and CSC and now we use GSC CS instead of MEI to talk to firmware to start HDCP authentication --v2 -Fixing some checkpatch changes which I forgot before sending out the series --v3 -Drop cp and fw to make naming more agnostic[Jani] -Sort header[Jani] -remove static inline function from i915_hdcp_interface[Jani] -abstract DISPLAY_VER check[Jani] --v4 -Remove stale comment P2 [Jani] -Fix part where file rename looks like its removed in P2 and added in P3 [Jani] -Add bitmask definition for host session id[Alan] -Seprating gsc load and heci cmd submission into different funcs[Alan] -Create comman function to fill gsc_mtl_header[Alan] --v5 -No need to make hdcp_message field null as we use kzalloc [Alan] -use i915->drm instead of gt->i915->drm [Alan] --v6 -Make each patch build individually [Jani] -drop cp_fw stale commit subject [Jani] -fix the date on license [Jani] -revert back to orginal design where mei and gsc fill their own header Anshuman Gupta (1): drm/i915/hdcp: Keep hdcp agonstic naming convention Suraj Kandpal (5): drm/i915/gsc: Create GSC request submission mechanism i915/hdcp: HDCP2.x Refactoring to agnostic hdcp drm/i915/hdcp: Refactor HDCP API structures drm/i915/mtl: Add function to send command to GSC CS drm/i915/mtl: Add HDCP GSC interface drivers/gpu/drm/i915/Makefile | 2 + .../gpu/drm/i915/display/intel_display_core.h | 3 +- .../drm/i915/display/intel_display_types.h| 2 +- drivers/gpu/drm/i915/display/intel_hdcp.c | 109 ++- drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 831 ++ drivers/gpu/drm/i915/display/intel_hdcp_gsc.h | 21 + drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 2 + drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h | 1 + .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 109 +++ .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h | 51 ++ drivers/misc/mei/hdcp/mei_hdcp.c | 102 +-- drivers/misc/mei/hdcp/mei_hdcp.h | 354 include/drm/i915_hdcp_interface.h | 539 include/drm/i915_mei_hdcp_interface.h | 184 14 files changed, 1673 insertions(+), 637 deletions(-) create mode 100644 drivers/gpu/drm/i915/display/intel_hdcp_gsc.c create mode 100644 drivers/gpu/drm/i915/display/intel_hdcp_gsc.h create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h create mode 100644 include/drm/i915_hdcp_interface.h delete mode 100644 include/drm/i915_mei_hdcp_interface.h -- 2.25.1
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gt: Cover rest of SVG unit MCR registers (rev2)
== Series Details == Series: drm/i915/gt: Cover rest of SVG unit MCR registers (rev2) URL : https://patchwork.freedesktop.org/series/112440/ State : success == Summary == CI Bug Log - changes from CI_DRM_12559_full -> Patchwork_112440v2_full Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/index.html Participating hosts (14 -> 10) -- Missing(4): shard-rkl0 pig-kbl-iris pig-glk-j5005 pig-skl-6260u Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_112440v2_full: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@kms_chamelium_color@ctm-limited-range}: - {shard-tglu-10}:NOTRUN -> [SKIP][1] +4 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-tglu-10/igt@kms_chamelium_co...@ctm-limited-range.html * {igt@kms_chamelium_color@gamma}: - {shard-tglu}: NOTRUN -> [SKIP][2] +8 similar issues [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-tglu-1/igt@kms_chamelium_co...@gamma.html * {igt@kms_chamelium_frames@dp-crc-fast}: - {shard-rkl}:NOTRUN -> [SKIP][3] +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-rkl-6/igt@kms_chamelium_fra...@dp-crc-fast.html * {igt@kms_chamelium_hpd@common-hpd-after-suspend}: - {shard-tglu-9}: NOTRUN -> [SKIP][4] +4 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-tglu-9/igt@kms_chamelium_...@common-hpd-after-suspend.html Known issues Here are the changes found in Patchwork_112440v2_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_fair@basic-deadline: - shard-glk: [PASS][5] -> [FAIL][6] ([i915#2846]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-glk3/igt@gem_exec_f...@basic-deadline.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-glk3/igt@gem_exec_f...@basic-deadline.html * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions: - shard-glk: [PASS][7] -> [FAIL][8] ([i915#2346]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-glk8/igt@kms_cursor_legacy@flip-vs-cur...@atomic-transitions.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-glk6/igt@kms_cursor_legacy@flip-vs-cur...@atomic-transitions.html * igt@kms_flip@2x-flip-vs-expired-vblank@bc-hdmi-a1-hdmi-a2: - shard-glk: [PASS][9] -> [FAIL][10] ([i915#79]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-glk6/igt@kms_flip@2x-flip-vs-expired-vbl...@bc-hdmi-a1-hdmi-a2.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-glk9/igt@kms_flip@2x-flip-vs-expired-vbl...@bc-hdmi-a1-hdmi-a2.html * igt@kms_flip@plain-flip-ts-check@a-hdmi-a1: - shard-glk: [PASS][11] -> [FAIL][12] ([i915#2122]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-glk7/igt@kms_flip@plain-flip-ts-ch...@a-hdmi-a1.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-glk7/igt@kms_flip@plain-flip-ts-ch...@a-hdmi-a1.html Possible fixes * igt@drm_fdinfo@most-busy-check-all@rcs0: - {shard-rkl}:[FAIL][13] ([i915#7742]) -> [PASS][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-3/igt@drm_fdinfo@most-busy-check-...@rcs0.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-rkl-4/igt@drm_fdinfo@most-busy-check-...@rcs0.html * igt@fbdev@unaligned-write: - {shard-rkl}:[SKIP][15] ([i915#2582]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-3/igt@fb...@unaligned-write.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-rkl-6/igt@fb...@unaligned-write.html * igt@gem_create@hog-create@smem0: - {shard-rkl}:[FAIL][17] ([i915#7679]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-1/igt@gem_create@hog-cre...@smem0.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-rkl-5/igt@gem_create@hog-cre...@smem0.html * igt@gem_ctx_persistence@legacy-engines-hang@blt: - {shard-rkl}:[SKIP][19] ([i915#6252]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-5/igt@gem_ctx_persistence@legacy-engines-h...@blt.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/shard-rkl-1/igt@gem_ctx_persistence@legacy-engines-h...@blt.html * igt@gem_exec_endless@dispatch@bcs0: - {shard-rkl}:[SKIP][21]
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Fix timeslots argument for DP DSC SST case (rev2)
== Series Details == Series: drm/i915: Fix timeslots argument for DP DSC SST case (rev2) URL : https://patchwork.freedesktop.org/series/112349/ State : success == Summary == CI Bug Log - changes from CI_DRM_12559_full -> Patchwork_112349v2_full Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/index.html Participating hosts (14 -> 10) -- Missing(4): shard-rkl0 pig-kbl-iris pig-glk-j5005 pig-skl-6260u Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_112349v2_full: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@kms_chamelium_color@gamma}: - {shard-tglu}: NOTRUN -> [SKIP][1] +15 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-tglu-3/igt@kms_chamelium_co...@gamma.html * {igt@kms_chamelium_frames@dp-crc-fast}: - {shard-rkl}:NOTRUN -> [SKIP][2] +1 similar issue [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-rkl-5/igt@kms_chamelium_fra...@dp-crc-fast.html * {igt@kms_chamelium_frames@hdmi-crc-multiple}: - {shard-tglu-10}:NOTRUN -> [SKIP][3] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-tglu-10/igt@kms_chamelium_fra...@hdmi-crc-multiple.html Known issues Here are the changes found in Patchwork_112349v2_full that come from known issues: ### IGT changes ### Issues hit * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions: - shard-glk: [PASS][4] -> [FAIL][5] ([i915#2346]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-glk8/igt@kms_cursor_legacy@flip-vs-cur...@atomic-transitions.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-glk6/igt@kms_cursor_legacy@flip-vs-cur...@atomic-transitions.html Possible fixes * igt@drm_fdinfo@virtual-idle: - {shard-rkl}:[FAIL][6] ([i915#7742]) -> [PASS][7] +1 similar issue [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-3/igt@drm_fdi...@virtual-idle.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-rkl-2/igt@drm_fdi...@virtual-idle.html * igt@fbdev@read: - {shard-rkl}:[SKIP][8] ([i915#2582]) -> [PASS][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-1/igt@fb...@read.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-rkl-6/igt@fb...@read.html * igt@gem_ctx_persistence@legacy-engines-hang@blt: - {shard-rkl}:[SKIP][10] ([i915#6252]) -> [PASS][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-5/igt@gem_ctx_persistence@legacy-engines-h...@blt.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-rkl-3/igt@gem_ctx_persistence@legacy-engines-h...@blt.html * igt@gem_eio@suspend: - {shard-rkl}:[FAIL][12] ([i915#7052]) -> [PASS][13] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-3/igt@gem_...@suspend.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-rkl-1/igt@gem_...@suspend.html * igt@gem_exec_reloc@basic-scanout@vcs0: - {shard-tglu}: [SKIP][14] ([i915#3639]) -> [PASS][15] +4 similar issues [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-tglu-6/igt@gem_exec_reloc@basic-scan...@vcs0.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-tglu-5/igt@gem_exec_reloc@basic-scan...@vcs0.html * igt@gem_exec_suspend@basic-s3@smem: - {shard-rkl}:[FAIL][16] ([fdo#103375]) -> [PASS][17] [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-3/igt@gem_exec_suspend@basic...@smem.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-rkl-1/igt@gem_exec_suspend@basic...@smem.html * igt@gem_mmap_gtt@coherency: - {shard-rkl}:[SKIP][18] ([fdo#111656]) -> [PASS][19] [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-6/igt@gem_mmap_...@coherency.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-rkl-5/igt@gem_mmap_...@coherency.html * igt@gem_partial_pwrite_pread@writes-after-reads: - {shard-rkl}:[SKIP][20] ([i915#3282]) -> [PASS][21] +1 similar issue [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/shard-rkl-6/igt@gem_partial_pwrite_pr...@writes-after-reads.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/shard-rkl-5/igt@gem_partial_pwrite_pr...@writes-after-reads.html * igt@gen9_exec_parse@bb-chained: - {shard-rkl}:[SKIP][22] ([i915#2527]) -> [PASS][23] [22]:
Re: [Intel-gfx] [RFC 2/2] drm/i915/display: Add 480 MHz CDCLK steps for RPL-U
On Sat, Jan 07, 2023 at 11:06:43AM +0530, Chaitanya Kumar Borah wrote: > A new step of 480MHz has been added on SKUs that have a RPL-U > device id to support 120Hz displays more efficiently. Use a > new quirk to identify the machine for which this change needs > to be applied. > > BSpec: 55409 > > Signed-off-by: Chaitanya Kumar Borah > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 0c107a38f9d0..a437ac446871 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -1329,6 +1329,27 @@ static const struct intel_cdclk_vals > adlp_cdclk_table[] = { > {} > }; > > +static const struct intel_cdclk_vals rplu_cdclk_table[] = { > + { .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 }, > + { .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 }, Are we missing an entry for 307.2 MHz here? > + { .refclk = 19200, .cdclk = 48, .divider = 2, .ratio = 50 }, > + { .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 }, > + { .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 }, > + > + { .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 }, > + { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 }, And 312 MHz here? > + { .refclk = 24000, .cdclk = 48, .divider = 2, .ratio = 40 }, > + { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 }, > + { .refclk = 24400, .cdclk = 648000, .divider = 2, .ratio = 54 }, Typo in refclk here? Actually, it looks like we may have the same typo in the ADL-P table as well. > + > + { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 }, > + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 }, And missing 307.2 MHz again? Matt > + { .refclk = 38400, .cdclk = 48, .divider = 2, .ratio = 25 }, > + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 }, > + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 }, > + {} > +}; > + > static const struct intel_cdclk_vals dg2_cdclk_table[] = { > { .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, > .waveform = 0x }, > { .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, > .waveform = 0x9248 }, > @@ -3353,6 +3374,8 @@ void intel_init_cdclk_hooks(struct drm_i915_private > *dev_priv) > /* Wa_22011320316:adl-p[a0] */ > if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > dev_priv->display.cdclk.table = adlp_a_step_cdclk_table; > + else if (IS_ADLP_RPLU(dev_priv)) > + dev_priv->display.cdclk.table = rplu_cdclk_table; > else > dev_priv->display.cdclk.table = adlp_cdclk_table; > } else if (IS_ROCKETLAKE(dev_priv)) { > -- > 2.25.1 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
Re: [Intel-gfx] [RFC 1/2] drm/i915: Add rplu sub platform
On Sat, Jan 07, 2023 at 11:06:42AM +0530, Chaitanya Kumar Borah wrote: > Adding RPL-U as a sub platform. In RPL-U a new CDCLK step has > been added so we need to make a distinction between RPL-P > and RPL-U while CDCLK initialization. > > Adding a sub-platform, enables us to make this differentiation > in the code. > > Signed-off-by: Chaitanya Kumar Borah > --- > arch/x86/kernel/early-quirks.c | 1 + > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_pci.c | 1 + > drivers/gpu/drm/i915/intel_device_info.c | 7 +++ > drivers/gpu/drm/i915/intel_device_info.h | 1 + > drivers/gpu/drm/i915/intel_step.c| 3 +++ > include/drm/i915_pciids.h| 7 +-- > 7 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > index a6c1867fc7aa..1ba9926c8974 100644 > --- a/arch/x86/kernel/early-quirks.c > +++ b/arch/x86/kernel/early-quirks.c > @@ -559,6 +559,7 @@ static const struct pci_device_id intel_early_ids[] > __initconst = { > INTEL_ADLN_IDS(_early_ops), > INTEL_RPLS_IDS(_early_ops), > INTEL_RPLP_IDS(_early_ops), > + INTEL_RPLU_IDS(_early_ops), > }; > > struct resource intel_graphics_stolen_res __ro_after_init = > DEFINE_RES_MEM(0, 0); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 48fd82722f12..c88e514728a0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -619,6 +619,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, > IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_N) > #define IS_ADLP_RPLP(dev_priv) \ > IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPL) > +#define IS_ADLP_RPLU(dev_priv) \ > + IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_RPLU) > #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \ > (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00) > #define IS_BDW_ULT(dev_priv) \ > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 6cc65079b18d..e9f3b99b3e00 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -1234,6 +1234,7 @@ static const struct pci_device_id pciidlist[] = { > INTEL_DG1_IDS(_info), > INTEL_RPLS_IDS(_s_info), > INTEL_RPLP_IDS(_p_info), > + INTEL_RPLU_IDS(_p_info), > INTEL_DG2_IDS(_info), > INTEL_ATS_M_IDS(_m_info), > INTEL_MTL_IDS(_info), > diff --git a/drivers/gpu/drm/i915/intel_device_info.c > b/drivers/gpu/drm/i915/intel_device_info.c > index 849baf6c3b3c..88f3da63948b 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -201,6 +201,10 @@ static const u16 subplatform_rpl_ids[] = { > INTEL_RPLP_IDS(0), > }; > > +static const u16 subplatform_rplu_ids[] = { > + INTEL_RPLU_IDS(0), > +}; > + > static const u16 subplatform_g10_ids[] = { > INTEL_DG2_G10_IDS(0), > INTEL_ATS_M150_IDS(0), > @@ -268,6 +272,9 @@ static void intel_device_info_subplatform_init(struct > drm_i915_private *i915) > } else if (find_devid(devid, subplatform_rpl_ids, > ARRAY_SIZE(subplatform_rpl_ids))) { > mask = BIT(INTEL_SUBPLATFORM_RPL); > + } else if (find_devid(devid, subplatform_rplu_ids, > + ARRAY_SIZE(subplatform_rplu_ids))) { > + mask = BIT(INTEL_SUBPLATFORM_RPLU); > } else if (find_devid(devid, subplatform_g10_ids, > ARRAY_SIZE(subplatform_g10_ids))) { > mask = BIT(INTEL_SUBPLATFORM_G10); > diff --git a/drivers/gpu/drm/i915/intel_device_info.h > b/drivers/gpu/drm/i915/intel_device_info.h > index d588e5fd2eea..3e3ca5eb073f 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -127,6 +127,7 @@ enum intel_platform { > * bit set > */ > #define INTEL_SUBPLATFORM_N1 > +#define INTEL_SUBPLATFORM_RPLU 2 > > /* MTL */ > #define INTEL_SUBPLATFORM_M 0 > diff --git a/drivers/gpu/drm/i915/intel_step.c > b/drivers/gpu/drm/i915/intel_step.c > index 84a6fe736a3b..df75057eaa65 100644 > --- a/drivers/gpu/drm/i915/intel_step.c > +++ b/drivers/gpu/drm/i915/intel_step.c > @@ -194,6 +194,9 @@ void intel_step_init(struct drm_i915_private *i915) > } else if (IS_ADLP_RPLP(i915)) { > revids = adlp_rplp_revids; > size = ARRAY_SIZE(adlp_rplp_revids); > + } else if (IS_ADLP_RPLU(i915)) { Since the two blocks are identical, it might be slightly preferable to just combine the conditions; that will also help make it clear that this is intentional and not a copy/paste mistake. } else if (IS_ADLP_RPLU(i915) || IS_ADLP_RPLP(i915)) { ... > + revids = adlp_rplp_revids; > + size =
[Intel-gfx] ✗ Fi.CI.BAT: failure for Add module oriented dmesg output (rev3)
== Series Details == Series: Add module oriented dmesg output (rev3) URL : https://patchwork.freedesktop.org/series/111050/ State : failure == Summary == CI Bug Log - changes from CI_DRM_12560 -> Patchwork_111050v3 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_111050v3 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_111050v3, 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_111050v3/index.html Participating hosts (37 -> 38) -- Additional (2): fi-bsw-kefka fi-rkl-11600 Missing(1): fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_111050v3: ### IGT changes ### Possible regressions * igt@gem_exec_store@basic: - fi-rkl-11600: NOTRUN -> [INCOMPLETE][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111050v3/fi-rkl-11600/igt@gem_exec_st...@basic.html * igt@i915_selftest@live@hangcheck: - fi-ivb-3770:[PASS][2] -> [INCOMPLETE][3] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/fi-ivb-3770/igt@i915_selftest@l...@hangcheck.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111050v3/fi-ivb-3770/igt@i915_selftest@l...@hangcheck.html Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@kms_chamelium_hpd@common-hpd-after-suspend}: - {bat-rpls-2}: NOTRUN -> [SKIP][4] [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111050v3/bat-rpls-2/igt@kms_chamelium_...@common-hpd-after-suspend.html Known issues Here are the changes found in Patchwork_111050v3 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - fi-rkl-11600: NOTRUN -> [SKIP][5] ([i915#7456]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111050v3/fi-rkl-11600/igt@debugfs_t...@basic-hwmon.html * igt@prime_vgem@basic-fence-flip: - fi-bsw-kefka: NOTRUN -> [SKIP][6] ([fdo#109271]) +17 similar issues [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111050v3/fi-bsw-kefka/igt@prime_v...@basic-fence-flip.html * igt@runner@aborted: - fi-ivb-3770:NOTRUN -> [FAIL][7] ([fdo#109271] / [i915#4312]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111050v3/fi-ivb-3770/igt@run...@aborted.html - fi-rkl-11600: NOTRUN -> [FAIL][8] ([i915#4312]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111050v3/fi-rkl-11600/igt@run...@aborted.html Possible fixes * igt@i915_selftest@live@reset: - {bat-rpls-2}: [DMESG-FAIL][9] ([i915#4983]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/bat-rpls-2/igt@i915_selftest@l...@reset.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111050v3/bat-rpls-2/igt@i915_selftest@l...@reset.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size: - fi-bsw-n3050: [FAIL][11] ([i915#6298]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111050v3/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817 [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983 [i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257 [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298 [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367 [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456 Build changes - * Linux: CI_DRM_12560 -> Patchwork_111050v3 CI-20190529: 20190529 CI_DRM_12560: 0eccbe4822259a5e1a78bb7a4c027514d26a2b23 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7113: 15562e123ee6ed88163c7da2ef330dfe9bbd16a5 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_111050v3: 0eccbe4822259a5e1a78bb7a4c027514d26a2b23 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 6a3eefba7385
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Add module oriented dmesg output (rev3)
== Series Details == Series: Add module oriented dmesg output (rev3) URL : https://patchwork.freedesktop.org/series/111050/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. +drivers/gpu/drm/i915/gt/intel_gt_mcr.c:397:9: warning: context imbalance in 'intel_gt_mcr_lock' - wrong count at exit -O:drivers/gpu/drm/i915/gt/intel_gt_mcr.c:396:9: warning: context imbalance in 'intel_gt_mcr_lock' - wrong count at exit
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Add module oriented dmesg output (rev3)
== Series Details == Series: Add module oriented dmesg output (rev3) URL : https://patchwork.freedesktop.org/series/111050/ State : warning == Summary == Error: dim checkpatch failed 77fe0b8dbc3f drm/i915/gt: Start adding module oriented dmesg output Traceback (most recent call last): File "scripts/spdxcheck.py", line 6, in from ply import lex, yacc ModuleNotFoundError: No module named 'ply' -:361: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #361: new file mode 100644 -:378: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_gt' - possible side-effects? #378: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:13: +#define gt_err(_gt, _fmt, ...) \ + drm_err(&(_gt)->i915->drm, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) -:381: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_gt' - possible side-effects? #381: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:16: +#define gt_warn(_gt, _fmt, ...) \ + drm_warn(&(_gt)->i915->drm, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) -:384: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_gt' - possible side-effects? #384: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:19: +#define gt_notice(_gt, _fmt, ...) \ + drm_notice(&(_gt)->i915->drm, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) -:387: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_gt' - possible side-effects? #387: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:22: +#define gt_info(_gt, _fmt, ...) \ + drm_info(&(_gt)->i915->drm, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) -:390: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_gt' - possible side-effects? #390: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:25: +#define gt_dbg(_gt, _fmt, ...) \ + drm_dbg(&(_gt)->i915->drm, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) -:393: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_gt' - possible side-effects? #393: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:28: +#define gt_err_ratelimited(_gt, _fmt, ...) \ + drm_err_ratelimited(&(_gt)->i915->drm, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) -:396: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_gt' - possible side-effects? #396: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:31: +#define gt_probe_error(_gt, _fmt, ...) \ + do { \ + if (i915_error_injected()) \ + gt_dbg(_gt, _fmt, ##__VA_ARGS__); \ + else \ + gt_err(_gt, _fmt, ##__VA_ARGS__); \ + } while (0) -:396: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_fmt' - possible side-effects? #396: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:31: +#define gt_probe_error(_gt, _fmt, ...) \ + do { \ + if (i915_error_injected()) \ + gt_dbg(_gt, _fmt, ##__VA_ARGS__); \ + else \ + gt_err(_gt, _fmt, ##__VA_ARGS__); \ + } while (0) -:410: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_gt' - possible side-effects? #410: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:45: +#define gt_WARN(_gt, _condition, _fmt, ...) \ + drm_WARN(&(_gt)->i915->drm, _condition, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) -:413: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_gt' - possible side-effects? #413: FILE: drivers/gpu/drm/i915/gt/intel_gt_print.h:48: +#define gt_WARN_ONCE(_gt, _condition, _fmt, ...) \ + drm_WARN_ONCE(&(_gt)->i915->drm, _condition, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) total: 0 errors, 1 warnings, 10 checks, 459 lines checked
[Intel-gfx] [PATCH v3 1/1] drm/i915/gt: Start adding module oriented dmesg output
From: John Harrison When trying to analyse bug reports from CI, customers, etc. it can be difficult to work out exactly what is happening on which GT in a multi-GT system. So add GT oriented debug/error message wrappers. If used instead of the drm_ equivalents, you get the same output but with a GT# prefix on it. v2: Go back to using lower case names (combined review feedback). Convert intel_gt.c as a first step. v3: Add gt_err_ratelimited() as well, undo one conversation that might not have a GT pointer in some scenarios (review feedback from Michal W). Split definitions into separate header (review feedback from Jani). Convert all intel_gt*.c files. Signed-off-by: John Harrison --- drivers/gpu/drm/i915/gt/intel_gt.c| 96 +-- .../gpu/drm/i915/gt/intel_gt_clock_utils.c| 8 +- drivers/gpu/drm/i915/gt/intel_gt_irq.c| 9 +- drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 9 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 +- drivers/gpu/drm/i915/gt/intel_gt_print.h | 51 ++ drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 4 +- drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 34 ++- drivers/gpu/drm/i915/gt/intel_gtt.c | 7 +- 9 files changed, 129 insertions(+), 98 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_print.h diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 75a7cb33c..bb6152da89706 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -22,6 +22,7 @@ #include "intel_gt_debugfs.h" #include "intel_gt_mcr.h" #include "intel_gt_pm.h" +#include "intel_gt_print.h" #include "intel_gt_regs.h" #include "intel_gt_requests.h" #include "intel_migrate.h" @@ -89,9 +90,8 @@ static int intel_gt_probe_lmem(struct intel_gt *gt) if (err == -ENODEV) return 0; - drm_err(>drm, - "Failed to setup region(%d) type=%d\n", - err, INTEL_MEMORY_LOCAL); + gt_err(gt, "Failed to setup region(%d) type=%d\n", + err, INTEL_MEMORY_LOCAL); return err; } @@ -200,14 +200,14 @@ int intel_gt_init_hw(struct intel_gt *gt) ret = i915_ppgtt_init_hw(gt); if (ret) { - drm_err(>drm, "Enabling PPGTT failed (%d)\n", ret); + gt_err(gt, "Enabling PPGTT failed (%d)\n", ret); goto out; } /* We can't enable contexts until all firmware is loaded */ ret = intel_uc_init_hw(>uc); if (ret) { - i915_probe_error(i915, "Enabling uc failed (%d)\n", ret); + gt_probe_error(gt, "Enabling uc failed (%d)\n", ret); goto out; } @@ -257,7 +257,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt, * some errors might have become stuck, * mask them. */ - drm_dbg(>i915->drm, "EIR stuck: 0x%08x, masking\n", eir); + gt_dbg(gt, "EIR stuck: 0x%08x, masking\n", eir); intel_uncore_rmw(uncore, EMR, 0, eir); intel_uncore_write(uncore, GEN2_IIR, I915_MASTER_ERROR_INTERRUPT); @@ -291,16 +291,16 @@ static void gen6_check_faults(struct intel_gt *gt) for_each_engine(engine, gt, id) { fault = GEN6_RING_FAULT_REG_READ(engine); if (fault & RING_FAULT_VALID) { - drm_dbg(>i915->drm, "Unexpected fault\n" - "\tAddr: 0x%08lx\n" - "\tAddress space: %s\n" - "\tSource ID: %d\n" - "\tType: %d\n", - fault & PAGE_MASK, - fault & RING_FAULT_GTTSEL_MASK ? - "GGTT" : "PPGTT", - RING_FAULT_SRCID(fault), - RING_FAULT_FAULT_TYPE(fault)); + gt_dbg(gt, "Unexpected fault\n" + "\tAddr: 0x%08lx\n" + "\tAddress space: %s\n" + "\tSource ID: %d\n" + "\tType: %d\n", + fault & PAGE_MASK, + fault & RING_FAULT_GTTSEL_MASK ? + "GGTT" : "PPGTT", + RING_FAULT_SRCID(fault), + RING_FAULT_FAULT_TYPE(fault)); } } } @@ -327,17 +327,17 @@ static void xehp_check_faults(struct intel_gt *gt) fault_addr = ((u64)(fault_data1 & FAULT_VA_HIGH_BITS) << 44) | ((u64)fault_data0 << 12); - drm_dbg(>i915->drm, "Unexpected fault\n" - "\tAddr: 0x%08x_%08x\n" -
[Intel-gfx] [PATCH v3 0/1] Add module oriented dmesg output
From: John Harrison When trying to analyse bug reports from CI, customers, etc. it can be difficult to work out exactly what is happening on which GT in a multi-GT system. So add GT oriented debug/error message wrappers. If used instead of the drm_ equivalents, you get the same output but with a GT# prefix on it. This patch set updates the gt/intel_gt*.c files to use the new helpers as a first step. The intention would be to convert all output messages throughout the driver as long as they have access to a GT structure. v2: Go back to using lower case names, add more wrapper sets (combined review feedback). Also, wrap up probe injection and WARN entries. v3: Split definitions out to separate header files. Tweak some messages. Wrap a couple more functions. (review feedback from Jani and Michal W). Convert all gt/intel_gt*.c but drop the GuC/HuC/CT files as too much bikeshedding about formatting. Signed-off-by: John Harrison John Harrison (1): drm/i915/gt: Start adding module oriented dmesg output drivers/gpu/drm/i915/gt/intel_gt.c| 96 +-- .../gpu/drm/i915/gt/intel_gt_clock_utils.c| 8 +- drivers/gpu/drm/i915/gt/intel_gt_irq.c| 9 +- drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 9 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 +- drivers/gpu/drm/i915/gt/intel_gt_print.h | 51 ++ drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 4 +- drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 34 ++- drivers/gpu/drm/i915/gt/intel_gtt.c | 7 +- 9 files changed, 129 insertions(+), 98 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_print.h -- 2.39.0
Re: [Intel-gfx] [PATCH v3] drm/i915: Do not cover all future platforms in TLB invalidation
On Mon, Jan 09, 2023 at 12:24:42PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Revert to the original explicit approach and document the reasoning > behind it. > > v2: > * DG2 needs to be covered too. (Matt) > > v3: > * Full version check for Gen12 to avoid catching all future platforms. >(Matt) > > Signed-off-by: Tvrtko Ursulin > Cc: Matt Roper > Cc: Balasubramani Vivekanandan > Cc: Andrzej Hajda > Reviewed-by: Andrzej Hajda # v1 Reviewed-by: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_gt.c | 17 +++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c > b/drivers/gpu/drm/i915/gt/intel_gt.c > index 75a7cb33..5521fa057aab 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -1070,10 +1070,23 @@ static void mmio_invalidate_full(struct intel_gt *gt) > unsigned int num = 0; > unsigned long flags; > > - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) { > + /* > + * New platforms should not be added with catch-all-newer (>=) > + * condition so that any later platform added triggers the below warning > + * and in turn mandates a human cross-check of whether the invalidation > + * flows have compatible semantics. > + * > + * For instance with the 11.00 -> 12.00 transition three out of five > + * respective engine registers were moved to masked type. Then after the > + * 12.00 -> 12.50 transition multi cast handling is required too. > + */ > + > + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50) && > + GRAPHICS_VER_FULL(i915) <= IP_VER(12, 55)) { > regs = NULL; > num = ARRAY_SIZE(xehp_regs); > - } else if (GRAPHICS_VER(i915) == 12) { > + } else if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 0) || > +GRAPHICS_VER_FULL(i915) == IP_VER(12, 10)) { > regs = gen12_regs; > num = ARRAY_SIZE(gen12_regs); > } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) { > -- > 2.34.1 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
[Intel-gfx] ✗ Fi.CI.BAT: failure for ACPI: Fix selecting the wrong ACPI fwnode for the iGPU on some Dell laptops
== Series Details == Series: ACPI: Fix selecting the wrong ACPI fwnode for the iGPU on some Dell laptops URL : https://patchwork.freedesktop.org/series/112572/ State : failure == Summary == CI Bug Log - changes from CI_DRM_12560 -> Patchwork_112572v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_112572v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_112572v1, 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_112572v1/index.html Participating hosts (37 -> 39) -- Additional (3): fi-kbl-soraka fi-rkl-11600 fi-bsw-kefka Missing(1): fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_112572v1: ### IGT changes ### Possible regressions * igt@i915_selftest@live@guc: - fi-kbl-soraka: NOTRUN -> [INCOMPLETE][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-kbl-soraka/igt@i915_selftest@l...@guc.html Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@gem_exec_suspend@basic-s0@smem: - {bat-rpls-1}: NOTRUN -> [DMESG-WARN][2] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/bat-rpls-1/igt@gem_exec_suspend@basic...@smem.html * {igt@kms_chamelium_hpd@common-hpd-after-suspend}: - {bat-rpls-2}: NOTRUN -> [SKIP][3] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/bat-rpls-2/igt@kms_chamelium_...@common-hpd-after-suspend.html * {igt@kms_chamelium_hpd@dp-hpd-fast}: - fi-rkl-11600: NOTRUN -> [SKIP][4] +7 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-rkl-11600/igt@kms_chamelium_...@dp-hpd-fast.html Known issues Here are the changes found in Patchwork_112572v1 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - fi-rkl-11600: NOTRUN -> [SKIP][5] ([i915#7456]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-rkl-11600/igt@debugfs_t...@basic-hwmon.html * igt@gem_exec_gttfill@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][6] ([fdo#109271]) +7 similar issues [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-kbl-soraka/igt@gem_exec_gttf...@basic.html * igt@gem_huc_copy@huc-copy: - fi-rkl-11600: NOTRUN -> [SKIP][7] ([i915#2190]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-rkl-11600/igt@gem_huc_c...@huc-copy.html - fi-kbl-soraka: NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#2190]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-kbl-soraka/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][9] ([fdo#109271] / [i915#4613]) +3 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-kbl-soraka/igt@gem_lmem_swapp...@basic.html * igt@gem_lmem_swapping@parallel-random-engines: - fi-rkl-11600: NOTRUN -> [SKIP][10] ([i915#4613]) +3 similar issues [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-rkl-11600/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@gem_tiled_pread_basic: - fi-rkl-11600: NOTRUN -> [SKIP][11] ([i915#3282]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-rkl-11600/igt@gem_tiled_pread_basic.html * igt@i915_pm_backlight@basic-brightness: - fi-rkl-11600: NOTRUN -> [SKIP][12] ([i915#7561]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-rkl-11600/igt@i915_pm_backli...@basic-brightness.html * igt@i915_selftest@live@gt_heartbeat: - fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][13] ([i915#5334]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html - fi-apl-guc: [PASS][14] -> [DMESG-FAIL][15] ([i915#5334]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html * igt@i915_selftest@live@gt_pm: - fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][16] ([i915#1886]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112572v1/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html * igt@i915_suspend@basic-s3-without-i915: - fi-rkl-11600: NOTRUN -> [INCOMPLETE][17] ([i915#4817]) [17]:
Re: [Intel-gfx] [PATCH 2/2] drm_print: fix stale macro-name in comment
On Thu, Jan 5, 2023 at 5:46 AM Daniel Vetter wrote: > > On Mon, Dec 05, 2022 at 09:10:05AM -0700, Jim Cromie wrote: > > Cited commit uses stale macro name, fix this, and explain better. > > > > When DRM_USE_DYNAMIC_DEBUG=y, DYNDBG_CLASSMAP_DEFINE() maps DRM_UT_* > > onto BITs in drm.debug. This still uses enum drm_debug_category, but > > it is somewhat indirect, with the ordered set of DRM_UT_* enum-vals. > > This requires that the macro args: DRM_UT_* list must be kept in sync > > and in order. > > > > Fixes: f158936b60a7 ("drm: POC drm on dyndbg - use in core, 2 helpers, 3 > > drivers.") > > Signed-off-by: Jim Cromie > > What's the status of this series? > dead - superseded by https://patchwork.freedesktop.org/series/111652/ which is still WIP, but improved since that post. I'll resubmit soon, with same title so patchwork calls it rev 2 > Greg, you landed the original entire pile that wasn't quite ready yet? Or > should I apply these two? > -Daniel > > > --- > > . emphasize ABI non-change despite enum val change - Jani Nikula > > . reorder to back of patchset to follow API name changes. > > --- > > include/drm/drm_print.h | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > > index a44fb7ef257f..e4c0c7e6d49d 100644 > > --- a/include/drm/drm_print.h > > +++ b/include/drm/drm_print.h > > @@ -276,7 +276,10 @@ static inline struct drm_printer drm_err_printer(const > > char *prefix) > > * > > */ > > enum drm_debug_category { > > - /* These names must match those in DYNAMIC_DEBUG_CLASSBITS */ > > + /* > > + * Keep DYNDBG_CLASSMAP_DEFINE args in sync with changes here, > > + * the enum-values define BIT()s in drm.debug, so are ABI. > > + */ > > /** > >* @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c, > >* drm_memory.c, ... > > -- > > 2.38.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
[Intel-gfx] ✓ Fi.CI.BAT: success for kvm/vfio: fix potential deadlock on vfio group lock
== Series Details == Series: kvm/vfio: fix potential deadlock on vfio group lock URL : https://patchwork.freedesktop.org/series/112568/ State : success == Summary == CI Bug Log - changes from CI_DRM_12560 -> Patchwork_112568v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/index.html Participating hosts (37 -> 39) -- Additional (3): fi-kbl-soraka fi-rkl-11600 fi-bsw-kefka Missing(1): fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_112568v1: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@kms_chamelium_hpd@common-hpd-after-suspend}: - {bat-rpls-2}: NOTRUN -> [SKIP][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/bat-rpls-2/igt@kms_chamelium_...@common-hpd-after-suspend.html * {igt@kms_chamelium_hpd@dp-hpd-fast}: - fi-rkl-11600: NOTRUN -> [SKIP][2] +7 similar issues [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-rkl-11600/igt@kms_chamelium_...@dp-hpd-fast.html Known issues Here are the changes found in Patchwork_112568v1 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - fi-rkl-11600: NOTRUN -> [SKIP][3] ([i915#7456]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-rkl-11600/igt@debugfs_t...@basic-hwmon.html * igt@gem_exec_gttfill@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][4] ([fdo#109271]) +7 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-kbl-soraka/igt@gem_exec_gttf...@basic.html * igt@gem_huc_copy@huc-copy: - fi-rkl-11600: NOTRUN -> [SKIP][5] ([i915#2190]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-rkl-11600/igt@gem_huc_c...@huc-copy.html - fi-kbl-soraka: NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#2190]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-kbl-soraka/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#4613]) +3 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-kbl-soraka/igt@gem_lmem_swapp...@basic.html * igt@gem_lmem_swapping@parallel-random-engines: - fi-rkl-11600: NOTRUN -> [SKIP][8] ([i915#4613]) +3 similar issues [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-rkl-11600/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@gem_tiled_pread_basic: - fi-rkl-11600: NOTRUN -> [SKIP][9] ([i915#3282]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-rkl-11600/igt@gem_tiled_pread_basic.html * igt@i915_pm_backlight@basic-brightness: - fi-rkl-11600: NOTRUN -> [SKIP][10] ([i915#7561]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-rkl-11600/igt@i915_pm_backli...@basic-brightness.html * igt@i915_selftest@live@gt_heartbeat: - fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][11] ([i915#5334]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html - fi-apl-guc: [PASS][12] -> [DMESG-FAIL][13] ([i915#5334]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12560/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html * igt@i915_selftest@live@gt_pm: - fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][14] ([i915#1886]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html * igt@i915_selftest@live@ring_submission: - fi-kbl-soraka: NOTRUN -> [INCOMPLETE][15] ([i915#7640]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-kbl-soraka/igt@i915_selftest@live@ring_submission.html * igt@i915_suspend@basic-s3-without-i915: - fi-rkl-11600: NOTRUN -> [INCOMPLETE][16] ([i915#4817]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor: - fi-rkl-11600: NOTRUN -> [SKIP][17] ([i915#4103]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112568v1/fi-rkl-11600/igt@kms_cursor_leg...@basic-busy-flip-before-cursor.html * igt@kms_force_connector_basic@force-load-detect: - fi-rkl-11600: NOTRUN -> [SKIP][18] ([fdo#109285] / [i915#4098]) [18]:
Re: [Intel-gfx] [PATCH 1/2] KVM: async kvm_destroy_vm for vfio devices
LGTM Reviewed-by: Tony Krowiak On 1/9/23 3:10 PM, Matthew Rosato wrote: Currently it is possible that the final put of a KVM reference comes from vfio during its device close operation. This occurs while the vfio group lock is held; however, if the vfio device is still in the kvm device list, then the following call chain could result in a deadlock: kvm_put_kvm -> kvm_destroy_vm -> kvm_destroy_devices -> kvm_vfio_destroy -> kvm_vfio_file_set_kvm -> vfio_file_set_kvm -> group->group_lock/group_rwsem Avoid this scenario by adding kvm_put_kvm_async which will perform the kvm_destroy_vm asynchronously if the refcount reaches 0. Fixes: 421cfe6596f6 ("vfio: remove VFIO_GROUP_NOTIFY_SET_KVM") Reported-by: Alex Williamson Signed-off-by: Matthew Rosato --- drivers/gpu/drm/i915/gvt/kvmgt.c | 6 +- drivers/s390/crypto/vfio_ap_ops.c | 7 ++- include/linux/kvm_host.h | 3 +++ virt/kvm/kvm_main.c | 22 ++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 8ae7039b3683..24511c877572 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -703,7 +703,11 @@ static void intel_vgpu_close_device(struct vfio_device *vfio_dev) kvm_page_track_unregister_notifier(vgpu->vfio_device.kvm, >track_node); - kvm_put_kvm(vgpu->vfio_device.kvm); + /* +* Avoid possible deadlock on any currently-held vfio lock by +* ensuring the potential kvm_destroy_vm call is done asynchronously +*/ + kvm_put_kvm_async(vgpu->vfio_device.kvm); kvmgt_protect_table_destroy(vgpu); gvt_cache_destroy(vgpu); diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index e93bb9c468ce..a37b2baefb36 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1574,7 +1574,12 @@ static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev) kvm_arch_crypto_clear_masks(kvm); vfio_ap_mdev_reset_queues(_mdev->qtable); - kvm_put_kvm(kvm); + /* +* Avoid possible deadlock on any currently-held vfio lock by +* ensuring the potential kvm_destroy_vm call is done +* asynchronously +*/ + kvm_put_kvm_async(kvm); matrix_mdev->kvm = NULL; release_update_locks_for_kvm(kvm); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 4f26b244f6d0..2ef6a5102265 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -793,6 +794,7 @@ struct kvm { struct kvm_stat_data **debugfs_stat_data; struct srcu_struct srcu; struct srcu_struct irq_srcu; + struct work_struct async_work; pid_t userspace_pid; bool override_halt_poll_ns; unsigned int max_halt_poll_ns; @@ -963,6 +965,7 @@ void kvm_exit(void); void kvm_get_kvm(struct kvm *kvm); bool kvm_get_kvm_safe(struct kvm *kvm); void kvm_put_kvm(struct kvm *kvm); +void kvm_put_kvm_async(struct kvm *kvm); bool file_is_kvm(struct file *file); void kvm_put_kvm_no_destroy(struct kvm *kvm); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 13e88297f999..fbe8d127028b 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1353,6 +1353,28 @@ void kvm_put_kvm(struct kvm *kvm) } EXPORT_SYMBOL_GPL(kvm_put_kvm); +static void kvm_put_async_fn(struct work_struct *work) +{ + struct kvm *kvm = container_of(work, struct kvm, + async_work); + + kvm_destroy_vm(kvm); +} + +/* + * Put a reference but only destroy the vm asynchronously. Can be used in + * cases where the caller holds a mutex that could cause deadlock if + * kvm_destroy_vm is triggered + */ +void kvm_put_kvm_async(struct kvm *kvm) +{ + if (refcount_dec_and_test(>users_count)) { + INIT_WORK(>async_work, kvm_put_async_fn); + schedule_work(>async_work); + } +} +EXPORT_SYMBOL_GPL(kvm_put_kvm_async); + /* * Used to put a reference that was taken on behalf of an object associated * with a user-visible file descriptor, e.g. a vcpu or device, if installation
Re: [Intel-gfx] [PATCH] ACPI: Fix selecting the wrong ACPI fwnode for the iGPU on some Dell laptops
p.s. This fixes a regression in 6.1, adding the regressions list to the Cc. Once we figure out the best way to fix this (this patch is more of a proposal how to fix this rather then a definitive fix), we should also backport the fix to 6.1.y. On 1/9/23 21:57, Hans de Goede wrote: > The Dell Latitude E6430 both with and without the optional NVidia dGPU > has a bug in its ACPI tables which is causing Linux to assign the wrong > ACPI fwnode / companion to the pci_device for the i915 iGPU. > > Specifically under the PCI root bridge there are these 2 ACPI Device()s : > > Scope (_SB.PCI0) > { > Device (GFX0) > { > Name (_ADR, 0x0002) // _ADR: Address > } > > ... > > Device (VID) > { > Name (_ADR, 0x0002) // _ADR: Address > ... > > Method (_DOS, 1, NotSerialized) // _DOS: Disable Output Switching > { > VDP8 = Arg0 > VDP1 (One, VDP8) > } > > Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices > { > ... > } > ... > } > } > > The non-functional GFX0 ACPI device is a problem, because this gets > returned as ACPI companion-device by acpi_find_child_device() for the iGPU. > > This is a long standing problem and the i915 driver does use the ACPI > companion for some things, but works fine without it. > > However since commit 63f534b8bad9 ("ACPI: PCI: Rework acpi_get_pci_dev()") > acpi_get_pci_dev() relies on the physical-node pointer in the acpi_device > and that is set on the wrong acpi_device because of the wrong > acpi_find_child_device() return. This breaks the ACPI video code, leading > to non working backlight control in some cases. > > Make find_child_checks() return a higher score for children which have > pnp-ids set by various scan helpers like acpi_is_video_device(), so > that it picks the right companion-device. > > An alternative approach would be to directly call acpi_is_video_device() > from find_child_checks() but that would be somewhat computationally > expensive given that acpi_find_child_device() iterates over all the > PCI0 children every time it is called. > > Fixes: 63f534b8bad9 ("ACPI: PCI: Rework acpi_get_pci_dev()") > Signed-off-by: Hans de Goede > --- > drivers/acpi/glue.c | 14 -- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c > index 204fe94c7e45..2055dfd7678b 100644 > --- a/drivers/acpi/glue.c > +++ b/drivers/acpi/glue.c > @@ -75,7 +75,7 @@ static struct acpi_bus_type *acpi_get_bus_type(struct > device *dev) > } > > #define FIND_CHILD_MIN_SCORE 1 > -#define FIND_CHILD_MAX_SCORE 2 > +#define FIND_CHILD_MAX_SCORE 3 > > static int match_any(struct acpi_device *adev, void *not_used) > { > @@ -89,15 +89,25 @@ static bool acpi_dev_has_children(struct acpi_device > *adev) > > static int find_child_checks(struct acpi_device *adev, bool check_children) > { > + int score = FIND_CHILD_MIN_SCORE; > unsigned long long sta; > acpi_status status; > > if (check_children && !acpi_dev_has_children(adev)) > return -ENODEV; > > + /* > + * For devices without a _STA method, prefer devices without a _HID > + * (which conflicts with having an _ADR) but which have been matched > + * in some other way, like e.g. by acpi_is_video_device() over devices > + * with no ids at all. > + */ > + if (!adev->pnp.type.platform_id && adev->pnp.type.hardware_id) > + score++; > + > status = acpi_evaluate_integer(adev->handle, "_STA", NULL, ); > if (status == AE_NOT_FOUND) > - return FIND_CHILD_MIN_SCORE; > + return score; > > if (ACPI_FAILURE(status) || !(sta & ACPI_STA_DEVICE_ENABLED)) > return -ENODEV;
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for kvm/vfio: fix potential deadlock on vfio group lock
== Series Details == Series: kvm/vfio: fix potential deadlock on vfio group lock URL : https://patchwork.freedesktop.org/series/112568/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. - +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
[Intel-gfx] [PATCH] ACPI: Fix selecting the wrong ACPI fwnode for the iGPU on some Dell laptops
The Dell Latitude E6430 both with and without the optional NVidia dGPU has a bug in its ACPI tables which is causing Linux to assign the wrong ACPI fwnode / companion to the pci_device for the i915 iGPU. Specifically under the PCI root bridge there are these 2 ACPI Device()s : Scope (_SB.PCI0) { Device (GFX0) { Name (_ADR, 0x0002) // _ADR: Address } ... Device (VID) { Name (_ADR, 0x0002) // _ADR: Address ... Method (_DOS, 1, NotSerialized) // _DOS: Disable Output Switching { VDP8 = Arg0 VDP1 (One, VDP8) } Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices { ... } ... } } The non-functional GFX0 ACPI device is a problem, because this gets returned as ACPI companion-device by acpi_find_child_device() for the iGPU. This is a long standing problem and the i915 driver does use the ACPI companion for some things, but works fine without it. However since commit 63f534b8bad9 ("ACPI: PCI: Rework acpi_get_pci_dev()") acpi_get_pci_dev() relies on the physical-node pointer in the acpi_device and that is set on the wrong acpi_device because of the wrong acpi_find_child_device() return. This breaks the ACPI video code, leading to non working backlight control in some cases. Make find_child_checks() return a higher score for children which have pnp-ids set by various scan helpers like acpi_is_video_device(), so that it picks the right companion-device. An alternative approach would be to directly call acpi_is_video_device() from find_child_checks() but that would be somewhat computationally expensive given that acpi_find_child_device() iterates over all the PCI0 children every time it is called. Fixes: 63f534b8bad9 ("ACPI: PCI: Rework acpi_get_pci_dev()") Signed-off-by: Hans de Goede --- drivers/acpi/glue.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c index 204fe94c7e45..2055dfd7678b 100644 --- a/drivers/acpi/glue.c +++ b/drivers/acpi/glue.c @@ -75,7 +75,7 @@ static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) } #define FIND_CHILD_MIN_SCORE 1 -#define FIND_CHILD_MAX_SCORE 2 +#define FIND_CHILD_MAX_SCORE 3 static int match_any(struct acpi_device *adev, void *not_used) { @@ -89,15 +89,25 @@ static bool acpi_dev_has_children(struct acpi_device *adev) static int find_child_checks(struct acpi_device *adev, bool check_children) { + int score = FIND_CHILD_MIN_SCORE; unsigned long long sta; acpi_status status; if (check_children && !acpi_dev_has_children(adev)) return -ENODEV; + /* +* For devices without a _STA method, prefer devices without a _HID +* (which conflicts with having an _ADR) but which have been matched +* in some other way, like e.g. by acpi_is_video_device() over devices +* with no ids at all. +*/ + if (!adev->pnp.type.platform_id && adev->pnp.type.hardware_id) + score++; + status = acpi_evaluate_integer(adev->handle, "_STA", NULL, ); if (status == AE_NOT_FOUND) - return FIND_CHILD_MIN_SCORE; + return score; if (ACPI_FAILURE(status) || !(sta & ACPI_STA_DEVICE_ENABLED)) return -ENODEV; -- 2.39.0
Re: [Intel-gfx] [RFC PATCH 04/20] drm/sched: Convert drm scheduler to use a work queue rather than kthread
On Mon, Jan 09, 2023 at 06:17:48PM +0100, Boris Brezillon wrote: > Hi Jason, > > On Mon, 9 Jan 2023 09:45:09 -0600 > Jason Ekstrand wrote: > > > On Thu, Jan 5, 2023 at 1:40 PM Matthew Brost > > wrote: > > > > > On Mon, Jan 02, 2023 at 08:30:19AM +0100, Boris Brezillon wrote: > > > > On Fri, 30 Dec 2022 12:55:08 +0100 > > > > Boris Brezillon wrote: > > > > > > > > > On Fri, 30 Dec 2022 11:20:42 +0100 > > > > > Boris Brezillon wrote: > > > > > > > > > > > Hello Matthew, > > > > > > > > > > > > On Thu, 22 Dec 2022 14:21:11 -0800 > > > > > > Matthew Brost wrote: > > > > > > > > > > > > > In XE, the new Intel GPU driver, a choice has made to have a 1 to > > > > > > > 1 > > > > > > > mapping between a drm_gpu_scheduler and drm_sched_entity. At > > > > > > > first > > > this > > > > > > > seems a bit odd but let us explain the reasoning below. > > > > > > > > > > > > > > 1. In XE the submission order from multiple drm_sched_entity is > > > > > > > not > > > > > > > guaranteed to be the same completion even if targeting the same > > > hardware > > > > > > > engine. This is because in XE we have a firmware scheduler, the > > > GuC, > > > > > > > which allowed to reorder, timeslice, and preempt submissions. If > > > > > > > a > > > using > > > > > > > shared drm_gpu_scheduler across multiple drm_sched_entity, the > > > > > > > TDR > > > falls > > > > > > > apart as the TDR expects submission order == completion order. > > > Using a > > > > > > > dedicated drm_gpu_scheduler per drm_sched_entity solve this > > > problem. > > > > > > > > > > > > Oh, that's interesting. I've been trying to solve the same sort of > > > > > > issues to support Arm's new Mali GPU which is relying on a > > > FW-assisted > > > > > > scheduling scheme (you give the FW N streams to execute, and it does > > > > > > the scheduling between those N command streams, the kernel driver > > > > > > does timeslice scheduling to update the command streams passed to > > > > > > the > > > > > > FW). I must admit I gave up on using drm_sched at some point, mostly > > > > > > because the integration with drm_sched was painful, but also > > > > > > because > > > I > > > > > > felt trying to bend drm_sched to make it interact with a > > > > > > timeslice-oriented scheduling model wasn't really future proof. > > > Giving > > > > > > drm_sched_entity exlusive access to a drm_gpu_scheduler probably > > > might > > > > > > help for a few things (didn't think it through yet), but I feel it's > > > > > > coming short on other aspects we have to deal with on Arm GPUs. > > > > > > > > > > Ok, so I just had a quick look at the Xe driver and how it > > > > > instantiates the drm_sched_entity and drm_gpu_scheduler, and I think I > > > > > have a better understanding of how you get away with using drm_sched > > > > > while still controlling how scheduling is really done. Here > > > > > drm_gpu_scheduler is just a dummy abstract that let's you use the > > > > > drm_sched job queuing/dep/tracking mechanism. The whole run-queue > > > > > > You nailed it here, we use the DRM scheduler for queuing jobs, > > > dependency tracking and releasing jobs to be scheduled when dependencies > > > are met, and lastly a tracking mechanism of inflights jobs that need to > > > be cleaned up if an error occurs. It doesn't actually do any scheduling > > > aside from the most basic level of not overflowing the submission ring > > > buffer. In this sense, a 1 to 1 relationship between entity and > > > scheduler fits quite well. > > > > > > > Yeah, I think there's an annoying difference between what AMD/NVIDIA/Intel > > want here and what you need for Arm thanks to the number of FW queues > > available. I don't remember the exact number of GuC queues but it's at > > least 1k. This puts it in an entirely different class from what you have on > > Mali. Roughly, there's about three categories here: > > > > 1. Hardware where the kernel is placing jobs on actual HW rings. This is > > old Mali, Intel Haswell and earlier, and probably a bunch of others. > > (Intel BDW+ with execlists is a weird case that doesn't fit in this > > categorization.) > > > > 2. Hardware (or firmware) with a very limited number of queues where > > you're going to have to juggle in the kernel in order to run desktop Linux. > > > > 3. Firmware scheduling with a high queue count. In this case, you don't > > want the kernel scheduling anything. Just throw it at the firmware and let > > it go br. If we ever run out of queues (unlikely), the kernel can > > temporarily pause some low-priority contexts and do some juggling or, > > frankly, just fail userspace queue creation and tell the user to close some > > windows. > > > > The existence of this 2nd class is a bit annoying but it's where we are. I > > think it's worth recognizing that Xe and panfrost are in different places > > here and will require different designs. For Xe, we really are just
Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Add GuC CT specific debug print wrappers
On 1/9/2023 01:39, Tvrtko Ursulin wrote: On 06/01/2023 18:57, John Harrison wrote: On 12/6/2022 03:06, Tvrtko Ursulin wrote: On 05/12/2022 18:44, Michal Wajdeczko wrote: On 05.12.2022 14:16, Tvrtko Ursulin wrote: On 02/12/2022 20:14, John Harrison wrote: [snip] Random meaningless (to me) message that is apparently a display thing: drm_dbg_kms(_priv->drm, "disabling %s\n", pll->info->name); i915 :00:02.0: [drm:intel_disable_shared_dpll [i915]] disabling PORT PLL B Plan is to not touch outside gt/. For some unexplicable reason that means it is almost impossible to see the actual problems in most CI dmesg logs because they are swamped with irrelevant display messages that cannot be filtered out. For example, I recently manually grep'd out all the display spam from a bug report log. The dmesg file went from 12MB to 700KB. That is a significant problem that makes bug triage way harder than it needs to be. I didn't get this part, how it would reduce the amount of spam by adding new macros? Anyway, that's something to split out and discuss with display folks. It will allow someone to trivially filter out everything with that tag. Which then makes it orders of magnitude easy to scan through the log to see what happened. Maybe as a way forward work could be split? If John wants to deal with gt_xxx macros, avoid touching GuC (putting his original motivation aside) and you want to convert the gt/uc folder? Assuming John you are okay with "GuC:" and "CT:" prefixes. Meaning just repost patch #1 only and expand to more intel_gt_* files? Sure, if someone will actually reply to that patch with some kind of r-b first so I know I'm not still wasting my time on a huge re-write that will to be redone multiple times when someone objects to the use of a colon or the lack of spaces, braces or whatever. First patch looks good to me (ack in principle) apart that Michal found one potential null pointer dereference if I understood it right. That other comment about the ratelimited call is maybe okay to leave for later, *if* it will be a single instance, otherwise needs a gt logger as well. I can r-b once you re-send with the first issue fixed. I've already fixed those two issues locally. I'm not going to touch the TRACE macros. Okay. I'll extend it further and repost. John. Regards, Tvrtko
Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Add GuC CT specific debug print wrappers
On 1/9/2023 01:38, Jani Nikula wrote: On Fri, 06 Jan 2023, John Harrison wrote: On 12/6/2022 03:06, Tvrtko Ursulin wrote: On 05/12/2022 18:44, Michal Wajdeczko wrote: On 05.12.2022 14:16, Tvrtko Ursulin wrote: On 02/12/2022 20:14, John Harrison wrote: [snip] Random meaningless (to me) message that is apparently a display thing: drm_dbg_kms(_priv->drm, "disabling %s\n", pll->info->name); i915 :00:02.0: [drm:intel_disable_shared_dpll [i915]] disabling PORT PLL B Plan is to not touch outside gt/. For some unexplicable reason that means it is almost impossible to see the actual problems in most CI dmesg logs because they are swamped with irrelevant display messages that cannot be filtered out. For example, I recently manually grep'd out all the display spam from a bug report log. The dmesg file went from 12MB to 700KB. That is a significant problem that makes bug triage way harder than it needs to be. You can adjust drm.debug module parameter to get rid of almost all display debugs. They're logged using the appropriate debug categories. No, you can't. See above comment about 'most CI dmesg logs'. This is when trying to triage bugs created by the CI systems. In that case, the log already exists and it was generated at full debug and it is tens if not hundreds of MBs in size. And there is no single tag attached to the display messages to run 'grep -v' on. They are just a random collection of disparate function names. John. BR, Jani.
Re: [Intel-gfx] [PATCH 1/2] KVM: async kvm_destroy_vm for vfio devices
On 1/9/23 3:13 PM, Jason Gunthorpe wrote: > On Mon, Jan 09, 2023 at 03:10:36PM -0500, Matthew Rosato wrote: >> Currently it is possible that the final put of a KVM reference comes from >> vfio during its device close operation. This occurs while the vfio group >> lock is held; however, if the vfio device is still in the kvm device list, >> then the following call chain could result in a deadlock: >> >> kvm_put_kvm >> -> kvm_destroy_vm >> -> kvm_destroy_devices >>-> kvm_vfio_destroy >> -> kvm_vfio_file_set_kvm >> -> vfio_file_set_kvm >> -> group->group_lock/group_rwsem >> >> Avoid this scenario by adding kvm_put_kvm_async which will perform the >> kvm_destroy_vm asynchronously if the refcount reaches 0. >> >> Fixes: 421cfe6596f6 ("vfio: remove VFIO_GROUP_NOTIFY_SET_KVM") >> Reported-by: Alex Williamson >> Signed-off-by: Matthew Rosato >> --- >> drivers/gpu/drm/i915/gvt/kvmgt.c | 6 +- >> drivers/s390/crypto/vfio_ap_ops.c | 7 ++- >> include/linux/kvm_host.h | 3 +++ >> virt/kvm/kvm_main.c | 22 ++ >> 4 files changed, 36 insertions(+), 2 deletions(-) > > Why two patches? > > It looks OK to me Mentioned in the cover, the fixes: tag is different on the 2nd patch as the s390 PCI passthrough kvm_puts were added later soemtime after 421cfe6596f6 ("vfio: remove VFIO_GROUP_NOTIFY_SET_KVM"). The blamed commit for those changes also landed in a different release (6.0 vs 5.19). But, now that you mention it, neither is an LTS so it probably doesn't matter all that much and could be squashed if preferred. > > Reviewed-by: Jason Gunthorpe Thanks! > > Jason
Re: [Intel-gfx] [PATCH 1/2] KVM: async kvm_destroy_vm for vfio devices
On Mon, Jan 09, 2023 at 03:10:36PM -0500, Matthew Rosato wrote: > Currently it is possible that the final put of a KVM reference comes from > vfio during its device close operation. This occurs while the vfio group > lock is held; however, if the vfio device is still in the kvm device list, > then the following call chain could result in a deadlock: > > kvm_put_kvm > -> kvm_destroy_vm > -> kvm_destroy_devices >-> kvm_vfio_destroy > -> kvm_vfio_file_set_kvm > -> vfio_file_set_kvm > -> group->group_lock/group_rwsem > > Avoid this scenario by adding kvm_put_kvm_async which will perform the > kvm_destroy_vm asynchronously if the refcount reaches 0. > > Fixes: 421cfe6596f6 ("vfio: remove VFIO_GROUP_NOTIFY_SET_KVM") > Reported-by: Alex Williamson > Signed-off-by: Matthew Rosato > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 +- > drivers/s390/crypto/vfio_ap_ops.c | 7 ++- > include/linux/kvm_host.h | 3 +++ > virt/kvm/kvm_main.c | 22 ++ > 4 files changed, 36 insertions(+), 2 deletions(-) Why two patches? It looks OK to me Reviewed-by: Jason Gunthorpe Jason
[Intel-gfx] [PATCH 2/2] KVM: s390: pci: use asyncronous kvm put
It's possible that the kvm refcount will reach 0 at this point while the associated device is still in kvm device list - this would result in a deadlock on the vfio group lock. Avoid this possibility by using kvm_put_kvm_async to do the kvm_destroy_vm asynchronously. Fixes: 09340b2fca00 ("KVM: s390: pci: add routines to start/stop interpretive execution") Signed-off-by: Matthew Rosato --- arch/s390/kvm/pci.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c index ec51e810e381..d1d528438138 100644 --- a/arch/s390/kvm/pci.c +++ b/arch/s390/kvm/pci.c @@ -509,7 +509,7 @@ static int kvm_s390_pci_register_kvm(void *opaque, struct kvm *kvm) kvm_s390_pci_dev_release(zdev); mutex_unlock(>lock); mutex_unlock(>kzdev_lock); - kvm_put_kvm(kvm); + kvm_put_kvm_async(kvm); return rc; } @@ -567,7 +567,11 @@ static void kvm_s390_pci_unregister_kvm(void *opaque) mutex_unlock(>lock); mutex_unlock(>kzdev_lock); - kvm_put_kvm(kvm); + /* +* Avoid possible deadlock on any currently-held vfio lock by +* ensuring the potential kvm_destroy_vm call is done asynchronously +*/ + kvm_put_kvm_async(kvm); } void kvm_s390_pci_init_list(struct kvm *kvm) -- 2.39.0
[Intel-gfx] [PATCH 0/2] kvm/vfio: fix potential deadlock on vfio group lock
Hi Alex, Paolo, As reported by Alex [1], since commit 421cfe6596f6 it is possible for a kvm_put_kvm call to hit a refcount of 0 and trigger kvm_destroy_vm while the vfio group lock is held. However, if this occurs, and the associated group is still in the kvm device list, this thread of execution will attempt to acquire the vfio group lock again, resulting in a deadlock. This series proposes to resolve this by adding a new kvm_put_kvm_async which behaves the same as kvm_put_kvm but, in the case where the refcount hits 0, will use a workqueue to perform the kvm_destroy_vm asynchronously. The fix is provided in 2 patches because s390 PCI passthrough has the same issue, albeit introduced slightly later via a different commit. [1]: https://lore.kernel.org/kvm/20230105150930.6ee65182.alex.william...@redhat.com/ Matthew Rosato (2): KVM: async kvm_destroy_vm for vfio devices KVM: s390: pci: use asyncronous kvm put arch/s390/kvm/pci.c | 8 ++-- drivers/gpu/drm/i915/gvt/kvmgt.c | 6 +- drivers/s390/crypto/vfio_ap_ops.c | 7 ++- include/linux/kvm_host.h | 3 +++ virt/kvm/kvm_main.c | 22 ++ 5 files changed, 42 insertions(+), 4 deletions(-) -- 2.39.0
[Intel-gfx] [PATCH 1/2] KVM: async kvm_destroy_vm for vfio devices
Currently it is possible that the final put of a KVM reference comes from vfio during its device close operation. This occurs while the vfio group lock is held; however, if the vfio device is still in the kvm device list, then the following call chain could result in a deadlock: kvm_put_kvm -> kvm_destroy_vm -> kvm_destroy_devices -> kvm_vfio_destroy -> kvm_vfio_file_set_kvm -> vfio_file_set_kvm -> group->group_lock/group_rwsem Avoid this scenario by adding kvm_put_kvm_async which will perform the kvm_destroy_vm asynchronously if the refcount reaches 0. Fixes: 421cfe6596f6 ("vfio: remove VFIO_GROUP_NOTIFY_SET_KVM") Reported-by: Alex Williamson Signed-off-by: Matthew Rosato --- drivers/gpu/drm/i915/gvt/kvmgt.c | 6 +- drivers/s390/crypto/vfio_ap_ops.c | 7 ++- include/linux/kvm_host.h | 3 +++ virt/kvm/kvm_main.c | 22 ++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 8ae7039b3683..24511c877572 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -703,7 +703,11 @@ static void intel_vgpu_close_device(struct vfio_device *vfio_dev) kvm_page_track_unregister_notifier(vgpu->vfio_device.kvm, >track_node); - kvm_put_kvm(vgpu->vfio_device.kvm); + /* +* Avoid possible deadlock on any currently-held vfio lock by +* ensuring the potential kvm_destroy_vm call is done asynchronously +*/ + kvm_put_kvm_async(vgpu->vfio_device.kvm); kvmgt_protect_table_destroy(vgpu); gvt_cache_destroy(vgpu); diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index e93bb9c468ce..a37b2baefb36 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1574,7 +1574,12 @@ static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev) kvm_arch_crypto_clear_masks(kvm); vfio_ap_mdev_reset_queues(_mdev->qtable); - kvm_put_kvm(kvm); + /* +* Avoid possible deadlock on any currently-held vfio lock by +* ensuring the potential kvm_destroy_vm call is done +* asynchronously +*/ + kvm_put_kvm_async(kvm); matrix_mdev->kvm = NULL; release_update_locks_for_kvm(kvm); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 4f26b244f6d0..2ef6a5102265 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -793,6 +794,7 @@ struct kvm { struct kvm_stat_data **debugfs_stat_data; struct srcu_struct srcu; struct srcu_struct irq_srcu; + struct work_struct async_work; pid_t userspace_pid; bool override_halt_poll_ns; unsigned int max_halt_poll_ns; @@ -963,6 +965,7 @@ void kvm_exit(void); void kvm_get_kvm(struct kvm *kvm); bool kvm_get_kvm_safe(struct kvm *kvm); void kvm_put_kvm(struct kvm *kvm); +void kvm_put_kvm_async(struct kvm *kvm); bool file_is_kvm(struct file *file); void kvm_put_kvm_no_destroy(struct kvm *kvm); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 13e88297f999..fbe8d127028b 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1353,6 +1353,28 @@ void kvm_put_kvm(struct kvm *kvm) } EXPORT_SYMBOL_GPL(kvm_put_kvm); +static void kvm_put_async_fn(struct work_struct *work) +{ + struct kvm *kvm = container_of(work, struct kvm, + async_work); + + kvm_destroy_vm(kvm); +} + +/* + * Put a reference but only destroy the vm asynchronously. Can be used in + * cases where the caller holds a mutex that could cause deadlock if + * kvm_destroy_vm is triggered + */ +void kvm_put_kvm_async(struct kvm *kvm) +{ + if (refcount_dec_and_test(>users_count)) { + INIT_WORK(>async_work, kvm_put_async_fn); + schedule_work(>async_work); + } +} +EXPORT_SYMBOL_GPL(kvm_put_kvm_async); + /* * Used to put a reference that was taken on behalf of an object associated * with a user-visible file descriptor, e.g. a vcpu or device, if installation -- 2.39.0
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Cover rest of SVG unit MCR registers (rev2)
== Series Details == Series: drm/i915/gt: Cover rest of SVG unit MCR registers (rev2) URL : https://patchwork.freedesktop.org/series/112440/ State : success == Summary == CI Bug Log - changes from CI_DRM_12559 -> Patchwork_112440v2 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/index.html Participating hosts (38 -> 38) -- Additional (1): fi-kbl-soraka Missing(1): fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_112440v2: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@kms_chamelium_hpd@common-hpd-after-suspend}: - {bat-adlp-6}: NOTRUN -> [SKIP][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/bat-adlp-6/igt@kms_chamelium_...@common-hpd-after-suspend.html - fi-rkl-11600: NOTRUN -> [SKIP][2] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/fi-rkl-11600/igt@kms_chamelium_...@common-hpd-after-suspend.html Known issues Here are the changes found in Patchwork_112440v2 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_gttfill@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][3] ([fdo#109271]) +7 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/fi-kbl-soraka/igt@gem_exec_gttf...@basic.html * igt@gem_exec_suspend@basic-s3@smem: - fi-rkl-11600: NOTRUN -> [FAIL][4] ([fdo#103375]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/fi-rkl-11600/igt@gem_exec_suspend@basic...@smem.html * igt@gem_huc_copy@huc-copy: - fi-kbl-soraka: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#2190]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/fi-kbl-soraka/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#4613]) +3 similar issues [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/fi-kbl-soraka/igt@gem_lmem_swapp...@basic.html * igt@i915_selftest@live@gt_pm: - fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][7] ([i915#1886]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html * igt@i915_selftest@live@ring_submission: - fi-kbl-soraka: NOTRUN -> [INCOMPLETE][8] ([i915#7640]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/fi-kbl-soraka/igt@i915_selftest@live@ring_submission.html * igt@kms_setmode@basic-clone-single-crtc: - fi-snb-2600:NOTRUN -> [SKIP][9] ([fdo#109271]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/fi-snb-2600/igt@kms_setm...@basic-clone-single-crtc.html Possible fixes * igt@i915_selftest@live@requests: - {bat-adlp-6}: [INCOMPLETE][10] ([i915#6257]) -> [PASS][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/bat-adlp-6/igt@i915_selftest@l...@requests.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/bat-adlp-6/igt@i915_selftest@l...@requests.html * igt@i915_suspend@basic-s3-without-i915: - fi-rkl-11600: [INCOMPLETE][12] ([i915#4817]) -> [PASS][13] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112440v2/fi-rkl-11600/igt@i915_susp...@basic-s3-without-i915.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886 [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817 [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983 [i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257 [i915#7640]: https://gitlab.freedesktop.org/drm/intel/issues/7640 Build changes - * Linux: CI_DRM_12559 -> Patchwork_112440v2 CI-20190529: 20190529 CI_DRM_12559: b4a7d5ac8957c0b894b81d125b15b1ff45eaf5b2 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7113: 15562e123ee6ed88163c7da2ef330dfe9bbd16a5 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_112440v2: b4a7d5ac8957c0b894b81d125b15b1ff45eaf5b2 @
Re: [Intel-gfx] [PATCH v2 9/9] drm/i915/display/misc: use intel_de_rmw if possible
On Thu, Jan 05, 2023 at 02:10:46PM +0100, Andrzej Hajda wrote: > The helper makes the code more compact and readable. > > Signed-off-by: Andrzej Hajda > --- > drivers/gpu/drm/i915/display/g4x_dp.c | 12 > drivers/gpu/drm/i915/display/intel_drrs.c | 12 +++- > drivers/gpu/drm/i915/display/intel_dvo.c | 7 ++- > drivers/gpu/drm/i915/display/intel_lvds.c | 12 > drivers/gpu/drm/i915/display/intel_tv.c | 18 +- > 5 files changed, 18 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c > b/drivers/gpu/drm/i915/display/g4x_dp.c > index 24ef36ec2d3d3c..9629b174ec5d2c 100644 > --- a/drivers/gpu/drm/i915/display/g4x_dp.c > +++ b/drivers/gpu/drm/i915/display/g4x_dp.c > @@ -136,16 +136,12 @@ static void intel_dp_prepare(struct intel_encoder > *encoder, > > intel_dp->DP |= DP_PIPE_SEL_IVB(crtc->pipe); > } else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) { > - u32 trans_dp; > - > intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT; > > - trans_dp = intel_de_read(dev_priv, TRANS_DP_CTL(crtc->pipe)); > - if (drm_dp_enhanced_frame_cap(intel_dp->dpcd)) > - trans_dp |= TRANS_DP_ENH_FRAMING; > - else > - trans_dp &= ~TRANS_DP_ENH_FRAMING; > - intel_de_write(dev_priv, TRANS_DP_CTL(crtc->pipe), trans_dp); > + intel_de_rmw(dev_priv, TRANS_DP_CTL(crtc->pipe), > + TRANS_DP_ENH_FRAMING, > + drm_dp_enhanced_frame_cap(intel_dp->dpcd) ? > + TRANS_DP_ENH_FRAMING : 0); > } else { > if (IS_G4X(dev_priv) && pipe_config->limited_color_range) > intel_dp->DP |= DP_COLOR_RANGE_16_235; > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c > b/drivers/gpu/drm/i915/display/intel_drrs.c > index 5b9e3814e9..a52974f5f66042 100644 > --- a/drivers/gpu/drm/i915/display/intel_drrs.c > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c > @@ -68,21 +68,15 @@ intel_drrs_set_refresh_rate_pipeconf(struct intel_crtc > *crtc, > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > enum transcoder cpu_transcoder = crtc->drrs.cpu_transcoder; > - u32 val, bit; > + u32 bit; > > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > bit = PIPECONF_REFRESH_RATE_ALT_VLV; > else > bit = PIPECONF_REFRESH_RATE_ALT_ILK; > > - val = intel_de_read(dev_priv, PIPECONF(cpu_transcoder)); > - > - if (refresh_rate == DRRS_REFRESH_RATE_LOW) > - val |= bit; > - else > - val &= ~bit; > - > - intel_de_write(dev_priv, PIPECONF(cpu_transcoder), val); > + intel_de_rmw(dev_priv, PIPECONF(cpu_transcoder), > + bit, refresh_rate == DRRS_REFRESH_RATE_LOW ? bit : 0); > } > > static void > diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c > b/drivers/gpu/drm/i915/display/intel_dvo.c > index 4aeae0f3ac9172..77d413781020de 100644 > --- a/drivers/gpu/drm/i915/display/intel_dvo.c > +++ b/drivers/gpu/drm/i915/display/intel_dvo.c > @@ -444,11 +444,8 @@ static bool intel_dvo_init_dev(struct drm_i915_private > *dev_priv, >* the clock enabled before we attempt to initialize >* the device. >*/ > - for_each_pipe(dev_priv, pipe) { > - dpll[pipe] = intel_de_read(dev_priv, DPLL(pipe)); > - intel_de_write(dev_priv, DPLL(pipe), > -dpll[pipe] | DPLL_DVO_2X_MODE); > - } > + for_each_pipe(dev_priv, pipe) > + dpll[pipe] = intel_de_rmw(dev_priv, DPLL(pipe), 0, > DPLL_DVO_2X_MODE); > > ret = dvo->dev_ops->init(_dvo->dev, i2c); > > diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c > b/drivers/gpu/drm/i915/display/intel_lvds.c > index aecec992cd0d2d..e8f47b7ef87649 100644 > --- a/drivers/gpu/drm/i915/display/intel_lvds.c > +++ b/drivers/gpu/drm/i915/display/intel_lvds.c > @@ -316,11 +316,9 @@ static void intel_enable_lvds(struct intel_atomic_state > *state, > struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder); > struct drm_i915_private *dev_priv = to_i915(dev); > > - intel_de_write(dev_priv, lvds_encoder->reg, > -intel_de_read(dev_priv, lvds_encoder->reg) | > LVDS_PORT_EN); > + intel_de_rmw(dev_priv, lvds_encoder->reg, 0, LVDS_PORT_EN); > > - intel_de_write(dev_priv, PP_CONTROL(0), > -intel_de_read(dev_priv, PP_CONTROL(0)) | PANEL_POWER_ON); > + intel_de_rmw(dev_priv, PP_CONTROL(0), 0, PANEL_POWER_ON); > intel_de_posting_read(dev_priv, lvds_encoder->reg); > > if (intel_de_wait_for_set(dev_priv, PP_STATUS(0), PP_ON, 5000)) > @@ -338,14 +336,12 @@ static void intel_disable_lvds(struct > intel_atomic_state *state, > struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder); >
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix timeslots argument for DP DSC SST case (rev2)
== Series Details == Series: drm/i915: Fix timeslots argument for DP DSC SST case (rev2) URL : https://patchwork.freedesktop.org/series/112349/ State : success == Summary == CI Bug Log - changes from CI_DRM_12559 -> Patchwork_112349v2 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/index.html Participating hosts (38 -> 37) -- Additional (1): fi-bsw-kefka Missing(2): fi-rkl-11600 fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_112349v2: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@kms_chamelium_hpd@common-hpd-after-suspend}: - {bat-adlp-6}: NOTRUN -> [SKIP][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/bat-adlp-6/igt@kms_chamelium_...@common-hpd-after-suspend.html Known issues Here are the changes found in Patchwork_112349v2 that come from known issues: ### IGT changes ### Issues hit * igt@i915_selftest@live@gt_heartbeat: - fi-apl-guc: [PASS][2] -> [DMESG-FAIL][3] ([i915#5334]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size: - fi-bsw-n3050: [PASS][4] -> [FAIL][5] ([i915#6298]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html * igt@kms_setmode@basic-clone-single-crtc: - fi-snb-2600:NOTRUN -> [SKIP][6] ([fdo#109271]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/fi-snb-2600/igt@kms_setm...@basic-clone-single-crtc.html * igt@prime_vgem@basic-fence-flip: - fi-bsw-kefka: NOTRUN -> [SKIP][7] ([fdo#109271]) +17 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/fi-bsw-kefka/igt@prime_v...@basic-fence-flip.html Possible fixes * igt@gem_exec_gttfill@basic: - fi-pnv-d510:[FAIL][8] ([i915#7229]) -> [PASS][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/fi-pnv-d510/igt@gem_exec_gttf...@basic.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/fi-pnv-d510/igt@gem_exec_gttf...@basic.html * igt@i915_selftest@live@requests: - {bat-adlp-6}: [INCOMPLETE][10] ([i915#6257]) -> [PASS][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12559/bat-adlp-6/igt@i915_selftest@l...@requests.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/bat-adlp-6/igt@i915_selftest@l...@requests.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#5153]: https://gitlab.freedesktop.org/drm/intel/issues/5153 [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334 [i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257 [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298 [i915#7229]: https://gitlab.freedesktop.org/drm/intel/issues/7229 [i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359 Build changes - * Linux: CI_DRM_12559 -> Patchwork_112349v2 CI-20190529: 20190529 CI_DRM_12559: b4a7d5ac8957c0b894b81d125b15b1ff45eaf5b2 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7113: 15562e123ee6ed88163c7da2ef330dfe9bbd16a5 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_112349v2: b4a7d5ac8957c0b894b81d125b15b1ff45eaf5b2 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112349v2/index.html
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: Cover rest of SVG unit MCR registers (rev2)
== Series Details == Series: drm/i915/gt: Cover rest of SVG unit MCR registers (rev2) URL : https://patchwork.freedesktop.org/series/112440/ State : warning == Summary == Error: dim checkpatch failed 7dc367129881 drm/i915/gt: Cover rest of SVG unit MCR registers -:6: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("")' - ie: 'commit a9e69428b1b4 ("drm/i915: Define MCR registers explicitly")' #6: CHICKEN_RASTER_{1,2} got overlooked with the move done in a9e69428b1b4 total: 1 errors, 0 warnings, 0 checks, 28 lines checked
Re: [Intel-gfx] [PATCH v2 8/9] drm/i915/display/interfaces: use intel_de_rmw if possible
On Thu, Jan 05, 2023 at 02:10:45PM +0100, Andrzej Hajda wrote: > The helper makes the code more compact and readable. > > Signed-off-by: Andrzej Hajda more cases in this patch where we are now always cleaning the bits, but as every other place I believe this is the right thing to do. Reviewed-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 49 +++--- > drivers/gpu/drm/i915/display/intel_fdi.c | 3 +- > drivers/gpu/drm/i915/display/intel_gmbus.c | 30 +++-- > 3 files changed, 22 insertions(+), 60 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 1f5a471a0adf27..500dac59a14157 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -644,19 +644,14 @@ int intel_ddi_toggle_hdcp_bits(struct intel_encoder > *intel_encoder, > struct drm_i915_private *dev_priv = to_i915(dev); > intel_wakeref_t wakeref; > int ret = 0; > - u32 tmp; > > wakeref = intel_display_power_get_if_enabled(dev_priv, > > intel_encoder->power_domain); > if (drm_WARN_ON(dev, !wakeref)) > return -ENXIO; > > - tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder)); > - if (enable) > - tmp |= hdcp_mask; > - else > - tmp &= ~hdcp_mask; > - intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder), tmp); > + intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder), > + hdcp_mask, enable ? hdcp_mask : 0); > intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref); > return ret; > } > @@ -2200,15 +2195,13 @@ static void intel_ddi_enable_fec(struct intel_encoder > *encoder, > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_dp *intel_dp; > - u32 val; > > if (!crtc_state->fec_enable) > return; > > intel_dp = enc_to_intel_dp(encoder); > - val = intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state)); > - val |= DP_TP_CTL_FEC_ENABLE; > - intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), val); > + intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), > + 0, DP_TP_CTL_FEC_ENABLE); > } > > static void intel_ddi_disable_fec_state(struct intel_encoder *encoder, > @@ -2216,15 +2209,13 @@ static void intel_ddi_disable_fec_state(struct > intel_encoder *encoder, > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_dp *intel_dp; > - u32 val; > > if (!crtc_state->fec_enable) > return; > > intel_dp = enc_to_intel_dp(encoder); > - val = intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state)); > - val &= ~DP_TP_CTL_FEC_ENABLE; > - intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), val); > + intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), > + DP_TP_CTL_FEC_ENABLE, 0); > intel_de_posting_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state)); > } > > @@ -2622,12 +2613,10 @@ static void intel_disable_ddi_buf(struct > intel_encoder *encoder, > wait = true; > } > > - if (intel_crtc_has_dp_encoder(crtc_state)) { > - val = intel_de_read(dev_priv, dp_tp_ctl_reg(encoder, > crtc_state)); > - val &= ~(DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK); > - val |= DP_TP_CTL_LINK_TRAIN_PAT1; > - intel_de_write(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), > val); > - } > + if (intel_crtc_has_dp_encoder(crtc_state)) > + intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state), > + DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK, > + DP_TP_CTL_LINK_TRAIN_PAT1); > > /* Disable FEC in DP Sink */ > intel_ddi_disable_fec_state(encoder, crtc_state); > @@ -2660,15 +2649,10 @@ static void intel_ddi_post_disable_dp(struct > intel_atomic_state *state, > if (DISPLAY_VER(dev_priv) >= 12) { > if (is_mst) { > enum transcoder cpu_transcoder = > old_crtc_state->cpu_transcoder; > - u32 val; > > - val = intel_de_read(dev_priv, > - TRANS_DDI_FUNC_CTL(cpu_transcoder)); > - val &= ~(TGL_TRANS_DDI_PORT_MASK | > - TRANS_DDI_MODE_SELECT_MASK); > - intel_de_write(dev_priv, > -TRANS_DDI_FUNC_CTL(cpu_transcoder), > -val); > + intel_de_rmw(dev_priv, > TRANS_DDI_FUNC_CTL(cpu_transcoder), > + TGL_TRANS_DDI_PORT_MASK | > TRANS_DDI_MODE_SELECT_MASK, > +
Re: [Intel-gfx] [PATCH v2 7/9] drm/i915/display/panel: use intel_de_rmw if possible in panel related code
On Thu, Jan 05, 2023 at 02:10:44PM +0100, Andrzej Hajda wrote: > The helper makes the code more compact and readable. > > Signed-off-by: Andrzej Hajda > --- > .../gpu/drm/i915/display/intel_backlight.c| 59 +++ > drivers/gpu/drm/i915/display/intel_pps.c | 14 ++--- > drivers/gpu/drm/i915/display/intel_psr.c | 40 - > 3 files changed, 37 insertions(+), 76 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c > b/drivers/gpu/drm/i915/display/intel_backlight.c > index 5b7da72c95b8c5..b088921c543eaa 100644 > --- a/drivers/gpu/drm/i915/display/intel_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_backlight.c > @@ -349,8 +349,7 @@ static void lpt_disable_backlight(const struct > drm_connector_state *old_conn_sta > intel_de_write(i915, BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE); > } > > - tmp = intel_de_read(i915, BLC_PWM_PCH_CTL1); > - intel_de_write(i915, BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE); > + tmp = intel_de_rmw(i915, BLC_PWM_PCH_CTL1, BLM_PCH_PWM_ENABLE, 0); > } > > static void pch_disable_backlight(const struct drm_connector_state > *old_conn_state, u32 val) > @@ -361,11 +360,9 @@ static void pch_disable_backlight(const struct > drm_connector_state *old_conn_sta > > intel_backlight_set_pwm_level(old_conn_state, val); > > - tmp = intel_de_read(i915, BLC_PWM_CPU_CTL2); > - intel_de_write(i915, BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE); > + intel_de_rmw(i915, BLC_PWM_CPU_CTL2, BLM_PWM_ENABLE, 0); > > - tmp = intel_de_read(i915, BLC_PWM_PCH_CTL1); > - intel_de_write(i915, BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE); > + tmp = intel_de_rmw(i915, BLC_PWM_PCH_CTL1, BLM_PCH_PWM_ENABLE, 0); > } > > static void i9xx_disable_backlight(const struct drm_connector_state > *old_conn_state, u32 val) > @@ -380,8 +377,7 @@ static void i965_disable_backlight(const struct > drm_connector_state *old_conn_st > > intel_backlight_set_pwm_level(old_conn_state, val); > > - tmp = intel_de_read(i915, BLC_PWM_CTL2); > - intel_de_write(i915, BLC_PWM_CTL2, tmp & ~BLM_PWM_ENABLE); > + tmp = intel_de_rmw(i915, BLC_PWM_CTL2, BLM_PWM_ENABLE, 0); > } > > static void vlv_disable_backlight(const struct drm_connector_state > *old_conn_state, u32 val) > @@ -393,8 +389,7 @@ static void vlv_disable_backlight(const struct > drm_connector_state *old_conn_sta > > intel_backlight_set_pwm_level(old_conn_state, val); > > - tmp = intel_de_read(i915, VLV_BLC_PWM_CTL2(pipe)); > - intel_de_write(i915, VLV_BLC_PWM_CTL2(pipe), tmp & ~BLM_PWM_ENABLE); > + tmp = intel_de_rmw(i915, VLV_BLC_PWM_CTL2(pipe), BLM_PWM_ENABLE, 0); > } > > static void bxt_disable_backlight(const struct drm_connector_state > *old_conn_state, u32 val) > @@ -402,19 +397,14 @@ static void bxt_disable_backlight(const struct > drm_connector_state *old_conn_sta > struct intel_connector *connector = > to_intel_connector(old_conn_state->connector); > struct drm_i915_private *i915 = to_i915(connector->base.dev); > struct intel_panel *panel = >panel; > - u32 tmp; > > intel_backlight_set_pwm_level(old_conn_state, val); > > - tmp = intel_de_read(i915, BXT_BLC_PWM_CTL(panel->backlight.controller)); > - intel_de_write(i915, BXT_BLC_PWM_CTL(panel->backlight.controller), > -tmp & ~BXT_BLC_PWM_ENABLE); > + intel_de_rmw(i915, BXT_BLC_PWM_CTL(panel->backlight.controller), > + BXT_BLC_PWM_ENABLE, 0); > > - if (panel->backlight.controller == 1) { > - val = intel_de_read(i915, UTIL_PIN_CTL); > - val &= ~UTIL_PIN_ENABLE; > - intel_de_write(i915, UTIL_PIN_CTL, val); > - } > + if (panel->backlight.controller == 1) > + intel_de_rmw(i915, UTIL_PIN_CTL, UTIL_PIN_ENABLE, 0); > } > > static void cnp_disable_backlight(const struct drm_connector_state > *old_conn_state, u32 val) > @@ -422,13 +412,11 @@ static void cnp_disable_backlight(const struct > drm_connector_state *old_conn_sta > struct intel_connector *connector = > to_intel_connector(old_conn_state->connector); > struct drm_i915_private *i915 = to_i915(connector->base.dev); > struct intel_panel *panel = >panel; > - u32 tmp; > > intel_backlight_set_pwm_level(old_conn_state, val); > > - tmp = intel_de_read(i915, BXT_BLC_PWM_CTL(panel->backlight.controller)); > - intel_de_write(i915, BXT_BLC_PWM_CTL(panel->backlight.controller), > -tmp & ~BXT_BLC_PWM_ENABLE); > + intel_de_rmw(i915, BXT_BLC_PWM_CTL(panel->backlight.controller), > + BXT_BLC_PWM_ENABLE, 0); > } > > static void ext_pwm_disable_backlight(const struct drm_connector_state > *old_conn_state, u32 level) > @@ -478,7 +466,7 @@ static void lpt_enable_backlight(const struct > intel_crtc_state *crtc_state, > struct intel_connector *connector = >
Re: [Intel-gfx] [PATCH] drm/i915/fbc: Avoid full proxy f_ops for FBC debug attributes
On Sun, Jan 08, 2023 at 01:33:41AM +0530, Deepak R Varma wrote: > On Thu, Jan 05, 2023 at 09:13:35AM +0100, Julia Lawall wrote: > > > Hi Julia, thanks for helping here. > > > > > > So, my question is why this > > > > > > make coccicheck M=drivers/gpu/drm/i915/ MODE=context > > > COCCI=./scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > > > > > > didn't catch this chunck: > > > > > > - debugfs_create_file("i915_fbc_false_color", 0644, parent, > > > - fbc, _fbc_debugfs_false_color_fops); > > > + debugfs_create_file_unsafe("i915_fbc_false_color", 0644, parent, > > > +fbc, > > > _fbc_debugfs_false_color_fops); > > > > > > When I run it it only catches and replaces this: > > > > > > - DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > > > + DEFINE_DEBUGFS_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt); > > > > There is something strange in your question. You have MODE=context but > > you show the output for MODE=patch. The rule dcf matches a call to > > debugfs_create_file, and the context rule matching DEFINE_SIMPLE_ATTRIBUTE > > is only activated if dcf succeeds. So when the context rule gives a > > report, there is always a corresponding call to debugfs_create_file in the > > same file, it is just not highlighted. So the request is that it should > > be highlighted as well? > > Hello Rodrigo, > Not trying to speak for you, but I think Julia's comment appears to be the > correct interpretation of your observation. Would you mind > confirming/clarifying > and suggest next steps for this proposal? doh! newby coccinelle user detected! My bad, sorry! make coccicheck M=drivers/gpu/drm/i915/ MODE=patch COCCI=./scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci do shows everything. So, could you please mention this line in the commit message so we don't forget that? Also could you please provide patches for the other cases? 1 patch for each file is desirable in this case since it touches different areas. > > Thank you, > ./drv > > > > > julia > > > >
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Fix timeslots argument for DP DSC SST case (rev2)
== Series Details == Series: drm/i915: Fix timeslots argument for DP DSC SST case (rev2) URL : https://patchwork.freedesktop.org/series/112349/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2
Re: [Intel-gfx] [RFC PATCH 04/20] drm/sched: Convert drm scheduler to use a work queue rather than kthread
On Mon, Jan 9, 2023 at 7:46 AM Tvrtko Ursulin < tvrtko.ursu...@linux.intel.com> wrote: > > On 06/01/2023 23:52, Matthew Brost wrote: > > On Thu, Jan 05, 2023 at 09:43:41PM +, Matthew Brost wrote: > >> On Tue, Jan 03, 2023 at 01:02:15PM +, Tvrtko Ursulin wrote: > >>> > >>> On 02/01/2023 07:30, Boris Brezillon wrote: > On Fri, 30 Dec 2022 12:55:08 +0100 > Boris Brezillon wrote: > > > On Fri, 30 Dec 2022 11:20:42 +0100 > > Boris Brezillon wrote: > > > >> Hello Matthew, > >> > >> On Thu, 22 Dec 2022 14:21:11 -0800 > >> Matthew Brost wrote: > >>> In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 > >>> mapping between a drm_gpu_scheduler and drm_sched_entity. At first > this > >>> seems a bit odd but let us explain the reasoning below. > >>> > >>> 1. In XE the submission order from multiple drm_sched_entity is not > >>> guaranteed to be the same completion even if targeting the same > hardware > >>> engine. This is because in XE we have a firmware scheduler, the > GuC, > >>> which allowed to reorder, timeslice, and preempt submissions. If a > using > >>> shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR > falls > >>> apart as the TDR expects submission order == completion order. > Using a > >>> dedicated drm_gpu_scheduler per drm_sched_entity solve this > problem. > >> > >> Oh, that's interesting. I've been trying to solve the same sort of > >> issues to support Arm's new Mali GPU which is relying on a > FW-assisted > >> scheduling scheme (you give the FW N streams to execute, and it does > >> the scheduling between those N command streams, the kernel driver > >> does timeslice scheduling to update the command streams passed to > the > >> FW). I must admit I gave up on using drm_sched at some point, mostly > >> because the integration with drm_sched was painful, but also > because I > >> felt trying to bend drm_sched to make it interact with a > >> timeslice-oriented scheduling model wasn't really future proof. > Giving > >> drm_sched_entity exlusive access to a drm_gpu_scheduler probably > might > >> help for a few things (didn't think it through yet), but I feel it's > >> coming short on other aspects we have to deal with on Arm GPUs. > > > > Ok, so I just had a quick look at the Xe driver and how it > > instantiates the drm_sched_entity and drm_gpu_scheduler, and I think > I > > have a better understanding of how you get away with using drm_sched > > while still controlling how scheduling is really done. Here > > drm_gpu_scheduler is just a dummy abstract that let's you use the > > drm_sched job queuing/dep/tracking mechanism. The whole run-queue > > selection is dumb because there's only one entity ever bound to the > > scheduler (the one that's part of the xe_guc_engine object which also > > contains the drm_gpu_scheduler instance). I guess the main issue we'd > > have on Arm is the fact that the stream doesn't necessarily get > > scheduled when ->run_job() is called, it can be placed in the > runnable > > queue and be picked later by the kernel-side scheduler when a FW slot > > gets released. That can probably be sorted out by manually disabling > the > > job timer and re-enabling it when the stream gets picked by the > > scheduler. But my main concern remains, we're basically abusing > > drm_sched here. > > > > For the Arm driver, that means turning the following sequence > > > > 1. wait for job deps > > 2. queue job to ringbuf and push the stream to the runnable > > queue (if it wasn't queued already). Wakeup the timeslice > scheduler > > to re-evaluate (if the stream is not on a FW slot already) > > 3. stream gets picked by the timeslice scheduler and sent to the FW > for > > execution > > > > into > > > > 1. queue job to entity which takes care of waiting for job deps for > > us > > 2. schedule a drm_sched_main iteration > > 3. the only available entity is picked, and the first job from this > > entity is dequeued. ->run_job() is called: the job is queued to > the > > ringbuf and the stream is pushed to the runnable queue (if it > wasn't > > queued already). Wakeup the timeslice scheduler to re-evaluate > (if > > the stream is not on a FW slot already) > > 4. stream gets picked by the timeslice scheduler and sent to the FW > for > > execution > > > > That's one extra step we don't really need. To sum-up, yes, all the > > job/entity tracking might be interesting to share/re-use, but I > wonder > > if we couldn't have that without pulling out the scheduling part of > > drm_sched, or maybe I'm missing something, and there's something in > > drm_gpu_scheduler you really need. > > On second
Re: [Intel-gfx] [RFC PATCH 04/20] drm/sched: Convert drm scheduler to use a work queue rather than kthread
Hi Jason, On Mon, 9 Jan 2023 09:45:09 -0600 Jason Ekstrand wrote: > On Thu, Jan 5, 2023 at 1:40 PM Matthew Brost > wrote: > > > On Mon, Jan 02, 2023 at 08:30:19AM +0100, Boris Brezillon wrote: > > > On Fri, 30 Dec 2022 12:55:08 +0100 > > > Boris Brezillon wrote: > > > > > > > On Fri, 30 Dec 2022 11:20:42 +0100 > > > > Boris Brezillon wrote: > > > > > > > > > Hello Matthew, > > > > > > > > > > On Thu, 22 Dec 2022 14:21:11 -0800 > > > > > Matthew Brost wrote: > > > > > > > > > > > In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 > > > > > > mapping between a drm_gpu_scheduler and drm_sched_entity. At first > > this > > > > > > seems a bit odd but let us explain the reasoning below. > > > > > > > > > > > > 1. In XE the submission order from multiple drm_sched_entity is not > > > > > > guaranteed to be the same completion even if targeting the same > > hardware > > > > > > engine. This is because in XE we have a firmware scheduler, the > > GuC, > > > > > > which allowed to reorder, timeslice, and preempt submissions. If a > > using > > > > > > shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR > > falls > > > > > > apart as the TDR expects submission order == completion order. > > Using a > > > > > > dedicated drm_gpu_scheduler per drm_sched_entity solve this > > problem. > > > > > > > > > > Oh, that's interesting. I've been trying to solve the same sort of > > > > > issues to support Arm's new Mali GPU which is relying on a > > FW-assisted > > > > > scheduling scheme (you give the FW N streams to execute, and it does > > > > > the scheduling between those N command streams, the kernel driver > > > > > does timeslice scheduling to update the command streams passed to the > > > > > FW). I must admit I gave up on using drm_sched at some point, mostly > > > > > because the integration with drm_sched was painful, but also because > > I > > > > > felt trying to bend drm_sched to make it interact with a > > > > > timeslice-oriented scheduling model wasn't really future proof. > > Giving > > > > > drm_sched_entity exlusive access to a drm_gpu_scheduler probably > > might > > > > > help for a few things (didn't think it through yet), but I feel it's > > > > > coming short on other aspects we have to deal with on Arm GPUs. > > > > > > > > Ok, so I just had a quick look at the Xe driver and how it > > > > instantiates the drm_sched_entity and drm_gpu_scheduler, and I think I > > > > have a better understanding of how you get away with using drm_sched > > > > while still controlling how scheduling is really done. Here > > > > drm_gpu_scheduler is just a dummy abstract that let's you use the > > > > drm_sched job queuing/dep/tracking mechanism. The whole run-queue > > > > You nailed it here, we use the DRM scheduler for queuing jobs, > > dependency tracking and releasing jobs to be scheduled when dependencies > > are met, and lastly a tracking mechanism of inflights jobs that need to > > be cleaned up if an error occurs. It doesn't actually do any scheduling > > aside from the most basic level of not overflowing the submission ring > > buffer. In this sense, a 1 to 1 relationship between entity and > > scheduler fits quite well. > > > > Yeah, I think there's an annoying difference between what AMD/NVIDIA/Intel > want here and what you need for Arm thanks to the number of FW queues > available. I don't remember the exact number of GuC queues but it's at > least 1k. This puts it in an entirely different class from what you have on > Mali. Roughly, there's about three categories here: > > 1. Hardware where the kernel is placing jobs on actual HW rings. This is > old Mali, Intel Haswell and earlier, and probably a bunch of others. > (Intel BDW+ with execlists is a weird case that doesn't fit in this > categorization.) > > 2. Hardware (or firmware) with a very limited number of queues where > you're going to have to juggle in the kernel in order to run desktop Linux. > > 3. Firmware scheduling with a high queue count. In this case, you don't > want the kernel scheduling anything. Just throw it at the firmware and let > it go br. If we ever run out of queues (unlikely), the kernel can > temporarily pause some low-priority contexts and do some juggling or, > frankly, just fail userspace queue creation and tell the user to close some > windows. > > The existence of this 2nd class is a bit annoying but it's where we are. I > think it's worth recognizing that Xe and panfrost are in different places > here and will require different designs. For Xe, we really are just using > drm/scheduler as a front-end and the firmware does all the real scheduling. > > How do we deal with class 2? That's an interesting question. We may > eventually want to break that off into a separate discussion and not litter > the Xe thread but let's keep going here for a bit. I think there are some > pretty reasonable solutions but they're
Re: [Intel-gfx] [RFC PATCH 04/20] drm/sched: Convert drm scheduler to use a work queue rather than kthread
On Thu, Jan 5, 2023 at 1:40 PM Matthew Brost wrote: > On Mon, Jan 02, 2023 at 08:30:19AM +0100, Boris Brezillon wrote: > > On Fri, 30 Dec 2022 12:55:08 +0100 > > Boris Brezillon wrote: > > > > > On Fri, 30 Dec 2022 11:20:42 +0100 > > > Boris Brezillon wrote: > > > > > > > Hello Matthew, > > > > > > > > On Thu, 22 Dec 2022 14:21:11 -0800 > > > > Matthew Brost wrote: > > > > > > > > > In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 > > > > > mapping between a drm_gpu_scheduler and drm_sched_entity. At first > this > > > > > seems a bit odd but let us explain the reasoning below. > > > > > > > > > > 1. In XE the submission order from multiple drm_sched_entity is not > > > > > guaranteed to be the same completion even if targeting the same > hardware > > > > > engine. This is because in XE we have a firmware scheduler, the > GuC, > > > > > which allowed to reorder, timeslice, and preempt submissions. If a > using > > > > > shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR > falls > > > > > apart as the TDR expects submission order == completion order. > Using a > > > > > dedicated drm_gpu_scheduler per drm_sched_entity solve this > problem. > > > > > > > > Oh, that's interesting. I've been trying to solve the same sort of > > > > issues to support Arm's new Mali GPU which is relying on a > FW-assisted > > > > scheduling scheme (you give the FW N streams to execute, and it does > > > > the scheduling between those N command streams, the kernel driver > > > > does timeslice scheduling to update the command streams passed to the > > > > FW). I must admit I gave up on using drm_sched at some point, mostly > > > > because the integration with drm_sched was painful, but also because > I > > > > felt trying to bend drm_sched to make it interact with a > > > > timeslice-oriented scheduling model wasn't really future proof. > Giving > > > > drm_sched_entity exlusive access to a drm_gpu_scheduler probably > might > > > > help for a few things (didn't think it through yet), but I feel it's > > > > coming short on other aspects we have to deal with on Arm GPUs. > > > > > > Ok, so I just had a quick look at the Xe driver and how it > > > instantiates the drm_sched_entity and drm_gpu_scheduler, and I think I > > > have a better understanding of how you get away with using drm_sched > > > while still controlling how scheduling is really done. Here > > > drm_gpu_scheduler is just a dummy abstract that let's you use the > > > drm_sched job queuing/dep/tracking mechanism. The whole run-queue > > You nailed it here, we use the DRM scheduler for queuing jobs, > dependency tracking and releasing jobs to be scheduled when dependencies > are met, and lastly a tracking mechanism of inflights jobs that need to > be cleaned up if an error occurs. It doesn't actually do any scheduling > aside from the most basic level of not overflowing the submission ring > buffer. In this sense, a 1 to 1 relationship between entity and > scheduler fits quite well. > Yeah, I think there's an annoying difference between what AMD/NVIDIA/Intel want here and what you need for Arm thanks to the number of FW queues available. I don't remember the exact number of GuC queues but it's at least 1k. This puts it in an entirely different class from what you have on Mali. Roughly, there's about three categories here: 1. Hardware where the kernel is placing jobs on actual HW rings. This is old Mali, Intel Haswell and earlier, and probably a bunch of others. (Intel BDW+ with execlists is a weird case that doesn't fit in this categorization.) 2. Hardware (or firmware) with a very limited number of queues where you're going to have to juggle in the kernel in order to run desktop Linux. 3. Firmware scheduling with a high queue count. In this case, you don't want the kernel scheduling anything. Just throw it at the firmware and let it go br. If we ever run out of queues (unlikely), the kernel can temporarily pause some low-priority contexts and do some juggling or, frankly, just fail userspace queue creation and tell the user to close some windows. The existence of this 2nd class is a bit annoying but it's where we are. I think it's worth recognizing that Xe and panfrost are in different places here and will require different designs. For Xe, we really are just using drm/scheduler as a front-end and the firmware does all the real scheduling. How do we deal with class 2? That's an interesting question. We may eventually want to break that off into a separate discussion and not litter the Xe thread but let's keep going here for a bit. I think there are some pretty reasonable solutions but they're going to look a bit different. The way I did this for Xe with execlists was to keep the 1:1:1 mapping between drm_gpu_scheduler, drm_sched_entity, and userspace xe_engine. Instead of feeding a GuC ring, though, it would feed a fixed-size execlist ring and then there was a tiny kernel which operated entirely in IRQ
Re: [Intel-gfx] [PATCH] drm/i915: Implement workaround for DP2 UHBR bandwidth check
On Tue, Jan 03, 2023 at 11:32:28AM -0500, Rodrigo Vivi wrote: > > on the subject: This is not a hw workaround. Please remove the workaround from > the subject and the wrong comment. > > "The HSD given is a 'feature' rather than 'bugeco' so no workaround details > are > present here." > > > On Mon, Jan 02, 2023 at 01:39:37PM +0200, Stanislav Lisovskiy wrote: > > According to spec, we should check if output_bpp * pixel_rate is less > > than DDI clock * 72, if UHBR is used. > > > > Signed-off-by: Stanislav Lisovskiy > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 11 +++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index bf80f296a8fdb..13baf3cb5f934 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -1582,6 +1582,17 @@ int intel_dp_dsc_compute_config(struct intel_dp > > *intel_dp, > > drm_dbg_kms(_priv->drm, "DSC: compressed bpp %d slice count > > %d\n", > > pipe_config->dsc.compressed_bpp, > > pipe_config->dsc.slice_count); > > + > > + /* wa1406899791 */ > > even if it was a bugeco, the notation doesn't follow the standard. > > But anyway, as I pointed out, this is not a workaround so > you probably just want a > > HSDES: 1406899791 > BSPEC: 49259 > > in your commit msg. Ok, will add this thanks. > > Also maybe a "Fixes:" tag pointing to the commit that added the sequence > but didn't added this part of the sequence? > > > + if (intel_dp_is_uhbr(pipe_config)) { > > + int output_bpp = pipe_config->dsc.compressed_bpp; > > + > > + if (output_bpp * adjusted_mode->crtc_clock >= > > + pipe_config->port_clock * 72) { > > + drm_dbg_kms(_priv->drm, "DP2 UHBR check > > failed\n"); > > some probably dummy question: > do we need to add a check for the DP 2.0 above as well? > or it is unecessary/redundant? I think this check is more related to hardware limitation, rather than DP 2.0 standard. I mean if it was even not DP 2.0, but still UHBR I really doubt that this limitation wouldn't be essential still. Stan > > > + return -EINVAL; > > + } > > + } > > } > > /* > > * VDSC engine operates at 1 Pixel per clock, so if peak pixel rate > > -- > > 2.37.3 > >
Re: [Intel-gfx] [PATCH] drm: Alloc high address for drm buddy topdown flag
On Mon, Jan 9, 2023 at 5:13 AM Christian König wrote: > > Am 07.01.23 um 16:15 schrieb Arunpravin Paneer Selvam: > > As we are observing low numbers in viewperf graphics benchmark, we > > are strictly not allowing the top down flag enabled allocations > > to steal the memory space from cpu visible region. > > > > The approach is, we are sorting each order list entries in > > ascending order and compare the last entry of each order > > list in the freelist and return the max block. > > > > This patch improves the viewperf 3D benchmark scores. > > > > Signed-off-by: Arunpravin Paneer Selvam > > Acked-by: Christian König , but somebody with more > insight of the drm buddy allocator should take a closer look at this. I'm not a drm_buddy expert either, but this patch fixes a lot of issues on both dGPUs and APUs: Acked-by: Alex Deucher > > > > --- > > drivers/gpu/drm/drm_buddy.c | 81 - > > 1 file changed, 54 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c > > index 11bb59399471..50916b2f2fc5 100644 > > --- a/drivers/gpu/drm/drm_buddy.c > > +++ b/drivers/gpu/drm/drm_buddy.c > > @@ -38,6 +38,25 @@ static void drm_block_free(struct drm_buddy *mm, > > kmem_cache_free(slab_blocks, block); > > } > > > > +static void list_insert_sorted(struct drm_buddy *mm, > > +struct drm_buddy_block *block) > > +{ > > + struct drm_buddy_block *node; > > + struct list_head *head; > > + > > + head = >free_list[drm_buddy_block_order(block)]; > > + if (list_empty(head)) { > > + list_add(>link, head); > > + return; > > + } > > + > > + list_for_each_entry(node, head, link) > > + if (drm_buddy_block_offset(block) < > > drm_buddy_block_offset(node)) > > + break; > > + > > + __list_add(>link, node->link.prev, >link); > > +} > > + > > static void mark_allocated(struct drm_buddy_block *block) > > { > > block->header &= ~DRM_BUDDY_HEADER_STATE; > > @@ -52,8 +71,7 @@ static void mark_free(struct drm_buddy *mm, > > block->header &= ~DRM_BUDDY_HEADER_STATE; > > block->header |= DRM_BUDDY_FREE; > > > > - list_add(>link, > > - >free_list[drm_buddy_block_order(block)]); > > + list_insert_sorted(mm, block); > > } > > > > static void mark_split(struct drm_buddy_block *block) > > @@ -387,20 +405,26 @@ alloc_range_bias(struct drm_buddy *mm, > > } > > > > static struct drm_buddy_block * > > -get_maxblock(struct list_head *head) > > +get_maxblock(struct drm_buddy *mm, unsigned int order) > > { > > struct drm_buddy_block *max_block = NULL, *node; > > + unsigned int i; > > > > - max_block = list_first_entry_or_null(head, > > - struct drm_buddy_block, > > - link); > > - if (!max_block) > > - return NULL; > > + for (i = order; i <= mm->max_order; ++i) { > > + if (!list_empty(>free_list[i])) { > > + node = list_last_entry(>free_list[i], > > +struct drm_buddy_block, > > +link); > > + if (!max_block) { > > + max_block = node; > > + continue; > > + } > > > > - list_for_each_entry(node, head, link) { > > - if (drm_buddy_block_offset(node) > > > - drm_buddy_block_offset(max_block)) > > - max_block = node; > > + if (drm_buddy_block_offset(node) > > > + drm_buddy_block_offset(max_block)) { > > + max_block = node; > > + } > > + } > > } > > > > return max_block; > > @@ -412,20 +436,23 @@ alloc_from_freelist(struct drm_buddy *mm, > > unsigned long flags) > > { > > struct drm_buddy_block *block = NULL; > > - unsigned int i; > > + unsigned int tmp; > > int err; > > > > - for (i = order; i <= mm->max_order; ++i) { > > - if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) { > > - block = get_maxblock(>free_list[i]); > > - if (block) > > - break; > > - } else { > > - block = list_first_entry_or_null(>free_list[i], > > - struct > > drm_buddy_block, > > - link); > > - if (block) > > - break; > > + if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) { > > + block = get_maxblock(mm, order); > > + if (block) > > + /* Store the obtained block order */ > > +
Re: [Intel-gfx] LOOKS GOOD: Possible regression in drm/i915 driver: memleak
On 25/12/2022 22:48, Mirsad Goran Todorovac wrote: On 22. 12. 2022. 09:04, Tvrtko Ursulin wrote: In the meantime definitely thanks a lot for testing this quickly and reporting back! Don't think much of it - anyone with CONFIG_KMEMLEAK enabled could have caught this bug. I was surprised that you found the fix in less than an hour without me having to bisect :) Fix sadly has a problem handling shared buffers so different version will hopefully appear soon. Regards, Tvrtko
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/display: Enable FBC for fbcon (rev2)
== Series Details == Series: drm/i915/display: Enable FBC for fbcon (rev2) URL : https://patchwork.freedesktop.org/series/112548/ State : success == Summary == CI Bug Log - changes from CI_DRM_12556_full -> Patchwork_112548v2_full Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/index.html Participating hosts (14 -> 10) -- Missing(4): shard-rkl0 pig-kbl-iris pig-glk-j5005 pig-skl-6260u New tests - New tests have been introduced between CI_DRM_12556_full and Patchwork_112548v2_full: ### New IGT tests (1) ### * igt@kms_flip_tiling: - Statuses : - Exec time: [None] s Known issues Here are the changes found in Patchwork_112548v2_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_fair@basic-deadline: - shard-glk: [PASS][1] -> [FAIL][2] ([i915#2846]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-glk2/igt@gem_exec_f...@basic-deadline.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-glk6/igt@gem_exec_f...@basic-deadline.html Possible fixes * igt@drm_read@invalid-buffer: - {shard-tglu}: [SKIP][3] ([i915#1845]) -> [PASS][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-tglu-6/igt@drm_r...@invalid-buffer.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-tglu-3/igt@drm_r...@invalid-buffer.html * igt@fbdev@info: - {shard-rkl}:[SKIP][5] ([i915#2582]) -> [PASS][6] +1 similar issue [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-4/igt@fb...@info.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-rkl-6/igt@fb...@info.html * igt@gem_exec_fair@basic-flow@rcs0: - {shard-rkl}:[FAIL][7] ([i915#2842]) -> [PASS][8] +2 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-2/igt@gem_exec_fair@basic-f...@rcs0.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-rkl-1/igt@gem_exec_fair@basic-f...@rcs0.html * igt@gem_exec_flush@basic-batch-kernel-default-cmd: - {shard-rkl}:[SKIP][9] ([fdo#109313]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-6/igt@gem_exec_fl...@basic-batch-kernel-default-cmd.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-rkl-5/igt@gem_exec_fl...@basic-batch-kernel-default-cmd.html * igt@gem_exec_reloc@basic-gtt-wc-noreloc: - {shard-rkl}:[SKIP][11] ([i915#3281]) -> [PASS][12] +6 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-6/igt@gem_exec_re...@basic-gtt-wc-noreloc.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-rkl-5/igt@gem_exec_re...@basic-gtt-wc-noreloc.html * igt@gem_mmap_gtt@coherency: - {shard-rkl}:[SKIP][13] ([fdo#111656]) -> [PASS][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-6/igt@gem_mmap_...@coherency.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-rkl-5/igt@gem_mmap_...@coherency.html * igt@gem_pread@snoop: - {shard-rkl}:[SKIP][15] ([i915#3282]) -> [PASS][16] +3 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-6/igt@gem_pr...@snoop.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-rkl-5/igt@gem_pr...@snoop.html * igt@gen9_exec_parse@shadow-peek: - {shard-rkl}:[SKIP][17] ([i915#2527]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-6/igt@gen9_exec_pa...@shadow-peek.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-rkl-5/igt@gen9_exec_pa...@shadow-peek.html * igt@i915_hangman@gt-engine-error@bcs0: - {shard-rkl}:[SKIP][19] ([i915#6258]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-5/igt@i915_hangman@gt-engine-er...@bcs0.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-rkl-3/igt@i915_hangman@gt-engine-er...@bcs0.html * igt@i915_pm_dc@dc6-psr: - {shard-rkl}:[SKIP][21] ([i915#658]) -> [PASS][22] +1 similar issue [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-3/igt@i915_pm...@dc6-psr.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/shard-rkl-6/igt@i915_pm...@dc6-psr.html * igt@i915_pm_dc@dc9-dpms: - {shard-tglu}: [SKIP][23] ([i915#4281]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-tglu-8/igt@i915_pm...@dc9-dpms.html [24]:
[Intel-gfx] [PATCH] drm/i915: Fix timeslots argument for DP DSC SST case
We now accept timeslots param exactly how the variable sounds: amount of timeslots, but not ratio timeslots/64. So for SST case(when we have all timeslots for use), it should be 64, but not 1. This caused some issues in the tests. v2: Fixed comments References: https://gitlab.freedesktop.org/drm/intel/-/issues/6860 Fixes: 52f14682ac4d ("drm/i915: Bpp/timeslot calculation fixes for DP MST DSC") Reviewed-by: Manasi Navare Signed-off-by: Stanislav Lisovskiy --- drivers/gpu/drm/i915/display/intel_dp.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index bf80f296a8fd..30c55f980014 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -713,9 +713,9 @@ u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915, /* * Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)* -* (LinkSymbolClock)* 8 * (TimeSlotsPerMTP) -* for SST -> TimeSlotsPerMTP is 1, -* for MST -> TimeSlotsPerMTP has to be calculated +* (LinkSymbolClock)* 8 * (TimeSlots / 64) +* for SST -> TimeSlots is 64(i.e all TimeSlots that are available) +* for MST -> TimeSlots has to be calculated, based on mode requirements */ bits_per_pixel = DIV_ROUND_UP((link_clock * lane_count) * timeslots, intel_dp_mode_to_fec_clock(mode_clock) * 8); @@ -1685,7 +1685,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder, str_yes_no(ret), str_yes_no(joiner_needs_dsc), str_yes_no(intel_dp->force_dsc_en)); ret = intel_dp_dsc_compute_config(intel_dp, pipe_config, - conn_state, , 1, true); + conn_state, , 64, true); if (ret < 0) return ret; } -- 2.37.3
Re: [Intel-gfx] [PATCH] drm/i915: Update docs in intel_wakeref.h
Hi Nirmoy, On Thu, Jan 05, 2023 at 09:38:43PM +0100, Nirmoy Das wrote: > Fix docs for __intel_wakeref_put() and intel_wakeref_get() to > reflect current behaviour. > > Signed-off-by: Nirmoy Das Thanks for adding also the change suggested by Ashutosh! Reviewed-by: Andi Shyti Andi > --- > drivers/gpu/drm/i915/intel_wakeref.h | 21 ++--- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_wakeref.h > b/drivers/gpu/drm/i915/intel_wakeref.h > index 4f4c2e15e736..71b8a63f6f10 100644 > --- a/drivers/gpu/drm/i915/intel_wakeref.h > +++ b/drivers/gpu/drm/i915/intel_wakeref.h > @@ -68,11 +68,12 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf, > unsigned long flags); > * @wf: the wakeref > * > * Acquire a hold on the wakeref. The first user to do so, will acquire > - * the runtime pm wakeref and then call the @fn underneath the wakeref > - * mutex. > + * the runtime pm wakeref and then call the intel_wakeref_ops->get() > + * underneath the wakeref mutex. > * > - * Note that @fn is allowed to fail, in which case the runtime-pm wakeref > - * will be released and the acquisition unwound, and an error reported. > + * Note that intel_wakeref_ops->get() is allowed to fail, in which case > + * the runtime-pm wakeref will be released and the acquisition unwound, > + * and an error reported. > * > * Returns: 0 if the wakeref was acquired successfully, or a negative error > * code otherwise. > @@ -130,19 +131,17 @@ intel_wakeref_might_get(struct intel_wakeref *wf) > } > > /** > - * intel_wakeref_put_flags: Release the wakeref > + * __intel_wakeref_put: Release the wakeref > * @wf: the wakeref > * @flags: control flags > * > * Release our hold on the wakeref. When there are no more users, > - * the runtime pm wakeref will be released after the @fn callback is called > - * underneath the wakeref mutex. > + * the runtime pm wakeref will be released after the intel_wakeref_ops->put() > + * callback is called underneath the wakeref mutex. > * > - * Note that @fn is allowed to fail, in which case the runtime-pm wakeref > - * is retained and an error reported. > + * Note that intel_wakeref_ops->put() is allowed to fail, in which case the > + * runtime-pm wakeref is retained. > * > - * Returns: 0 if the wakeref was released successfully, or a negative error > - * code otherwise. > */ > static inline void > __intel_wakeref_put(struct intel_wakeref *wf, unsigned long flags) > -- > 2.38.0
Re: [Intel-gfx] [PATCH 1/2] drm/i915: use proper helper in igt_vma_move_to_active_unlocked
Hi Andrzej, On Tue, Dec 13, 2022 at 01:19:50PM +0100, Andrzej Hajda wrote: > There is no need to use _i915_vma_move_to_active. > No functional changes. > > Signed-off-by: Andrzej Hajda both patches pushed in drm-intel-gt-next. Thanks, Andi > --- > drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.h > b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.h > index 1379fbc1443126..71a3ca8a886506 100644 > --- a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.h > +++ b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.h > @@ -38,7 +38,7 @@ igt_vma_move_to_active_unlocked(struct i915_vma *vma, > struct i915_request *rq, > int err; > > i915_vma_lock(vma); > - err = _i915_vma_move_to_active(vma, rq, >fence, flags); > + err = i915_vma_move_to_active(vma, rq, flags); > i915_vma_unlock(vma); > return err; > } > -- > 2.34.1
Re: [Intel-gfx] [RFC PATCH 04/20] drm/sched: Convert drm scheduler to use a work queue rather than kthread
On 06/01/2023 23:52, Matthew Brost wrote: On Thu, Jan 05, 2023 at 09:43:41PM +, Matthew Brost wrote: On Tue, Jan 03, 2023 at 01:02:15PM +, Tvrtko Ursulin wrote: On 02/01/2023 07:30, Boris Brezillon wrote: On Fri, 30 Dec 2022 12:55:08 +0100 Boris Brezillon wrote: On Fri, 30 Dec 2022 11:20:42 +0100 Boris Brezillon wrote: Hello Matthew, On Thu, 22 Dec 2022 14:21:11 -0800 Matthew Brost wrote: In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 mapping between a drm_gpu_scheduler and drm_sched_entity. At first this seems a bit odd but let us explain the reasoning below. 1. In XE the submission order from multiple drm_sched_entity is not guaranteed to be the same completion even if targeting the same hardware engine. This is because in XE we have a firmware scheduler, the GuC, which allowed to reorder, timeslice, and preempt submissions. If a using shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls apart as the TDR expects submission order == completion order. Using a dedicated drm_gpu_scheduler per drm_sched_entity solve this problem. Oh, that's interesting. I've been trying to solve the same sort of issues to support Arm's new Mali GPU which is relying on a FW-assisted scheduling scheme (you give the FW N streams to execute, and it does the scheduling between those N command streams, the kernel driver does timeslice scheduling to update the command streams passed to the FW). I must admit I gave up on using drm_sched at some point, mostly because the integration with drm_sched was painful, but also because I felt trying to bend drm_sched to make it interact with a timeslice-oriented scheduling model wasn't really future proof. Giving drm_sched_entity exlusive access to a drm_gpu_scheduler probably might help for a few things (didn't think it through yet), but I feel it's coming short on other aspects we have to deal with on Arm GPUs. Ok, so I just had a quick look at the Xe driver and how it instantiates the drm_sched_entity and drm_gpu_scheduler, and I think I have a better understanding of how you get away with using drm_sched while still controlling how scheduling is really done. Here drm_gpu_scheduler is just a dummy abstract that let's you use the drm_sched job queuing/dep/tracking mechanism. The whole run-queue selection is dumb because there's only one entity ever bound to the scheduler (the one that's part of the xe_guc_engine object which also contains the drm_gpu_scheduler instance). I guess the main issue we'd have on Arm is the fact that the stream doesn't necessarily get scheduled when ->run_job() is called, it can be placed in the runnable queue and be picked later by the kernel-side scheduler when a FW slot gets released. That can probably be sorted out by manually disabling the job timer and re-enabling it when the stream gets picked by the scheduler. But my main concern remains, we're basically abusing drm_sched here. For the Arm driver, that means turning the following sequence 1. wait for job deps 2. queue job to ringbuf and push the stream to the runnable queue (if it wasn't queued already). Wakeup the timeslice scheduler to re-evaluate (if the stream is not on a FW slot already) 3. stream gets picked by the timeslice scheduler and sent to the FW for execution into 1. queue job to entity which takes care of waiting for job deps for us 2. schedule a drm_sched_main iteration 3. the only available entity is picked, and the first job from this entity is dequeued. ->run_job() is called: the job is queued to the ringbuf and the stream is pushed to the runnable queue (if it wasn't queued already). Wakeup the timeslice scheduler to re-evaluate (if the stream is not on a FW slot already) 4. stream gets picked by the timeslice scheduler and sent to the FW for execution That's one extra step we don't really need. To sum-up, yes, all the job/entity tracking might be interesting to share/re-use, but I wonder if we couldn't have that without pulling out the scheduling part of drm_sched, or maybe I'm missing something, and there's something in drm_gpu_scheduler you really need. On second thought, that's probably an acceptable overhead (not even sure the extra step I was mentioning exists in practice, because dep fence signaled state is checked as part of the drm_sched_main iteration, so that's basically replacing the worker I schedule to check job deps), and I like the idea of being able to re-use drm_sched dep-tracking without resorting to invasive changes to the existing logic, so I'll probably give it a try. I agree with the concerns and think that how Xe proposes to integrate with drm_sched is a problem, or at least significantly inelegant. Inelegant is a matter of opinion, I actually rather like this solution. BTW this isn't my design rather this was Jason's idea. AFAICT it proposes to have 1:1 between *userspace* created contexts (per context _and_ engine) and drm_sched.
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v2,1/9] drm/i915/display/core: use intel_de_rmw if possible (rev2)
== Series Details == Series: series starting with [v2,1/9] drm/i915/display/core: use intel_de_rmw if possible (rev2) URL : https://patchwork.freedesktop.org/series/112438/ State : success == Summary == CI Bug Log - changes from CI_DRM_12556_full -> Patchwork_112438v2_full Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/index.html Participating hosts (14 -> 10) -- Missing(4): shard-rkl0 pig-kbl-iris pig-glk-j5005 pig-skl-6260u Known issues Here are the changes found in Patchwork_112438v2_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_fair@basic-deadline: - shard-glk: [PASS][1] -> [FAIL][2] ([i915#2846]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-glk2/igt@gem_exec_f...@basic-deadline.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-glk5/igt@gem_exec_f...@basic-deadline.html * igt@gem_exec_fair@basic-pace-share@rcs0: - shard-glk: [PASS][3] -> [FAIL][4] ([i915#2842]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-glk4/igt@gem_exec_fair@basic-pace-sh...@rcs0.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-glk7/igt@gem_exec_fair@basic-pace-sh...@rcs0.html Possible fixes * igt@fbdev@read: - {shard-rkl}:[SKIP][5] ([i915#2582]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-1/igt@fb...@read.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-rkl-6/igt@fb...@read.html * igt@gem_exec_fair@basic-flow@rcs0: - {shard-rkl}:[FAIL][7] ([i915#2842]) -> [PASS][8] +2 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-2/igt@gem_exec_fair@basic-f...@rcs0.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-rkl-5/igt@gem_exec_fair@basic-f...@rcs0.html * igt@gem_exec_reloc@basic-wc-read-noreloc: - {shard-rkl}:[SKIP][9] ([i915#3281]) -> [PASS][10] +13 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-2/igt@gem_exec_re...@basic-wc-read-noreloc.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-rkl-5/igt@gem_exec_re...@basic-wc-read-noreloc.html * igt@gem_set_tiling_vs_pwrite: - {shard-rkl}:[SKIP][11] ([i915#3282]) -> [PASS][12] +6 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-1/igt@gem_set_tiling_vs_pwrite.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-rkl-5/igt@gem_set_tiling_vs_pwrite.html * igt@gen9_exec_parse@bb-start-far: - {shard-rkl}:[SKIP][13] ([i915#2527]) -> [PASS][14] +2 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-4/igt@gen9_exec_pa...@bb-start-far.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-rkl-5/igt@gen9_exec_pa...@bb-start-far.html * igt@i915_hangman@gt-engine-error@bcs0: - {shard-rkl}:[SKIP][15] ([i915#6258]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-5/igt@i915_hangman@gt-engine-er...@bcs0.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-rkl-4/igt@i915_hangman@gt-engine-er...@bcs0.html * igt@i915_pm_rpm@modeset-lpsp: - {shard-dg1}:[SKIP][17] ([i915#1397]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-dg1-15/igt@i915_pm_...@modeset-lpsp.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-dg1-14/igt@i915_pm_...@modeset-lpsp.html * igt@kms_atomic@atomic_plane_damage: - {shard-rkl}:[SKIP][19] ([i915#4098]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-1/igt@kms_atomic@atomic_plane_damage.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-rkl-6/igt@kms_atomic@atomic_plane_damage.html * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-async-flip: - {shard-rkl}:[SKIP][21] ([i915#1845] / [i915#4098]) -> [PASS][22] +13 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-3/igt@kms_big...@x-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/shard-rkl-6/igt@kms_big...@x-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html * igt@kms_frontbuffer_tracking@psr-rgb565-draw-render: - {shard-rkl}:[SKIP][23] ([i915#1849] / [i915#4098]) -> [PASS][24] +9 similar issues [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/shard-rkl-1/igt@kms_frontbuffer_track...@psr-rgb565-draw-render.html [24]:
Re: [Intel-gfx] [PATCH] drm/i915: Fix timeslots argument for DP DSC SST case
On Tue, Jan 03, 2023 at 11:14:16AM -0500, Rodrigo Vivi wrote: > On Mon, Jan 02, 2023 at 03:23:06PM +0200, Stanislav Lisovskiy wrote: > > We now accept timeslots param exactly how the variable > > sounds: amount of timeslots, but not ratio timeslots/64. > > So for SST case(when we have all timeslots for use), it > > should be 64, but not 1. > > I noticed that at intel_dp_dsc_get_output_bpp() we have this comment: > > * for SST -> TimeSlotsPerMTP is 1, > > and there's a bunch of math used with this timeslots, but none of them > is a direct division by 64. Yep "TimeSlotsPerMTP is 1" already means that this is a ratio. We have 64 timeslots alltogether and obviously for SST we have all of them. To be honest that comment above isn't way too informative, I've had to dig into the spec and calculate everything by myself before I could figure out what is going on here. For the Link Total Bw can be calculated like this: Link Total Bw = link_symbol_clock * lane_count * 8 Amount of Bw we need for the current mode is: Required bpp = mode_clock * bpp For generic MST case, amount BW we have is split to 64 timeslots, i.e each 1 timeslot has Bw: 1 Timeslot Bw = Link Total Bw / 64 = (link_symbol_clock * lane_count * 8) / 64 Given amount if timeslots the bw which we will get is: Link Total Bw * (timeslots / 64) = (link_symbol_clock * lane_count * 8 * timeslots) / 64 However required Bw is as said above: Required Bw = mode_clock * bpp Thus in order to figure out the max bpp we can afford, we need to solve equation: Link Total Bw * (timeslots / 64) = mode_clock * bpp i.e bpp = (Link Total Bw * timeslots) / (mode_clock * 64) => bpp = (link_symbol_clock * lane_count * 8 * timeslots) / (mode_clock * 64) which can be simplified to: bpp = (link_symbol_clock * lane_count * timeslots) / (mode_clock * 8) In order for this to work timeslots should be actual amount of timeslots but not the ratio. > > So I wonder if a refactor to reflect the "perMTP" is not needed there. > Or the reverse math instead of passing the 64 directly. > > > This caused some issues in the tests. > > could you also expand on what tests? > any "References:" link to cibuglog or so? Asked from CI, but as I understand quite a lot of tests are affected, like _many_ :)) Will add the link. Stan > > Oh, any "Fixes:" tag as well? > > Thanks for the patch, > Rodrigo. > > > > > Signed-off-by: Stanislav Lisovskiy > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 13baf3cb5f934..362fb394d613c 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -1696,7 +1696,7 @@ intel_dp_compute_link_config(struct intel_encoder > > *encoder, > > str_yes_no(ret), str_yes_no(joiner_needs_dsc), > > str_yes_no(intel_dp->force_dsc_en)); > > ret = intel_dp_dsc_compute_config(intel_dp, pipe_config, > > - conn_state, , 1, true); > > + conn_state, , 64, > > true); > > if (ret < 0) > > return ret; > > } > > -- > > 2.37.3 > >
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: Enable FBC for fbcon (rev2)
== Series Details == Series: drm/i915/display: Enable FBC for fbcon (rev2) URL : https://patchwork.freedesktop.org/series/112548/ State : success == Summary == CI Bug Log - changes from CI_DRM_12556 -> Patchwork_112548v2 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/index.html Participating hosts (39 -> 38) -- Additional (1): bat-rpls-2 Missing(2): fi-snb-2520m bat-adlp-4 Known issues Here are the changes found in Patchwork_112548v2 that come from known issues: ### IGT changes ### Issues hit * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size: - fi-bsw-n3050: [PASS][1] -> [FAIL][2] ([i915#6298]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html Possible fixes * igt@i915_selftest@live@gt_lrc: - {bat-rpls-1}: [INCOMPLETE][3] ([i915#4983]) -> [PASS][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/bat-rpls-1/igt@i915_selftest@live@gt_lrc.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/bat-rpls-1/igt@i915_selftest@live@gt_lrc.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285 [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295 [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827 [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072 [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845 [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849 [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582 [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282 [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555 [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637 [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708 [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983 [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298 [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367 [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621 [i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359 [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456 [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561 Build changes - * Linux: CI_DRM_12556 -> Patchwork_112548v2 CI-20190529: 20190529 CI_DRM_12556: ac04152253dccfb02dcedfa0c57443122cf79314 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7110: db10a19b94d1d7ae5ba62eb48d52c47ccb27766f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_112548v2: ac04152253dccfb02dcedfa0c57443122cf79314 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 06c3d3b0a848 drm/i915/display: Enable FBC for fbcon == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112548v2/index.html
[Intel-gfx] [PATCH v3] drm/i915: Do not cover all future platforms in TLB invalidation
From: Tvrtko Ursulin Revert to the original explicit approach and document the reasoning behind it. v2: * DG2 needs to be covered too. (Matt) v3: * Full version check for Gen12 to avoid catching all future platforms. (Matt) Signed-off-by: Tvrtko Ursulin Cc: Matt Roper Cc: Balasubramani Vivekanandan Cc: Andrzej Hajda Reviewed-by: Andrzej Hajda # v1 --- drivers/gpu/drm/i915/gt/intel_gt.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 75a7cb33..5521fa057aab 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -1070,10 +1070,23 @@ static void mmio_invalidate_full(struct intel_gt *gt) unsigned int num = 0; unsigned long flags; - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) { + /* +* New platforms should not be added with catch-all-newer (>=) +* condition so that any later platform added triggers the below warning +* and in turn mandates a human cross-check of whether the invalidation +* flows have compatible semantics. +* +* For instance with the 11.00 -> 12.00 transition three out of five +* respective engine registers were moved to masked type. Then after the +* 12.00 -> 12.50 transition multi cast handling is required too. +*/ + + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50) && + GRAPHICS_VER_FULL(i915) <= IP_VER(12, 55)) { regs = NULL; num = ARRAY_SIZE(xehp_regs); - } else if (GRAPHICS_VER(i915) == 12) { + } else if (GRAPHICS_VER_FULL(i915) == IP_VER(12, 0) || + GRAPHICS_VER_FULL(i915) == IP_VER(12, 10)) { regs = gen12_regs; num = ARRAY_SIZE(gen12_regs); } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) { -- 2.34.1
[Intel-gfx] [PATCH fixed] drm/i915/display: Enable FBC for fbcon
Remove frontbuffer invalidation code from FBC, and just use the dirtyfb helper. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/display/intel_fbdev.c | 73 +- 1 file changed, 15 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index 03ed4607a46d..d42fa38aec9d 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -40,6 +40,7 @@ #include #include #include +#include #include "gem/i915_gem_lmem.h" @@ -67,70 +68,17 @@ struct intel_fbdev { struct mutex hpd_lock; }; -static struct intel_frontbuffer *to_frontbuffer(struct intel_fbdev *ifbdev) -{ - return ifbdev->fb->frontbuffer; -} - -static void intel_fbdev_invalidate(struct intel_fbdev *ifbdev) -{ - intel_frontbuffer_invalidate(to_frontbuffer(ifbdev), ORIGIN_CPU); -} - -static int intel_fbdev_set_par(struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct intel_fbdev *ifbdev = - container_of(fb_helper, struct intel_fbdev, helper); - int ret; - - ret = drm_fb_helper_set_par(info); - if (ret == 0) - intel_fbdev_invalidate(ifbdev); - - return ret; -} - -static int intel_fbdev_blank(int blank, struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct intel_fbdev *ifbdev = - container_of(fb_helper, struct intel_fbdev, helper); - int ret; - - ret = drm_fb_helper_blank(blank, info); - if (ret == 0) - intel_fbdev_invalidate(ifbdev); - - return ret; -} - -static int intel_fbdev_pan_display(struct fb_var_screeninfo *var, - struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct intel_fbdev *ifbdev = - container_of(fb_helper, struct intel_fbdev, helper); - int ret; - - ret = drm_fb_helper_pan_display(var, info); - if (ret == 0) - intel_fbdev_invalidate(ifbdev); - - return ret; -} - static const struct fb_ops intelfb_ops = { .owner = THIS_MODULE, DRM_FB_HELPER_DEFAULT_OPS, - .fb_set_par = intel_fbdev_set_par, + .fb_set_par = drm_fb_helper_set_par, .fb_read = drm_fb_helper_cfb_read, .fb_write = drm_fb_helper_cfb_write, .fb_fillrect = drm_fb_helper_cfb_fillrect, .fb_copyarea = drm_fb_helper_cfb_copyarea, .fb_imageblit = drm_fb_helper_cfb_imageblit, - .fb_pan_display = intel_fbdev_pan_display, - .fb_blank = intel_fbdev_blank, + .fb_pan_display = drm_fb_helper_pan_display, + .fb_blank = drm_fb_helper_blank, }; static int intelfb_alloc(struct drm_fb_helper *helper, @@ -328,8 +276,18 @@ static int intelfb_create(struct drm_fb_helper *helper, return ret; } +static int intel_fbdev_fb_dirty(struct drm_fb_helper *helper, struct drm_clip_rect *clip) +{ + struct intel_fbdev *ifbdev = + container_of(helper, struct intel_fbdev, helper); + struct drm_framebuffer *fb = >fb->base; + + return fb->funcs->dirty(fb, NULL, 0, 0, clip, 1); +} + static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { .fb_probe = intelfb_create, + .fb_dirty = intel_fbdev_fb_dirty, }; static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) @@ -704,8 +662,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev) if (!ifbdev->vma) return; - if (drm_fb_helper_restore_fbdev_mode_unlocked(>helper) == 0) - intel_fbdev_invalidate(ifbdev); + drm_fb_helper_restore_fbdev_mode_unlocked(>helper); } struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev) -- 2.34.1
Re: [Intel-gfx] [PATCH v7 1/2] drm/i915/mtl: limit second scaler vertical scaling in ver >= 14
On Mon, 2023-01-09 at 09:06 +0200, Lisovskiy, Stanislav wrote: > On Fri, Dec 23, 2022 at 03:05:08PM +0200, Luca Coelho wrote: > > In newer hardware versions (i.e. display version >= 14), the second > > scaler doesn't support vertical scaling. > > > > The current implementation of the scaling limits is simplified and > > only occurs when the planes are created, so we don't know which scaler > > is being used. > > > > In order to handle separate scaling limits for horizontal and vertical > > scaling, and different limits per scaler, split the checks in two > > phases. We first do a simple check during plane creation and use the > > best-case scenario (because we don't know the scaler that may be used > > at a later point) and then do a more specific check when the scalers > > are actually being set up. > > > > Signed-off-by: Luca Coelho > > --- > > > > In v2: > >* fix DRM_PLANE_NO_SCALING renamed macros; > > > > In v3: > >* No changes. > > > > In v4: > >* Got rid of the changes in the general planes max scale code; > >* Added a couple of FIXMEs; > >* Made intel_atomic_setup_scaler() return an int with errors; > > > > In v5: > >* Just resent with a cover letter. > > > > In v6: > >* Now the correct version again (same as v4). > > > > In v7: > >* Constify a couple of local variables; > >* Return -EINVAL, instead of -EOPNOTSUPP and -EBUSY; > >* Add another FIXME; > >* Remove unnecessary undoing of change in error cases. > > > > > > drivers/gpu/drm/i915/display/intel_atomic.c | 85 ++--- > > 1 file changed, 75 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > > b/drivers/gpu/drm/i915/display/intel_atomic.c > > index 6621aa245caf..a9a3f3715279 100644 > > --- a/drivers/gpu/drm/i915/display/intel_atomic.c > > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > > @@ -41,6 +41,7 @@ > > #include "intel_global_state.h" > > #include "intel_hdcp.h" > > #include "intel_psr.h" > > +#include "intel_fb.h" > > #include "skl_universal_plane.h" > > > > /** > > @@ -310,11 +311,11 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, > > kfree(crtc_state); > > } > > > > -static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state > > *scaler_state, > > - int num_scalers_need, struct intel_crtc > > *intel_crtc, > > - const char *name, int idx, > > - struct intel_plane_state *plane_state, > > - int *scaler_id) > > +static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state > > *scaler_state, > > +int num_scalers_need, struct intel_crtc > > *intel_crtc, > > +const char *name, int idx, > > +struct intel_plane_state *plane_state, > > +int *scaler_id) > > { > > struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); > > int j; > > @@ -334,7 +335,7 @@ static void intel_atomic_setup_scaler(struct > > intel_crtc_scaler_state *scaler_sta > > > > if (drm_WARN(_priv->drm, *scaler_id < 0, > > "Cannot find scaler for %s:%d\n", name, idx)) > > - return; > > + return -EINVAL; > > As I understand that change is a bit irrelevant to the patch topic, > ideally it should be reflected in the commit message, that we are doing > this and most importantly why. Right, maybe this should have been mentioned in the commit message. I initially didn't return an error for the new failure path, but Ville asked me to do so, so I changed the function to return an error here as well. > However I'm not going to be picky here, as it is a small thing, just > as a side note. Thanks! > Reviewed-by: Stanislav Lisovskiy Thanks for the review, Stan! -- Cheers, Luca.
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/9] drm/i915/display/core: use intel_de_rmw if possible (rev2)
== Series Details == Series: series starting with [v2,1/9] drm/i915/display/core: use intel_de_rmw if possible (rev2) URL : https://patchwork.freedesktop.org/series/112438/ State : success == Summary == CI Bug Log - changes from CI_DRM_12556 -> Patchwork_112438v2 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/index.html Participating hosts (39 -> 38) -- Additional (1): bat-rpls-2 Missing(2): fi-snb-2520m bat-adlp-4 Known issues Here are the changes found in Patchwork_112438v2 that come from known issues: ### IGT changes ### Possible fixes * igt@i915_selftest@live@gt_lrc: - {bat-rpls-1}: [INCOMPLETE][1] ([i915#4983]) -> [PASS][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/bat-rpls-1/igt@i915_selftest@live@gt_lrc.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/bat-rpls-1/igt@i915_selftest@live@gt_lrc.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285 [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295 [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827 [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072 [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845 [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849 [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582 [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282 [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555 [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637 [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708 [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983 [i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257 [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621 [i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359 [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456 [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561 Build changes - * Linux: CI_DRM_12556 -> Patchwork_112438v2 CI-20190529: 20190529 CI_DRM_12556: ac04152253dccfb02dcedfa0c57443122cf79314 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7110: db10a19b94d1d7ae5ba62eb48d52c47ccb27766f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_112438v2: ac04152253dccfb02dcedfa0c57443122cf79314 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 06c2b65be747 drm/i915/display/misc: use intel_de_rmw if possible b5c5ee4a0016 drm/i915/display/interfaces: use intel_de_rmw if possible cca98fd0b977 drm/i915/display/panel: use intel_de_rmw if possible in panel related code 4b2224aafa01 drm/i915/display/hdmi: use intel_de_rmw if possible 7abd22b21efb drm/i915/display/pch: use intel_de_rmw if possible 4e8eed12ac14 drm/i915/display/phys: use intel_de_rmw if possible a15c0ef6cce2 drm/i915/display/dpll: use intel_de_rmw if possible 05b4dff11f26 drm/i915/display/power: use intel_de_rmw if possible ff0bd552fdd5 drm/i915/display/core: use intel_de_rmw if possible == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112438v2/index.html
[Intel-gfx] [PATCH] drm/i915/display: Enable FBC for fbcon
Remove frontbuffer invalidation code from FBC, and just use the dirtyfb helper. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/display/intel_fbdev.c | 68 +- 1 file changed, 15 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index ccee65d9ac16..04406f1162dd 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -40,6 +40,7 @@ #include #include #include +#include #ifdef I915 #include "gem/i915_gem_lmem.h" @@ -80,65 +81,17 @@ struct intel_fbdev { struct mutex hpd_lock; }; -static void intel_fbdev_invalidate(struct intel_fbdev *ifbdev) -{ - intel_frontbuffer_invalidate(ifbdev->fb, ORIGIN_CPU); -} - -static int intel_fbdev_set_par(struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct intel_fbdev *ifbdev = - container_of(fb_helper, struct intel_fbdev, helper); - int ret; - - ret = drm_fb_helper_set_par(info); - if (ret == 0) - intel_fbdev_invalidate(ifbdev); - - return ret; -} - -static int intel_fbdev_blank(int blank, struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct intel_fbdev *ifbdev = - container_of(fb_helper, struct intel_fbdev, helper); - int ret; - - ret = drm_fb_helper_blank(blank, info); - if (ret == 0) - intel_fbdev_invalidate(ifbdev); - - return ret; -} - -static int intel_fbdev_pan_display(struct fb_var_screeninfo *var, - struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct intel_fbdev *ifbdev = - container_of(fb_helper, struct intel_fbdev, helper); - int ret; - - ret = drm_fb_helper_pan_display(var, info); - if (ret == 0) - intel_fbdev_invalidate(ifbdev); - - return ret; -} - static const struct fb_ops intelfb_ops = { .owner = THIS_MODULE, DRM_FB_HELPER_DEFAULT_OPS, - .fb_set_par = intel_fbdev_set_par, + .fb_set_par = drm_fb_helper_set_par, .fb_read = drm_fb_helper_cfb_read, .fb_write = drm_fb_helper_cfb_write, .fb_fillrect = drm_fb_helper_cfb_fillrect, .fb_copyarea = drm_fb_helper_cfb_copyarea, .fb_imageblit = drm_fb_helper_cfb_imageblit, - .fb_pan_display = intel_fbdev_pan_display, - .fb_blank = intel_fbdev_blank, + .fb_pan_display = drm_fb_helper_pan_display, + .fb_blank = drm_fb_helper_blank, }; static int intelfb_alloc(struct drm_fb_helper *helper, @@ -399,8 +352,18 @@ static int intelfb_create(struct drm_fb_helper *helper, return ret; } +static int intel_fbdev_fb_dirty(struct drm_fb_helper *helper, struct drm_clip_rect *clip) +{ + struct intel_fbdev *ifbdev = + container_of(helper, struct intel_fbdev, helper); + struct drm_framebuffer *fb = >fb->base; + + return fb->funcs->dirty(fb, NULL, 0, 0, clip, 1); +} + static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { .fb_probe = intelfb_create, + .fb_dirty = intel_fbdev_fb_dirty, }; static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) @@ -780,8 +743,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev) if (!ifbdev->vma) return; - if (drm_fb_helper_restore_fbdev_mode_unlocked(>helper) == 0) - intel_fbdev_invalidate(ifbdev); + drm_fb_helper_restore_fbdev_mode_unlocked(>helper); } struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev) -- 2.34.1
Re: [Intel-gfx] [PATCH v2 6/9] drm/i915/display/hdmi: use intel_de_rmw if possible
On Mon, 09 Jan 2023, Andrzej Hajda wrote: > On 06.01.2023 16:35, Rodrigo Vivi wrote: >> On Thu, Jan 05, 2023 at 02:10:43PM +0100, Andrzej Hajda wrote: >>> The helper makes the code more compact and readable. >>> >>> Signed-off-by: Andrzej Hajda >>> --- >>> drivers/gpu/drm/i915/display/g4x_hdmi.c | 8 ++--- >>> drivers/gpu/drm/i915/display/intel_hdcp.c | 15 - >>> drivers/gpu/drm/i915/display/intel_hdmi.c | 40 +++ >>> 3 files changed, 22 insertions(+), 41 deletions(-) >>> > > (...) > >>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c >>> b/drivers/gpu/drm/i915/display/intel_hdmi.c >>> index efa2da080f62d4..4b09f17aa4b23b 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c >>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c >>> @@ -237,15 +237,11 @@ static void g4x_read_infoframe(struct intel_encoder >>> *encoder, >>>void *frame, ssize_t len) >>> { >>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >>> - u32 val, *data = frame; >>> + u32 *data = frame; >>> int i; >>> >>> - val = intel_de_read(dev_priv, VIDEO_DIP_CTL); >>> - >>> - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ >> >> A probably good follow-up clean up would be to define the missing masks >> and remove the hardcoded things like this 0xf. >> >> And also something that I had noticed on the previous patches but I forgot >> to mention: it would be good as a followup to replace the local value << >> shift >> per FIELD_PREP() helpers and remove the shift definitions... >> >> But really nothing related directly with this patch. For this: >> >> Reviewed-by: Rodrigo Vivi >> >> Oh, and I also noticed that CI didn't return yet for these patches... >> https://patchwork.freedesktop.org/series/112438/ >> >> a strange delay... I will probably hit the retest if we don't get >> anything by the end of the day today. > > Thx for reviews. > Apparently CI missed this series, I have just hit retest. Cc: Michal and Mateusz This has been happening a lot lately. BR, Jani. > > Regards > Andrzej > > >> >>> - val |= g4x_infoframe_index(type); >>> - >>> - intel_de_write(dev_priv, VIDEO_DIP_CTL, val); >>> + intel_de_rmw(dev_priv, VIDEO_DIP_CTL, >>> +VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); >>> >>> for (i = 0; i < len; i += 4) >>> *data++ = intel_de_read(dev_priv, VIDEO_DIP_DATA); >>> @@ -313,15 +309,11 @@ static void ibx_read_infoframe(struct intel_encoder >>> *encoder, >>> { >>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >>> - u32 val, *data = frame; >>> + u32 *data = frame; >>> int i; >>> >>> - val = intel_de_read(dev_priv, TVIDEO_DIP_CTL(crtc->pipe)); >>> - >>> - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ >>> - val |= g4x_infoframe_index(type); >>> - >>> - intel_de_write(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), val); >>> + intel_de_rmw(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), >>> +VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); >>> >>> for (i = 0; i < len; i += 4) >>> *data++ = intel_de_read(dev_priv, TVIDEO_DIP_DATA(crtc->pipe)); >>> @@ -395,15 +387,11 @@ static void cpt_read_infoframe(struct intel_encoder >>> *encoder, >>> { >>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >>> - u32 val, *data = frame; >>> + u32 *data = frame; >>> int i; >>> >>> - val = intel_de_read(dev_priv, TVIDEO_DIP_CTL(crtc->pipe)); >>> - >>> - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ >>> - val |= g4x_infoframe_index(type); >>> - >>> - intel_de_write(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), val); >>> + intel_de_rmw(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), >>> +VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); >>> >>> for (i = 0; i < len; i += 4) >>> *data++ = intel_de_read(dev_priv, TVIDEO_DIP_DATA(crtc->pipe)); >>> @@ -471,15 +459,11 @@ static void vlv_read_infoframe(struct intel_encoder >>> *encoder, >>> { >>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >>> - u32 val, *data = frame; >>> + u32 *data = frame; >>> int i; >>> >>> - val = intel_de_read(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe)); >>> - >>> - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ >>> - val |= g4x_infoframe_index(type); >>> - >>> - intel_de_write(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe), val); >>> + intel_de_rmw(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe), >>> +VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); >>> >>> for (i = 0; i < len; i += 4) >>> *data++ = intel_de_read(dev_priv,
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/9] drm/i915/display/core: use intel_de_rmw if possible (rev2)
== Series Details == Series: series starting with [v2,1/9] drm/i915/display/core: use intel_de_rmw if possible (rev2) URL : https://patchwork.freedesktop.org/series/112438/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:154:26:
[Intel-gfx] [PATCH 5/5] drm/plane: Unexport drm_any_plane_has_format()
As the format validation is being dealt with exclusively inside framebuffer_check(), there is no need to export the drm_any_plane_has_format() symbol. Therefore, unexport the drm_any_plane_has_format() symbol, reinforcing that format validation is being dealt with by the DRM API. Signed-off-by: Maíra Canal --- drivers/gpu/drm/drm_plane.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 24e7998d1731..67c0ab60c7b6 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -837,7 +837,6 @@ bool drm_any_plane_has_format(struct drm_device *dev, return false; } -EXPORT_SYMBOL(drm_any_plane_has_format); /* * __setplane_internal - setplane handler for internal callers -- 2.39.0
[Intel-gfx] [PATCH 4/5] drm/vmwgfx: Remove redundant framebuffer format check
Now that framebuffer_check() verifies that the format is properly supported, there is no need to check it again on vmwgfx's inside helpers. Therefore, remove the redundant framebuffer format check from the vmw_kms_new_framebuffer_surface() and vmw_kms_new_framebuffer_bo() functions, letting framebuffer_check() perform the framebuffer validation. Signed-off-by: Maíra Canal --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 18 -- 1 file changed, 18 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 257f090071f1..05b8d8f912bf 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -1317,15 +1317,6 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv, * Sanity checks. */ - if (!drm_any_plane_has_format(_priv->drm, - mode_cmd->pixel_format, - mode_cmd->modifier[0])) { - drm_dbg(_priv->drm, - "unsupported pixel format %p4cc / modifier 0x%llx\n", - _cmd->pixel_format, mode_cmd->modifier[0]); - return -EINVAL; - } - /* Surface must be marked as a scanout. */ if (unlikely(!surface->metadata.scanout)) return -EINVAL; @@ -1648,15 +1639,6 @@ static int vmw_kms_new_framebuffer_bo(struct vmw_private *dev_priv, return -EINVAL; } - if (!drm_any_plane_has_format(_priv->drm, - mode_cmd->pixel_format, - mode_cmd->modifier[0])) { - drm_dbg(_priv->drm, - "unsupported pixel format %p4cc / modifier 0x%llx\n", - _cmd->pixel_format, mode_cmd->modifier[0]); - return -EINVAL; - } - vfbd = kzalloc(sizeof(*vfbd), GFP_KERNEL); if (!vfbd) { ret = -ENOMEM; -- 2.39.0
[Intel-gfx] [PATCH 3/5] drm/i915: Remove redundant framebuffer format check
Now that framebuffer_check() verifies that the format is properly supported, there is no need to check it again on i915's inside helpers. Therefore, remove the redundant framebuffer format check from the intel_framebuffer_init() function, letting framebuffer_check() perform the framebuffer validation. Signed-off-by: Maíra Canal --- drivers/gpu/drm/i915/display/intel_fb.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c index 63137ae5ab21..230b729e42d6 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.c +++ b/drivers/gpu/drm/i915/display/intel_fb.c @@ -1914,15 +1914,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb, } } - if (!drm_any_plane_has_format(_priv->drm, - mode_cmd->pixel_format, - mode_cmd->modifier[0])) { - drm_dbg_kms(_priv->drm, - "unsupported pixel format %p4cc / modifier 0x%llx\n", - _cmd->pixel_format, mode_cmd->modifier[0]); - goto err; - } - /* * gen2/3 display engine uses the fence if present, * so the tiling mode must match the fb modifier exactly. -- 2.39.0
[Intel-gfx] [PATCH 2/5] drm/amdgpu: Remove redundant framebuffer format check
Now that framebuffer_check() verifies that the format is properly supported, there is no need to check it again on amdgpu's inside helpers. Therefore, remove the redundant framebuffer format check from the amdgpu_display_gem_fb_verify_and_init() function, letting framebuffer_check() perform the framebuffer validation. Signed-off-by: Maíra Canal --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index b22471b3bd63..611b7a4b086c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -1120,16 +1120,6 @@ static int amdgpu_display_gem_fb_verify_and_init(struct drm_device *dev, rfb->base.obj[0] = obj; drm_helper_mode_fill_fb_struct(dev, >base, mode_cmd); - /* Verify that the modifier is supported. */ - if (!drm_any_plane_has_format(dev, mode_cmd->pixel_format, - mode_cmd->modifier[0])) { - drm_dbg_kms(dev, - "unsupported pixel format %p4cc / modifier 0x%llx\n", - _cmd->pixel_format, mode_cmd->modifier[0]); - - ret = -EINVAL; - goto err; - } ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj); if (ret) -- 2.39.0
[Intel-gfx] [PATCH 0/5] Check for valid framebuffer's format
This series is a follow-up of [1] in which I introduced a check for valid formats on drm_gem_fb_create(). During the discussion, I realized that would be a better idea to put the check inside framebuffer_check() so that it wouldn't be needed to hit any driver-specific code path when the check fails. Thanks to Daniel, Simon, Rob and Thomas for the insightful discussion! Therefore, add the valid format check inside framebuffer_check() and remove the same check from the drivers. Adding the check to framebuffer_check() will guarantee that the igt@kms_addfb_basic@addfb25-bad-modifier IGT test passes, showing the correct behavior of the check. This patchset was tested on i915, amdgpu, and vc4 with the IGT tests. [1] https://lore.kernel.org/dri-devel/20230103125322.855089-1-mca...@igalia.com/T/ Best Regards, - Maíra Canal Maíra Canal (5): drm/framebuffer: Check for valid formats drm/amdgpu: Remove redundant framebuffer format check drm/i915: Remove redundant framebuffer format check drm/vmwgfx: Remove redundant framebuffer format check drm/plane: Unexport drm_any_plane_has_format() Documentation/gpu/todo.rst | 9 - drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 10 -- drivers/gpu/drm/drm_framebuffer.c | 8 drivers/gpu/drm/drm_plane.c | 1 - drivers/gpu/drm/i915/display/intel_fb.c | 9 - drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 18 -- 6 files changed, 12 insertions(+), 43 deletions(-) -- 2.39.0
[Intel-gfx] [PATCH 1/5] drm/framebuffer: Check for valid formats
Currently, framebuffer_check() doesn't check if the pixel format is supported, which can lead to the acceptance of invalid pixel formats e.g. the acceptance of invalid modifiers. Therefore, add a check for valid formats on framebuffer_check(), so that the ADDFB2 IOCTL rejects calls with invalid formats. Moreover, note that this check is only valid for atomic drivers, because, for non-atomic drivers, checking drm_any_plane_has_format() is not possible since the format list for the primary plane is fake, and we'd therefore reject valid formats. Signed-off-by: Maíra Canal --- Documentation/gpu/todo.rst| 9 - drivers/gpu/drm/drm_framebuffer.c | 8 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 1f8a5ebe188e..3a79c26c5cc7 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -276,11 +276,10 @@ Various hold-ups: - Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb setup code can't be deleted. -- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For - atomic drivers we could check for valid formats by calling - drm_plane_check_pixel_format() against all planes, and pass if any plane - supports the format. For non-atomic that's not possible since like the format - list for the primary plane is fake and we'd therefor reject valid formats. +- Need to switch to drm_gem_fb_create(), as now framebuffer_check() checks for + valid formats for atomic drivers. + +- Add an addfb format validation for non-atomic drivers. - Many drivers subclass drm_framebuffer, we'd need a embedding compatible version of the varios drm_gem_fb_create functions. Maybe called diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index aff3746dedfb..605642bf3650 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -280,6 +280,14 @@ static int framebuffer_check(struct drm_device *dev, } } + /* Verify that the modifier is supported. */ + if (drm_drv_uses_atomic_modeset(dev) && + !drm_any_plane_has_format(dev, r->pixel_format, r->modifier[0])) { + drm_dbg_kms(dev, "Unsupported pixel format %p4cc / modifier 0x%llx\n", + >pixel_format, r->modifier[0]); + return -EINVAL; + } + return 0; } -- 2.39.0
Re: [Intel-gfx] [PATCH v2 6/9] drm/i915/display/hdmi: use intel_de_rmw if possible
On 06.01.2023 16:35, Rodrigo Vivi wrote: On Thu, Jan 05, 2023 at 02:10:43PM +0100, Andrzej Hajda wrote: The helper makes the code more compact and readable. Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/i915/display/g4x_hdmi.c | 8 ++--- drivers/gpu/drm/i915/display/intel_hdcp.c | 15 - drivers/gpu/drm/i915/display/intel_hdmi.c | 40 +++ 3 files changed, 22 insertions(+), 41 deletions(-) (...) diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index efa2da080f62d4..4b09f17aa4b23b 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -237,15 +237,11 @@ static void g4x_read_infoframe(struct intel_encoder *encoder, void *frame, ssize_t len) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); - u32 val, *data = frame; + u32 *data = frame; int i; - val = intel_de_read(dev_priv, VIDEO_DIP_CTL); - - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ A probably good follow-up clean up would be to define the missing masks and remove the hardcoded things like this 0xf. And also something that I had noticed on the previous patches but I forgot to mention: it would be good as a followup to replace the local value << shift per FIELD_PREP() helpers and remove the shift definitions... But really nothing related directly with this patch. For this: Reviewed-by: Rodrigo Vivi Oh, and I also noticed that CI didn't return yet for these patches... https://patchwork.freedesktop.org/series/112438/ a strange delay... I will probably hit the retest if we don't get anything by the end of the day today. Thx for reviews. Apparently CI missed this series, I have just hit retest. Regards Andrzej - val |= g4x_infoframe_index(type); - - intel_de_write(dev_priv, VIDEO_DIP_CTL, val); + intel_de_rmw(dev_priv, VIDEO_DIP_CTL, +VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); for (i = 0; i < len; i += 4) *data++ = intel_de_read(dev_priv, VIDEO_DIP_DATA); @@ -313,15 +309,11 @@ static void ibx_read_infoframe(struct intel_encoder *encoder, { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); - u32 val, *data = frame; + u32 *data = frame; int i; - val = intel_de_read(dev_priv, TVIDEO_DIP_CTL(crtc->pipe)); - - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ - val |= g4x_infoframe_index(type); - - intel_de_write(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), val); + intel_de_rmw(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), +VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); for (i = 0; i < len; i += 4) *data++ = intel_de_read(dev_priv, TVIDEO_DIP_DATA(crtc->pipe)); @@ -395,15 +387,11 @@ static void cpt_read_infoframe(struct intel_encoder *encoder, { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); - u32 val, *data = frame; + u32 *data = frame; int i; - val = intel_de_read(dev_priv, TVIDEO_DIP_CTL(crtc->pipe)); - - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ - val |= g4x_infoframe_index(type); - - intel_de_write(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), val); + intel_de_rmw(dev_priv, TVIDEO_DIP_CTL(crtc->pipe), +VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); for (i = 0; i < len; i += 4) *data++ = intel_de_read(dev_priv, TVIDEO_DIP_DATA(crtc->pipe)); @@ -471,15 +459,11 @@ static void vlv_read_infoframe(struct intel_encoder *encoder, { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); - u32 val, *data = frame; + u32 *data = frame; int i; - val = intel_de_read(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe)); - - val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ - val |= g4x_infoframe_index(type); - - intel_de_write(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe), val); + intel_de_rmw(dev_priv, VLV_TVIDEO_DIP_CTL(crtc->pipe), +VIDEO_DIP_SELECT_MASK | 0xf, g4x_infoframe_index(type)); for (i = 0; i < len; i += 4) *data++ = intel_de_read(dev_priv, -- 2.34.1
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/display: drop redundant display/ from #includes (rev3)
== Series Details == Series: drm/i915/display: drop redundant display/ from #includes (rev3) URL : https://patchwork.freedesktop.org/series/111803/ State : failure == Summary == CI Bug Log - changes from CI_DRM_12556 -> Patchwork_111803v3 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_111803v3 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_111803v3, 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_111803v3/index.html Participating hosts (39 -> 39) -- Additional (2): fi-kbl-soraka bat-rpls-2 Missing(2): fi-bsw-kefka fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_111803v3: ### IGT changes ### Possible regressions * igt@i915_selftest@live@slpc: - fi-kbl-soraka: NOTRUN -> [INCOMPLETE][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111803v3/fi-kbl-soraka/igt@i915_selftest@l...@slpc.html Known issues Here are the changes found in Patchwork_111803v3 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_gttfill@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][2] ([fdo#109271]) +7 similar issues [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111803v3/fi-kbl-soraka/igt@gem_exec_gttf...@basic.html * igt@gem_huc_copy@huc-copy: - fi-kbl-soraka: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111803v3/fi-kbl-soraka/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111803v3/fi-kbl-soraka/igt@gem_lmem_swapp...@basic.html * igt@i915_selftest@live@gt_pm: - fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][5] ([i915#1886]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111803v3/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html * igt@kms_chamelium@hdmi-hpd-fast: - fi-kbl-soraka: NOTRUN -> [SKIP][6] ([fdo#109271] / [fdo#111827]) +7 similar issues [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111803v3/fi-kbl-soraka/igt@kms_chamel...@hdmi-hpd-fast.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size: - fi-bsw-n3050: [PASS][7] -> [FAIL][8] ([i915#6298]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111803v3/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cur...@atomic-transitions-varying-size.html Possible fixes * igt@i915_selftest@live@gt_lrc: - {bat-rpls-1}: [INCOMPLETE][9] ([i915#4983]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12556/bat-rpls-1/igt@i915_selftest@live@gt_lrc.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111803v3/bat-rpls-1/igt@i915_selftest@live@gt_lrc.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285 [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295 [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827 [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072 [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845 [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849 [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886 [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190 [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582 [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282 [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555 [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637 [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708 [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983 [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298 [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621 [i915#7456]:
Re: [Intel-gfx] [PATCH v2 04/21] drm/i915/mtl: Add Support for C10 PHY message bus and pll programming
> -Original Message- > From: Nikula, Jani > Sent: Monday, January 9, 2023 11:50 AM > To: Kahola, Mika ; intel-gfx@lists.freedesktop.org > Cc: Deak, Imre ; Sripada, Radhakrishna > ; Kahola, Mika ; > Shankar, Uma > Subject: Re: [PATCH v2 04/21] drm/i915/mtl: Add Support for C10 PHY message > bus and pll programming > > On Thu, 05 Jan 2023, Mika Kahola wrote: > > +static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum > > +port port, int lane, u32 *val) { > > + enum phy phy = intel_port_to_phy(i915, port); > > + > > + if (__intel_wait_for_register(>uncore, > > There's now an __intel_de_ variant of this that should be used within > display/. Ah, ok. I will check that one out and switch to use that. Thanks! -Mika- > > BR, > Jani. > > > -- > Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 03/27] drm/i915/gvt: Incorporate KVM memslot info into check for 2MiB GTT entry
On Fri, Jan 06, 2023 at 11:01:53PM +, Sean Christopherson wrote: > On Fri, Jan 06, 2023, Yan Zhao wrote: > > On Thu, Jan 05, 2023 at 05:40:32PM +, Sean Christopherson wrote: > > > On Thu, Jan 05, 2023, Yan Zhao wrote: > > > I'm totally fine if KVMGT's ABI is that VFIO is the source of truth for > > > mappings > > > and permissions, and that the only requirement for KVM memslots is that > > > GTT page > > > tables need to be visible in KVM's memslots. But if that's the ABI, then > > > intel_gvt_is_valid_gfn() should be probing VFIO, not KVM (commit > > > cc753fbe1ac4 > > > ("drm/i915/gvt: validate gfn before set shadow page entry"). > > > > > > In other words, pick either VFIO or KVM. Checking that X is valid > > > according to > > > KVM and then mapping X through VFIO is confusing and makes assumptions > > > about how > > > userspace configures KVM and VFIO. It works because QEMU always > > > configures KVM > > > and VFIO as expected, but IMO it's unnecessarily fragile and again > > > confusing for > > > unaware readers because the code is technically flawed. > > > > > Agreed. > > Then after some further thought, I think maybe we can just remove > > intel_gvt_is_valid_gfn() in KVMGT, because > > > > (1) both intel_gvt_is_valid_gfn() in emulate_ggtt_mmio_write() and > > ppgtt_populate_spt() are not for page track purpose, but to validate bogus > > GFN. > > (2) gvt_pin_guest_page() with gfn and size can do the validity checking, > > which is called in intel_gvt_dma_map_guest_page(). So, we can move the > > mapping of scratch page to the error path after > > intel_gvt_dma_map_guest_page(). > > IIUC, that will re-introduce the problem commit cc753fbe1ac4 ("drm/i915/gvt: > validate > gfn before set shadow page entry") solved by poking into KVM. Lack of > pre-validation > means that bogus GFNs will trigger error messages, e.g. > > gvt_vgpu_err("vfio_pin_pages failed for iova %pad, ret > %d\n", >_iova, ret); > > and > > gvt_vgpu_err("fail to populate guest ggtt entry\n"); Thanks for pointing it out. I checked this commit message and found below original intentions to introduce pre-validation: "GVT may receive partial write on one guest PTE update. Validate gfn not to translate incomplete gfn. This avoids some unnecessary error messages incurred by the incomplete gfn translating. Also fix the bug that the whole PPGTT shadow page update is aborted on any invalid gfn entry" (1) first intention -- unnecessary error message came from GGTT partial write. For guest GGTT writes, the guest calls writeq to an MMIO GPA, which is 8 bytes in length, while QEMU splits the MMIO write into 2 4-byte writes. The splitted 2 writes can cause invalid GFN to be found. But this partial write issue has been fixed by the two follow-up commits: bc0686ff5fad drm/i915/gvt: support inconsecutive partial gtt entry write 510fe10b6180 drm/i915/gvt: fix a bug of partially write ggtt enties so pre-validation to reduce noise is not necessary any more here. (2) the second intention -- "the whole PPGTT shadow page update is aborted on any invalid gfn entry" As PPGTT resides in normal guest RAM and we only treat 8-byte writes as valid page table writes, any invalid GPA found is regarded as an error, either due to guest misbehavior/attack or bug in host shadow code. So, direct abort looks good too. Like below: @@ -1340,13 +1338,6 @@ static int ppgtt_populate_spt(struct intel_vgpu_ppgtt_spt *spt) ppgtt_generate_shadow_entry(, s, ); ppgtt_set_shadow_entry(spt, , i); } else { - gfn = ops->get_pfn(); - if (!intel_gvt_is_valid_gfn(vgpu, gfn)) { - ops->set_pfn(, gvt->gtt.scratch_mfn); - ppgtt_set_shadow_entry(spt, , i); - continue; - } - ret = ppgtt_populate_shadow_entry(vgpu, spt, i, ); if (ret) goto fail; (I actually found that the original code will print "invalid entry type" warning which indicates it's broken for a while due to lack of test in this invalid gfn path) > One thought would be to turn those printks into tracepoints to eliminate > unwanted > noise, and to prevent the guest from spamming the host kernel log by > programming > garbage into the GTT (gvt_vgpu_err() isn't ratelimited). As those printks would not happen in normal conditions and printks may have some advantages to discover the attack or bug, could we just convert gvt_vgpu_err() to be ratelimited ? Thanks Yan > > > > On a related topic, ppgtt_populate_shadow_entry() should check the > > > validity of the > > > gfn. If I'm reading the code correctly, checking only in > > >
Re: [Intel-gfx] [PATCH] drm: Alloc high address for drm buddy topdown flag
Am 07.01.23 um 16:15 schrieb Arunpravin Paneer Selvam: As we are observing low numbers in viewperf graphics benchmark, we are strictly not allowing the top down flag enabled allocations to steal the memory space from cpu visible region. The approach is, we are sorting each order list entries in ascending order and compare the last entry of each order list in the freelist and return the max block. This patch improves the viewperf 3D benchmark scores. Signed-off-by: Arunpravin Paneer Selvam Acked-by: Christian König , but somebody with more insight of the drm buddy allocator should take a closer look at this. --- drivers/gpu/drm/drm_buddy.c | 81 - 1 file changed, 54 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index 11bb59399471..50916b2f2fc5 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -38,6 +38,25 @@ static void drm_block_free(struct drm_buddy *mm, kmem_cache_free(slab_blocks, block); } +static void list_insert_sorted(struct drm_buddy *mm, + struct drm_buddy_block *block) +{ + struct drm_buddy_block *node; + struct list_head *head; + + head = >free_list[drm_buddy_block_order(block)]; + if (list_empty(head)) { + list_add(>link, head); + return; + } + + list_for_each_entry(node, head, link) + if (drm_buddy_block_offset(block) < drm_buddy_block_offset(node)) + break; + + __list_add(>link, node->link.prev, >link); +} + static void mark_allocated(struct drm_buddy_block *block) { block->header &= ~DRM_BUDDY_HEADER_STATE; @@ -52,8 +71,7 @@ static void mark_free(struct drm_buddy *mm, block->header &= ~DRM_BUDDY_HEADER_STATE; block->header |= DRM_BUDDY_FREE; - list_add(>link, ->free_list[drm_buddy_block_order(block)]); + list_insert_sorted(mm, block); } static void mark_split(struct drm_buddy_block *block) @@ -387,20 +405,26 @@ alloc_range_bias(struct drm_buddy *mm, } static struct drm_buddy_block * -get_maxblock(struct list_head *head) +get_maxblock(struct drm_buddy *mm, unsigned int order) { struct drm_buddy_block *max_block = NULL, *node; + unsigned int i; - max_block = list_first_entry_or_null(head, -struct drm_buddy_block, -link); - if (!max_block) - return NULL; + for (i = order; i <= mm->max_order; ++i) { + if (!list_empty(>free_list[i])) { + node = list_last_entry(>free_list[i], + struct drm_buddy_block, + link); + if (!max_block) { + max_block = node; + continue; + } - list_for_each_entry(node, head, link) { - if (drm_buddy_block_offset(node) > - drm_buddy_block_offset(max_block)) - max_block = node; + if (drm_buddy_block_offset(node) > + drm_buddy_block_offset(max_block)) { + max_block = node; + } + } } return max_block; @@ -412,20 +436,23 @@ alloc_from_freelist(struct drm_buddy *mm, unsigned long flags) { struct drm_buddy_block *block = NULL; - unsigned int i; + unsigned int tmp; int err; - for (i = order; i <= mm->max_order; ++i) { - if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) { - block = get_maxblock(>free_list[i]); - if (block) - break; - } else { - block = list_first_entry_or_null(>free_list[i], -struct drm_buddy_block, -link); - if (block) - break; + if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) { + block = get_maxblock(mm, order); + if (block) + /* Store the obtained block order */ + tmp = drm_buddy_block_order(block); + } else { + for (tmp = order; tmp <= mm->max_order; ++tmp) { + if (!list_empty(>free_list[tmp])) { + block = list_last_entry(>free_list[tmp], + struct drm_buddy_block, + link); + if (block) + break; + }
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Implement workaround for CDCLK PLL disable/enable
On Thu, Jan 05, 2023 at 03:05:40AM +0200, Srivatsa, Anusha wrote: > > > > -Original Message- > > From: Lisovskiy, Stanislav > > Sent: Thursday, December 15, 2022 2:14 AM > > To: Srivatsa, Anusha > > Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani > > Subject: Re: [Intel-gfx] [PATCH 1/1] drm/i915: Implement workaround for > > CDCLK PLL disable/enable > > > > On Wed, Dec 14, 2022 at 09:15:07PM +0200, Srivatsa, Anusha wrote: > > > > > > > > > > -Original Message- > > > > From: Lisovskiy, Stanislav > > > > Sent: Wednesday, December 14, 2022 2:31 AM > > > > To: Srivatsa, Anusha > > > > Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani > > > > > > > > Subject: Re: [Intel-gfx] [PATCH 1/1] drm/i915: Implement workaround > > > > for CDCLK PLL disable/enable > > > > > > > > On Tue, Nov 29, 2022 at 09:19:40PM +0200, Srivatsa, Anusha wrote: > > > > > > > > > > > > > > > > -Original Message- > > > > > > From: Intel-gfx On > > > > > > Behalf Of Stanislav Lisovskiy > > > > > > Sent: Thursday, November 24, 2022 2:36 AM > > > > > > To: intel-gfx@lists.freedesktop.org > > > > > > Cc: Nikula, Jani > > > > > > Subject: [Intel-gfx] [PATCH 1/1] drm/i915: Implement workaround > > > > > > for CDCLK PLL disable/enable > > > > > > > > > > > > It was reported that we might get a hung and loss of register > > > > > > access in some cases when CDCLK PLL is disabled and then > > > > > > enabled, while squashing is enabled. > > > > > > As a workaround it was proposed by HW team that SW should > > > > > > disable squashing when CDCLK PLL is being reenabled. > > > > > > > > > > > > Signed-off-by: Stanislav Lisovskiy > > > > > > > > > > > > --- > > > > > > drivers/gpu/drm/i915/display/intel_cdclk.c | 14 -- > > > > > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > > > > > > b/drivers/gpu/drm/i915/display/intel_cdclk.c > > > > > > index 0c107a38f9d0..e338f288c9ac 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > > > > > @@ -1801,6 +1801,13 @@ static bool > > > > > > cdclk_compute_crawl_and_squash_midpoint(struct > > drm_i915_private > > > > *i91 > > > > > > return true; > > > > > > } > > > > > > > > > > > > +static bool pll_enable_wa_needed(struct drm_i915_private > > > > > > +*dev_priv) > > > > { > > > > > > + return ((IS_DG2(dev_priv) || IS_METEORLAKE(dev_priv)) > > > > > > + && dev_priv->display.cdclk.hw.vco > 0 > > > > > > + && HAS_CDCLK_SQUASH(dev_priv)); > > > > > Redundant check. If it is MTL or DG2, then it will have > > > > HAS_CDCLK_SQUASH set to true always. Shouldn't vco be 0 instead of > > > 0. > > > > The commit message says the hang can be observed when moving from > > 0 > > > > to > > > > > 0 vco. > > > > > > > > > > Anusha > > > > > > > > Idea was that we probably should bind more to the feature rather > > > > than platform, I agree checking both "IS_DG2" and if platform has > > > > squashing is redundant, because then we would have to add each new > > > > platform manually, so I would leave HAS_CDCLK_SQUASH and then at > > > > some point just cut off using some INTEL_GEN or other check all the > > > > new platforms where this is fixed in HW. > > > > > > > > Regarding vco, the icl_cdclk_pll_update func works as follows: > > > > > > > > if (i915->display.cdclk.hw.vco != 0 && > > > > i915->display.cdclk.hw.vco != vco) > > > > icl_cdclk_pll_disable(i915); > > > > > > > > if (i915->display.cdclk.hw.vco != vco) > > > > icl_cdclk_pll_enable(i915, vco); > > > > > > > > 1) if vco changes from zero value(i915->display.cdclk.hw.vco) to > > > > non-zero value(vco), that means > > > >currently squashing is anyway disabled(if vco == 0, cdclk is set to > > "bypass" > > > > and squash waveform > > > >is 0), so the W/A is not needed. > > > > > > > > 2) if vco changes from non-zero value in i915->display.cdclk.hw.vco > > > > to zero value(vco), we are not > > > >going to call icl_cdclk_pll_enable anyway so W/A is also not needed. > > > > > > > > 3) if vco changes from some non-zero value in > > > > i915->display.cdclk.hw.vco to other non-zero value(vco), > > > >which can happen if CDCLK changes, then icl_cdclk_pll_disable(hw > > > > vco!=0 and vco!=0) and then > > > >icl_cdclk_pll_enable would be called(hw vco is still not equal to > > > > vco) > > > >So that disabling enabling in between is what we are interested > > > > and where we should make sure > > > >squashing is disabled. > > > >BTW I have another question - is this code even correct? > > > > Shouldn't we avoid disabling/enabling PLL if we have > > > >squashing/crawling? As I understood the whole point of having > > > > swuashing/crawling is avoiding swithing the PLL > > > >on and off. > > > > > > > Squashing works when we don't need to change the PLL ratio. We fix the > > PLL ratio to
Re: [Intel-gfx] [PATCH v2 04/21] drm/i915/mtl: Add Support for C10 PHY message bus and pll programming
On Thu, 05 Jan 2023, Mika Kahola wrote: > +static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port > port, int lane, u32 *val) > +{ > + enum phy phy = intel_port_to_phy(i915, port); > + > + if (__intel_wait_for_register(>uncore, There's now an __intel_de_ variant of this that should be used within display/. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 11/13] drm/i915/dsb: Add mode DSB opcodes
On Thu, 05 Jan 2023, "Manna, Animesh" wrote: >> -Original Message- >> From: Intel-gfx On Behalf Of Ville >> Syrjala >> Sent: Friday, December 16, 2022 6:08 AM >> To: intel-gfx@lists.freedesktop.org >> Subject: [Intel-gfx] [PATCH 11/13] drm/i915/dsb: Add mode DSB opcodes >> >> From: Ville Syrjälä >> >> Add all the know DSB instruction opcodes. >> >> Signed-off-by: Ville Syrjälä >> --- >> drivers/gpu/drm/i915/display/intel_dsb.c | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c >> b/drivers/gpu/drm/i915/display/intel_dsb.c >> index 7c593ec84d41..96bc117fd6a0 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dsb.c >> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c >> @@ -67,8 +67,16 @@ struct intel_dsb { >> >> /* DSB opcodes. */ >> #define DSB_OPCODE_SHIFT24 >> +#define DSB_OPCODE_NOOP 0x0 >> #define DSB_OPCODE_MMIO_WRITE 0x1 >> +#define DSB_OPCODE_WAIT_USEC0x2 >> +#define DSB_OPCODE_WAIT_LINES 0x3 >> +#define DSB_OPCODE_WAIT_VBLANKS 0x4 >> +#define DSB_OPCODE_WAIT_DSL_IN 0x5 >> +#define DSB_OPCODE_WAIT_DSL_OUT 0x6 >> +#define DSB_OPCODE_INTERRUPT0x7 >> #define DSB_OPCODE_INDEXED_WRITE0x9 >> +#define DSB_OPCODE_POLL 0xA >> #define DSB_BYTE_EN 0xF >> #define DSB_BYTE_EN_SHIFT 20 >> #define DSB_REG_VALUE_MASK 0xf > > Not sure if we can check-in the above macros without its usage. It depends on the case. Here, I think we can and we should. BR, Jani. > > Regards, > Animesh > >> -- >> 2.37.4 > -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Add GuC CT specific debug print wrappers
On 06/01/2023 18:57, John Harrison wrote: On 12/6/2022 03:06, Tvrtko Ursulin wrote: On 05/12/2022 18:44, Michal Wajdeczko wrote: On 05.12.2022 14:16, Tvrtko Ursulin wrote: On 02/12/2022 20:14, John Harrison wrote: [snip] Random meaningless (to me) message that is apparently a display thing: drm_dbg_kms(_priv->drm, "disabling %s\n", pll->info->name); i915 :00:02.0: [drm:intel_disable_shared_dpll [i915]] disabling PORT PLL B Plan is to not touch outside gt/. For some unexplicable reason that means it is almost impossible to see the actual problems in most CI dmesg logs because they are swamped with irrelevant display messages that cannot be filtered out. For example, I recently manually grep'd out all the display spam from a bug report log. The dmesg file went from 12MB to 700KB. That is a significant problem that makes bug triage way harder than it needs to be. I didn't get this part, how it would reduce the amount of spam by adding new macros? Anyway, that's something to split out and discuss with display folks. Maybe as a way forward work could be split? If John wants to deal with gt_xxx macros, avoid touching GuC (putting his original motivation aside) and you want to convert the gt/uc folder? Assuming John you are okay with "GuC:" and "CT:" prefixes. Meaning just repost patch #1 only and expand to more intel_gt_* files? Sure, if someone will actually reply to that patch with some kind of r-b first so I know I'm not still wasting my time on a huge re-write that will to be redone multiple times when someone objects to the use of a colon or the lack of spaces, braces or whatever. First patch looks good to me (ack in principle) apart that Michal found one potential null pointer dereference if I understood it right. That other comment about the ratelimited call is maybe okay to leave for later, *if* it will be a single instance, otherwise needs a gt logger as well. I can r-b once you re-send with the first issue fixed. Regards, Tvrtko
Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Add GuC CT specific debug print wrappers
On Fri, 06 Jan 2023, John Harrison wrote: > On 12/6/2022 03:06, Tvrtko Ursulin wrote: >> On 05/12/2022 18:44, Michal Wajdeczko wrote: >>> On 05.12.2022 14:16, Tvrtko Ursulin wrote: On 02/12/2022 20:14, John Harrison wrote: [snip] > Random meaningless (to me) message that is apparently a display thing: > drm_dbg_kms(_priv->drm, "disabling %s\n", pll->info->name); > i915 :00:02.0: [drm:intel_disable_shared_dpll [i915]] disabling > PORT PLL B Plan is to not touch outside gt/. > For some unexplicable reason that means it is almost impossible to see > the actual problems in most CI dmesg logs because they are swamped with > irrelevant display messages that cannot be filtered out. For example, I > recently manually grep'd out all the display spam from a bug report log. > The dmesg file went from 12MB to 700KB. That is a significant problem > that makes bug triage way harder than it needs to be. You can adjust drm.debug module parameter to get rid of almost all display debugs. They're logged using the appropriate debug categories. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center