[PATCH] drm/nouveau: Fix spelling typo in comments
From: pengfuyuan Fix spelling typo in comments. Reported-by: k2ci Signed-off-by: pengfuyuan --- drivers/gpu/drm/nouveau/include/nvhw/drf.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/include/nvhw/drf.h b/drivers/gpu/drm/nouveau/include/nvhw/drf.h index bd0fc41446e2..d6969c0e2f29 100644 --- a/drivers/gpu/drm/nouveau/include/nvhw/drf.h +++ b/drivers/gpu/drm/nouveau/include/nvhw/drf.h @@ -190,7 +190,7 @@ #define DRF_MD_(X,_1,_2,_3,_4,_5,_6,_7,_8,_9,_10,IMPL,...) IMPL #define DRF_MD(A...) DRF_MD_(X, ##A, DRF_MD_I, DRF_MD_N)(X, ##A) -/* Helper for testing against field value in aribtrary object */ +/* Helper for testing against field value in arbitrary object */ #define DRF_TV_N(X,e,p,o,d,r, f,cmp,v) \ NVVAL_TEST_X(DRF_RD_X(e, (p), (o), d##_##r ), d##_##r##_##f, cmp, (v)) #define DRF_TV_I(X,e,p,o,d,r,i,f,cmp,v) \ @@ -198,7 +198,7 @@ #define DRF_TV_(X,_1,_2,_3,_4,_5,_6,_7,_8,_9,IMPL,...) IMPL #define DRF_TV(A...) DRF_TV_(X, ##A, DRF_TV_I, DRF_TV_N)(X, ##A) -/* Helper for testing against field definition in aribtrary object */ +/* Helper for testing against field definition in arbitrary object */ #define DRF_TD_N(X,e,p,o,d,r, f,cmp,v) \ NVVAL_TEST_X(DRF_RD_X(e, (p), (o), d##_##r ), d##_##r##_##f, cmp, d##_##r##_##f##_##v) #define DRF_TD_I(X,e,p,o,d,r,i,f,cmp,v) \ -- 2.25.1 No virus found Checked by Hillstone Network AntiVirus
Re: [PATCH 1/6] drm/i915/gt: Ignore TLB invalidations on idle engines
On 15/06/2022 16:27, Mauro Carvalho Chehab wrote: From: Chris Wilson As an extension of the current skip TLB invalidations, check if the device is powered down prior to any engine activity, as, on such cases, all the TLBs were already invalidated, so an explicit TLB invalidation is not needed. This becomes more significant with GuC, as it can only do so when the connection to the GuC is awake. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Hmmm is this a fix or "an extension" as the commit text mentions both options?! GuC angle does not appear relevant for upstream yet so is cc: stable really required is the question. Regards, Tvrtko Signed-off-by: Chris Wilson Cc: Fei Yang Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/gem/i915_gem_pages.c | 10 + drivers/gpu/drm/i915/gt/intel_gt.c| 26 +-- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 3 +++ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c index 97c820eee115..6835279943df 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c @@ -6,14 +6,15 @@ #include +#include "gt/intel_gt.h" +#include "gt/intel_gt_pm.h" + #include "i915_drv.h" #include "i915_gem_object.h" #include "i915_scatterlist.h" #include "i915_gem_lmem.h" #include "i915_gem_mman.h" -#include "gt/intel_gt.h" - void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj, struct sg_table *pages, unsigned int sg_page_sizes) @@ -217,10 +218,11 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj) if (test_and_clear_bit(I915_BO_WAS_BOUND_BIT, &obj->flags)) { struct drm_i915_private *i915 = to_i915(obj->base.dev); + struct intel_gt *gt = to_gt(i915); intel_wakeref_t wakeref; - with_intel_runtime_pm_if_active(&i915->runtime_pm, wakeref) - intel_gt_invalidate_tlbs(to_gt(i915)); + with_intel_gt_pm_if_awake(gt, wakeref) + intel_gt_invalidate_tlbs(gt); } return pages; diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index f33290358c51..d5ed6a6ac67c 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -11,6 +11,7 @@ #include "i915_drv.h" #include "intel_context.h" +#include "intel_engine_pm.h" #include "intel_engine_regs.h" #include "intel_gt.h" #include "intel_gt_buffer_pool.h" @@ -1216,6 +1217,7 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) struct drm_i915_private *i915 = gt->i915; struct intel_uncore *uncore = gt->uncore; struct intel_engine_cs *engine; + intel_engine_mask_t awake, tmp; enum intel_engine_id id; const i915_reg_t *regs; unsigned int num = 0; @@ -1239,12 +1241,27 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) GEM_TRACE("\n"); - assert_rpm_wakelock_held(&i915->runtime_pm); - mutex_lock(>->tlb_invalidate_lock); intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL); + awake = 0; for_each_engine(engine, gt, id) { + struct reg_and_bit rb; + + if (!intel_engine_pm_is_awake(engine)) + continue; + + rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num); + if (!i915_mmio_reg_offset(rb.reg)) + continue; + + intel_uncore_write_fw(uncore, rb.reg, rb.bit); + awake |= engine->mask; + } + + for_each_engine_masked(engine, gt, awake, tmp) { + struct reg_and_bit rb; + /* * HW architecture suggest typical invalidation time at 40us, * with pessimistic cases up to 100us and a recommendation to @@ -1252,13 +1269,8 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) */ const unsigned int timeout_us = 100; const unsigned int timeout_ms = 4; - struct reg_and_bit rb; rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num); - if (!i915_mmio_reg_offset(rb.reg)) - continue; - - intel_uncore_write_fw(uncore, rb.reg, rb.bit); if (__intel_wait_for_register_fw(uncore, rb.reg, rb.bit, 0, timeout_us, timeout_ms, diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index bc898df7a48c..a334787a4939 100644 --- a
[PATCH] drm/connector: Remove usage of the deprecated ida_simple_xxx API
Use ida_alloc_xxx()/ida_free() instead of ida_simple_get()/ida_simple_remove(). The latter is deprecated and more verbose. Signed-off-by: Bo Liu --- drivers/gpu/drm/drm_connector.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 1c48d162c77e..e3484b115ae6 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -250,7 +250,7 @@ int drm_connector_init(struct drm_device *dev, connector->funcs = funcs; /* connector index is used with 32bit bitmasks */ - ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL); + ret = ida_alloc_max(&config->connector_ida, 31, GFP_KERNEL); if (ret < 0) { DRM_DEBUG_KMS("Failed to allocate %s connector index: %d\n", drm_connector_enum_list[connector_type].name, @@ -262,7 +262,7 @@ int drm_connector_init(struct drm_device *dev, connector->connector_type = connector_type; connector->connector_type_id = - ida_simple_get(connector_ida, 1, 0, GFP_KERNEL); + ida_alloc_min(connector_ida, 1, GFP_KERNEL); if (connector->connector_type_id < 0) { ret = connector->connector_type_id; goto out_put_id; @@ -322,10 +322,10 @@ int drm_connector_init(struct drm_device *dev, connector->debugfs_entry = NULL; out_put_type_id: if (ret) - ida_simple_remove(connector_ida, connector->connector_type_id); + ida_free(connector_ida, connector->connector_type_id); out_put_id: if (ret) - ida_simple_remove(&config->connector_ida, connector->index); + ida_free(&config->connector_ida, connector->index); out_put: if (ret) drm_mode_object_unregister(dev, &connector->base); @@ -479,10 +479,10 @@ void drm_connector_cleanup(struct drm_connector *connector) list_for_each_entry_safe(mode, t, &connector->modes, head) drm_mode_remove(connector, mode); - ida_simple_remove(&drm_connector_enum_list[connector->connector_type].ida, + ida_free(&drm_connector_enum_list[connector->connector_type].ida, connector->connector_type_id); - ida_simple_remove(&dev->mode_config.connector_ida, + ida_free(&dev->mode_config.connector_ida, connector->index); kfree(connector->display_info.bus_formats); -- 2.27.0
[PATCH] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. This driver calls drm_detect_hdmi_monitor() to receive the same information, which is less efficient. Avoid calling drm_detect_hdmi_monitor() and use drm_display_info.is_hdmi instead. Signed-off-by: hongao --- drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 7655142a4651..17e9f5efbcfc 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -893,7 +893,7 @@ static int hdmi_get_modes(struct drm_connector *connector) if (!edid) return -ENODEV; - hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); + hdata->dvi_mode = !connector->display_info.is_hdmi; DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n", (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), edid->width_cm, edid->height_cm); -- 2.20.1
[PULL] drm-misc-fixes
Hi Dave, Daniel, Here's this week drm-misc-fixes PR Maxime drm-misc-fixes-2022-06-16: Two fixes for TTM, one for a NULL pointer dereference and one to make sure the buffer is pinned prior to a bulk move, and a fix for a spurious compiler warning. The following changes since commit 477277c7fd43d48ae68cbdcaa7c0f82024a87421: drm/ast: Support multiple outputs (2022-06-09 10:27:49 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2022-06-16 for you to fetch changes up to 0f9cd1ea10d307cad221d6693b648a8956e812b0: drm/ttm: fix bulk move handling v2 (2022-06-14 11:15:19 +0200) Two fixes for TTM, one for a NULL pointer dereference and one to make sure the buffer is pinned prior to a bulk move, and a fix for a spurious compiler warning. Christian König (2): drm/ttm: fix missing NULL check in ttm_device_swapout drm/ttm: fix bulk move handling v2 GONG, Ruiqi (1): drm/atomic: fix warning of unused variable drivers/gpu/drm/ttm/ttm_bo.c | 22 ++-- drivers/gpu/drm/ttm/ttm_device.c | 6 - drivers/gpu/drm/ttm/ttm_resource.c | 52 ++ include/drm/drm_atomic.h | 1 + include/drm/ttm/ttm_resource.h | 8 +++--- 5 files changed, 60 insertions(+), 29 deletions(-) signature.asc Description: PGP signature
Re: [PATCH 3/6] drm/i915/gt: Skip TLB invalidations once wedged
On 15/06/2022 16:27, Mauro Carvalho Chehab wrote: From: Chris Wilson Skip all further TLB invalidations once the device is wedged and had been reset, as, on such cases, it can no longer process instructions on the GPU and the user no longer has access to the TLB's in each engine. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Are there any real problems fixed or it's just a logical thing to do? Not much harm tagging it as fixes in terms of process since it is tiny but, again, wanting a clear picture. Regards, Tvrtko Signed-off-by: Chris Wilson Cc: Fei Yang Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/gt/intel_gt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 61b7ec5118f9..fb4fd5273ca4 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -1226,6 +1226,9 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt) if (I915_SELFTEST_ONLY(gt->awake == -ENODEV)) return; + if (intel_gt_is_wedged(gt)) + return; + if (GRAPHICS_VER(i915) == 12) { regs = gen12_regs; num = ARRAY_SIZE(gen12_regs);
Re: [PATCH 4/6] drm/i915/gt: Only invalidate TLBs exposed to user manipulation
On 15/06/2022 16:27, Mauro Carvalho Chehab wrote: From: Chris Wilson Don't flush TLBs when the buffer is only used in the GGTT under full control of the kernel, as there's no risk of of concurrent access and stale access from prefetch. We only need to invalidate the TLB if they are accessible by the user. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Same question as against the other patch - fix or optimisation? Regards, Tvrtko Signed-off-by: Chris Wilson Cc: Fei Yang Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/i915_vma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 0bffb70b3c5f..7989986161e8 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -537,7 +537,8 @@ int i915_vma_bind(struct i915_vma *vma, bind_flags); } - set_bit(I915_BO_WAS_BOUND_BIT, &vma->obj->flags); + if (bind_flags & I915_VMA_LOCAL_BIND) + set_bit(I915_BO_WAS_BOUND_BIT, &vma->obj->flags); atomic_or(bind_flags, &vma->flags); return 0;
Re: [PATCH 5/6] drm/i915/gt: Serialize GRDOM access between multiple engine resets
On 15/06/2022 16:27, Mauro Carvalho Chehab wrote: From: Chris Wilson Don't allow two engines to be reset in parallel, as they would both try to select a reset bit (and send requests to common registers) and wait on that register, at the same time. Serialize control of the reset requests/acks using the uncore->lock, which will also ensure that no other GT state changes at the same time as the actual reset. Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store") Ah okay I get it, the fixes tag was applied indiscriminately to the whole series. :) It definitely does not belong in this patch. Otherwise LGTM: Reviewed-by: Tvrtko Ursulin Regards, Tvrtko Reported-by: Mika Kuoppala Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Andi Shyti Cc: sta...@vger.kernel.org Acked-by: Thomas Hellström Signed-off-by: Mauro Carvalho Chehab --- See [PATCH 0/6] at: https://lore.kernel.org/all/cover.1655306128.git.mche...@kernel.org/ drivers/gpu/drm/i915/gt/intel_reset.c | 37 --- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index a5338c3fde7a..c68d36fb5bbd 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -300,9 +300,9 @@ static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask) return err; } -static int gen6_reset_engines(struct intel_gt *gt, - intel_engine_mask_t engine_mask, - unsigned int retry) +static int __gen6_reset_engines(struct intel_gt *gt, + intel_engine_mask_t engine_mask, + unsigned int retry) { struct intel_engine_cs *engine; u32 hw_mask; @@ -321,6 +321,20 @@ static int gen6_reset_engines(struct intel_gt *gt, return gen6_hw_domain_reset(gt, hw_mask); } +static int gen6_reset_engines(struct intel_gt *gt, + intel_engine_mask_t engine_mask, + unsigned int retry) +{ + unsigned long flags; + int ret; + + spin_lock_irqsave(>->uncore->lock, flags); + ret = __gen6_reset_engines(gt, engine_mask, retry); + spin_unlock_irqrestore(>->uncore->lock, flags); + + return ret; +} + static struct intel_engine_cs *find_sfc_paired_vecs_engine(struct intel_engine_cs *engine) { int vecs_id; @@ -487,9 +501,9 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine) rmw_clear_fw(uncore, sfc_lock.lock_reg, sfc_lock.lock_bit); } -static int gen11_reset_engines(struct intel_gt *gt, - intel_engine_mask_t engine_mask, - unsigned int retry) +static int __gen11_reset_engines(struct intel_gt *gt, +intel_engine_mask_t engine_mask, +unsigned int retry) { struct intel_engine_cs *engine; intel_engine_mask_t tmp; @@ -583,8 +597,11 @@ static int gen8_reset_engines(struct intel_gt *gt, struct intel_engine_cs *engine; const bool reset_non_ready = retry >= 1; intel_engine_mask_t tmp; + unsigned long flags; int ret; + spin_lock_irqsave(>->uncore->lock, flags); + for_each_engine_masked(engine, gt, engine_mask, tmp) { ret = gen8_engine_reset_prepare(engine); if (ret && !reset_non_ready) @@ -612,17 +629,19 @@ static int gen8_reset_engines(struct intel_gt *gt, * This is best effort, so ignore any error from the initial reset. */ if (IS_DG2(gt->i915) && engine_mask == ALL_ENGINES) - gen11_reset_engines(gt, gt->info.engine_mask, 0); + __gen11_reset_engines(gt, gt->info.engine_mask, 0); if (GRAPHICS_VER(gt->i915) >= 11) - ret = gen11_reset_engines(gt, engine_mask, retry); + ret = __gen11_reset_engines(gt, engine_mask, retry); else - ret = gen6_reset_engines(gt, engine_mask, retry); + ret = __gen6_reset_engines(gt, engine_mask, retry); skip_reset: for_each_engine_masked(engine, gt, engine_mask, tmp) gen8_engine_reset_cancel(engine); + spin_unlock_irqrestore(>->uncore->lock, flags); + return ret; }
Re: [PATCH v4 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support
Quoting Prashant Malani (2022-06-15 10:20:20) > > .../display/bridge/analogix,anx7625.yaml | 64 +++ > 1 file changed, 64 insertions(+) Can this file get a link to the product brief[1]? It helps to quickly find the block diagram. > > diff --git > a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > index 35a48515836e..bc6f7644db31 100644 > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > @@ -105,6 +105,34 @@ properties: >- port@0 >- port@1 > > + switches: > +type: object > +description: Set of switches controlling DisplayPort traffic on > + outgoing RX/TX lanes to Type C ports. > +additionalProperties: false > + > +properties: > + '#address-cells': > +const: 1 > + > + '#size-cells': > +const: 0 > + > +patternProperties: > + '^switch@[01]$': > +$ref: /schemas/usb/typec-switch.yaml# > +unevaluatedProperties: false > + > +properties: > + reg: > +maxItems: 1 > + > +required: > + - reg > + > +required: > + - switch@0 > + > required: >- compatible >- reg > @@ -167,5 +195,41 @@ examples: > }; > }; > }; > +switches { Is "switches" a bus? > +#address-cells = <1>; > +#size-cells = <0>; > +switch@0 { > +compatible = "typec-switch"; Is this compatible matched against a driver that's populated on this "switches" bus? > +reg = <0>; > +mode-switch; > + > +ports { > +#address-cells = <1>; > +#size-cells = <0>; > +port@0 { > +reg = <0>; > +anx_typec0: endpoint { > +remote-endpoint = <&typec_port0>; > +}; > +}; > +}; I was expecting to see these simply be more ports in the existing graph binding of this device, and then have the 'mode-switch' or 'orientation-switch' properties be at the same level as the compatible string "analogix,anx7625". Here's the reasoning, based on looking at the product brief and the existing binding/implementation. Looking at the only existing implementation of this binding upstream in mt8183-kukui-jacuzzi.dtsi it looks like one of these typec ports is actually the same physically as the 'anx7625_out' endpoint (reg address of 1) that is already defined in the binding. It seems that MIPI DSI/DPI comes in and is output through 2 lanes, SSRX2 and SSTX2 according to the product brief[1], and that is connected to some eDP panel ("auo,b116xw03"). Presumably that is the same as anx_typec1 in this patch? I suspect the USB3.1 input is not connected on this board, and thus the crosspoint switch is never used, nor the SSRX1/SSTX1 pins. The existing binding defines the MIPI DSI/DPI input as port0 and two of the four lanes of output that is probably by default connected to the "DisplayPort Transmitter" as port1 because that's how the crosspoint switch comes out of reset. That leaves the USB3.1 input possibly needing a port in the ports binding, and the other two lanes of output needing a port in the ports binding to describe their connection to the downstream device. And finally information about if the crosspoint switch needs to be registered with the typec framework to do typec things, which can be achieved by the presence of the 'mode-switch' property. On a board like kukui-jacuzzi these new properties and ports wouldn't be specified, because what is there is already sufficient. If this chip is connected to a usb-c-connector then I'd expect to see a connection from the output ports in the graph binding to the connector node's ports. There aren't any ports in the usb-c-connector binding though from what I see. I believe there's also one more use case here where USB3.1 or MIPI DSI/DPI is connected on the input side and this device is used to steer USB3.1 or DP through the crosspoint switch to either of the two output pairs. This last scenario means that we have to describe both output pairs, SSRX1/SSTX1 and SSRX2/SSTX2, as different ports in the binding so they can be connected to different usb-c-connectors if the hardware engineer wired the output pins that way. TL;DR: Can we add 'mode-switch' as an optional property and two more ports at address 2 and 3 for the USB3.1 input and the SSRX1/SSTX1 pair respectively to the existing graph part of this binding? > +}; > +switch@1 { > +compatible = "typec-switch"; > +reg = <1>; > +
Re: [PATCH v3 07/14] drm/msm/hdmi: enable core-vcc/core-vdda-supply for 8996 platform
Quoting Dmitry Baryshkov (2022-06-09 05:23:43) > DB820c makes use of core-vcc-supply and core-vdda-supply, however the > driver code doesn't support these regulators. Enable them for HDMI on > 8996 platform. > > Fixes: 0afbe59edd3f ("drm/msm/hdmi: Add basic HDMI support for msm8996") > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v2 0/6] drm/sun4i: HDMI PHY cleanup/refactoring
On Tue, 14 Jun 2022 23:55:37 -0500, Samuel Holland wrote: > This series prepares the sun8i HDMI PHY driver for supporting the new > custom PHY in the Allwinner D1 SoC. No functional change intended here. > > This series was tested on D1, H3, and H6. > > Changes in v2: > - Move error handling inside variant checks in probe function > > [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
[PULL] drm-intel-fixes
Hi Dave & Daniel - drm-intel-fixes-2022-06-16: drm/i915 fixes for v5.19-rc3: - Fix page fault on error state read - Fix memory leaks in per-gt sysfs - Fix multiple fence handling - Remove accidental static from a local variable BR, Jani. The following changes since commit b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3: Linux 5.19-rc2 (2022-06-12 16:11:37 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2022-06-16 for you to fetch changes up to 2636e008112465ca54559ac4898da5a2515e118a: drm/i915/uc: remove accidental static from a local variable (2022-06-13 13:53:35 +0300) drm/i915 fixes for v5.19-rc3: - Fix page fault on error state read - Fix memory leaks in per-gt sysfs - Fix multiple fence handling - Remove accidental static from a local variable Alan Previn (1): drm/i915/reset: Fix error_state_read ptr + offset use Ashutosh Dixit (1): drm/i915/gt: Fix memory leaks in per-gt sysfs Jani Nikula (1): drm/i915/uc: remove accidental static from a local variable Nirmoy Das (1): drm/i915: Individualize fences before adding to dma_resv obj drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 3 +- drivers/gpu/drm/i915/gt/intel_gt.c | 1 + drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 29 +++- drivers/gpu/drm/i915/gt/intel_gt_sysfs.h | 6 +--- drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 ++ drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 2 +- drivers/gpu/drm/i915/i915_sysfs.c | 17 +++-- drivers/gpu/drm/i915/i915_vma.c| 48 +++--- 8 files changed, 62 insertions(+), 47 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm/sun4i: Fix crash during suspend after component bind failure
Hi, On Wed, Jun 15, 2022 at 12:42:53AM -0500, Samuel Holland wrote: > If the component driver fails to bind, or is unbound, the driver data > for the top-level platform device points to a freed drm_device. If the > system is then suspended, the driver passes this dangling pointer to > drm_mode_config_helper_suspend(), which crashes. > > Fix this by only setting the driver data while the platform driver holds > a reference to the drm_device. > > Fixes: 624b4b48d9d8 ("drm: sun4i: Add support for suspending the display > driver") > Signed-off-by: Samuel Holland Yeah, it's far from the only issue regarding structure lifetimes in the driver. We should convert as much as possible to the DRM-managed functions to fix those. Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/sun4i: dw-hdmi: Fix ddc-en GPIO consumer conflict
On Tue, 14 Jun 2022 02:31:00 -0500, Samuel Holland wrote: > commit 6de79dd3a920 ("drm/bridge: display-connector: add ddc-en gpio > support") added a consumer for this GPIO in the HDMI connector device. > This new consumer conflicts with the pre-existing GPIO consumer in the > sun8i HDMI controller driver, which prevents the driver from probing: > > [4.983358] display-connector connector: GPIO lookup for consumer ddc-en > [4.983364] display-connector connector: using device tree for GPIO > lookup > [4.983392] gpio-226 (ddc-en): gpiod_request: status -16 > [4.983399] sun8i-dw-hdmi 600.hdmi: Couldn't get ddc-en gpio > [4.983618] sun4i-drm display-engine: failed to bind 600.hdmi (ops > sun8i_dw_hdmi_ops [sun8i_drm_hdmi]): -16 > [4.984082] sun4i-drm display-engine: Couldn't bind all pipelines > components > [4.984171] sun4i-drm display-engine: adev bind failed: -16 > [4.984179] sun8i-dw-hdmi: probe of 600.hdmi failed with error -16 > > [...] Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
Re: [PATCH] drm/sun4i: Fix crash during suspend after component bind failure
On Wed, 15 Jun 2022 00:42:53 -0500, Samuel Holland wrote: > If the component driver fails to bind, or is unbound, the driver data > for the top-level platform device points to a freed drm_device. If the > system is then suspended, the driver passes this dangling pointer to > drm_mode_config_helper_suspend(), which crashes. > > Fix this by only setting the driver data while the platform driver holds > a reference to the drm_device. > > [...] Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
[PATCH 0/3] drm/msm/hdmi: move resource allocation to probe function
As pointed several times in the discussions, start moving resource allocation from component bind to the probe function. This simplifies boot process, as the component will not be registered until all resources (clocks, regulators, IRQ, etc.) are not registered. Dmitry Baryshkov (3): drm/msm/hdmi: use devres helper for runtime PM management drm/msm/hdmi: drop constant resource names from platform config drm/msm/hdmi: move resource allocation to probe function drivers/gpu/drm/msm/hdmi/hdmi.c | 298 ++-- drivers/gpu/drm/msm/hdmi/hdmi.h | 3 - 2 files changed, 134 insertions(+), 167 deletions(-) -- 2.35.1
[PATCH 2/3] drm/msm/hdmi: drop constant resource names from platform config
All MSM HDMI devices use "core_physical" and "qfprom_physical" names for register areas. Drop them from the platform config. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/hdmi/hdmi.c | 9 +++-- drivers/gpu/drm/msm/hdmi/hdmi.h | 3 --- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 9ff9a68b201b..8dfe5690366b 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -133,7 +133,7 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev) hdmi->config = config; spin_lock_init(&hdmi->reg_lock); - hdmi->mmio = msm_ioremap(pdev, config->mmio_name); + hdmi->mmio = msm_ioremap(pdev, "core_physical"); if (IS_ERR(hdmi->mmio)) { ret = PTR_ERR(hdmi->mmio); goto fail; @@ -141,14 +141,14 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev) /* HDCP needs physical address of hdmi register */ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, - config->mmio_name); + "core_physical"); if (!res) { ret = -EINVAL; goto fail; } hdmi->mmio_phy_addr = res->start; - hdmi->qfprom_mmio = msm_ioremap(pdev, config->qfprom_mmio_name); + hdmi->qfprom_mmio = msm_ioremap(pdev, "qfprom_physical"); if (IS_ERR(hdmi->qfprom_mmio)) { DRM_DEV_INFO(&pdev->dev, "can't find qfprom resource\n"); hdmi->qfprom_mmio = NULL; @@ -510,9 +510,6 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data) return -ENXIO; } - hdmi_cfg->mmio_name = "core_physical"; - hdmi_cfg->qfprom_mmio_name = "qfprom_physical"; - dev->platform_data = hdmi_cfg; hdmi = msm_hdmi_init(to_platform_device(dev)); diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h index a6c88d157bc3..7263bcbf4d06 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.h +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h @@ -84,9 +84,6 @@ struct hdmi { /* platform config data (ie. from DT, or pdata) */ struct hdmi_platform_config { - const char *mmio_name; - const char *qfprom_mmio_name; - /* regulators that need to be on for hpd: */ const char **hpd_reg_names; int hpd_reg_cnt; -- 2.35.1
[PATCH 1/3] drm/msm/hdmi: use devres helper for runtime PM management
Use devm_pm_runtime_enable() to enable runtime PM. This way its effect will be reverted on device unbind/destruction. Fixes: 6ed9ed484d04 ("drm/msm/hdmi: Set up runtime PM for HDMI") Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 6acc17e0efc1..9ff9a68b201b 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -247,7 +247,7 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev) if (hdmi->hpd_gpiod) gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD"); - pm_runtime_enable(&pdev->dev); + devm_pm_runtime_enable(&pdev->dev); hdmi->workq = alloc_ordered_workqueue("msm_hdmi", 0); -- 2.35.1
[PATCH 3/3] drm/msm/hdmi: move resource allocation to probe function
Rather than having all resource allocation happen in the _bind function (resulting in possible EPROBE_DEFER returns and component bind/unbind cycles) allocate and check all resources in _probe function. While we are at it, use platform_get_irq() to get the IRQ rather than going through the irq_of_parse_and_map(). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/hdmi/hdmi.c | 295 +++- 1 file changed, 134 insertions(+), 161 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 8dfe5690366b..429abd622c81 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -75,8 +75,6 @@ static void msm_hdmi_destroy(struct hdmi *hdmi) if (hdmi->i2c) msm_hdmi_i2c_destroy(hdmi->i2c); - - platform_set_drvdata(hdmi->pdev, NULL); } static int msm_hdmi_get_phy(struct hdmi *hdmi) @@ -116,138 +114,10 @@ static int msm_hdmi_get_phy(struct hdmi *hdmi) * we are to EPROBE_DEFER we want to do it here, rather than later * at modeset_init() time */ -static struct hdmi *msm_hdmi_init(struct platform_device *pdev) +static int msm_hdmi_init(struct hdmi *hdmi) { - struct hdmi_platform_config *config = pdev->dev.platform_data; - struct hdmi *hdmi = NULL; - struct resource *res; - int i, ret; - - hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL); - if (!hdmi) { - ret = -ENOMEM; - goto fail; - } - - hdmi->pdev = pdev; - hdmi->config = config; - spin_lock_init(&hdmi->reg_lock); - - hdmi->mmio = msm_ioremap(pdev, "core_physical"); - if (IS_ERR(hdmi->mmio)) { - ret = PTR_ERR(hdmi->mmio); - goto fail; - } - - /* HDCP needs physical address of hdmi register */ - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, - "core_physical"); - if (!res) { - ret = -EINVAL; - goto fail; - } - hdmi->mmio_phy_addr = res->start; - - hdmi->qfprom_mmio = msm_ioremap(pdev, "qfprom_physical"); - if (IS_ERR(hdmi->qfprom_mmio)) { - DRM_DEV_INFO(&pdev->dev, "can't find qfprom resource\n"); - hdmi->qfprom_mmio = NULL; - } - - hdmi->hpd_regs = devm_kcalloc(&pdev->dev, - config->hpd_reg_cnt, - sizeof(hdmi->hpd_regs[0]), - GFP_KERNEL); - if (!hdmi->hpd_regs) { - ret = -ENOMEM; - goto fail; - } - for (i = 0; i < config->hpd_reg_cnt; i++) - hdmi->hpd_regs[i].supply = config->hpd_reg_names[i]; - - ret = devm_regulator_bulk_get(&pdev->dev, config->hpd_reg_cnt, hdmi->hpd_regs); - if (ret) { - DRM_DEV_ERROR(&pdev->dev, "failed to get hpd regulator: %d\n", ret); - goto fail; - } - - hdmi->pwr_regs = devm_kcalloc(&pdev->dev, - config->pwr_reg_cnt, - sizeof(hdmi->pwr_regs[0]), - GFP_KERNEL); - if (!hdmi->pwr_regs) { - ret = -ENOMEM; - goto fail; - } - - for (i = 0; i < config->pwr_reg_cnt; i++) - hdmi->pwr_regs[i].supply = config->pwr_reg_names[i]; - - ret = devm_regulator_bulk_get(&pdev->dev, config->pwr_reg_cnt, hdmi->pwr_regs); - if (ret) { - DRM_DEV_ERROR(&pdev->dev, "failed to get pwr regulator: %d\n", ret); - goto fail; - } - - hdmi->hpd_clks = devm_kcalloc(&pdev->dev, - config->hpd_clk_cnt, - sizeof(hdmi->hpd_clks[0]), - GFP_KERNEL); - if (!hdmi->hpd_clks) { - ret = -ENOMEM; - goto fail; - } - for (i = 0; i < config->hpd_clk_cnt; i++) { - struct clk *clk; - - clk = msm_clk_get(pdev, config->hpd_clk_names[i]); - if (IS_ERR(clk)) { - ret = PTR_ERR(clk); - DRM_DEV_ERROR(&pdev->dev, "failed to get hpd clk: %s (%d)\n", - config->hpd_clk_names[i], ret); - goto fail; - } - - hdmi->hpd_clks[i] = clk; - } - - hdmi->pwr_clks = devm_kcalloc(&pdev->dev, - config->pwr_clk_cnt, - sizeof(hdmi->pwr_clks[0]), - GFP_KERNEL); - if (!hdmi->pwr_clks) { - ret = -ENOMEM; - goto fail; - } - for (i = 0; i < config->pwr_clk_cnt; i++) { - struct clk *clk; - - clk = msm_clk_get(pdev, config->pwr_clk_names[i]); - if (IS_E
Re: [PATCH v2 4/5] drm/msm: move KMS aspace init to the separate helper
On 16/06/2022 05:34, Stephen Boyd wrote: Quoting Dmitry Baryshkov (2022-05-04 17:16:04) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index a37a3bbc04d9..98ae0036ab57 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -262,6 +263,46 @@ static int msm_drm_uninit(struct device *dev) #include +struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev) +{ [...] + } + + aspace = msm_gem_address_space_create(mmu, "mdp_kms", + 0x1000, 0x1 - 0x1000); + if (IS_ERR(aspace)) { + mmu->funcs->destroy(mmu); + return aspace; + } + + return aspace; This can be 'return aspace' one time instead of two. Yes. I was just always in favour of explicit error returns rather than falling through. I'll send v2. +} + bool msm_use_mmu(struct drm_device *dev) { struct msm_drm_private *priv = dev->dev_private; -- With best wishes Dmitry
[PATCH v3 0/5] drm/msm: fixes for KMS iommu handling
This series started from the applied and then reverted [2] patch by Robin Murphy [1]. After the MDSS rework [3] has landed it is now possible to reapply the extended version of the original patch. While we are at it, also rework the IOMMU init code for DPU and MDP5 drivers. For MDP5 this moves iommu_domain_alloc() call and removes struct mdp5_cfg_platform remains. For DPU this allows specifying the iommus = <...> either in the DPU device (like all DPU devices do) or in the MDSS device (like MDP5 devices do). Changes since v2: - Merge two return statements in msm_kms_init_aspace() (requested by Stephen) Changes since v1: - Move aspace init to common helper - Use device_iommu_mapped() rather than semi-internal dev_iommu_fwspec_get() (suggested by Robin Murphy) [1] https://patchwork.freedesktop.org/patch/480707/ [2] https://patchwork.freedesktop.org/patch/482453/ [3] https://patchwork.freedesktop.org/series/98525/ Dmitry Baryshkov (5): drm/msm/dpu: check both DPU and MDSS devices for the IOMMU drm/msm/mdp5: move iommu_domain_alloc() call close to its usage drm/msm: Stop using iommu_present() drm/msm: move KMS aspace init to the separate helper drm/msm: switch msm_kms_init_aspace() to use device_iommu_mapped() drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 24 ++-- drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h | 6 --- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 31 +++ drivers/gpu/drm/msm/msm_drv.c| 49 +++- drivers/gpu/drm/msm/msm_drv.h| 1 + 6 files changed, 57 insertions(+), 70 deletions(-) -- 2.35.1
[PATCH v3 2/5] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage
Move iommu_domain_alloc() in front of adress space/IOMMU initialization. This allows us to drop final bits of struct mdp5_cfg_platform which remained from the pre-DT days. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h | 6 -- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 -- 3 files changed, 4 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c index 1bf9ff5dbabc..714effb967ff 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c @@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = { { .revision = 3, .config = { .hw = &sdm630_config } }, }; -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev); - const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler *cfg_handler) { return cfg_handler->config.hw; @@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms, uint32_t major, uint32_t minor) { struct drm_device *dev = mdp5_kms->dev; - struct platform_device *pdev = to_platform_device(dev->dev); struct mdp5_cfg_handler *cfg_handler; const struct mdp5_cfg_handler *cfg_handlers; - struct mdp5_cfg_platform *pconfig; int i, ret = 0, num_handlers; cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL); @@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms, cfg_handler->revision = minor; cfg_handler->config.hw = mdp5_cfg; - pconfig = mdp5_get_config(pdev); - memcpy(&cfg_handler->config.platform, pconfig, sizeof(*pconfig)); - DBG("MDP5: %s hw config selected", mdp5_cfg->name); return cfg_handler; @@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms, return ERR_PTR(ret); } - -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev) -{ - static struct mdp5_cfg_platform config = {}; - - config.iommu = iommu_domain_alloc(&platform_bus_type); - - return &config; -} diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h index 6b03d7899309..c2502cc33864 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h @@ -104,14 +104,8 @@ struct mdp5_cfg_hw { uint32_t max_clk; }; -/* platform config data (ie. from DT, or pdata) */ -struct mdp5_cfg_platform { - struct iommu_domain *iommu; -}; - struct mdp5_cfg { const struct mdp5_cfg_hw *hw; - struct mdp5_cfg_platform platform; }; struct mdp5_kms; diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 3d5621a68f85..a69e23f10d91 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev) struct msm_gem_address_space *aspace; int irq, i, ret; struct device *iommu_dev; + struct iommu_domain *iommu; ret = mdp5_init(to_platform_device(dev->dev), dev); @@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev) } mdelay(16); - if (config->platform.iommu) { + iommu = iommu_domain_alloc(&platform_bus_type); + if (iommu) { struct msm_mmu *mmu; iommu_dev = &pdev->dev; if (!dev_iommu_fwspec_get(iommu_dev)) iommu_dev = iommu_dev->parent; - mmu = msm_iommu_new(iommu_dev, config->platform.iommu); + mmu = msm_iommu_new(iommu_dev, iommu); aspace = msm_gem_address_space_create(mmu, "mdp5", 0x1000, 0x1 - 0x1000); -- 2.35.1
[PATCH v3 4/5] drm/msm: move KMS aspace init to the separate helper
MDP5 and DPU drivers have the same piece of code now to initialize IOMMU and GEM address space. Move it to the msm_drv.c Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 32 ++- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 33 drivers/gpu/drm/msm/msm_drv.c| 39 drivers/gpu/drm/msm/msm_drv.h| 1 + 4 files changed, 49 insertions(+), 56 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index e8bc6d5f6ac1..8cbe56694892 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -997,40 +997,14 @@ static void _dpu_kms_mmu_destroy(struct dpu_kms *dpu_kms) static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) { - struct iommu_domain *domain; struct msm_gem_address_space *aspace; - struct msm_mmu *mmu; - struct device *dpu_dev = dpu_kms->dev->dev; - struct device *mdss_dev = dpu_dev->parent; - struct device *iommu_dev; - - domain = iommu_domain_alloc(&platform_bus_type); - if (!domain) - return 0; - - /* -* IOMMUs can be a part of MDSS device tree binding, or the -* MDP/DPU device. -*/ - if (dev_iommu_fwspec_get(dpu_dev)) - iommu_dev = dpu_dev; - else - iommu_dev = mdss_dev; - - mmu = msm_iommu_new(iommu_dev, domain); - if (IS_ERR(mmu)) { - iommu_domain_free(domain); - return PTR_ERR(mmu); - } - aspace = msm_gem_address_space_create(mmu, "dpu1", - 0x1000, 0x1 - 0x1000); - if (IS_ERR(aspace)) { - mmu->funcs->destroy(mmu); + aspace = msm_kms_init_aspace(dpu_kms->dev); + if (IS_ERR(aspace)) return PTR_ERR(aspace); - } dpu_kms->base.aspace = aspace; + return 0; } diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index a69e23f10d91..d2a48caf9d27 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -557,8 +557,6 @@ static int mdp5_kms_init(struct drm_device *dev) struct msm_kms *kms; struct msm_gem_address_space *aspace; int irq, i, ret; - struct device *iommu_dev; - struct iommu_domain *iommu; ret = mdp5_init(to_platform_device(dev->dev), dev); @@ -602,33 +600,14 @@ static int mdp5_kms_init(struct drm_device *dev) } mdelay(16); - iommu = iommu_domain_alloc(&platform_bus_type); - if (iommu) { - struct msm_mmu *mmu; - - iommu_dev = &pdev->dev; - if (!dev_iommu_fwspec_get(iommu_dev)) - iommu_dev = iommu_dev->parent; - - mmu = msm_iommu_new(iommu_dev, iommu); - - aspace = msm_gem_address_space_create(mmu, "mdp5", - 0x1000, 0x1 - 0x1000); - - if (IS_ERR(aspace)) { - if (!IS_ERR(mmu)) - mmu->funcs->destroy(mmu); - ret = PTR_ERR(aspace); - goto fail; - } - - kms->aspace = aspace; - } else { - DRM_DEV_INFO(&pdev->dev, -"no iommu, fallback to phys contig buffers for scanout\n"); - aspace = NULL; + aspace = msm_kms_init_aspace(mdp5_kms->dev); + if (IS_ERR(aspace)) { + ret = PTR_ERR(aspace); + goto fail; } + kms->aspace = aspace; + pm_runtime_put_sync(&pdev->dev); ret = modeset_init(mdp5_kms); diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index c781307464a0..258632d5c411 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -26,6 +26,7 @@ #include "msm_gem.h" #include "msm_gpu.h" #include "msm_kms.h" +#include "msm_mmu.h" #include "adreno/adreno_gpu.h" /* @@ -267,6 +268,44 @@ static int msm_drm_uninit(struct device *dev) #include +struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev) +{ + struct iommu_domain *domain; + struct msm_gem_address_space *aspace; + struct msm_mmu *mmu; + struct device *mdp_dev = dev->dev; + struct device *mdss_dev = mdp_dev->parent; + struct device *iommu_dev; + + domain = iommu_domain_alloc(&platform_bus_type); + if (!domain) { + drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n"); + return NULL; + } + + /* +* IOMMUs can be a part of MDSS device tree binding, or the +* MDP/DPU device. +*/ + if (dev_iommu_fwspec_get(mdp_dev)) + iommu_dev = mdp_dev; + else + iommu_dev = mdss_dev; + +
[PATCH v3 1/5] drm/msm/dpu: check both DPU and MDSS devices for the IOMMU
Follow the lead of MDP5 driver and check both DPU and MDSS devices for the IOMMU specifiers. Historically DPU devices had IOMMU specified in the MDSS device tree node, but as some of MDP5 devices are being converted to the supported by the DPU driver, the driver should adapt and check both devices. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index e23e2552e802..e8bc6d5f6ac1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1002,14 +1002,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) struct msm_mmu *mmu; struct device *dpu_dev = dpu_kms->dev->dev; struct device *mdss_dev = dpu_dev->parent; + struct device *iommu_dev; domain = iommu_domain_alloc(&platform_bus_type); if (!domain) return 0; - /* IOMMUs are a part of MDSS device tree binding, not the -* MDP/DPU device. */ - mmu = msm_iommu_new(mdss_dev, domain); + /* +* IOMMUs can be a part of MDSS device tree binding, or the +* MDP/DPU device. +*/ + if (dev_iommu_fwspec_get(dpu_dev)) + iommu_dev = dpu_dev; + else + iommu_dev = mdss_dev; + + mmu = msm_iommu_new(iommu_dev, domain); if (IS_ERR(mmu)) { iommu_domain_free(domain); return PTR_ERR(mmu); -- 2.35.1
[PATCH v3 5/5] drm/msm: switch msm_kms_init_aspace() to use device_iommu_mapped()
Change msm_kms_init_aspace() to use generic function device_iommu_mapped() instead of the fwnode-specific interface dev_iommu_fwspec_get(). While we are at it, stop referencing platform_bus_type directly and use the bus of the IOMMU device. Suggested-by: Robin Murphy Reviewed-by: Robin Murphy Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/msm_drv.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 258632d5c411..8e18eca82bc6 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -277,21 +277,21 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev) struct device *mdss_dev = mdp_dev->parent; struct device *iommu_dev; - domain = iommu_domain_alloc(&platform_bus_type); - if (!domain) { - drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n"); - return NULL; - } - /* * IOMMUs can be a part of MDSS device tree binding, or the * MDP/DPU device. */ - if (dev_iommu_fwspec_get(mdp_dev)) + if (device_iommu_mapped(mdp_dev)) iommu_dev = mdp_dev; else iommu_dev = mdss_dev; + domain = iommu_domain_alloc(iommu_dev->bus); + if (!domain) { + drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n"); + return NULL; + } + mmu = msm_iommu_new(iommu_dev, domain); if (IS_ERR(mmu)) { iommu_domain_free(domain); -- 2.35.1
[PATCH v3 3/5] drm/msm: Stop using iommu_present()
Even if some IOMMU has registered itself on the platform "bus", that doesn't necessarily mean it provides translation for the device we care about. Replace iommu_present() with a more appropriate check. On Qualcomm platforms the IOMMU can be specified either for the MDP/DPU device or for its parent MDSS device depending on the actual platform. Check both of them, since that is how both DPU and MDP5 drivers work. Co-developed-by: Robin Murphy Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/msm_drv.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 44485363f37a..c781307464a0 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -271,8 +271,14 @@ bool msm_use_mmu(struct drm_device *dev) { struct msm_drm_private *priv = dev->dev_private; - /* a2xx comes with its own MMU */ - return priv->is_a2xx || iommu_present(&platform_bus_type); + /* +* a2xx comes with its own MMU +* On other platforms IOMMU can be declared specified either for the +* MDP/DPU device or for its parent, MDSS device. +*/ + return priv->is_a2xx || + device_iommu_mapped(dev->dev) || + device_iommu_mapped(dev->dev->parent); } static int msm_init_vram(struct drm_device *dev) -- 2.35.1
Re: [PATCH] drm/msm: Fix fence rollover issue
On 15/06/2022 19:24, Rob Clark wrote: From: Rob Clark And while we are at it, let's start the fence counter close to the rollover point so that if issues slip in, they are more obvious. Signed-off-by: Rob Clark Should it also have Fixes: fde5de6cb461 ("drm/msm: move fence code to it's own file") Or maybe Fixes: 5f3aee4ceb5b ("drm/msm: Handle fence rollover") Otherwise: Reviewed: Dmitry Baryshkov --- drivers/gpu/drm/msm/msm_fence.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c index 3df255402a33..a35a6746c7cd 100644 --- a/drivers/gpu/drm/msm/msm_fence.c +++ b/drivers/gpu/drm/msm/msm_fence.c @@ -28,6 +28,14 @@ msm_fence_context_alloc(struct drm_device *dev, volatile uint32_t *fenceptr, fctx->fenceptr = fenceptr; spin_lock_init(&fctx->spinlock); + /* +* Start out close to the 32b fence rollover point, so we can +* catch bugs with fence comparisons. +*/ + fctx->last_fence = 0xff00; + fctx->completed_fence = fctx->last_fence; + *fctx->fenceptr = fctx->last_fence; This looks like a debugging hack. But probably it's fine to have it, as it wouldn't cause any side effects. + return fctx; } @@ -46,11 +54,12 @@ bool msm_fence_completed(struct msm_fence_context *fctx, uint32_t fence) (int32_t)(*fctx->fenceptr - fence) >= 0; } -/* called from workqueue */ +/* called from irq handler */ void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence) { spin_lock(&fctx->spinlock); - fctx->completed_fence = max(fence, fctx->completed_fence); + if (fence_after(fence, fctx->completed_fence)) + fctx->completed_fence = fence; spin_unlock(&fctx->spinlock); } -- With best wishes Dmitry
Re: [PATCH] drm/msm/gem: Drop obj lock in msm_gem_free_object()
Quoting Rob Clark (2022-06-13 13:50:32) > diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h > index d608339c1643..432032ad4aed 100644 > --- a/drivers/gpu/drm/msm/msm_gem.h > +++ b/drivers/gpu/drm/msm/msm_gem.h > @@ -229,7 +229,19 @@ msm_gem_unlock(struct drm_gem_object *obj) > static inline bool > msm_gem_is_locked(struct drm_gem_object *obj) > { > - return dma_resv_is_locked(obj->resv); > + /* > +* Destroying the object is a special case.. msm_gem_free_object() > +* calls many things that WARN_ON if the obj lock is not held. But > +* acquiring the obj lock in msm_gem_free_object() can cause a > +* locking order inversion between reservation_ww_class_mutex and > +* fs_reclaim. > +* > +* This deadlock is not actually possible, because no one should > +* be already holding the lock when msm_gem_free_object() is called. > +* Unfortunately lockdep is not aware of this detail. So when the > +* refcount drops to zero, we pretend it is already locked. > +*/ > + return dma_resv_is_locked(obj->resv) || (kref_read(&obj->refcount) == > 0); Instead of modifying this function can we push down the fact that this function is being called from the free path and skip checking this condition in that case? Or add some "_locked/free_path" wrappers that skip the lock assertion? That would make it clearer to understand while reading the code that it is locked when it is asserted to be locked, and that we don't care when we're freeing because all references to the object are gone. Here's a totally untested patch to show the idea. The comment about pretending the lock is held can be put in msm_gem_free_object() to clarify why it's OK to call the locked variants of the functions. ---8<--- diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 97d5b4d8b9b0..01f19d37bfb6 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -346,13 +346,11 @@ static void del_vma(struct msm_gem_vma *vma) * mapping. */ static void -put_iova_spaces(struct drm_gem_object *obj, bool close) +put_iova_spaces_locked(struct drm_gem_object *obj, bool close) { struct msm_gem_object *msm_obj = to_msm_bo(obj); struct msm_gem_vma *vma; - GEM_WARN_ON(!msm_gem_is_locked(obj)); - list_for_each_entry(vma, &msm_obj->vmas, list) { if (vma->aspace) { msm_gem_purge_vma(vma->aspace, vma); @@ -362,18 +360,28 @@ put_iova_spaces(struct drm_gem_object *obj, bool close) } } -/* Called with msm_obj locked */ +static void put_iova_spaces(struct drm_gem_object *obj, bool close) +{ + GEM_WARN_ON(!msm_gem_is_locked(obj)); + put_iova_spaces_locked(obj, close); +} + +/* Called with msm_obj locked or on the free path */ static void -put_iova_vmas(struct drm_gem_object *obj) +put_iova_vmas_locked(struct drm_gem_object *obj) { struct msm_gem_object *msm_obj = to_msm_bo(obj); struct msm_gem_vma *vma, *tmp; - GEM_WARN_ON(!msm_gem_is_locked(obj)); - - list_for_each_entry_safe(vma, tmp, &msm_obj->vmas, list) { + list_for_each_entry_safe(vma, tmp, &msm_obj->vmas, list) del_vma(vma); - } +} + +static void +put_iova_vmas(struct drm_gem_object *obj) +{ + GEM_WARN_ON(!msm_gem_is_locked(obj)); + put_iova_vmas_locked(obj); } static struct msm_gem_vma *get_vma_locked(struct drm_gem_object *obj, @@ -795,12 +803,10 @@ void msm_gem_evict(struct drm_gem_object *obj) update_inactive(msm_obj); } -void msm_gem_vunmap(struct drm_gem_object *obj) +static void msm_gem_vunmap_locked(struct drm_gem_object *obj) { struct msm_gem_object *msm_obj = to_msm_bo(obj); - GEM_WARN_ON(!msm_gem_is_locked(obj)); - if (!msm_obj->vaddr || GEM_WARN_ON(!is_vunmapable(msm_obj))) return; @@ -808,6 +814,12 @@ void msm_gem_vunmap(struct drm_gem_object *obj) msm_obj->vaddr = NULL; } +void msm_gem_vunmap(struct drm_gem_object *obj) +{ + GEM_WARN_ON(!msm_gem_is_locked(obj)); + msm_gem_vunmap_locked(obj); +} + void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu) { struct msm_gem_object *msm_obj = to_msm_bo(obj); @@ -1021,12 +1033,11 @@ void msm_gem_free_object(struct drm_gem_object *obj) list_del(&msm_obj->mm_list); mutex_unlock(&priv->mm_lock); - msm_gem_lock(obj); - /* object should not be on active list: */ GEM_WARN_ON(is_active(msm_obj)); - put_iova_spaces(obj, true); + put_iova_spaces_locked(obj, true); + if (obj->import_attach) { GEM_WARN_ON(msm_obj->vaddr); @@ -1036,19 +1047,13 @@ void msm_gem_free_object(struct drm_gem_object *obj) */ kvfree(msm_obj->pages); - put_iova_vmas(obj); - - /* dma_buf_detach() grabs resv lock, so we ne
Re: [PATCH] drm/msm/dpu: set preferred mode for writeback connector
On 16/06/2022 02:23, Abhinav Kumar wrote: After [1] was merged to IGT, we use either the first supported mode in the list OR the preferred mode to determine the primary plane to use for the sub-test due to the IGT API [2]. Since writeback does not set any preferred mode, this was selecting 4k as that was the first entry in the list. We use maxlinewidth to add the list of supported modes for the writeback connector which is the right thing to do, however since we do not have dual-SSPP support yet for DPU, this fails the bandwidth check in dpu_core_perf_crtc_check(). Till we have dual-SSPP support, workaround this mismatch between the list of supported modes and maxlinewidth limited modes by marking 640x480 as the preferred mode for DPU writeback because kms_writeback tests 640x480 mode only [3]. Telling that we support modes up to 4k, failing to set 4k mode and then using the preferred mode to force IGT to lower resolution sounds like a hack. As adding wide dual-SSPP support will take some time. I'd suggest dropping support for 4k modes for now and using DEFAULT_DPU_LINE_WIDTH instead (or hw_wb->caps->maxlinewidth). A comment in the source code that the check should be removed/modified once dual-SSPP is supported would be helpful. [1]: https://patchwork.freedesktop.org/patch/486441/ [2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/lib/igt_kms.c#L1562 [3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L68 Signed-off-by: Abhinav Kumar Any Fixes tags? --- drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c index 399115e4e217..104cc59d6466 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c @@ -10,9 +10,14 @@ static int dpu_wb_conn_get_modes(struct drm_connector *connector) struct drm_device *dev = connector->dev; struct msm_drm_private *priv = dev->dev_private; struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); + int count; - return drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, + count = drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, dev->mode_config.max_height); + + drm_set_preferred_mode(connector, 640, 480); + + return count; } static const struct drm_connector_funcs dpu_wb_conn_funcs = { -- With best wishes Dmitry
Re: [PATCH v2 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()
On 16/06/2022 00:22, Abhinav Kumar wrote: intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. changes in v2: - add phys->hw_intf and phys->hw_wb checks back Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Reviewed: Dmitry Baryshkov Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 36 ++--- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..3be73211d631 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) - phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm, phys->intf_idx); - - if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) - phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); - - if (!phys->hw_intf && !phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "no intf or wb block assigned at idx: %d\n", i); - return; - } - - if (phys->hw_intf && phys->hw_wb) { - DPU_ERROR_ENC(dpu_enc, - "invalid phys both intf and wb block at idx: %d\n", i); - return; - } - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,25 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(&phys->vsync_cnt, 0); atomic_set(&phys->underrun_cnt, 0); + + if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) + phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm, phys->intf_idx); + + if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); + + if (!phys->hw_intf && !phys->hw_wb) { + DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned at idx: %d\n", i); + ret = -EINVAL; + } + + if (phys->hw_intf && phys->hw_wb) { + DPU_ERROR_ENC(dpu_enc, + "invalid phys both intf and wb block at idx: %d\n", i); + ret = -EINVAL; + } } + mutex_unlock(&dpu_enc->enc_lock); return ret; -- With best wishes Dmitry
Re: [PATCH 25/64] drm/vc4: dpi: Add action to disable the clock
Hi Dave, On Tue, Jun 14, 2022 at 05:47:28PM +0100, Dave Stevenson wrote: > Hi Maxime. > > On Fri, 10 Jun 2022 at 10:30, Maxime Ripard wrote: > > > > Adding a device-managed action will make the error path easier, so let's > > create one to disable our clock. > > The DPI block has two clocks (core and pixel), and this is only > affecting one of them (the core clock). Thanks for the suggestion, I've amended the commit message. > (I'm actually puzzling over what it's wanting to do with the core > clock here as it's only enabling it rather than setting a rate. I > think it may be redundant). Could it be that it a "bus" clock that we need it to access the registers? Maxime signature.asc Description: PGP signature
Re: [Linaro-mm-sig] Re: [PATCH] drivers: tty: serial: Add missing of_node_put() in serial-tegra.c
On Thu, Jun 16, 2022 at 06:23:26AM +1000, Dave Airlie wrote: > On Wed, 15 Jun 2022 at 20:53, Greg KH wrote: > > > > On Wed, Jun 15, 2022 at 06:48:33PM +0800, heliang wrote: > > > In tegra_uart_init(), of_find_matching_node() will return a node > > > pointer with refcount incremented. We should use of_node_put() > > > when it is not used anymore. > > > > > > Signed-off-by: heliang > > > > We need a real name please, one you sign documents with. > > How do we enforce that? What if Wong, Adele or Beyonce submit a patch? As per our rules in the kernel, we can not accept a signed-off-by by a known anonymous person. When you can obviously see that the name is incorrect (as has happened recently with different names being used for the from: and the s-o-b lines in the email address), you have to ask for clarification. It's something that all maintainers should be doing. > What happens if that patch gets reposted, with S-o-b: He Liang > or Hel Iang, Heli Ang? Do you know any of those are > real names? What happens if they post a real name in > Mandarin/Thai/Cyrillic, can you validate it? I would love for people to use their names in their native language, there's no need to make up english-sounding names at all. We have many examples of this in commits already, I would be happy if we get more. And yes, validating names in those languages is usually not very difficult. thanks, greg k-h
Re: [Linaro-mm-sig] Re: [PATCH] drivers: tty: serial: Add missing of_node_put() in serial-tegra.c
On Wed, Jun 15, 2022 at 10:30:47PM +0200, Daniel Vetter wrote: > On Wed, 15 Jun 2022 at 22:23, Dave Airlie wrote: > > > > On Wed, 15 Jun 2022 at 20:53, Greg KH wrote: > > > > > > On Wed, Jun 15, 2022 at 06:48:33PM +0800, heliang wrote: > > > > In tegra_uart_init(), of_find_matching_node() will return a node > > > > pointer with refcount incremented. We should use of_node_put() > > > > when it is not used anymore. > > > > > > > > Signed-off-by: heliang > > > > > > We need a real name please, one you sign documents with. > > > > How do we enforce that? What if Wong, Adele or Beyonce submit a patch? > > > > What happens if that patch gets reposted, with S-o-b: He Liang > > or Hel Iang, Heli Ang? Do you know any of those are > > real names? What happens if they post a real name in > > Mandarin/Thai/Cyrillic, can you validate it? > > > > Really we require you have an identity attached to an email. If there > > is a problem in the future, we'd prefer the email continues to work so > > that you are contactable. If you are submitting a small amount of > > changes it's probably never going to matter. If you are submitting > > larger bodies of work of course it would be good to have a company or > > larger org attached to track things down legally later, but again that > > isn't always possible. > > > > I don't think alienating the numerous developers who no longer use > > their legal names are identified by one name, but haven't changed > > their legal one yet people who get married and change their legal name > > but don't change their contribution name and I could run this sentence > > on forever. > > Yeah like absolute best case trying to "enforce" this just results in > encouraging people to come up with entirely fake but English looking > names for themselves. Which ... just no. Agree, again, I'd prefer to take real names in native languages, our tools can handle that just fine. No need to make up anything. thanks, greg k-h
Re: [PATCH v2 13/14] drm/vc4: crtc: Fix out of order frames during asynchronous page flips
On 06/10, Maxime Ripard wrote: > When doing an asynchronous page flip (PAGE_FLIP ioctl with the > DRM_MODE_PAGE_FLIP_ASYNC flag set), the current code waits for the > possible GPU buffer being rendered through a call to > vc4_queue_seqno_cb(). > > On the BCM2835-37, the GPU driver is part of the vc4 driver and that > function is defined in vc4_gem.c to wait for the buffer to be rendered, > and once it's done, call a callback. > > However, on the BCM2711 used on the RaspberryPi4, the GPU driver is > separate (v3d) and that function won't do anything. This was working > because we were going into a path, due to uninitialized variables, that > was always scheduling the callback. > > However, we were never actually waiting for the buffer to be rendered > which was resulting in frames being displayed out of order. > > The generic API to signal those kind of completion in the kernel are the > DMA fences, and fortunately the v3d drivers supports them and signal > when its job is done. That API also provides an equivalent function that > allows to have a callback being executed when the fence is signalled as > done. > > Let's change our driver a bit to rely on the previous function for the > older SoCs, and on DMA fences for the BCM2711. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/vc4/vc4_crtc.c | 50 +++--- > 1 file changed, 46 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c > index a3c04d6cbd20..9355213dc883 100644 > --- a/drivers/gpu/drm/vc4/vc4_crtc.c > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c > @@ -776,6 +776,7 @@ struct vc4_async_flip_state { > struct drm_pending_vblank_event *event; > > union { > + struct dma_fence_cb fence; > struct vc4_seqno_cb seqno; > } cb; > }; > @@ -835,6 +836,50 @@ static void vc4_async_page_flip_seqno_complete(struct > vc4_seqno_cb *cb) > vc4_bo_dec_usecnt(bo); > } > > +static void vc4_async_page_flip_fence_complete(struct dma_fence *fence, > +struct dma_fence_cb *cb) > +{ > + struct vc4_async_flip_state *flip_state = > + container_of(cb, struct vc4_async_flip_state, cb.fence); > + > + vc4_async_page_flip_complete(flip_state); > + dma_fence_put(fence); > +} > + > +static int vc4_async_set_fence_cb(struct drm_device *dev, > + struct vc4_async_flip_state *flip_state) > +{ > + struct drm_framebuffer *fb = flip_state->fb; > + struct drm_gem_cma_object *cma_bo = drm_fb_cma_get_gem_obj(fb, 0); > + struct vc4_dev *vc4 = to_vc4_dev(dev); > + struct dma_fence *fence; > + int ret; > + > + if (!vc4->is_vc5) { > + struct vc4_bo *bo = to_vc4_bo(&cma_bo->base); > + > + return vc4_queue_seqno_cb(dev, &flip_state->cb.seqno, bo->seqno, > + vc4_async_page_flip_seqno_complete); > + } > + > + ret = dma_resv_get_singleton(cma_bo->base.resv, DMA_RESV_USAGE_READ, > &fence); > + if (ret) > + return ret; > + > + /* If there's no fence, complete the page flip immediately */ > + if (!fence) { > + vc4_async_page_flip_fence_complete(fence, > &flip_state->cb.fence); > + return 0; > + } Hi, this change lgtm. Reviewed-by: Melissa Wen Thanks > + > + /* If the fence has already been completed, complete the page flip */ > + if (dma_fence_add_callback(fence, &flip_state->cb.fence, > +vc4_async_page_flip_fence_complete)) > + vc4_async_page_flip_fence_complete(fence, > &flip_state->cb.fence); > + > + return 0; > +} > + > static int > vc4_async_page_flip_common(struct drm_crtc *crtc, > struct drm_framebuffer *fb, > @@ -844,8 +889,6 @@ vc4_async_page_flip_common(struct drm_crtc *crtc, > struct drm_device *dev = crtc->dev; > struct drm_plane *plane = crtc->primary; > struct vc4_async_flip_state *flip_state; > - struct drm_gem_cma_object *cma_bo = drm_fb_cma_get_gem_obj(fb, 0); > - struct vc4_bo *bo = to_vc4_bo(&cma_bo->base); > > flip_state = kzalloc(sizeof(*flip_state), GFP_KERNEL); > if (!flip_state) > @@ -876,8 +919,7 @@ vc4_async_page_flip_common(struct drm_crtc *crtc, >*/ > drm_atomic_set_fb_for_plane(plane->state, fb); > > - vc4_queue_seqno_cb(dev, &flip_state->cb.seqno, bo->seqno, > -vc4_async_page_flip_seqno_complete); > + vc4_async_set_fence_cb(dev, flip_state); > > /* Driver takes ownership of state on successful async commit. */ > return 0; > -- > 2.36.1 > signature.asc Description: PGP signature
[PATCH] drm/msm/hdmi: drop empty bridge callbacks
Drop empty callbacks msm_hdmi_bridge_enable() and msm_hdmi_bridge_disable(). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index 97c24010c4d1..d569f0c6cab7 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -159,14 +159,6 @@ static void msm_hdmi_bridge_pre_enable(struct drm_bridge *bridge) msm_hdmi_hdcp_on(hdmi->hdcp_ctrl); } -static void msm_hdmi_bridge_enable(struct drm_bridge *bridge) -{ -} - -static void msm_hdmi_bridge_disable(struct drm_bridge *bridge) -{ -} - static void msm_hdmi_bridge_post_disable(struct drm_bridge *bridge) { struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); @@ -306,8 +298,6 @@ static enum drm_mode_status msm_hdmi_bridge_mode_valid(struct drm_bridge *bridge static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = { .pre_enable = msm_hdmi_bridge_pre_enable, - .enable = msm_hdmi_bridge_enable, - .disable = msm_hdmi_bridge_disable, .post_disable = msm_hdmi_bridge_post_disable, .mode_set = msm_hdmi_bridge_mode_set, .mode_valid = msm_hdmi_bridge_mode_valid, -- 2.35.1
[PATCH] drm/msm/hdmi: support attaching the "next" bridge
There might be a chain of bridges attached to the HDMI node (including but not limited to the display-connector bridge). Add support for attaching them right to the HDMI bridge chain. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/hdmi/hdmi.c | 14 ++ drivers/gpu/drm/msm/hdmi/hdmi.h | 2 ++ 2 files changed, 16 insertions(+) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index cf24e68864ba..9fdb17317589 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -9,6 +9,7 @@ #include #include +#include #include #include "hdmi.h" @@ -133,6 +134,10 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev) hdmi->config = config; spin_lock_init(&hdmi->reg_lock); + ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL, &hdmi->next_bridge); + if (ret && ret != -ENODEV) + goto fail; + hdmi->mmio = msm_ioremap(pdev, config->mmio_name); if (IS_ERR(hdmi->mmio)) { ret = PTR_ERR(hdmi->mmio); @@ -291,6 +296,15 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, goto fail; } + if (hdmi->next_bridge) { + ret = drm_bridge_attach(hdmi->encoder, hdmi->next_bridge, hdmi->bridge, + DRM_BRIDGE_ATTACH_NO_CONNECTOR); + if (ret) { + DRM_DEV_ERROR(dev->dev, "failed to attach next HDMI bridge: %d\n", ret); + goto fail; + } + } + hdmi->connector = drm_bridge_connector_init(hdmi->dev, encoder); if (IS_ERR(hdmi->connector)) { ret = PTR_ERR(hdmi->connector); diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h index 736f348befb3..5241735438ac 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.h +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h @@ -68,6 +68,8 @@ struct hdmi { struct drm_connector *connector; struct drm_bridge *bridge; + struct drm_bridge *next_bridge; + /* the encoder we are hooked to (outside of hdmi block) */ struct drm_encoder *encoder; -- 2.35.1
Re:Re: [Linaro-mm-sig] Re: [PATCH] drivers: tty: serial: Add missing of_node_put() in serial-tegra.c
At 2022-06-16 16:43:43, "Greg KH" wrote: >On Wed, Jun 15, 2022 at 10:30:47PM +0200, Daniel Vetter wrote: >> On Wed, 15 Jun 2022 at 22:23, Dave Airlie wrote: >> > >> > On Wed, 15 Jun 2022 at 20:53, Greg KH wrote: >> > > >> > > On Wed, Jun 15, 2022 at 06:48:33PM +0800, heliang wrote: >> > > > In tegra_uart_init(), of_find_matching_node() will return a node >> > > > pointer with refcount incremented. We should use of_node_put() >> > > > when it is not used anymore. >> > > > >> > > > Signed-off-by: heliang >> > > >> > > We need a real name please, one you sign documents with. >> > >> > How do we enforce that? What if Wong, Adele or Beyonce submit a patch? >> > >> > What happens if that patch gets reposted, with S-o-b: He Liang >> > or Hel Iang, Heli Ang? Do you know any of those are >> > real names? What happens if they post a real name in >> > Mandarin/Thai/Cyrillic, can you validate it? >> > >> > Really we require you have an identity attached to an email. If there >> > is a problem in the future, we'd prefer the email continues to work so >> > that you are contactable. If you are submitting a small amount of >> > changes it's probably never going to matter. If you are submitting >> > larger bodies of work of course it would be good to have a company or >> > larger org attached to track things down legally later, but again that >> > isn't always possible. >> > >> > I don't think alienating the numerous developers who no longer use >> > their legal names are identified by one name, but haven't changed >> > their legal one yet people who get married and change their legal name >> > but don't change their contribution name and I could run this sentence >> > on forever. >> >> Yeah like absolute best case trying to "enforce" this just results in >> encouraging people to come up with entirely fake but English looking >> names for themselves. Which ... just no. > >Agree, again, I'd prefer to take real names in native languages, our >tools can handle that just fine. No need to make up anything. > >thanks, > >greg k-h hi, Greg K-H, I have resent a new patch for my commit of tegra_uart_init() bug with my real name for Sob. So there is anyother thing I should do?
Re: [Intel-gfx] [PATCH 3/3] drm/doc/rfc: VM_BIND uapi definition
On 15/06/2022 16:20, Niranjana Vishwanathapura wrote: On Wed, Jun 15, 2022 at 08:22:23AM +0100, Tvrtko Ursulin wrote: On 14/06/2022 17:42, Niranjana Vishwanathapura wrote: On Tue, Jun 14, 2022 at 05:07:37PM +0100, Tvrtko Ursulin wrote: On 14/06/2022 17:02, Tvrtko Ursulin wrote: On 14/06/2022 16:43, Niranjana Vishwanathapura wrote: On Tue, Jun 14, 2022 at 08:16:41AM +0100, Tvrtko Ursulin wrote: On 14/06/2022 00:39, Matthew Brost wrote: On Mon, Jun 13, 2022 at 07:09:06PM +0100, Tvrtko Ursulin wrote: On 13/06/2022 18:49, Niranjana Vishwanathapura wrote: On Mon, Jun 13, 2022 at 05:22:02PM +0100, Tvrtko Ursulin wrote: On 13/06/2022 16:05, Niranjana Vishwanathapura wrote: On Mon, Jun 13, 2022 at 09:24:18AM +0100, Tvrtko Ursulin wrote: On 10/06/2022 17:14, Niranjana Vishwanathapura wrote: On Fri, Jun 10, 2022 at 05:48:39PM +0300, Lionel Landwerlin wrote: On 10/06/2022 13:37, Tvrtko Ursulin wrote: On 10/06/2022 08:07, Niranjana Vishwanathapura wrote: VM_BIND and related uapi definitions Signed-off-by: Niranjana Vishwanathapura --- Documentation/gpu/rfc/i915_vm_bind.h | 490 +++ 1 file changed, 490 insertions(+) create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h new file mode 100644 index ..9fc854969cfb --- /dev/null +++ b/Documentation/gpu/rfc/i915_vm_bind.h @@ -0,0 +1,490 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2022 Intel Corporation + */ + +/** + * DOC: I915_PARAM_HAS_VM_BIND + * + * VM_BIND feature availability. + * See typedef drm_i915_getparam_t param. + * bit[0]: If set, VM_BIND is supported, otherwise not. + * bits[8-15]: VM_BIND implementation version. + * version 0 will not have VM_BIND/UNBIND timeline fence array support. + */ +#define I915_PARAM_HAS_VM_BIND 57 + +/** + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND + * + * Flag to opt-in for VM_BIND mode of binding during VM creation. + * See struct drm_i915_gem_vm_control flags. + * + * The older execbuf2 ioctl will not support VM_BIND mode of operation. + * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any + * execlist (See struct drm_i915_gem_execbuffer3 for more details). + * + */ +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0) + +/** + * DOC: I915_CONTEXT_CREATE_FLAGS_LONG_RUNNING + * + * Flag to declare context as long running. + * See struct drm_i915_gem_context_create_ext flags. + * + * Usage of dma-fence expects that they complete in reasonable amount of time. + * Compute on the other hand can be long running. Hence it is not appropriate + * for compute contexts to export request completion dma-fence to user. + * The dma-fence usage will be limited to in-kernel consumption only. + * Compute contexts need to use user/memory fence. + * + * So, long running contexts do not support output fences. Hence, + * I915_EXEC_FENCE_SIGNAL (See &drm_i915_gem_exec_fence.flags) is expected + * to be not used. DRM_I915_GEM_WAIT ioctl call is also not supported for + * objects mapped to long running contexts. + */ +#define I915_CONTEXT_CREATE_FLAGS_LONG_RUNNING (1u << 2) + +/* VM_BIND related ioctls */ +#define DRM_I915_GEM_VM_BIND 0x3d +#define DRM_I915_GEM_VM_UNBIND 0x3e +#define DRM_I915_GEM_EXECBUFFER3 0x3f +#define DRM_I915_GEM_WAIT_USER_FENCE 0x40 + +#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind) +#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind) +#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3) +#define DRM_IOCTL_I915_GEM_WAIT_USER_FENCE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_WAIT_USER_FENCE, struct drm_i915_gem_wait_user_fence) + +/** + * struct drm_i915_gem_vm_bind - VA to object mapping to bind. + * + * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU + * virtual address (VA) range to the section of an object that should be bound + * in the device page table of the specified address space (VM). + * The VA range specified must be unique (ie., not currently bound) and can + * be mapped to whole object or a section of the object (partial binding). + * Multiple VA mappings can be created to the same section of the object + * (aliasing). + * + * The @queue_idx specifies the queue to use for binding. Same queue can be + * used for both VM_BIND and VM_UNBIND calls. All submitted bind and unbind + * operations in a queue are performed in the order of submission. + * + * The @start, @offset and @length should be 4K page aligned. However the DG2 + * and XEHPSDV has 64K page size for device local-memory and has compact page + * table. On those platforms, for binding device local-memory objects, the + * @start should be 2M aligned, @offset and @length should be 64K aligned. + * Also, on
Re: [PATCH v4 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support
On Thu, Jun 16, 2022 at 12:42 AM Stephen Boyd wrote: > > Quoting Prashant Malani (2022-06-15 10:20:20) > > > > .../display/bridge/analogix,anx7625.yaml | 64 +++ > > 1 file changed, 64 insertions(+) > > Can this file get a link to the product brief[1]? It helps to quickly > find the block diagram. Sure, but I don't really think that should be included in this patch (or series). I'd be happy to submit a separate patch once this series is resolved. > > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > index 35a48515836e..bc6f7644db31 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > @@ -105,6 +105,34 @@ properties: > >- port@0 > >- port@1 > > > > + switches: > > +type: object > > +description: Set of switches controlling DisplayPort traffic on > > + outgoing RX/TX lanes to Type C ports. > > +additionalProperties: false > > + > > +properties: > > + '#address-cells': > > +const: 1 > > + > > + '#size-cells': > > +const: 0 > > + > > +patternProperties: > > + '^switch@[01]$': > > +$ref: /schemas/usb/typec-switch.yaml# > > +unevaluatedProperties: false > > + > > +properties: > > + reg: > > +maxItems: 1 > > + > > +required: > > + - reg > > + > > +required: > > + - switch@0 > > + > > required: > >- compatible > >- reg > > @@ -167,5 +195,41 @@ examples: > > }; > > }; > > }; > > +switches { > > Is "switches" a bus? No. > > > +#address-cells = <1>; > > +#size-cells = <0>; > > +switch@0 { > > +compatible = "typec-switch"; > > Is this compatible matched against a driver that's populated on this > "switches" bus? No. Patch 6/7 has the implementation details on how the anx driver performs the enumeration of switches. > > > +reg = <0>; > > +mode-switch; > > + > > +ports { > > +#address-cells = <1>; > > +#size-cells = <0>; > > +port@0 { > > +reg = <0>; > > +anx_typec0: endpoint { > > +remote-endpoint = <&typec_port0>; > > +}; > > +}; > > +}; > > I was expecting to see these simply be more ports in the existing graph > binding of this device, and then have the 'mode-switch' or > 'orientation-switch' properties be at the same level as the compatible > string "analogix,anx7625". Here's the reasoning, based on looking at the > product brief and the existing binding/implementation. > > Looking at the only existing implementation of this binding upstream in > mt8183-kukui-jacuzzi.dtsi it looks like one of these typec ports is > actually the same physically as the 'anx7625_out' endpoint (reg address > of 1) that is already defined in the binding. It seems that MIPI DSI/DPI > comes in and is output through 2 lanes, SSRX2 and SSTX2 according to the > product brief[1], and that is connected to some eDP panel > ("auo,b116xw03"). Presumably that is the same as anx_typec1 in this > patch? I suspect the USB3.1 input is not connected on this board, and > thus the crosspoint switch is never used, nor the SSRX1/SSTX1 pins. > > The existing binding defines the MIPI DSI/DPI input as port0 and two of > the four lanes of output that is probably by default connected to the > "DisplayPort Transmitter" as port1 because that's how the crosspoint > switch comes out of reset. That leaves the USB3.1 input possibly needing > a port in the ports binding, and the other two lanes of output needing a > port in the ports binding to describe their connection to the downstream > device. And finally information about if the crosspoint switch needs to > be registered with the typec framework to do typec things, which can be > achieved by the presence of the 'mode-switch' property. > > On a board like kukui-jacuzzi these new properties and ports wouldn't be > specified, because what is there is already sufficient. If this chip is > connected to a usb-c-connector then I'd expect to see a connection from > the output ports in the graph binding to the connector node's ports. > There aren't any ports in the usb-c-connector binding though from what I > see. > > I believe there's also one more use case here where USB3.1 or MIPI > DSI/DPI is connected on the input side and this device is used to steer > USB3.1 or DP through the crosspoint switch to either of the two output > pairs. This last scenario means that we have to describe
Re: [PATCH v4 0/7] usb: typec: Introduce typec-switch binding
Il 15/06/22 19:20, Prashant Malani ha scritto: This series introduces a binding for Type-C data lane switches. These control the routing and operating modes of USB Type-C data lanes based on the PD messaging from the Type-C port driver regarding connected peripherals. The first patch introduces a change to the Type-C mux class mode-switch matching code, while the second adds a config guard to a Type-C header. The next couple of patches introduce the new "typec-switch" binding as well as one user of it (the ANX7625 drm bridge). The remaining patches add functionality to the anx7625 driver to register the mode-switches, as well as program its crosspoint switch depending on which Type-C port has a DisplayPort (DP) peripheral connected to it. v3: https://lore.kernel.org/linux-usb/20220614193558.1163205-1-pmal...@chromium.org/ For the entire series: Reviewed-by: AngeloGioacchino Del Regno Regards, Angelo
[RFC PATCH 0/2] drm/msm/mdp4: rework LVDS/LCDC panel support
MDP4 uses custom code to handle LVDS panel. It predates handling EPROBE_DEFER, it tries to work when the panel device is not available, etc. Switch MDP4 LCDC code to use drm_panel_bridge/drm_bridge_connector to follow contemporary DRM practices. Note, this code has been compile-tested only. Testing on the real device is still pending (and will be performed before the merge). Dmitry Baryshkov (2): drm/msm/mdp4: move move_valid callback to lcdc_encoder drm/msm/mdp4: switch LVDS to use drm_bridge/_connector drivers/gpu/drm/msm/Makefile | 1 - drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 32 - drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 7 +- .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 46 +++ .../drm/msm/disp/mdp4/mdp4_lvds_connector.c | 120 -- 5 files changed, 47 insertions(+), 159 deletions(-) delete mode 100644 drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c -- 2.35.1
[RFC PATCH 1/2] drm/msm/mdp4: move move_valid callback to lcdc_encoder
We can check the LCDC clock directly from the LCDC encoder driver, so remove it from the LVDS connector. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 1 - .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 26 ++- .../drm/msm/disp/mdp4/mdp4_lvds_connector.c | 20 -- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h index e8ee92ab7956..d27fa761bfc2 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h @@ -197,7 +197,6 @@ struct drm_crtc *mdp4_crtc_init(struct drm_device *dev, long mdp4_dtv_round_pixclk(struct drm_encoder *encoder, unsigned long rate); struct drm_encoder *mdp4_dtv_encoder_init(struct drm_device *dev); -long mdp4_lcdc_round_pixclk(struct drm_encoder *encoder, unsigned long rate); struct drm_encoder *mdp4_lcdc_encoder_init(struct drm_device *dev, struct device_node *panel_node); diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c index 10eb3e5b218e..341dcd5087da 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c @@ -366,19 +366,31 @@ static void mdp4_lcdc_encoder_enable(struct drm_encoder *encoder) mdp4_lcdc_encoder->enabled = true; } +static int mdp4_lcdc_encoder_mode_valid(struct drm_encoder *encoder, + const struct drm_display_mode *mode) +{ + struct mdp4_lcdc_encoder *mdp4_lcdc_encoder = + to_mdp4_lcdc_encoder(encoder); + long actual, requested; + + requested = 1000 * mode->clock; + actual = clk_round_rate(mdp4_lcdc_encoder->lcdc_clk, requested); + + DBG("requested=%ld, actual=%ld", requested, actual); + + if (actual != requested) + return MODE_CLOCK_RANGE; + + return MODE_OK; +} + static const struct drm_encoder_helper_funcs mdp4_lcdc_encoder_helper_funcs = { .mode_set = mdp4_lcdc_encoder_mode_set, .disable = mdp4_lcdc_encoder_disable, .enable = mdp4_lcdc_encoder_enable, + .mode_valid = mdp4_lcdc_encoder_mode_valid, }; -long mdp4_lcdc_round_pixclk(struct drm_encoder *encoder, unsigned long rate) -{ - struct mdp4_lcdc_encoder *mdp4_lcdc_encoder = - to_mdp4_lcdc_encoder(encoder); - return clk_round_rate(mdp4_lcdc_encoder->lcdc_clk, rate); -} - /* initialize encoder */ struct drm_encoder *mdp4_lcdc_encoder_init(struct drm_device *dev, struct device_node *panel_node) diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c index 7288041dd86a..4755eb13ef79 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c @@ -56,25 +56,6 @@ static int mdp4_lvds_connector_get_modes(struct drm_connector *connector) return ret; } -static int mdp4_lvds_connector_mode_valid(struct drm_connector *connector, -struct drm_display_mode *mode) -{ - struct mdp4_lvds_connector *mdp4_lvds_connector = - to_mdp4_lvds_connector(connector); - struct drm_encoder *encoder = mdp4_lvds_connector->encoder; - long actual, requested; - - requested = 1000 * mode->clock; - actual = mdp4_lcdc_round_pixclk(encoder, requested); - - DBG("requested=%ld, actual=%ld", requested, actual); - - if (actual != requested) - return MODE_CLOCK_RANGE; - - return MODE_OK; -} - static const struct drm_connector_funcs mdp4_lvds_connector_funcs = { .detect = mdp4_lvds_connector_detect, .fill_modes = drm_helper_probe_single_connector_modes, @@ -86,7 +67,6 @@ static const struct drm_connector_funcs mdp4_lvds_connector_funcs = { static const struct drm_connector_helper_funcs mdp4_lvds_connector_helper_funcs = { .get_modes = mdp4_lvds_connector_get_modes, - .mode_valid = mdp4_lvds_connector_mode_valid, }; /* initialize connector */ -- 2.35.1
[RFC PATCH 2/2] drm/msm/mdp4: switch LVDS to use drm_bridge/_connector
LVDS support in MDP4 driver makes use of drm_connector directly. However LCDC encoder and LVDS connector are wrappers around drm_panel. Switch them to use drm_panel_bridge/drm_bridge_connector. This allows using standard interface for the drm_panel and also inserting additional bridges between encoder and panel. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/Makefile | 1 - drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 32 -- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 6 +- .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 20 +--- .../drm/msm/disp/mdp4/mdp4_lvds_connector.c | 100 -- 5 files changed, 28 insertions(+), 131 deletions(-) delete mode 100644 drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 66395ee0862a..709952c998e0 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -35,7 +35,6 @@ msm-$(CONFIG_DRM_MSM_MDP4) += \ disp/mdp4/mdp4_dsi_encoder.o \ disp/mdp4/mdp4_dtv_encoder.o \ disp/mdp4/mdp4_lcdc_encoder.o \ - disp/mdp4/mdp4_lvds_connector.o \ disp/mdp4/mdp4_lvds_pll.o \ disp/mdp4/mdp4_irq.o \ disp/mdp4/mdp4_kms.o \ diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c index fb48c8c19ec3..77ef3114a919 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c @@ -6,6 +6,8 @@ #include +#include +#include #include #include "msm_drv.h" @@ -199,7 +201,7 @@ static int mdp4_modeset_init_intf(struct mdp4_kms *mdp4_kms, struct msm_drm_private *priv = dev->dev_private; struct drm_encoder *encoder; struct drm_connector *connector; - struct device_node *panel_node; + struct drm_bridge *next_bridge; int dsi_id; int ret; @@ -209,11 +211,15 @@ static int mdp4_modeset_init_intf(struct mdp4_kms *mdp4_kms, * bail out early if there is no panel node (no need to * initialize LCDC encoder and LVDS connector) */ - panel_node = of_graph_get_remote_node(dev->dev->of_node, 0, 0); - if (!panel_node) - return 0; + next_bridge = devm_drm_of_get_bridge(dev->dev, dev->dev->of_node, 0, 0); + if (IS_ERR(next_bridge)) { + ret = PTR_ERR(next_bridge); + if (ret == -ENODEV) + return 0; + return ret; + } - encoder = mdp4_lcdc_encoder_init(dev, panel_node); + encoder = mdp4_lcdc_encoder_init(dev); if (IS_ERR(encoder)) { DRM_DEV_ERROR(dev->dev, "failed to construct LCDC encoder\n"); return PTR_ERR(encoder); @@ -222,12 +228,26 @@ static int mdp4_modeset_init_intf(struct mdp4_kms *mdp4_kms, /* LCDC can be hooked to DMA_P (TODO: Add DMA_S later?) */ encoder->possible_crtcs = 1 << DMA_P; - connector = mdp4_lvds_connector_init(dev, panel_node, encoder); + ret = drm_bridge_attach(encoder, next_bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); + if (ret) { + DRM_DEV_ERROR(dev->dev, "failed to attach LVDS panel/bridge: %d\n", ret); + + return ret; + } + + connector = drm_bridge_connector_init(dev, encoder); if (IS_ERR(connector)) { DRM_DEV_ERROR(dev->dev, "failed to initialize LVDS connector\n"); return PTR_ERR(connector); } + ret = drm_connector_attach_encoder(connector, encoder); + if (ret) { + DRM_DEV_ERROR(dev->dev, "failed to attach LVDS connector: %d\n", ret); + + return ret; + } + break; case DRM_MODE_ENCODER_TMDS: encoder = mdp4_dtv_encoder_init(dev); diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h index d27fa761bfc2..7535dc343b64 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h @@ -197,11 +197,7 @@ struct drm_crtc *mdp4_crtc_init(struct drm_device *dev, long mdp4_dtv_round_pixclk(struct drm_encoder *encoder, unsigned long rate); struct drm_encoder *mdp4_dtv_encoder_init(struct drm_device *dev); -struct drm_encoder *mdp4_lcdc_encoder_init(struct drm_device *dev, - struct device_node *panel_node); - -struct drm_connector *mdp4_lvds_connector_init(struct drm_device *dev, - struct device_node *panel_node, struct drm_encoder *encoder); +struct drm_encoder *mdp4_lcdc_encoder_init(struct drm_device *dev); #ifdef CONFIG_DRM_MSM_DSI struct drm_encoder
[PATCH] usb: gadget: Remove unnecessary print function dev_err()
The print function dev_err() is redundant because platform_get_irq() already prints an error. This was found by coccicheck: ./drivers/usb/gadget/udc/aspeed_udc.c:1546:2-9: line 1546 is redundant because platform_get_irq() already prints an error. Signed-off-by: Jiapeng Chong --- drivers/usb/gadget/udc/aspeed_udc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c index 1fc15228ff15..2c3dc80d6b8c 100644 --- a/drivers/usb/gadget/udc/aspeed_udc.c +++ b/drivers/usb/gadget/udc/aspeed_udc.c @@ -1543,7 +1543,6 @@ static int ast_udc_probe(struct platform_device *pdev) /* Find interrupt and install handler */ udc->irq = platform_get_irq(pdev, 0); if (udc->irq < 0) { - dev_err(&pdev->dev, "Failed to get interrupt\n"); rc = udc->irq; goto err; } -- 2.20.1.7.g153144c
Re: [PATCH v2 00/14] drm/vc4: Properly separate v3d on BCM2711, and fix frame ordering
On Fri, 10 Jun 2022 13:51:35 +0200, Maxime Ripard wrote: > Here's a series that fixes a significant issue we missed when adding support > for the BCM2711 / RaspberryPi4 in the vc4 driver. > > Indeed, before the introduction of the BCM2711 support, the GPU was fairly > intertwined with the display hardware, and was thus supported by the vc4 > driver. Among other things, the driver needed to accomodate for a bunch of > hardware limitations (such as a lack of IOMMU) by implementing a custom memory > management backend, tied with the v3d hardware. > > [...] Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
Re: [PATCH 02/64] drm/crtc: Introduce drmm_crtc_init_with_planes
On Wed, Jun 15, 2022 at 12:34:46PM +0200, Thomas Zimmermann wrote: > Hi > > Am 15.06.22 um 10:32 schrieb Maxime Ripard: > [...] > > > See, helpers should be useful to many drivers. If we add them, we also > > > add a > > > resources and maintenance overhead to our libraries. And right now, these > > > new functions appear to work around the design of the vc4 driver's data > > > structures. If you want to keep them, maybe let's first merge them into > > > vc4 > > > (something like vc4_crtc_init_with_planes(), etc). If another driver with > > > a > > > use case comes along, we can still move them out easily. > > > > Not that I disagree, but there's also the fact that people will start > > using helpers because they are available. > > > > You mentioned drmm_crtc_alloc_with_planes(). It was introduced in 5.12 > > with a single user (ipuv3-crtc.c). And then, because it was available, > > in 5.17 was merged the Unisoc driver that was the second user of that > > function. > > OTOH, it actually took 5 releases to find another user. Maybe we need to > look harder for possible reuse of helpers, but I wouldn't count 5 releases > as a good track record. Indeed, but I'm not sure it's due to the helper itself. I'm fairly sure nobody really cared or knows about the lifetime issues solved by the drm-managed functions, and so nobody feels an urge to convert. And one can't ask during review to use it if they're not aware of the helpers existence. Between 5.12 and 5.17, only hyperv and sprd were merged. 50% of the new drivers using it is not too bad. > > drmm_simple_encoder_alloc() and drmm_universal_plane_alloc() are in the > > same situation and we wouldn't have had that discussion if it was kept > > in the imx driver. > > > > The helper being there allows driver authors to discover them easily, > > pointing out an issue that possibly wasn't obvious to the author, and we > > can also point during review that the helpers are there to be used. > > > > None of that would be possible if we were to keep them in a driver, > > because no one but the author would know about it. > > > > My feeling is that the rule you mention works great when you know that > > some deviation is going to happen. But we're replacing an init function > > that has been proved good enough here, so it's not rocket science > > really. > > > > drmm_mutex_init() is a great example of that actually. You merged it > > recently with two users. We could have used the exact same argument that > > it belonged in those drivers because it wasn't generic enough or > > something. But it's trivial, so it was a good decision to merge it as a > > helper. And because you did so, I later found out that mutex_destroy() > > was supposed to be called in the first place, I converted vc4 to > > drmm_mutex_init(), and now that bug is fixed. > > But when I added it, there actually were two users. I would not have added > drmm_mutex_init() if it was only useful for a single driver. > > In other cases, we tend to push single-user helpers into the drivers. That > happened several times with TTM. Code was moved into vmwgfx, because there > where no other users. Yeah, and I introduced some in that series too. It makes sense to have that restriction for stuff that we're not really sure about. But for strict equivalents to functions that already exists with the same API I'm not sure that restriction makes sense. In fact, we also merged recently devm_drm_bridge_add with a single user and that was fine too, because that function has been there for a while and we know it's not going to change much. > Anyway, as you insist on using this helper, go for it. But please, at least > reimplement drm_crtc_alloc_with_planes() on top of a shared internal > implementation. AFAICT drm_crtc_alloc_with_planes() is drmm_kzalloc + > drmm_crtc_init_with_planes(). Ack > Same for other related helpers in the other patches, if there are any. drmm_encoder_alloc() and drmm_simple_encoder_alloc() are in the same situation, I'll fix those too. Maxime signature.asc Description: PGP signature
Re:Re: [Linaro-mm-sig] Re: [PATCH] drivers: tty: serial: Add missing of_node_put() in serial-tegra.c
At 2022-06-16 17:20:24, conor.doo...@microchip.com wrote: >On 16/06/2022 09:52, Liang He wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the >> content is safe >> >> At 2022-06-16 16:43:43, "Greg KH" wrote: >>> On Wed, Jun 15, 2022 at 10:30:47PM +0200, Daniel Vetter wrote: On Wed, 15 Jun 2022 at 22:23, Dave Airlie wrote: > > On Wed, 15 Jun 2022 at 20:53, Greg KH wrote: >> >> On Wed, Jun 15, 2022 at 06:48:33PM +0800, heliang wrote: >>> In tegra_uart_init(), of_find_matching_node() will return a node >>> pointer with refcount incremented. We should use of_node_put() >>> when it is not used anymore. >>> >>> Signed-off-by: heliang >> >> We need a real name please, one you sign documents with. > > How do we enforce that? What if Wong, Adele or Beyonce submit a patch? > > What happens if that patch gets reposted, with S-o-b: He Liang > or Hel Iang, Heli Ang? Do you know any of those are > real names? What happens if they post a real name in > Mandarin/Thai/Cyrillic, can you validate it? > > Really we require you have an identity attached to an email. If there > is a problem in the future, we'd prefer the email continues to work so > that you are contactable. If you are submitting a small amount of > changes it's probably never going to matter. If you are submitting > larger bodies of work of course it would be good to have a company or > larger org attached to track things down legally later, but again that > isn't always possible. > > I don't think alienating the numerous developers who no longer use > their legal names are identified by one name, but haven't changed > their legal one yet people who get married and change their legal name > but don't change their contribution name and I could run this sentence > on forever. Yeah like absolute best case trying to "enforce" this just results in encouraging people to come up with entirely fake but English looking names for themselves. Which ... just no. >>> >>> Agree, again, I'd prefer to take real names in native languages, our >>> tools can handle that just fine. No need to make up anything. > >Since this is the only mail from this chain in my inbox and I asked the >same question as Greg on other patches: >I think it is pretty reasonable to /ask/ if something is not a real name >when you see something like "heliang " where there's a >clear difference. And "It is my real name" is a perfectly reasonable >response /shrug. >Trust but verify right? It's not like I'm gonna argue the toss with >someone if they say it is their real name... > >Thanks, >Conchubhar ;) > >>> >>> thanks, >>> >>> greg k-h >> >> hi, Greg K-H, >> >> I have resent a new patch for my commit of tegra_uart_init() bug with my >> real name for Sob. >> >> So there is anyother thing I should do? > Sorry, Conor and Grep K-H. I did not explain clearly and I respect all your suggestions. I have resent with 'Liang He' as my real name for all patches which I sent with heliang yesterday. And I use '[PATCH v2]' for there resent patches. Sorry again.
Re: [PATCH 17/64] drm/vc4: crtc: Move debugfs_name to crtc_data
Hi Dave, On Tue, Jun 14, 2022 at 04:57:45PM +0100, Dave Stevenson wrote: > On Fri, 10 Jun 2022 at 10:30, Maxime Ripard wrote: > > > > All the CRTCs, including the TXP, have a debugfs file and name so we can > > consolidate it into vc4_crtc_data. > > > > Signed-off-by: Maxime Ripard > > Reviewed-by: Dave Stevenson > > I was sort of expecting the vc4_debugfs_add_regset32 call to move to > vc4_crtc_init so that it would be a common call for crtcs and txp, but > that doesn't appear to happen in this set. The debugfs_name for the > txp is therefore actually unused. As of this patch, you're right This is later changed by some other patch though: https://lore.kernel.org/all/20220610092924.754942-60-max...@cerno.tech/ Maxime signature.asc Description: PGP signature
Re: [PATCH v2] drm/arm/hdlcd: Take over EFI framebuffer properly
On Wed, Jun 15, 2022 at 05:09:15PM +0100, Robin Murphy wrote: > The Arm Juno board EDK2 port has provided an EFI GOP display via HDLCD0 > for some time now, which works nicely as an early framebuffer. However, > once the HDLCD driver probes and takes over the hardware, it should > take over the logical framebuffer as well, otherwise the now-defunct GOP > device hangs about and virtual console output inevitably disappears into > the wrong place most of the time. > > We'll do this after binding the HDMI encoder, since that's the most > likely thing to fail, and the EFI console is still better than nothing > when that happens. However, the two HDLCD controllers on Juno are > independent, and many users will still be using older firmware without > any display support, so we'll only bother if we find that the HDLCD > we're probing is already enabled. And if it is, then we'll also stop it, > since otherwise the display can end up shifted if it's still scanning > out while the rest of the registers are subsequently reconfigured. Agree on trying to figure out if the controller is enabled before taking over the framebuffer, but we're also making the big asssumption (currently true) that EKD2 will only initialise one controller. If that is ever false, we should be looking into HDLCD_REG_FB_BASE to figure out the start of the firmware framebuffer and request that via drm_aperture_remove_conflicting_framebuffers(). Not a big deal at the moment and I think the patch can be merged as is. What I'm interested to know more is this > since otherwise the display can end up shifted if it's still scanning > out while the rest of the registers are subsequently reconfigured. Now I know that probe time is not atomic and it can have some weird behaviour, but I was not expecting pixels to shift. Maybe it is because of clock reprogramming? > > Signed-off-by: Robin Murphy Acked-by: Liviu Dudau Will merge this into drm-misc-next if/when there are no more comments. Best regards, Liviu > --- > > Since I ended up adding (relatively) a lot here, I didn't want to > second-guess Javier's opinion so left off the R-b tag from v1. > > drivers/gpu/drm/arm/hdlcd_drv.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c > index e89ae0ec60eb..1f1171f2f16a 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > @@ -21,6 +21,7 @@ > #include > #include > > +#include > #include > #include > #include > @@ -314,6 +315,12 @@ static int hdlcd_drm_bind(struct device *dev) > goto err_vblank; > } > > + /* If EFI left us running, take over from efifb/sysfb */ > + if (hdlcd_read(hdlcd, HDLCD_REG_COMMAND)) { > + hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0); > + drm_aperture_remove_framebuffers(false, &hdlcd_driver); > + } > + > drm_mode_config_reset(drm); > drm_kms_helper_poll_init(drm); > > -- > 2.36.1.dirty > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯
Re: [PATCH 17/64] drm/vc4: crtc: Move debugfs_name to crtc_data
On Thu, 16 Jun 2022 at 10:41, Maxime Ripard wrote: > > Hi Dave, > > On Tue, Jun 14, 2022 at 04:57:45PM +0100, Dave Stevenson wrote: > > On Fri, 10 Jun 2022 at 10:30, Maxime Ripard wrote: > > > > > > All the CRTCs, including the TXP, have a debugfs file and name so we can > > > consolidate it into vc4_crtc_data. > > > > > > Signed-off-by: Maxime Ripard > > > > Reviewed-by: Dave Stevenson > > > > I was sort of expecting the vc4_debugfs_add_regset32 call to move to > > vc4_crtc_init so that it would be a common call for crtcs and txp, but > > that doesn't appear to happen in this set. The debugfs_name for the > > txp is therefore actually unused. > > As of this patch, you're right > > This is later changed by some other patch though: > https://lore.kernel.org/all/20220610092924.754942-60-max...@cerno.tech/ Indeed, and that cleans it all up. Perfect. Dave
Re: [PATCH 25/64] drm/vc4: dpi: Add action to disable the clock
On Thu, 16 Jun 2022 at 09:38, Maxime Ripard wrote: > > Hi Dave, > > On Tue, Jun 14, 2022 at 05:47:28PM +0100, Dave Stevenson wrote: > > Hi Maxime. > > > > On Fri, 10 Jun 2022 at 10:30, Maxime Ripard wrote: > > > > > > Adding a device-managed action will make the error path easier, so let's > > > create one to disable our clock. > > > > The DPI block has two clocks (core and pixel), and this is only > > affecting one of them (the core clock). > > Thanks for the suggestion, I've amended the commit message. > > > (I'm actually puzzling over what it's wanting to do with the core > > clock here as it's only enabling it rather than setting a rate. I > > think it may be redundant). > > Could it be that it a "bus" clock that we need it to access the > registers? No idea. Normally it's the power domain that is required to access registers. AFAIK the core clock is never turned off, so the request for it is just a little odd. It is what it is though, so fine to leave it alone. Dave
RE: [PATCH] usb: gadget: Remove unnecessary print function dev_err()
> The print function dev_err() is redundant because platform_get_irq() already > prints an error. > > This was found by coccicheck: > > ./drivers/usb/gadget/udc/aspeed_udc.c:1546:2-9: line 1546 is redundant > because platform_get_irq() already prints an error. > > Signed-off-by: Jiapeng Chong Acked-by: Neal Liu > --- > drivers/usb/gadget/udc/aspeed_udc.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/usb/gadget/udc/aspeed_udc.c > b/drivers/usb/gadget/udc/aspeed_udc.c > index 1fc15228ff15..2c3dc80d6b8c 100644 > --- a/drivers/usb/gadget/udc/aspeed_udc.c > +++ b/drivers/usb/gadget/udc/aspeed_udc.c > @@ -1543,7 +1543,6 @@ static int ast_udc_probe(struct platform_device > *pdev) > /* Find interrupt and install handler */ > udc->irq = platform_get_irq(pdev, 0); > if (udc->irq < 0) { > - dev_err(&pdev->dev, "Failed to get interrupt\n"); > rc = udc->irq; > goto err; > } > -- > 2.20.1.7.g153144c
Re: [PATCH v2] drm/arm/hdlcd: Take over EFI framebuffer properly
Hello Robin, On 6/15/22 18:09, Robin Murphy wrote: > The Arm Juno board EDK2 port has provided an EFI GOP display via HDLCD0 > for some time now, which works nicely as an early framebuffer. However, > once the HDLCD driver probes and takes over the hardware, it should > take over the logical framebuffer as well, otherwise the now-defunct GOP > device hangs about and virtual console output inevitably disappears into > the wrong place most of the time. > > We'll do this after binding the HDMI encoder, since that's the most > likely thing to fail, and the EFI console is still better than nothing > when that happens. However, the two HDLCD controllers on Juno are > independent, and many users will still be using older firmware without > any display support, so we'll only bother if we find that the HDLCD > we're probing is already enabled. And if it is, then we'll also stop it, > since otherwise the display can end up shifted if it's still scanning > out while the rest of the registers are subsequently reconfigured. > > Signed-off-by: Robin Murphy > --- > > Since I ended up adding (relatively) a lot here, I didn't want to > second-guess Javier's opinion so left off the R-b tag from v1. > Yes, v2 looks good to me so feel free to keep my R-b: Reviewed-by: Javier Martinez Canillas [snip] > > + /* If EFI left us running, take over from efifb/sysfb */ There are other drivers such as simplefb and simpledrm that also use a simple platform-provided framebuffer. So instead of mentioning all drivers maybe you could just have something like the following ? /* If EFI left us running, take over from simple framebuffer drivers */ And maybe you can also add some words about why you are checking the HDLCD_REG_COMMAND register ? Liviu gave more context in this thread, I believe some of this info could be in the comment. -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH] drm/arm/hdlcd: Simplify IRQ install/uninstall
On Wed, Jun 15, 2022 at 05:11:09PM +0100, Robin Murphy wrote: > Since we no longer need to conform to the structure of the various DRM > IRQ callbacks, we can streamline the code by consolidating the piecemeal > functions and passing around our private data structure directly. We're > also a platform device so should never see IRQ_NOTCONNECTED either. > > Furthermore we can also get rid of all the unnecesary read-modify-write > operations, since on install we know we cleared the whole interrupt mask > before enabling the debug IRQs, and thus on uninstall we're always > clearing everything as well. > > Signed-off-by: Robin Murphy Acked-by: Liviu Dudau Thanks for the cleanup! Best regards, Liviu > --- > drivers/gpu/drm/arm/hdlcd_drv.c | 62 + > 1 file changed, 16 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c > index 1f1171f2f16a..7d6aa9b3b577 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > @@ -41,8 +41,7 @@ > > static irqreturn_t hdlcd_irq(int irq, void *arg) > { > - struct drm_device *drm = arg; > - struct hdlcd_drm_private *hdlcd = drm->dev_private; > + struct hdlcd_drm_private *hdlcd = arg; > unsigned long irq_status; > > irq_status = hdlcd_read(hdlcd, HDLCD_REG_INT_STATUS); > @@ -70,61 +69,32 @@ static irqreturn_t hdlcd_irq(int irq, void *arg) > return IRQ_HANDLED; > } > > -static void hdlcd_irq_preinstall(struct drm_device *drm) > -{ > - struct hdlcd_drm_private *hdlcd = drm->dev_private; > - /* Ensure interrupts are disabled */ > - hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0); > - hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0); > -} > - > -static void hdlcd_irq_postinstall(struct drm_device *drm) > -{ > -#ifdef CONFIG_DEBUG_FS > - struct hdlcd_drm_private *hdlcd = drm->dev_private; > - unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK); > - > - /* enable debug interrupts */ > - irq_mask |= HDLCD_DEBUG_INT_MASK; > - > - hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask); > -#endif > -} > - > -static int hdlcd_irq_install(struct drm_device *drm, int irq) > +static int hdlcd_irq_install(struct hdlcd_drm_private *hdlcd) > { > int ret; > > - if (irq == IRQ_NOTCONNECTED) > - return -ENOTCONN; > + /* Ensure interrupts are disabled */ > + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0); > + hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0); > > - hdlcd_irq_preinstall(drm); > - > - ret = request_irq(irq, hdlcd_irq, 0, drm->driver->name, drm); > + ret = request_irq(hdlcd->irq, hdlcd_irq, 0, "hdlcd", hdlcd); > if (ret) > return ret; > > - hdlcd_irq_postinstall(drm); > +#ifdef CONFIG_DEBUG_FS > + /* enable debug interrupts */ > + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, HDLCD_DEBUG_INT_MASK); > +#endif > > return 0; > } > > -static void hdlcd_irq_uninstall(struct drm_device *drm) > +static void hdlcd_irq_uninstall(struct hdlcd_drm_private *hdlcd) > { > - struct hdlcd_drm_private *hdlcd = drm->dev_private; > /* disable all the interrupts that we might have enabled */ > - unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK); > + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0); > > -#ifdef CONFIG_DEBUG_FS > - /* disable debug interrupts */ > - irq_mask &= ~HDLCD_DEBUG_INT_MASK; > -#endif > - > - /* disable vsync interrupts */ > - irq_mask &= ~HDLCD_INTERRUPT_VSYNC; > - hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask); > - > - free_irq(hdlcd->irq, drm); > + free_irq(hdlcd->irq, hdlcd); > } > > static int hdlcd_load(struct drm_device *drm, unsigned long flags) > @@ -184,7 +154,7 @@ static int hdlcd_load(struct drm_device *drm, unsigned > long flags) > goto irq_fail; > hdlcd->irq = ret; > > - ret = hdlcd_irq_install(drm, hdlcd->irq); > + ret = hdlcd_irq_install(hdlcd); > if (ret < 0) { > DRM_ERROR("failed to install IRQ handler\n"); > goto irq_fail; > @@ -342,7 +312,7 @@ static int hdlcd_drm_bind(struct device *dev) > err_unload: > of_node_put(hdlcd->crtc.port); > hdlcd->crtc.port = NULL; > - hdlcd_irq_uninstall(drm); > + hdlcd_irq_uninstall(hdlcd); > of_reserved_mem_device_release(drm->dev); > err_free: > drm_mode_config_cleanup(drm); > @@ -364,7 +334,7 @@ static void hdlcd_drm_unbind(struct device *dev) > hdlcd->crtc.port = NULL; > pm_runtime_get_sync(dev); > drm_atomic_helper_shutdown(drm); > - hdlcd_irq_uninstall(drm); > + hdlcd_irq_uninstall(hdlcd); > pm_runtime_put(dev); > if (pm_runtime_enabled(dev)) > pm_runtime_disable(dev); > -- > 2.36.1.dirty > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/
Re: [Intel-gfx] [PATCH 01/10] drm/doc: add rfc section for small BAR uapi
On 5/25/22 20:43, Matthew Auld wrote: Add an entry for the new uapi needed for small BAR on DG2+. v2: - Some spelling fixes and other small tweaks. (Akeem & Thomas) - Rework error capture interactions, including no longer needing NEEDS_CPU_ACCESS for objects marked for capture. (Thomas) - Add probed_cpu_visible_size. (Lionel) v3: - Drop the vma query for now. - Add unallocated_cpu_visible_size as part of the region query. - Improve the docs some more, including documenting the expected behaviour on older kernels, since this came up in some offline discussion. v4: - Various improvements all over. (Tvrtko) Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Lionel Landwerlin Cc: Tvrtko Ursulin Cc: Jon Bloomfield Cc: Daniel Vetter Cc: Jordan Justen Cc: Kenneth Graunke Cc: Akeem G Abodunrin Cc: mesa-...@lists.freedesktop.org Acked-by: Tvrtko Ursulin Acked-by: Akeem G Abodunrin --- Documentation/gpu/rfc/i915_small_bar.h | 189 +++ Documentation/gpu/rfc/i915_small_bar.rst | 47 ++ Documentation/gpu/rfc/index.rst | 4 + 3 files changed, 240 insertions(+) create mode 100644 Documentation/gpu/rfc/i915_small_bar.h create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst diff --git a/Documentation/gpu/rfc/i915_small_bar.h b/Documentation/gpu/rfc/i915_small_bar.h new file mode 100644 index ..752bb2ceb399 --- /dev/null +++ b/Documentation/gpu/rfc/i915_small_bar.h @@ -0,0 +1,189 @@ +/** + * struct __drm_i915_memory_region_info - Describes one region as known to the + * driver. + * + * Note this is using both struct drm_i915_query_item and struct drm_i915_query. + * For this new query we are adding the new query id DRM_I915_QUERY_MEMORY_REGIONS + * at &drm_i915_query_item.query_id. + */ +struct __drm_i915_memory_region_info { + /** @region: The class:instance pair encoding */ + struct drm_i915_gem_memory_class_instance region; + + /** @rsvd0: MBZ */ + __u32 rsvd0; + + /** +* @probed_size: Memory probed by the driver (-1 = unknown) +* +* Note that it should not be possible to ever encounter a zero value +* here, also note that no current region type will ever return -1 here. +* Although for future region types, this might be a possibility. The +* same applies to the other size fields. +*/ + __u64 probed_size; + + /** +* @unallocated_size: Estimate of memory remaining (-1 = unknown) +* +* Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable accounting. +* Without this (or if this is an older kernel) the value here will +* always equal the @probed_size. Note this is only currently tracked +* for I915_MEMORY_CLASS_DEVICE regions (for other types the value here +* will always equal the @probed_size). +*/ + __u64 unallocated_size; + + union { + /** @rsvd1: MBZ */ + __u64 rsvd1[8]; + struct { + /** +* @probed_cpu_visible_size: Memory probed by the driver +* that is CPU accessible. (-1 = unknown). +* +* This will be always be <= @probed_size, and the +* remainder (if there is any) will not be CPU +* accessible. +* +* On systems without small BAR, the @probed_size will +* always equal the @probed_cpu_visible_size, since all +* of it will be CPU accessible. +* +* Note this is only tracked for +* I915_MEMORY_CLASS_DEVICE regions (for other types the +* value here will always equal the @probed_size). +* +* Note that if the value returned here is zero, then +* this must be an old kernel which lacks the relevant +* small-bar uAPI support (including +* I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on +* such systems we should never actually end up with a +* small BAR configuration, assuming we are able to load +* the kernel module. Hence it should be safe to treat +* this the same as when @probed_cpu_visible_size == +* @probed_size. +*/ + __u64 probed_cpu_visible_size; + + /** +* @unallocated_cpu_visible_size: Estimate of CPU +* visible memory remaining (-1 = unknown). +* +* Note this is only tracked for +* I915_MEMORY_CLASS_DEVICE
Re: [PATCH v9 00/14] Add some DRM bridge drivers support for i.MX8qm/qxp SoCs
On Sat, Jun 11, 2022 at 10:14:07PM +0800, Liu Ying wrote: > Patch 1/14 and 2/14 add bus formats used by pixel combiner. Thanks! For these: Acked-by: Sakari Ailus -- Sakari Ailus
[PATCH v2 0/9] DG2 VRAM_SR Support
This series add DG2 D3Cold VRAM_SR support. TODO: GuC Interface state save/restore on VRAM_SR entry/exit. Anshuman Gupta (8): drm/i915/dgfx: OpRegion VRAM Self Refresh Support drm/i915/dg1: OpRegion PCON DG1 MBD config support drm/i915/dg2: Add DG2_NB_MBD subplatform drm/i915/dg2: DG2 MBD config drm/i915/dgfx: Add has_lmem_sr drm/i915/dgfx: Setup VRAM SR with D3COLD drm/i915/rpm: Enable D3Cold VRAM SR Support drm/i915/rpm: d3cold Policy Tvrtko Ursulin (1): drm/i915/xehpsdv: Store lmem region in gt drivers/gpu/drm/i915/display/intel_opregion.c | 107 +- drivers/gpu/drm/i915/display/intel_opregion.h | 17 +++ drivers/gpu/drm/i915/gt/intel_gt.c| 1 + drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 + drivers/gpu/drm/i915/i915_driver.c| 49 drivers/gpu/drm/i915/i915_drv.h | 20 drivers/gpu/drm/i915/i915_params.c| 4 + drivers/gpu/drm/i915/i915_params.h| 3 +- drivers/gpu/drm/i915/i915_pci.c | 2 + drivers/gpu/drm/i915/i915_reg.h | 4 + drivers/gpu/drm/i915/intel_device_info.c | 21 drivers/gpu/drm/i915/intel_device_info.h | 12 +- drivers/gpu/drm/i915/intel_pcode.c| 28 + drivers/gpu/drm/i915/intel_pcode.h| 2 + drivers/gpu/drm/i915/intel_pm.c | 43 +++ drivers/gpu/drm/i915/intel_pm.h | 2 + drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +- include/drm/i915_pciids.h | 23 ++-- 18 files changed, 329 insertions(+), 15 deletions(-) -- 2.26.2
[PATCH v2 1/9] drm/i915/dgfx: OpRegion VRAM Self Refresh Support
Intel DGFX cards provides a feature Video Ram Self Refrsh(VRSR). DGFX VRSR can be enabled with runtime suspend D3Cold flow and with opportunistic S0ix system wide suspend flow as well. Without VRSR enablement i915 has to evict the lmem objects to system memory. Depending on some heuristics driver will evict lmem objects without VRSR. VRSR feature requires Host BIOS support, VRSR will be enable/disable by HOST BIOS using ACPI OpRegion. Adding OpRegion VRSR support in order to enable/disable VRSR on discrete cards. BSpec: 53440 Cc: Jani Nikula Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_opregion.c | 62 ++- drivers/gpu/drm/i915/display/intel_opregion.h | 11 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c index 6876ba30d5a9..11d8c5bb23ac 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.c +++ b/drivers/gpu/drm/i915/display/intel_opregion.c @@ -53,6 +53,8 @@ #define MBOX_ASLE_EXT BIT(4) /* Mailbox #5 */ #define MBOX_BACKLIGHT BIT(5) /* Mailbox #2 (valid from v3.x) */ +#define PCON_DGFX_BIOS_SUPPORTS_VRSR BIT(11) +#define PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID BIT(12) #define PCON_HEADLESS_SKU BIT(13) struct opregion_header { @@ -130,7 +132,8 @@ struct opregion_asle { u64 rvda; /* Physical (2.0) or relative from opregion (2.1+) * address of raw VBT data. */ u32 rvds; /* Size of raw vbt data */ - u8 rsvd[58]; + u8 vrsr;/* DGFX Video Ram Self Refresh */ + u8 rsvd[57]; } __packed; /* OpRegion mailbox #5: ASLE ext */ @@ -201,6 +204,9 @@ struct opregion_asle_ext { #define ASLE_PHED_EDID_VALID_MASK 0x3 +/* VRAM SR */ +#define ASLE_VRSR_ENABLE BIT(0) + /* Software System Control Interrupt (SWSCI) */ #define SWSCI_SCIC_INDICATOR (1 << 0) #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT 1 @@ -921,6 +927,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv) opregion->header->over.minor, opregion->header->over.revision); + drm_dbg(&dev_priv->drm, "OpRegion PCON values 0x%x\n", opregion->header->pcon); + mboxes = opregion->header->mboxes; if (mboxes & MBOX_ACPI) { drm_dbg(&dev_priv->drm, "Public ACPI methods supported\n"); @@ -1246,3 +1254,55 @@ void intel_opregion_unregister(struct drm_i915_private *i915) opregion->vbt = NULL; opregion->lid_state = NULL; } + +/** + * intel_opregion_bios_supports_vram_sr() get HOST BIOS VRAM Self + * Refresh capability support. + * @i915: pointer to i915 device. + * + * It checks opregion pcon vram_sr fields to get HOST BIOS vram_sr + * capability support. It is only applocable to DGFX. + * + * Returns: + * true when bios supports vram_sr, or false if bios doesn't support. + */ +bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915) +{ + struct intel_opregion *opregion = &i915->opregion; + + if (!IS_DGFX(i915)) + return false; + + if (!opregion) + return false; + + if (opregion->header->pcon & PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID) + return opregion->header->pcon & PCON_DGFX_BIOS_SUPPORTS_VRSR; + else + return false; +} + +/** + * intel_opregion_vram_sr() - enable/disable VRAM Self Refresh. + * @i915: pointer to i915 device. + * @enable: Argument to enable/disable VRSR. + * + * It enables/disables vram_sr in opregion ASLE MBOX, based upon that + * HOST BIOS will enables and disbales VRAM_SR during + * ACPI _PS3/_OFF and _PS/_ON glue method. + */ +void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable) +{ + struct intel_opregion *opregion = &i915->opregion; + + if (!opregion) + return; + + if (drm_WARN(&i915->drm, !opregion->asle, "ASLE MAILBOX3 is not available\n")) + return; + + if (enable) + opregion->asle->vrsr |= ASLE_VRSR_ENABLE; + else + opregion->asle->vrsr &= ~ASLE_VRSR_ENABLE; +} diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h index 2f261f985400..73c9d81d5ee6 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.h +++ b/drivers/gpu/drm/i915/display/intel_opregion.h @@ -75,6 +75,8 @@ int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv, pci_power_t state); int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv); struct edid *intel_opregion_get_edid(struct intel_connector *connector); +bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915); +void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable); bool intel_opregion_headless_sku(struct drm_i915_private *i915); @
[PATCH v2 2/9] drm/i915/dg1: OpRegion PCON DG1 MBD config support
DGFX cards support both Add in Card(AIC) and Mother Board Down(MBD) configs. MBD config requires HOST BIOS GPIO toggling support in order to enable/disable VRAM SR using ACPI OpRegion. i915 requires to check OpRegion PCON MBD Config bits to discover whether Gfx Card is MBD config before enabling VRSR. BSpec: 53440 Cc: Jani Nikula Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_opregion.c | 43 +++ drivers/gpu/drm/i915/display/intel_opregion.h | 6 +++ 2 files changed, 49 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c index 11d8c5bb23ac..c8cdcde89dfc 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.c +++ b/drivers/gpu/drm/i915/display/intel_opregion.c @@ -53,6 +53,8 @@ #define MBOX_ASLE_EXT BIT(4) /* Mailbox #5 */ #define MBOX_BACKLIGHT BIT(5) /* Mailbox #2 (valid from v3.x) */ +#define PCON_DG1_MBD_CONFIGBIT(9) +#define PCON_DG1_MBD_CONFIG_FIELD_VALIDBIT(10) #define PCON_DGFX_BIOS_SUPPORTS_VRSR BIT(11) #define PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID BIT(12) #define PCON_HEADLESS_SKU BIT(13) @@ -1255,6 +1257,44 @@ void intel_opregion_unregister(struct drm_i915_private *i915) opregion->lid_state = NULL; } +static bool intel_opregion_dg1_mbd_config(struct drm_i915_private *i915) +{ + struct intel_opregion *opregion = &i915->opregion; + + if (!IS_DG1(i915)) + return false; + + if (!opregion) + return false; + + if (opregion->header->pcon & PCON_DG1_MBD_CONFIG_FIELD_VALID) + return opregion->header->pcon & PCON_DG1_MBD_CONFIG; + else + return false; +} + +/** + * intel_opregion_vram_sr_required(). + * @i915 i915 device priv data. + * + * It checks whether a DGFX card is Mother Board Down config depending + * on respective discrete platform. + * + * Returns: + * It returns a boolean whether opregion vram_sr support is required. + */ +bool +intel_opregion_vram_sr_required(struct drm_i915_private *i915) +{ + if (!IS_DGFX(i915)) + return false; + + if (IS_DG1(i915)) + return intel_opregion_dg1_mbd_config(i915); + + return false; +} + /** * intel_opregion_bios_supports_vram_sr() get HOST BIOS VRAM Self * Refresh capability support. @@ -1298,6 +1338,9 @@ void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable) if (!opregion) return; + if (!intel_opregion_vram_sr_required(i915)) + return; + if (drm_WARN(&i915->drm, !opregion->asle, "ASLE MAILBOX3 is not available\n")) return; diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h index 73c9d81d5ee6..ad40c97f9565 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.h +++ b/drivers/gpu/drm/i915/display/intel_opregion.h @@ -77,6 +77,7 @@ int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv); struct edid *intel_opregion_get_edid(struct intel_connector *connector); bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915); void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable); +bool intel_opregion_vram_sr_required(struct drm_i915_private *i915); bool intel_opregion_headless_sku(struct drm_i915_private *i915); @@ -145,6 +146,11 @@ static void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable) { } +static bool intel_opregion_vram_sr_required(struct drm_i915_private *i915) +{ + return false; +} + #endif /* CONFIG_ACPI */ #endif -- 2.26.2
[PATCH v2 3/9] drm/i915/dg2: Add DG2_NB_MBD subplatform
DG2 NB SKU need to distinguish between MBD and AIC to probe the VRAM Self Refresh feature support. Adding those sub platform accordingly. Cc: Matt Roper Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_device_info.c | 21 + drivers/gpu/drm/i915/intel_device_info.h | 11 +++ include/drm/i915_pciids.h| 23 --- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a5bc6a774c5a..f1f8699eedfd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1007,10 +1007,13 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define IS_PONTEVECCHIO(dev_priv) IS_PLATFORM(dev_priv, INTEL_PONTEVECCHIO) #define IS_DG2_G10(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10) #define IS_DG2_G11(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11) #define IS_DG2_G12(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12) #define IS_ADLS_RPLS(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index f0bf23726ed8..93da555adc4e 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -187,6 +187,18 @@ static const u16 subplatform_rpl_ids[] = { INTEL_RPLP_IDS(0), }; +static const u16 subplatform_g10_mb_mbd_ids[] = { + INTEL_DG2_G10_NB_MBD_IDS(0), +}; + +static const u16 subplatform_g11_mb_mbd_ids[] = { + INTEL_DG2_G11_NB_MBD_IDS(0), +}; + +static const u16 subplatform_g12_mb_mbd_ids[] = { + INTEL_DG2_G12_NB_MBD_IDS(0), +}; + static const u16 subplatform_g10_ids[] = { INTEL_DG2_G10_IDS(0), INTEL_ATS_M150_IDS(0), @@ -246,6 +258,15 @@ 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_g10_mb_mbd_ids, + ARRAY_SIZE(subplatform_g10_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G10_NB_MBD); + } else if (find_devid(devid, subplatform_g11_mb_mbd_ids, + ARRAY_SIZE(subplatform_g11_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G11_NB_MBD); + } else if (find_devid(devid, subplatform_g12_mb_mbd_ids, + ARRAY_SIZE(subplatform_g12_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G12_NB_MBD); } 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 08341174ee0a..c929e2d7e59c 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -97,7 +97,7 @@ enum intel_platform { * it is fine for the same bit to be used on multiple parent platforms. */ -#define INTEL_SUBPLATFORM_BITS (3) +#define INTEL_SUBPLATFORM_BITS (6) #define INTEL_SUBPLATFORM_MASK (BIT(INTEL_SUBPLATFORM_BITS) - 1) /* HSW/BDW/SKL/KBL/CFL */ @@ -111,9 +111,12 @@ enum intel_platform { #define INTEL_SUBPLATFORM_UY (0) /* DG2 */ -#define INTEL_SUBPLATFORM_G10 0 -#define INTEL_SUBPLATFORM_G11 1 -#define INTEL_SUBPLATFORM_G12 2 +#define INTEL_SUBPLATFORM_G10_NB_MBD 0 +#define INTEL_SUBPLATFORM_G11_NB_MBD 1 +#define INTEL_SUBPLATFORM_G12_NB_MBD 2 +#define INTEL_SUBPLATFORM_G10 3 +#define INTEL_SUBPLATFORM_G11 4 +#define INTEL_SUBPLATFORM_G12 5 /* ADL */ #define INTEL_SUBPLATFORM_RPL 0 diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 4585fed4e41e..198be417bb2d 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -693,32 +693,41 @@ INTEL_VGA_DEVICE(0xA7A9, info) /* DG2 */ -#define INTEL_DG2_G10_IDS(info) \ +#define INTEL_DG2_G10_NB_MBD_IDS(info) \ INTEL_VGA_DEVICE(0x5690, info), \ INTEL_VGA_DEVICE(0x5691, info), \ - INTEL_VGA_DEVICE(0x5692, info), \ + INTEL_VGA_DEVICE(0x5692, info) + +#define INTEL_DG2_G11_NB_MBD_IDS(info) \ + INTEL_VGA_DEVICE(0x5693, info), \ + INTEL_VGA_DEVICE(0x5694, info), \ + INTEL_VGA_DEVICE(0x5695, info) + +#define INTEL_DG2_G12_NB_MBD_IDS(info) \ + INTEL_VGA_DEVICE(0x5696, info), \ + INTEL_VGA_DEVICE(0x5697, info)
[PATCH v2 5/9] drm/i915/dgfx: Add has_lmem_sr
Add has_lmem_sr platform specific flag to know, whether platform has VRAM self refresh support. As of now both DG1 and DG2 client platforms supports VRAM self refresh with D3Cold but let it enable first on DG2 as primary lead platform for D3Cold support. Let it get enable on DG1 once this feature is stable. Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_pci.c | 2 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + 3 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 28eee8088822..7983b36c1720 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1313,6 +1313,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) #define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM) +#define HAS_LMEM_SR(i915) (INTEL_INFO(i915)->has_lmem_sr) /* * Platform has the dedicated compression control state for each lmem surfaces diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 5e51fc29bb8b..04aad54033dd 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -917,6 +917,7 @@ static const struct intel_device_info dg1_info = { DGFX_FEATURES, .graphics.rel = 10, PLATFORM(INTEL_DG1), + .has_lmem_sr = 0, .display.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C) | BIT(PIPE_D), .require_force_probe = 1, .platform_engine_mask = @@ -1074,6 +1075,7 @@ static const struct intel_device_info xehpsdv_info = { static const struct intel_device_info dg2_info = { DG2_FEATURES, XE_LPD_FEATURES, + .has_lmem_sr = 1, .display.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C) | BIT(TRANSCODER_D), .require_force_probe = 1, diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index c929e2d7e59c..db51cdb9e09a 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -157,6 +157,7 @@ enum intel_ppgtt_type { func(has_l3_ccs_read); \ func(has_l3_dpf); \ func(has_llc); \ + func(has_lmem_sr); \ func(has_logical_ring_contexts); \ func(has_logical_ring_elsq); \ func(has_media_ratio_mode); \ -- 2.26.2
[PATCH v2 4/9] drm/i915/dg2: DG2 MBD config
Add DG2 Motherboard Down Config check support. v2: - Don't use pciid to check DG2 MBD. [Jani] BSpec: 44477 Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_opregion.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 9 + 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c index c8cdcde89dfc..50dcd6d3558e 100644 --- a/drivers/gpu/drm/i915/display/intel_opregion.c +++ b/drivers/gpu/drm/i915/display/intel_opregion.c @@ -1291,6 +1291,8 @@ intel_opregion_vram_sr_required(struct drm_i915_private *i915) if (IS_DG1(i915)) return intel_opregion_dg1_mbd_config(i915); + else if (IS_DG2_MBD(i915)) + return true; return false; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f1f8699eedfd..28eee8088822 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1006,6 +1006,12 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define IS_DG2(dev_priv) IS_PLATFORM(dev_priv, INTEL_DG2) #define IS_PONTEVECCHIO(dev_priv) IS_PLATFORM(dev_priv, INTEL_PONTEVECCHIO) +#define IS_DG2_G10_NB_MBD(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10_NB_MBD) +#define IS_DG2_G11_NB_MBD(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11_NB_MBD) +#define IS_DG2_G12_NB_MBD(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12_NB_MBD) #define IS_DG2_G10(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10) @@ -1015,6 +1021,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define IS_DG2_G12(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12) + +#define IS_DG2_MBD(dev_priv) (IS_DG2_G10_NB_MBD(dev_priv) || IS_DG2_G11_NB_MBD(dev_priv) || \ + IS_DG2_G12_NB_MBD(dev_priv)) #define IS_ADLS_RPLS(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL) #define IS_ADLP_N(dev_priv) \ -- 2.26.2
[PATCH v2 6/9] drm/i915/dgfx: Setup VRAM SR with D3COLD
Setup VRAM Self Refresh with D3COLD state. VRAM Self Refresh will retain the context of VRAM, driver need to save any corresponding hardware state that needs to be restore on D3COLD exit, example PCI state. Cc: Jani Nikula Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_driver.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h| 7 + drivers/gpu/drm/i915/i915_reg.h| 4 +++ drivers/gpu/drm/i915/intel_pcode.c | 28 +++ drivers/gpu/drm/i915/intel_pcode.h | 2 ++ drivers/gpu/drm/i915/intel_pm.c| 43 ++ drivers/gpu/drm/i915/intel_pm.h| 2 ++ 7 files changed, 88 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index d26dcca7e654..aa1fb15b1f11 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -649,6 +649,8 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv) if (ret) goto err_msi; + intel_pm_vram_sr_setup(dev_priv); + /* * Fill the dram structure to get the system dram info. This will be * used for memory latency calculation. diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7983b36c1720..09f53aeda8d0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -624,6 +624,13 @@ struct drm_i915_private { u32 bxt_phy_grc; u32 suspend_count; + + struct { + /* lock to protect vram_sr flags */ + struct mutex lock; + bool supported; + } vram_sr; + struct i915_suspend_saved_registers regfile; struct vlv_s0ix_state *vlv_s0ix_state; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 932bd6aa4a0a..0e3dc4a8846a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6766,6 +6766,8 @@ #define DG1_PCODE_STATUS 0x7E #define DG1_UNCORE_GET_INIT_STATUS 0x0 #define DG1_UNCORE_INIT_STATUS_COMPLETE0x1 +#define DG1_PCODE_D3_VRAM_SR 0x71 +#define DG1_ENABLE_SR0x1 #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US0x23 #define XEHPSDV_PCODE_FREQUENCY_CONFIG 0x6e/* xehpsdv, pvc */ /* XEHPSDV_PCODE_FREQUENCY_CONFIG sub-commands (param1) */ @@ -6779,6 +6781,8 @@ #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8 #define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16 #define GEN6_PCODE_DATA1 _MMIO(0x13812C) +#define VRAM_CAPABILITY _MMIO(0x138144) +#define VRAM_SUPPORTEDREG_BIT(0) /* IVYBRIDGE DPF */ #define GEN7_L3CDERRST1(slice) _MMIO(0xB008 + (slice) * 0x200) /* L3CD Error Status 1 */ diff --git a/drivers/gpu/drm/i915/intel_pcode.c b/drivers/gpu/drm/i915/intel_pcode.c index a234d9b4ed14..88bd1f44cfb2 100644 --- a/drivers/gpu/drm/i915/intel_pcode.c +++ b/drivers/gpu/drm/i915/intel_pcode.c @@ -246,3 +246,31 @@ int snb_pcode_write_p(struct intel_uncore *uncore, u32 mbcmd, u32 p1, u32 p2, u3 return err; } + +/** + * intel_pcode_enable_vram_sr - Enable pcode vram_sr. + * @dev_priv: i915 device + * + * This function triggers the required pcode flow to enable vram_sr. + * This function stictly need to call from rpm handlers, as i915 is + * transitioning to rpm idle/suspend, it doesn't require to grab + * rpm wakeref. + * + * Returns: + * returns returned value from pcode mbox write. + */ +int intel_pcode_enable_vram_sr(struct drm_i915_private *i915) +{ + int ret = 0; + + if (!HAS_LMEM_SR(i915)) + return ret; + + ret = snb_pcode_write(&i915->uncore, + REG_FIELD_PREP(GEN6_PCODE_MB_COMMAND, + DG1_PCODE_D3_VRAM_SR) | + REG_FIELD_PREP(GEN6_PCODE_MB_PARAM1, + DG1_ENABLE_SR), 0); /* no data needed for this cmd */ + + return ret; +} diff --git a/drivers/gpu/drm/i915/intel_pcode.h b/drivers/gpu/drm/i915/intel_pcode.h index 8d2198e29422..295594514d49 100644 --- a/drivers/gpu/drm/i915/intel_pcode.h +++ b/drivers/gpu/drm/i915/intel_pcode.h @@ -9,6 +9,7 @@ #include struct intel_uncore; +struct drm_i915_private; int snb_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32 *val1); int snb_pcode_write_timeout(struct intel_uncore *uncore, u32 mbox, u32 val, @@ -26,5 +27,6 @@ int intel_pcode_init(struct intel_uncore *uncore); */ int snb_pcode_read_p(struct intel_uncore *uncore, u32 mbcmd, u32 p1, u32 p2, u32 *val); int snb_pcode_write_p(struct intel_uncore *uncore, u32 mbcmd, u32 p1, u32 p2, u32 val); +int intel_pcode_enable_vram_sr(struct drm_i915_private *i915); #endif /* _INTEL_PCODE_H */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5a61fc3f26c1..299fbc5375a9 100644 --
[PATCH v2 7/9] drm/i915/rpm: Enable D3Cold VRAM SR Support
Intel Client DGFX card supports D3Cold with two option. D3Cold-off zero watt, D3Cold-VRAM Self Refresh. i915 requires to evict the lmem objects to smem in order to support D3Cold-Off, which increases i915 the suspend/resume latency. Enabling VRAM Self Refresh feature optimize the latency with additional power cost which required to retain the lmem. Adding intel_runtime_idle (runtime_idle callback) to enable VRAM_SR, it will be used for policy to choose between D3Cold-off vs D3Cold-VRAM_SR. Since we have introduced i915 runtime_idle callback. It need to be warranted that Runtime PM Core invokes runtime_idle callback when runtime usages count becomes zero. That requires to use pm_runtime_put instead of pm_runtime_put_autosuspend. TODO: GuC interface state save/restore. Cc: Rodrigo Vivi Cc: Chris Wilson Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_driver.c | 26 + drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index aa1fb15b1f11..fcff5f3fe05e 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -1557,6 +1557,31 @@ static int i915_pm_restore(struct device *kdev) return i915_pm_resume(kdev); } +static int intel_runtime_idle(struct device *kdev) +{ + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); + int ret = 1; + + if (!HAS_LMEM_SR(dev_priv)) { + /*TODO: Prepare for D3Cold-Off */ + goto out; + } + + disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + + ret = intel_pm_vram_sr(dev_priv, true); + if (!ret) + drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); + + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + +out: + pm_runtime_mark_last_busy(kdev); + pm_runtime_autosuspend(kdev); + + return ret; +} + static int intel_runtime_suspend(struct device *kdev) { struct drm_i915_private *dev_priv = kdev_to_i915(kdev); @@ -1742,6 +1767,7 @@ const struct dev_pm_ops i915_pm_ops = { .restore = i915_pm_restore, /* S0ix (via runtime suspend) event handlers */ + .runtime_idle = intel_runtime_idle, .runtime_suspend = intel_runtime_suspend, .runtime_resume = intel_runtime_resume, }; diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 6ed5786bcd29..4dade7e8a795 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -492,8 +492,7 @@ static void __intel_runtime_pm_put(struct intel_runtime_pm *rpm, intel_runtime_pm_release(rpm, wakelock); - pm_runtime_mark_last_busy(kdev); - pm_runtime_put_autosuspend(kdev); + pm_runtime_put(kdev); } /** -- 2.26.2
[PATCH v2 9/9] drm/i915/rpm: d3cold Policy
Add d3cold_sr_lmem_threshold modparam to choose between d3cold-off zero watt and d3cold-VRAM Self Refresh. i915 requires to evict the lmem objects to smem in order to support d3cold-Off. If gfx root port is not capable of sending PME from d3cold then i915 don't need to program d3cold-off/d3cold-vram_sr sequence. FIXME: Eviction of lmem objects in case of D3Cold off is wip. Cc: Rodrigo Vivi Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_driver.c | 27 --- drivers/gpu/drm/i915/i915_params.c | 4 drivers/gpu/drm/i915/i915_params.h | 3 ++- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index fcff5f3fe05e..aef4b17efdbe 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -1560,15 +1560,36 @@ static int i915_pm_restore(struct device *kdev) static int intel_runtime_idle(struct device *kdev) { struct drm_i915_private *dev_priv = kdev_to_i915(kdev); + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); + u64 lmem_total = to_gt(dev_priv)->lmem->total; + u64 lmem_avail = to_gt(dev_priv)->lmem->avail; + u64 lmem_used = lmem_total - lmem_avail; + struct pci_dev *root_pdev; int ret = 1; - if (!HAS_LMEM_SR(dev_priv)) { - /*TODO: Prepare for D3Cold-Off */ + root_pdev = pcie_find_root_port(pdev); + if (!root_pdev) + goto out; + + if (!pci_pme_capable(root_pdev, PCI_D3cold)) goto out; - } disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + if (lmem_used < dev_priv->params.d3cold_sr_lmem_threshold * 1024 * 1024) { + drm_dbg(&dev_priv->drm, "Prepare for D3Cold off\n"); + pci_d3cold_enable(root_pdev); + /* FIXME: Eviction of lmem objects and guc reset is wip */ + intel_pm_vram_sr(dev_priv, false); + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + goto out; + } else if (!HAS_LMEM_SR(dev_priv)) { + /* Disable D3Cold to reduce the eviction latency */ + pci_d3cold_disable(root_pdev); + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + goto out; + } + ret = intel_pm_vram_sr(dev_priv, true); if (!ret) drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 701fbc98afa0..6c6b3c372d4d 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -197,6 +197,10 @@ i915_param_named(enable_gvt, bool, 0400, "Enable support for Intel GVT-g graphics virtualization host support(default:false)"); #endif +i915_param_named_unsafe(d3cold_sr_lmem_threshold, int, 0400, + "Enable Vidoe RAM Self refresh when size of lmem is greater to this threshold. " + "It helps to optimize the suspend/resume latecy. (default: 300mb)"); + #if CONFIG_DRM_I915_REQUEST_TIMEOUT i915_param_named_unsafe(request_timeout_ms, uint, 0600, "Default request/fence/batch buffer expiration timeout."); diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index b5e7ea45d191..28f20ebaf41f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -83,7 +83,8 @@ struct drm_printer; param(bool, verbose_state_checks, true, 0) \ param(bool, nuclear_pageflip, false, 0400) \ param(bool, enable_dp_mst, true, 0600) \ - param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) + param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) \ + param(int, d3cold_sr_lmem_threshold, 300, 0600) \ #define MEMBER(T, member, ...) T member; struct i915_params { -- 2.26.2
[PATCH v2 8/9] drm/i915/xehpsdv: Store lmem region in gt
From: Tvrtko Ursulin Store a pointer to respective local memory region in intel_gt so it can be used when memory local to a GT needs to be allocated. Cc: Andi Shyti Signed-off-by: Tvrtko Ursulin Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/gt/intel_gt.c | 1 + drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index f33290358c51..7a535f670ae1 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -91,6 +91,7 @@ static int intel_gt_probe_lmem(struct intel_gt *gt) GEM_BUG_ON(!HAS_REGION(i915, id)); GEM_BUG_ON(i915->mm.regions[id]); i915->mm.regions[id] = mem; + gt->lmem = mem; return 0; } diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index df708802889d..cd7744eaaeaa 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -23,6 +23,7 @@ #include "intel_gt_buffer_pool_types.h" #include "intel_hwconfig.h" #include "intel_llc_types.h" +#include "intel_memory_region.h" #include "intel_reset_types.h" #include "intel_rc6_types.h" #include "intel_rps_types.h" @@ -202,6 +203,8 @@ struct intel_gt { */ phys_addr_t phys_addr; + struct intel_memory_region *lmem; + struct intel_gt_info { unsigned int id; -- 2.26.2
Re: [Intel-gfx] [PATCH v2 3/9] drm/i915/dg2: Add DG2_NB_MBD subplatform
On 16/06/2022 13:01, Anshuman Gupta wrote: DG2 NB SKU need to distinguish between MBD and AIC to probe the VRAM Self Refresh feature support. Adding those sub platform accordingly. Cc: Matt Roper Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_device_info.c | 21 + drivers/gpu/drm/i915/intel_device_info.h | 11 +++ include/drm/i915_pciids.h| 23 --- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a5bc6a774c5a..f1f8699eedfd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1007,10 +1007,13 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define IS_PONTEVECCHIO(dev_priv) IS_PLATFORM(dev_priv, INTEL_PONTEVECCHIO) #define IS_DG2_G10(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10) #define IS_DG2_G11(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11) #define IS_DG2_G12(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12) #define IS_ADLS_RPLS(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index f0bf23726ed8..93da555adc4e 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -187,6 +187,18 @@ static const u16 subplatform_rpl_ids[] = { INTEL_RPLP_IDS(0), }; +static const u16 subplatform_g10_mb_mbd_ids[] = { + INTEL_DG2_G10_NB_MBD_IDS(0), +}; + +static const u16 subplatform_g11_mb_mbd_ids[] = { + INTEL_DG2_G11_NB_MBD_IDS(0), +}; + +static const u16 subplatform_g12_mb_mbd_ids[] = { + INTEL_DG2_G12_NB_MBD_IDS(0), +}; + static const u16 subplatform_g10_ids[] = { INTEL_DG2_G10_IDS(0), INTEL_ATS_M150_IDS(0), @@ -246,6 +258,15 @@ 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_g10_mb_mbd_ids, + ARRAY_SIZE(subplatform_g10_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G10_NB_MBD); + } else if (find_devid(devid, subplatform_g11_mb_mbd_ids, + ARRAY_SIZE(subplatform_g11_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G11_NB_MBD); + } else if (find_devid(devid, subplatform_g12_mb_mbd_ids, + ARRAY_SIZE(subplatform_g12_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G12_NB_MBD); } 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 08341174ee0a..c929e2d7e59c 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -97,7 +97,7 @@ enum intel_platform { * it is fine for the same bit to be used on multiple parent platforms. */ -#define INTEL_SUBPLATFORM_BITS (3) +#define INTEL_SUBPLATFORM_BITS (6) #define INTEL_SUBPLATFORM_MASK (BIT(INTEL_SUBPLATFORM_BITS) - 1) /* HSW/BDW/SKL/KBL/CFL */ @@ -111,9 +111,12 @@ enum intel_platform { #define INTEL_SUBPLATFORM_UY (0) /* DG2 */ -#define INTEL_SUBPLATFORM_G10 0 -#define INTEL_SUBPLATFORM_G11 1 -#define INTEL_SUBPLATFORM_G12 2 +#define INTEL_SUBPLATFORM_G10_NB_MBD 0 +#define INTEL_SUBPLATFORM_G11_NB_MBD 1 +#define INTEL_SUBPLATFORM_G12_NB_MBD 2 +#define INTEL_SUBPLATFORM_G10 3 +#define INTEL_SUBPLATFORM_G11 4 +#define INTEL_SUBPLATFORM_G12 5 Ugh I feel this "breaks" the subplatform idea.. feels like it is just too many bits when two separate sets of information get tracked (Gxx plus MBD). How about a separate "is_mbd" flag in runtime_info? You can split the PCI IDs split as you have done, but do a search against the MBD ones and set the flag. Regards, Tvrtko /* ADL */ #define INTEL_SUBPLATFORM_RPL 0 diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 4585fed4e41e..198be417bb2d 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -693,32 +693,41 @@ INTEL_VGA_DEVICE(0xA7A9, info) /* DG2 */ -#define INTEL_DG2_G10_IDS(info) \ +#define INTEL_DG2_G10_NB_MBD_IDS(info) \ INTEL_VGA_DEVICE(0x5690, info), \
Re: [PATCH v2 6/9] drm/i915/dgfx: Setup VRAM SR with D3COLD
On Thu, 16 Jun 2022, Anshuman Gupta wrote: > Setup VRAM Self Refresh with D3COLD state. > VRAM Self Refresh will retain the context of VRAM, driver > need to save any corresponding hardware state that needs > to be restore on D3COLD exit, example PCI state. > > Cc: Jani Nikula > Cc: Rodrigo Vivi > Signed-off-by: Anshuman Gupta > --- > drivers/gpu/drm/i915/i915_driver.c | 2 ++ > drivers/gpu/drm/i915/i915_drv.h| 7 + > drivers/gpu/drm/i915/i915_reg.h| 4 +++ > drivers/gpu/drm/i915/intel_pcode.c | 28 +++ > drivers/gpu/drm/i915/intel_pcode.h | 2 ++ > drivers/gpu/drm/i915/intel_pm.c| 43 ++ > drivers/gpu/drm/i915/intel_pm.h| 2 ++ > 7 files changed, 88 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c > b/drivers/gpu/drm/i915/i915_driver.c > index d26dcca7e654..aa1fb15b1f11 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -649,6 +649,8 @@ static int i915_driver_hw_probe(struct drm_i915_private > *dev_priv) > if (ret) > goto err_msi; > > + intel_pm_vram_sr_setup(dev_priv); > + > /* >* Fill the dram structure to get the system dram info. This will be >* used for memory latency calculation. > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 7983b36c1720..09f53aeda8d0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -624,6 +624,13 @@ struct drm_i915_private { > u32 bxt_phy_grc; > > u32 suspend_count; > + > + struct { > + /* lock to protect vram_sr flags */ > + struct mutex lock; > + bool supported; > + } vram_sr; > + > struct i915_suspend_saved_registers regfile; > struct vlv_s0ix_state *vlv_s0ix_state; > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 932bd6aa4a0a..0e3dc4a8846a 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6766,6 +6766,8 @@ > #define DG1_PCODE_STATUS 0x7E > #define DG1_UNCORE_GET_INIT_STATUS 0x0 > #define DG1_UNCORE_INIT_STATUS_COMPLETE 0x1 > +#define DG1_PCODE_D3_VRAM_SR 0x71 > +#define DG1_ENABLE_SR0x1 > #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US 0x23 > #define XEHPSDV_PCODE_FREQUENCY_CONFIG 0x6e/* xehpsdv, pvc > */ > /* XEHPSDV_PCODE_FREQUENCY_CONFIG sub-commands (param1) */ > @@ -6779,6 +6781,8 @@ > #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8 > #define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16 > #define GEN6_PCODE_DATA1 _MMIO(0x13812C) > +#define VRAM_CAPABILITY _MMIO(0x138144) > +#define VRAM_SUPPORTEDREG_BIT(0) > > /* IVYBRIDGE DPF */ > #define GEN7_L3CDERRST1(slice) _MMIO(0xB008 + (slice) * 0x200) > /* L3CD Error Status 1 */ > diff --git a/drivers/gpu/drm/i915/intel_pcode.c > b/drivers/gpu/drm/i915/intel_pcode.c > index a234d9b4ed14..88bd1f44cfb2 100644 > --- a/drivers/gpu/drm/i915/intel_pcode.c > +++ b/drivers/gpu/drm/i915/intel_pcode.c > @@ -246,3 +246,31 @@ int snb_pcode_write_p(struct intel_uncore *uncore, u32 > mbcmd, u32 p1, u32 p2, u3 > > return err; > } > + > +/** > + * intel_pcode_enable_vram_sr - Enable pcode vram_sr. > + * @dev_priv: i915 device > + * > + * This function triggers the required pcode flow to enable vram_sr. > + * This function stictly need to call from rpm handlers, as i915 is > + * transitioning to rpm idle/suspend, it doesn't require to grab > + * rpm wakeref. > + * > + * Returns: > + * returns returned value from pcode mbox write. > + */ > +int intel_pcode_enable_vram_sr(struct drm_i915_private *i915) > +{ > + int ret = 0; > + > + if (!HAS_LMEM_SR(i915)) > + return ret; > + > + ret = snb_pcode_write(&i915->uncore, > + REG_FIELD_PREP(GEN6_PCODE_MB_COMMAND, > + DG1_PCODE_D3_VRAM_SR) | > + REG_FIELD_PREP(GEN6_PCODE_MB_PARAM1, > + DG1_ENABLE_SR), 0); /* no data needed for this > cmd */ > + > + return ret; > +} This function doesn't belong here. intel_pcode.c provides the *mechanisms* for pcode access, not specific stuff like this. Just put this near the use in intel_pm.c I think. > diff --git a/drivers/gpu/drm/i915/intel_pcode.h > b/drivers/gpu/drm/i915/intel_pcode.h > index 8d2198e29422..295594514d49 100644 > --- a/drivers/gpu/drm/i915/intel_pcode.h > +++ b/drivers/gpu/drm/i915/intel_pcode.h > @@ -9,6 +9,7 @@ > #include > > struct intel_uncore; > +struct drm_i915_private; > > int snb_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32 > *val1); > int snb_pcode_write_timeout(struct intel_uncore *uncore, u32 mbox, u32 val, > @@ -26,5 +27,6 @@ int intel_pcode_init(struct
Re: [PATCH v2 1/9] drm/i915/dgfx: OpRegion VRAM Self Refresh Support
On Thu, 16 Jun 2022, Anshuman Gupta wrote: > Intel DGFX cards provides a feature Video Ram Self Refrsh(VRSR). > DGFX VRSR can be enabled with runtime suspend D3Cold flow and with > opportunistic S0ix system wide suspend flow as well. > > Without VRSR enablement i915 has to evict the lmem objects to > system memory. Depending on some heuristics driver will evict > lmem objects without VRSR. > > VRSR feature requires Host BIOS support, VRSR will be enable/disable > by HOST BIOS using ACPI OpRegion. > > Adding OpRegion VRSR support in order to enable/disable > VRSR on discrete cards. > > BSpec: 53440 > Cc: Jani Nikula > Cc: Rodrigo Vivi > Signed-off-by: Anshuman Gupta > --- > drivers/gpu/drm/i915/display/intel_opregion.c | 62 ++- > drivers/gpu/drm/i915/display/intel_opregion.h | 11 > 2 files changed, 72 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c > b/drivers/gpu/drm/i915/display/intel_opregion.c > index 6876ba30d5a9..11d8c5bb23ac 100644 > --- a/drivers/gpu/drm/i915/display/intel_opregion.c > +++ b/drivers/gpu/drm/i915/display/intel_opregion.c > @@ -53,6 +53,8 @@ > #define MBOX_ASLE_EXTBIT(4) /* Mailbox #5 */ > #define MBOX_BACKLIGHT BIT(5) /* Mailbox #2 (valid from v3.x) > */ > > +#define PCON_DGFX_BIOS_SUPPORTS_VRSR BIT(11) > +#define PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID BIT(12) > #define PCON_HEADLESS_SKUBIT(13) > > struct opregion_header { > @@ -130,7 +132,8 @@ struct opregion_asle { > u64 rvda; /* Physical (2.0) or relative from opregion (2.1+) >* address of raw VBT data. */ > u32 rvds; /* Size of raw vbt data */ > - u8 rsvd[58]; > + u8 vrsr;/* DGFX Video Ram Self Refresh */ > + u8 rsvd[57]; > } __packed; > > /* OpRegion mailbox #5: ASLE ext */ > @@ -201,6 +204,9 @@ struct opregion_asle_ext { > > #define ASLE_PHED_EDID_VALID_MASK0x3 > > +/* VRAM SR */ > +#define ASLE_VRSR_ENABLE BIT(0) > + > /* Software System Control Interrupt (SWSCI) */ > #define SWSCI_SCIC_INDICATOR (1 << 0) > #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT 1 > @@ -921,6 +927,8 @@ int intel_opregion_setup(struct drm_i915_private > *dev_priv) > opregion->header->over.minor, > opregion->header->over.revision); > > + drm_dbg(&dev_priv->drm, "OpRegion PCON values 0x%x\n", > opregion->header->pcon); > + > mboxes = opregion->header->mboxes; > if (mboxes & MBOX_ACPI) { > drm_dbg(&dev_priv->drm, "Public ACPI methods supported\n"); > @@ -1246,3 +1254,55 @@ void intel_opregion_unregister(struct drm_i915_private > *i915) > opregion->vbt = NULL; > opregion->lid_state = NULL; > } > + > +/** > + * intel_opregion_bios_supports_vram_sr() get HOST BIOS VRAM Self > + * Refresh capability support. > + * @i915: pointer to i915 device. > + * > + * It checks opregion pcon vram_sr fields to get HOST BIOS vram_sr > + * capability support. It is only applocable to DGFX. > + * > + * Returns: > + * true when bios supports vram_sr, or false if bios doesn't support. > + */ > +bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915) > +{ > + struct intel_opregion *opregion = &i915->opregion; > + > + if (!IS_DGFX(i915)) > + return false; > + > + if (!opregion) This is always true. You should check for !opregion->header. > + return false; > + > + if (opregion->header->pcon & PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID) > + return opregion->header->pcon & PCON_DGFX_BIOS_SUPPORTS_VRSR; > + else > + return false; > +} > + > +/** > + * intel_opregion_vram_sr() - enable/disable VRAM Self Refresh. > + * @i915: pointer to i915 device. > + * @enable: Argument to enable/disable VRSR. > + * > + * It enables/disables vram_sr in opregion ASLE MBOX, based upon that > + * HOST BIOS will enables and disbales VRAM_SR during > + * ACPI _PS3/_OFF and _PS/_ON glue method. > + */ > +void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable) > +{ > + struct intel_opregion *opregion = &i915->opregion; > + > + if (!opregion) Same as above. > + return; > + > + if (drm_WARN(&i915->drm, !opregion->asle, "ASLE MAILBOX3 is not > available\n")) > + return; I'd just bundle !opregion->asle into the early return. > + > + if (enable) > + opregion->asle->vrsr |= ASLE_VRSR_ENABLE; > + else > + opregion->asle->vrsr &= ~ASLE_VRSR_ENABLE; > +} > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h > b/drivers/gpu/drm/i915/display/intel_opregion.h > index 2f261f985400..73c9d81d5ee6 100644 > --- a/drivers/gpu/drm/i915/display/intel_opregion.h > +++ b/drivers/gpu/drm/i915/display/intel_opregion.h > @@ -75,6 +75,8 @@ int intel_opregion_notify_adapter(struct drm_i915_private > *dev_priv, >
Re: [PATCH v2 2/9] drm/i915/dg1: OpRegion PCON DG1 MBD config support
On Thu, 16 Jun 2022, Anshuman Gupta wrote: > DGFX cards support both Add in Card(AIC) and Mother Board Down(MBD) > configs. MBD config requires HOST BIOS GPIO toggling support > in order to enable/disable VRAM SR using ACPI OpRegion. > > i915 requires to check OpRegion PCON MBD Config bits to > discover whether Gfx Card is MBD config before enabling > VRSR. > > BSpec: 53440 > Cc: Jani Nikula > Cc: Rodrigo Vivi > Signed-off-by: Anshuman Gupta > --- > drivers/gpu/drm/i915/display/intel_opregion.c | 43 +++ > drivers/gpu/drm/i915/display/intel_opregion.h | 6 +++ > 2 files changed, 49 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c > b/drivers/gpu/drm/i915/display/intel_opregion.c > index 11d8c5bb23ac..c8cdcde89dfc 100644 > --- a/drivers/gpu/drm/i915/display/intel_opregion.c > +++ b/drivers/gpu/drm/i915/display/intel_opregion.c > @@ -53,6 +53,8 @@ > #define MBOX_ASLE_EXTBIT(4) /* Mailbox #5 */ > #define MBOX_BACKLIGHT BIT(5) /* Mailbox #2 (valid from v3.x) > */ > > +#define PCON_DG1_MBD_CONFIG BIT(9) > +#define PCON_DG1_MBD_CONFIG_FIELD_VALID BIT(10) > #define PCON_DGFX_BIOS_SUPPORTS_VRSR BIT(11) > #define PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID BIT(12) > #define PCON_HEADLESS_SKUBIT(13) > @@ -1255,6 +1257,44 @@ void intel_opregion_unregister(struct drm_i915_private > *i915) > opregion->lid_state = NULL; > } > > +static bool intel_opregion_dg1_mbd_config(struct drm_i915_private *i915) > +{ > + struct intel_opregion *opregion = &i915->opregion; > + > + if (!IS_DG1(i915)) > + return false; > + > + if (!opregion) Like in previous patch, opregion is always non-NULL. Check for !opregion->header. > + return false; > + > + if (opregion->header->pcon & PCON_DG1_MBD_CONFIG_FIELD_VALID) > + return opregion->header->pcon & PCON_DG1_MBD_CONFIG; > + else > + return false; > +} > + > +/** > + * intel_opregion_vram_sr_required(). > + * @i915 i915 device priv data. > + * > + * It checks whether a DGFX card is Mother Board Down config depending > + * on respective discrete platform. > + * > + * Returns: > + * It returns a boolean whether opregion vram_sr support is required. > + */ > +bool > +intel_opregion_vram_sr_required(struct drm_i915_private *i915) > +{ > + if (!IS_DGFX(i915)) > + return false; > + > + if (IS_DG1(i915)) > + return intel_opregion_dg1_mbd_config(i915); Only check for IS_DG1() here or in the function being called, not both. > + > + return false; > +} > + > /** > * intel_opregion_bios_supports_vram_sr() get HOST BIOS VRAM Self > * Refresh capability support. > @@ -1298,6 +1338,9 @@ void intel_opregion_vram_sr(struct drm_i915_private > *i915, bool enable) > if (!opregion) > return; > > + if (!intel_opregion_vram_sr_required(i915)) > + return; Feels like maybe this patch should be combined with the previous patch due to this dependency. > + > if (drm_WARN(&i915->drm, !opregion->asle, "ASLE MAILBOX3 is not > available\n")) > return; > > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h > b/drivers/gpu/drm/i915/display/intel_opregion.h > index 73c9d81d5ee6..ad40c97f9565 100644 > --- a/drivers/gpu/drm/i915/display/intel_opregion.h > +++ b/drivers/gpu/drm/i915/display/intel_opregion.h > @@ -77,6 +77,7 @@ int intel_opregion_get_panel_type(struct drm_i915_private > *dev_priv); > struct edid *intel_opregion_get_edid(struct intel_connector *connector); > bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915); > void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable); > +bool intel_opregion_vram_sr_required(struct drm_i915_private *i915); > > bool intel_opregion_headless_sku(struct drm_i915_private *i915); > > @@ -145,6 +146,11 @@ static void intel_opregion_vram_sr(struct > drm_i915_private *i915, bool enable) > { > } > > +static bool intel_opregion_vram_sr_required(struct drm_i915_private *i915) static inline. BR, Jani. > +{ > + return false; > +} > + > #endif /* CONFIG_ACPI */ > > #endif -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v2] dt-bindings: display: Add Arm virtual platforms display
On Mon, Jun 13, 2022 at 4:57 PM Rob Herring wrote: > 'arm,rtsm-display' is a panel for Arm, Ltd. virtual platforms (e.g. FVP). > The binding has been in use for a long time, but was never documented. > > Some users and an example have a 'panel-dpi' compatible, but that's not > needed without a 'panel-timing' node which none of the users have since > commit 928faf5e3e8d ("arm64: dts: fvp: Remove panel timings"). The > example does have a 'panel-timing' node, but it should not for the > same reasons the node was removed in the dts files. So update the > example in arm,pl11x.yaml to match the schema. > > Cc: Linus Walleij > Cc: Robin Murphy > Cc: Andre Przywara > Signed-off-by: Rob Herring > --- > v2: > - Make arm,rtsm-display its own schema file instead of using >panel-simple. Thanks for fixing this Rob! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH] drm/msm/gem: Drop obj lock in msm_gem_free_object()
On Thu, Jun 16, 2022 at 1:28 AM Stephen Boyd wrote: > > Quoting Rob Clark (2022-06-13 13:50:32) > > diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h > > index d608339c1643..432032ad4aed 100644 > > --- a/drivers/gpu/drm/msm/msm_gem.h > > +++ b/drivers/gpu/drm/msm/msm_gem.h > > @@ -229,7 +229,19 @@ msm_gem_unlock(struct drm_gem_object *obj) > > static inline bool > > msm_gem_is_locked(struct drm_gem_object *obj) > > { > > - return dma_resv_is_locked(obj->resv); > > + /* > > +* Destroying the object is a special case.. msm_gem_free_object() > > +* calls many things that WARN_ON if the obj lock is not held. But > > +* acquiring the obj lock in msm_gem_free_object() can cause a > > +* locking order inversion between reservation_ww_class_mutex and > > +* fs_reclaim. > > +* > > +* This deadlock is not actually possible, because no one should > > +* be already holding the lock when msm_gem_free_object() is called. > > +* Unfortunately lockdep is not aware of this detail. So when the > > +* refcount drops to zero, we pretend it is already locked. > > +*/ > > + return dma_resv_is_locked(obj->resv) || (kref_read(&obj->refcount) > > == 0); > > Instead of modifying this function can we push down the fact that this > function is being called from the free path and skip checking this > condition in that case? Or add some "_locked/free_path" wrappers that > skip the lock assertion? That would make it clearer to understand while > reading the code that it is locked when it is asserted to be locked, and > that we don't care when we're freeing because all references to the > object are gone. that was my earlier attempt, and I wasn't too happy with the result. And then I realized if refcount==0 then by definition we aren't racing with anyone else ;-) > Here's a totally untested patch to show the idea. The comment about > pretending the lock is held can be put in msm_gem_free_object() to > clarify why it's OK to call the locked variants of the functions. > > ---8<--- > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c > index 97d5b4d8b9b0..01f19d37bfb6 100644 > --- a/drivers/gpu/drm/msm/msm_gem.c > +++ b/drivers/gpu/drm/msm/msm_gem.c > @@ -346,13 +346,11 @@ static void del_vma(struct msm_gem_vma *vma) > * mapping. > */ > static void > -put_iova_spaces(struct drm_gem_object *obj, bool close) > +put_iova_spaces_locked(struct drm_gem_object *obj, bool close) > { > struct msm_gem_object *msm_obj = to_msm_bo(obj); > struct msm_gem_vma *vma; > > - GEM_WARN_ON(!msm_gem_is_locked(obj)); > - > list_for_each_entry(vma, &msm_obj->vmas, list) { > if (vma->aspace) { > msm_gem_purge_vma(vma->aspace, vma); > @@ -362,18 +360,28 @@ put_iova_spaces(struct drm_gem_object *obj, bool close) > } > } > > -/* Called with msm_obj locked */ > +static void put_iova_spaces(struct drm_gem_object *obj, bool close) > +{ > + GEM_WARN_ON(!msm_gem_is_locked(obj)); > + put_iova_spaces_locked(obj, close); > +} they are both _locked paths ;-) But in general I think the parallel code paths make things more confusing about what is the right thing to call. And I would like to put more WARN_ON(!locked()) in the gem_vma code, to make it clear that the obj lock is protecting things there as well.. which, with this approach would turn into parallel code paths multiple levels deep BR, -R > + > +/* Called with msm_obj locked or on the free path */ > static void > -put_iova_vmas(struct drm_gem_object *obj) > +put_iova_vmas_locked(struct drm_gem_object *obj) > { > struct msm_gem_object *msm_obj = to_msm_bo(obj); > struct msm_gem_vma *vma, *tmp; > > - GEM_WARN_ON(!msm_gem_is_locked(obj)); > - > - list_for_each_entry_safe(vma, tmp, &msm_obj->vmas, list) { > + list_for_each_entry_safe(vma, tmp, &msm_obj->vmas, list) > del_vma(vma); > - } > +} > + > +static void > +put_iova_vmas(struct drm_gem_object *obj) > +{ > + GEM_WARN_ON(!msm_gem_is_locked(obj)); > + put_iova_vmas_locked(obj); > } > > static struct msm_gem_vma *get_vma_locked(struct drm_gem_object *obj, > @@ -795,12 +803,10 @@ void msm_gem_evict(struct drm_gem_object *obj) > update_inactive(msm_obj); > } > > -void msm_gem_vunmap(struct drm_gem_object *obj) > +static void msm_gem_vunmap_locked(struct drm_gem_object *obj) > { > struct msm_gem_object *msm_obj = to_msm_bo(obj); > > - GEM_WARN_ON(!msm_gem_is_locked(obj)); > - > if (!msm_obj->vaddr || GEM_WARN_ON(!is_vunmapable(msm_obj))) > return; > > @@ -808,6 +814,12 @@ void msm_gem_vunmap(struct drm_gem_object *obj) > msm_obj->vaddr = NULL; > } > > +void msm_gem_vunmap(struct drm_gem_object *obj) > +{ > + GEM_WARN_ON(!msm_gem_is_locked(obj)); > + msm_gem_vunmap_locked(obj)
Re: [PATCH] drm/msm: Fix fence rollover issue
On Thu, Jun 16, 2022 at 1:27 AM Dmitry Baryshkov wrote: > > On 15/06/2022 19:24, Rob Clark wrote: > > From: Rob Clark > > > > And while we are at it, let's start the fence counter close to the > > rollover point so that if issues slip in, they are more obvious. > > > > Signed-off-by: Rob Clark > > Should it also have > > Fixes: fde5de6cb461 ("drm/msm: move fence code to it's own file") > > Or maybe > > Fixes: 5f3aee4ceb5b ("drm/msm: Handle fence rollover") arguably it fixes the first commit that added GPU support (and finishes up a couple spots that the above commit missed) I guess I could use the fixes tag just to indicate how far back it would be reasonable to backport to stable branches. > Otherwise: > > Reviewed: Dmitry Baryshkov > > > > --- > > drivers/gpu/drm/msm/msm_fence.c | 13 +++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_fence.c > > b/drivers/gpu/drm/msm/msm_fence.c > > index 3df255402a33..a35a6746c7cd 100644 > > --- a/drivers/gpu/drm/msm/msm_fence.c > > +++ b/drivers/gpu/drm/msm/msm_fence.c > > @@ -28,6 +28,14 @@ msm_fence_context_alloc(struct drm_device *dev, volatile > > uint32_t *fenceptr, > > fctx->fenceptr = fenceptr; > > spin_lock_init(&fctx->spinlock); > > > > + /* > > + * Start out close to the 32b fence rollover point, so we can > > + * catch bugs with fence comparisons. > > + */ > > + fctx->last_fence = 0xff00; > > + fctx->completed_fence = fctx->last_fence; > > + *fctx->fenceptr = fctx->last_fence; > > This looks like a debugging hack. But probably it's fine to have it, as > it wouldn't cause any side effects. I was originally going to add a modparam or kconfig to enable this.. but then thought, if there is a bug and thing are to go wrong, it's best for that to happen ASAP rather than after 200-400 days of uptime.. the latter case can be rather hard to reproduce bugs ;-) IIRC the kernel does something similar with jiffies to ensure the rollover point is hit quickly BR, -R > > + > > return fctx; > > } > > > > @@ -46,11 +54,12 @@ bool msm_fence_completed(struct msm_fence_context > > *fctx, uint32_t fence) > > (int32_t)(*fctx->fenceptr - fence) >= 0; > > } > > > > -/* called from workqueue */ > > +/* called from irq handler */ > > void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence) > > { > > spin_lock(&fctx->spinlock); > > - fctx->completed_fence = max(fence, fctx->completed_fence); > > + if (fence_after(fence, fctx->completed_fence)) > > + fctx->completed_fence = fence; > > spin_unlock(&fctx->spinlock); > > } > > > > > -- > With best wishes > Dmitry
Re: [Intel-gfx] [PATCH v2 3/9] drm/i915/dg2: Add DG2_NB_MBD subplatform
On Thu, 16 Jun 2022, Tvrtko Ursulin wrote: > On 16/06/2022 13:01, Anshuman Gupta wrote: >> DG2 NB SKU need to distinguish between MBD and AIC to probe >> the VRAM Self Refresh feature support. Adding those sub platform >> accordingly. >> >> Cc: Matt Roper >> Signed-off-by: Anshuman Gupta >> --- >> drivers/gpu/drm/i915/i915_drv.h | 3 +++ >> drivers/gpu/drm/i915/intel_device_info.c | 21 + >> drivers/gpu/drm/i915/intel_device_info.h | 11 +++ >> include/drm/i915_pciids.h| 23 --- >> 4 files changed, 47 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index a5bc6a774c5a..f1f8699eedfd 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -1007,10 +1007,13 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, >> #define IS_PONTEVECCHIO(dev_priv) IS_PLATFORM(dev_priv, INTEL_PONTEVECCHIO) >> >> #define IS_DG2_G10(dev_priv) \ >> +IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10_NB_MBD) || \ >> IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10) >> #define IS_DG2_G11(dev_priv) \ >> +IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11_NB_MBD) || \ >> IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11) >> #define IS_DG2_G12(dev_priv) \ >> +IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12_NB_MBD) || \ >> IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12) >> #define IS_ADLS_RPLS(dev_priv) \ >> IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL) >> diff --git a/drivers/gpu/drm/i915/intel_device_info.c >> b/drivers/gpu/drm/i915/intel_device_info.c >> index f0bf23726ed8..93da555adc4e 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.c >> +++ b/drivers/gpu/drm/i915/intel_device_info.c >> @@ -187,6 +187,18 @@ static const u16 subplatform_rpl_ids[] = { >> INTEL_RPLP_IDS(0), >> }; >> >> +static const u16 subplatform_g10_mb_mbd_ids[] = { >> +INTEL_DG2_G10_NB_MBD_IDS(0), >> +}; >> + >> +static const u16 subplatform_g11_mb_mbd_ids[] = { >> +INTEL_DG2_G11_NB_MBD_IDS(0), >> +}; >> + >> +static const u16 subplatform_g12_mb_mbd_ids[] = { >> +INTEL_DG2_G12_NB_MBD_IDS(0), >> +}; >> + >> static const u16 subplatform_g10_ids[] = { >> INTEL_DG2_G10_IDS(0), >> INTEL_ATS_M150_IDS(0), >> @@ -246,6 +258,15 @@ 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_g10_mb_mbd_ids, >> + ARRAY_SIZE(subplatform_g10_mb_mbd_ids))) { >> +mask = BIT(INTEL_SUBPLATFORM_G10_NB_MBD); >> +} else if (find_devid(devid, subplatform_g11_mb_mbd_ids, >> + ARRAY_SIZE(subplatform_g11_mb_mbd_ids))) { >> +mask = BIT(INTEL_SUBPLATFORM_G11_NB_MBD); >> +} else if (find_devid(devid, subplatform_g12_mb_mbd_ids, >> + ARRAY_SIZE(subplatform_g12_mb_mbd_ids))) { >> +mask = BIT(INTEL_SUBPLATFORM_G12_NB_MBD); >> } 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 08341174ee0a..c929e2d7e59c 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.h >> +++ b/drivers/gpu/drm/i915/intel_device_info.h >> @@ -97,7 +97,7 @@ enum intel_platform { >>* it is fine for the same bit to be used on multiple parent platforms. >>*/ >> >> -#define INTEL_SUBPLATFORM_BITS (3) >> +#define INTEL_SUBPLATFORM_BITS (6) >> #define INTEL_SUBPLATFORM_MASK (BIT(INTEL_SUBPLATFORM_BITS) - 1) >> >> /* HSW/BDW/SKL/KBL/CFL */ >> @@ -111,9 +111,12 @@ enum intel_platform { >> #define INTEL_SUBPLATFORM_UY (0) >> >> /* DG2 */ >> -#define INTEL_SUBPLATFORM_G10 0 >> -#define INTEL_SUBPLATFORM_G11 1 >> -#define INTEL_SUBPLATFORM_G12 2 >> +#define INTEL_SUBPLATFORM_G10_NB_MBD0 >> +#define INTEL_SUBPLATFORM_G11_NB_MBD1 >> +#define INTEL_SUBPLATFORM_G12_NB_MBD2 >> +#define INTEL_SUBPLATFORM_G10 3 >> +#define INTEL_SUBPLATFORM_G11 4 >> +#define INTEL_SUBPLATFORM_G12 5 > > Ugh I feel this "breaks" the subplatform idea.. feels like it is just > too many bits when two separate sets of information get tracked (Gxx > plus MBD). I think they could be specified independent of each other, though. The subplatform if-else ladder would have to be replaced with independent ifs. You'd have the G10/G11/G12 and 1 bit separately for MBD. Only the macros for PCI IDs need to be separate (MBD vs not). You'll then have: s
[PATCH V2 0/2] Add Xilinx DSI-Tx subsystem DRM driver
MIPI DSI TX subsystem allows you to quickly create systems based on the MIPI protocol. It interfaces between the video processing subsystems and MIPI-based displays. An internal high-speed physical layer design, D-PHY, is provided to allow direct connection to display peripherals. The subsystem consists of the following sub-blocks: - MIPI D-PHY - MIPI DSI TX Controller - AXI Crossbar Please refer pg238 [1]. The DSI TX Controller receives stream of image data through an input stream interface. At design time, this subsystem can be configured to number of lanes and pixel format. This patch series adds the dt-binding and DRM driver for Xilinx DSI-Tx soft IP. Changes in V2: - Rebased on 5.19 kernel - Moved mode_set functionality to atomic_enable as its deprecrated - Mask fixes - Replaced panel calls with bridge API's - Added mandatory atomic operations - Removed unnecessary logging - Added ARCH_ZYNQMP dependency in Kconfig - Fixed YAML warnings - Cleanup Venkateshwar Rao Gannavarapu (2): dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver .../bindings/display/xlnx/xlnx,dsi-tx.yaml | 101 + drivers/gpu/drm/xlnx/Kconfig | 12 + drivers/gpu/drm/xlnx/Makefile | 1 + drivers/gpu/drm/xlnx/xlnx_dsi.c| 429 + 4 files changed, 543 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c -- 1.8.3.1
[PATCH V2 1/2] dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation
This patch adds dt binding for Xilinx DSI-TX subsystem. The Xilinx MIPI DSI (Display serial interface) Transmitter Subsystem implements the Mobile Industry Processor Interface (MIPI) based display interface. It supports the interface with the programmable logic (FPGA). Signed-off-by: Venkateshwar Rao Gannavarapu --- .../bindings/display/xlnx/xlnx,dsi-tx.yaml | 101 + 1 file changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml new file mode 100644 index 000..644934d --- /dev/null +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml @@ -0,0 +1,101 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/xlnx/xlnx,dsi-tx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Xilinx DSI Transmitter subsystem Device Tree Bindings + +maintainers: + - Venkateshwar Rao Gannavarapu + +description: | + The Xilinx DSI Transmitter Subsystem implements the Mobile Industry + Processor Interface based display interface. It supports the interface + with the programmable logic (FPGA). + + For more details refer to PG238 Xilinx MIPI DSI-V2.0 Tx Subsystem. + +properties: + compatible: +const: xlnx,dsi-tx-v2.0 + + reg: +maxItems: 1 + + clocks: +items: + - description: AXI Lite CPU clock + - description: D-PHY clock + + clock-names: +items: + - const: s_axis_aclk + - const: dphy_clk_200M + + ports: +$ref: /schemas/graph.yaml#/properties/ports + +properties: + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: + This port should be the input endpoint. + + port@1: +$ref: /schemas/graph.yaml#/properties/port +description: + This port should be the output endpoint. + +required: + - "#address-cells" + - "#size-cells" + - compatible + - reg + - clocks + - clock-names + - ports + +additionalProperties: false + +examples: + - | +dsi0: dsi_tx@8002 { +compatible = "xlnx,dsi-tx-v2.0"; +reg = <0x8002 0x2>; +clocks = <&misc_clk_0>, <&misc_clk_1>; +clock-names = "s_axis_aclk", "dphy_clk_200M"; +#address-cells = <1>; +#size-cells = <0>; + +ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { +reg = <0>; +mipi_dsi_in: endpoint { +remote-endpoint = <&pl_disp>; +}; + }; + + port@1 { +reg = <1>; +mipi_dsi_out: endpoint { +remote-endpoint = <&panel_in>; +}; + }; +}; + +panel@0 { + compatible = "auo,b101uan01"; + reg = <0>; + port { +panel_in: endpoint { +remote-endpoint = <&mipi_dsi_out>; +}; + }; +}; +}; + +... -- 1.8.3.1
[PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver
The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video data from AXI-4 stream interface. It supports upto 4 lanes, optional register interface for the DPHY and multiple RGB color formats. This is a MIPI-DSI host driver and provides DSI bus for panels. This driver also helps to communicate with its panel using panel framework. Signed-off-by: Venkateshwar Rao Gannavarapu --- drivers/gpu/drm/xlnx/Kconfig| 12 ++ drivers/gpu/drm/xlnx/Makefile | 1 + drivers/gpu/drm/xlnx/xlnx_dsi.c | 429 3 files changed, 442 insertions(+) create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig index f9cf93c..a75bd76 100644 --- a/drivers/gpu/drm/xlnx/Kconfig +++ b/drivers/gpu/drm/xlnx/Kconfig @@ -1,3 +1,15 @@ +config DRM_XLNX_DSI + tristate "Xilinx DRM DSI Subsystem Driver" + depends on ARCH_ZYNQMP || COMPILE_TEST + depends on DRM && OF + select DRM_KMS_HELPER + select DRM_MIPI_DSI + select DRM_PANEL_BRIDGE + help + DRM bridge driver for Xilinx programmable DSI subsystem controller. + choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in + video pipeline. + config DRM_ZYNQMP_DPSUB tristate "ZynqMP DisplayPort Controller Driver" depends on ARCH_ZYNQMP || COMPILE_TEST diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile index 51c24b7..f90849b 100644 --- a/drivers/gpu/drm/xlnx/Makefile +++ b/drivers/gpu/drm/xlnx/Makefile @@ -1,2 +1,3 @@ +obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c new file mode 100644 index 000..39d8947 --- /dev/null +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c @@ -0,0 +1,429 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Xilinx FPGA MIPI DSI Tx Controller driver. + * + * Copyright (C) 2022 Xilinx, Inc. + * + * Author: Venkateshwar Rao Gannavarapu + */ + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +/* DSI Tx IP registers */ +#define XDSI_CCR 0x00 +#define XDSI_CCR_COREENB BIT(0) +#define XDSI_CCR_SOFTRST BIT(1) +#define XDSI_CCR_CRREADY BIT(2) +#define XDSI_CCR_CMDMODE BIT(3) +#define XDSI_CCR_DFIFORST BIT(4) +#define XDSI_CCR_CMDFIFORSTBIT(5) +#define XDSI_PCR 0x04 +#define XDSI_PCR_LANES_MASK3 +#define XDSI_PCR_VIDEOMODE(x) (((x) & 0x3) << 3) +#define XDSI_PCR_VIDEOMODE_MASKGENMASK(4, 3) +#define XDSI_PCR_VIDEOMODE_SHIFT 3 +#define XDSI_PCR_BLLPTYPE(x) ((x) << 5) +#define XDSI_PCR_BLLPMODE(x) ((x) << 6) +#define XDSI_PCR_PIXELFORMAT_MASK GENMASK(12, 11) +#define XDSI_PCR_PIXELFORMAT_SHIFT 11 +#define XDSI_PCR_EOTPENABLE(x) ((x) << 13) +#define XDSI_GIER 0x20 +#define XDSI_ISR 0x24 +#define XDSI_IER 0x28 +#define XDSI_STR 0x2C +#define XDSI_STR_RDY_SHPKT BIT(6) +#define XDSI_STR_RDY_LNGPKTBIT(7) +#define XDSI_STR_DFIFO_FULLBIT(8) +#define XDSI_STR_DFIFO_EMPTY BIT(9) +#define XDSI_STR_WAITFR_DATA BIT(10) +#define XDSI_STR_CMD_EXE_PGS BIT(11) +#define XDSI_STR_CCMD_PROC BIT(12) +#define XDSI_CMD 0x30 +#define XDSI_CMD_QUEUE_PACKET(x) ((x) & GENMASK(23, 0)) +#define XDSI_DFR 0x34 +#define XDSI_TIME1 0x50 +#define XDSI_TIME1_BLLP_BURST(x) ((x) & GENMASK(15, 0)) +#define XDSI_TIME1_HSA(x) (((x) & GENMASK(15, 0)) << 16) +#define XDSI_TIME2 0x54 +#define XDSI_TIME2_VACT(x) ((x) & GENMASK(15, 0)) +#define XDSI_TIME2_HACT(x) (((x) & GENMASK(15, 0)) << 16) +#define XDSI_HACT_MULTIPLIER GENMASK(1, 0) +#define XDSI_TIME3 0x58 +#define XDSI_TIME3_HFP(x) ((x) & GENMASK(15, 0)) +#define XDSI_TIME3_HBP(x) (((x) & GENMASK(15, 0)) << 16) +#define XDSI_TIME4 0x5c +#define XDSI_TIME4_VFP(x) ((x) & GENMASK(7, 0)) +#define XDSI_TIME4_VBP(x) (((x) & GENMASK(7, 0)) << 8) +#define XDSI_TIME4_VSA(x) (((x) & GENMASK(7, 0)) << 16) +#define XDSI_NUM_DATA_T4 + +/** + * struct xlnx_dsi - Xilinx DSI-TX core + * @bridge: DRM bridge structure + * @dsi_host: DSI host device + * @next_bridge: bridge structure + * @dev: device structure + * @clks: clock source structure + * @iomem: Base address of DSI subsystem + * @mode_flags: DSI operation mode related flags + * @lanes: number of active data lanes supported by DSI controller + * @mul_fa
Re: [Intel-gfx] [PATCH v2 9/9] drm/i915/rpm: d3cold Policy
On Thu, 16 Jun 2022, Anshuman Gupta wrote: > Add d3cold_sr_lmem_threshold modparam to choose between > d3cold-off zero watt and d3cold-VRAM Self Refresh. > i915 requires to evict the lmem objects to smem in order to > support d3cold-Off. > > If gfx root port is not capable of sending PME from d3cold > then i915 don't need to program d3cold-off/d3cold-vram_sr > sequence. > > FIXME: Eviction of lmem objects in case of D3Cold off is wip. > > Cc: Rodrigo Vivi > Signed-off-by: Anshuman Gupta > --- > drivers/gpu/drm/i915/i915_driver.c | 27 --- > drivers/gpu/drm/i915/i915_params.c | 4 > drivers/gpu/drm/i915/i915_params.h | 3 ++- > 3 files changed, 30 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c > b/drivers/gpu/drm/i915/i915_driver.c > index fcff5f3fe05e..aef4b17efdbe 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -1560,15 +1560,36 @@ static int i915_pm_restore(struct device *kdev) > static int intel_runtime_idle(struct device *kdev) > { > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); > + u64 lmem_total = to_gt(dev_priv)->lmem->total; > + u64 lmem_avail = to_gt(dev_priv)->lmem->avail; > + u64 lmem_used = lmem_total - lmem_avail; > + struct pci_dev *root_pdev; > int ret = 1; > > - if (!HAS_LMEM_SR(dev_priv)) { > - /*TODO: Prepare for D3Cold-Off */ > + root_pdev = pcie_find_root_port(pdev); > + if (!root_pdev) > + goto out; > + > + if (!pci_pme_capable(root_pdev, PCI_D3cold)) > goto out; > - } > > disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > + if (lmem_used < dev_priv->params.d3cold_sr_lmem_threshold * 1024 * > 1024) { > + drm_dbg(&dev_priv->drm, "Prepare for D3Cold off\n"); > + pci_d3cold_enable(root_pdev); > + /* FIXME: Eviction of lmem objects and guc reset is wip */ > + intel_pm_vram_sr(dev_priv, false); > + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > + goto out; > + } else if (!HAS_LMEM_SR(dev_priv)) { > + /* Disable D3Cold to reduce the eviction latency */ > + pci_d3cold_disable(root_pdev); > + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > + goto out; > + } This is *way* too low level code for such high level function. This needs to be abstracted better. > + > ret = intel_pm_vram_sr(dev_priv, true); > if (!ret) > drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); > diff --git a/drivers/gpu/drm/i915/i915_params.c > b/drivers/gpu/drm/i915/i915_params.c > index 701fbc98afa0..6c6b3c372d4d 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -197,6 +197,10 @@ i915_param_named(enable_gvt, bool, 0400, > "Enable support for Intel GVT-g graphics virtualization host > support(default:false)"); > #endif > > +i915_param_named_unsafe(d3cold_sr_lmem_threshold, int, 0400, > + "Enable Vidoe RAM Self refresh when size of lmem is greater to this > threshold. " > + "It helps to optimize the suspend/resume latecy. (default: 300mb)"); > + > #if CONFIG_DRM_I915_REQUEST_TIMEOUT > i915_param_named_unsafe(request_timeout_ms, uint, 0600, > "Default request/fence/batch buffer expiration > timeout."); > diff --git a/drivers/gpu/drm/i915/i915_params.h > b/drivers/gpu/drm/i915/i915_params.h > index b5e7ea45d191..28f20ebaf41f 100644 > --- a/drivers/gpu/drm/i915/i915_params.h > +++ b/drivers/gpu/drm/i915/i915_params.h > @@ -83,7 +83,8 @@ struct drm_printer; > param(bool, verbose_state_checks, true, 0) \ > param(bool, nuclear_pageflip, false, 0400) \ > param(bool, enable_dp_mst, true, 0600) \ > - param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : > 0) > + param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : > 0) \ > + param(int, d3cold_sr_lmem_threshold, 300, 0600) \ What's the point of the parameter? Also, please read the comment /* leave bools at the end to not create holes */ above. BR, Jani. > > #define MEMBER(T, member, ...) T member; > struct i915_params { -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v2 8/9] drm/i915/xehpsdv: Store lmem region in gt
On Thu, 16 Jun 2022, Anshuman Gupta wrote: > From: Tvrtko Ursulin > > Store a pointer to respective local memory region in intel_gt so it can be > used when memory local to a GT needs to be allocated. > > Cc: Andi Shyti > Signed-off-by: Tvrtko Ursulin > Signed-off-by: Anshuman Gupta > --- > drivers/gpu/drm/i915/gt/intel_gt.c | 1 + > drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c > b/drivers/gpu/drm/i915/gt/intel_gt.c > index f33290358c51..7a535f670ae1 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -91,6 +91,7 @@ static int intel_gt_probe_lmem(struct intel_gt *gt) > GEM_BUG_ON(!HAS_REGION(i915, id)); > GEM_BUG_ON(i915->mm.regions[id]); > i915->mm.regions[id] = mem; > + gt->lmem = mem; > > return 0; > } > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h > b/drivers/gpu/drm/i915/gt/intel_gt_types.h > index df708802889d..cd7744eaaeaa 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h > @@ -23,6 +23,7 @@ > #include "intel_gt_buffer_pool_types.h" > #include "intel_hwconfig.h" > #include "intel_llc_types.h" > +#include "intel_memory_region.h" Please never add includes in headers when a forward declaration is sufficient. I'm spending a lot of time trying to reduce the include dependencies we have. BR, Jani. > #include "intel_reset_types.h" > #include "intel_rc6_types.h" > #include "intel_rps_types.h" > @@ -202,6 +203,8 @@ struct intel_gt { >*/ > phys_addr_t phys_addr; > > + struct intel_memory_region *lmem; > + > struct intel_gt_info { > unsigned int id; -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v2 7/9] drm/i915/rpm: Enable D3Cold VRAM SR Support
On Thu, 16 Jun 2022, Anshuman Gupta wrote: > Intel Client DGFX card supports D3Cold with two option. > D3Cold-off zero watt, D3Cold-VRAM Self Refresh. > > i915 requires to evict the lmem objects to smem in order to > support D3Cold-Off, which increases i915 the suspend/resume > latency. Enabling VRAM Self Refresh feature optimize the > latency with additional power cost which required to retain > the lmem. > > Adding intel_runtime_idle (runtime_idle callback) to enable > VRAM_SR, it will be used for policy to choose > between D3Cold-off vs D3Cold-VRAM_SR. > > Since we have introduced i915 runtime_idle callback. > It need to be warranted that Runtime PM Core invokes runtime_idle > callback when runtime usages count becomes zero. That requires > to use pm_runtime_put instead of pm_runtime_put_autosuspend. > > TODO: GuC interface state save/restore. > > Cc: Rodrigo Vivi > Cc: Chris Wilson > Signed-off-by: Anshuman Gupta > --- > drivers/gpu/drm/i915/i915_driver.c | 26 + > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-- > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c > b/drivers/gpu/drm/i915/i915_driver.c > index aa1fb15b1f11..fcff5f3fe05e 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -1557,6 +1557,31 @@ static int i915_pm_restore(struct device *kdev) > return i915_pm_resume(kdev); > } > > +static int intel_runtime_idle(struct device *kdev) > +{ > + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > + int ret = 1; > + > + if (!HAS_LMEM_SR(dev_priv)) { > + /*TODO: Prepare for D3Cold-Off */ > + goto out; > + } > + > + disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > + > + ret = intel_pm_vram_sr(dev_priv, true); > + if (!ret) > + drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); Please add the debug in the intel_pm_vram_sr() function instead. BR, Jani. > + > + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > + > +out: > + pm_runtime_mark_last_busy(kdev); > + pm_runtime_autosuspend(kdev); > + > + return ret; > +} > + > static int intel_runtime_suspend(struct device *kdev) > { > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > @@ -1742,6 +1767,7 @@ const struct dev_pm_ops i915_pm_ops = { > .restore = i915_pm_restore, > > /* S0ix (via runtime suspend) event handlers */ > + .runtime_idle = intel_runtime_idle, > .runtime_suspend = intel_runtime_suspend, > .runtime_resume = intel_runtime_resume, > }; > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 6ed5786bcd29..4dade7e8a795 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -492,8 +492,7 @@ static void __intel_runtime_pm_put(struct > intel_runtime_pm *rpm, > > intel_runtime_pm_release(rpm, wakelock); > > - pm_runtime_mark_last_busy(kdev); > - pm_runtime_put_autosuspend(kdev); > + pm_runtime_put(kdev); > } > > /** -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v2 3/9] drm/i915/dg2: Add DG2_NB_MBD subplatform
On 16/06/2022 15:15, Jani Nikula wrote: On Thu, 16 Jun 2022, Tvrtko Ursulin wrote: On 16/06/2022 13:01, Anshuman Gupta wrote: DG2 NB SKU need to distinguish between MBD and AIC to probe the VRAM Self Refresh feature support. Adding those sub platform accordingly. Cc: Matt Roper Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_device_info.c | 21 + drivers/gpu/drm/i915/intel_device_info.h | 11 +++ include/drm/i915_pciids.h| 23 --- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a5bc6a774c5a..f1f8699eedfd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1007,10 +1007,13 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define IS_PONTEVECCHIO(dev_priv) IS_PLATFORM(dev_priv, INTEL_PONTEVECCHIO) #define IS_DG2_G10(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10) #define IS_DG2_G11(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11) #define IS_DG2_G12(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12) #define IS_ADLS_RPLS(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index f0bf23726ed8..93da555adc4e 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -187,6 +187,18 @@ static const u16 subplatform_rpl_ids[] = { INTEL_RPLP_IDS(0), }; +static const u16 subplatform_g10_mb_mbd_ids[] = { + INTEL_DG2_G10_NB_MBD_IDS(0), +}; + +static const u16 subplatform_g11_mb_mbd_ids[] = { + INTEL_DG2_G11_NB_MBD_IDS(0), +}; + +static const u16 subplatform_g12_mb_mbd_ids[] = { + INTEL_DG2_G12_NB_MBD_IDS(0), +}; + static const u16 subplatform_g10_ids[] = { INTEL_DG2_G10_IDS(0), INTEL_ATS_M150_IDS(0), @@ -246,6 +258,15 @@ 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_g10_mb_mbd_ids, + ARRAY_SIZE(subplatform_g10_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G10_NB_MBD); + } else if (find_devid(devid, subplatform_g11_mb_mbd_ids, + ARRAY_SIZE(subplatform_g11_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G11_NB_MBD); + } else if (find_devid(devid, subplatform_g12_mb_mbd_ids, + ARRAY_SIZE(subplatform_g12_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G12_NB_MBD); } 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 08341174ee0a..c929e2d7e59c 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -97,7 +97,7 @@ enum intel_platform { * it is fine for the same bit to be used on multiple parent platforms. */ -#define INTEL_SUBPLATFORM_BITS (3) +#define INTEL_SUBPLATFORM_BITS (6) #define INTEL_SUBPLATFORM_MASK (BIT(INTEL_SUBPLATFORM_BITS) - 1) /* HSW/BDW/SKL/KBL/CFL */ @@ -111,9 +111,12 @@ enum intel_platform { #define INTEL_SUBPLATFORM_UY (0) /* DG2 */ -#define INTEL_SUBPLATFORM_G10 0 -#define INTEL_SUBPLATFORM_G11 1 -#define INTEL_SUBPLATFORM_G12 2 +#define INTEL_SUBPLATFORM_G10_NB_MBD 0 +#define INTEL_SUBPLATFORM_G11_NB_MBD 1 +#define INTEL_SUBPLATFORM_G12_NB_MBD 2 +#define INTEL_SUBPLATFORM_G10 3 +#define INTEL_SUBPLATFORM_G11 4 +#define INTEL_SUBPLATFORM_G12 5 Ugh I feel this "breaks" the subplatform idea.. feels like it is just too many bits when two separate sets of information get tracked (Gxx plus MBD). I think they could be specified independent of each other, though. The subplatform if-else ladder would have to be replaced with independent ifs. You'd have the G10/G11/G12 and 1 bit separately for MBD. Only the macros for PCI IDs need to be separate (MBD vs not). You'll then have: static const u16 subplatform_g10_ids[] = { INTEL_DG2_G10_IDS(0), INTEL_DG2_G10_NB_MBD_IDS(0), INTEL_ATS_M150_IDS(0), }; Ditto for g11 and g12, and separately: static const u16 su
Re: [Intel-gfx] [PATCH v2 3/9] drm/i915/dg2: Add DG2_NB_MBD subplatform
On Thu, 16 Jun 2022, Tvrtko Ursulin wrote: > On 16/06/2022 15:15, Jani Nikula wrote: >> On Thu, 16 Jun 2022, Tvrtko Ursulin wrote: >>> On 16/06/2022 13:01, Anshuman Gupta wrote: DG2 NB SKU need to distinguish between MBD and AIC to probe the VRAM Self Refresh feature support. Adding those sub platform accordingly. Cc: Matt Roper Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_device_info.c | 21 + drivers/gpu/drm/i915/intel_device_info.h | 11 +++ include/drm/i915_pciids.h| 23 --- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a5bc6a774c5a..f1f8699eedfd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1007,10 +1007,13 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define IS_PONTEVECCHIO(dev_priv) IS_PLATFORM(dev_priv, INTEL_PONTEVECCHIO) #define IS_DG2_G10(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10) #define IS_DG2_G11(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11) #define IS_DG2_G12(dev_priv) \ + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12_NB_MBD) || \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12) #define IS_ADLS_RPLS(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index f0bf23726ed8..93da555adc4e 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -187,6 +187,18 @@ static const u16 subplatform_rpl_ids[] = { INTEL_RPLP_IDS(0), }; +static const u16 subplatform_g10_mb_mbd_ids[] = { + INTEL_DG2_G10_NB_MBD_IDS(0), +}; + +static const u16 subplatform_g11_mb_mbd_ids[] = { + INTEL_DG2_G11_NB_MBD_IDS(0), +}; + +static const u16 subplatform_g12_mb_mbd_ids[] = { + INTEL_DG2_G12_NB_MBD_IDS(0), +}; + static const u16 subplatform_g10_ids[] = { INTEL_DG2_G10_IDS(0), INTEL_ATS_M150_IDS(0), @@ -246,6 +258,15 @@ 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_g10_mb_mbd_ids, +ARRAY_SIZE(subplatform_g10_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G10_NB_MBD); + } else if (find_devid(devid, subplatform_g11_mb_mbd_ids, +ARRAY_SIZE(subplatform_g11_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G11_NB_MBD); + } else if (find_devid(devid, subplatform_g12_mb_mbd_ids, +ARRAY_SIZE(subplatform_g12_mb_mbd_ids))) { + mask = BIT(INTEL_SUBPLATFORM_G12_NB_MBD); } 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 08341174ee0a..c929e2d7e59c 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -97,7 +97,7 @@ enum intel_platform { * it is fine for the same bit to be used on multiple parent platforms. */ -#define INTEL_SUBPLATFORM_BITS (3) +#define INTEL_SUBPLATFORM_BITS (6) #define INTEL_SUBPLATFORM_MASK (BIT(INTEL_SUBPLATFORM_BITS) - 1) /* HSW/BDW/SKL/KBL/CFL */ @@ -111,9 +111,12 @@ enum intel_platform { #define INTEL_SUBPLATFORM_UY(0) /* DG2 */ -#define INTEL_SUBPLATFORM_G10 0 -#define INTEL_SUBPLATFORM_G11 1 -#define INTEL_SUBPLATFORM_G12 2 +#define INTEL_SUBPLATFORM_G10_NB_MBD 0 +#define INTEL_SUBPLATFORM_G11_NB_MBD 1 +#define INTEL_SUBPLATFORM_G12_NB_MBD 2 +#define INTEL_SUBPLATFORM_G10 3 +#define INTEL_SUBPLATFORM_G11 4 +#define INTEL_SUBPLATFORM_G12 5 >>> >>> Ugh I feel this "breaks" the subplatform idea.. feels like it is just >>> too many bits
Re: [PATCH 2/2] dt-bindings: phy: List supplies for qcom,edp-phy
Hi, On Mon, Apr 25, 2022 at 2:07 PM Douglas Anderson wrote: > > We're supposed to list the supplies in the dt bindings but there are > none in the eDP PHY bindings. > > Looking at the driver in Linux, I can see that there seem to be two > relevant supplies: "vdda-phy" and "vdda-pll". Let's add those to the > bindings. > > NOTE: from looking at the Qualcomm datasheet for sc7280, it's not > immediately clear how to figure out how to fill in these supplies. The > only two eDP related supplies are simply described as "power for eDP > 0.9V circuits" and "power for eDP 1.2V circuits". From guessing and > from comparing how a similar PHY is hooked up on other similar > Qualcomm boards, I'll make the educated guess that the 1.2V supply > goes to "vdda-phy" and the 0.9V supply goes to "vdda-pll" and I'll use > that in the example here. > > Signed-off-by: Douglas Anderson > --- > > Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 6 ++ > 1 file changed, 6 insertions(+) Even though patch #1 in this series should be dropped, this patch (patch #2) is still valid. Vinod: I assume this would land in your tree along with the first two patches in Kuogee's series [1], which are related. Please let me know if you need me to re-send or anything. Thanks! [1] https://lore.kernel.org/r/1653507433-22585-1-git-send-email-quic_khs...@quicinc.com/ -Doug
Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver
On 6/16/22 07:17, Venkateshwar Rao Gannavarapu wrote: > diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig > index f9cf93c..a75bd76 100644 > --- a/drivers/gpu/drm/xlnx/Kconfig > +++ b/drivers/gpu/drm/xlnx/Kconfig > @@ -1,3 +1,15 @@ > +config DRM_XLNX_DSI > + tristate "Xilinx DRM DSI Subsystem Driver" > + depends on ARCH_ZYNQMP || COMPILE_TEST > + depends on DRM && OF > + select DRM_KMS_HELPER > + select DRM_MIPI_DSI > + select DRM_PANEL_BRIDGE > + help > + DRM bridge driver for Xilinx programmable DSI subsystem controller. > + choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in Choose > + video pipeline. -- ~Randy
Re: [Freedreno] [PATCH] drm/msm/dpu: set preferred mode for writeback connector
On 6/16/2022 1:36 AM, Dmitry Baryshkov wrote: On 16/06/2022 02:23, Abhinav Kumar wrote: After [1] was merged to IGT, we use either the first supported mode in the list OR the preferred mode to determine the primary plane to use for the sub-test due to the IGT API [2]. Since writeback does not set any preferred mode, this was selecting 4k as that was the first entry in the list. We use maxlinewidth to add the list of supported modes for the writeback connector which is the right thing to do, however since we do not have dual-SSPP support yet for DPU, this fails the bandwidth check in dpu_core_perf_crtc_check(). Till we have dual-SSPP support, workaround this mismatch between the list of supported modes and maxlinewidth limited modes by marking 640x480 as the preferred mode for DPU writeback because kms_writeback tests 640x480 mode only [3]. Telling that we support modes up to 4k, failing to set 4k mode and then using the preferred mode to force IGT to lower resolution sounds like a hack. As adding wide dual-SSPP support will take some time. I'd suggest dropping support for 4k modes for now and using DEFAULT_DPU_LINE_WIDTH instead (or hw_wb->caps->maxlinewidth). A comment in the source code that the check should be removed/modified once dual-SSPP is supported would be helpful. Yes, I am planning to drop this one and use max_mixerwidth instead as i posted on IRC. [1]: https://patchwork.freedesktop.org/patch/486441/ [2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/lib/igt_kms.c#L1562 [3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L68 Signed-off-by: Abhinav Kumar Any Fixes tags? Yes, will add it in the new patch. --- drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c index 399115e4e217..104cc59d6466 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c @@ -10,9 +10,14 @@ static int dpu_wb_conn_get_modes(struct drm_connector *connector) struct drm_device *dev = connector->dev; struct msm_drm_private *priv = dev->dev_private; struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); + int count; - return drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, + count = drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, dev->mode_config.max_height); + + drm_set_preferred_mode(connector, 640, 480); + + return count; } static const struct drm_connector_funcs dpu_wb_conn_funcs = {
Re: [PATCH 04/11] drm/r128: Fix undefined behavior due to shift overflowing the constant
On 5/19/22 06:05, Daniel Vetter wrote: > On Mon, Apr 25, 2022 at 11:46:53AM -0700, Randy Dunlap wrote: >> >> >> On 4/5/22 08:15, Borislav Petkov wrote: >>> From: Borislav Petkov >>> >>> Fix: >>> >>> drivers/gpu/drm/r128/r128_cce.c: In function ‘r128_do_init_cce’: >>> drivers/gpu/drm/r128/r128_cce.c:417:2: error: case label does not reduce >>> to an integer constant >>> case R128_PM4_64BM_64VCBM_64INDBM: >>> ^~~~ >>> drivers/gpu/drm/r128/r128_cce.c:418:2: error: case label does not reduce >>> to an integer constant >>> case R128_PM4_64PIO_64VCPIO_64INDPIO: >>> ^~~~ >>> >>> See https://lore.kernel.org/r/ykwq6%2btih8gqp...@zn.tnic for the gory >>> details as to why it triggers with older gccs only. >>> >>> Signed-off-by: Borislav Petkov >>> Cc: David Airlie >>> Cc: Daniel Vetter >>> Cc: Alex Deucher >>> Cc: dri-devel@lists.freedesktop.org >> >> Reviewed-by: Randy Dunlap > > Pushed to drm-misc-next, thanks for patch&review. > -Daniel > Hi, Will this be merged to mainline any time soonish? thanks. >> >> Thanks. >> >>> --- >>> drivers/gpu/drm/r128/r128_drv.h | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/r128/r128_drv.h >>> b/drivers/gpu/drm/r128/r128_drv.h >>> index 2e1bc01aa5c9..970e192b0d51 100644 >>> --- a/drivers/gpu/drm/r128/r128_drv.h >>> +++ b/drivers/gpu/drm/r128/r128_drv.h >>> @@ -300,8 +300,8 @@ extern long r128_compat_ioctl(struct file *filp, >>> unsigned int cmd, >>> # define R128_PM4_64PIO_128INDBM (5 << 28) >>> # define R128_PM4_64BM_128INDBM (6 << 28) >>> # define R128_PM4_64PIO_64VCBM_64INDBM(7 << 28) >>> -# define R128_PM4_64BM_64VCBM_64INDBM (8 << 28) >>> -# define R128_PM4_64PIO_64VCPIO_64INDPIO (15 << 28) >>> +# define R128_PM4_64BM_64VCBM_64INDBM (8U << 28) >>> +# define R128_PM4_64PIO_64VCPIO_64INDPIO (15U << 28) >>> # define R128_PM4_BUFFER_CNTL_NOUPDATE(1 << 27) >>> >>> #define R128_PM4_BUFFER_WM_CNTL0x0708 >> >> -- >> ~Randy > -- ~Randy
Re: [PATCH 00/10] drm: selftest: Convert to KUnit
On 6/16/22 16:55, David Gow wrote: > On Wed, Jun 15, 2022 at 9:59 PM Maíra Canal wrote: >> >> KUnit unifies the test structure and provides helper tools that simplify >> the development of tests. The basic use case allows running tests as regular >> processes, which makes it easier to run unit tests on a development machine >> and to integrate the tests into a CI system. >> >> That said, the conversion of selftests for DRM to KUnit tests is beneficial >> as it unifies the testing API by using the KUnit API. >> >> KUnit is beneficial for developers as it eases the process to run unit tests. >> It is possible to run the tests by using the kunit-tool on userspace with the >> following command: >> >> ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests >> --arch=x86_64 >> >> For CI system, it is possible to execute during the build. But, we also think >> about IGT: we are developing a patch to introduce KUnit to IGT. >> >> These patches were developed during a KUnit hackathon [0] last October. Now, >> we believe that both the IGT side and the Kernel side are in good shape for >> submission. >> >> If you are willing to check the output, here is the Pastebin with the output >> and execution times [1]. >> >> [0] https://groups.google.com/g/kunit-dev/c/YqFR1q2uZvk/m/IbvItSfHBAAJ >> [1] https://pastebin.com/FJjLPKsC >> >> - Arthur Grillo, Isabella Basso, and Maíra Canal > > Great to see these going upstream! > Indeed, this is pretty awesome! I haven't reviewed the patches yet but just have a meta comment. There's a TODO entry for this [0] in Documentation/gpu/todo.rst, so I think that you could add a patch removing that as a part of this series. [0]: https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n620 -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
[PATCH v2] drm: shmobile: Use backlight helper
This started with work on the removal of backlight_properties' deprecated fb_blank field, much of which can be taken care of by using helper functions provided by backlight.h instead of directly accessing fields in backlight_properties. This patch series doesn't involve fb_blank, but it still seems useful to use helper functions where appropriate. Instead of retrieving the backlight brightness in struct backlight_properties manually, and then checking whether the backlight should be on at all, use backlight_get_brightness() which does all this and insulates this from future changes. Signed-off-by: Stephen Kitt Cc: Laurent Pinchart Cc: Kieran Bingham Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org --- Changes since v1: clarified commit message, this doesn't touch fb_blank --- drivers/gpu/drm/shmobile/shmob_drm_backlight.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/shmobile/shmob_drm_backlight.c b/drivers/gpu/drm/shmobile/shmob_drm_backlight.c index f6628a5ee95f..794573badfe8 100644 --- a/drivers/gpu/drm/shmobile/shmob_drm_backlight.c +++ b/drivers/gpu/drm/shmobile/shmob_drm_backlight.c @@ -18,11 +18,7 @@ static int shmob_drm_backlight_update(struct backlight_device *bdev) struct shmob_drm_connector *scon = bl_get_data(bdev); struct shmob_drm_device *sdev = scon->connector.dev->dev_private; const struct shmob_drm_backlight_data *bdata = &sdev->pdata->backlight; - int brightness = bdev->props.brightness; - - if (bdev->props.power != FB_BLANK_UNBLANK || - bdev->props.state & BL_CORE_SUSPENDED) - brightness = 0; + int brightness = backlight_get_brightness(bdev); return bdata->set_brightness(brightness); } base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56 -- 2.30.2
[PATCH v8 0/2] force link training for display resolution change
1) force link training for display resolution change 2) remove pixel_rate from struct dp_ctrl Kuogee Hsieh (2): drm/msm/dp: force link training for display resolution change drm/msm/dp: clean up pixel_rate from dp_ctrl.c drivers/gpu/drm/msm/dp/dp_ctrl.c| 147 drivers/gpu/drm/msm/dp/dp_ctrl.h| 3 +- drivers/gpu/drm/msm/dp/dp_display.c | 13 ++-- 3 files changed, 88 insertions(+), 75 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v8 1/2] drm/msm/dp: force link training for display resolution change
Display resolution change is implemented through drm modeset. Older modeset (resolution) has to be disabled first before newer modeset (resolution) can be enabled. Display disable will turn off both pixel clock and main link clock so that main link have to be re-trained during display enable to have new video stream flow again. At current implementation, display enable function manually kicks up irq_hpd_handle which will read panel link status and start link training if link status is not in sync state. However, there is rare case that a particular panel links status keep staying in sync for some period of time after main link had been shut down previously at display disabled. In this case, main link retraining will not be executed by irq_hdp_handle(). Hence video stream of newer display resolution will fail to be transmitted to panel due to main link is not in sync between host and panel. This patch will bypass irq_hpd_handle() in favor of directly call dp_ctrl_on_stream() to always perform link training in regardless of main link status. So that no unexpected exception resolution change failure cases will happen. Also this implementation are more efficient than manual kicking off irq_hpd_handle function. Changes in v2: -- set force_link_train flag on DP only (is_edp == false) Changes in v3: -- revise commit text -- add Fixes tag Changes in v4: -- revise commit text Changes in v5: -- fix spelling at commit text Changes in v6: -- split dp_ctrl_on_stream() for phy test case -- revise commit text for modeset Changes in v7: -- drop 0 assignment at local variable (ret = 0) Changes in v8: -- add patch to remove pixel_rate from dp_ctrl Fixes: 62671d2ef24b ("drm/msm/dp: fixes wrong connection state caused by failure of link train") Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/dp/dp_ctrl.c| 31 +++ drivers/gpu/drm/msm/dp/dp_ctrl.h| 3 ++- drivers/gpu/drm/msm/dp/dp_display.c | 13 ++--- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index af7a80c..01028b5 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1551,7 +1551,7 @@ static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl) ret = dp_ctrl_on_link(&ctrl->dp_ctrl); if (!ret) - ret = dp_ctrl_on_stream(&ctrl->dp_ctrl); + ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl); else DRM_ERROR("failed to enable DP link controller\n"); @@ -1807,7 +1807,27 @@ static int dp_ctrl_link_retrain(struct dp_ctrl_private *ctrl) return dp_ctrl_setup_main_link(ctrl, &training_step); } -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl) +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl) +{ + int ret; + struct dp_ctrl_private *ctrl; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock; + + ret = dp_ctrl_enable_stream_clocks(ctrl); + if (ret) { + DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret); + return ret; + } + + dp_ctrl_send_phy_test_pattern(ctrl); + + return 0; +} + +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train) { int ret = 0; bool mainlink_ready = false; @@ -1843,12 +1863,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl) goto end; } - if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) { - dp_ctrl_send_phy_test_pattern(ctrl); - return 0; - } - - if (!dp_ctrl_channel_eq_ok(ctrl)) + if (force_link_train || !dp_ctrl_channel_eq_ok(ctrl)) dp_ctrl_link_retrain(ctrl); /* stop txing train pattern to end link training */ diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h index 0745fde..9a39b00 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h @@ -21,7 +21,8 @@ struct dp_ctrl { }; int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl); -int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl); +int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train); +int dp_ctrl_on_stream_phy_test_report(struct dp_ctrl *dp_ctrl); int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl); int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl); int dp_ctrl_off(struct dp_ctrl *dp_ctrl); diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index c388323..b6d25ab 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -872,7 +872,7 @@ static int dp_display_enable(struct dp_display_private *dp, u32 data) return 0; } - rc = dp_ctrl_on_stream(dp->ctrl); + rc = dp_ctrl_on_stream(dp->ctrl, data); if (!rc) dp
[PATCH v8 2/2] drm/msm/dp: clean up pixel_rate from dp_ctrl.c
dp_ctrl keep an local cache of pixel_rate which increase confusing in regrading how pixel_rate being used. This patch refer pixel_rate directly from dp_panel to eliminate unnecessary pixel_rate variable from struct dp_ctrl. Changes in v8: -- add this patch to remove pixel_rate from dp_ctrl Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/dp/dp_ctrl.c | 158 +++ drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 - 2 files changed, 79 insertions(+), 81 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 01028b5..6fd 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1237,8 +1237,6 @@ static int dp_ctrl_link_train_2(struct dp_ctrl_private *ctrl, return -ETIMEDOUT; } -static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl); - static int dp_ctrl_link_train(struct dp_ctrl_private *ctrl, int *training_step) { @@ -1337,7 +1335,8 @@ static void dp_ctrl_set_clock_rate(struct dp_ctrl_private *ctrl, name, rate); } -static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl) +static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl, + unsigned long pixel_rate) { int ret = 0; struct dp_io *dp_io = &ctrl->parser->io; @@ -1358,25 +1357,25 @@ static int dp_ctrl_enable_mainlink_clocks(struct dp_ctrl_private *ctrl) if (ret) DRM_ERROR("Unable to start link clocks. ret=%d\n", ret); - drm_dbg_dp(ctrl->drm_dev, "link rate=%d pixel_clk=%d\n", - ctrl->link->link_params.rate, ctrl->dp_ctrl.pixel_rate); + drm_dbg_dp(ctrl->drm_dev, "link rate=%d pixel_clk=%lu\n", + ctrl->link->link_params.rate, pixel_rate); return ret; } -static int dp_ctrl_enable_stream_clocks(struct dp_ctrl_private *ctrl) +static int dp_ctrl_enable_stream_clocks(struct dp_ctrl_private *ctrl, + unsigned long pixel_rate) { - int ret = 0; + int ret; - dp_ctrl_set_clock_rate(ctrl, DP_STREAM_PM, "stream_pixel", - ctrl->dp_ctrl.pixel_rate * 1000); + dp_ctrl_set_clock_rate(ctrl, DP_STREAM_PM, "stream_pixel", pixel_rate * 1000); ret = dp_power_clk_enable(ctrl->power, DP_STREAM_PM, true); if (ret) DRM_ERROR("Unabled to start pixel clocks. ret=%d\n", ret); - drm_dbg_dp(ctrl->drm_dev, "link rate=%d pixel_clk=%d\n", - ctrl->link->link_params.rate, ctrl->dp_ctrl.pixel_rate); + drm_dbg_dp(ctrl->drm_dev, "link rate=%d pixel_clk=%lu\n", + ctrl->link->link_params.rate, pixel_rate); return ret; } @@ -1441,7 +1440,8 @@ static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl) return false; } -static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl) +static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl, + unsigned long pixel_rate) { int ret = 0; struct dp_io *dp_io = &ctrl->parser->io; @@ -1465,7 +1465,7 @@ static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl) /* hw recommended delay before re-enabling clocks */ msleep(20); - ret = dp_ctrl_enable_mainlink_clocks(ctrl); + ret = dp_ctrl_enable_mainlink_clocks(ctrl, pixel_rate); if (ret) { DRM_ERROR("Failed to enable mainlink clks. ret=%d\n", ret); return ret; @@ -1513,8 +1513,6 @@ static int dp_ctrl_link_maintenance(struct dp_ctrl_private *ctrl) ctrl->link->phy_params.p_level = 0; ctrl->link->phy_params.v_level = 0; - ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock; - ret = dp_ctrl_setup_main_link(ctrl, &training_step); if (ret) goto end; @@ -1528,36 +1526,6 @@ static int dp_ctrl_link_maintenance(struct dp_ctrl_private *ctrl) return ret; } -static int dp_ctrl_process_phy_test_request(struct dp_ctrl_private *ctrl) -{ - int ret = 0; - - if (!ctrl->link->phy_params.phy_test_pattern_sel) { - drm_dbg_dp(ctrl->drm_dev, - "no test pattern selected by sink\n"); - return ret; - } - - /* -* The global reset will need DP link related clocks to be -* running. Add the global reset just before disabling the -* link clocks and core clocks. -*/ - ret = dp_ctrl_off(&ctrl->dp_ctrl); - if (ret) { - DRM_ERROR("failed to disable DP controller\n"); - return ret; - } - - ret = dp_ctrl_on_link(&ctrl->dp_ctrl); - if (!ret) - ret = dp_ctrl_on_stream_phy_test_report(&ctrl->dp_ctrl); - else - DRM_ERROR("failed to enable DP link c
Re: [PATCH 1/2] dt-bindings: display: simple: add Ampire AM-800600P5TMQW-TB8H panel
On Fri, 10 Jun 2022 13:15:10 +0200, Bastian Krause wrote: > Add Ampire AM-800600P5TMQW-TB8H 8" TFT LCD panel compatible string. > > Signed-off-by: Bastian Krause > --- > .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring
[PATCH v2 0/3] drm/panel: Use backlight helpers
backlight_properties.fb_blank is deprecated. The states it represents are handled by other properties; but instead of accessing those properties directly, drivers should use the helpers provided by backlight.h. This will ultimately allow fb_blank to be removed. Changes since v1: - remove the last remaining fb_blank reference in drm/panel in the last patch - remove unnecessary parentheses Stephen Kitt (3): drm/panel: Use backlight helper drm/panel: panel-dsi-cm: Use backlight helpers drm/panel: sony-acx565akm: Use backlight helpers .../drm/panel/panel-asus-z00t-tm5p5-n35596.c | 7 + drivers/gpu/drm/panel/panel-dsi-cm.c | 29 --- drivers/gpu/drm/panel/panel-sony-acx565akm.c | 12 ++-- 3 files changed, 9 insertions(+), 39 deletions(-) base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56 -- 2.30.2
[PATCH v2 2/3] drm/panel: panel-dsi-cm: Use backlight helpers
Instead of retrieving the backlight brightness in struct backlight_properties manually, and then checking whether the backlight should be on at all, use backlight_get_brightness() which does all this and insulates this from future changes. Instead of setting the power state by manually updating fields in struct backlight_properties, use backlight_enable() and backlight_disable(). These also call backlight_update_status() so the separate call is no longer needed. Signed-off-by: Stephen Kitt Cc: Thierry Reding Cc: Sam Ravnborg Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org --- Changes since v1: removed unnecessary parentheses --- drivers/gpu/drm/panel/panel-dsi-cm.c | 29 ++-- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c index b58cb064975f..b0213a518f9d 100644 --- a/drivers/gpu/drm/panel/panel-dsi-cm.c +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c @@ -85,17 +85,10 @@ static void dsicm_bl_power(struct panel_drv_data *ddata, bool enable) else return; - if (enable) { - backlight->props.fb_blank = FB_BLANK_UNBLANK; - backlight->props.state = ~(BL_CORE_FBBLANK | BL_CORE_SUSPENDED); - backlight->props.power = FB_BLANK_UNBLANK; - } else { - backlight->props.fb_blank = FB_BLANK_NORMAL; - backlight->props.power = FB_BLANK_POWERDOWN; - backlight->props.state |= BL_CORE_FBBLANK | BL_CORE_SUSPENDED; - } - - backlight_update_status(backlight); + if (enable) + backlight_enable(backlight); + else + backlight_disable(backlight); } static void hw_guard_start(struct panel_drv_data *ddata, int guard_msec) @@ -196,13 +189,7 @@ static int dsicm_bl_update_status(struct backlight_device *dev) { struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev); int r = 0; - int level; - - if (dev->props.fb_blank == FB_BLANK_UNBLANK && - dev->props.power == FB_BLANK_UNBLANK) - level = dev->props.brightness; - else - level = 0; + int level = backlight_get_brightness(dev); dev_dbg(&ddata->dsi->dev, "update brightness to %d\n", level); @@ -219,11 +206,7 @@ static int dsicm_bl_update_status(struct backlight_device *dev) static int dsicm_bl_get_intensity(struct backlight_device *dev) { - if (dev->props.fb_blank == FB_BLANK_UNBLANK && - dev->props.power == FB_BLANK_UNBLANK) - return dev->props.brightness; - - return 0; + return backlight_get_brightness(dev); } static const struct backlight_ops dsicm_bl_ops = { -- 2.30.2
[PATCH v2 3/3] drm/panel: sony-acx565akm: Use backlight helpers
Instead of retrieving the backlight brightness in struct backlight_properties manually, and then checking whether the backlight should be on at all, use backlight_get_brightness() which does all this and insulates this from future changes. Instead of manually checking the power state in struct backlight_properties, use backlight_is_blank(). While we're at it, drop .fb_blank from the initialisation function; it is deprecated, and this helps make progress towards enabling its removal. This change makes no functional difference since FB_BLANK_UNBLANK is the default value. Signed-off-by: Stephen Kitt Cc: Thierry Reding Cc: Sam Ravnborg Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org --- Changes since v1: removed the last remaining .fb_blank reference --- drivers/gpu/drm/panel/panel-sony-acx565akm.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-sony-acx565akm.c b/drivers/gpu/drm/panel/panel-sony-acx565akm.c index 0d7541a33f87..3d6a286056a0 100644 --- a/drivers/gpu/drm/panel/panel-sony-acx565akm.c +++ b/drivers/gpu/drm/panel/panel-sony-acx565akm.c @@ -298,13 +298,7 @@ static void acx565akm_set_brightness(struct acx565akm_panel *lcd, int level) static int acx565akm_bl_update_status_locked(struct backlight_device *dev) { struct acx565akm_panel *lcd = dev_get_drvdata(&dev->dev); - int level; - - if (dev->props.fb_blank == FB_BLANK_UNBLANK && - dev->props.power == FB_BLANK_UNBLANK) - level = dev->props.brightness; - else - level = 0; + int level = backlight_get_brightness(dev); acx565akm_set_brightness(lcd, level); @@ -330,8 +324,7 @@ static int acx565akm_bl_get_intensity(struct backlight_device *dev) mutex_lock(&lcd->mutex); - if (dev->props.fb_blank == FB_BLANK_UNBLANK && - dev->props.power == FB_BLANK_UNBLANK) + if (!backlight_is_blank(dev)) intensity = acx565akm_get_actual_brightness(lcd); else intensity = 0; @@ -349,7 +342,6 @@ static const struct backlight_ops acx565akm_bl_ops = { static int acx565akm_backlight_init(struct acx565akm_panel *lcd) { struct backlight_properties props = { - .fb_blank = FB_BLANK_UNBLANK, .power = FB_BLANK_UNBLANK, .type = BACKLIGHT_RAW, }; -- 2.30.2
[PATCH v2 1/3] drm/panel: Use backlight helper
backlight_properties.fb_blank is deprecated. The states it represents are handled by other properties; but instead of accessing those properties directly, drivers should use the helpers provided by backlight.h. Instead of retrieving the backlight brightness in struct backlight_properties manually, and then checking whether the backlight should be on at all, use backlight_get_brightness() which does all this and insulates this from future changes. Signed-off-by: Stephen Kitt Cc: Thierry Reding Cc: Sam Ravnborg Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c b/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c index 44674ebedf59..174ff434bd71 100644 --- a/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c +++ b/drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c @@ -215,14 +215,9 @@ static const struct drm_panel_funcs tm5p5_nt35596_panel_funcs = { static int tm5p5_nt35596_bl_update_status(struct backlight_device *bl) { struct mipi_dsi_device *dsi = bl_get_data(bl); - u16 brightness = bl->props.brightness; + u16 brightness = backlight_get_brightness(bl); int ret; - if (bl->props.power != FB_BLANK_UNBLANK || - bl->props.fb_blank != FB_BLANK_UNBLANK || - bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) - brightness = 0; - dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness); -- 2.30.2
Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
Hi! Javier Martinez Canillas wrote: > Before merging this, could you please reach the folks working on [0] ? > I think that would be good to have some consistency with regard to KUnit > tests from the start to avoid future refactorings. For instance, you are > adding the tests under a `kunit` sub-directory while they are doing it > in a `tests` sub-dir. > > Also there may be other things that could be made more consistent, like > the naming conventions for the tests, suites, etc. > > [0]: https://lore.kernel.org/all/20220608010709.272962-4-maira.ca...@usp.br/T/ David Gow wrote: > [+Maíra, Isabella, Tales, Magali for other drm,amdgpu,KUnit work.] > > These seem pretty good to me, but I'd echo Javier's comments about > consistency with other DRM tests. I agree, I'd need to look with more detail into the selftest conversion and the AMD series, but it'd be nice to avoid unnecessary refactors. > In particular, we now have three concurrently developed DRM-related > test suites, each doing things slightly differently: > - This series is putting tests in drm/kunit, and providing a > .kunitconfig in that directory, > > - The selftest ports here[1] are putting tests in drm/tests, and > provide a separate Kconfig file, as well as a .kunitconfig Now that "selftests/" is going to be removed, "tests/" is a good name for the folder, I'll rename it in v4. > - And the AMDGPU tests[2] are doing something totally different, with > their own tests in drm/amd/display/amdgpu_dm/tests, which get compiled > directly into the amdgpu module (and, at present, can't be run at all > via kunit_tool) > > Certainly the general DRM tests should be in the same place, and use > the same Kconfig entries, etc. A mix of the separate Kconfig file from > [1] (if there's enough benefit to having the ability to turn on and > off suites individually, which seems plausible) and the documentation > from this series seems good to me. I agree about having the DRM-core tests in drm/tests/ and driver tests in drm/driver/tests/. About allowing turning on or off test suites, it was agreed to use a generic symbol to group them (DRM_KUNIT_TEST) [1]. I won't have time merge all patches toghether and run them until next week, but if it takes too long to run them or you find it beneficial to split the symbols, I'll change my patch. [1] https://lore.kernel.org/dri-devel/e26de140-afb7-7b1b-4826-6ac4f3a4f...@redhat.com/ > There's some basic guidelines around test nomenclature in > Documentation/dev-tools/kunit/style.rst[3], though all of these > patches seem pretty consistent with that. Either 'kunit' or 'tests' > would work as a directory name: given the AMDGPU patches are using > 'tests', maybe that's easier to stick with. I'll have to rename my kunit_suite to use underscores, as well as the test cases, that at the moment are using English sentences. Maíra: We'd also need to agree on the file names used, the documentation [2] suggest to use *_test.c, it'd need to be changed in the selftest to KUnit series. Best wishes, Jose [2] https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-file-and-module-names > Cheers, > -- David > > [1]: > https://lore.kernel.org/linux-kselftest/20220615135824.15522-1-maira.ca...@usp.br/ > [2]: > https://lore.kernel.org/dri-devel/20220608010709.272962-1-maira.ca...@usp.br/ > [3]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html
[Bug 216119] 087451f372bf76d breaks hibernation on amdgpu Radeon R9 390
https://bugzilla.kernel.org/show_bug.cgi?id=216119 Harald Judt (h.j...@gmx.at) changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|ANSWERED|--- --- Comment #4 from Harald Judt (h.j...@gmx.at) --- I have tried 5.18.4 which includes this patch, and I also think 5.18.3 came with this patch. Unfortunately, this does not fix hibernation for me, though the symptoms are a bit different: No longer can I see any kernel messages like before (see comment #1), but the screen stays black. I can reboot my kernel with keyboard sysrq keys, though. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] drm/fourcc: Add formats for packed YUV 4:4:4 AVUY and XVUY permutations
Add FourCCs for two missing permutations of the packed YUV 4:4:4 color components, namely AVUY and XVUY. These formats are needed by the NXP i.MX8 ISI. While the ISI is supported by a V4L2 device (corresponding formats have been submitted to V4L2), it is handled in userspace by libcamera, which uses DRM FourCCs for pixel formats. Signed-off-by: Laurent Pinchart --- include/uapi/drm/drm_fourcc.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index f1972154a594..399d950c53e3 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -205,7 +205,9 @@ extern "C" { #define DRM_FORMAT_VYUYfourcc_code('V', 'Y', 'U', 'Y') /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */ #define DRM_FORMAT_AYUVfourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ +#define DRM_FORMAT_AVUYfourcc_code('A', 'V', 'U', 'Y') /* [31:0] A:Cr:Cb:Y 8:8:8:8 little endian */ #define DRM_FORMAT_XYUVfourcc_code('X', 'Y', 'U', 'V') /* [31:0] X:Y:Cb:Cr 8:8:8:8 little endian */ +#define DRM_FORMAT_XVUYfourcc_code('X', 'V', 'U', 'Y') /* [31:0] X:Cr:Cb:Y 8:8:8:8 little endian */ #define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] Cr:Cb:Y 8:8:8 little endian */ #define DRM_FORMAT_VUY101010 fourcc_code('V', 'U', '3', '0') /* Y followed by U then V, 10:10:10. Non-linear modifier only */ -- Regards, Laurent Pinchart
Re: ✗ Fi.CI.IGT: failure for drm/dp/mst: Read the extended DPCD capabilities during system resume
On Wed, Jun 15, 2022 at 04:25:34AM +, Patchwork wrote: > == Series Details == > > Series: drm/dp/mst: Read the extended DPCD capabilities during system resume > URL : https://patchwork.freedesktop.org/series/105102/ > State : failure Thanks for the reviews, pushed the patch to drm-misc-next also adding Cc: stable. The failure is unrelated, see below. > == Summary == > > CI Bug Log - changes from CI_DRM_11756_full -> Patchwork_105102v1_full > > > Summary > --- > > **FAILURE** > > Serious unknown changes coming with Patchwork_105102v1_full absolutely need > to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_105102v1_full, please notify your bug team to allow > them > to document this new failure mode, which will reduce false positives in CI. > > > > Participating hosts (13 -> 13) > -- > > No changes in participating hosts > > Possible new issues > --- > > Here are the unknown changes that may have been introduced in > Patchwork_105102v1_full: > > ### IGT changes ### > > Possible regressions > > * igt@gem_eio@reset-stress: > - shard-snb: [PASS][1] -> [TIMEOUT][2] >[1]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11756/shard-snb7/igt@gem_...@reset-stress.html >[2]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105102v1/shard-snb2/igt@gem_...@reset-stress.html <7> [109.948420] heartbeat vcs0 heartbeat {seqno:5:11771, prio:-2147483648} not ticking SNB doesn't support MST, so it's some unrelated issue. > New tests > - > > New tests have been introduced between CI_DRM_11756_full and > Patchwork_105102v1_full: > > ### New IGT tests (5) ### > > * igt@kms_atomic_interruptible@legacy-setmode@hdmi-a-3-pipe-a: > - Statuses : 1 pass(s) > - Exec time: [6.14] s > > * igt@kms_plane_lowres@tiling-none@pipe-a-hdmi-a-3: > - Statuses : 1 pass(s) > - Exec time: [8.02] s > > * igt@kms_plane_lowres@tiling-none@pipe-b-hdmi-a-3: > - Statuses : 1 pass(s) > - Exec time: [7.91] s > > * igt@kms_plane_lowres@tiling-none@pipe-c-hdmi-a-3: > - Statuses : 1 pass(s) > - Exec time: [7.93] s > > * igt@kms_plane_lowres@tiling-none@pipe-d-hdmi-a-3: > - Statuses : 1 pass(s) > - Exec time: [7.94] s > > > > Known issues > > > Here are the changes found in Patchwork_105102v1_full that come from known > issues: > > ### IGT changes ### > > Issues hit > > * igt@gem_eio@in-flight-contexts-1us: > - shard-apl: [PASS][3] -> [TIMEOUT][4] ([i915#3063]) >[3]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11756/shard-apl2/igt@gem_...@in-flight-contexts-1us.html >[4]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105102v1/shard-apl2/igt@gem_...@in-flight-contexts-1us.html > > * igt@gem_exec_balancer@parallel-keep-in-fence: > - shard-iclb: [PASS][5] -> [SKIP][6] ([i915#4525]) +2 similar > issues >[5]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11756/shard-iclb2/igt@gem_exec_balan...@parallel-keep-in-fence.html >[6]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105102v1/shard-iclb5/igt@gem_exec_balan...@parallel-keep-in-fence.html > > * igt@gem_exec_capture@pi@vcs0: > - shard-iclb: [PASS][7] -> [INCOMPLETE][8] ([i915#3371]) >[7]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11756/shard-iclb7/igt@gem_exec_capture@p...@vcs0.html >[8]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105102v1/shard-iclb1/igt@gem_exec_capture@p...@vcs0.html > > * igt@gem_exec_endless@dispatch@bcs0: > - shard-tglb: [PASS][9] -> [INCOMPLETE][10] ([i915#3778]) >[9]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11756/shard-tglb2/igt@gem_exec_endless@dispa...@bcs0.html >[10]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105102v1/shard-tglb1/igt@gem_exec_endless@dispa...@bcs0.html > > * igt@gem_exec_fair@basic-none-solo@rcs0: > - shard-kbl: NOTRUN -> [FAIL][11] ([i915#2842]) >[11]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105102v1/shard-kbl6/igt@gem_exec_fair@basic-none-s...@rcs0.html > > * igt@gem_exec_fair@basic-pace@bcs0: > - shard-tglb: [PASS][12] -> [FAIL][13] ([i915#2842]) +1 similar > issue >[12]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11756/shard-tglb1/igt@gem_exec_fair@basic-p...@bcs0.html >[13]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105102v1/shard-tglb2/igt@gem_exec_fair@basic-p...@bcs0.html > > * igt@gem_exec_fair@basic-pace@vecs0: > - shard-iclb: [PASS][14] -> [FAIL][15] ([i915#2842]) >[14]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11756/shard-iclb3/igt@gem_exec_fair@basic-p...@vecs0.html >[15]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchw