Re: [PATCH] drm/fbdev: Clamp fbdev surface size if too large
Hi Am 04.10.21 um 10:23 schrieb Ville Syrjälä: On Mon, Oct 04, 2021 at 10:15:06AM +0200, Thomas Zimmermann wrote: Clamp the fbdev surface size of the available maximum height to avoid failing to init console emulation. An example error is shown below. bad framebuffer height 2304, should be >= 768 && <= 768 [drm] Initialized simpledrm 1.0.0 20200625 for simple-framebuffer.0 on minor 0 simple-framebuffer simple-framebuffer.0: [drm] *ERROR* fbdev: Failed to setup generic emulation (ret=-22) This is especially a problem with drivers that have very small screen sizes and cannot over-allocate at all. Signed-off-by: Thomas Zimmermann Fixes: 11e8f5fd223b ("drm: Add simpledrm driver") Reported-by: Amanoel Dawod Reported-by: Zoltán Kővágó Reported-by: Michael Stapelberg Cc: Daniel Vetter Cc: Maxime Ripard Cc: dri-devel@lists.freedesktop.org Cc: # v5.14+ --- drivers/gpu/drm/drm_fb_helper.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 6860223f0068..364f11900b37 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1508,6 +1508,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, { struct drm_client_dev *client = &fb_helper->client; struct drm_device *dev = fb_helper->dev; + struct drm_mode_config *config = &dev->mode_config; int ret = 0; int crtc_count = 0; struct drm_connector_list_iter conn_iter; @@ -1665,6 +1666,11 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, /* Handle our overallocation */ sizes.surface_height *= drm_fbdev_overalloc; sizes.surface_height /= 100; + if (sizes.surface_height > config->max_height) { + drm_warn(dev, "Fbdev over-allocation too large; clamping height to %d\n", +config->max_height); drm_warn() seems a bit excessive. drm_info()? Or could just have no printk and use a simple min() perhaps. The framebuffer code uses debug_kms for this kind of message. I guess, I'll go with that. Best regards Thamas + sizes.surface_height = config->max_height; + } /* push down into drivers */ ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes); -- 2.33.0 -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature
[RFC PATCH] drm: Increase DRM_OBJECT_MAX_PROPERTY by 18.
The warning poped up, it says it increase it by the number of occurrence. I saw it 18 times so here it is. It started to up since commit 2f425cf5242a0 ("drm: Fix oops in damage self-tests by mocking damage property") Increase DRM_OBJECT_MAX_PROPERTY by 18. Signed-off-by: Sebastian Andrzej Siewior --- I have no idea whether this is correct or just a symptom of another problem. This has been observed with i915 and full debug. include/drm/drm_mode_object.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index c34a3e8030e12..1e5399e47c3a5 100644 --- a/include/drm/drm_mode_object.h +++ b/include/drm/drm_mode_object.h @@ -60,7 +60,7 @@ struct drm_mode_object { void (*free_cb)(struct kref *kref); }; -#define DRM_OBJECT_MAX_PROPERTY 24 +#define DRM_OBJECT_MAX_PROPERTY 42 /** * struct drm_object_properties - property tracking for &drm_mode_object */ -- 2.33.0
Re: [PATCH] DRM: delete DRM IRQ legacy midlayer docs
Hi Am 05.10.21 um 04:53 schrieb Randy Dunlap: Remove documentation associated with the removal of the DRM IRQ legacy midlayer. Eliminates these documentation warnings: ../drivers/gpu/drm/drm_irq.c:1: warning: 'irq helpers' not found ../drivers/gpu/drm/drm_irq.c:1: warning: no structured comments found Fixes: c1736b9008cb ("drm: IRQ midlayer is now legacy") Signed-off-by: Randy Dunlap Cc: Thomas Zimmermann Cc: Sam Ravnborg Cc: dri-devel@lists.freedesktop.org Cc: Jonathan Corbet Cc: linux-...@vger.kernel.org Cc: David Airlie Cc: Daniel Vetter Applied to drm-misc-fixes. Thanks for the patch. Best regards Thomas --- Documentation/gpu/drm-internals.rst |9 - 1 file changed, 9 deletions(-) --- lnx-515-rc4.orig/Documentation/gpu/drm-internals.rst +++ lnx-515-rc4/Documentation/gpu/drm-internals.rst @@ -111,15 +111,6 @@ Component Helper Usage .. kernel-doc:: drivers/gpu/drm/drm_drv.c :doc: component helper usage recommendations -IRQ Helper Library -~~ - -.. kernel-doc:: drivers/gpu/drm/drm_irq.c - :doc: irq helpers - -.. kernel-doc:: drivers/gpu/drm/drm_irq.c - :export: - Memory Manager Initialization ~ -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature
Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
On Mon, Oct 04, 2021 at 01:52:27PM -0700, Lucas De Marchi wrote: > Cc'ing Dan Carpenter > > On Fri, Oct 01, 2021 at 12:57:13PM +0300, Jani Nikula wrote: > > On Fri, 01 Oct 2021, Chris Wilson wrote: > > > Quoting Lucas De Marchi (2021-10-01 08:40:41) > > > > When trying to bring IS_ACTIVE to linux/kconfig.h I thought it wouldn't > > > > provide much value just encapsulating it in a boolean context. So I also > > > > added the support for handling undefined macros as the IS_ENABLED() > > > > counterpart. However the feedback received from Masahiro Yamada was that > > > > it is too ugly, not providing much value. And just wrapping in a boolean > > > > context is too dumb - we could simply open code it. > > > > > > > > As detailed in commit babaab2f4738 ("drm/i915: Encapsulate kconfig > > > > constant values inside boolean predicates"), the IS_ACTIVE macro was > > > > added to workaround a compilation warning. However after checking again > > > > our current uses of IS_ACTIVE it turned out there is only > > > > 1 case in which it would potentially trigger a warning. All the others > > > > can simply use the shorter version, without wrapping it in any macro. > > > > And even that single one didn't trigger any warning in gcc 10.3. > > > > > > > > So here I'm dialing all the way back to simply removing the macro. If it > > > > triggers warnings in future we may change the few cases to check for > 0 > > > > or != 0. Another possibility would be to use the great "not not > > > > operator" for all positive checks, which would allow us to maintain > > > > consistency. However let's try first the simplest form though, > > > > hopefully > > > > we don't hit broken compilers spitting a warning: > > > > > > You didn't prevent the compilation warning this re-introduces. > > > > > > drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: > > > should this be a bitwise op? > > > drivers/gpu/drm/i915/i915_request.c:1679 i915_request_wait() warn: should > > > this be a bitwise op? > > > > Looks like that's a Smatch warning. The immediate fix would be to just > > add the != 0 in the relevant places. But this is stuff that's just going > > to get broken again unless we add Smatch to CI. Most people aren't > > running it on a regular basis. I would really prefer that instead of ensuring that code doesn't generate Smatch warnings, people just look over the warnings and then mass mark them all as false positives and never look at them again. It let's us warn about more complicated things without worrying so much about being perfect. When code is fresh in your head then warnings are not a big deal to review and you want to warn about every possible issue After a year then they take forever and so you really want them to be correct or it's a huge waste of time. I'd prefer Smatch live in the space where people run it when the code is fresh. You would have received some automated emails about this Smatch warning but I look over the zero day output and filter the results. > > clang gives a warning only in drivers/gpu/drm/i915/i915_config.c and the > warning is gone if the condition swapped: > > - if (context && CONFIG_DRM_I915_FENCE_TIMEOUT) > + if (CONFIG_DRM_I915_FENCE_TIMEOUT && context) I like this rule that when the constant is on the left it's not a mask. That makes sense. I will add that. > > which would make sense if we think about shortcutting the if condition. > However smatch still reports the warning and an additional one > in drivers/gpu/drm/i915/i915_request.c. The ways I found to stop the > false positives with smatch are: > > if (context && CONFIG_DRM_I915_FENCE_TIMEOUT != 0) > or > if (context && !!CONFIG_DRM_I915_FENCE_TIMEOUT) > or > if (context && CONFIG_DRM_I915_FENCE_TIMEOUT > 0) > I guess I prefer the first and third but I'll add the rule that Clang uses to silence the warning. regards, dan carpenter
Re: [PATCH] drm/msm/disp: fix endian bug in debugfs code
On Tue, Oct 05, 2021 at 02:31:12AM +0300, Dmitry Baryshkov wrote: > On 04/10/2021 16:47, Dan Carpenter wrote: > > The "vbif->features" is type unsigned long but the debugfs file > > is treating it as a u32 type. This will work in little endian > > systems, but the correct thing is to change the debugfs to use > > an unsigned long. > > > > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") > > Signed-off-by: Dan Carpenter > > --- > > You might wonder why this code has so many casts. It's required because > > this data is const. Which is fine because the file is read only. > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > > index 21d20373eb8b..e645a886e3c6 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > > @@ -305,8 +305,8 @@ void dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, > > struct dentry *debugfs_root) > > debugfs_vbif = debugfs_create_dir(vbif_name, entry); > > - debugfs_create_u32("features", 0600, debugfs_vbif, > > - (u32 *)&vbif->features); > > + debugfs_create_ulong("features", 0600, debugfs_vbif, > > +(unsigned long *)&vbif->features); > > As you are converting this to the ulong file, could you please also remove > the now-unnecessary type cast? I wanted to remove all the casting but they are required because of the const. regards, dan carpenter
[PATCH v2] host1x: bus.c: drop excess kernel-doc entry @key
Fix kernel-doc warning in host1x: ../drivers/gpu/host1x/bus.c:774: warning: Excess function parameter 'key' description in '__host1x_client_register' Fixes: 0cfe5a6e758f ("gpu: host1x: Split up client initalization and registration") Signed-off-by: Randy Dunlap Cc: Thierry Reding Cc: dri-devel@lists.freedesktop.org Cc: linux-te...@vger.kernel.org Cc: David Airlie Cc: Daniel Vetter --- v2: rebase and resend drivers/gpu/host1x/bus.c |1 - 1 file changed, 1 deletion(-) --- lnx-515-rc4.orig/drivers/gpu/host1x/bus.c +++ lnx-515-rc4/drivers/gpu/host1x/bus.c @@ -761,7 +761,6 @@ EXPORT_SYMBOL(host1x_client_exit); /** * __host1x_client_register() - register a host1x client * @client: host1x client - * @key: lock class key for the client-specific mutex * * Registers a host1x client with each host1x controller instance. Note that * each client will only match their parent host1x controller and will only be
[PATCH] DRM: delete DRM IRQ legacy midlayer docs
Remove documentation associated with the removal of the DRM IRQ legacy midlayer. Eliminates these documentation warnings: ../drivers/gpu/drm/drm_irq.c:1: warning: 'irq helpers' not found ../drivers/gpu/drm/drm_irq.c:1: warning: no structured comments found Fixes: c1736b9008cb ("drm: IRQ midlayer is now legacy") Signed-off-by: Randy Dunlap Cc: Thomas Zimmermann Cc: Sam Ravnborg Cc: dri-devel@lists.freedesktop.org Cc: Jonathan Corbet Cc: linux-...@vger.kernel.org Cc: David Airlie Cc: Daniel Vetter --- Documentation/gpu/drm-internals.rst |9 - 1 file changed, 9 deletions(-) --- lnx-515-rc4.orig/Documentation/gpu/drm-internals.rst +++ lnx-515-rc4/Documentation/gpu/drm-internals.rst @@ -111,15 +111,6 @@ Component Helper Usage .. kernel-doc:: drivers/gpu/drm/drm_drv.c :doc: component helper usage recommendations -IRQ Helper Library -~~ - -.. kernel-doc:: drivers/gpu/drm/drm_irq.c - :doc: irq helpers - -.. kernel-doc:: drivers/gpu/drm/drm_irq.c - :export: - Memory Manager Initialization ~
[PATCH] drm/msm/dp: Shorten SETUP timeout
Found in the middle of a patch from Sankeerth was the reduction of the INIT_SETUP timeout from 10s to 100ms. Upon INIT_SETUP timeout the host is initalized and HPD interrupt start to be serviced, so in the case of eDP this reduction improves the user experience dramatically - i.e. removes 9.9s of bland screen time at boot. Suggested-by: Sankeerth Billakanti Signed-off-by: Bjorn Andersson --- drivers/gpu/drm/msm/dp/dp_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 21b9c1de4ecb..46d9f3eb6d13 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1438,7 +1438,7 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display) dp_hpd_event_setup(dp); - dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100); + dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 1); } void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor) -- 2.29.2
[RFC] drm/msm/dp: Add typec_mux implementation
Implement a typec_mux in order to allow a Type-C controller to signal the connection and attention of DisplayPort to the related USB-C port. The remains of support for something along this lines was left in the dp_display as the driver was upstreamed, so these are reused with minimal modifications necessary. When operating in this mode, HPD interrupts has still been observed in the ISR so, in line with the downstream kernel, these are ignored. Signed-off-by: Bjorn Andersson --- This applies on top of https://lore.kernel.org/linux-arm-msm/20211001180058.1021913-1-bjorn.anders...@linaro.org/ drivers/gpu/drm/msm/Kconfig | 1 + drivers/gpu/drm/msm/dp/dp_display.c | 52 --- drivers/gpu/drm/msm/dp/dp_hpd.c | 54 + 3 files changed, 87 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 5879f67bc88c..4e4b98c448cb 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -9,6 +9,7 @@ config DRM_MSM depends on QCOM_OCMEM || QCOM_OCMEM=n depends on QCOM_LLCC || QCOM_LLCC=n depends on QCOM_COMMAND_DB || QCOM_COMMAND_DB=n + depends on TYPEC || TYPEC=n select IOMMU_IO_PGTABLE select QCOM_MDT_LOADER if ARCH_QCOM select REGULATOR diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 56a79aeffed4..e863f537047a 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -85,6 +85,8 @@ struct dp_display_private { bool hpd_irq_on; bool audio_supported; + bool use_hw_hpd; + struct platform_device *pdev; struct dentry *root; @@ -466,11 +468,10 @@ static int dp_display_handle_irq_hpd(struct dp_display_private *dp) return 0; } -static int dp_display_usbpd_attention_cb(struct device *dev) +static int dp_display_usbpd_attention(struct dp_display_private *dp) { int rc = 0; u32 sink_request; - struct dp_display_private *dp = dev_get_dp_display_private(dev); /* check for any test request issued by sink */ rc = dp_link_process_request(dp->link); @@ -690,7 +691,7 @@ static int dp_irq_hpd_handle(struct dp_display_private *dp, u32 data) return 0; } - ret = dp_display_usbpd_attention_cb(&dp->pdev->dev); + ret = dp_display_usbpd_attention(dp); if (ret == -ECONNRESET) { /* cable unplugged */ dp->core_initialized = false; } @@ -709,6 +710,13 @@ static void dp_display_deinit_sub_modules(struct dp_display_private *dp) dp_audio_put(dp->audio); } +static int dp_display_usbpd_attention_cb(struct device *dev) +{ + struct dp_display_private *dp = dev_get_dp_display_private(dev); + + return dp_irq_hpd_handle(dp, 0); +} + static int dp_init_sub_modules(struct dp_display_private *dp) { int rc = 0; @@ -731,6 +739,8 @@ static int dp_init_sub_modules(struct dp_display_private *dp) goto error; } + dp->use_hw_hpd = !of_property_read_bool(dev->of_node, "mode-switch"); + dp->parser = dp_parser_get(dp->pdev); if (IS_ERR(dp->parser)) { rc = PTR_ERR(dp->parser); @@ -1135,27 +1145,29 @@ static irqreturn_t dp_display_irq_handler(int irq, void *dev_id) return IRQ_NONE; } - hpd_isr_status = dp_catalog_hpd_get_intr_status(dp->catalog); + if (dp->use_hw_hpd) { + hpd_isr_status = dp_catalog_hpd_get_intr_status(dp->catalog); - DRM_DEBUG_DP("hpd isr status=%#x\n", hpd_isr_status); - if (hpd_isr_status & 0x0F) { - /* hpd related interrupts */ - if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK) - dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0); + DRM_DEBUG_DP("hpd isr status=%#x\n", hpd_isr_status); + if (hpd_isr_status & 0x0F) { + /* hpd related interrupts */ + if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK) + dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0); - if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK) { - /* stop sentinel connect pending checking */ - dp_del_event(dp, EV_CONNECT_PENDING_TIMEOUT); - dp_add_event(dp, EV_IRQ_HPD_INT, 0, 0); - } + if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK) { + /* stop sentinel connect pending checking */ + dp_del_event(dp, EV_CONNECT_PENDING_TIMEOUT); + dp_add_event(dp, EV_IRQ_HPD_INT, 0, 0); + } - if (hpd_isr_status & DP_DP_HPD_REPLUG_INT_MASK) { - dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0); - dp_add_event(dp, EV_HPD_PLUG_INT,
Re: [RFC] drm/msm/dp: Allow attaching a drm_panel
On Mon 04 Oct 20:50 CDT 2021, Stephen Boyd wrote: > Quoting Bjorn Andersson (2021-10-04 18:11:11) > > On Mon 04 Oct 17:36 PDT 2021, Doug Anderson wrote: > > > > > Hi, > > > > > > On Fri, Oct 1, 2021 at 2:00 PM Bjorn Andersson > > > wrote: > > > > > > > > On Fri 27 Aug 13:52 PDT 2021, Doug Anderson wrote: > > > > > > > > > Hi, > > > > > > > > > > On Mon, Jul 26, 2021 at 4:15 PM Bjorn Andersson > > > > > wrote: > > > > > > > > > > > > +static int dp_parser_find_panel(struct dp_parser *parser) > > > > > > +{ > > > > > > + struct device_node *np = parser->pdev->dev.of_node; > > > > > > + int rc; > > > > > > + > > > > > > + rc = drm_of_find_panel_or_bridge(np, 2, 0, > > > > > > &parser->drm_panel, NULL); > > > > > > > > > > Why port 2? Shouldn't this just be port 1 always? The yaml says that > > > > > port 1 is "Output endpoint of the controller". We should just use port > > > > > 1 here, right? > > > > > > > > > > > > > Finally got back to this, changed it to 1 and figured out why I left it > > > > at 2. > > > > > > > > drm_of_find_panel_or_bridge() on a DP controller will find the of_graph > > > > reference to the USB-C controller, scan through the registered panels > > > > and conclude that the of_node of the USB-C controller isn't a registered > > > > panel and return -EPROBE_DEFER. > > > > > > I'm confused, but maybe it would help if I could see something > > > concrete. Is there a specific board this was happening on? > > > > > > > Right, let's make this more concrete with a snippet from the actual > > SC8180x DT. > > Where is this DT? Is it in the kernel tree? > Still missing a bunch of driver pieces, so I haven't yet pushed any of this upstream. But if you're interested you can find some work-in-progress here: https://github.com/andersson/kernel/commits/wip/sc8180x-next-20210819 > > > > > Under the DP node in the device tree I expect: > > > > > > ports { > > > port@1 { > > > reg = <1>; > > > edp_out: endpoint { > > > remote-endpoint = <&edp_panel_in>; > > > }; > > > }; > > > }; > > > > > > > /* We got a panel */ > > panel { > > ... > > ports { > > port { > > auo_b133han05_in: endpoint { > > remote-endpoint = <&mdss_edp_out>; > > }; > > }; > > }; > > }; > > > > /* And a 2-port USB-C controller */ > > type-c-controller { > > ... > > connector@0 { > > ports { > > port@0 { > > reg = <0>; > > ucsi_port_0_dp: endpoint { > > remote-endpoint = <&dp0_mode>; > > }; > > }; > > > > port@1 { > > reg = <1>; > > ucsi_port_0_switch: endpoint { > > remote-endpoint = <&primary_qmp_phy>; > > }; > > }; > > }; > > }; > > > > connector@1 { > > ports { > > port@0 { > > reg = <0>; > > ucsi_port_1_dp: endpoint { > > remote-endpoint = <&dp1_mode>; > > }; > > }; > > > > port@1 { > > reg = <1>; > > ucsi_port_1_switch: endpoint { > > remote-endpoint = <&second_qmp_phy>; > > }; > > }; > > }; > > }; > > }; > > > > /* And then our 2 DP and single eDP controllers */ > > &mdss_dp0 { > > ports { > > port@1 { > > reg = <1>; > > dp0_mode: endpoint { > > remote-endpoint = <&ucsi_port_0_dp>; > > }; > > }; > > }; > > }; > > > > &mdss_dp1 { > > ports { > > port@1 { > > reg = <1>; > > dp1_mode: endpoint { > > remote-endpoint = <&ucsi_port_1_dp>; > > }; > > }; > > }; > > }; > > > > &mdss_edp { > > ports { > > port@1 { > > reg = <1>; > > mdss_edp_out: endpoint { > > remote-endpoint = <&auo_b133han05_in>; > > }; > > }; > > }; > > }; > > > > > If you have "port@1" pointing to a USB-C controller but this instance > > > of the DP controller is actually hooked up straight to a panel then > > > you should simply delete the "port@1" that points to the typeC and > > > replace it with one that points to a panel, right? > > > > > > > As you can see, port 1 on &mdss_dp0 and &mdss_dp1 points to the two UCSI > > connectors and the eDP points to the panel, exactly like we agreed. > > > > So now I call: > > drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); > > > > which for the two DP nodes will pass respective UCSI connector to > > drm_find_panel() and get EPROBE_DEFER back - because they are not on > > panel_list. > > That's "good" right? > Well, it's expected that the connectors aren't panels... > > > > There's nothing indicating in the of_graph that the USB connectors > > aren't
linux-next: manual merge of the drm-msm tree with the drm tree
Hi all, Today's linux-next merge of the drm-msm tree got a conflict in: drivers/gpu/drm/msm/msm_gem_submit.c between commit: 0e10e9a1db23 ("drm/sched: drop entity parameter from drm_sched_push_job") from the drm tree and commit: 68002469e571 ("drm/msm: One sched entity per process per priority") from the drm-msm tree. The conflict was this: /* The scheduler owns a ref now: */ msm_gem_submit_get(submit); <<< HEAD drm_sched_entity_push_job(&submit->base); === drm_sched_entity_push_job(&submit->base, queue->entity); >>> drm-msm/msm-next args->fence = submit->fence_id; I fixed it up (I just used the HEAD version) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell pgpJT8u_iI_Qf.pgp Description: OpenPGP digital signature
RE: [Intel-gfx] [PATCH v5 09/13] drm/i915/ttm: add tt shmem backend
Hi Matthew/Thomas, See one question inline Regards, Oak -Original Message- From: Intel-gfx On Behalf Of Matthew Auld Sent: September 27, 2021 7:41 AM To: intel-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org; Thomas Hellström ; Christian König Subject: [Intel-gfx] [PATCH v5 09/13] drm/i915/ttm: add tt shmem backend For cached objects we can allocate our pages directly in shmem. This should make it possible(in a later patch) to utilise the existing i915-gem shrinker code for such objects. For now this is still disabled. v2(Thomas): - Add optional try_to_writeback hook for objects. Importantly we need to check if the object is even still shrinkable; in between us dropping the shrinker LRU lock and acquiring the object lock it could for example have been moved. Also we need to differentiate between "lazy" shrinking and the immediate writeback mode. Also later we need to handle objects which don't even have mm.pages, so bundling this into put_pages() would require somehow handling that edge case, hence just letting the ttm backend handle everything in try_to_writeback doesn't seem too bad. v3(Thomas): - Likely a bad idea to touch the object from the unpopulate hook, since it's not possible to hold a reference, without also creating circular dependency, so likely this is too fragile. For now just ensure we at least mark the pages as dirty/accessed when called from the shrinker on WILLNEED objects. - s/try_to_writeback/shrinker_release_pages, since this can do more than just writeback. - Get rid of do_backup boolean and just set the SWAPPED flag prior to calling unpopulate. - Keep shmem_tt as lowest priority for the TTM LRU bo_swapout walk, since these just get skipped anyway. We can try to come up with something better later. Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Christian König --- drivers/gpu/drm/i915/gem/i915_gem_object.h| 8 + .../gpu/drm/i915/gem/i915_gem_object_types.h | 2 + drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 14 +- drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 17 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 240 -- 5 files changed, 245 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 3043fcbd31bd..1c9a1d8d3434 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -601,6 +601,14 @@ int i915_gem_object_wait_migration(struct drm_i915_gem_object *obj, bool i915_gem_object_placement_possible(struct drm_i915_gem_object *obj, enum intel_memory_type type); +struct sg_table *shmem_alloc_st(struct drm_i915_private *i915, + size_t size, struct intel_memory_region *mr, + struct address_space *mapping, + unsigned int max_segment); +void shmem_free_st(struct sg_table *st, struct address_space *mapping, + bool dirty, bool backup); +void __shmem_writeback(size_t size, struct address_space *mapping); + #ifdef CONFIG_MMU_NOTIFIER static inline bool i915_gem_object_is_userptr(struct drm_i915_gem_object *obj) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index fa2ba9e2a4d0..f0fb17be2f7a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -56,6 +56,8 @@ struct drm_i915_gem_object_ops { struct sg_table *pages); void (*truncate)(struct drm_i915_gem_object *obj); void (*writeback)(struct drm_i915_gem_object *obj); + int (*shrinker_release_pages)(struct drm_i915_gem_object *obj, + bool should_writeback); int (*pread)(struct drm_i915_gem_object *obj, const struct drm_i915_gem_pread *arg); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 36b711ae9e28..19e55cc29a15 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -25,8 +25,8 @@ static void check_release_pagevec(struct pagevec *pvec) cond_resched(); } -static void shmem_free_st(struct sg_table *st, struct address_space *mapping, - bool dirty, bool backup) +void shmem_free_st(struct sg_table *st, struct address_space *mapping, + bool dirty, bool backup) { struct sgt_iter sgt_iter; struct pagevec pvec; @@ -52,10 +52,10 @@ static void shmem_free_st(struct sg_table *st, struct address_space *mapping, kfree(st); } -static struct sg_table *shmem_alloc_st(struct drm_i915_private *i915, - size_t size, struct intel_memory_region *mr, -
Re: [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes
Quoting mkri...@codeaurora.org (2021-09-30 23:39:07) > On 2021-09-30 23:28, Stephen Boyd wrote: > > Quoting mkri...@codeaurora.org (2021-09-30 04:56:59) > >> On 2021-08-19 01:27, Stephen Boyd wrote: > >> > Quoting Krishna Manikandan (2021-08-18 03:27:02) > >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi > >> >> b/arch/arm64/boot/dts/qcom/sc7280.dtsi > >> >> index 53a21d0..fd7ff1c 100644 > >> >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > >> >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > >> >> + > >> >> + status = "disabled"; > >> >> + > >> >> + mdp: mdp@ae01000 { > >> > > >> > display-controller@ae01000 > >> > >> Stephen, > >> In the current driver code, there is a substring comparison for > >> "mdp" > >> in device node name as part of probe sequence. If "mdp" is not present > >> in the node name, it will > >> return an error resulting in probe failure. Can we continue using > >> mdp > >> as nodename instead of display controller? > >> > > > > Can we fix the driver to not look for node names and look for > > compatible > > strings instead? It took me a minute to find compare_name_mdp() in > > drivers/gpu/drm/msm/msm_drv.c to understand what you're talking about. > > Perhaps looking for qcom,mdp5 in there will be sufficient instead of > > looking at the node name. > > Sure Stephen. I will make the necessary changes in msm_drv.c to look for > compatible string instead of node name. > Can I include these two changes (changing mdp--> display controller and > msm_drv.c changes) in a separate series ? > Sure. So you'll send the drm driver change now and we'll get the DT change after that with the more generic node name?
Re: [RFC] drm/msm/dp: Allow attaching a drm_panel
Quoting Bjorn Andersson (2021-10-04 18:11:11) > On Mon 04 Oct 17:36 PDT 2021, Doug Anderson wrote: > > > Hi, > > > > On Fri, Oct 1, 2021 at 2:00 PM Bjorn Andersson > > wrote: > > > > > > On Fri 27 Aug 13:52 PDT 2021, Doug Anderson wrote: > > > > > > > Hi, > > > > > > > > On Mon, Jul 26, 2021 at 4:15 PM Bjorn Andersson > > > > wrote: > > > > > > > > > > +static int dp_parser_find_panel(struct dp_parser *parser) > > > > > +{ > > > > > + struct device_node *np = parser->pdev->dev.of_node; > > > > > + int rc; > > > > > + > > > > > + rc = drm_of_find_panel_or_bridge(np, 2, 0, > > > > > &parser->drm_panel, NULL); > > > > > > > > Why port 2? Shouldn't this just be port 1 always? The yaml says that > > > > port 1 is "Output endpoint of the controller". We should just use port > > > > 1 here, right? > > > > > > > > > > Finally got back to this, changed it to 1 and figured out why I left it > > > at 2. > > > > > > drm_of_find_panel_or_bridge() on a DP controller will find the of_graph > > > reference to the USB-C controller, scan through the registered panels > > > and conclude that the of_node of the USB-C controller isn't a registered > > > panel and return -EPROBE_DEFER. > > > > I'm confused, but maybe it would help if I could see something > > concrete. Is there a specific board this was happening on? > > > > Right, let's make this more concrete with a snippet from the actual > SC8180x DT. Where is this DT? Is it in the kernel tree? > > > Under the DP node in the device tree I expect: > > > > ports { > > port@1 { > > reg = <1>; > > edp_out: endpoint { > > remote-endpoint = <&edp_panel_in>; > > }; > > }; > > }; > > > > /* We got a panel */ > panel { > ... > ports { > port { > auo_b133han05_in: endpoint { > remote-endpoint = <&mdss_edp_out>; > }; > }; > }; > }; > > /* And a 2-port USB-C controller */ > type-c-controller { > ... > connector@0 { > ports { > port@0 { > reg = <0>; > ucsi_port_0_dp: endpoint { > remote-endpoint = <&dp0_mode>; > }; > }; > > port@1 { > reg = <1>; > ucsi_port_0_switch: endpoint { > remote-endpoint = <&primary_qmp_phy>; > }; > }; > }; > }; > > connector@1 { > ports { > port@0 { > reg = <0>; > ucsi_port_1_dp: endpoint { > remote-endpoint = <&dp1_mode>; > }; > }; > > port@1 { > reg = <1>; > ucsi_port_1_switch: endpoint { > remote-endpoint = <&second_qmp_phy>; > }; > }; > }; > }; > }; > > /* And then our 2 DP and single eDP controllers */ > &mdss_dp0 { > ports { > port@1 { > reg = <1>; > dp0_mode: endpoint { > remote-endpoint = <&ucsi_port_0_dp>; > }; > }; > }; > }; > > &mdss_dp1 { > ports { > port@1 { > reg = <1>; > dp1_mode: endpoint { > remote-endpoint = <&ucsi_port_1_dp>; > }; > }; > }; > }; > > &mdss_edp { > ports { > port@1 { > reg = <1>; > mdss_edp_out: endpoint { > remote-endpoint = <&auo_b133han05_in>; > }; > }; > }; > }; > > > If you have "port@1" pointing to a USB-C controller but this instance > > of the DP controller is actually hooked up straight to a panel then > > you should simply delete the "port@1" that points to the typeC and > > replace it with one that points to a panel, right? > > > > As you can see, port 1 on &mdss_dp0 and &mdss_dp1 points to the two UCSI > connectors and the eDP points to the panel, exactly like we agreed. > > So now I call: > drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); > > which for the two DP nodes will pass respective UCSI connector to > drm_find_panel() and get EPROBE_DEFER back - because they are not on > panel_list. That's "good" right? > > There's nothing indicating in the of_graph that the USB connectors > aren't panels (or bridges), so I don't see a way to distinguish the two > types remotes. > I'd like to create a bridge, not panel, for USB connectors, so that we can push sideband HPD signaling through to the DP driver. But either way this should work, right? If drm_of_find_panel_or_bridge() returns -EPROBE_DEFER, then assume the connector is DP. Otherwise if there's a valid pointer then treat it as eDP. We can't go too crazy though because once we attach a bridge we're assuming eDP which may not actually be true. If we make a bridge for type-C USB connectors then we'll be able to use the drm_bridge_connector code to automatically figure out the connector type
Re: [PATCH v3 3/5] drm/msm/dp: Support up to 3 DP controllers
Quoting Bjorn Andersson (2021-10-04 18:15:20) > On Mon 04 Oct 17:58 PDT 2021, Stephen Boyd wrote: > > > Quoting Bjorn Andersson (2021-10-01 11:00:56) > > > Based on the removal of the g_dp_display and the movement of the > > > priv->dp lookup into the DP code it's now possible to have multiple > > > DP instances. > > > > > > In line with the other controllers in the MSM driver, introduce a > > > per-compatible list of base addresses which is used to resolve the > > > "instance id" for the given DP controller. This instance id is used as > > > index in the priv->dp[] array. > > > > > > Then extend the initialization code to initialize struct drm_encoder for > > > each of the registered priv->dp[] and update the logic for associating > > > each struct msm_dp with the struct dpu_encoder_virt. > > > > > > Lastly, bump the number of struct msm_dp instances carries by priv->dp > > > to 3, the currently known maximum number of controllers found in a > > > Qualcomm SoC. > > > > > > Signed-off-by: Bjorn Andersson > > > --- > > > > Reviewed-by: Stephen Boyd > > > > Some nits below. > > > > > > > > Changes since v2: > > > - Added MSM_DRM_DP_COUNT to link the two 3s > > > - Moved NULL check for msm_dp_debugfs_init() to the call site > > > - Made struct dp_display_private->id unsigned > > > > > > I also implemented added connector_type to each of the DP instances and > > > propagated this to dp_drm_connector_init() but later dropped this again > > > per > > > Doug's suggestion that we'll base this on the presence/absence of a > > > associated > > > drm bridge or panel. > > > > Sad but OK. We can take up that topic in another patch. > > > > So you don't agree with the solution from sn65dsi86? > > The only reason I haven't yet send this other patch is the of_graph > thing Doug an I are discussing on the RFC. But if we agree to base this > on compatible we could decide to look only for panels for the edp > instances and avoid that problem... > > We would however never be able to describe the USB-less DP instance with > a panel explicitly described in DT going that route. I'll reply on that thread. > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > > index f655adbc2421..875b07e7183d 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > > @@ -203,8 +204,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, > > > struct drm_minor *minor) > > > dpu_debugfs_vbif_init(dpu_kms, entry); > > > dpu_debugfs_core_irq_init(dpu_kms, entry); > > > > > > - if (priv->dp) > > > - msm_dp_debugfs_init(priv->dp, minor); > > > + for (i = 0; i < ARRAY_SIZE(priv->dp); i++) { > > > + if (priv->dp[i]) > > > + msm_dp_debugfs_init(priv->dp[i], minor); > > > > This seems to cause a bunch of debugfs warnings when there are multiple > > nodes created with the same name. > > > > Yes, that's true. I have a half-baked follow up that attempts to create > instance-specific debugfs directories. Can we take that in a follow up? Sure. > > > > + } > > > > > > return dpu_core_perf_debugfs_init(dpu_kms, entry); > > > } > > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > > b/drivers/gpu/drm/msm/dp/dp_display.c > > > index 5d3ee5ef07c2..ff3477474c5d 100644 > > > --- a/drivers/gpu/drm/msm/dp/dp_display.c > > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > > > @@ -1180,10 +1192,31 @@ int dp_display_request_irq(struct msm_dp > > > *dp_display) > > > return 0; > > > } > > > > > > +static int dp_display_find_id(struct platform_device *pdev) > > > +{ > > > + const struct msm_dp_config *cfg = > > > of_device_get_match_data(&pdev->dev); > > > + struct resource *res; > > > + int i; > > > + > > > + > > > > Nitpick: Remove a newline here. > > > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > + if (!res) > > > + return -EINVAL; > > > + > > > + for (i = 0; i < cfg->num_descs; i++) { > > > + if (cfg->io_start[i] == res->start) > > > + return i; > > > + } > > > > Nitpick: Drop braces on single line if inside for loop. > > > > Not when the loop spans multiple lines? Kernel style is to remove braces from single "statement" for loops where in this case the statement is the if condition. > > > > + > > > + dev_err(&pdev->dev, "unknown displayport instance\n"); > > > + return -EINVAL; > > > +} > > > +
Re: [PATCH v3 3/5] drm/msm/dp: Refactor ioremap wrapper
On Mon 04 Oct 18:04 PDT 2021, Stephen Boyd wrote: > Quoting Bjorn Andersson (2021-10-01 10:43:58) > > In order to deal with multiple memory ranges in the following commit > > change the ioremap wrapper to not poke directly into the dss_io_data > > struct. > > > > While at it, devm_ioremap_resource() already prints useful error > > messages on failure, so omit the unnecessary prints from the caller. > > > > Signed-off-by: Bjorn Andersson > > --- > > Reviewed-by: Stephen Boyd > > I realize this will cause some prints when we use old DTs. I suppose > that's OK though because we'll have more incentive to update existing > DT. The use of the current binding is fairly limited, so I think that makes sense. Abhinav also requested earlier that we do that and drop the fallback sooner rather than later, which I would like to see as well. Thanks, Bjorn
Re: [PATCH v3 3/5] drm/msm/dp: Support up to 3 DP controllers
On Mon 04 Oct 17:58 PDT 2021, Stephen Boyd wrote: > Quoting Bjorn Andersson (2021-10-01 11:00:56) > > Based on the removal of the g_dp_display and the movement of the > > priv->dp lookup into the DP code it's now possible to have multiple > > DP instances. > > > > In line with the other controllers in the MSM driver, introduce a > > per-compatible list of base addresses which is used to resolve the > > "instance id" for the given DP controller. This instance id is used as > > index in the priv->dp[] array. > > > > Then extend the initialization code to initialize struct drm_encoder for > > each of the registered priv->dp[] and update the logic for associating > > each struct msm_dp with the struct dpu_encoder_virt. > > > > Lastly, bump the number of struct msm_dp instances carries by priv->dp > > to 3, the currently known maximum number of controllers found in a > > Qualcomm SoC. > > > > Signed-off-by: Bjorn Andersson > > --- > > Reviewed-by: Stephen Boyd > > Some nits below. > > > > > Changes since v2: > > - Added MSM_DRM_DP_COUNT to link the two 3s > > - Moved NULL check for msm_dp_debugfs_init() to the call site > > - Made struct dp_display_private->id unsigned > > > > I also implemented added connector_type to each of the DP instances and > > propagated this to dp_drm_connector_init() but later dropped this again per > > Doug's suggestion that we'll base this on the presence/absence of a > > associated > > drm bridge or panel. > > Sad but OK. We can take up that topic in another patch. > So you don't agree with the solution from sn65dsi86? The only reason I haven't yet send this other patch is the of_graph thing Doug an I are discussing on the RFC. But if we agree to base this on compatible we could decide to look only for panels for the edp instances and avoid that problem... We would however never be able to describe the USB-less DP instance with a panel explicitly described in DT going that route. > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > index f655adbc2421..875b07e7183d 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > @@ -203,8 +204,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, > > struct drm_minor *minor) > > dpu_debugfs_vbif_init(dpu_kms, entry); > > dpu_debugfs_core_irq_init(dpu_kms, entry); > > > > - if (priv->dp) > > - msm_dp_debugfs_init(priv->dp, minor); > > + for (i = 0; i < ARRAY_SIZE(priv->dp); i++) { > > + if (priv->dp[i]) > > + msm_dp_debugfs_init(priv->dp[i], minor); > > This seems to cause a bunch of debugfs warnings when there are multiple > nodes created with the same name. > Yes, that's true. I have a half-baked follow up that attempts to create instance-specific debugfs directories. Can we take that in a follow up? > > + } > > > > return dpu_core_perf_debugfs_init(dpu_kms, entry); > > } > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > b/drivers/gpu/drm/msm/dp/dp_display.c > > index 5d3ee5ef07c2..ff3477474c5d 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_display.c > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > > @@ -1180,10 +1192,31 @@ int dp_display_request_irq(struct msm_dp > > *dp_display) > > return 0; > > } > > > > +static int dp_display_find_id(struct platform_device *pdev) > > +{ > > + const struct msm_dp_config *cfg = > > of_device_get_match_data(&pdev->dev); > > + struct resource *res; > > + int i; > > + > > + > > Nitpick: Remove a newline here. > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) > > + return -EINVAL; > > + > > + for (i = 0; i < cfg->num_descs; i++) { > > + if (cfg->io_start[i] == res->start) > > + return i; > > + } > > Nitpick: Drop braces on single line if inside for loop. > Not when the loop spans multiple lines? > > + > > + dev_err(&pdev->dev, "unknown displayport instance\n"); > > + return -EINVAL; > > +} > > +
Re: [RFC] drm/msm/dp: Allow attaching a drm_panel
On Mon 04 Oct 17:36 PDT 2021, Doug Anderson wrote: > Hi, > > On Fri, Oct 1, 2021 at 2:00 PM Bjorn Andersson > wrote: > > > > On Fri 27 Aug 13:52 PDT 2021, Doug Anderson wrote: > > > > > Hi, > > > > > > On Mon, Jul 26, 2021 at 4:15 PM Bjorn Andersson > > > wrote: > > > > > > > > +static int dp_parser_find_panel(struct dp_parser *parser) > > > > +{ > > > > + struct device_node *np = parser->pdev->dev.of_node; > > > > + int rc; > > > > + > > > > + rc = drm_of_find_panel_or_bridge(np, 2, 0, &parser->drm_panel, > > > > NULL); > > > > > > Why port 2? Shouldn't this just be port 1 always? The yaml says that > > > port 1 is "Output endpoint of the controller". We should just use port > > > 1 here, right? > > > > > > > Finally got back to this, changed it to 1 and figured out why I left it > > at 2. > > > > drm_of_find_panel_or_bridge() on a DP controller will find the of_graph > > reference to the USB-C controller, scan through the registered panels > > and conclude that the of_node of the USB-C controller isn't a registered > > panel and return -EPROBE_DEFER. > > I'm confused, but maybe it would help if I could see something > concrete. Is there a specific board this was happening on? > Right, let's make this more concrete with a snippet from the actual SC8180x DT. > Under the DP node in the device tree I expect: > > ports { > port@1 { > reg = <1>; > edp_out: endpoint { > remote-endpoint = <&edp_panel_in>; > }; > }; > }; > /* We got a panel */ panel { ... ports { port { auo_b133han05_in: endpoint { remote-endpoint = <&mdss_edp_out>; }; }; }; }; /* And a 2-port USB-C controller */ type-c-controller { ... connector@0 { ports { port@0 { reg = <0>; ucsi_port_0_dp: endpoint { remote-endpoint = <&dp0_mode>; }; }; port@1 { reg = <1>; ucsi_port_0_switch: endpoint { remote-endpoint = <&primary_qmp_phy>; }; }; }; }; connector@1 { ports { port@0 { reg = <0>; ucsi_port_1_dp: endpoint { remote-endpoint = <&dp1_mode>; }; }; port@1 { reg = <1>; ucsi_port_1_switch: endpoint { remote-endpoint = <&second_qmp_phy>; }; }; }; }; }; /* And then our 2 DP and single eDP controllers */ &mdss_dp0 { ports { port@1 { reg = <1>; dp0_mode: endpoint { remote-endpoint = <&ucsi_port_0_dp>; }; }; }; }; &mdss_dp1 { ports { port@1 { reg = <1>; dp1_mode: endpoint { remote-endpoint = <&ucsi_port_1_dp>; }; }; }; }; &mdss_edp { ports { port@1 { reg = <1>; mdss_edp_out: endpoint { remote-endpoint = <&auo_b133han05_in>; }; }; }; }; > If you have "port@1" pointing to a USB-C controller but this instance > of the DP controller is actually hooked up straight to a panel then > you should simply delete the "port@1" that points to the typeC and > replace it with one that points to a panel, right? > As you can see, port 1 on &mdss_dp0 and &mdss_dp1 points to the two UCSI connectors and the eDP points to the panel, exactly like we agreed. So now I call: drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); which for the two DP nodes will pass respective UCSI connector to drm_find_panel() and get EPROBE_DEFER back - because they are not on panel_list. There's nothing indicating in the of_graph that the USB connectors aren't panels (or bridges), so I don't see a way to distinguish the two types remotes. Hope that clarifies my conundrum. Regards, Bjorn
Re: [PATCH v3 3/5] drm/msm/dp: Refactor ioremap wrapper
Quoting Bjorn Andersson (2021-10-01 10:43:58) > In order to deal with multiple memory ranges in the following commit > change the ioremap wrapper to not poke directly into the dss_io_data > struct. > > While at it, devm_ioremap_resource() already prints useful error > messages on failure, so omit the unnecessary prints from the caller. > > Signed-off-by: Bjorn Andersson > --- Reviewed-by: Stephen Boyd I realize this will cause some prints when we use old DTs. I suppose that's OK though because we'll have more incentive to update existing DT.
[Bug 206349] WARNING: dcn20_validate_bandwidth+0x87/0x9e [ amdgpu on 5700 XT ]
https://bugzilla.kernel.org/show_bug.cgi?id=206349 Jimmy Berry (ji...@boombatower.com) changed: What|Removed |Added CC||ji...@boombatower.com --- Comment #1 from Jimmy Berry (ji...@boombatower.com) --- Looks like I have the same thing on 5.14.6 with a Radeon 6900 XT. >From 5.13.8 I was able to run 4 x 4k @ 60hz + 2 x 4k @30hz. Hardware should support all 6 at 60hz, but that is all I can get to work. As of the update to 5.14.6 I can now only run 3 x 4k @ 60hz + 3 x 4k @ 30hz. Wondering if this error is related. I can supply more detail. [ 43.785782] [ cut here ] [ 43.785784] WARNING: CPU: 20 PID: 1392 at drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dsc.c:267 dsc2_disable+0x12b/0x140 [amdgpu] [ 43.785879] Modules linked in: af_packet dmi_sysfs dm_crypt essiv authenc trusted asn1_encoder tee intel_rapl_msr intel_rapl_common edac_mce_amd snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi kvm snd_usb_audio snd_hda_codec uvcvideo snd_usbmidi_lib snd_hda_core snd_rawmidi irqbypass videobuf2_vmalloc snd_seq_device videobuf2_memops snd_hwdep eeepc_wmi videobuf2_v4l2 asus_wmi snd_pcm videobuf2_common battery sparse_keymap rfkill joydev video pcspkr snd_timer wmi_bmof i2c_piix4 snd k10temp igc soundcore tiny_power_button gpio_amdpt gpio_generic acpi_cpufreq button fuse configfs ext4 mbcache jbd2 hid_logitech_hidpp hid_logitech_dj hid_generic usbhid amdgpu drm_ttm_helper ttm iommu_v2 gpu_sched i2c_algo_bit drm_kms_helper crct10dif_pclmul crc32_pclmul crc32c_intel syscopyarea sysfillrect sysimgblt ghash_clmulni_intel fb_sys_fops cec xhci_pci xhci_pci_renesas rc_core xhci_hcd drm usbcore [ 43.785899] aesni_intel nvme ccp crypto_simd cryptd nvme_core sp5100_tco wmi v4l2loopback(O) videodev mc sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua msr [ 43.785906] CPU: 20 PID: 1392 Comm: Xorg.bin Tainted: GW O 5.14.6-1-default #1 openSUSE Tumbleweed 539bdc3afabb63aa01656ffde27ce5025c985e0c [ 43.785908] Hardware name: ASUS System Product Name/ROG STRIX B550-F GAMING, BIOS 1401 12/03/2020 [ 43.785908] RIP: 0010:dsc2_disable+0x12b/0x140 [amdgpu] [ 43.785987] Code: 10 65 48 2b 04 25 28 00 00 00 75 25 48 83 c4 18 5b c3 8b 53 10 8b 4c 24 0c bf 04 00 00 00 48 c7 c6 50 15 9c c0 e8 95 fc 8b 00 <0f> 0b e9 75 ff ff ff e8 c9 5b ae ec 66 0f 1f 84 00 00 00 00 00 0f [ 43.785988] RSP: 0018:a32302df3608 EFLAGS: 00010246 [ 43.785989] RAX: RBX: 8d30610c6000 RCX: [ 43.785990] RDX: 0004 RSI: c09c1550 RDI: [ 43.785990] RBP: 8d30610c6000 R08: a32302df3610 R09: 0004 [ 43.785990] R10: R11: 000a319e8f4b R12: 8d30c522 [ 43.785991] R13: 8d30c52214a8 R14: 8d3c0855 R15: 8d3044934000 [ 43.785992] FS: 7efc485ea940() GS:8d3f2ef0() knlGS: [ 43.785992] CS: 0010 DS: ES: CR0: 80050033 [ 43.785993] CR2: 7efc2405a358 CR3: 000107c2a000 CR4: 00750ee0 [ 43.785993] PKRU: 5554 [ 43.785994] Call Trace: [ 43.785997] dp_set_dsc_on_stream+0x2be/0x360 [amdgpu 791908deeeccac56f7b565af20aa0cff4bc40e3b] [ 43.786074] ? drm_dp_dpcd_write+0x65/0xd0 [drm_kms_helper 750f79a7da02c8d460f017cfe9bf7182a2652479] [ 43.786081] dp_set_dsc_enable+0x6a/0x80 [amdgpu 791908deeeccac56f7b565af20aa0cff4bc40e3b] [ 43.786152] dcn20_reset_hw_ctx_wrap+0x11c/0x370 [amdgpu 791908deeeccac56f7b565af20aa0cff4bc40e3b] [ 43.786228] dce110_apply_ctx_to_hw+0x4f/0x560 [amdgpu 791908deeeccac56f7b565af20aa0cff4bc40e3b] [ 43.786301] ? __free_pages_ok+0x2d8/0x410 [ 43.786304] dc_commit_state+0x333/0xa80 [amdgpu 791908deeeccac56f7b565af20aa0cff4bc40e3b] [ 43.786374] amdgpu_dm_atomic_commit_tail+0x53a/0x25f0 [amdgpu 791908deeeccac56f7b565af20aa0cff4bc40e3b] [ 43.786452] ? dcn30_validate_bandwidth+0x11f/0x270 [amdgpu 791908deeeccac56f7b565af20aa0cff4bc40e3b] [ 43.786526] ? kfree+0xba/0x3c0 [ 43.786528] ? dm_plane_helper_prepare_fb+0x1b4/0x270 [amdgpu 791908deeeccac56f7b565af20aa0cff4bc40e3b] [ 43.786602] ? __cond_resched+0x16/0x40 [ 43.786604] ? __wait_for_common+0x3b/0x160 [ 43.786605] ? __cond_resched+0x16/0x40 [ 43.786605] ? __wait_for_common+0x3b/0x160 [ 43.786607] commit_tail+0x94/0x120 [drm_kms_helper 750f79a7da02c8d460f017cfe9bf7182a2652479] [ 43.786614] drm_atomic_helper_commit+0x113/0x140 [drm_kms_helper 750f79a7da02c8d460f017cfe9bf7182a2652479] [ 43.786619] drm_atomic_helper_set_config+0x70/0xb0 [drm_kms_helper 750f79a7da02c8d460f017cfe9bf7182a2652479] [ 43.786625] drm_mode_setcrtc+0x1d3/0x6d0 [drm 13a94d3e4d6d5c79a58cf45473f1abe284f6b940] [ 43.786637] ? drm_mode_getcrtc+0x170/0x170 [drm 13a94d3e4d6d5c79
Re: [PATCH v3 3/5] drm/msm/dp: Support up to 3 DP controllers
Quoting Bjorn Andersson (2021-10-01 11:00:56) > Based on the removal of the g_dp_display and the movement of the > priv->dp lookup into the DP code it's now possible to have multiple > DP instances. > > In line with the other controllers in the MSM driver, introduce a > per-compatible list of base addresses which is used to resolve the > "instance id" for the given DP controller. This instance id is used as > index in the priv->dp[] array. > > Then extend the initialization code to initialize struct drm_encoder for > each of the registered priv->dp[] and update the logic for associating > each struct msm_dp with the struct dpu_encoder_virt. > > Lastly, bump the number of struct msm_dp instances carries by priv->dp > to 3, the currently known maximum number of controllers found in a > Qualcomm SoC. > > Signed-off-by: Bjorn Andersson > --- Reviewed-by: Stephen Boyd Some nits below. > > Changes since v2: > - Added MSM_DRM_DP_COUNT to link the two 3s > - Moved NULL check for msm_dp_debugfs_init() to the call site > - Made struct dp_display_private->id unsigned > > I also implemented added connector_type to each of the DP instances and > propagated this to dp_drm_connector_init() but later dropped this again per > Doug's suggestion that we'll base this on the presence/absence of a associated > drm bridge or panel. Sad but OK. We can take up that topic in another patch. > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index f655adbc2421..875b07e7183d 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -203,8 +204,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, > struct drm_minor *minor) > dpu_debugfs_vbif_init(dpu_kms, entry); > dpu_debugfs_core_irq_init(dpu_kms, entry); > > - if (priv->dp) > - msm_dp_debugfs_init(priv->dp, minor); > + for (i = 0; i < ARRAY_SIZE(priv->dp); i++) { > + if (priv->dp[i]) > + msm_dp_debugfs_init(priv->dp[i], minor); This seems to cause a bunch of debugfs warnings when there are multiple nodes created with the same name. > + } > > return dpu_core_perf_debugfs_init(dpu_kms, entry); > } > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index 5d3ee5ef07c2..ff3477474c5d 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -1180,10 +1192,31 @@ int dp_display_request_irq(struct msm_dp *dp_display) > return 0; > } > > +static int dp_display_find_id(struct platform_device *pdev) > +{ > + const struct msm_dp_config *cfg = > of_device_get_match_data(&pdev->dev); > + struct resource *res; > + int i; > + > + Nitpick: Remove a newline here. > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > + > + for (i = 0; i < cfg->num_descs; i++) { > + if (cfg->io_start[i] == res->start) > + return i; > + } Nitpick: Drop braces on single line if inside for loop. > + > + dev_err(&pdev->dev, "unknown displayport instance\n"); > + return -EINVAL; > +} > +
Re: [RFC] drm/msm/dp: Allow attaching a drm_panel
Hi, On Fri, Oct 1, 2021 at 2:00 PM Bjorn Andersson wrote: > > On Fri 27 Aug 13:52 PDT 2021, Doug Anderson wrote: > > > Hi, > > > > On Mon, Jul 26, 2021 at 4:15 PM Bjorn Andersson > > wrote: > > > > > > +static int dp_parser_find_panel(struct dp_parser *parser) > > > +{ > > > + struct device_node *np = parser->pdev->dev.of_node; > > > + int rc; > > > + > > > + rc = drm_of_find_panel_or_bridge(np, 2, 0, &parser->drm_panel, > > > NULL); > > > > Why port 2? Shouldn't this just be port 1 always? The yaml says that > > port 1 is "Output endpoint of the controller". We should just use port > > 1 here, right? > > > > Finally got back to this, changed it to 1 and figured out why I left it > at 2. > > drm_of_find_panel_or_bridge() on a DP controller will find the of_graph > reference to the USB-C controller, scan through the registered panels > and conclude that the of_node of the USB-C controller isn't a registered > panel and return -EPROBE_DEFER. I'm confused, but maybe it would help if I could see something concrete. Is there a specific board this was happening on? Under the DP node in the device tree I expect: ports { port@1 { reg = <1>; edp_out: endpoint { remote-endpoint = <&edp_panel_in>; }; }; }; If you have "port@1" pointing to a USB-C controller but this instance of the DP controller is actually hooked up straight to a panel then you should simply delete the "port@1" that points to the typeC and replace it with one that points to a panel, right? -Doug
Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID
Hi, On Mon, Oct 4, 2021 at 10:14 AM Geert Uytterhoeven wrote: > > Hi Douglas, > > On Mon, Oct 4, 2021 at 6:22 PM Douglas Anderson wrote: > > In the commit bac9c2948224 ("drm/edid: Break out reading block 0 of > > the EDID") I broke out reading the base block of the EDID to its own > > function. Unfortunately, when I did that I messed up the handling when > > drm_edid_is_zero() indicated that we had an EDID that was all 0x00 or > > when we went through 4 loops and didn't get a valid EDID. Specifically > > I needed to pass the broken EDID to connector_bad_edid() but now I was > > passing an error-pointer. > > > > Let's re-jigger things so we can pass the bad EDID in properly. > > > > Fixes: bac9c2948224 ("drm/edid: Break out reading block 0 of the EDID") > > Reported-by: kernel test robot > > Reported-by: Geert Uytterhoeven > > Signed-off-by: Douglas Anderson > > The crash is was seeing is gone, so > Tested-by: Geert Uytterhoeven Thanks for testing! I'll plan to apply tomorrow morning (California time) to balance between giving folks a chance to yell at me for my patch and the urgency of fixing the breakage. -Doug
Re: [PATCH V4 2/2] drm/bridge: lvds-codec: Add support for LVDS data mapping select
Hi Marek, Thank you for the patch, and all my apologies for the delay. On Tue, Jul 27, 2021 at 06:13:57PM +0200, Marek Vasut wrote: > Decoder input LVDS format is a property of the decoder chip or even > its strapping. Handle data-mapping the same way lvds-panel does. In > case data-mapping is not present, do nothing, since there are still > legacy bindings which do not specify this property. > > Signed-off-by: Marek Vasut > Cc: Laurent Pinchart > Cc: Sam Ravnborg > To: dri-devel@lists.freedesktop.org > --- > V2: - Move the data-mapping to endpoint > V3: - Rebase on V2 submitted a while ago, reinstate changelog > - Use .atomic_get_input_bus_fmts for the decoder, separate funcs for > encoder > V4: - No change > --- > drivers/gpu/drm/bridge/lvds-codec.c | 76 - > 1 file changed, 75 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/lvds-codec.c > b/drivers/gpu/drm/bridge/lvds-codec.c > index dcf579a4cf833..afa7ce7ea01e8 100644 > --- a/drivers/gpu/drm/bridge/lvds-codec.c > +++ b/drivers/gpu/drm/bridge/lvds-codec.c > @@ -12,6 +12,7 @@ > #include > #include > > +#include > #include > #include > > @@ -22,6 +23,7 @@ struct lvds_codec { > struct regulator *vcc; > struct gpio_desc *powerdown_gpio; > u32 connector_type; > + unsigned int bus_format; > }; > > static inline struct lvds_codec *to_lvds_codec(struct drm_bridge *bridge) > @@ -74,12 +76,50 @@ static const struct drm_bridge_funcs funcs = { > .disable = lvds_codec_disable, > }; > > +#define MAX_INPUT_SEL_FORMATS 1 > +static u32 * > +lvds_codec_atomic_get_input_bus_fmts(struct drm_bridge *bridge, > + struct drm_bridge_state *bridge_state, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state, > + u32 output_fmt, > + unsigned int *num_input_fmts) > +{ > + struct lvds_codec *lvds_codec = to_lvds_codec(bridge); > + u32 *input_fmts; > + > + *num_input_fmts = 0; > + > + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), > + GFP_KERNEL); > + if (!input_fmts) > + return NULL; > + > + input_fmts[0] = lvds_codec->bus_format; > + *num_input_fmts = MAX_INPUT_SEL_FORMATS; Are there cases where the input format would depend on the output format ? For instance, a 24-bit RGB output that could also work in RGB666 mode, where we would select between MEDIA_BUS_FMT_RGB666_1X7X3_SPWG and MEDIA_BUS_FMT_RGB888_1X7X4_SPWG depending on whether the output is RGB666 or RGB888 ? I don't think we need to handle this now, and I believe it will be possible to do this in a backward-compatible way if the need ever arises. A confirmation that I'm not missing something obvious would be nice. > + > + return input_fmts; > +} > + > +static const struct drm_bridge_funcs funcs_decoder = { > + .attach = lvds_codec_attach, > + .enable = lvds_codec_enable, > + .disable = lvds_codec_disable, > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > + .atomic_reset = drm_atomic_helper_bridge_reset, > + .atomic_get_input_bus_fmts = lvds_codec_atomic_get_input_bus_fmts, > +}; > + > static int lvds_codec_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct device_node *panel_node; > + struct device_node *bus_node; > struct drm_panel *panel; > struct lvds_codec *lvds_codec; > + const char *mapping; > + int ret; > > lvds_codec = devm_kzalloc(dev, sizeof(*lvds_codec), GFP_KERNEL); > if (!lvds_codec) > @@ -119,13 +159,47 @@ static int lvds_codec_probe(struct platform_device > *pdev) > if (IS_ERR(lvds_codec->panel_bridge)) > return PTR_ERR(lvds_codec->panel_bridge); > > + lvds_codec->bridge.funcs = &funcs; > + > + /* > + * Decoder input LVDS format is a property of the decoder chip or even > + * its strapping. Handle data-mapping the same way lvds-panel does. In > + * case data-mapping is not present, do nothing, since there are still > + * legacy bindings which do not specify this property. > + */ > + if (lvds_codec->connector_type != DRM_MODE_CONNECTOR_LVDS) { > + bus_node = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0); I think this should be bus_node = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0); (see the comment on patch 1/2). > + if (!bus_node) { > + dev_dbg(dev, "bus DT node not found\n"); > + return -ENXIO; > + } > + > + ret = of_property_read_string(bus_node, "data-mapping", > + &mapping); > + of_node_put
Re: [PATCH V4 1/2] dt-bindings: display: bridge: lvds-codec: Document LVDS data mapping select
On Tue, Oct 05, 2021 at 03:03:40AM +0300, Laurent Pinchart wrote: > Hi Marek, > > Thank you for the patch. > > On Tue, Jul 27, 2021 at 06:13:56PM +0200, Marek Vasut wrote: > > Decoder input LVDS format is a property of the decoder chip or even > > its strapping. Add DT property data-mapping the same way lvds-panel > > does, to define the LVDS data mapping. > > > > Signed-off-by: Marek Vasut > > Cc: Laurent Pinchart > > Cc: Rob Herring > > Cc: Sam Ravnborg > > Cc: devicet...@vger.kernel.org > > To: dri-devel@lists.freedesktop.org > > Reviewed-by: Laurent Pinchart I think I have to retract this, see below :-( > And all my apologies for the delay. > > > --- > > V2: - Use allOf > > - Move the data-mapping to endpoint > > V3: - Rebase on V2 submitted a while ago, reinstate changelog > > - Drop the allOf and un-rebase on previous pclk patch > > V4: - port@1, remove $ref: /schemas/graph.yaml#/properties/port and > > add $ref: /schemas/graph.yaml#/$defs/port-base > > --- > > .../bindings/display/bridge/lvds-codec.yaml | 33 ++- > > 1 file changed, 32 insertions(+), 1 deletion(-) > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > index 304a1367faaa7..c0400c60f272a 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > @@ -55,11 +55,26 @@ properties: > >For LVDS decoders, port 0 is the LVDS input > > > >port@1: > > -$ref: /schemas/graph.yaml#/properties/port > > +$ref: /schemas/graph.yaml#/$defs/port-base > > description: | > >For LVDS encoders, port 1 is the LVDS output > >For LVDS decoders, port 1 is the parallel output > > > > +properties: > > + endpoint: > > +$ref: /schemas/media/video-interfaces.yaml# > > +unevaluatedProperties: false > > + > > +properties: > > + data-mapping: > > +enum: > > + - jeida-18 > > + - jeida-24 > > + - vesa-24 > > +description: | > > + The color signals mapping order. See details in > > + Documentation/devicetree/bindings/display/panel/lvds.yaml > > + Shouldn't this be for port 0 ? For decoders, port 1 is the parallel side, not the LVDS side. With this fixed, you can retain the Reviewed-by: Laurent Pinchart > > required: > >- port@0 > >- port@1 > > @@ -71,6 +86,22 @@ properties: > > > >power-supply: true > > > > +if: > > + not: > > +properties: > > + compatible: > > +contains: > > + const: lvds-decoder > > +then: > > + properties: > > +ports: > > + properties: > > +port@1: > > + properties: > > +endpoint: > > + properties: > > +data-mapping: false > > + > > required: > >- compatible > >- ports -- Regards, Laurent Pinchart
Re: [PATCH V4 1/2] dt-bindings: display: bridge: lvds-codec: Document LVDS data mapping select
Hi Marek, Thank you for the patch. On Tue, Jul 27, 2021 at 06:13:56PM +0200, Marek Vasut wrote: > Decoder input LVDS format is a property of the decoder chip or even > its strapping. Add DT property data-mapping the same way lvds-panel > does, to define the LVDS data mapping. > > Signed-off-by: Marek Vasut > Cc: Laurent Pinchart > Cc: Rob Herring > Cc: Sam Ravnborg > Cc: devicet...@vger.kernel.org > To: dri-devel@lists.freedesktop.org Reviewed-by: Laurent Pinchart And all my apologies for the delay. > --- > V2: - Use allOf > - Move the data-mapping to endpoint > V3: - Rebase on V2 submitted a while ago, reinstate changelog > - Drop the allOf and un-rebase on previous pclk patch > V4: - port@1, remove $ref: /schemas/graph.yaml#/properties/port and > add $ref: /schemas/graph.yaml#/$defs/port-base > --- > .../bindings/display/bridge/lvds-codec.yaml | 33 ++- > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > index 304a1367faaa7..c0400c60f272a 100644 > --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > @@ -55,11 +55,26 @@ properties: >For LVDS decoders, port 0 is the LVDS input > >port@1: > -$ref: /schemas/graph.yaml#/properties/port > +$ref: /schemas/graph.yaml#/$defs/port-base > description: | >For LVDS encoders, port 1 is the LVDS output >For LVDS decoders, port 1 is the parallel output > > +properties: > + endpoint: > +$ref: /schemas/media/video-interfaces.yaml# > +unevaluatedProperties: false > + > +properties: > + data-mapping: > +enum: > + - jeida-18 > + - jeida-24 > + - vesa-24 > +description: | > + The color signals mapping order. See details in > + Documentation/devicetree/bindings/display/panel/lvds.yaml > + > required: >- port@0 >- port@1 > @@ -71,6 +86,22 @@ properties: > >power-supply: true > > +if: > + not: > +properties: > + compatible: > +contains: > + const: lvds-decoder > +then: > + properties: > +ports: > + properties: > +port@1: > + properties: > +endpoint: > + properties: > +data-mapping: false > + > required: >- compatible >- ports -- Regards, Laurent Pinchart
Re: [Freedreno] [PATCH v3 03/14] drm/hdcp: Update property value on content type and user changes
On 2021-10-01 08:11, Sean Paul wrote: From: Sean Paul This patch updates the connector's property value in 2 cases which were previously missed: 1- Content type changes. The value should revert back to DESIRED from ENABLED in case the driver must re-authenticate the link due to the new content type. 2- Userspace sets value to DESIRED while ENABLED. In this case, the value should be reset immediately to ENABLED since the link is actively being encrypted. To accommodate these changes, I've split up the conditionals to make things a bit more clear (as much as one can with this mess of state). Acked-by: Jani Nikula Signed-off-by: Sean Paul Reviewed-by: Abhinav Kumar Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-s...@poorly.run #v1 Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-s...@poorly.run #v2 Changes in v2: -None Changes in v3: -Fixed indentation issue identified by 0-day --- drivers/gpu/drm/drm_hdcp.c | 26 +- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c index dd8fa91c51d6..8c851d40cd45 100644 --- a/drivers/gpu/drm/drm_hdcp.c +++ b/drivers/gpu/drm/drm_hdcp.c @@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector *connector, return true; /* -* Nothing to do if content type is unchanged and one of: -* - state didn't change +* Content type changes require an HDCP disable/enable cycle. +*/ + if (new_conn_state->hdcp_content_type != old_conn_state->hdcp_content_type) { + new_conn_state->content_protection = + DRM_MODE_CONTENT_PROTECTION_DESIRED; + return true; + } + + /* +* Ignore meaningless state changes: * - HDCP was activated since the last commit -* - attempting to set to desired while already enabled +* - Attempting to set to desired while already enabled */ - if (old_hdcp == new_hdcp || - (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED && + if ((old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED && new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) || (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED && new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) { - if (old_conn_state->hdcp_content_type == - new_conn_state->hdcp_content_type) - return false; + new_conn_state->content_protection = + DRM_MODE_CONTENT_PROTECTION_ENABLED; + return false; } - return true; + /* Finally, if state changes, we need action */ + return old_hdcp != new_hdcp; } EXPORT_SYMBOL(drm_hdcp_atomic_check);
Re: [PATCH v7 1/3] drm/dsi: transer dsi hs packet aligned
Hi, Jitao: Jitao Shi 於 2021年9月16日 週四 上午6:31寫道: > > Some DSI devices reqire the hs packet starting and ending > at same time on all dsi lanes. So use a flag to those devices. > Reviewed-by: Chun-Kuang Hu > Signed-off-by: Jitao Shi > --- > include/drm/drm_mipi_dsi.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > index af7ba8071eb0..8e8563792682 100644 > --- a/include/drm/drm_mipi_dsi.h > +++ b/include/drm/drm_mipi_dsi.h > @@ -177,6 +177,7 @@ struct mipi_dsi_device_info { > * @lp_rate: maximum lane frequency for low power mode in hertz, this should > * be set to the real limits of the hardware, zero is only accepted for > * legacy drivers > + * @hs_packet_end_aligned: transfer dsi hs packet ending aligned > */ > struct mipi_dsi_device { > struct mipi_dsi_host *host; > @@ -189,6 +190,7 @@ struct mipi_dsi_device { > unsigned long mode_flags; > unsigned long hs_rate; > unsigned long lp_rate; > + bool hs_packet_end_aligned; > }; > > #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:" > -- > 2.25.1
[Bug 213201] [KAVERI] memory leak - unreferenced object 0xffff8881700cf988 (size 56)
https://bugzilla.kernel.org/show_bug.cgi?id=213201 --- Comment #14 from Erhard F. (erhar...@mailbox.org) --- Created attachment 299101 --> https://bugzilla.kernel.org/attachment.cgi?id=299101&action=edit utput of /sys/kernel/debug/kmemleak (kernel 5.15-rc4, AMD PRO A10-8750B) Same board, another CPU: # cat /sys/kernel/debug/kmemleak unreferenced object 0x8881010b5278 (size 56): comm "systemd-udevd", pid 198, jiffies 4294881523 (age 926.930s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 0d 01 70 00 00 00 00 00 ..p. 4b 5f 02 00 00 c9 ff ff 00 00 00 00 00 00 00 00 K_.. backtrace: [] acpi_ps_alloc_op+0x8b/0x1c4 [] acpi_ps_create_op+0x48c/0x8ca [] acpi_ps_parse_loop+0x401/0x1062 [] acpi_ps_parse_aml+0x1cd/0x6ea [] acpi_ps_execute_method+0x51f/0x57b [] acpi_ns_evaluate+0x64b/0x885 [] acpi_evaluate_object+0x333/0x68d [] amdgpu_atcs_call.constprop.0+0x129/0x1f0 [amdgpu] [] amdgpu_atcs_pci_probe_handle.isra.0+0x159/0x2d0 [amdgpu] [] amdgpu_acpi_detect+0xb0/0x470 [amdgpu] [] chacha_2block_xor_avx2+0xa8/0x220 [chacha_x86_64] [] do_one_initcall+0xbb/0x3e0 [] do_init_module+0x1c9/0x6e0 [] load_module+0x78c7/0x9bb0 [] __do_sys_finit_module+0x110/0x180 [] do_syscall_64+0x66/0x90 unreferenced object 0x8881010b5c38 (size 56): comm "systemd-udevd", pid 198, jiffies 4294881523 (age 926.930s) hex dump (first 32 bytes): 78 52 0b 01 81 88 ff ff 0d 01 2d 00 00 00 00 00 xR-. 4c 5f 02 00 00 c9 ff ff 00 00 00 00 00 00 00 00 L_.. backtrace: [] acpi_ps_alloc_op+0x8b/0x1c4 [] acpi_ps_create_op+0x48c/0x8ca [] acpi_ps_parse_loop+0x401/0x1062 [] acpi_ps_parse_aml+0x1cd/0x6ea [] acpi_ps_execute_method+0x51f/0x57b [] acpi_ns_evaluate+0x64b/0x885 [] acpi_evaluate_object+0x333/0x68d [] amdgpu_atcs_call.constprop.0+0x129/0x1f0 [amdgpu] [] amdgpu_atcs_pci_probe_handle.isra.0+0x159/0x2d0 [amdgpu] [] amdgpu_acpi_detect+0xb0/0x470 [amdgpu] [] chacha_2block_xor_avx2+0xa8/0x220 [chacha_x86_64] [] do_one_initcall+0xbb/0x3e0 [] do_init_module+0x1c9/0x6e0 [] load_module+0x78c7/0x9bb0 [] __do_sys_finit_module+0x110/0x180 [] do_syscall_64+0x66/0x90 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v2, 1/1] mailbox: cmdq: add instruction time-out interrupt support
Hi, Yongqiang: Yongqiang Niu 於 2021年9月30日 週四 下午9:18寫道: > > add time-out cycle setting to make sure time-out interrupt irq > will happened when instruction time-out for wait and poll > > Signed-off-by: Yongqiang Niu > --- > drivers/mailbox/mtk-cmdq-mailbox.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > b/drivers/mailbox/mtk-cmdq-mailbox.c > index 64175a893312..197b03222f94 100644 > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > @@ -36,6 +36,7 @@ > #define CMDQ_THR_END_ADDR 0x24 > #define CMDQ_THR_WAIT_TOKEN0x30 > #define CMDQ_THR_PRIORITY 0x40 > +#define CMDQ_THR_INSTN_TIMEOUT_CYCLES 0x50 > > #define GCE_GCTL_VALUE 0x48 > > @@ -54,6 +55,15 @@ > #define CMDQ_JUMP_BY_OFFSET0x1000 > #define CMDQ_JUMP_BY_PA0x1001 > > +/* > + * instruction time-out > + * cycles to issue instruction time-out interrupt for wait and poll > instructions > + * GCE axi_clock 156MHz > + * 1 cycle = 6.41ns > + * instruction time out 2^22*2*6.41ns = 53ms For different clients, the timeout value would be different, and each client could use timer to detect timeout, so it's not necessary to enable timeout in cmdq driver. Regards, Chun-Kuang. > + */ > +#define CMDQ_INSTN_TIMEOUT_CYCLES 22 > + > struct cmdq_thread { > struct mbox_chan*chan; > void __iomem*base; > @@ -376,6 +386,7 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, > void *data) > writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa, >thread->base + CMDQ_THR_END_ADDR); > > + writel(CMDQ_INSTN_TIMEOUT_CYCLES, thread->base + > CMDQ_THR_INSTN_TIMEOUT_CYCLES); > writel(thread->priority, thread->base + CMDQ_THR_PRIORITY); > writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE); > writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK); > -- > 2.25.1 >
[Bug 214621] WARNING: CPU: 3 PID: 521 at drivers/gpu/drm/ttm/ttm_bo.c:409 ttm_bo_release+0xb64/0xe40 [ttm]
https://bugzilla.kernel.org/show_bug.cgi?id=214621 --- Comment #1 from Erhard F. (erhar...@mailbox.org) --- Created attachment 299099 --> https://bugzilla.kernel.org/attachment.cgi?id=299099&action=edit kernel .config (5.15-rc4, AMD PRO A10-8750B) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 214621] New: WARNING: CPU: 3 PID: 521 at drivers/gpu/drm/ttm/ttm_bo.c:409 ttm_bo_release+0xb64/0xe40 [ttm]
https://bugzilla.kernel.org/show_bug.cgi?id=214621 Bug ID: 214621 Summary: WARNING: CPU: 3 PID: 521 at drivers/gpu/drm/ttm/ttm_bo.c:409 ttm_bo_release+0xb64/0xe40 [ttm] Product: Drivers Version: 2.5 Kernel Version: 5,15-rc4 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: erhar...@mailbox.org CC: ray.hu...@amd.com Regression: No Created attachment 299097 --> https://bugzilla.kernel.org/attachment.cgi?id=299097&action=edit kernel dmesg (5.15-rc4, AMD PRO A10-8750B) Happened during reboot. Machine was able to reboot succesfully however. [...] [ cut here ] WARNING: CPU: 3 PID: 521 at drivers/gpu/drm/ttm/ttm_bo.c:409 ttm_bo_release+0xb64/0xe40 [ttm] Modules linked in: rfcomm cmac bnep btusb btrtl btbcm btintel bluetooth jitterentropy_rng sha512_ssse3 sha512_generic drbg ansi_cprng ecdh_generic ecc rfkill dm_crypt nhpoly1305_sse2 nhpoly1305 chacha_generic chacha_x86_64 libchacha adiantum libpoly1305 algif_skcipher input_leds led_class joydev dm_mod hid_generic usbhid hid f2fs evdev crc32_generic lz4hc_compress raid456 async_raid6_recov async_memcpy lz4_compress async_pq async_xor lz4_decompress async_tx crc32_pclmul ohci_pci md_mod aesni_intel libaes crypto_simd cryptd amdgpu ext4 crc16 fam15h_power k10temp snd_hda_codec_hdmi mbcache ohci_hcd ehci_pci jbd2 ehci_hcd i2c_piix4 snd_hda_intel drm_ttm_helper ttm snd_intel_dspcfg mfd_core snd_hda_codec gpu_sched i2c_algo_bit xhci_pci snd_hwdep snd_hda_core xhci_hcd drm_kms_helper snd_pcm usbcore snd_timer usb_common syscopyarea sysfillrect snd sysimgblt fb_sys_fops soundcore acpi_cpufreq video button processor zram zsmalloc nct6775 hwmon_vid hwmon nfsd auth_rpcgss lockd grace drm fuse drm_panel_orientation_quirks backlight configfs sunrpc efivarfs CPU: 3 PID: 521 Comm: X Not tainted 5.15.0-rc4-bdver3 #2 Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./A88M-G/3.1, BIOS P1.40C 11/21/2016 RIP: 0010:ttm_bo_release+0xb64/0xe40 [ttm] Code: c1 ea 03 80 3c 02 00 0f 85 77 01 00 00 48 8b bb f0 fe ff ff b9 28 23 00 00 31 d2 be 01 00 00 00 e8 81 bc 50 dd e9 d3 fe ff ff <0f> 0b e9 1c f5 ff ff 4c 89 e7 e8 4d 4d 50 dd e9 26 fc ff ff be 03 RSP: 0018:c90001a8fbe0 EFLAGS: 00010206 RAX: 0007 RBX: 888106ade698 RCX: 0009 RDX: RSI: 0004 RDI: 888106ade698 RBP: 888106ade458 R08: c0ba3689 R09: 888106ade69b R10: ed1020d5bcd3 R11: 0001 R12: 88814db40010 R13: 88814c0ad2f8 R14: 88814c0ad340 R15: 88816a9fb1c0 FS: 7f52f84f2dc0() GS:8883d178() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f1fd3d6d078 CR3: 000142fda000 CR4: 000506e0 Call Trace: ? fsnotify_grab_connector+0xcc/0x190 ? fsnotify_destroy_marks+0x5f/0x200 amdgpu_bo_unref+0x2c/0x60 [amdgpu] amdgpu_gem_object_free+0x6a/0xa0 [amdgpu] ? amdgpu_gem_object_mmap+0xe0/0xe0 [amdgpu] ? trace_hardirqs_on+0x1c/0x110 drm_gem_dmabuf_release+0x82/0xb0 [drm] dma_buf_release+0x127/0x230 __dentry_kill+0x376/0x550 ? dma_buf_file_release+0x177/0x200 __fput+0x2c0/0x8c0 task_work_run+0xc5/0x150 do_exit+0x799/0x20c0 ? mm_update_next_owner+0x6d0/0x6d0 do_group_exit+0xe7/0x290 __x64_sys_exit_group+0x35/0x40 do_syscall_64+0x66/0x90 ? do_syscall_64+0xe/0x90 entry_SYSCALL_64_after_hwframe+0x44/0xae RIP: 0033:0x7f52f7d8c2f9 Code: Unable to access opcode bytes at RIP 0x7f52f7d8c2cf. RSP: 002b:7ffc11abca58 EFLAGS: 0246 ORIG_RAX: 00e7 RAX: ffda RBX: 7f52f7e74920 RCX: 7f52f7d8c2f9 RDX: 003c RSI: 00e7 RDI: RBP: 7f52f7e74920 R08: fd40 R09: 55d959e97190 R10: 7f52f75386b8 R11: 0246 R12: R13: R14: 0668 R15: irq event stamp: 815615493 hardirqs last enabled at (815615499): [] vprintk_emit+0x2dc/0x310 hardirqs last disabled at (815615504): [] vprintk_emit+0x28b/0x310 softirqs last enabled at (815614376): [] unix_release_sock+0x23c/0xa70 softirqs last disabled at (815614374): [] unix_release_sock+0x1c2/0xa70 ---[ end trace 4449f17f76814cfa ]--- # lspci 00:00.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 30h-3fh) Processor Root Complex 00:00.2 IOMMU: Advanced Micro Devices, Inc. [AMD] Family 15h (Models 30h-3fh) I/O Memory Management Unit 00:01.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Kaveri [Radeon R7 Graphics] (rev d7) 00:01.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Kaveri HDMI/DP Audio Controller 00:02.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family
Re: [PATCH] drm/msm: potential error pointer dereference in init()
On 04/10/2021 13:38, Dan Carpenter wrote: The msm_iommu_new() returns error pointers on failure so check for that to avoid an Oops. Fixes: ccac7ce373c1 ("drm/msm: Refactor address space initialization") Signed-off-by: Dan Carpenter --- Reviewed-by: Dmitry Baryshkov drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ae48f41821cf..ad247c06e198 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -908,6 +908,10 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) return 0; mmu = msm_iommu_new(dpu_kms->dev->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); -- With best wishes Dmitry
Re: [PATCH] drm/msm: potential error pointer dereference in init()
On 2021-10-04 03:38, Dan Carpenter wrote: The msm_iommu_new() returns error pointers on failure so check for that to avoid an Oops. Fixes: ccac7ce373c1 ("drm/msm: Refactor address space initialization") Signed-off-by: Dan Carpenter Reviewed-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ae48f41821cf..ad247c06e198 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -908,6 +908,10 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) return 0; mmu = msm_iommu_new(dpu_kms->dev->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);
Re: [PATCH] drm/msm: Fix potential Oops in a6xx_gmu_rpmh_init()
On 04/10/2021 16:45, Dan Carpenter wrote: There are two problems here: 1) The "seqptr" is used uninitalized when we free it at the end. This looks like a nice catch, potentially causing troubles. 2) The a6xx_gmu_get_mmio() function returns error pointers. It never returns true. Fixes: 64245fc55172 ("drm/msm/a6xx: use AOP-initialized PDC for a650") Fixes: f8fc924e088e ("drm/msm/a6xx: Fix PDC register overlap") Signed-off-by: Dan Carpenter Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index a7c58018959f..3bd6e579ea89 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -512,11 +512,11 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu) struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; struct platform_device *pdev = to_platform_device(gmu->dev); void __iomem *pdcptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc"); - void __iomem *seqptr; + void __iomem *seqptr = NULL; uint32_t pdc_address_offset; bool pdc_in_aop = false; - if (!pdcptr) + if (IS_ERR(pdcptr)) goto err; if (adreno_is_a650(adreno_gpu) || adreno_is_a660_family(adreno_gpu)) @@ -528,7 +528,7 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu) if (!pdc_in_aop) { seqptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc_seq"); - if (!seqptr) + if (IS_ERR(seqptr)) goto err; } -- With best wishes Dmitry
Re: [PATCH] drm/msm/disp: fix endian bug in debugfs code
On 2021-10-04 06:47, Dan Carpenter wrote: The "vbif->features" is type unsigned long but the debugfs file is treating it as a u32 type. This will work in little endian systems, but the correct thing is to change the debugfs to use an unsigned long. Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") Signed-off-by: Dan Carpenter Reviewed-by: Abhinav Kumar --- You might wonder why this code has so many casts. It's required because this data is const. Which is fine because the file is read only. drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c index 21d20373eb8b..e645a886e3c6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c @@ -305,8 +305,8 @@ void dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root) debugfs_vbif = debugfs_create_dir(vbif_name, entry); - debugfs_create_u32("features", 0600, debugfs_vbif, - (u32 *)&vbif->features); + debugfs_create_ulong("features", 0600, debugfs_vbif, +(unsigned long *)&vbif->features); debugfs_create_u32("xin_halt_timeout", 0400, debugfs_vbif, (u32 *)&vbif->xin_halt_timeout);
Re: [PATCH] drm/msm/disp: fix endian bug in debugfs code
On 04/10/2021 16:47, Dan Carpenter wrote: The "vbif->features" is type unsigned long but the debugfs file is treating it as a u32 type. This will work in little endian systems, but the correct thing is to change the debugfs to use an unsigned long. Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") Signed-off-by: Dan Carpenter --- You might wonder why this code has so many casts. It's required because this data is const. Which is fine because the file is read only. drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c index 21d20373eb8b..e645a886e3c6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c @@ -305,8 +305,8 @@ void dpu_debugfs_vbif_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root) debugfs_vbif = debugfs_create_dir(vbif_name, entry); - debugfs_create_u32("features", 0600, debugfs_vbif, - (u32 *)&vbif->features); + debugfs_create_ulong("features", 0600, debugfs_vbif, +(unsigned long *)&vbif->features); As you are converting this to the ulong file, could you please also remove the now-unnecessary type cast? debugfs_create_u32("xin_halt_timeout", 0400, debugfs_vbif, (u32 *)&vbif->xin_halt_timeout); -- With best wishes Dmitry
Re: [PATCH v3 11/14] drm/msm/dp: Re-order dp_audio_put in deinit_sub_modules
On 01/10/2021 18:11, Sean Paul wrote: From: Sean Paul Audio is initialized last, it should be de-initialized first to match the order in dp_init_sub_modules(). Reviewed-by: Abhinav Kumar Reviewed-by: Stephen Boyd Signed-off-by: Sean Paul Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-12-s...@poorly.run #v1 Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-12-s...@poorly.run #v2 Reviewed-by: Dmitry Baryshkov Changes in v2: -None --- drivers/gpu/drm/msm/dp/dp_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index fbe4c2cd52a3..19946024e235 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -714,9 +714,9 @@ static int dp_irq_hpd_handle(struct dp_display_private *dp, u32 data) static void dp_display_deinit_sub_modules(struct dp_display_private *dp) { dp_debug_put(dp->debug); + dp_audio_put(dp->audio); dp_panel_put(dp->panel); dp_aux_put(dp->aux); - dp_audio_put(dp->audio); } static int dp_init_sub_modules(struct dp_display_private *dp) -- With best wishes Dmitry
Re: [PATCH v3 10/14] drm/msm/dpu: Remove encoder->enable() hack
On 01/10/2021 18:11, Sean Paul wrote: From: Sean Paul encoder->commit() was being misused because there were some global resources which needed to be tweaked in encoder->enable() which were not accessible in dpu_encoder.c. That is no longer true and the redirect serves no purpose any longer. So remove the indirection. Reviewed-by: Stephen Boyd Tested-by: Stephen Boyd Signed-off-by: Sean Paul Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-11-s...@poorly.run #v1 Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-11-s...@poorly.run #v2 Reviewed-by: Dmitry Baryshkov Changes in v2: -None Changes in v3: -None --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 22 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 4 4 files changed, 1 insertion(+), 32 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 984f8a59cb73..ddc542a0d41f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2122,11 +2122,8 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t) static const struct drm_encoder_helper_funcs dpu_encoder_helper_funcs = { .mode_set = dpu_encoder_virt_mode_set, .disable = dpu_encoder_virt_disable, - .enable = dpu_kms_encoder_enable, + .enable = dpu_encoder_virt_enable, .atomic_check = dpu_encoder_virt_atomic_check, - - /* This is called by dpu_kms_encoder_enable */ - .commit = dpu_encoder_virt_enable, }; static const struct drm_encoder_funcs dpu_encoder_funcs = { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index fb0d9f781c66..4a0b55d145ad 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -381,28 +381,6 @@ static void dpu_kms_flush_commit(struct msm_kms *kms, unsigned crtc_mask) } } -/* - * Override the encoder enable since we need to setup the inline rotator and do - * some crtc magic before enabling any bridge that might be present. - */ -void dpu_kms_encoder_enable(struct drm_encoder *encoder) -{ - const struct drm_encoder_helper_funcs *funcs = encoder->helper_private; - struct drm_device *dev = encoder->dev; - struct drm_crtc *crtc; - - /* Forward this enable call to the commit hook */ - if (funcs && funcs->commit) - funcs->commit(encoder); - - drm_for_each_crtc(crtc, dev) { - if (!(crtc->state->encoder_mask & drm_encoder_mask(encoder))) - continue; - - trace_dpu_kms_enc_enable(DRMID(crtc)); - } -} - static void dpu_kms_complete_commit(struct msm_kms *kms, unsigned crtc_mask) { struct dpu_kms *dpu_kms = to_dpu_kms(kms); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 323a6bce9e64..f1ebb60dacab 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -248,8 +248,6 @@ void *dpu_debugfs_get_root(struct dpu_kms *dpu_kms); int dpu_enable_vblank(struct msm_kms *kms, struct drm_crtc *crtc); void dpu_disable_vblank(struct msm_kms *kms, struct drm_crtc *crtc); -void dpu_kms_encoder_enable(struct drm_encoder *encoder); - /** * dpu_kms_get_clk_rate() - get the clock rate * @dpu_kms: pointer to dpu_kms structure diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 37bba57675a8..54d74341e690 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -266,10 +266,6 @@ DEFINE_EVENT(dpu_drm_obj_template, dpu_crtc_complete_commit, TP_PROTO(uint32_t drm_id), TP_ARGS(drm_id) ); -DEFINE_EVENT(dpu_drm_obj_template, dpu_kms_enc_enable, - TP_PROTO(uint32_t drm_id), - TP_ARGS(drm_id) -); DEFINE_EVENT(dpu_drm_obj_template, dpu_kms_commit, TP_PROTO(uint32_t drm_id), TP_ARGS(drm_id) -- With best wishes Dmitry
Re: [PATCH v3 09/14] drm/msm/dpu: Remove useless checks in dpu_encoder
On 01/10/2021 18:11, Sean Paul wrote: From: Sean Paul A couple more useless checks to remove in dpu_encoder. Reviewed-by: Stephen Boyd Signed-off-by: Sean Paul Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-10-s...@poorly.run #v1 Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-10-s...@poorly.run #v2 Reviewed-by: Dmitry Baryshkov Changes in v2: -None Changes in v3: -None --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 1 file changed, 12 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 0e9d3fa1544b..984f8a59cb73 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1153,10 +1153,6 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc) struct msm_drm_private *priv; struct drm_display_mode *cur_mode = NULL; - if (!drm_enc) { - DPU_ERROR("invalid encoder\n"); - return; - } dpu_enc = to_dpu_encoder_virt(drm_enc); mutex_lock(&dpu_enc->enc_lock); @@ -1203,14 +1199,6 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc) struct msm_drm_private *priv; int i = 0; - if (!drm_enc) { - DPU_ERROR("invalid encoder\n"); - return; - } else if (!drm_enc->dev) { - DPU_ERROR("invalid dev\n"); - return; - } - dpu_enc = to_dpu_encoder_virt(drm_enc); DPU_DEBUG_ENC(dpu_enc, "\n"); -- With best wishes Dmitry
Re: [PATCH v3 08/14] drm/msm/dpu_kms: Re-order dpu includes
On 01/10/2021 18:11, Sean Paul wrote: From: Sean Paul Make includes alphabetical in dpu_kms.c Reviewed-by: Abhinav Kumar Reviewed-by: Stephen Boyd Signed-off-by: Sean Paul Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-9-s...@poorly.run #v1 Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-9-s...@poorly.run #v2 Changes in v2: -None Changes in v3: -None Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ae48f41821cf..fb0d9f781c66 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -21,14 +21,14 @@ #include "msm_gem.h" #include "disp/msm_disp_snapshot.h" -#include "dpu_kms.h" #include "dpu_core_irq.h" +#include "dpu_crtc.h" +#include "dpu_encoder.h" #include "dpu_formats.h" #include "dpu_hw_vbif.h" -#include "dpu_vbif.h" -#include "dpu_encoder.h" +#include "dpu_kms.h" #include "dpu_plane.h" -#include "dpu_crtc.h" +#include "dpu_vbif.h" #define CREATE_TRACE_POINTS #include "dpu_trace.h" -- With best wishes Dmitry
[PATCH i-g-t v12 00/15] Introduce PXP Test
This series adds gem_pxp tests for the new PXP subsystem currently being reviewed at https://patchwork.freedesktop.org/series/90503/. This series currently includes 4 groups of tests addressing the features and restrictions described by Daniele's series : 1. test i915 interfaces for allocation of protected bo's and contexts and enforcement of UAPI rule disallowing the modification of parameters after it's been created. 2. verify PXP subsystem protected sessions generate encrypted content on protected output buffers and decrypt protected inputs buffers. 3. verify i915 PXP auto-teardown succeeds on suspend-resume cycles and gem-exec of stale protected assets fail. Ensure protected-contexts adhere to stricter invalidation enforcement upon teardown event. 4. Ensure that display plane decryption works as expected with protected buffers. NOTE: This series is on the tenth revision. All R-v-b's have been received except UAPI patch which will be dropped at merge time (after kernel patches gets merged and igt's drm UAPI gets sync'd). Changes from prior rev1 to now: v12: - Rebase on latest igt and updated the UAPI changes to align with commits from kernel side. v11: - When detecting hw support, retry pxp context creation multiple times a timeout as per HW SLA. - initialize bo or ctx handles to zero before calling creation ioctl wrapper. v10: - In patch #2, reuse existing gem_create_ext wrapper. - In patch #10, kernel side changed the debugfs file name (but no difference in behavior / usage). - Removed patch #14 from Rev9 as decision on kernel side was to drop the usage of RESET_STATS IOCTL to track invalidated pxp contexts. v9: - Remove patch #2 from rev7 as it was duplicating an existing ioctl wrapper helper - Fix the false-negative warnings when triggering auto-suspend-resume (remove checking if we are suspending after the system has already resumed). v8: - Nothing - mistaken detection from patchwork v7: - In prior rev, Patches #11->13 was testing expected results from calling gem_execbuf with stale pxp-context, pxp-buffer or combinations of them (including an opt-out usage). All of them used a single suspend-resume power state cycles to trigger the PXP teardown event. These patches have been combined into patch #14 that continues to carry the prior rev Rvb. - In its place, the new patches of #11->#13 do the identical set of tests as before (results from gem_execbuf with various combinations of stale pxp context and buffer), but this time using a debugfs file handle that triggers the same code path taken when the HW triggers the pxp teardown. That said, the code is nearly identical as v6 but I did not keep the Rvb's. - In patch #15, RESET_STAT now reports invalidated / banned pxp contexts via the existing batch_active's lost count. v6: - Addressed rev5 review comments for patch #1, #7, #14 and #17. - For #17, I'm using Rodrigo's Rv-b because offline discussions concluded that we couldn't use those test sequences with HDCP and so it was removed it. - Added Rv-b into all patches that received it. - Modified the test requirement from a list of device ids to checking if runtime PXP interface succeeds due to kernel's build config dependency. v5: - Addressed all rev4 review comments. No changes to overall flow and logic compared to the last rev. v4: - Addressed all rev3 review comments. NOTE: that all test cases and code logic are the same but a decent amount of refactoring has occured due to address v3 comments to break out subtests into separate functions while combining certain checks into the same function to reduce test time by minimizing number of suspend-resume power cycles. v3: - Addressed all rev2 review comments. - In line with one of the rev2 comments, a thorough fixup of all line-breaks in function calls was made for a more consistent styling. - Rebased on igt upstream repo and updated to latest kernel UAPI that added GEM_CREATE_EXT. v2: - Addressed all rev1 review comments except these: 1.Chris Wilson : "...have the caller do 1-3 once for its protected context. Call it something like intel_bb_enable_pxp(), intel_bb_set_pxp if it should be reversible.". - This couldn't be implemented because [1] HW needs different instruction sequences for enabling/disabling PXP depending on the engine class and [2] the pair of "pxp-enable" and "pxp- disable" instructions need to be contained within the same batch tha
[PATCH 25/26] drm/i915: Enable multi-bb execbuf
Enable multi-bb execbuf by enabling the set_parallel extension. Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 6290bc20ccb1..605440388988 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -536,9 +536,6 @@ set_proto_ctx_engines_parallel_submit(struct i915_user_extension __user *base, struct intel_engine_cs **siblings = NULL; intel_engine_mask_t prev_mask; - /* Disabling for now */ - return -ENODEV; - /* FIXME: This is NIY for execlists */ if (!(intel_uc_uses_guc_submission(&i915->gt.uc))) return -ENODEV; -- 2.32.0
[PATCH 22/26] drm/i915/guc: Handle errors in multi-lrc requests
If an error occurs in the front end when multi-lrc requests are getting generated we need to skip these in the backend but we still need to emit the breadcrumbs seqno. An issues arises because with multi-lrc breadcrumbs there is a handshake between the parent and children to make forward progress. If all the requests are not present this handshake doesn't work. To work around this, if multi-lrc request has an error we skip the handshake but still emit the breadcrumbs seqno. v2: (John Harrison) - Add comment explaining the skipping of the handshake logic - Fix typos in the commit message Signed-off-by: Matthew Brost --- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 71 ++- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 83b0d2a114af..05e8b199e4ce 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -4072,8 +4072,8 @@ static int emit_bb_start_child_no_preempt_mid_batch(struct i915_request *rq, } static u32 * -emit_fini_breadcrumb_parent_no_preempt_mid_batch(struct i915_request *rq, -u32 *cs) +__emit_fini_breadcrumb_parent_no_preempt_mid_batch(struct i915_request *rq, + u32 *cs) { struct intel_context *ce = rq->context; u8 i; @@ -4101,6 +4101,46 @@ emit_fini_breadcrumb_parent_no_preempt_mid_batch(struct i915_request *rq, get_children_go_addr(ce), 0); + return cs; +} + +/* + * If this true, a submission of multi-lrc requests had an error and the + * requests need to be skipped. The front end (execuf IOCTL) should've called + * i915_request_skip which squashes the BB but we still need to emit the fini + * breadrcrumbs seqno write. At this point we don't know how many of the + * requests in the multi-lrc submission were generated so we can't do the + * handshake between the parent and children (e.g. if 4 requests should be + * generated but 2nd hit an error only 1 would be seen by the GuC backend). + * Simply skip the handshake, but still emit the breadcrumbd seqno, if an error + * has occurred on any of the requests in submission / relationship. + */ +static inline bool skip_handshake(struct i915_request *rq) +{ + return test_bit(I915_FENCE_FLAG_SKIP_PARALLEL, &rq->fence.flags); +} + +static u32 * +emit_fini_breadcrumb_parent_no_preempt_mid_batch(struct i915_request *rq, +u32 *cs) +{ + struct intel_context *ce = rq->context; + + GEM_BUG_ON(!intel_context_is_parent(ce)); + + if (unlikely(skip_handshake(rq))) { + /* +* NOP everything in +* __emit_fini_breadcrumb_parent_no_preempt_mid_batch, the -6 +* comes of the length emission below. +*/ + memset(cs, 0, sizeof(u32) * + (ce->engine->emit_fini_breadcrumb_dw - 6)); + cs += ce->engine->emit_fini_breadcrumb_dw - 6; + } else { + cs = __emit_fini_breadcrumb_parent_no_preempt_mid_batch(rq, cs); + } + /* Emit fini breadcrumb */ cs = gen8_emit_ggtt_write(cs, rq->fence.seqno, @@ -4117,7 +4157,8 @@ emit_fini_breadcrumb_parent_no_preempt_mid_batch(struct i915_request *rq, } static u32 * -emit_fini_breadcrumb_child_no_preempt_mid_batch(struct i915_request *rq, u32 *cs) +__emit_fini_breadcrumb_child_no_preempt_mid_batch(struct i915_request *rq, + u32 *cs) { struct intel_context *ce = rq->context; struct intel_context *parent = intel_context_to_parent(ce); @@ -4144,6 +4185,30 @@ emit_fini_breadcrumb_child_no_preempt_mid_batch(struct i915_request *rq, u32 *cs *cs++ = get_children_go_addr(parent); *cs++ = 0; + return cs; +} + +static u32 * +emit_fini_breadcrumb_child_no_preempt_mid_batch(struct i915_request *rq, + u32 *cs) +{ + struct intel_context *ce = rq->context; + + GEM_BUG_ON(!intel_context_is_child(ce)); + + if (unlikely(skip_handshake(rq))) { + /* +* NOP everything in +* __emit_fini_breadcrumb_child_no_preempt_mid_batch, the -6 +* comes from the length the emission below. +*/ + memset(cs, 0, sizeof(u32) * + (ce->engine->emit_fini_breadcrumb_dw - 6)); + cs += ce->engine->emit_fini_breadcrumb_dw - 6; + } else { + cs = __emit_fini_breadcrumb_child_no_preempt_mid_batch(rq, cs); + } + /* Emit fini breadcrumb */ cs = gen8_emit_ggtt_write(cs, r
[PATCH 20/26] drm/i915/guc: Implement no mid batch preemption for multi-lrc
For some users of multi-lrc, e.g. split frame, it isn't safe to preempt mid BB. To safely enable preemption at the BB boundary, a handshake between to parent and child is needed. This is implemented via custom emit_bb_start & emit_fini_breadcrumb functions and enabled via by default if a context is configured by set parallel extension. v2: (John Harrison) - Fix a few comments wording - Add struture for parent page layout Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_context.c | 2 +- drivers/gpu/drm/i915/gt/intel_context_types.h | 2 + drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 2 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 330 +- 4 files changed, 324 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 3b340eb59ada..ee84259959d0 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -569,7 +569,7 @@ void intel_context_bind_parent_child(struct intel_context *parent, GEM_BUG_ON(intel_context_is_child(child)); GEM_BUG_ON(intel_context_is_parent(child)); - parent->parallel.number_children++; + parent->parallel.child_index = parent->parallel.number_children++; list_add_tail(&child->parallel.child_link, &parent->parallel.child_list); child->parallel.parent = parent; diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 1d880303a7e4..95a5b94b4ece 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -250,6 +250,8 @@ struct intel_context { struct i915_request *last_rq; /** @number_children: number of children if parent */ u8 number_children; + /** @child_index: index into child_list if child */ + u8 child_index; /** @guc: GuC specific members for parallel submission */ struct { /** @wqi_head: head pointer in work queue */ diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h index a00eeddc1449..663950d3badc 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h @@ -181,7 +181,7 @@ struct guc_process_desc { u32 wq_status; u32 engine_presence; u32 priority; - u32 reserved[30]; + u32 reserved[36]; } __packed; #define CONTEXT_REGISTRATION_FLAG_KMD BIT(0) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 12ee8ca76249..f28e36aa77c2 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -11,6 +11,7 @@ #include "gt/intel_context.h" #include "gt/intel_engine_pm.h" #include "gt/intel_engine_heartbeat.h" +#include "gt/intel_gpu_commands.h" #include "gt/intel_gt.h" #include "gt/intel_gt_irq.h" #include "gt/intel_gt_pm.h" @@ -368,10 +369,16 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb) /* * When using multi-lrc submission an extra page in the context state is - * reserved for the process descriptor and work queue. + * reserved for the process descriptor, work queue, and handshake between the + * parent + childlren contexts to insert safe preemption points between each set + * of BBs. * * The layout of this page is below: * 0 guc_process_desc + * + sizeof(struct guc_process_desc) child go + * + CACHELINE_BYTES child join[0] + * ... + * + CACHELINE_BYTES child join[n - 1] * ... unused * PAGE_SIZE / 2 work queue start * ... work queue @@ -379,7 +386,25 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb) */ #define WQ_SIZE(PAGE_SIZE / 2) #define WQ_OFFSET (PAGE_SIZE - WQ_SIZE) -static u32 __get_process_desc_offset(struct intel_context *ce) + +struct parent_page { + struct guc_process_desc pdesc; + + u32 child_go_memory; + u8 unused0[CACHELINE_BYTES - sizeof(u32)]; + + struct { + u32 child_join_memory; + u8 unused1[CACHELINE_BYTES - sizeof(u32)]; + } join[MAX_ENGINE_INSTANCE + 1]; + + u8 unused2[(WQ_OFFSET - sizeof(struct guc_process_desc) - + CACHELINE_BYTES * (MAX_ENGINE_INSTANCE + 2))]; + + u32 wq[WQ_SIZE / sizeof(u32)]; +}; + +static u32 __get_parent_page_offset(struct intel_context *ce) { GEM_BUG_ON(!ce->parallel.guc.parent_page); @@ -388,23 +413,35 @@ static u32 __get_process_desc_offset(struct intel_context *ce) static u3
[PATCH 26/26] drm/i915/execlists: Weak parallel submission support for execlists
A weak implementation of parallel submission (multi-bb execbuf IOCTL) for execlists. Doing as little as possible to support this interface for execlists - basically just passing submit fences between each request generated and virtual engines are not allowed. This is on par with what is there for the existing (hopefully soon deprecated) bonding interface. We perma-pin these execlists contexts to align with GuC implementation. Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 10 ++-- drivers/gpu/drm/i915/gt/intel_context.c | 4 +- .../drm/i915/gt/intel_execlists_submission.c | 56 ++- drivers/gpu/drm/i915/gt/intel_lrc.c | 2 + .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 - 5 files changed, 64 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 605440388988..732111457dd2 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -536,10 +536,6 @@ set_proto_ctx_engines_parallel_submit(struct i915_user_extension __user *base, struct intel_engine_cs **siblings = NULL; intel_engine_mask_t prev_mask; - /* FIXME: This is NIY for execlists */ - if (!(intel_uc_uses_guc_submission(&i915->gt.uc))) - return -ENODEV; - if (get_user(slot, &ext->engine_index)) return -EFAULT; @@ -549,6 +545,12 @@ set_proto_ctx_engines_parallel_submit(struct i915_user_extension __user *base, if (get_user(num_siblings, &ext->num_siblings)) return -EFAULT; + if (!intel_uc_uses_guc_submission(&i915->gt.uc) && num_siblings != 1) { + drm_dbg(&i915->drm, "Only 1 sibling (%d) supported in non-GuC mode\n", + num_siblings); + return -EINVAL; + } + if (slot >= set->num_engines) { drm_dbg(&i915->drm, "Invalid placement value, %d >= %d\n", slot, set->num_engines); diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index ee84259959d0..3fc1c5155fd4 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -79,7 +79,8 @@ static int intel_context_active_acquire(struct intel_context *ce) __i915_active_acquire(&ce->active); - if (intel_context_is_barrier(ce) || intel_engine_uses_guc(ce->engine)) + if (intel_context_is_barrier(ce) || intel_engine_uses_guc(ce->engine) || + intel_context_is_parallel(ce)) return 0; /* Preallocate tracking nodes */ @@ -562,7 +563,6 @@ void intel_context_bind_parent_child(struct intel_context *parent, * Callers responsibility to validate that this function is used * correctly but we use GEM_BUG_ON here ensure that they do. */ - GEM_BUG_ON(!intel_engine_uses_guc(parent->engine)); GEM_BUG_ON(intel_context_is_pinned(parent)); GEM_BUG_ON(intel_context_is_child(parent)); GEM_BUG_ON(intel_context_is_pinned(child)); diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 8d7f571029df..a747fbbf18b5 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -927,8 +927,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) static bool ctx_single_port_submission(const struct intel_context *ce) { - return (IS_ENABLED(CONFIG_DRM_I915_GVT) && - intel_context_force_single_submission(ce)); + return intel_context_force_single_submission(ce); } static bool can_merge_ctx(const struct intel_context *prev, @@ -2598,6 +2597,58 @@ static void execlists_context_cancel_request(struct intel_context *ce, current->comm); } +static struct intel_context * +execlists_create_parallel(struct intel_engine_cs **engines, + unsigned int num_siblings, + unsigned int width) +{ + struct intel_engine_cs **siblings = NULL; + struct intel_context *parent = NULL, *ce, *err; + int i, j; + + GEM_BUG_ON(num_siblings != 1); + + siblings = kmalloc_array(num_siblings, +sizeof(*siblings), +GFP_KERNEL); + if (!siblings) + return ERR_PTR(-ENOMEM); + + for (i = 0; i < width; ++i) { + for (j = 0; j < num_siblings; ++j) + siblings[j] = engines[i * num_siblings + j]; + + ce = intel_context_create(siblings[0]); + if (!ce) { + err = ERR_PTR(-ENOMEM); + goto unwind; + } + + if (i == 0) + parent = ce; +
[PATCH 23/26] drm/i915: Make request conflict tracking understand parallel submits
If an object in the excl or shared slot is a composite fence from a parallel submit and the current request in the conflict tracking is from the same parallel context there is no need to enforce ordering as the ordering already implicit. Make the request conflict tracking understand this by comparing the parents parallel fence values and skipping the conflict insertion if the values match. Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/i915_request.c | 43 +++-- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index e9bfa32f9270..cf89624020ad 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1325,6 +1325,25 @@ i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) return err; } +static inline bool is_parallel_rq(struct i915_request *rq) +{ + return intel_context_is_parallel(rq->context); +} + +static inline struct intel_context *request_to_parent(struct i915_request *rq) +{ + return intel_context_to_parent(rq->context); +} + +static bool is_same_parallel_context(struct i915_request *to, +struct i915_request *from) +{ + if (is_parallel_rq(to)) + return request_to_parent(to) == request_to_parent(from); + + return false; +} + int i915_request_await_execution(struct i915_request *rq, struct dma_fence *fence) @@ -1356,11 +1375,14 @@ i915_request_await_execution(struct i915_request *rq, * want to run our callback in all cases. */ - if (dma_fence_is_i915(fence)) + if (dma_fence_is_i915(fence)) { + if (is_same_parallel_context(rq, to_request(fence))) + continue; ret = __i915_request_await_execution(rq, to_request(fence)); - else + } else { ret = i915_request_await_external(rq, fence); + } if (ret < 0) return ret; } while (--nchild); @@ -1461,10 +1483,13 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) fence)) continue; - if (dma_fence_is_i915(fence)) + if (dma_fence_is_i915(fence)) { + if (is_same_parallel_context(rq, to_request(fence))) + continue; ret = i915_request_await_request(rq, to_request(fence)); - else + } else { ret = i915_request_await_external(rq, fence); + } if (ret < 0) return ret; @@ -1539,16 +1564,6 @@ i915_request_await_object(struct i915_request *to, return ret; } -static inline bool is_parallel_rq(struct i915_request *rq) -{ - return intel_context_is_parallel(rq->context); -} - -static inline struct intel_context *request_to_parent(struct i915_request *rq) -{ - return intel_context_to_parent(rq->context); -} - static struct i915_request * __i915_request_ensure_parallel_ordering(struct i915_request *rq, struct intel_timeline *timeline) -- 2.32.0
[PATCH 24/26] drm/i915: Update I915_GEM_BUSY IOCTL to understand composite fences
Parallel submission create composite fences (dma_fence_array) for excl / shared slots in objects. The I915_GEM_BUSY IOCTL checks these slots to determine the busyness of the object. Prior to patch it only check if the fence in the slot was a i915_request. Update the check to understand composite fences and correctly report the busyness. Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gem/i915_gem_busy.c | 60 +++ .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 5 +- drivers/gpu/drm/i915/i915_request.h | 6 ++ 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_busy.c b/drivers/gpu/drm/i915/gem/i915_gem_busy.c index 6234e17259c1..b89d173c62eb 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_busy.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_busy.c @@ -4,6 +4,8 @@ * Copyright © 2014-2016 Intel Corporation */ +#include + #include "gt/intel_engine.h" #include "i915_gem_ioctls.h" @@ -36,7 +38,7 @@ static __always_inline u32 __busy_write_id(u16 id) } static __always_inline unsigned int -__busy_set_if_active(const struct dma_fence *fence, u32 (*flag)(u16 id)) +__busy_set_if_active(struct dma_fence *fence, u32 (*flag)(u16 id)) { const struct i915_request *rq; @@ -46,29 +48,63 @@ __busy_set_if_active(const struct dma_fence *fence, u32 (*flag)(u16 id)) * to eventually flush us, but to minimise latency just ask the * hardware. * -* Note we only report on the status of native fences. +* Note we only report on the status of native fences and we currently +* have two native fences: +* +* 1. A composite fence (dma_fence_array) constructed of i915 requests +* created during a parallel submission. In this case we deconstruct the +* composite fence into individual i915 requests and check the status of +* each request. +* +* 2. A single i915 request. */ - if (!dma_fence_is_i915(fence)) + if (dma_fence_is_array(fence)) { + struct dma_fence_array *array = to_dma_fence_array(fence); + struct dma_fence **child = array->fences; + unsigned int nchild = array->num_fences; + + do { + struct dma_fence *current_fence = *child++; + + /* Not an i915 fence, can't be busy per above */ + if (!dma_fence_is_i915(current_fence) || + !test_bit(I915_FENCE_FLAG_COMPOSITE, + ¤t_fence->flags)) { + return 0; + } + + rq = to_request(current_fence); + if (!i915_request_completed(rq)) { + BUILD_BUG_ON(!typecheck(u16, + rq->engine->uabi_class)); + return flag(rq->engine->uabi_class); + } + } while (--nchild); + + /* All requests in array complete, not busy */ return 0; + } else { + if (!dma_fence_is_i915(fence)) + return 0; - /* opencode to_request() in order to avoid const warnings */ - rq = container_of(fence, const struct i915_request, fence); - if (i915_request_completed(rq)) - return 0; + rq = to_request(fence); + if (i915_request_completed(rq)) + return 0; - /* Beware type-expansion follies! */ - BUILD_BUG_ON(!typecheck(u16, rq->engine->uabi_class)); - return flag(rq->engine->uabi_class); + /* Beware type-expansion follies! */ + BUILD_BUG_ON(!typecheck(u16, rq->engine->uabi_class)); + return flag(rq->engine->uabi_class); + } } static __always_inline unsigned int -busy_check_reader(const struct dma_fence *fence) +busy_check_reader(struct dma_fence *fence) { return __busy_set_if_active(fence, __busy_read_flag); } static __always_inline unsigned int -busy_check_writer(const struct dma_fence *fence) +busy_check_writer(struct dma_fence *fence) { if (!fence) return 0; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 5c7fb6f68bbb..16276f406fd6 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2988,8 +2988,11 @@ eb_composite_fence_create(struct i915_execbuffer *eb, int out_fence_fd) if (!fences) return ERR_PTR(-ENOMEM); - for_each_batch_create_order(eb, i) + for_each_batch_create_order(eb, i) { fences[i] = &eb->requests[i]->fence; + __set_bit(I915_FENCE_FLAG_COMPOSITE, + &eb->requests[i]->fence.flags); + }
[PATCH 19/26] drm/i915/guc: Add basic GuC multi-lrc selftest
Add very basic (single submission) multi-lrc selftest. Signed-off-by: Matthew Brost Reviewed-by: John Harrison --- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 1 + .../drm/i915/gt/uc/selftest_guc_multi_lrc.c | 179 ++ .../drm/i915/selftests/i915_live_selftests.h | 1 + 3 files changed, 181 insertions(+) create mode 100644 drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 9b19e0d830a2..12ee8ca76249 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -3957,4 +3957,5 @@ bool intel_guc_virtual_engine_has_heartbeat(const struct intel_engine_cs *ve) #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftest_guc.c" +#include "selftest_guc_multi_lrc.c" #endif diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c new file mode 100644 index ..50953c8e8b53 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright �� 2019 Intel Corporation + */ + +#include "selftests/igt_spinner.h" +#include "selftests/igt_reset.h" +#include "selftests/intel_scheduler_helpers.h" +#include "gt/intel_engine_heartbeat.h" +#include "gem/selftests/mock_context.h" + +static void logical_sort(struct intel_engine_cs **engines, int num_engines) +{ + struct intel_engine_cs *sorted[MAX_ENGINE_INSTANCE + 1]; + int i, j; + + for (i = 0; i < num_engines; ++i) + for (j = 0; j < MAX_ENGINE_INSTANCE + 1; ++j) { + if (engines[j]->logical_mask & BIT(i)) { + sorted[i] = engines[j]; + break; + } + } + + memcpy(*engines, *sorted, + sizeof(struct intel_engine_cs *) * num_engines); +} + +static struct intel_context * +multi_lrc_create_parent(struct intel_gt *gt, u8 class, + unsigned long flags) +{ + struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1]; + struct intel_engine_cs *engine; + enum intel_engine_id id; + int i = 0; + + for_each_engine(engine, gt, id) { + if (engine->class != class) + continue; + + siblings[i++] = engine; + } + + if (i <= 1) + return ERR_PTR(0); + + logical_sort(siblings, i); + + return intel_engine_create_parallel(siblings, 1, i); +} + +static void multi_lrc_context_unpin(struct intel_context *ce) +{ + struct intel_context *child; + + GEM_BUG_ON(!intel_context_is_parent(ce)); + + for_each_child(ce, child) + intel_context_unpin(child); + intel_context_unpin(ce); +} + +static void multi_lrc_context_put(struct intel_context *ce) +{ + GEM_BUG_ON(!intel_context_is_parent(ce)); + + /* +* Only the parent gets the creation ref put in the uAPI, the parent +* itself is responsible for creation ref put on the children. +*/ + intel_context_put(ce); +} + +static struct i915_request * +multi_lrc_nop_request(struct intel_context *ce) +{ + struct intel_context *child; + struct i915_request *rq, *child_rq; + int i = 0; + + GEM_BUG_ON(!intel_context_is_parent(ce)); + + rq = intel_context_create_request(ce); + if (IS_ERR(rq)) + return rq; + + i915_request_get(rq); + i915_request_add(rq); + + for_each_child(ce, child) { + child_rq = intel_context_create_request(child); + if (IS_ERR(child_rq)) + goto child_error; + + if (++i == ce->parallel.number_children) + set_bit(I915_FENCE_FLAG_SUBMIT_PARALLEL, + &child_rq->fence.flags); + i915_request_add(child_rq); + } + + return rq; + +child_error: + i915_request_put(rq); + + return ERR_PTR(-ENOMEM); +} + +static int __intel_guc_multi_lrc_basic(struct intel_gt *gt, unsigned int class) +{ + struct intel_context *parent; + struct i915_request *rq; + int ret; + + parent = multi_lrc_create_parent(gt, class, 0); + if (IS_ERR(parent)) { + pr_err("Failed creating contexts: %ld", PTR_ERR(parent)); + return PTR_ERR(parent); + } else if (!parent) { + pr_debug("Not enough engines in class: %d", class); + return 0; + } + + rq = multi_lrc_nop_request(parent); + if (IS_ERR(rq)) { + ret = PTR_ERR(rq); + pr_err("Failed creating requests: %d", ret); + goto out; + } + + ret = intel_selftest_wait_for_rq(rq); + if (ret) + pr_err("Failed waiting on request: %d
[PATCH 21/26] drm/i915: Multi-BB execbuf
Allow multiple batch buffers to be submitted in a single execbuf IOCTL after a context has been configured with the 'set_parallel' extension. The number batches is implicit based on the contexts configuration. This is implemented with a series of loops. First a loop is used to find all the batches, a loop to pin all the HW contexts, a loop to create all the requests, a loop to submit (emit BB start, etc...) all the requests, a loop to tie the requests to the VMAs they touch, and finally a loop to commit the requests to the backend. A composite fence is also created for the generated requests to return to the user and to stick in dma resv slots. No behavior from the existing IOCTL should be changed aside from when throttling because the ring for a context is full, wait on the request while holding the object locks. IGT: https://patchwork.freedesktop.org/patch/447008/?series=93071&rev=1 media UMD: https://github.com/intel/media-driver/pull/1252 v2: (Matthew Brost) - Return proper error value if i915_request_create fails v3: (John Harrison) - Add comment explaining create / add order loops + locking - Update commit message explaining different in IOCTL behavior - Line wrap some comments - eb_add_request returns void - Return -EINVAL rather triggering BUG_ON if cmd parser used (Checkpatch) - Check eb->batch_len[*current_batch] Signed-off-by: Matthew Brost --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 793 -- drivers/gpu/drm/i915/gt/intel_context.h | 8 +- drivers/gpu/drm/i915/gt/intel_context_types.h | 10 + .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 + drivers/gpu/drm/i915/i915_request.h | 9 + drivers/gpu/drm/i915/i915_vma.c | 21 +- drivers/gpu/drm/i915/i915_vma.h | 13 +- 7 files changed, 599 insertions(+), 257 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 2f2434b52317..5c7fb6f68bbb 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -244,17 +244,25 @@ struct i915_execbuffer { struct drm_i915_gem_exec_object2 *exec; /** ioctl execobj[] */ struct eb_vma *vma; - struct intel_engine_cs *engine; /** engine to queue the request to */ + struct intel_gt *gt; /* gt for the execbuf */ struct intel_context *context; /* logical state for the request */ struct i915_gem_context *gem_context; /** caller's context */ - struct i915_request *request; /** our request to build */ - struct eb_vma *batch; /** identity of the batch obj/vma */ + /** our requests to build */ + struct i915_request *requests[MAX_ENGINE_INSTANCE + 1]; + /** identity of the batch obj/vma */ + struct eb_vma *batches[MAX_ENGINE_INSTANCE + 1]; struct i915_vma *trampoline; /** trampoline used for chaining */ + /** used for excl fence in dma_resv objects when > 1 BB submitted */ + struct dma_fence *composite_fence; + /** actual size of execobj[] as we may extend it for the cmdparser */ unsigned int buffer_count; + /* number of batches in execbuf IOCTL */ + unsigned int num_batches; + /** list of vma not yet bound during reservation phase */ struct list_head unbound; @@ -281,7 +289,8 @@ struct i915_execbuffer { u64 invalid_flags; /** Set of execobj.flags that are invalid */ - u64 batch_len; /** Length of batch within object */ + /** Length of batch within object */ + u64 batch_len[MAX_ENGINE_INSTANCE + 1]; u32 batch_start_offset; /** Location within object of batch */ u32 batch_flags; /** Flags composed for emit_bb_start() */ struct intel_gt_buffer_pool_node *batch_pool; /** pool node for batch buffer */ @@ -299,14 +308,13 @@ struct i915_execbuffer { }; static int eb_parse(struct i915_execbuffer *eb); -static struct i915_request *eb_pin_engine(struct i915_execbuffer *eb, - bool throttle); +static int eb_pin_engine(struct i915_execbuffer *eb, bool throttle); static void eb_unpin_engine(struct i915_execbuffer *eb); static inline bool eb_use_cmdparser(const struct i915_execbuffer *eb) { - return intel_engine_requires_cmd_parser(eb->engine) || - (intel_engine_using_cmd_parser(eb->engine) && + return intel_engine_requires_cmd_parser(eb->context->engine) || + (intel_engine_using_cmd_parser(eb->context->engine) && eb->args->batch_len); } @@ -544,11 +552,21 @@ eb_validate_vma(struct i915_execbuffer *eb, return 0; } -static void +static inline bool +is_batch_buffer(struct i915_execbuffer *eb, unsigned int buffer_idx) +{ + return eb->args->flags & I915_EXEC_BATCH_FIRST ? + buffer_idx < eb->num_batches : + buffer_idx >= eb->args->buffer_count - eb->num_batches;
[PATCH 15/26] drm/i915/guc: Update debugfs for GuC multi-lrc
Display the workqueue status in debugfs for GuC contexts that are in parent-child relationship. v2: (John Harrison) - Output number children in debugfs Signed-off-by: Matthew Brost --- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 53 ++- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index d661a69ef4f7..f69e984683aa 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -3704,6 +3704,26 @@ static inline void guc_log_context_priority(struct drm_printer *p, drm_printf(p, "\n"); } + +static inline void guc_log_context(struct drm_printer *p, + struct intel_context *ce) +{ + drm_printf(p, "GuC lrc descriptor %u:\n", ce->guc_id.id); + drm_printf(p, "\tHW Context Desc: 0x%08x\n", ce->lrc.lrca); + drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n", + ce->ring->head, + ce->lrc_reg_state[CTX_RING_HEAD]); + drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n", + ce->ring->tail, + ce->lrc_reg_state[CTX_RING_TAIL]); + drm_printf(p, "\t\tContext Pin Count: %u\n", + atomic_read(&ce->pin_count)); + drm_printf(p, "\t\tGuC ID Ref Count: %u\n", + atomic_read(&ce->guc_id.ref)); + drm_printf(p, "\t\tSchedule State: 0x%x\n\n", + ce->guc_state.sched_state); +} + void intel_guc_submission_print_context_info(struct intel_guc *guc, struct drm_printer *p) { @@ -3713,22 +3733,27 @@ void intel_guc_submission_print_context_info(struct intel_guc *guc, xa_lock_irqsave(&guc->context_lookup, flags); xa_for_each(&guc->context_lookup, index, ce) { - drm_printf(p, "GuC lrc descriptor %u:\n", ce->guc_id.id); - drm_printf(p, "\tHW Context Desc: 0x%08x\n", ce->lrc.lrca); - drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n", - ce->ring->head, - ce->lrc_reg_state[CTX_RING_HEAD]); - drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n", - ce->ring->tail, - ce->lrc_reg_state[CTX_RING_TAIL]); - drm_printf(p, "\t\tContext Pin Count: %u\n", - atomic_read(&ce->pin_count)); - drm_printf(p, "\t\tGuC ID Ref Count: %u\n", - atomic_read(&ce->guc_id.ref)); - drm_printf(p, "\t\tSchedule State: 0x%x\n\n", - ce->guc_state.sched_state); + GEM_BUG_ON(intel_context_is_child(ce)); + guc_log_context(p, ce); guc_log_context_priority(p, ce); + + if (intel_context_is_parent(ce)) { + struct guc_process_desc *desc = __get_process_desc(ce); + struct intel_context *child; + + drm_printf(p, "\t\tNumber children: %u\n", + ce->parallel.number_children); + drm_printf(p, "\t\tWQI Head: %u\n", + READ_ONCE(desc->head)); + drm_printf(p, "\t\tWQI Tail: %u\n", + READ_ONCE(desc->tail)); + drm_printf(p, "\t\tWQI Status: %u\n\n", + READ_ONCE(desc->wq_status)); + + for_each_child(ce, child) + guc_log_context(p, child); + } } xa_unlock_irqrestore(&guc->context_lookup, flags); } -- 2.32.0
[PATCH 18/26] drm/i915/doc: Update parallel submit doc to point to i915_drm.h
Update parallel submit doc to point to i915_drm.h Signed-off-by: Matthew Brost Reviewed-by: John Harrison --- Documentation/gpu/rfc/i915_parallel_execbuf.h | 122 -- Documentation/gpu/rfc/i915_scheduler.rst | 4 +- 2 files changed, 2 insertions(+), 124 deletions(-) delete mode 100644 Documentation/gpu/rfc/i915_parallel_execbuf.h diff --git a/Documentation/gpu/rfc/i915_parallel_execbuf.h b/Documentation/gpu/rfc/i915_parallel_execbuf.h deleted file mode 100644 index 8cbe2c4e0172.. --- a/Documentation/gpu/rfc/i915_parallel_execbuf.h +++ /dev/null @@ -1,122 +0,0 @@ -/* SPDX-License-Identifier: MIT */ -/* - * Copyright © 2021 Intel Corporation - */ - -#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see i915_context_engines_parallel_submit */ - -/** - * struct drm_i915_context_engines_parallel_submit - Configure engine for - * parallel submission. - * - * Setup a slot in the context engine map to allow multiple BBs to be submitted - * in a single execbuf IOCTL. Those BBs will then be scheduled to run on the GPU - * in parallel. Multiple hardware contexts are created internally in the i915 - * run these BBs. Once a slot is configured for N BBs only N BBs can be - * submitted in each execbuf IOCTL and this is implicit behavior e.g. The user - * doesn't tell the execbuf IOCTL there are N BBs, the execbuf IOCTL knows how - * many BBs there are based on the slot's configuration. The N BBs are the last - * N buffer objects or first N if I915_EXEC_BATCH_FIRST is set. - * - * The default placement behavior is to create implicit bonds between each - * context if each context maps to more than 1 physical engine (e.g. context is - * a virtual engine). Also we only allow contexts of same engine class and these - * contexts must be in logically contiguous order. Examples of the placement - * behavior described below. Lastly, the default is to not allow BBs to - * preempted mid BB rather insert coordinated preemption on all hardware - * contexts between each set of BBs. Flags may be added in the future to change - * both of these default behaviors. - * - * Returns -EINVAL if hardware context placement configuration is invalid or if - * the placement configuration isn't supported on the platform / submission - * interface. - * Returns -ENODEV if extension isn't supported on the platform / submission - * interface. - * - * .. code-block:: none - * - * Example 1 pseudo code: - * CS[X] = generic engine of same class, logical instance X - * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE - * set_engines(INVALID) - * set_parallel(engine_index=0, width=2, num_siblings=1, - * engines=CS[0],CS[1]) - * - * Results in the following valid placement: - * CS[0], CS[1] - * - * Example 2 pseudo code: - * CS[X] = generic engine of same class, logical instance X - * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE - * set_engines(INVALID) - * set_parallel(engine_index=0, width=2, num_siblings=2, - * engines=CS[0],CS[2],CS[1],CS[3]) - * - * Results in the following valid placements: - * CS[0], CS[1] - * CS[2], CS[3] - * - * This can also be thought of as 2 virtual engines described by 2-D array - * in the engines the field with bonds placed between each index of the - * virtual engines. e.g. CS[0] is bonded to CS[1], CS[2] is bonded to - * CS[3]. - * VE[0] = CS[0], CS[2] - * VE[1] = CS[1], CS[3] - * - * Example 3 pseudo code: - * CS[X] = generic engine of same class, logical instance X - * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE - * set_engines(INVALID) - * set_parallel(engine_index=0, width=2, num_siblings=2, - * engines=CS[0],CS[1],CS[1],CS[3]) - * - * Results in the following valid and invalid placements: - * CS[0], CS[1] - * CS[1], CS[3] - Not logical contiguous, return -EINVAL - */ -struct drm_i915_context_engines_parallel_submit { - /** -* @base: base user extension. -*/ - struct i915_user_extension base; - - /** -* @engine_index: slot for parallel engine -*/ - __u16 engine_index; - - /** -* @width: number of contexts per parallel engine -*/ - __u16 width; - - /** -* @num_siblings: number of siblings per context -*/ - __u16 num_siblings; - - /** -* @mbz16: reserved for future use; must be zero -*/ - __u16 mbz16; - - /** -* @flags: all undefined flags must be zero, currently not defined flags -*/ - __u64 flags; - - /** -* @mbz64: reserved for future use; must be zero -*/ - __u64 mbz64[3]; - - /** -* @engines: 2-d array of engine instances to configure parallel engine -* -* length = width (i) * num_siblings (j) -* index = j + i * n
[PATCH 03/26] drm/i915/guc: Take engine PM when a context is pinned with GuC submission
Taking a PM reference to prevent intel_gt_wait_for_idle from short circuiting while a scheduling of user context could be enabled. Returning GT idle when it is not can cause all sorts of issues throughout the stack. v2: (Daniel Vetter) - Add might_lock annotations to pin / unpin function v3: (CI) - Drop intel_engine_pm_might_put from unpin path as an async put is used v4: (John Harrison) - Make intel_engine_pm_might_get/put work with GuC virtual engines - Update commit message Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_context.c | 2 ++ drivers/gpu/drm/i915/gt/intel_engine_pm.h | 32 + drivers/gpu/drm/i915/gt/intel_gt_pm.h | 10 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 36 +-- drivers/gpu/drm/i915/intel_wakeref.h | 12 +++ 5 files changed, 89 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 1076066f41e0..f601323b939f 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -240,6 +240,8 @@ int __intel_context_do_pin_ww(struct intel_context *ce, if (err) goto err_post_unpin; + intel_engine_pm_might_get(ce->engine); + if (unlikely(intel_context_is_closed(ce))) { err = -ENOENT; goto err_unlock; diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h index 6fdeae668e6e..d68675925b79 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h @@ -6,9 +6,11 @@ #ifndef INTEL_ENGINE_PM_H #define INTEL_ENGINE_PM_H +#include "i915_drv.h" #include "i915_request.h" #include "intel_engine_types.h" #include "intel_wakeref.h" +#include "intel_gt_pm.h" static inline bool intel_engine_pm_is_awake(const struct intel_engine_cs *engine) @@ -31,6 +33,21 @@ static inline bool intel_engine_pm_get_if_awake(struct intel_engine_cs *engine) return intel_wakeref_get_if_active(&engine->wakeref); } +static inline void intel_engine_pm_might_get(struct intel_engine_cs *engine) +{ + if (!intel_engine_is_virtual(engine)) { + intel_wakeref_might_get(&engine->wakeref); + } else { + struct intel_gt *gt = engine->gt; + struct intel_engine_cs *tengine; + intel_engine_mask_t tmp, mask = engine->mask; + + for_each_engine_masked(tengine, gt, mask, tmp) + intel_wakeref_might_get(&tengine->wakeref); + } + intel_gt_pm_might_get(engine->gt); +} + static inline void intel_engine_pm_put(struct intel_engine_cs *engine) { intel_wakeref_put(&engine->wakeref); @@ -52,6 +69,21 @@ static inline void intel_engine_pm_flush(struct intel_engine_cs *engine) intel_wakeref_unlock_wait(&engine->wakeref); } +static inline void intel_engine_pm_might_put(struct intel_engine_cs *engine) +{ + if (!intel_engine_is_virtual(engine)) { + intel_wakeref_might_put(&engine->wakeref); + } else { + struct intel_gt *gt = engine->gt; + struct intel_engine_cs *tengine; + intel_engine_mask_t tmp, mask = engine->mask; + + for_each_engine_masked(tengine, gt, mask, tmp) + intel_wakeref_might_put(&tengine->wakeref); + } + intel_gt_pm_might_put(engine->gt); +} + static inline struct i915_request * intel_engine_create_kernel_request(struct intel_engine_cs *engine) { diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index 05de6c1af25b..bc898df7a48c 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -31,6 +31,11 @@ static inline bool intel_gt_pm_get_if_awake(struct intel_gt *gt) return intel_wakeref_get_if_active(>->wakeref); } +static inline void intel_gt_pm_might_get(struct intel_gt *gt) +{ + intel_wakeref_might_get(>->wakeref); +} + static inline void intel_gt_pm_put(struct intel_gt *gt) { intel_wakeref_put(>->wakeref); @@ -41,6 +46,11 @@ static inline void intel_gt_pm_put_async(struct intel_gt *gt) intel_wakeref_put_async(>->wakeref); } +static inline void intel_gt_pm_might_put(struct intel_gt *gt) +{ + intel_wakeref_might_put(>->wakeref); +} + #define with_intel_gt_pm(gt, tmp) \ for (tmp = 1, intel_gt_pm_get(gt); tmp; \ intel_gt_pm_put(gt), tmp = 0) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 17da2fea1bff..8b82da50c2bc 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1571,7 +1571,12 @@ static int guc_context_pre_pin(struct intel_context *ce, static int guc_context_pin(struct intel_context *ce, void *vaddr) { -
[PATCH 04/26] drm/i915/guc: Don't call switch_to_kernel_context with GuC submission
Calling switch_to_kernel_context isn't needed if the engine PM reference is taken while all user contexts are pinned as if don't have PM ref that guarantees that all user contexts scheduling is disabled. By not calling switch_to_kernel_context we save on issuing a request to the engine. v2: (Daniel Vetter) - Add FIXME comment about pushing switch_to_kernel_context to backend v3: (John Harrison) - Update commit message - Fix workding comment Signed-off-by: Matthew Brost Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/gt/intel_engine_pm.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index dacd62773735..a1334b48dde7 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -162,6 +162,19 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine) unsigned long flags; bool result = true; + /* +* This is execlist specific behaviour intended to ensure the GPU is +* idle by switching to a known 'safe' context. With GuC submission, the +* same idle guarantee is achieved by other means (disabling +* scheduling). Further, switching to a 'safe' context has no effect +* with GuC submission as the scheduler can just switch back again. +* +* FIXME: Move this backend scheduler specific behaviour into the +* scheduler backend. +*/ + if (intel_engine_uses_guc(engine)) + return true; + /* GPU is pointing to the void, as good as in the kernel context. */ if (intel_gt_is_wedged(engine->gt)) return true; -- 2.32.0
[PATCH 12/26] drm/i915/guc: Implement multi-lrc submission
Implement multi-lrc submission via a single workqueue entry and single H2G. The workqueue entry contains an updated tail value for each request, of all the contexts in the multi-lrc submission, and updates these values simultaneously. As such, the tasklet and bypass path have been updated to coalesce requests into a single submission. v2: (John Harrison) - s/wqe/wqi - Use FIELD_PREP macros - Add GEM_BUG_ONs ensures length fits within field - Add comment / white space to intel_guc_write_barrier (Kernel test robot) - Make need_tasklet a static function Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/intel_guc.c| 26 ++ drivers/gpu/drm/i915/gt/uc/intel_guc.h| 8 + drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 24 +- drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 23 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 319 -- drivers/gpu/drm/i915/i915_request.h | 8 + 6 files changed, 335 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c index 8f8182bf7c11..7191e8439290 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c @@ -756,3 +756,29 @@ void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p) } } } + +void intel_guc_write_barrier(struct intel_guc *guc) +{ + struct intel_gt *gt = guc_to_gt(guc); + + if (i915_gem_object_is_lmem(guc->ct.vma->obj)) { + /* +* Ensure intel_uncore_write_fw can be used rather than +* intel_uncore_write. +*/ + GEM_BUG_ON(guc->send_regs.fw_domains); + + /* +* This register is used by the i915 and GuC for MMIO based +* communication. Once we are in this code CTBs are the only +* method the i915 uses to communicate with the GuC so it is +* safe to write to this register (a value of 0 is NOP for MMIO +* communication). If we ever start mixing CTBs and MMIOs a new +* register will have to be chosen. +*/ + intel_uncore_write_fw(gt->uncore, GEN11_SOFT_SCRATCH(0), 0); + } else { + /* wmb() sufficient for a barrier if in smem */ + wmb(); + } +} diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index a9f4ec972bfb..147f39cc0f2f 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -46,6 +46,12 @@ struct intel_guc { * submitted until the stalled request is processed. */ struct i915_request *stalled_request; + enum { + STALL_NONE, + STALL_REGISTER_CONTEXT, + STALL_MOVE_LRC_TAIL, + STALL_ADD_REQUEST, + } submission_stall_reason; /* intel_guc_recv interrupt related state */ /** @irq_lock: protects GuC irq state */ @@ -361,4 +367,6 @@ void intel_guc_submission_cancel_requests(struct intel_guc *guc); void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p); +void intel_guc_write_barrier(struct intel_guc *guc); + #endif diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c index 20c710a74498..10d1878d2826 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c @@ -377,28 +377,6 @@ static u32 ct_get_next_fence(struct intel_guc_ct *ct) return ++ct->requests.last_fence; } -static void write_barrier(struct intel_guc_ct *ct) -{ - struct intel_guc *guc = ct_to_guc(ct); - struct intel_gt *gt = guc_to_gt(guc); - - if (i915_gem_object_is_lmem(guc->ct.vma->obj)) { - GEM_BUG_ON(guc->send_regs.fw_domains); - /* -* This register is used by the i915 and GuC for MMIO based -* communication. Once we are in this code CTBs are the only -* method the i915 uses to communicate with the GuC so it is -* safe to write to this register (a value of 0 is NOP for MMIO -* communication). If we ever start mixing CTBs and MMIOs a new -* register will have to be chosen. -*/ - intel_uncore_write_fw(gt->uncore, GEN11_SOFT_SCRATCH(0), 0); - } else { - /* wmb() sufficient for a barrier if in smem */ - wmb(); - } -} - static int ct_write(struct intel_guc_ct *ct, const u32 *action, u32 len /* in dwords */, @@ -468,7 +446,7 @@ static int ct_write(struct intel_guc_ct *ct, * make sure H2G buffer update and LRC tail update (if this triggering a * submission) are visible before updating the descriptor tail */ - write_barrier(ct); +
[PATCH 05/26] drm/i915: Add logical engine mapping
Add logical engine mapping. This is required for split-frame, as workloads need to be placed on engines in a logically contiguous manner. v2: (Daniel Vetter) - Add kernel doc for new fields v3 (Tvrtko) - Update comment for new logical_mask field Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 60 --- drivers/gpu/drm/i915/gt/intel_engine_types.h | 7 +++ .../drm/i915/gt/intel_execlists_submission.c | 1 + drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 2 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 21 +-- 5 files changed, 62 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 2ae57e4656a3..2eb798ad068b 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -290,7 +290,8 @@ static void nop_irq_handler(struct intel_engine_cs *engine, u16 iir) GEM_DEBUG_WARN_ON(iir); } -static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id) +static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id, + u8 logical_instance) { const struct engine_info *info = &intel_engines[id]; struct drm_i915_private *i915 = gt->i915; @@ -335,6 +336,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id) engine->class = info->class; engine->instance = info->instance; + engine->logical_mask = BIT(logical_instance); __sprint_engine_name(engine); engine->props.heartbeat_interval_ms = @@ -588,6 +590,37 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt) return info->engine_mask; } +static void populate_logical_ids(struct intel_gt *gt, u8 *logical_ids, +u8 class, const u8 *map, u8 num_instances) +{ + int i, j; + u8 current_logical_id = 0; + + for (j = 0; j < num_instances; ++j) { + for (i = 0; i < ARRAY_SIZE(intel_engines); ++i) { + if (!HAS_ENGINE(gt, i) || + intel_engines[i].class != class) + continue; + + if (intel_engines[i].instance == map[j]) { + logical_ids[intel_engines[i].instance] = + current_logical_id++; + break; + } + } + } +} + +static void setup_logical_ids(struct intel_gt *gt, u8 *logical_ids, u8 class) +{ + int i; + u8 map[MAX_ENGINE_INSTANCE + 1]; + + for (i = 0; i < MAX_ENGINE_INSTANCE + 1; ++i) + map[i] = i; + populate_logical_ids(gt, logical_ids, class, map, ARRAY_SIZE(map)); +} + /** * intel_engines_init_mmio() - allocate and prepare the Engine Command Streamers * @gt: pointer to struct intel_gt @@ -599,7 +632,8 @@ int intel_engines_init_mmio(struct intel_gt *gt) struct drm_i915_private *i915 = gt->i915; const unsigned int engine_mask = init_engine_mask(gt); unsigned int mask = 0; - unsigned int i; + unsigned int i, class; + u8 logical_ids[MAX_ENGINE_INSTANCE + 1]; int err; drm_WARN_ON(&i915->drm, engine_mask == 0); @@ -609,15 +643,23 @@ int intel_engines_init_mmio(struct intel_gt *gt) if (i915_inject_probe_failure(i915)) return -ENODEV; - for (i = 0; i < ARRAY_SIZE(intel_engines); i++) { - if (!HAS_ENGINE(gt, i)) - continue; + for (class = 0; class < MAX_ENGINE_CLASS + 1; ++class) { + setup_logical_ids(gt, logical_ids, class); - err = intel_engine_setup(gt, i); - if (err) - goto cleanup; + for (i = 0; i < ARRAY_SIZE(intel_engines); ++i) { + u8 instance = intel_engines[i].instance; + + if (intel_engines[i].class != class || + !HAS_ENGINE(gt, i)) + continue; - mask |= BIT(i); + err = intel_engine_setup(gt, i, +logical_ids[instance]); + if (err) + goto cleanup; + + mask |= BIT(i); + } } /* diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 5ae1207c363b..68010da468a4 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -269,6 +269,13 @@ struct intel_engine_cs { unsigned int guc_id; intel_engine_mask_t mask; + /** +* @logical_mask: logical mask of engine, reported to user space via +* query IOCTL and used to communicate with the GuC in logic
[PATCH 11/26] drm/i915/guc: Implement parallel context pin / unpin functions
Parallel contexts are perma-pinned by the upper layers which makes the backend implementation rather simple. The parent pins the guc_id and children increment the parent's pin count on pin to ensure all the contexts are unpinned before we disable scheduling with the GuC / or deregister the context. v2: (Daniel Vetter) - Perma-pin parallel contexts Signed-off-by: Matthew Brost --- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 70 +++ 1 file changed, 70 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 79e7732e83b2..031b1bf5ba91 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -2583,6 +2583,76 @@ static const struct intel_context_ops virtual_guc_context_ops = { .get_sibling = guc_virtual_get_sibling, }; +/* Future patches will use this function */ +__maybe_unused +static int guc_parent_context_pin(struct intel_context *ce, void *vaddr) +{ + struct intel_engine_cs *engine = guc_virtual_get_sibling(ce->engine, 0); + struct intel_guc *guc = ce_to_guc(ce); + int ret; + + GEM_BUG_ON(!intel_context_is_parent(ce)); + GEM_BUG_ON(!intel_engine_is_virtual(ce->engine)); + + ret = pin_guc_id(guc, ce); + if (unlikely(ret < 0)) + return ret; + + return __guc_context_pin(ce, engine, vaddr); +} + +/* Future patches will use this function */ +__maybe_unused +static int guc_child_context_pin(struct intel_context *ce, void *vaddr) +{ + struct intel_engine_cs *engine = guc_virtual_get_sibling(ce->engine, 0); + + GEM_BUG_ON(!intel_context_is_child(ce)); + GEM_BUG_ON(!intel_engine_is_virtual(ce->engine)); + + __intel_context_pin(ce->parallel.parent); + return __guc_context_pin(ce, engine, vaddr); +} + +/* Future patches will use this function */ +__maybe_unused +static void guc_parent_context_unpin(struct intel_context *ce) +{ + struct intel_guc *guc = ce_to_guc(ce); + + GEM_BUG_ON(context_enabled(ce)); + GEM_BUG_ON(intel_context_is_barrier(ce)); + GEM_BUG_ON(!intel_context_is_parent(ce)); + GEM_BUG_ON(!intel_engine_is_virtual(ce->engine)); + + unpin_guc_id(guc, ce); + lrc_unpin(ce); +} + +/* Future patches will use this function */ +__maybe_unused +static void guc_child_context_unpin(struct intel_context *ce) +{ + GEM_BUG_ON(context_enabled(ce)); + GEM_BUG_ON(intel_context_is_barrier(ce)); + GEM_BUG_ON(!intel_context_is_child(ce)); + GEM_BUG_ON(!intel_engine_is_virtual(ce->engine)); + + lrc_unpin(ce); +} + +/* Future patches will use this function */ +__maybe_unused +static void guc_child_context_post_unpin(struct intel_context *ce) +{ + GEM_BUG_ON(!intel_context_is_child(ce)); + GEM_BUG_ON(!intel_context_is_pinned(ce->parallel.parent)); + GEM_BUG_ON(!intel_engine_is_virtual(ce->engine)); + + lrc_post_unpin(ce); + intel_context_unpin(ce->parallel.parent); +} + static bool guc_irq_enable_breadcrumbs(struct intel_breadcrumbs *b) { -- 2.32.0
[PATCH 10/26] drm/i915/guc: Assign contexts in parent-child relationship consecutive guc_ids
Assign contexts in parent-child relationship consecutive guc_ids. This is accomplished by partitioning guc_id space between ones that need to be consecutive (1/16 available guc_ids) and ones that do not (15/16 of available guc_ids). The consecutive search is implemented via the bitmap API. This is a precursor to the full GuC multi-lrc implementation but aligns to how GuC mutli-lrc interface is defined - guc_ids must be consecutive when using the GuC multi-lrc interface. v2: (Daniel Vetter) - Explicitly state why we assign consecutive guc_ids v3: (John Harrison) - Bring back in spin lock Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 6 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 104 ++ 2 files changed, 86 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 25a598e2b6e8..a9f4ec972bfb 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -76,9 +76,13 @@ struct intel_guc { */ spinlock_t lock; /** -* @guc_ids: used to allocate new guc_ids +* @guc_ids: used to allocate new guc_ids, single-lrc */ struct ida guc_ids; + /** +* @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc +*/ + unsigned long *guc_ids_bitmap; /** * @guc_id_list: list of intel_context with valid guc_ids but no * refs diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 1f2809187513..79e7732e83b2 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -128,6 +128,16 @@ guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count); #define GUC_REQUEST_SIZE 64 /* bytes */ +/* + * We reserve 1/16 of the guc_ids for multi-lrc as these need to be contiguous + * per the GuC submission interface. A different allocation algorithm is used + * (bitmap vs. ida) between multi-lrc and single-lrc hence the reason to + * partition the guc_id space. We believe the number of multi-lrc contexts in + * use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids for + * multi-lrc. + */ +#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) + /* * Below is a set of functions which control the GuC scheduling state which * require a lock. @@ -1206,6 +1216,11 @@ int intel_guc_submission_init(struct intel_guc *guc) INIT_WORK(&guc->submission_state.destroyed_worker, destroyed_worker_func); + guc->submission_state.guc_ids_bitmap = + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); + if (!guc->submission_state.guc_ids_bitmap) + return -ENOMEM; + return 0; } @@ -1217,6 +1232,7 @@ void intel_guc_submission_fini(struct intel_guc *guc) guc_lrc_desc_pool_destroy(guc); guc_flush_destroyed_contexts(guc); i915_sched_engine_put(guc->sched_engine); + bitmap_free(guc->submission_state.guc_ids_bitmap); } static inline void queue_request(struct i915_sched_engine *sched_engine, @@ -1268,18 +1284,43 @@ static void guc_submit_request(struct i915_request *rq) spin_unlock_irqrestore(&sched_engine->lock, flags); } -static int new_guc_id(struct intel_guc *guc) +static int new_guc_id(struct intel_guc *guc, struct intel_context *ce) { - return ida_simple_get(&guc->submission_state.guc_ids, 0, - GUC_MAX_LRC_DESCRIPTORS, GFP_KERNEL | - __GFP_RETRY_MAYFAIL | __GFP_NOWARN); + int ret; + + GEM_BUG_ON(intel_context_is_child(ce)); + + if (intel_context_is_parent(ce)) + ret = bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, + NUMBER_MULTI_LRC_GUC_ID, + order_base_2(ce->parallel.number_children + + 1)); + else + ret = ida_simple_get(&guc->submission_state.guc_ids, +NUMBER_MULTI_LRC_GUC_ID, +GUC_MAX_LRC_DESCRIPTORS, +GFP_KERNEL | __GFP_RETRY_MAYFAIL | +__GFP_NOWARN); + if (unlikely(ret < 0)) + return ret; + + ce->guc_id.id = ret; + return 0; } static void __release_guc_id(struct intel_guc *guc, struct intel_context *ce) { + GEM_BUG_ON(intel_context_is_child(ce)); + if (!context_guc_id_invalid(ce)) { - ida_simple_remove(&guc->submission_state.guc_ids, - ce->guc_id.id
[PATCH 07/26] drm/i915/guc: Introduce context parent-child relationship
Introduce context parent-child relationship. Once this relationship is created all pinning / unpinning operations are directed to the parent context. The parent context is responsible for pinning all of its' children and itself. This is a precursor to the full GuC multi-lrc implementation but aligns to how GuC mutli-lrc interface is defined - a single H2G is used register / deregister all of the contexts simultaneously. Subsequent patches in the series will implement the pinning / unpinning operations for parent / child contexts. v2: (Daniel Vetter) - Add kernel doc, add wrapper to access parent to ensure safety v3: (John Harrison) - Fix comment explaing GEM_BUG_ON in to_parent() - Make variable names generic (non-GuC specific) Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_context.c | 29 + drivers/gpu/drm/i915/gt/intel_context.h | 41 +++ drivers/gpu/drm/i915/gt/intel_context_types.h | 21 ++ 3 files changed, 91 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index f601323b939f..c5bb7ccfb3f8 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -403,6 +403,8 @@ intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine) INIT_LIST_HEAD(&ce->destroyed_link); + INIT_LIST_HEAD(&ce->parallel.child_list); + /* * Initialize fence to be complete as this is expected to be complete * unless there is a pending schedule disable outstanding. @@ -417,10 +419,17 @@ intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine) void intel_context_fini(struct intel_context *ce) { + struct intel_context *child, *next; + if (ce->timeline) intel_timeline_put(ce->timeline); i915_vm_put(ce->vm); + /* Need to put the creation ref for the children */ + if (intel_context_is_parent(ce)) + for_each_child_safe(ce, child, next) + intel_context_put(child); + mutex_destroy(&ce->pin_mutex); i915_active_fini(&ce->active); i915_sw_fence_fini(&ce->guc_state.blocked); @@ -537,6 +546,26 @@ struct i915_request *intel_context_find_active_request(struct intel_context *ce) return active; } +void intel_context_bind_parent_child(struct intel_context *parent, +struct intel_context *child) +{ + /* +* Callers responsibility to validate that this function is used +* correctly but we use GEM_BUG_ON here ensure that they do. +*/ + GEM_BUG_ON(!intel_engine_uses_guc(parent->engine)); + GEM_BUG_ON(intel_context_is_pinned(parent)); + GEM_BUG_ON(intel_context_is_child(parent)); + GEM_BUG_ON(intel_context_is_pinned(child)); + GEM_BUG_ON(intel_context_is_child(child)); + GEM_BUG_ON(intel_context_is_parent(child)); + + parent->parallel.number_children++; + list_add_tail(&child->parallel.child_link, + &parent->parallel.child_list); + child->parallel.parent = parent; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftest_context.c" #endif diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h index c41098950746..b63c10a144af 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.h +++ b/drivers/gpu/drm/i915/gt/intel_context.h @@ -44,6 +44,47 @@ void intel_context_free(struct intel_context *ce); int intel_context_reconfigure_sseu(struct intel_context *ce, const struct intel_sseu sseu); +static inline bool intel_context_is_child(struct intel_context *ce) +{ + return !!ce->parallel.parent; +} + +static inline bool intel_context_is_parent(struct intel_context *ce) +{ + return !!ce->parallel.number_children; +} + +static inline bool intel_context_is_pinned(struct intel_context *ce); + +static inline struct intel_context * +intel_context_to_parent(struct intel_context *ce) +{ + if (intel_context_is_child(ce)) { + /* +* The parent holds ref count to the child so it is always safe +* for the parent to access the child, but the child has a +* pointer to the parent without a ref. To ensure this is safe +* the child should only access the parent pointer while the +* parent is pinned. +*/ + GEM_BUG_ON(!intel_context_is_pinned(ce->parallel.parent)); + + return ce->parallel.parent; + } else { + return ce; + } +} + +void intel_context_bind_parent_child(struct intel_context *parent, +struct intel_context *child); + +#define for_each_child(parent, ce)\ + list_for_each_entry(ce, &(parent)->parallel.child_list,\ + parallel.ch
[PATCH 17/26] drm/i915/guc: Connect UAPI to GuC multi-lrc interface
Introduce 'set parallel submit' extension to connect UAPI to GuC multi-lrc interface. Kernel doc in new uAPI should explain it all. IGT: https://patchwork.freedesktop.org/patch/447008/?series=93071&rev=1 media UMD: https://github.com/intel/media-driver/pull/1252 v2: (Daniel Vetter) - Add IGT link and placeholder for media UMD link v3: (Kernel test robot) - Fix warning in unpin engines call (John Harrison) - Reword a bunch of the kernel doc Cc: Tvrtko Ursulin Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 221 +- .../gpu/drm/i915/gem/i915_gem_context_types.h | 6 + drivers/gpu/drm/i915/gt/intel_context_types.h | 9 +- drivers/gpu/drm/i915/gt/intel_engine.h| 12 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 +- .../drm/i915/gt/intel_execlists_submission.c | 6 +- drivers/gpu/drm/i915/gt/selftest_execlists.c | 12 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 114 - include/uapi/drm/i915_drm.h | 131 +++ 9 files changed, 489 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 8c7ea6e56262..6290bc20ccb1 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -522,9 +522,150 @@ set_proto_ctx_engines_bond(struct i915_user_extension __user *base, void *data) return 0; } +static int +set_proto_ctx_engines_parallel_submit(struct i915_user_extension __user *base, + void *data) +{ + struct i915_context_engines_parallel_submit __user *ext = + container_of_user(base, typeof(*ext), base); + const struct set_proto_ctx_engines *set = data; + struct drm_i915_private *i915 = set->i915; + u64 flags; + int err = 0, n, i, j; + u16 slot, width, num_siblings; + struct intel_engine_cs **siblings = NULL; + intel_engine_mask_t prev_mask; + + /* Disabling for now */ + return -ENODEV; + + /* FIXME: This is NIY for execlists */ + if (!(intel_uc_uses_guc_submission(&i915->gt.uc))) + return -ENODEV; + + if (get_user(slot, &ext->engine_index)) + return -EFAULT; + + if (get_user(width, &ext->width)) + return -EFAULT; + + if (get_user(num_siblings, &ext->num_siblings)) + return -EFAULT; + + if (slot >= set->num_engines) { + drm_dbg(&i915->drm, "Invalid placement value, %d >= %d\n", + slot, set->num_engines); + return -EINVAL; + } + + if (set->engines[slot].type != I915_GEM_ENGINE_TYPE_INVALID) { + drm_dbg(&i915->drm, + "Invalid placement[%d], already occupied\n", slot); + return -EINVAL; + } + + if (get_user(flags, &ext->flags)) + return -EFAULT; + + if (flags) { + drm_dbg(&i915->drm, "Unknown flags 0x%02llx", flags); + return -EINVAL; + } + + for (n = 0; n < ARRAY_SIZE(ext->mbz64); n++) { + err = check_user_mbz(&ext->mbz64[n]); + if (err) + return err; + } + + if (width < 2) { + drm_dbg(&i915->drm, "Width (%d) < 2\n", width); + return -EINVAL; + } + + if (num_siblings < 1) { + drm_dbg(&i915->drm, "Number siblings (%d) < 1\n", + num_siblings); + return -EINVAL; + } + + siblings = kmalloc_array(num_siblings * width, +sizeof(*siblings), +GFP_KERNEL); + if (!siblings) + return -ENOMEM; + + /* Create contexts / engines */ + for (i = 0; i < width; ++i) { + intel_engine_mask_t current_mask = 0; + struct i915_engine_class_instance prev_engine; + + for (j = 0; j < num_siblings; ++j) { + struct i915_engine_class_instance ci; + + n = i * num_siblings + j; + if (copy_from_user(&ci, &ext->engines[n], sizeof(ci))) { + err = -EFAULT; + goto out_err; + } + + siblings[n] = + intel_engine_lookup_user(i915, ci.engine_class, +ci.engine_instance); + if (!siblings[n]) { + drm_dbg(&i915->drm, + "Invalid sibling[%d]: { class:%d, inst:%d }\n", + n, ci.engine_class, ci.engine_instance); + err = -EINVAL; + goto out_err; + } + + i
[PATCH 06/26] drm/i915: Expose logical engine instance to user
Expose logical engine instance to user via query engine info IOCTL. This is required for split-frame workloads as these needs to be placed on engines in a logically contiguous order. The logical mapping can change based on fusing. Rather than having user have knowledge of the fusing we simply just expose the logical mapping with the existing query engine info IOCTL. IGT: https://patchwork.freedesktop.org/patch/445637/?series=92854&rev=1 media UMD: https://github.com/intel/media-driver/pull/1252 v2: (Daniel Vetter) - Add IGT link, placeholder for media UMD Cc: Tvrtko Ursulin Signed-off-by: Matthew Brost Reviewed-by: John Harrison --- drivers/gpu/drm/i915/i915_query.c | 2 ++ include/uapi/drm/i915_drm.h | 8 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c index 5e2b909827f4..51b368be0fc4 100644 --- a/drivers/gpu/drm/i915/i915_query.c +++ b/drivers/gpu/drm/i915/i915_query.c @@ -124,7 +124,9 @@ query_engine_info(struct drm_i915_private *i915, for_each_uabi_engine(engine, i915) { info.engine.engine_class = engine->uabi_class; info.engine.engine_instance = engine->uabi_instance; + info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE; info.capabilities = engine->uabi_capabilities; + info.logical_instance = ilog2(engine->logical_mask); if (copy_to_user(info_ptr, &info, sizeof(info))) return -EFAULT; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index bde5860b3686..b1248a67b4f8 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -2726,14 +2726,20 @@ struct drm_i915_engine_info { /** @flags: Engine flags. */ __u64 flags; +#define I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE (1 << 0) /** @capabilities: Capabilities of this engine. */ __u64 capabilities; #define I915_VIDEO_CLASS_CAPABILITY_HEVC (1 << 0) #define I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC(1 << 1) + /** @logical_instance: Logical instance of engine */ + __u16 logical_instance; + /** @rsvd1: Reserved fields. */ - __u64 rsvd1[4]; + __u16 rsvd1[3]; + /** @rsvd2: Reserved fields. */ + __u64 rsvd2[3]; }; /** -- 2.32.0
[PATCH 08/26] drm/i915/guc: Add multi-lrc context registration
Add multi-lrc context registration H2G. In addition a workqueue and process descriptor are setup during multi-lrc context registration as these data structures are needed for multi-lrc submission. v2: (John Harrison) - Move GuC specific fields into sub-struct - Clean up WQ defines - Add comment explaining math to derive WQ / PD address Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_context_types.h | 12 ++ drivers/gpu/drm/i915/gt/intel_lrc.c | 5 + .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 1 + drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 2 - .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 114 +- 5 files changed, 131 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 76dfca57cb45..48decb5ee954 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -239,6 +239,18 @@ struct intel_context { struct intel_context *parent; /** @number_children: number of children if parent */ u8 number_children; + /** @guc: GuC specific members for parallel submission */ + struct { + /** @wqi_head: head pointer in work queue */ + u16 wqi_head; + /** @wqi_tail: tail pointer in work queue */ + u16 wqi_tail; + /** +* @parent_page: page in context state (ce->state) used +* by parent for work queue, process descriptor +*/ + u8 parent_page; + } guc; } parallel; #ifdef CONFIG_DRM_I915_SELFTEST diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 3ef9eaf8c50e..57339d5c1fc8 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -942,6 +942,11 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine) context_size += PAGE_SIZE; } + if (intel_context_is_parent(ce) && intel_engine_uses_guc(engine)) { + ce->parallel.guc.parent_page = context_size / PAGE_SIZE; + context_size += PAGE_SIZE; + } + obj = i915_gem_object_create_lmem(engine->i915, context_size, I915_BO_ALLOC_PM_VOLATILE); if (IS_ERR(obj)) diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h index 8ff58aff..ba10bd374cee 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h @@ -142,6 +142,7 @@ enum intel_guc_action { INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505, INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER = 0x4506, INTEL_GUC_ACTION_DEREGISTER_CONTEXT_DONE = 0x4600, + INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601, INTEL_GUC_ACTION_RESET_CLIENT = 0x5507, INTEL_GUC_ACTION_LIMIT }; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h index fa4be13c8854..0eeb2a9feeed 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h @@ -52,8 +52,6 @@ #define GUC_DOORBELL_INVALID 256 -#define GUC_WQ_SIZE(PAGE_SIZE * 2) - /* Work queue item header definitions */ #define WQ_STATUS_ACTIVE 1 #define WQ_STATUS_SUSPENDED2 diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 451d9ae861a6..ab6d7fc1b0b1 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -344,6 +344,45 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb) return rb_entry(rb, struct i915_priolist, node); } +/* + * When using multi-lrc submission an extra page in the context state is + * reserved for the process descriptor and work queue. + * + * The layout of this page is below: + * 0 guc_process_desc + * ... unused + * PAGE_SIZE / 2 work queue start + * ... work queue + * PAGE_SIZE - 1 work queue end + */ +#define WQ_SIZE(PAGE_SIZE / 2) +#define WQ_OFFSET (PAGE_SIZE - WQ_SIZE) +static u32 __get_process_desc_offset(struct intel_context *ce) +{ + GEM_BUG_ON(!ce->parallel.guc.parent_page); + + return ce->parallel.guc.parent_page * PAGE_SIZE; +} + +static u32 __get_wq_offset(struct intel_context *ce) +{ + return __get_process_desc_offset(ce) + W
[PATCH 09/26] drm/i915/guc: Ensure GuC schedule operations do not operate on child contexts
In GuC parent-child contexts the parent context controls the scheduling, ensure only the parent does the scheduling operations. Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index ab6d7fc1b0b1..1f2809187513 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -324,6 +324,12 @@ static inline void decr_context_committed_requests(struct intel_context *ce) GEM_BUG_ON(ce->guc_state.number_committed_requests < 0); } +static struct intel_context * +request_to_scheduling_context(struct i915_request *rq) +{ + return intel_context_to_parent(rq->context); +} + static inline bool context_guc_id_invalid(struct intel_context *ce) { return ce->guc_id.id == GUC_INVALID_LRC_ID; @@ -1710,6 +1716,7 @@ static void __guc_context_sched_disable(struct intel_guc *guc, GEM_BUG_ON(guc_id == GUC_INVALID_LRC_ID); + GEM_BUG_ON(intel_context_is_child(ce)); trace_intel_context_sched_disable(ce); guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), @@ -1935,6 +1942,8 @@ static void guc_context_sched_disable(struct intel_context *ce) intel_wakeref_t wakeref; u16 guc_id; + GEM_BUG_ON(intel_context_is_child(ce)); + spin_lock_irqsave(&ce->guc_state.lock, flags); /* @@ -2303,6 +2312,8 @@ static void guc_signal_context_fence(struct intel_context *ce) { unsigned long flags; + GEM_BUG_ON(intel_context_is_child(ce)); + spin_lock_irqsave(&ce->guc_state.lock, flags); clr_context_wait_for_deregister_to_register(ce); __guc_signal_context_fence(ce); @@ -2333,7 +2344,7 @@ static void guc_context_init(struct intel_context *ce) static int guc_request_alloc(struct i915_request *rq) { - struct intel_context *ce = rq->context; + struct intel_context *ce = request_to_scheduling_context(rq); struct intel_guc *guc = ce_to_guc(ce); unsigned long flags; int ret; -- 2.32.0
[PATCH 01/26] drm/i915/guc: Move GuC guc_id allocation under submission state sub-struct
Move guc_id allocation under submission state sub-struct as a future patch will reuse the spin lock as a global submission state lock. Moving this into sub-struct makes ownership of fields / lock clear. Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_context_types.h | 6 +- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 26 + .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 56 ++- 3 files changed, 47 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 12252c411159..e7e3984aab78 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -197,18 +197,18 @@ struct intel_context { struct { /** * @id: handle which is used to uniquely identify this context -* with the GuC, protected by guc->contexts_lock +* with the GuC, protected by guc->submission_state.lock */ u16 id; /** * @ref: the number of references to the guc_id, when * transitioning in and out of zero protected by -* guc->contexts_lock +* guc->submission_state.lock */ atomic_t ref; /** * @link: in guc->guc_id_list when the guc_id has no refs but is -* still valid, protected by guc->contexts_lock +* still valid, protected by guc->submission_state.lock */ struct list_head link; } guc_id; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 5dd174babf7a..65b5e8eeef96 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -70,17 +70,21 @@ struct intel_guc { void (*disable)(struct intel_guc *guc); } interrupts; - /** -* @contexts_lock: protects guc_ids, guc_id_list, ce->guc_id.id, and -* ce->guc_id.ref when transitioning in and out of zero -*/ - spinlock_t contexts_lock; - /** @guc_ids: used to allocate unique ce->guc_id.id values */ - struct ida guc_ids; - /** -* @guc_id_list: list of intel_context with valid guc_ids but no refs -*/ - struct list_head guc_id_list; + struct { + /** +* @lock: protects everything in submission_state +*/ + spinlock_t lock; + /** +* @guc_ids: used to allocate new guc_ids +*/ + struct ida guc_ids; + /** +* @guc_id_list: list of intel_context with valid guc_ids but no +* refs +*/ + struct list_head guc_id_list; + } submission_state; /** * @submission_supported: tracks whether we support GuC submission on diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index ba0de35f6323..ad5c18119d92 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -68,16 +68,16 @@ * fence is used to stall all requests associated with this guc_id until the * corresponding G2H returns indicating the guc_id has been deregistered. * - * guc_ids: + * submission_state.guc_ids: * Unique number associated with private GuC context data passed in during * context registration / submission / deregistration. 64k available. Simple ida * is used for allocation. * - * Stealing guc_ids: - * If no guc_ids are available they can be stolen from another context at - * request creation time if that context is unpinned. If a guc_id can't be found - * we punt this problem to the user as we believe this is near impossible to hit - * during normal use cases. + * Stealing submission_state.guc_ids: + * If no submission_state.guc_ids are available they can be stolen from another + * context at request creation time if that context is unpinned. If a guc_id + * can't be found we punt this problem to the user as we believe this is near + * impossible to hit during normal use cases. * * Locking: * In the GuC submission code we have 3 basic spin locks which protect @@ -89,7 +89,7 @@ * sched_engine can be submitting at a time. Currently only one sched_engine is * used for all of GuC submission but that could change in the future. * - * guc->contexts_lock + * guc->submission_state.lock * Protects guc_id allocation for the given GuC, i.e. only one context can be * doing guc_id allocation operations at a time for each GuC in the system. * @@ -103,7 +103,7 @@ * * Lock ordering rules: * sched_engine->lock -> ce->guc_state.lock - * guc->contexts_lock -> ce->guc_state.lock + * guc->submission_state.lo
[PATCH 14/26] drm/i915/guc: Implement multi-lrc reset
Update context and full GPU reset to work with multi-lrc. The idea is parent context tracks all the active requests inflight for itself and its' children. The parent context owns the reset replaying / canceling requests as needed. v2: (John Harrison) - Simply loop in find active request - Add comments to find ative request / reset loop Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_context.c | 15 +++- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 69 ++- 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index c5bb7ccfb3f8..3b340eb59ada 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -528,20 +528,29 @@ struct i915_request *intel_context_create_request(struct intel_context *ce) struct i915_request *intel_context_find_active_request(struct intel_context *ce) { + struct intel_context *parent = intel_context_to_parent(ce); struct i915_request *rq, *active = NULL; unsigned long flags; GEM_BUG_ON(!intel_engine_uses_guc(ce->engine)); - spin_lock_irqsave(&ce->guc_state.lock, flags); - list_for_each_entry_reverse(rq, &ce->guc_state.requests, + /* +* We search the parent list to find an active request on the submitted +* context. The parent list contains the requests for all the contexts +* in the relationship so we have to do a compare of each request's +* context must be done. +*/ + spin_lock_irqsave(&parent->guc_state.lock, flags); + list_for_each_entry_reverse(rq, &parent->guc_state.requests, sched.link) { + if (rq->context != ce) + continue; if (i915_request_completed(rq)) break; active = rq; } - spin_unlock_irqrestore(&ce->guc_state.lock, flags); + spin_unlock_irqrestore(&parent->guc_state.lock, flags); return active; } diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 6be7adf89e4f..d661a69ef4f7 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -681,6 +681,11 @@ static inline int rq_prio(const struct i915_request *rq) return rq->sched.attr.priority; } +static inline bool is_multi_lrc(struct intel_context *ce) +{ + return intel_context_is_parallel(ce); +} + static bool is_multi_lrc_rq(struct i915_request *rq) { return intel_context_is_parallel(rq->context); @@ -1214,10 +1219,15 @@ __unwind_incomplete_requests(struct intel_context *ce) static void __guc_reset_context(struct intel_context *ce, bool stalled) { + bool local_stalled; struct i915_request *rq; unsigned long flags; u32 head; + int i, number_children = ce->parallel.number_children; bool skip = false; + struct intel_context *parent = ce; + + GEM_BUG_ON(intel_context_is_child(ce)); intel_context_get(ce); @@ -1243,25 +1253,38 @@ static void __guc_reset_context(struct intel_context *ce, bool stalled) if (unlikely(skip)) goto out_put; - rq = intel_context_find_active_request(ce); - if (!rq) { - head = ce->ring->tail; - stalled = false; - goto out_replay; - } + /* +* For each context in the relationship find the hanging request +* resetting each context / request as needed +*/ + for (i = 0; i < number_children + 1; ++i) { + if (!intel_context_is_pinned(ce)) + goto next_context; + + local_stalled = false; + rq = intel_context_find_active_request(ce); + if (!rq) { + head = ce->ring->tail; + goto out_replay; + } - if (!i915_request_started(rq)) - stalled = false; + GEM_BUG_ON(i915_active_is_idle(&ce->active)); + head = intel_ring_wrap(ce->ring, rq->head); - GEM_BUG_ON(i915_active_is_idle(&ce->active)); - head = intel_ring_wrap(ce->ring, rq->head); - __i915_request_reset(rq, stalled); + if (i915_request_started(rq)) + local_stalled = true; + __i915_request_reset(rq, local_stalled && stalled); out_replay: - guc_reset_state(ce, head, stalled); - __unwind_incomplete_requests(ce); + guc_reset_state(ce, head, local_stalled && stalled); +next_context: + if (i != number_children) + ce = list_next_entry(ce, parallel.child_link); + } + + __unwind_incomplete_requests(parent); out_put: - intel_context_put(ce); +
[PATCH 13/26] drm/i915/guc: Insert submit fences between requests in parent-child relationship
The GuC must receive requests in the order submitted for contexts in a parent-child relationship to function correctly. To ensure this, insert a submit fence between the current request and last request submitted for requests / contexts in a parent child relationship. This is conceptually similar to a single timeline. Signed-off-by: Matthew Brost Cc: John Harrison Reviewed-by: John Harrison --- drivers/gpu/drm/i915/gt/intel_context.h | 5 + drivers/gpu/drm/i915/gt/intel_context_types.h | 6 + .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 5 +- drivers/gpu/drm/i915/i915_request.c | 120 ++ 4 files changed, 108 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h index b63c10a144af..1bc705f98e2a 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.h +++ b/drivers/gpu/drm/i915/gt/intel_context.h @@ -75,6 +75,11 @@ intel_context_to_parent(struct intel_context *ce) } } +static inline bool intel_context_is_parallel(struct intel_context *ce) +{ + return intel_context_is_child(ce) || intel_context_is_parent(ce); +} + void intel_context_bind_parent_child(struct intel_context *parent, struct intel_context *child); diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 48decb5ee954..8309d1141d0a 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -237,6 +237,12 @@ struct intel_context { }; /** @parent: pointer to parent if child */ struct intel_context *parent; + /** +* @last_rq: last request submitted on a parallel context, used +* to insert submit fences between requests in the parallel +* context +*/ + struct i915_request *last_rq; /** @number_children: number of children if parent */ u8 number_children; /** @guc: GuC specific members for parallel submission */ diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 1610120e31a1..6be7adf89e4f 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -683,8 +683,7 @@ static inline int rq_prio(const struct i915_request *rq) static bool is_multi_lrc_rq(struct i915_request *rq) { - return intel_context_is_child(rq->context) || - intel_context_is_parent(rq->context); + return intel_context_is_parallel(rq->context); } static bool can_merge_rq(struct i915_request *rq, @@ -2870,6 +2869,8 @@ static void guc_parent_context_unpin(struct intel_context *ce) GEM_BUG_ON(!intel_context_is_parent(ce)); GEM_BUG_ON(!intel_engine_is_virtual(ce->engine)); + if (ce->parallel.last_rq) + i915_request_put(ce->parallel.last_rq); unpin_guc_id(guc, ce); lrc_unpin(ce); } diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 79da5eca60af..e9bfa32f9270 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1539,36 +1539,62 @@ i915_request_await_object(struct i915_request *to, return ret; } +static inline bool is_parallel_rq(struct i915_request *rq) +{ + return intel_context_is_parallel(rq->context); +} + +static inline struct intel_context *request_to_parent(struct i915_request *rq) +{ + return intel_context_to_parent(rq->context); +} + static struct i915_request * -__i915_request_add_to_timeline(struct i915_request *rq) +__i915_request_ensure_parallel_ordering(struct i915_request *rq, + struct intel_timeline *timeline) { - struct intel_timeline *timeline = i915_request_timeline(rq); struct i915_request *prev; - /* -* Dependency tracking and request ordering along the timeline -* is special cased so that we can eliminate redundant ordering -* operations while building the request (we know that the timeline -* itself is ordered, and here we guarantee it). -* -* As we know we will need to emit tracking along the timeline, -* we embed the hooks into our request struct -- at the cost of -* having to have specialised no-allocation interfaces (which will -* be beneficial elsewhere). -* -* A second benefit to open-coding i915_request_await_request is -* that we can apply a slight variant of the rules specialised -* for timelines that jump between engines (such as virtual engines). -* If we consider the case of virtual engine, we must emit a dma-fence -* to prevent scheduling of the second request until the first is -* compl
[PATCH 16/26] drm/i915: Fix bug in user proto-context creation that leaked contexts
Set number of engines before attempting to create contexts so the function free_engines can clean up properly. Also check return of alloc_engines for NULL. v2: (Tvrtko) - Send as stand alone patch (John Harrison) - Check for alloc_engines returning NULL v3: (Checkpatch / Tvrtko) - Remove braces around single line if statement Cc: Jason Ekstrand Fixes: d4433c7600f7 ("drm/i915/gem: Use the proto-context to handle create parameters (v5)") Reviewed-by: Tvrtko Ursulin Signed-off-by: Matthew Brost Cc: --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 8208fd5b72c3..8c7ea6e56262 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -898,6 +898,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, unsigned int n; e = alloc_engines(num_engines); + if (!e) + return ERR_PTR(-ENOMEM); + e->num_engines = num_engines; + for (n = 0; n < num_engines; n++) { struct intel_context *ce; int ret; @@ -931,7 +935,6 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, goto free_engines; } } - e->num_engines = num_engines; return e; -- 2.32.0
[PATCH 02/26] drm/i915/guc: Take GT PM ref when deregistering context
Taking a PM reference to prevent intel_gt_wait_for_idle from short circuiting while a deregister context H2G is in flight. To do this must issue the deregister H2G from a worker as context can be destroyed from an atomic context and taking GT PM ref blows up. Previously we took a runtime PM from this atomic context which worked but will stop working once runtime pm autosuspend in enabled. So this patch is two fold, stop intel_gt_wait_for_idle from short circuting and fix runtime pm autosuspend. v2: (John Harrison) - Split structure changes out in different patch (Tvrtko) - Don't drop lock in deregister_destroyed_contexts Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_context.c | 2 + drivers/gpu/drm/i915/gt/intel_context_types.h | 7 + drivers/gpu/drm/i915/gt/intel_engine_pm.h | 5 + drivers/gpu/drm/i915/gt/intel_gt_pm.h | 4 + drivers/gpu/drm/i915/gt/uc/intel_guc.h| 11 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 146 +++--- 6 files changed, 121 insertions(+), 54 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index e9a0cad5c34d..1076066f41e0 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -399,6 +399,8 @@ intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine) ce->guc_id.id = GUC_INVALID_LRC_ID; INIT_LIST_HEAD(&ce->guc_id.link); + INIT_LIST_HEAD(&ce->destroyed_link); + /* * Initialize fence to be complete as this is expected to be complete * unless there is a pending schedule disable outstanding. diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index e7e3984aab78..4613d027cbc3 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -213,6 +213,13 @@ struct intel_context { struct list_head link; } guc_id; + /** +* @destroyed_link: link in guc->submission_state.destroyed_contexts, in +* list when context is pending to be destroyed (deregistered with the +* GuC), protected by guc->submission_state.lock +*/ + struct list_head destroyed_link; + #ifdef CONFIG_DRM_I915_SELFTEST /** * @drop_schedule_enable: Force drop of schedule enable G2H for selftest diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h index 8520c595f5e1..6fdeae668e6e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h @@ -16,6 +16,11 @@ intel_engine_pm_is_awake(const struct intel_engine_cs *engine) return intel_wakeref_is_active(&engine->wakeref); } +static inline void __intel_engine_pm_get(struct intel_engine_cs *engine) +{ + __intel_wakeref_get(&engine->wakeref); +} + static inline void intel_engine_pm_get(struct intel_engine_cs *engine) { intel_wakeref_get(&engine->wakeref); diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index d0588d8aaa44..05de6c1af25b 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -41,6 +41,10 @@ static inline void intel_gt_pm_put_async(struct intel_gt *gt) intel_wakeref_put_async(>->wakeref); } +#define with_intel_gt_pm(gt, tmp) \ + for (tmp = 1, intel_gt_pm_get(gt); tmp; \ +intel_gt_pm_put(gt), tmp = 0) + static inline int intel_gt_pm_wait_for_idle(struct intel_gt *gt) { return intel_wakeref_wait_for_idle(>->wakeref); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 65b5e8eeef96..25a598e2b6e8 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -84,6 +84,17 @@ struct intel_guc { * refs */ struct list_head guc_id_list; + /** +* @destroyed_contexts: list of contexts waiting to be destroyed +* (deregistered with the GuC) +*/ + struct list_head destroyed_contexts; + /** +* @destroyed_worker: worker to deregister contexts, need as we +* need to take a GT PM reference and can't from destroy +* function as it might be in an atomic context (no sleeping) +*/ + struct work_struct destroyed_worker; } submission_state; /** diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index ad5c18119d92..17da2fea1bff 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -90,8 +90,8 @@ * used for all of GuC submission but that could change in the future. * *
[PATCH 00/26] Parallel submission aka multi-bb execbuf
As discussed in [1] we are introducing a new parallel submission uAPI for the i915 which allows more than 1 BB to be submitted in an execbuf IOCTL. This is the implemenation for both GuC and execlists. In addition to selftests in the series, an IGT is available implemented in the first 4 patches [2]. The execbuf IOCTL changes have been done in a single large patch (#21) as all the changes flow together and I believe a single patch will be better if some one has to lookup this change in the future. Can split in a series of smaller patches if desired. This code is available in a public [3] repo for UMD teams to test there code on. v2: Drop complicated state machine to block in kernel if no guc_ids available, perma-pin parallel contexts, reworker execbuf IOCTL to be a series of loops inside the IOCTL rather than 1 large one on the outside, address Daniel Vetter's comments v3: Address John Harrison's comments, add a couple of patches which fix bugs found internally Signed-off-by: Matthew Brost [1] https://patchwork.freedesktop.org/series/92028/ [2] https://patchwork.freedesktop.org/series/93071/ [3] https://gitlab.freedesktop.org/mbrost/mbrost-drm-intel/-/tree/drm-intel-parallel Matthew Brost (26): drm/i915/guc: Move GuC guc_id allocation under submission state sub-struct drm/i915/guc: Take GT PM ref when deregistering context drm/i915/guc: Take engine PM when a context is pinned with GuC submission drm/i915/guc: Don't call switch_to_kernel_context with GuC submission drm/i915: Add logical engine mapping drm/i915: Expose logical engine instance to user drm/i915/guc: Introduce context parent-child relationship drm/i915/guc: Add multi-lrc context registration drm/i915/guc: Ensure GuC schedule operations do not operate on child contexts drm/i915/guc: Assign contexts in parent-child relationship consecutive guc_ids drm/i915/guc: Implement parallel context pin / unpin functions drm/i915/guc: Implement multi-lrc submission drm/i915/guc: Insert submit fences between requests in parent-child relationship drm/i915/guc: Implement multi-lrc reset drm/i915/guc: Update debugfs for GuC multi-lrc drm/i915: Fix bug in user proto-context creation that leaked contexts drm/i915/guc: Connect UAPI to GuC multi-lrc interface drm/i915/doc: Update parallel submit doc to point to i915_drm.h drm/i915/guc: Add basic GuC multi-lrc selftest drm/i915/guc: Implement no mid batch preemption for multi-lrc drm/i915: Multi-BB execbuf drm/i915/guc: Handle errors in multi-lrc requests drm/i915: Make request conflict tracking understand parallel submits drm/i915: Update I915_GEM_BUSY IOCTL to understand composite fences drm/i915: Enable multi-bb execbuf drm/i915/execlists: Weak parallel submission support for execlists Documentation/gpu/rfc/i915_parallel_execbuf.h | 122 -- Documentation/gpu/rfc/i915_scheduler.rst |4 +- drivers/gpu/drm/i915/gem/i915_gem_busy.c | 60 +- drivers/gpu/drm/i915/gem/i915_gem_context.c | 225 ++- .../gpu/drm/i915/gem/i915_gem_context_types.h |6 + .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 796 ++--- drivers/gpu/drm/i915/gt/intel_context.c | 50 +- drivers/gpu/drm/i915/gt/intel_context.h | 54 +- drivers/gpu/drm/i915/gt/intel_context_types.h | 73 +- drivers/gpu/drm/i915/gt/intel_engine.h| 12 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 66 +- drivers/gpu/drm/i915/gt/intel_engine_pm.c | 13 + drivers/gpu/drm/i915/gt/intel_engine_pm.h | 37 + drivers/gpu/drm/i915/gt/intel_engine_types.h |7 + .../drm/i915/gt/intel_execlists_submission.c | 63 +- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 14 + drivers/gpu/drm/i915/gt/intel_lrc.c |7 + drivers/gpu/drm/i915/gt/selftest_execlists.c | 12 +- .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h |1 + drivers/gpu/drm/i915/gt/uc/intel_guc.c| 26 + drivers/gpu/drm/i915/gt/uc/intel_guc.h| 49 +- drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c|2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 24 +- drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 27 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 1456 ++--- .../drm/i915/gt/uc/selftest_guc_multi_lrc.c | 179 ++ drivers/gpu/drm/i915/i915_query.c |2 + drivers/gpu/drm/i915/i915_request.c | 143 +- drivers/gpu/drm/i915/i915_request.h | 23 + drivers/gpu/drm/i915/i915_vma.c | 21 +- drivers/gpu/drm/i915/i915_vma.h | 13 +- drivers/gpu/drm/i915/intel_wakeref.h | 12 + .../drm/i915/selftests/i915_live_selftests.h |1 + include/uapi/drm/i915_drm.h | 139 +- 34 files changed, 3038 insertions(+), 701 deletions(-) delete mode 100644 Documentation/gpu/rfc/i915_parallel_execbuf.h create mode 100644 drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c -- 2.32.0
Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
Cc'ing Dan Carpenter On Fri, Oct 01, 2021 at 12:57:13PM +0300, Jani Nikula wrote: On Fri, 01 Oct 2021, Chris Wilson wrote: Quoting Lucas De Marchi (2021-10-01 08:40:41) When trying to bring IS_ACTIVE to linux/kconfig.h I thought it wouldn't provide much value just encapsulating it in a boolean context. So I also added the support for handling undefined macros as the IS_ENABLED() counterpart. However the feedback received from Masahiro Yamada was that it is too ugly, not providing much value. And just wrapping in a boolean context is too dumb - we could simply open code it. As detailed in commit babaab2f4738 ("drm/i915: Encapsulate kconfig constant values inside boolean predicates"), the IS_ACTIVE macro was added to workaround a compilation warning. However after checking again our current uses of IS_ACTIVE it turned out there is only 1 case in which it would potentially trigger a warning. All the others can simply use the shorter version, without wrapping it in any macro. And even that single one didn't trigger any warning in gcc 10.3. So here I'm dialing all the way back to simply removing the macro. If it triggers warnings in future we may change the few cases to check for > 0 or != 0. Another possibility would be to use the great "not not operator" for all positive checks, which would allow us to maintain consistency. However let's try first the simplest form though, hopefully we don't hit broken compilers spitting a warning: You didn't prevent the compilation warning this re-introduces. drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: should this be a bitwise op? drivers/gpu/drm/i915/i915_request.c:1679 i915_request_wait() warn: should this be a bitwise op? Looks like that's a Smatch warning. The immediate fix would be to just add the != 0 in the relevant places. But this is stuff that's just going to get broken again unless we add Smatch to CI. Most people aren't running it on a regular basis. clang gives a warning only in drivers/gpu/drm/i915/i915_config.c and the warning is gone if the condition swapped: - if (context && CONFIG_DRM_I915_FENCE_TIMEOUT) + if (CONFIG_DRM_I915_FENCE_TIMEOUT && context) which would make sense if we think about shortcutting the if condition. However smatch still reports the warning and an additional one in drivers/gpu/drm/i915/i915_request.c. The ways I found to stop the false positives with smatch are: if (context && CONFIG_DRM_I915_FENCE_TIMEOUT != 0) or if (context && !!CONFIG_DRM_I915_FENCE_TIMEOUT) or if (context && CONFIG_DRM_I915_FENCE_TIMEOUT > 0) Dan, anything else that we could do here? This is about this kind of code: f (context && CONFIG_DRM_I915_FENCE_TIMEOUT) in which context is a u64 variable, that gives this warning: drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: should this be a bitwise op? thanks Lucas De Marchi BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 15/16] Revert "drm/i915: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()"
On 21/10/04 11:56AM, Sean Paul wrote: > @Fernando, hopefully you can revise and post again. Thank you for your patches > and your effort! No problem :) Just to be sure I do the right thing this time (and to better understand the process), please confirm that this is the correct sequence of events: 1. I fix the lock issue and test on my local machine. 2. I then post this new patch set (v3) rebased on top of drm-tip (instead of drm-next). This will automatically trigger tests on intel hardware (and maybe in other hardwares?) NOTE: I originally chose drm-next because that's what is mentioned here: https://01.org/linuxgraphics/gfx-docs/drm/gpu/introduction.html#contribution-process Maybe this doc should be updated? 3. Once reviewed and approved, someone (Sean?) merges them into "somewhere" (drm-next? drm-misc-next? drm-intel-next? How is this decided?). 4. Eventually, that other branch from the previous point is merged into drm-tip. 5. ?? 6. The branch is merged into linux-next. There must be something wrong in my description above, as it doesn't make sense to post the patch series based on "drm-tip" only to later have one of the mainteiners merge them into a different branch that will eventually be merged back into "drm-tip". Sorry for being completely lost! Is there a document explaining how all of this works so that I can learn for the next time? Thanks!
Re: [PATCH v3 12/14] dt-bindings: msm/dp: Add bindings for HDCP registers
On Fri 01 Oct 10:11 CDT 2021, Sean Paul wrote: > From: Sean Paul > > This patch adds the bindings for the MSM DisplayPort HDCP registers > which are required to write the HDCP key into the display controller as > well as the registers to enable HDCP authentication/key > exchange/encryption. > > We'll use a new compatible string for this since the fields are optional. > > Cc: Rob Herring > Cc: Stephen Boyd > Signed-off-by: Sean Paul > Link: > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-13-s...@poorly.run > #v1 > Link: > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-13-s...@poorly.run > #v2 > > Changes in v2: > -Drop register range names (Stephen) > -Fix yaml errors (Rob) > Changes in v3: > -Add new compatible string for dp-hdcp > -Add descriptions to reg > -Add minItems/maxItems to reg > -Make reg depend on the new hdcp compatible string > --- > > Disclaimer: I really don't know if this is the right way to approach > this. I tried using examples from other bindings, but feedback would be > very much welcome on how I could add the optional register ranges. > > > .../bindings/display/msm/dp-controller.yaml | 34 --- > 1 file changed, 30 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > index 64d8d9e5e47a..a176f97b2f4c 100644 > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > @@ -17,9 +17,10 @@ properties: >compatible: > enum: >- qcom,sc7180-dp > + - qcom,sc7180-dp-hdcp > > - reg: > -maxItems: 1 > + # See compatible-specific constraints below. > + reg: true > >interrupts: > maxItems: 1 > @@ -89,6 +90,29 @@ required: >- power-domains >- ports > > +allOf: > + - if: > + properties: > +compatible: > + contains: > +const: qcom,sc7180-dp-hdcp > +then: > + properties: > +reg: > + minItems: 3 > + maxItems: 3 > + items: > +- description: Registers for base DP functionality > +- description: (Optional) Registers for HDCP device key injection > +- description: (Optional) Registers for HDCP TrustZone > interaction > +else: > + properties: > +reg: > + minItems: 1 > + maxItems: 1 > + items: > +- description: Registers for base DP functionality > + > additionalProperties: false > > examples: > @@ -99,8 +123,10 @@ examples: > #include > > displayport-controller@ae9 { > -compatible = "qcom,sc7180-dp"; > -reg = <0xae9 0x1400>; > +compatible = "qcom,sc7180-dp-hdcp"; > +reg = <0 0x0ae9 0 0x1400>, > + <0 0x0aed1000 0 0x174>, > + <0 0x0aee1000 0 0x2c>; Forgot to mention, #address-cells = #size-cells = <1> in the example "environment", so you have to omit the lone 0s in the example to make it pass the tests. Regards, Bjorn > interrupt-parent = <&mdss>; > interrupts = <12>; > clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, > -- > Sean Paul, Software Engineer, Google / Chromium OS >
Re: [PATCH v3 12/14] dt-bindings: msm/dp: Add bindings for HDCP registers
On Fri 01 Oct 10:11 CDT 2021, Sean Paul wrote: > From: Sean Paul > > This patch adds the bindings for the MSM DisplayPort HDCP registers > which are required to write the HDCP key into the display controller as > well as the registers to enable HDCP authentication/key > exchange/encryption. > > We'll use a new compatible string for this since the fields are optional. > I don't think you need a new compatible, in particular since I presume we should use the hdcp compatible in all platforms? Or is there a reason for not picking that one? Instead I suggest that you simply do minItems: 1, maxItems: 3 and detect which of the two cases you have in the driver. PS. I hope to get https://lore.kernel.org/linux-arm-msm/20211001174400.981707-1-bjorn.anders...@linaro.org/ landed before we add these new optional regions... Regards, Bjorn > Cc: Rob Herring > Cc: Stephen Boyd > Signed-off-by: Sean Paul > Link: > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-13-s...@poorly.run > #v1 > Link: > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-13-s...@poorly.run > #v2 > > Changes in v2: > -Drop register range names (Stephen) > -Fix yaml errors (Rob) > Changes in v3: > -Add new compatible string for dp-hdcp > -Add descriptions to reg > -Add minItems/maxItems to reg > -Make reg depend on the new hdcp compatible string > --- > > Disclaimer: I really don't know if this is the right way to approach > this. I tried using examples from other bindings, but feedback would be > very much welcome on how I could add the optional register ranges. > > > .../bindings/display/msm/dp-controller.yaml | 34 --- > 1 file changed, 30 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > index 64d8d9e5e47a..a176f97b2f4c 100644 > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > @@ -17,9 +17,10 @@ properties: >compatible: > enum: >- qcom,sc7180-dp > + - qcom,sc7180-dp-hdcp > > - reg: > -maxItems: 1 > + # See compatible-specific constraints below. > + reg: true > >interrupts: > maxItems: 1 > @@ -89,6 +90,29 @@ required: >- power-domains >- ports > > +allOf: > + - if: > + properties: > +compatible: > + contains: > +const: qcom,sc7180-dp-hdcp > +then: > + properties: > +reg: > + minItems: 3 > + maxItems: 3 > + items: > +- description: Registers for base DP functionality > +- description: (Optional) Registers for HDCP device key injection > +- description: (Optional) Registers for HDCP TrustZone > interaction > +else: > + properties: > +reg: > + minItems: 1 > + maxItems: 1 > + items: > +- description: Registers for base DP functionality > + > additionalProperties: false > > examples: > @@ -99,8 +123,10 @@ examples: > #include > > displayport-controller@ae9 { > -compatible = "qcom,sc7180-dp"; > -reg = <0xae9 0x1400>; > +compatible = "qcom,sc7180-dp-hdcp"; > +reg = <0 0x0ae9 0 0x1400>, > + <0 0x0aed1000 0 0x174>, > + <0 0x0aee1000 0 0x2c>; > interrupt-parent = <&mdss>; > interrupts = <12>; > clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, > -- > Sean Paul, Software Engineer, Google / Chromium OS >
Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID
On Mon, Oct 04, 2021 at 09:21:27AM -0700, Douglas Anderson wrote: > In the commit bac9c2948224 ("drm/edid: Break out reading block 0 of > the EDID") I broke out reading the base block of the EDID to its own > function. Unfortunately, when I did that I messed up the handling when > drm_edid_is_zero() indicated that we had an EDID that was all 0x00 or > when we went through 4 loops and didn't get a valid EDID. Specifically > I needed to pass the broken EDID to connector_bad_edid() but now I was > passing an error-pointer. > > Let's re-jigger things so we can pass the bad EDID in properly. > > Fixes: bac9c2948224 ("drm/edid: Break out reading block 0 of the EDID") > Reported-by: kernel test robot > Reported-by: Geert Uytterhoeven > Signed-off-by: Douglas Anderson > --- > > drivers/gpu/drm/drm_edid.c | 27 +++ > 1 file changed, 11 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 9b19eee0e1b4..9c9463ec5465 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -1911,13 +1911,15 @@ int drm_add_override_edid_modes(struct drm_connector > *connector) > } > EXPORT_SYMBOL(drm_add_override_edid_modes); > > -static struct edid *drm_do_get_edid_base_block( > +static struct edid *drm_do_get_edid_base_block(struct drm_connector > *connector, > int (*get_edid_block)(void *data, u8 *buf, unsigned int block, > size_t len), > - void *data, bool *edid_corrupt, int *null_edid_counter) > + void *data) > { > - int i; > + int *null_edid_counter = connector ? &connector->null_edid_counter : > NULL; > + bool *edid_corrupt = connector ? &connector->edid_corrupt : NULL; > void *edid; > + int i; > > edid = kmalloc(EDID_LENGTH, GFP_KERNEL); > if (edid == NULL) > @@ -1941,9 +1943,8 @@ static struct edid *drm_do_get_edid_base_block( > return edid; > > carp: > - kfree(edid); > - return ERR_PTR(-EINVAL); > - > + if (connector) > + connector_bad_edid(connector, edid, 1); BTW I believe connector_bad_edid() itself is broken since commit e11f5bd8228f ("drm: Add support for DP 1.4 Compliance edid corruption test"). Before we've even allocated the memory for the extension blocks that code now assumes edid[0x7e] is to be 100% trusted and goes and calculates the checksum on a block based on that. So that's likely going to be pointing somewhere beyond the base block into memory we've not even allocated. So anyone who wanted could craft a bogus EDID and maybe get something interesting to happen. Would be good if someone could fix that while at it. Or just revert the offending commit if there is no simple solution immediately in sight. The fact that we're parsing entirely untrustworthy crap in the kernel always worries me. Either we need super careful review of all relevant code, and/or we need to think about moving the parser out of the kernel. I was considering playing around with the usermode helper stuff. IIRC there is a way to embed the userspace binary into the kernel and just fire it up when needed. But so far it's been the usual -ENOTIME for me... -- Ville Syrjälä Intel
[PULL] drm-intel-next
Hi Dave and Daniel, Here goes an accumulated pull request. A special highlight to the ADL-P (XE_LPD) and DG2 display support preparation and on a big clean-up in the display portion of the driver. Here goes drm-intel-next-2021-10-04: Cross-subsystem Changes: - fbdev/efifb: Release PCI device's runtime PM ref during FB destr\ oy (Imre) i915 Core Driver Changes: - Only access SFC_DONE in media when not fused off for graphics 12 and newer. - Double Memory latency values from pcode for DG2 (Matt Roper) - ADL-S PCI ID update (Tejas) - New DG1 PCI ID (Jose) - Fix regression with uncore refactoring (Dave) i915 Display Changes: - ADL-P display (XE_LPD) fixes and updates (Ankit, Jani, Matt Roper, Anusham, Jose, Imre, Vandita) - DG2 display fixes (Ankit, Jani) - Expand PCH_CNP tweaked display workaround to all newer displays (Anshuman) - General display simplifications and clean-ups (Jani, Swati, Jose, Ville) - PSR Clean-ups, dropping support for BDW/HSD and enable PSR2 selective fetch by default (Jose, Gwan-gyeong) - Nuke ORIGIN_GTT (Jose) - Return proper DPRX link training result (Lee) - FBC related refactor and fixes (Ville) - Yet another attempt to solve the fast+narrow vs slow+wide eDP link training (Kai-Heng) - DP 2.0 preparation work (Jani) - Silence __iomem sparse warn (Ville) - Clean up DPLL stuff (Ville) - Fix various dp/edp max rates (Matt Atwood, Animesh, Jani) - Remove VBT ddi_port_info caching (Jani) - DSI driver improvements (Lee) - HDCP fixes (Juston) - Associate ACPI connector nodes with connector entries (Heikki) - Add support for out-of-bound hotplug events (Hans) - VESA vendor block and drm/i915 MSO use of it (Jani) - Fixes for bigjoiner (Ville) - Update memory bandwidth parameters (RK) - DMC related fixes (Chris, Jose) - HDR related fixes and improvements (Tejas) - g4x/vlv/chv CxSR/wm fixes/cleanups (Ville) - Use BIOS provided value for RKL Audio's HDA link (Kai-Heng) - Fix the dsc check while selecting min_cdclk (Vandita) - Split and constify vtable (Dave) - Add ww context to intel_dpt_pin (Maarten) - Fix bdb version check (Lukasz) - DP per-lane drive settings prep work and other DP fixes (Ville) Thanks, Rodrigo. The following changes since commit 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f: Linux 5.15-rc1 (2021-09-12 16:28:37 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-2021-10-04 for you to fetch changes up to 104c1b3d6fb6a794babd5e2ffd6a5183b5a3d6c7: drm/i915: Allow per-lane drive settings with LTTPRs (2021-10-04 13:04:36 +0300) Cross-subsystem Changes: - fbdev/efifb: Release PCI device's runtime PM ref during FB destr\ oy (Imre) i915 Core Driver Changes: - Only access SFC_DONE in media when not fused off for graphics 12 and newer. - Double Memory latency values from pcode for DG2 (Matt Roper) - ADL-S PCI ID update (Tejas) - New DG1 PCI ID (Jose) - Fix regression with uncore refactoring (Dave) i915 Display Changes: - ADL-P display (XE_LPD) fixes and updates (Ankit, Jani, Matt Roper, Anusham, Jose, Imre, Vandita) - DG2 display fixes (Ankit, Jani) - Expand PCH_CNP tweaked display workaround to all newer displays (Anshuman) - General display simplifications and clean-ups (Jani, Swati, Jose, Ville) - PSR Clean-ups, dropping support for BDW/HSD and enable PSR2 selective fetch by default (Jose, Gwan-gyeong) - Nuke ORIGIN_GTT (Jose) - Return proper DPRX link training result (Lee) - FBC related refactor and fixes (Ville) - Yet another attempt to solve the fast+narrow vs slow+wide eDP link training (Kai-Heng) - DP 2.0 preparation work (Jani) - Silence __iomem sparse warn (Ville) - Clean up DPLL stuff (Ville) - Fix various dp/edp max rates (Matt Atwood, Animesh, Jani) - Remove VBT ddi_port_info caching (Jani) - DSI driver improvements (Lee) - HDCP fixes (Juston) - Associate ACPI connector nodes with connector entries (Heikki) - Add support for out-of-bound hotplug events (Hans) - VESA vendor block and drm/i915 MSO use of it (Jani) - Fixes for bigjoiner (Ville) - Update memory bandwidth parameters (RK) - DMC related fixes (Chris, Jose) - HDR related fixes and improvements (Tejas) - g4x/vlv/chv CxSR/wm fixes/cleanups (Ville) - Use BIOS provided value for RKL Audio's HDA link (Kai-Heng) - Fix the dsc check while selecting min_cdclk (Vandita) - Split and constify vtable (Dave) - Add ww context to intel_dpt_pin (Maarten) - Fix bdb version check (Lukasz) - DP per-lane drive settings prep work and other DP fixes (Ville) Animesh Manna (3): drm/i915/dg2: UHBR tables added for pll programming drm/i915/dp: fix EHL/JSL max source rates calculation drm/i915/dp: fix for ADL_P/S dp/edp max source rates Ankit Nautiyal (2): drm/i915/display: Fix the 12 BPC bits for PIPE_MISC reg drm/i915/dg2: Configure PCON in DP pre-enable path Anshuman Gupta (1): drm/i915: Twea
[PATCH 10/10] backlight: qcom-wled: Consider enabled_strings in autodetection
Following the previous commit using enabled_strings in set_brightness, enabled_strings is now also used in the autodetection path for consistent behaviour: when a list of strings is specified in DT only those strings will be probed for autodetection, analogous to how the number of strings that need to be probed is already bound by qcom,num-strings. Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index c0b8d10c20a2..2c49f5d8dc26 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -569,7 +569,7 @@ static irqreturn_t wled_short_irq_handler(int irq, void *_wled) static void wled_auto_string_detection(struct wled *wled) { - int rc = 0, i, delay_time_us; + int rc = 0, i, j, delay_time_us; u32 sink_config = 0; u8 sink_test = 0, sink_valid = 0, val; bool fault_set; @@ -616,14 +616,15 @@ static void wled_auto_string_detection(struct wled *wled) /* Iterate through the strings one by one */ for (i = 0; i < wled->cfg.num_strings; i++) { - sink_test = BIT((WLED4_SINK_REG_CURR_SINK_SHFT + i)); + j = wled->cfg.enabled_strings[i]; + sink_test = BIT((WLED4_SINK_REG_CURR_SINK_SHFT + j)); /* Enable feedback control */ rc = regmap_write(wled->regmap, wled->ctrl_addr + - WLED3_CTRL_REG_FEEDBACK_CONTROL, i + 1); + WLED3_CTRL_REG_FEEDBACK_CONTROL, j + 1); if (rc < 0) { dev_err(wled->dev, "Failed to enable feedback for SINK %d rc = %d\n", - i + 1, rc); + j + 1, rc); goto failed_detect; } @@ -632,7 +633,7 @@ static void wled_auto_string_detection(struct wled *wled) WLED4_SINK_REG_CURR_SINK, sink_test); if (rc < 0) { dev_err(wled->dev, "Failed to configure SINK %d rc=%d\n", - i + 1, rc); + j + 1, rc); goto failed_detect; } @@ -659,7 +660,7 @@ static void wled_auto_string_detection(struct wled *wled) if (fault_set) dev_dbg(wled->dev, "WLED OVP fault detected with SINK %d\n", - i + 1); + j + 1); else sink_valid |= sink_test; @@ -699,15 +700,16 @@ static void wled_auto_string_detection(struct wled *wled) /* Enable valid sinks */ if (wled->version == 4) { for (i = 0; i < wled->cfg.num_strings; i++) { + j = wled->cfg.enabled_strings[i]; if (sink_config & - BIT(WLED4_SINK_REG_CURR_SINK_SHFT + i)) + BIT(WLED4_SINK_REG_CURR_SINK_SHFT + j)) val = WLED4_SINK_REG_STR_MOD_MASK; else /* Disable modulator_en for unused sink */ val = 0; rc = regmap_write(wled->regmap, wled->sink_addr + - WLED4_SINK_REG_STR_MOD_EN(i), val); + WLED4_SINK_REG_STR_MOD_EN(j), val); if (rc < 0) { dev_err(wled->dev, "Failed to configure MODULATOR_EN rc=%d\n", rc); -- 2.33.0
[PATCH 07/10] backlight: qcom-wled: Provide enabled_strings default for wled 4 and 5
Only wled 3 sets a sensible default that allows operating this driver with just qcom,num-strings in the DT; wled 4 and 5 require qcom,enabled-strings to be provided otherwise enabled_strings remains zero-initialized, resuling in every string-specific register write (currently only the setup and config functions, brightness follows in a future patch) to only configure the zero'th string multiple times. Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 9ec1bdd374d2..f143b2563bfe 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -1077,6 +1077,7 @@ static const struct wled_config wled4_config_defaults = { .cabc = false, .external_pfet = false, .auto_detection_enabled = false, + .enabled_strings = {0, 1, 2, 3}, }; static int wled5_setup(struct wled *wled) @@ -1190,6 +1191,7 @@ static const struct wled_config wled5_config_defaults = { .cabc = false, .external_pfet = false, .auto_detection_enabled = false, + .enabled_strings = {0, 1, 2, 3}, }; static const u32 wled3_boost_i_limit_values[] = { -- 2.33.0
[PATCH 08/10] backlight: qcom-wled: Remove unnecessary double whitespace
Remove redundant spaces inside for loop conditions. No other double spaces were found that are not part of indentation with `[^\s] `. Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index f143b2563bfe..dbe503007ada 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -235,7 +235,7 @@ static int wled3_set_brightness(struct wled *wled, u16 brightness) v = cpu_to_le16(brightness & WLED3_SINK_REG_BRIGHT_MAX); - for (i = 0; i < wled->cfg.num_strings; ++i) { + for (i = 0; i < wled->cfg.num_strings; ++i) { rc = regmap_bulk_write(wled->regmap, wled->ctrl_addr + WLED3_SINK_REG_BRIGHT(i), &v, sizeof(v)); @@ -257,7 +257,7 @@ static int wled4_set_brightness(struct wled *wled, u16 brightness) v = cpu_to_le16(brightness & WLED3_SINK_REG_BRIGHT_MAX); - for (i = 0; i < wled->cfg.num_strings; ++i) { + for (i = 0; i < wled->cfg.num_strings; ++i) { rc = regmap_bulk_write(wled->regmap, wled->sink_addr + WLED4_SINK_REG_BRIGHT(i), &v, sizeof(v)); -- 2.33.0
[PATCH 09/10] backlight: qcom-wled: Consistently use enabled-strings in set_brightness
The hardware is capable of controlling any non-contiguous sequence of LEDs specified in the DT using qcom,enabled-strings as u32 array, and this also follows from the DT-bindings documentation. The numbers specified in this array represent indices of the LED strings that are to be enabled and disabled. Its value is appropriately used to setup and enable string modules, but completely disregarded in the set_brightness paths which only iterate over the number of strings linearly. Take an example where only string 2 is enabled with qcom,enabled_strings=<2>: this string is appropriately enabled but subsequent brightness changes would have only touched the zero'th brightness register because num_strings is 1 here. This is simply addressed by looking up the string for this index in the enabled_strings array just like the other codepaths that iterate over num_strings. Fixes: 775d2ffb4af6 ("backlight: qcom-wled: Restructure the driver for WLED3") Fixes: 03b2b5e86986 ("backlight: qcom-wled: Add support for WLED4 peripheral") Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index dbe503007ada..c0b8d10c20a2 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -237,7 +237,7 @@ static int wled3_set_brightness(struct wled *wled, u16 brightness) for (i = 0; i < wled->cfg.num_strings; ++i) { rc = regmap_bulk_write(wled->regmap, wled->ctrl_addr + - WLED3_SINK_REG_BRIGHT(i), + WLED3_SINK_REG_BRIGHT(wled->cfg.enabled_strings[i]), &v, sizeof(v)); if (rc < 0) return rc; @@ -259,7 +259,7 @@ static int wled4_set_brightness(struct wled *wled, u16 brightness) for (i = 0; i < wled->cfg.num_strings; ++i) { rc = regmap_bulk_write(wled->regmap, wled->sink_addr + - WLED4_SINK_REG_BRIGHT(i), + WLED4_SINK_REG_BRIGHT(wled->cfg.enabled_strings[i]), &v, sizeof(v)); if (rc < 0) return rc; -- 2.33.0
[PATCH 04/10] backlight: qcom-wled: Validate enabled string indices in DT
The strings passed in DT may possibly cause out-of-bounds register accesses and should be validated before use. Fixes: 775d2ffb4af6 ("backlight: qcom-wled: Restructure the driver for WLED3") Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 29910e603c42..27e8949c7922 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -1526,6 +1526,12 @@ static int wled_configure(struct wled *wled) "qcom,enabled-strings", sizeof(u32)); if (string_len > 0) { + if (string_len > wled->max_string_count) { + dev_err(dev, "Cannot have more than %d strings\n", + wled->max_string_count); + return -EINVAL; + } + rc = of_property_read_u32_array(dev->of_node, "qcom,enabled-strings", wled->cfg.enabled_strings, @@ -1537,6 +1543,14 @@ static int wled_configure(struct wled *wled) return -EINVAL; } + for (i = 0; i < string_len; ++i) { + if (wled->cfg.enabled_strings[i] >= wled->max_string_count) { + dev_err(dev, "qcom,enabled-strings index %d at %d is out of bounds\n", + wled->cfg.enabled_strings[i], i); + return -EINVAL; + } + } + cfg->num_strings = string_len; } -- 2.33.0
[PATCH 03/10] backlight: qcom-wled: Override num-strings when enabled-strings is set
DT-bindings do not specify num-strings as mandatory property, yet it is required to be specified even if enabled-strings is used. The length of that property-array should already be enough to determine exactly which and how many strings to enable. Fixes: 775d2ffb4af6 ("backlight: qcom-wled: Restructure the driver for WLED3") Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 9927ed98944a..29910e603c42 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -1536,6 +1536,8 @@ static int wled_configure(struct wled *wled) string_len, rc); return -EINVAL; } + + cfg->num_strings = string_len; } return 0; -- 2.33.0
[PATCH 01/10] backlight: qcom-wled: Pass number of elements to read to read_u32_array
of_property_read_u32_array takes the number of elements to read as last argument. This does not always need to be 4 (sizeof(u32)) but should instead be the size of the array in DT as read just above with of_property_count_elems_of_size. To not make such an error go unnoticed again the driver now bails accordingly when of_property_read_u32_array returns an error. Fixes: 775d2ffb4af6 ("backlight: qcom-wled: Restructure the driver for WLED3") Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index d094299c2a48..6af808af2328 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -1528,11 +1528,18 @@ static int wled_configure(struct wled *wled) string_len = of_property_count_elems_of_size(dev->of_node, "qcom,enabled-strings", sizeof(u32)); - if (string_len > 0) - of_property_read_u32_array(dev->of_node, + if (string_len > 0) { + rc = of_property_read_u32_array(dev->of_node, "qcom,enabled-strings", wled->cfg.enabled_strings, - sizeof(u32)); + string_len); + if (rc) { + dev_err(dev, "Failed to read %d elements from " + "qcom,enabled-strings: %d\n", + string_len, rc); + return -EINVAL; + } + } return 0; } -- 2.33.0
[PATCH 06/10] backlight: qcom-wled: Remove unnecessary 4th default string in wled3
The previous commit improves num_strings parsing to not go over the maximum of 3 strings for wled3 anymore. Likewise this default index for a hypothetical 4th string is invalid and could access registers that are not mapped to the desired purpose. Removing this value gets rid of undesired confusion and avoids the possibility of accessing registers at this offset even if the 4th array element is used by accident. Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 66ce77ee3099..9ec1bdd374d2 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -946,7 +946,7 @@ static const struct wled_config wled3_config_defaults = { .cs_out_en = false, .ext_gen = false, .cabc = false, - .enabled_strings = {0, 1, 2, 3}, + .enabled_strings = {0, 1, 2}, }; static int wled4_setup(struct wled *wled) -- 2.33.0
[PATCH 05/10] backlight: qcom-wled: Fix off-by-one maximum with default num_strings
When not specifying num-strings in the DT the default is used, but +1 is added to it which turns wled3 into 4 and wled4/5 into 5 strings instead of 3 and 4 respectively, causing out of bounds reads and register read/writes. This +1 exists for a deficiency in the DT parsing code, and is simply omitted entirely - solving this oob issue - by allowing one extra iteration of the wled_var_cfg function parsing this particular property. Fixes: 93c64f1ea1e8 ("leds: add Qualcomm PM8941 WLED driver") Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 27e8949c7922..66ce77ee3099 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -1255,17 +1255,17 @@ static const struct wled_var_cfg wled5_ovp_cfg = { static u32 wled3_num_strings_values_fn(u32 idx) { - return idx + 1; + return idx; } static const struct wled_var_cfg wled3_num_strings_cfg = { .fn = wled3_num_strings_values_fn, - .size = 3, + .size = 4, /* [0, 3] */ }; static const struct wled_var_cfg wled4_num_strings_cfg = { .fn = wled3_num_strings_values_fn, - .size = 4, + .size = 5, /* [0, 4] */ }; static u32 wled3_switch_freq_values_fn(u32 idx) @@ -1520,8 +1520,6 @@ static int wled_configure(struct wled *wled) *bool_opts[i].val_ptr = true; } - cfg->num_strings = cfg->num_strings + 1; - string_len = of_property_count_elems_of_size(dev->of_node, "qcom,enabled-strings", sizeof(u32)); -- 2.33.0
[PATCH 02/10] backlight: qcom-wled: Use cpu_to_le16 macro to perform conversion
The kernel already provides appropriate primitives to perform endianness conversion which should be used in favour of manual bit-wrangling. Signed-off-by: Marijn Suijten Reviewed-by: AngeloGioacchino Del Regno --- drivers/video/backlight/qcom-wled.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 6af808af2328..9927ed98944a 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -231,14 +231,14 @@ struct wled { static int wled3_set_brightness(struct wled *wled, u16 brightness) { int rc, i; - u8 v[2]; + u16 v; - v[0] = brightness & 0xff; - v[1] = (brightness >> 8) & 0xf; + v = cpu_to_le16(brightness & WLED3_SINK_REG_BRIGHT_MAX); for (i = 0; i < wled->cfg.num_strings; ++i) { rc = regmap_bulk_write(wled->regmap, wled->ctrl_addr + - WLED3_SINK_REG_BRIGHT(i), v, 2); + WLED3_SINK_REG_BRIGHT(i), + &v, sizeof(v)); if (rc < 0) return rc; } @@ -249,19 +249,18 @@ static int wled3_set_brightness(struct wled *wled, u16 brightness) static int wled4_set_brightness(struct wled *wled, u16 brightness) { int rc, i; - u16 low_limit = wled->max_brightness * 4 / 1000; - u8 v[2]; + u16 v, low_limit = wled->max_brightness * 4 / 1000; /* WLED4's lower limit of operation is 0.4% */ if (brightness > 0 && brightness < low_limit) brightness = low_limit; - v[0] = brightness & 0xff; - v[1] = (brightness >> 8) & 0xf; + v = cpu_to_le16(brightness & WLED3_SINK_REG_BRIGHT_MAX); for (i = 0; i < wled->cfg.num_strings; ++i) { rc = regmap_bulk_write(wled->regmap, wled->sink_addr + - WLED4_SINK_REG_BRIGHT(i), v, 2); + WLED4_SINK_REG_BRIGHT(i), + &v, sizeof(v)); if (rc < 0) return rc; } @@ -272,22 +271,20 @@ static int wled4_set_brightness(struct wled *wled, u16 brightness) static int wled5_set_brightness(struct wled *wled, u16 brightness) { int rc, offset; - u16 low_limit = wled->max_brightness * 1 / 1000; - u8 v[2]; + u16 v, low_limit = wled->max_brightness * 1 / 1000; /* WLED5's lower limit is 0.1% */ if (brightness < low_limit) brightness = low_limit; - v[0] = brightness & 0xff; - v[1] = (brightness >> 8) & 0x7f; + v = cpu_to_le16(brightness & WLED5_SINK_REG_BRIGHT_MAX_15B); offset = (wled->cfg.mod_sel == MOD_A) ? WLED5_SINK_REG_MOD_A_BRIGHTNESS_LSB : WLED5_SINK_REG_MOD_B_BRIGHTNESS_LSB; rc = regmap_bulk_write(wled->regmap, wled->sink_addr + offset, - v, 2); + &v, sizeof(v)); return rc; } -- 2.33.0
[PATCH 00/10] backlight: qcom-wled: fix and solidify handling of enabled-strings
This patchset fixes WLED's handling of enabled-strings: besides some cleanup it is now actually possible to specify a non-contiguous array of enabled strings (not necessarily starting at zero) and the values from DT are now validated to prevent possible unexpected out-of-bounds register and array element accesses. Off-by-one mistakes in the maximum number of strings, also causing out-of-bounds access, have been addressed as well. Marijn Suijten (10): backlight: qcom-wled: Pass number of elements to read to read_u32_array backlight: qcom-wled: Use cpu_to_le16 macro to perform conversion backlight: qcom-wled: Override num-strings when enabled-strings is set backlight: qcom-wled: Validate enabled string indices in DT backlight: qcom-wled: Fix off-by-one maximum with default num_strings backlight: qcom-wled: Remove unnecessary 4th default string in wled3 backlight: qcom-wled: Provide enabled_strings default for wled 4 and 5 backlight: qcom-wled: Remove unnecessary double whitespace backlight: qcom-wled: Consistently use enabled-strings in set_brightness backlight: qcom-wled: Consider enabled_strings in autodetection drivers/video/backlight/qcom-wled.c | 88 ++--- 1 file changed, 55 insertions(+), 33 deletions(-) -- 2.33.0
Re: [PATCH 10/10] drm/i915: Add privacy-screen support (v2)
On Mon, Oct 04, 2021 at 06:02:21PM +0200, Hans de Goede wrote: > Hi, > > On 10/4/21 5:37 PM, Ville Syrjälä wrote: > > On Sat, Oct 02, 2021 at 06:36:18PM +0200, Hans de Goede wrote: > >> Add support for eDP panels with a built-in privacy screen using the > >> new drm_privacy_screen class. > >> > >> Changes in v2: > >> - Call drm_connector_update_privacy_screen() from > >> intel_enable_ddi_dp() / intel_ddi_update_pipe_dp() instead of adding a > >> for_each_new_connector_in_state() loop to intel_atomic_commit_tail() > >> - Move the probe-deferral check to the intel_modeset_probe_defer() helper > >> > >> Signed-off-by: Hans de Goede > >> --- > >> drivers/gpu/drm/i915/display/intel_atomic.c | 1 + > >> drivers/gpu/drm/i915/display/intel_ddi.c | 3 +++ > >> drivers/gpu/drm/i915/display/intel_display.c | 10 ++ > >> drivers/gpu/drm/i915/display/intel_dp.c | 10 ++ > >> 4 files changed, 24 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > >> b/drivers/gpu/drm/i915/display/intel_atomic.c > >> index b4e7ac51aa31..a62550711e98 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_atomic.c > >> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > >> @@ -139,6 +139,7 @@ int intel_digital_connector_atomic_check(struct > >> drm_connector *conn, > >>new_conn_state->base.picture_aspect_ratio != > >> old_conn_state->base.picture_aspect_ratio || > >>new_conn_state->base.content_type != > >> old_conn_state->base.content_type || > >>new_conn_state->base.scaling_mode != > >> old_conn_state->base.scaling_mode || > >> + new_conn_state->base.privacy_screen_sw_state != > >> old_conn_state->base.privacy_screen_sw_state || > >>!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) > >>crtc_state->mode_changed = true; > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > >> b/drivers/gpu/drm/i915/display/intel_ddi.c > >> index 51cd0420e00e..e4496c830a35 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c > >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > >> @@ -25,6 +25,7 @@ > >> * > >> */ > >> > >> +#include > >> #include > >> > >> #include "i915_drv.h" > >> @@ -3022,6 +3023,7 @@ static void intel_enable_ddi_dp(struct > >> intel_atomic_state *state, > >>if (port == PORT_A && DISPLAY_VER(dev_priv) < 9) > >>intel_dp_stop_link_train(intel_dp, crtc_state); > >> > >> + drm_connector_update_privacy_screen(conn_state); > >>intel_edp_backlight_on(crtc_state, conn_state); > >> > >>if (!dig_port->lspcon.active || dig_port->dp.has_hdmi_sink) > >> @@ -3247,6 +3249,7 @@ static void intel_ddi_update_pipe_dp(struct > >> intel_atomic_state *state, > >>intel_drrs_update(intel_dp, crtc_state); > >> > >>intel_backlight_update(state, encoder, crtc_state, conn_state); > >> + drm_connector_update_privacy_screen(conn_state); > >> } > >> > >> void intel_ddi_update_pipe(struct intel_atomic_state *state, > >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c > >> b/drivers/gpu/drm/i915/display/intel_display.c > >> index e67f3207ba54..9a5dbe51458d 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display.c > >> +++ b/drivers/gpu/drm/i915/display/intel_display.c > >> @@ -42,6 +42,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> #include > >> #include > >> #include > >> @@ -12693,6 +12694,8 @@ void intel_modeset_driver_remove_nogem(struct > >> drm_i915_private *i915) > >> > >> bool intel_modeset_probe_defer(struct pci_dev *pdev) > >> { > >> + struct drm_privacy_screen *privacy_screen; > >> + > >>/* > >> * apple-gmux is needed on dual GPU MacBook Pro > >> * to probe the panel if we're the inactive GPU. > >> @@ -12700,6 +12703,13 @@ bool intel_modeset_probe_defer(struct pci_dev > >> *pdev) > >>if (vga_switcheroo_client_probe_defer(pdev)) > >>return true; > >> > >> + /* If the LCD panel has a privacy-screen, wait for it */ > >> + privacy_screen = drm_privacy_screen_get(&pdev->dev, NULL); > >> + if (IS_ERR(privacy_screen) && PTR_ERR(privacy_screen) == -EPROBE_DEFER) > >> + return true; > >> + > >> + drm_privacy_screen_put(privacy_screen); > >> + > >>return false; > >> } > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > >> b/drivers/gpu/drm/i915/display/intel_dp.c > >> index 74a657ae131a..91207310dc0d 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_dp.c > >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c > >> @@ -37,6 +37,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> #include > >> > >> #include "g4x_dp.h" > >> @@ -4808,6 +4809,7 @@ static bool intel_edp_init_connector(struct intel_dp > >> *intel_dp, > >>struct drm_connector *connector = &intel_connector->base; > >>struct drm_display_mode *fixed_mode = NULL; > >>struct drm_display_mode *downclock_mode = NULL; > >> + struct drm_privacy_screen *pr
Re: [PATCH v2] drm/i915: Clean up disabled warnings
On Tue, 14 Sep 2021, Nathan Chancellor wrote: > i915 enables a wider set of warnings with '-Wall -Wextra' then disables > several with cc-disable-warning. If an unknown flag gets added to > KBUILD_CFLAGS when building with clang, all subsequent calls to > cc-{disable-warning,option} will fail, meaning that all of these > warnings do not get disabled [1]. > > A separate series will address the root cause of the issue by not adding > these flags when building with clang [2]; however, the symptom of these > extra warnings appearing can be addressed separately by just removing > the calls to cc-disable-warning, which makes the build ever so slightly > faster because the compiler does not need to be called as much before > building. > > The following warnings are supported by GCC 4.9 and clang 10.0.1, which > are the minimum supported versions of these compilers so the call to > cc-disable-warning is not necessary. Masahiro cleaned this up for the > reset of the kernel in commit 4c8dd95a723d ("kbuild: add some extra > warning flags unconditionally"). > > * -Wmissing-field-initializers > * -Wsign-compare > * -Wtype-limits > * -Wunused-parameter > > -Wunused-but-set-variable was implemented in clang 13.0.0 and > -Wframe-address was implemented in clang 12.0.0 so the > cc-disable-warning calls are kept for these two warnings. > > Lastly, -Winitializer-overrides is clang's version of -Woverride-init, > which is disabled for the specific files that are problematic. clang > added a compatibility alias in clang 8.0.0 so -Winitializer-overrides > can be removed. > > [1]: https://lore.kernel.org/r/202108210311.cbtcgoul-...@intel.com/ > [2]: https://lore.kernel.org/r/20210824022640.2170859-1-nat...@kernel.org/ > > Reviewed-by: Nick Desaulniers > Signed-off-by: Nathan Chancellor Thanks for the patch, and sorry for the delay. Exceptionally pushed to drm-intel-gt-next instead of drm-intel-next because some of the dependencies such as 43192617f781 ("drm/i915: Enable -Wsometimes-uninitialized") were queued there too. BR, Jani. > --- > > v1 -> v2: > https://lore.kernel.org/r/20210824232237.2085342-1-nat...@kernel.org/ > > * Rebase on drm-intel-gt-next now that the prerequisite patch series has > been merged: https://lore.kernel.org/r/87wnnj13t5@intel.com/ > > * Add Nick's reviewed-by tag. > > drivers/gpu/drm/i915/Makefile | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index c584188aa15a..fd99374583d5 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -13,13 +13,11 @@ > # will most likely get a sudden build breakage... Hopefully we will fix > # new warnings before CI updates! > subdir-ccflags-y := -Wall -Wextra > -subdir-ccflags-y += $(call cc-disable-warning, unused-parameter) > -subdir-ccflags-y += $(call cc-disable-warning, type-limits) > -subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers) > +subdir-ccflags-y += -Wno-unused-parameter > +subdir-ccflags-y += -Wno-type-limits > +subdir-ccflags-y += -Wno-missing-field-initializers > +subdir-ccflags-y += -Wno-sign-compare > subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable) > -# clang warnings > -subdir-ccflags-y += $(call cc-disable-warning, sign-compare) > -subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides) > subdir-ccflags-y += $(call cc-disable-warning, frame-address) > subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror > > > base-commit: 43192617f7816bb74584c1df06f57363afd15337 -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v3 1/3] dt-bindings: msm: dsi: Add MSM8953 dsi phy
On Tue, 28 Sep 2021 18:49:27 +0530, Sireesh Kodali wrote: > SoCs based on the MSM8953 platform use the 14nm DSI PHY driver > > Signed-off-by: Sireesh Kodali > --- > Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring
Re: [PATCH 6/6] dt-bindings: gpu: Add ASPEED GFX bindings document
On Tue, 28 Sep 2021 10:57:03 +0800, tommy-huang wrote: > Add ast2600-gfx description for gfx driver. > > Signed-off-by: tommy-huang > --- > Documentation/devicetree/bindings/gpu/aspeed-gfx.txt | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring
Re: [PATCH] dt-bindings: display: simple: hardware can use ddc-i2c-bus
On Mon, 27 Sep 2021 23:45:03 +0200, David Heidelberg wrote: > Both hardware and driver can communicate DDC over i2c bus. > > Fixes warnings as: > arch/arm/boot/dts/tegra20-paz00.dt.yaml: panel: 'ddc-i2c-bus' does not match > any of the regexes: 'pinctrl-[0-9]+' > From schema: > /home/runner/work/linux/linux/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > > Signed-off-by: David Heidelberg > --- > .../devicetree/bindings/display/panel/panel-simple.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring
Re: [PATCH v3 1/2] dt-bindings: add bindings for the Sharp LS060T1SX01 panel
On Sun, 26 Sep 2021 03:10:04 +0300, Dmitry Baryshkov wrote: > Add devicetree bindings for the Sharp LS060T1SX01 6.0" FullHD panel > using NT35695 driver. This panel can be found i.e. in the Dragonboard > Display Adapter bundle. > > Signed-off-by: Dmitry Baryshkov > --- > .../display/panel/sharp,ls060t1sx01.yaml | 56 +++ > 1 file changed, 56 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/sharp,ls060t1sx01.yaml > Reviewed-by: Rob Herring
Re: [PATCH 2/2] dt-bindings: panel-simple-dsi: add JDI R63452 panel bindings
On Sat, 25 Sep 2021 12:31:33 +0200, Raffaele Tranquillini wrote: > This add the bindings for the JDI FHD_R63452 1080x1920 5.2" LCD DSI > panel used on the Xiaomi Mi 5 smartphone. > > Signed-off-by: Raffaele Tranquillini > --- > .../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring
Re: [PATCH net-next 2/5] dt-bindings: net: brcm,unimac-mdio: Add asp-v2.0
On Fri, 24 Sep 2021 14:44:48 -0700, Justin Chen wrote: > The ASP 2.0 Ethernet controller uses a brcm unimac. > > Signed-off-by: Justin Chen > Signed-off-by: Florian Fainelli > --- > Documentation/devicetree/bindings/net/brcm,unimac-mdio.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring
Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID
Hi Douglas, On Mon, Oct 4, 2021 at 6:22 PM Douglas Anderson wrote: > In the commit bac9c2948224 ("drm/edid: Break out reading block 0 of > the EDID") I broke out reading the base block of the EDID to its own > function. Unfortunately, when I did that I messed up the handling when > drm_edid_is_zero() indicated that we had an EDID that was all 0x00 or > when we went through 4 loops and didn't get a valid EDID. Specifically > I needed to pass the broken EDID to connector_bad_edid() but now I was > passing an error-pointer. > > Let's re-jigger things so we can pass the bad EDID in properly. > > Fixes: bac9c2948224 ("drm/edid: Break out reading block 0 of the EDID") > Reported-by: kernel test robot > Reported-by: Geert Uytterhoeven > Signed-off-by: Douglas Anderson The crash is was seeing is gone, so Tested-by: Geert Uytterhoeven Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v5 02/15] drm/edid: Break out reading block 0 of the EDID
Hi Doug, On Mon, Oct 4, 2021 at 6:26 PM Doug Anderson wrote: > On Mon, Oct 4, 2021 at 8:42 AM Geert Uytterhoeven > wrote: > > > - if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL) > > > + edid = (u8 *)drm_do_get_edid_base_block(get_edid_block, data, > > > + &connector->edid_corrupt, > > > + > > > &connector->null_edid_counter); > > > + if (IS_ERR_OR_NULL(edid)) { > > > + if (IS_ERR(edid)) > > > > So edid is an error code, not a valid pointer... > > > > > + connector_bad_edid(connector, edid, 1); > > > > ... while connector_bad_edid() expects edid to be a valid pointer, > > causing a crash: > > > > Unable to handle kernel NULL pointer dereference at virtual address > > Sigh. Thanks for the report and analysis. I guess I don't have any > displays reporting invalid EDIDs to test with. Hopefully this will > help: It doesn't happen all the time. Looks like my EDID is only invalid after a reset needed to resolve an s2ram crash in the adv7511 driver... > https://lore.kernel.org/r/20211004092100.1.Ic90a5ebd44c75db963112be167a03cc96f9fb249@changeid/ Thanks for the quick fix! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH] dt-bindings: drm/bridge: ti-sn65dsi86: Fix reg value
On Fri, 24 Sep 2021 14:35:12 +0200, Geert Uytterhoeven wrote: > make dtbs_check: > > arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dt.yaml: bridge@2c: > reg:0:0: 45 was expected > > According to the datasheet, the I2C address can be either 0x2c or 0x2d, > depending on the ADDR control input. > > Fixes: e3896e6dddf0b821 ("dt-bindings: drm/bridge: Document sn65dsi86 bridge > bindings") > Signed-off-by: Geert Uytterhoeven > --- > Also seen with the in-flight Falcon DSI display output patch: > > arch/arm64/boot/dts/renesas/r8a779a0-falcon.dt.yaml: sn65dsi86@2c: > reg:0:0: 45 was expected > --- > .../devicetree/bindings/display/bridge/ti,sn65dsi86.yaml| 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Applied, thanks!
Re: [PATCH v5 02/15] drm/edid: Break out reading block 0 of the EDID
Hi, On Mon, Oct 4, 2021 at 8:42 AM Geert Uytterhoeven wrote: > > > - if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL) > > + edid = (u8 *)drm_do_get_edid_base_block(get_edid_block, data, > > + &connector->edid_corrupt, > > + > > &connector->null_edid_counter); > > + if (IS_ERR_OR_NULL(edid)) { > > + if (IS_ERR(edid)) > > So edid is an error code, not a valid pointer... > > > + connector_bad_edid(connector, edid, 1); > > ... while connector_bad_edid() expects edid to be a valid pointer, > causing a crash: > > Unable to handle kernel NULL pointer dereference at virtual address Sigh. Thanks for the report and analysis. I guess I don't have any displays reporting invalid EDIDs to test with. Hopefully this will help: https://lore.kernel.org/r/20211004092100.1.Ic90a5ebd44c75db963112be167a03cc96f9fb249@changeid/ -Doug
[PATCH] drm/edid: Fix crash with zero/invalid EDID
In the commit bac9c2948224 ("drm/edid: Break out reading block 0 of the EDID") I broke out reading the base block of the EDID to its own function. Unfortunately, when I did that I messed up the handling when drm_edid_is_zero() indicated that we had an EDID that was all 0x00 or when we went through 4 loops and didn't get a valid EDID. Specifically I needed to pass the broken EDID to connector_bad_edid() but now I was passing an error-pointer. Let's re-jigger things so we can pass the bad EDID in properly. Fixes: bac9c2948224 ("drm/edid: Break out reading block 0 of the EDID") Reported-by: kernel test robot Reported-by: Geert Uytterhoeven Signed-off-by: Douglas Anderson --- drivers/gpu/drm/drm_edid.c | 27 +++ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 9b19eee0e1b4..9c9463ec5465 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1911,13 +1911,15 @@ int drm_add_override_edid_modes(struct drm_connector *connector) } EXPORT_SYMBOL(drm_add_override_edid_modes); -static struct edid *drm_do_get_edid_base_block( +static struct edid *drm_do_get_edid_base_block(struct drm_connector *connector, int (*get_edid_block)(void *data, u8 *buf, unsigned int block, size_t len), - void *data, bool *edid_corrupt, int *null_edid_counter) + void *data) { - int i; + int *null_edid_counter = connector ? &connector->null_edid_counter : NULL; + bool *edid_corrupt = connector ? &connector->edid_corrupt : NULL; void *edid; + int i; edid = kmalloc(EDID_LENGTH, GFP_KERNEL); if (edid == NULL) @@ -1941,9 +1943,8 @@ static struct edid *drm_do_get_edid_base_block( return edid; carp: - kfree(edid); - return ERR_PTR(-EINVAL); - + if (connector) + connector_bad_edid(connector, edid, 1); out: kfree(edid); return NULL; @@ -1982,14 +1983,9 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, if (override) return override; - edid = (u8 *)drm_do_get_edid_base_block(get_edid_block, data, - &connector->edid_corrupt, - &connector->null_edid_counter); - if (IS_ERR_OR_NULL(edid)) { - if (IS_ERR(edid)) - connector_bad_edid(connector, edid, 1); + edid = (u8 *)drm_do_get_edid_base_block(connector, get_edid_block, data); + if (!edid) return NULL; - } /* if there's no extensions or no connector, we're done */ valid_extensions = edid[0x7e]; @@ -2142,14 +2138,13 @@ u32 drm_edid_get_panel_id(struct i2c_adapter *adapter) struct edid *edid; u32 panel_id; - edid = drm_do_get_edid_base_block(drm_do_probe_ddc_edid, adapter, - NULL, NULL); + edid = drm_do_get_edid_base_block(NULL, drm_do_probe_ddc_edid, adapter); /* * There are no manufacturer IDs of 0, so if there is a problem reading * the EDID then we'll just return 0. */ - if (IS_ERR_OR_NULL(edid)) + if (!edid) return 0; panel_id = edid_extract_panel_id(edid); -- 2.33.0.800.g4c38ced690-goog
Re: [PATCH] drm/msm/a6xx: Serialize GMU communication
On 10/2/2021 1:02 AM, Rob Clark wrote: From: Rob Clark I've seen some crashes in our crash reporting that *look* like multiple threads stomping on each other while communicating with GMU. So wrap all those paths in a lock. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 3 ++ drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 40 +++ 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index a7c58018959f..8b73f70766a4 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -296,6 +296,8 @@ int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state) u32 val; int request, ack; + WARN_ON_ONCE(!mutex_is_locked(&gmu->lock)); + if (state >= ARRAY_SIZE(a6xx_gmu_oob_bits)) return -EINVAL; @@ -337,6 +339,8 @@ void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state) { int bit; + WARN_ON_ONCE(!mutex_is_locked(&gmu->lock)); + if (state >= ARRAY_SIZE(a6xx_gmu_oob_bits)) return; @@ -1482,6 +1486,8 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node) if (!pdev) return -ENODEV; + mutex_init(&gmu->lock); + gmu->dev = &pdev->dev; of_dma_configure(gmu->dev, node, true); diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h index 3c74f64e3126..84bd516f01e8 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h @@ -44,6 +44,9 @@ struct a6xx_gmu_bo { struct a6xx_gmu { struct device *dev; + /* For serializing communication with the GMU: */ + struct mutex lock; + struct msm_gem_address_space *aspace; void * __iomem mmio; diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index f6a4dbef796b..bd7bdeff5d6f 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -881,7 +881,7 @@ static int a6xx_zap_shader_init(struct msm_gpu *gpu) A6XX_RBBM_INT_0_MASK_UCHE_OOB_ACCESS | \ A6XX_RBBM_INT_0_MASK_UCHE_TRAP_INTR) -static int a6xx_hw_init(struct msm_gpu *gpu) +static int hw_init(struct msm_gpu *gpu) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); @@ -1135,6 +1135,19 @@ static int a6xx_hw_init(struct msm_gpu *gpu) return ret; } +static int a6xx_hw_init(struct msm_gpu *gpu) +{ + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); + int ret; + + mutex_lock(&a6xx_gpu->gmu.lock); + ret = hw_init(gpu); + mutex_unlock(&a6xx_gpu->gmu.lock); + + return ret; +} + static void a6xx_dump(struct msm_gpu *gpu) { DRM_DEV_INFO(&gpu->pdev->dev, "status: %08x\n", @@ -1509,7 +1522,9 @@ static int a6xx_pm_resume(struct msm_gpu *gpu) trace_msm_gpu_resume(0); + mutex_lock(&a6xx_gpu->gmu.lock); ret = a6xx_gmu_resume(a6xx_gpu); + mutex_unlock(&a6xx_gpu->gmu.lock); if (ret) return ret; @@ -1532,7 +1547,9 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu) msm_devfreq_suspend(gpu); + mutex_lock(&a6xx_gpu->gmu.lock); ret = a6xx_gmu_stop(a6xx_gpu); + mutex_unlock(&a6xx_gpu->gmu.lock); if (ret) return ret; @@ -1547,18 +1564,19 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); - static DEFINE_MUTEX(perfcounter_oob); - mutex_lock(&perfcounter_oob); + mutex_lock(&a6xx_gpu->gmu.lock); /* Force the GPU power on so we can read this register */ a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET); *value = gpu_read64(gpu, REG_A6XX_CP_ALWAYS_ON_COUNTER_LO, - REG_A6XX_CP_ALWAYS_ON_COUNTER_HI); + REG_A6XX_CP_ALWAYS_ON_COUNTER_HI); a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET); - mutex_unlock(&perfcounter_oob); + + mutex_unlock(&a6xx_gpu->gmu.lock); + return 0; } @@ -1622,6 +1640,16 @@ static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu) return (unsigned long)busy_time; } +void a6xx_gpu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp) +{ + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); + + mutex_lock(&a6xx_gpu->gmu.lock); + a6xx_gmu_set_freq(gpu, opp); + mutex_unlock(&a6xx_gpu->gmu.lock); +} + static struct msm_gem_address_space * a6xx_create_address_space(struct msm_gpu *g