RE: [PATCH v3 0/2] Add RZ/G2L DSI driver
Hi David and Laurent,
Gentle Ping.
Are you happy with this series?
Cheers,
Biju
> Subject: RE: [PATCH v3 0/2] Add RZ/G2L DSI driver
>
> Hi All,
>
> Gentle ping.
>
> Are you ok with this patch series? Please let me know.
>
> Cheers,
> Biju
>
> > Subject: [PATCH v3 0/2] Add RZ/G2L DSI driver
> >
> > This patch series aims to support the MIPI DSI encoder found in the
> > RZ/G2L SoC. It currently supports DSI mode only.
> >
> > This unit supports MIPI Alliance Specification for Display Serial
> > Interface
> > (DSI) Specification. This unit provides a solution for transmitting
> > MIPI DSI compliant digital video and packets. Normative References are
> below.
> > * MIPI Alliance Specification for Display Serial Interface Version
> > 1.3.1
> > * MIPI Alliance Specification for D-PHY Version 2.1
> >
> > The following are key features of this unit.
> >
> > * 1 channel
> > * The number of Lane: 4-lane
> > * Support up to Full HD (1920 × 1080), 60 fps (RGB888)
> > * Maximum Bandwidth: 1.5 Gbps per lane
> > * Support Output Data Format: RGB666 / RGB888
> >
> > v2->v3:
> > * Added Rb tag from Geert and Laurent for the binding patch.
> > * Fixed the typo "Receive" -> "transmit"
> > * Added accepible values for data-lanes
> > * Sorted Header file in the example
> > * Added SoC specific compaible along with generic one.
> > * pass rzg2l_mipi_dsi pointer to {Link,Phy} register rd/wr function
> > instead
> >of the memory pointer
> > * Fixed the comment in rzg2l_mipi_dsi_startup()
> > * Removed unnecessary dbg message from rzg2l_mipi_dsi_start_video()
> > * DRM bridge parameter initialization moved to probe
> > * Replaced dev_dbg->dev_err in rzg2l_mipi_dsi_parse_dt()
> > * Inserted the missing blank lane after return in probe()
> > * Added missing MODULE_DEVICE_TABLE
> > * Added include linux/bits.h in header file
> > * Fixed various macros in header file.
> > * Reorder the make file for DSI, so that it is no more dependent
> >on RZ/G2L DU patch series.
> > v1->v2:
> > * Added full path for dsi-controller.yaml
> > * Modeled DSI + D-PHY as single block and updated reg property
> > * Fixed typo D_PHY->D-PHY
> > * Updated description
> > * Added interrupts and interrupt-names and updated the example
> > * Driver rework based on dt-binding changes (DSI + D-PHY) as single
> > block
> > * Replaced link_mmio and phy_mmio with mmio in struct rzg2l_mipi_dsi
> > * Replaced rzg2l_mipi_phy_write with rzg2l_mipi_dsi_phy_write
> >and rzg2l_mipi_dsi_link_write
> > * Replaced rzg2l_mipi_phy_read->rzg2l_mipi_dsi_link_read
> > RFC->v1:
> > * Added a ref to dsi-controller.yaml.
> > * Added "depends on ARCH_RENESAS || COMPILE_TEST" on KCONFIG
> >and dropped DRM as it is implied by DRM_BRIDGE
> > * Used devm_reset_control_get_exclusive() for reset handle
> > * Removed bool hsclkmode from struct rzg2l_mipi_dsi
> > * Added error check for pm, using pm_runtime_resume_and_get() instead
> of
> >pm_runtime_get_sync()
> > * Added check for unsupported formats in rzg2l_mipi_dsi_host_attach()
> > * Avoided read-modify-write stopping hsclock
> > * Used devm_platform_ioremap_resource for resource allocation
> > * Removed unnecessary assert call from probe and remove.
> > * wrap the line after the PTR_ERR() in probe()
> > * Updated reset failure messages in probe
> > * Fixed the typo arstc->prstc
> > * Made hex constants to lower case.
> > RFC:
> > *
> >
> > Biju Das (2):
> > dt-bindings: display: bridge: Document RZ/G2L MIPI DSI TX bindings
> > drm: rcar-du: Add RZ/G2L DSI driver
> >
> > .../bindings/display/bridge/renesas,dsi.yaml | 182 +
> > drivers/gpu/drm/rcar-du/Kconfig | 8 +
> > drivers/gpu/drm/rcar-du/Makefile | 2 +
> > drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c | 690 ++
> > drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h | 151
> > 5 files changed, 1033 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/bridge/renesas,dsi.yaml
> > create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> > create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h
> >
> > --
> > 2.25.1
Re: [PATCH v4 13/13] video: backlight: mt6370: Add Mediatek MT6370 support
Andy Shevchenko 於 2022年7月13日 週三 晚上8:07寫道： > > On Wed, Jul 13, 2022 at 12:53 PM ChiaEn Wu wrote: > > Andy Shevchenko 於 2022年7月5日 週二 清晨5:14寫道： > > > On Mon, Jul 4, 2022 at 7:43 AM ChiaEn Wu wrote: > > Please, remove unneeded context when replying! > > ... > > > > > + brightness_val[0] = (brightness - 1) & > > > > MT6370_BL_DIM2_MASK; > > > > + brightness_val[1] = (brightness - 1) > > > > + >> fls(MT6370_BL_DIM2_MASK); > > > > > > Bad indentation. One line? > > > > Well... if indent to one line, it will be over 80 characters(or called > > columns?) > > From my understanding, it is not allowed, right?? > > It's allowed to some extent.Use your common sense. > Here it's obviously broken indentation. > > ... > > > > > + prop_val = (ilog2(roundup_pow_of_two(prop_val)) + 1) >> > > > > 1; > > > > > > Isn't something closer to get_order() or fls()? > > > > I will revise it to "(get_order(prop_va * PAGE_SIZE) + 1) / 2" and > > this change is meet your expectations?? > > Nope. Try again. What about fls()? I have tried two methods so far, as follows - /* * prop_val = 1 --> 1 steps --> b'00 * prop_val = 2 ~ 4 --> 4 steps --> b'01 * prop_val = 5 ~ 16 --> 16 steps --> b'10 * prop_val = 17 ~ 64 --> 64 steps --> b'11 */ // 1. use fls() and ffs() combination prop_val = ffs(prop_val) == fls(prop_val) ? fls(prop_val) >> 1 : (fls(prop_val) + 1) >> 1; // 2. use one line for-loop, but without fls() for (i = --prop_val, prop_val = 0; i >> 2 * prop_val != 0; prop_val++); - Do these changes meet your expectations?? > > ... > > > > > + props->max_brightness = min_t(u32, brightness, > > > > + MT6370_BL_MAX_BRIGHTNESS); > > > > > > One line? > > > > Ditto, it will be over 80 characters... > > As per above. > > -- > With Best Regards, > Andy Shevchenko
[PULL] drm-misc-fixes
Hi Dave, Daniel, Here's this week drm-misc-fixes PR Maxime drm-misc-fixes-2022-07-14: Only a revert for amdgpu reverting the switch to the drm buddy allocator. The following changes since commit b68277f19e31a25312c4acccadb5cf1502e52e84: drm/ssd130x: Fix pre-charge period setting (2022-07-07 10:52:03 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2022-07-14 for you to fetch changes up to 925b6e59138cefa47275c67891c65d48d3266d57: Revert "drm/amdgpu: add drm buddy support to amdgpu" (2022-07-08 14:24:30 +0200) Only a revert for amdgpu reverting the switch to the drm buddy allocator. Arunpravin Paneer Selvam (1): Revert "drm/amdgpu: add drm buddy support to amdgpu" drivers/gpu/drm/Kconfig| 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 97 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h| 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 359 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 89 -- 5 files changed, 176 insertions(+), 380 deletions(-) delete mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h signature.asc Description: PGP signature
Re: [PATCH] drm/imx/dcss: Add missing of_node_put() in fail path
Hi Liang,
Thanks for the patch.
The patch is ok but, since you're at it, maybe add of_node_put() in the
dcss_dev_destroy() too?
Thanks,
laurentiu
On Thu, Jul 07, 2022 at 10:32:14AM +0800, Liang He wrote:
> In dcss_dev_create(), we should call of_node_put() in fail path for
> of_graph_get_port_by_id() which will increase the refcount.
>
> Fixes: 9021c317b770 ("drm/imx: Add initial support for DCSS on iMX8MQ")
> Signed-off-by: Liang He
> ---
> drivers/gpu/drm/imx/dcss/dcss-dev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/imx/dcss/dcss-dev.c
> b/drivers/gpu/drm/imx/dcss/dcss-dev.c
> index c849533ca83e..a99141538621 100644
> --- a/drivers/gpu/drm/imx/dcss/dcss-dev.c
> +++ b/drivers/gpu/drm/imx/dcss/dcss-dev.c
> @@ -207,6 +207,7 @@ struct dcss_dev *dcss_dev_create(struct device *dev, bool
> hdmi_output)
>
> ret = dcss_submodules_init(dcss);
> if (ret) {
> + of_node_put(dcss->of_port);
> dev_err(dev, "submodules initialization failed\n");
> goto clks_err;
> }
> --
> 2.25.1
>
Re: [PATCH v2 5/8] drm/vboxvideo: Use the hotspot properties from cursor planes
On Wed, 13 Jul 2022 13:35:56 + Zack Rusin wrote: > On Wed, 2022-07-13 at 10:20 +0300, Pekka Paalanen wrote: > > On Wed, 13 Jul 2022 03:35:57 + > > Zack Rusin wrote: > > > > > On Tue, 2022-07-12 at 10:56 +0300, Pekka Paalanen wrote: > > > > On Mon, 11 Jul 2022 23:32:43 -0400 > > > > Zack Rusin wrote: > > > > > > > > > From: Zack Rusin > > > > > > > > > > Atomic modesetting got support for mouse hotspots via the hotspot > > > > > properties. Port the legacy kms hotspot handling to the new properties > > > > > on cursor planes. > > > > > > > > > > Signed-off-by: Zack Rusin > > > > > Cc: Hans de Goede > > > > > Cc: David Airlie > > > > > Cc: Daniel Vetter > > > > > --- > > > > > drivers/gpu/drm/vboxvideo/vbox_mode.c | 4 ++-- > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c > > > > > b/drivers/gpu/drm/vboxvideo/vbox_mode.c > > > > > index fa0d73ce07bc..ca3134adb104 100644 > > > > > --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c > > > > > +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c > > > > > @@ -429,8 +429,8 @@ static void vbox_cursor_atomic_update(struct > > > > > drm_plane *plane, > > > > > flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE | > > > > > VBOX_MOUSE_POINTER_ALPHA; > > > > > hgsmi_update_pointer_shape(vbox->guest_pool, flags, > > > > > -min_t(u32, max(fb->hot_x, 0), width), > > > > > -min_t(u32, max(fb->hot_y, 0), > > > > > height), > > > > > +min_t(u32, max(new_state->hotspot_x, > > > > > 0), width), > > > > > +min_t(u32, max(new_state->hotspot_y, > > > > > 0), height), > > > > > width, height, vbox->cursor_data, > > > > > data_size); > > > > > > > > > > mutex_unlock(&vbox->hw_mutex); > > > > > > > > Hi, > > > > > > > > this looks like silent clamping of the hotspot coordinates. > > > > > > > > Should the DRM core perhaps already ensure that the hotspot must reside > > > > inside the plane (fb) boundaries and reject the atomic (TEST_ONLY) > > > > commit when it's outside? > > > > > > > > Or if this restriction is not universal, maybe this driver should > > > > reject invalid hotspots rather than silently mangle them? > > > > > > TBH, I'm not really sure why vboxvideo is clamping those values. I didn't > > > want to > > > introduce any regressions by changing it here, but the hotspot code never > > > specified > > > that the hotspot offsets have to be positive or even within the plane. In > > > a quick > > > search I couldn't find real world cursors that were doing anything like > > > that though > > > so I just left it. > > > > > > > Also, if supports_virtual_cursor_plane is false, should there not be > > > > somewhere code that will ignore the values set to the atomic hotspot > > > > properties? > > > > > > Hmm, good question. I'm not sure if there's a case where that could be > > > possible: > > > without supports_virtual_cursor we either won't have a cursor plane or > > > the client > > > won't be atomic and will only be allowed to use the old legacy kms > > > ioctl's, i.e. > > > drmModeSetCursor2. > > > > > > > When one KMS client switches to another, the first one being hotspot > > > > aware and the latter not, and both atomic, then the latter KMS client > > > > who doesn't know to drive the hotspot will inherit potentially invalid > > > > hotspot from the previous KMS client. Does something prevent that from > > > > being a problem? > > > > > > That switch will result in plane state reset, ending in > > > __drm_atomic_helper_plane_state_reset which will reset both hotspot > > > properties (set > > > them to 0). > > > > It will? > > > > To my knowledge, KMS has never reset anything when one KMS client > > switches to the next, so that's new. > > > > What triggers it? > > > > Only if you actually switch to fbdev/fbcon instead of another userspace > > KMS client, the fbdev code might reset some things, but not all. > > > > > > The old KMS client may have left the virtual cursor plane visible, and > > > > the new KMS client doesn't even know the virtual cursor plane exists > > > > because it doesn't set the DRM client cap. Something should probably > > > > ensure the virtual cursor plane gets disabled, right? > > > > > > Hah, that's also a good question. I *think* the same code to above would > > > be ran, > > > i.e. plane reset which should result in the plane disappearing and the > > > new client > > > not being able to drive it anymore. At least when running gnome-shell, > > > switching to > > > weston and then switching to gnome-shell things look ok, but I haven't > > > tried running > > > such clients at the same time. > > > > That's an interesting experiment, but how is "at the same time" > > different from what you
Re:Re: [PATCH] drm/imx/dcss: Add missing of_node_put() in fail path
At 2022-07-14 15:37:41, "Laurentiu Palcu" wrote:
>Hi Liang,
>
>Thanks for the patch.
>
>The patch is ok but, since you're at it, maybe add of_node_put() in the
>dcss_dev_destroy() too?
>
Thanks, laurentiu,
I miss it and I will add it soon.
>Thanks,
>laurentiu
>
>On Thu, Jul 07, 2022 at 10:32:14AM +0800, Liang He wrote:
>> In dcss_dev_create(), we should call of_node_put() in fail path for
>> of_graph_get_port_by_id() which will increase the refcount.
>>
>> Fixes: 9021c317b770 ("drm/imx: Add initial support for DCSS on iMX8MQ")
>> Signed-off-by: Liang He
>> ---
>> drivers/gpu/drm/imx/dcss/dcss-dev.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/imx/dcss/dcss-dev.c
>> b/drivers/gpu/drm/imx/dcss/dcss-dev.c
>> index c849533ca83e..a99141538621 100644
>> --- a/drivers/gpu/drm/imx/dcss/dcss-dev.c
>> +++ b/drivers/gpu/drm/imx/dcss/dcss-dev.c
>> @@ -207,6 +207,7 @@ struct dcss_dev *dcss_dev_create(struct device *dev,
>> bool hdmi_output)
>>
>> ret = dcss_submodules_init(dcss);
>> if (ret) {
>> +of_node_put(dcss->of_port);
>> dev_err(dev, "submodules initialization failed\n");
>> goto clks_err;
>> }
>> --
>> 2.25.1
>>
[PATCH v2] drm/imx/dcss: Add missing of_node_put() in fail path
In dcss_dev_create() and dcss_dev_destroy(), we should call of_node_put()
in fail path or before the dcss's destroy as of_graph_get_port_by_id() has
increased the refcount.
Fixes: 9021c317b770 ("drm/imx: Add initial support for DCSS on iMX8MQ")
Signed-off-by: Liang He
---
changelog:
v2: add of_node_put() in dcss_dev_destroy() advised by Laurentiu
v1: only fix bug in dcss_dev_create().
v1 link: https://lore.kernel.org/all/20220707023214.307451-1-win...@126.com/
drivers/gpu/drm/imx/dcss/dcss-dev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/imx/dcss/dcss-dev.c
b/drivers/gpu/drm/imx/dcss/dcss-dev.c
index c849533ca83e..3f5750cc2673 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-dev.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-dev.c
@@ -207,6 +207,7 @@ struct dcss_dev *dcss_dev_create(struct device *dev, bool
hdmi_output)
ret = dcss_submodules_init(dcss);
if (ret) {
+ of_node_put(dcss->of_port);
dev_err(dev, "submodules initialization failed\n");
goto clks_err;
}
@@ -237,6 +238,8 @@ void dcss_dev_destroy(struct dcss_dev *dcss)
dcss_clocks_disable(dcss);
}
+ of_node_put(dcss->of_port);
+
pm_runtime_disable(dcss->dev);
dcss_submodules_stop(dcss);
--
2.25.1
Re: [PATCH v2 0/11] drm/via: Make via a single file driver
Hi Sam, Acked-by: Thomas Zimmermann for the new changes. Best regards Thomas Am 13.07.22 um 19:01 schrieb Sam Ravnborg: We have an upcoming openchrome driver for VIA where some of the files conflicts with the existing via driver. It is not acceptable to just delete the existing DRI1 based driver as there are most likely users out there, so a different approach was required. It was disccussed what to do and the least invasive solution was to keep the DRI1 driver in the current directory as a single file. Thomas Zimmermann already posted a patch to do the same but it attemped to have a single driver for the DRI1 and the upcoming new driver. This patchset embeds the files one by one to make the diffs remotely readable and end up with an independent DRI1 driver. The driver was built tested for each step. v2: - Drop the rename of the driver - keep the name via. We can name the new driver viakms or openchrome so there is no confusion in userspace if the old or the new driver is used. - Add a few patches to make via_3d_reg more readable, which has the nice side-effect that it is now checkpatch clean. - Added Kevin as cc: on all patches This set of patches should make it simpler to add the new openchrome driver, and I am happy to assist if there are open questions how to do it. Note: The patches has seen zero run-time testing - I only know they builds in my setup (for several archs). Sam Sam Ravnborg (13): drm/via: Rename via_drv to via_dri1 drm/via: Embed via_dma in via_dri1 drm/via: Embed via_map in via_dri1 drm/via: Embed via_mm in via_dri1 drm/via: Embed via_video in via_dri1 drm/via: Embed via_irq in via_dri1 drm/via: Embed via_dmablit in via_dri1 drm/via: Embed via_verifier in via_dri1 drm/via: Embed via_drv.h in via_dri1 drm/via: Update to the latest via_3d_reg header drm/via: Use SPDX tag for MIT license in via_3d_reg header drm/via: Make macros readable in the via_3d_reg header drm/via: Fix style issues in via_3d_reg header drivers/gpu/drm/via/Makefile |2 +- drivers/gpu/drm/via/via_3d_reg.h | 349 ++-- drivers/gpu/drm/via/via_dma.c | 744 drivers/gpu/drm/via/via_dmablit.c | 807 drivers/gpu/drm/via/via_dmablit.h | 140 -- drivers/gpu/drm/via/via_dri1.c | 3630 drivers/gpu/drm/via/via_drv.c | 124 -- drivers/gpu/drm/via/via_drv.h | 229 --- drivers/gpu/drm/via/via_irq.c | 388 drivers/gpu/drm/via/via_map.c | 132 -- drivers/gpu/drm/via/via_mm.c | 241 --- drivers/gpu/drm/via/via_verifier.c | 1110 --- drivers/gpu/drm/via/via_verifier.h | 62 - drivers/gpu/drm/via/via_video.c| 94 - 14 files changed, 3866 insertions(+), 4186 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
PSA: drm-misc-next-fixes is open
Hi, the email goes out a bit late, as -rc6 has already been tagged for a few days. This means that drm-misc-next-fixes is now open for bug fixes, as drm-next is in feature freeze until the next -rc1 comes out. Some rules of thumb: * if your patch fixes a bug in upstream, please put it into drm-misc-fixes, * if your patch fixes a bug in drm-next, please put it into drm-misc-next-fixes, * anything else should go into drm-misc-next. The flow chart is at [1]. The transition from/to drm-misc-fixes-next sometimes results in patches that are applied to the wrong tree and get stuck there for a long time. If you have fixes in drm-misc-next that must go into drm-next soon, please cherry-pick them into drm-misc-next-fixes. We have dim cherry-pick to help you with that. Best regards Thomas [1] https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] dma-buf: revert "return only unsignaled fences in dma_fence_unwrap_for_each v3"
Hi Christian
Am 12.07.22 um 12:28 schrieb Christian König:
This reverts commit 8f61973718485f3e89bc4f408f929048b7b47c83.
I only found this commit in drm-misc-next. Should the revert be
cherry-picked into drm-misc-next-fixes?
Best regards
Thomas
It turned out that this is not correct. Especially the sync_file info
IOCTL needs to see even signaled fences to correctly report back their
status to userspace.
Instead add the filter in the merge function again where it makes sense.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-fence-unwrap.c | 3 ++-
include/linux/dma-fence-unwrap.h | 6 +-
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-unwrap.c
b/drivers/dma-buf/dma-fence-unwrap.c
index 502a65ea6d44..7002bca792ff 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -72,7 +72,8 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned int
num_fences,
count = 0;
for (i = 0; i < num_fences; ++i) {
dma_fence_unwrap_for_each(tmp, &iter[i], fences[i])
- ++count;
+ if (!dma_fence_is_signaled(tmp))
+ ++count;
}
if (count == 0)
diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
index 390de1ee9d35..66b1e56fbb81 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -43,14 +43,10 @@ struct dma_fence *dma_fence_unwrap_next(struct
dma_fence_unwrap *cursor);
* Unwrap dma_fence_chain and dma_fence_array containers and deep dive into
all
* potential fences in them. If @head is just a normal fence only that one is
* returned.
- *
- * Note that signalled fences are opportunistically filtered out, which
- * means the iteration is potentially over no fence at all.
*/
#define dma_fence_unwrap_for_each(fence, cursor, head)
\
for (fence = dma_fence_unwrap_first(head, cursor); fence; \
-fence = dma_fence_unwrap_next(cursor)) \
- if (!dma_fence_is_signaled(fence))
+fence = dma_fence_unwrap_next(cursor))
struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
struct dma_fence **fences,
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH 4/5] drm/modes: Add support for driver-specific named modes
Hi Maxime, On Wed, Jul 13, 2022 at 11:37 AM Maxime Ripard wrote: > On Mon, Jul 11, 2022 at 02:08:06PM +0200, Geert Uytterhoeven wrote: > > On Mon, Jul 11, 2022 at 2:02 PM Maxime Ripard wrote: > > > On Mon, Jul 11, 2022 at 01:59:28PM +0200, Geert Uytterhoeven wrote: > > > > On Mon, Jul 11, 2022 at 1:42 PM Maxime Ripard wrote: > > > > > On Mon, Jul 11, 2022 at 01:11:14PM +0200, Thomas Zimmermann wrote: > > > > > > Am 11.07.22 um 11:35 schrieb Maxime Ripard: > > > > > > > On Mon, Jul 11, 2022 at 11:03:38AM +0200, Thomas Zimmermann wrote: > > > > > > > > Am 08.07.22 um 20:21 schrieb Geert Uytterhoeven: > > > > > > > > > The mode parsing code recognizes named modes only if they are > > > > > > > > > explicitly > > > > > > > > > listed in the internal whitelist, which is currently limited > > > > > > > > > to "NTSC" > > > > > > > > > and "PAL". > > > > > > > > > > > > > > > > > > Provide a mechanism for drivers to override this list to > > > > > > > > > support custom > > > > > > > > > mode names. > > > > > > > > > > > > > > > > > > Ideally, this list should just come from the driver's actual > > > > > > > > > list of > > > > > > > > > modes, but connector->probed_modes is not yet populated at > > > > > > > > > the time of > > > > > > > > > parsing. > > > > > > > > > > > > > > > > I've looked for code that uses these names, couldn't find any. > > > > > > > > How is this > > > > > > > > being used in practice? For example, if I say "PAL" on the > > > > > > > > command line, is > > > > > > > > there DRM code that fills in the PAL mode parameters? > > > > > > > > > > > > > > We have some code to deal with this in sun4i: > > > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/sun4i/sun4i_tv.c#L292 > > > > > > > > > > > > > > It's a bit off topic, but for TV standards, I'm still not sure > > > > > > > what the > > > > > > > best course of action is. There's several interactions that make > > > > > > > this a > > > > > > > bit troublesome: > > > > > > > > > > > > > >* Some TV standards differ by their mode (ie, PAL vs NSTC), > > > > > > > but some > > > > > > > other differ by parameters that are not part of > > > > > > > drm_display_mode > > > > > > > (NTSC vs NSTC-J where the only difference is the black and > > > > > > > blanking > > > > > > > signal levels for example). > > > > > > > > > > > > > >* The mode names allow to provide a fairly convenient way to > > > > > > > add that > > > > > > > extra information, but the userspace is free to create its > > > > > > > own mode > > > > > > > and might omit the mode name entirely. > > > > > > > > > > > > > > So in the code above, if the name has been preserved we match by > > > > > > > name, > > > > > > > but we fall back to matching by mode if it hasn't been, which in > > > > > > > this > > > > > > > case means that we have no way to differentiate between NTSC, > > > > > > > NTSC-J, > > > > > > > PAL-M in this case. > > > > > > > > > > > > > > We have some patches downstream for the RaspberryPi that has the > > > > > > > TV > > > > > > > standard as a property. There's a few extra logic required for the > > > > > > > userspace (like setting the PAL property, with the NTSC mode) so > > > > > > > I'm not > > > > > > > sure it's preferable. > > > > > > > > > > > > > > Or we could do something like a property to try that standard, and > > > > > > > another that reports the one we actually chose. > > > > > > > > > > > > > > > And another question I have is whether this whitelist belongs > > > > > > > > into the > > > > > > > > driver at all. Standard modes exist independent from drivers or > > > > > > > > hardware. > > > > > > > > Shouldn't there simply be a global list of all possible mode > > > > > > > > names? Drivers > > > > > > > > would filter out the unsupported modes anyway. > > > > > > > > > > > > > > We should totally do something like that, yeah > > > > > > > > > > > > That sun code already looks like sometihng the DRM core/helpers > > > > > > should be > > > > > > doing. And if we want to support named modes well, there's a long > > > > > > list of > > > > > > modes in Wikipedia. > > > > > > > > > > > > https://en.wikipedia.org/wiki/Video_Graphics_Array#/media/File:Vector_Video_Standards2.svg > > > > > > > > > > Yeah, and NTSC is missing :) > > > > > > > > And that diagram is about the "digital" variant of PAL. > > > > If you go the analog route, the only fixed parts are vfreq/hfreq, > > > > number of lines, and synchronization. Other parameters like overscan > > > > can vary. The actual dot clock can vary wildly: while there is an > > > > upper limit due to bandwidth limitations, you can come up with an > > > > almost infinite number of video modes that can be called PAL, which > > > > is one of the reasons why I don't want hardware-specific variants to > > > > end up in a global video mode database. > > > > > > Do you have an example of what that would look like? > > > > You mea
Re: [PATCH 1/3] Revert "drm/amdgpu: move internal vram_mgr function into the C file"
Hi
Am 08.07.22 um 11:30 schrieb Arunpravin Paneer Selvam:
This reverts commit 708d19d9f362766147cab79eccae60912c6d3068.
This commit is only present in drm-misc-next. Should the revert be
cherry-picked into drm-misc-next-fixes?
Best regards
Thomas
This is part of a revert of the following commits:
commit 708d19d9f362 ("drm/amdgpu: move internal vram_mgr function into the C
file")
commit 5e3f1e7729ec ("drm/amdgpu: fix start calculation in amdgpu_vram_mgr_new")
commit c9cad937c0c5 ("drm/amdgpu: add drm buddy support to amdgpu")
[WHY]
Few users reported garbaged graphics as soon as x starts,
reverting until this can be resolved.
Signed-off-by: Arunpravin Paneer Selvam
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 29
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 27 ++
2 files changed, 27 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 7a5e8a7b4a1b..51d9d3a4456c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -50,35 +50,6 @@ to_amdgpu_device(struct amdgpu_vram_mgr *mgr)
return container_of(mgr, struct amdgpu_device, mman.vram_mgr);
}
-static inline struct drm_buddy_block *
-amdgpu_vram_mgr_first_block(struct list_head *list)
-{
- return list_first_entry_or_null(list, struct drm_buddy_block, link);
-}
-
-static inline bool amdgpu_is_vram_mgr_blocks_contiguous(struct list_head *head)
-{
- struct drm_buddy_block *block;
- u64 start, size;
-
- block = amdgpu_vram_mgr_first_block(head);
- if (!block)
- return false;
-
- while (head != block->link.next) {
- start = amdgpu_vram_mgr_block_start(block);
- size = amdgpu_vram_mgr_block_size(block);
-
- block = list_entry(block->link.next, struct drm_buddy_block,
link);
- if (start + size != amdgpu_vram_mgr_block_start(block))
- return false;
- }
-
- return true;
-}
-
-
-
/**
* DOC: mem_info_vram_total
*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
index 4b267bf1c5db..9a2db87186c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
@@ -53,6 +53,33 @@ static inline u64 amdgpu_vram_mgr_block_size(struct
drm_buddy_block *block)
return PAGE_SIZE << drm_buddy_block_order(block);
}
+static inline struct drm_buddy_block *
+amdgpu_vram_mgr_first_block(struct list_head *list)
+{
+ return list_first_entry_or_null(list, struct drm_buddy_block, link);
+}
+
+static inline bool amdgpu_is_vram_mgr_blocks_contiguous(struct list_head *head)
+{
+ struct drm_buddy_block *block;
+ u64 start, size;
+
+ block = amdgpu_vram_mgr_first_block(head);
+ if (!block)
+ return false;
+
+ while (head != block->link.next) {
+ start = amdgpu_vram_mgr_block_start(block);
+ size = amdgpu_vram_mgr_block_size(block);
+
+ block = list_entry(block->link.next, struct drm_buddy_block,
link);
+ if (start + size != amdgpu_vram_mgr_block_start(block))
+ return false;
+ }
+
+ return true;
+}
+
static inline struct amdgpu_vram_mgr_resource *
to_amdgpu_vram_mgr_resource(struct ttm_resource *res)
{
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH] Revert "drm/amdgpu: add drm buddy support to amdgpu"
Hi
Am 08.07.22 um 12:21 schrieb Arunpravin Paneer Selvam:
This reverts the following commits:
commit 708d19d9f362 ("drm/amdgpu: move internal vram_mgr function into the C
file")
commit 5e3f1e7729ec ("drm/amdgpu: fix start calculation in amdgpu_vram_mgr_new")
commit c9cad937c0c5 ("drm/amdgpu: add drm buddy support to amdgpu")
Does the revert need to be cherry-picked into drm-misc-next-fixes?
Best regards
Thomas
[WHY]
Few users reported garbaged graphics as soon as x starts,
reverting until this can be resolved.
Signed-off-by: Arunpravin Paneer Selvam
---
drivers/gpu/drm/Kconfig | 1 -
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 97 +
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 10 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 394 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 62 ---
5 files changed, 176 insertions(+), 388 deletions(-)
delete mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 6c2256e8474b..d438d5ff8b40 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -272,7 +272,6 @@ config DRM_AMDGPU
select HWMON
select BACKLIGHT_CLASS_DEVICE
select INTERVAL_TREE
- select DRM_BUDDY
help
Choose this option if you have a recent AMD Radeon graphics card.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 6546552e596c..acfa207cf970 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -30,15 +30,12 @@
#include
#include
-#include "amdgpu_vram_mgr.h"
-
/* state back for walking over vram_mgr and gtt_mgr allocations */
struct amdgpu_res_cursor {
uint64_tstart;
uint64_tsize;
uint64_tremaining;
- void*node;
- uint32_tmem_type;
+ struct drm_mm_node *node;
};
/**
@@ -55,63 +52,27 @@ static inline void amdgpu_res_first(struct ttm_resource
*res,
uint64_t start, uint64_t size,
struct amdgpu_res_cursor *cur)
{
- struct drm_buddy_block *block;
- struct list_head *head, *next;
struct drm_mm_node *node;
- if (!res)
- goto fallback;
-
- BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
-
- cur->mem_type = res->mem_type;
-
- switch (cur->mem_type) {
- case TTM_PL_VRAM:
- head = &to_amdgpu_vram_mgr_resource(res)->blocks;
-
- block = list_first_entry_or_null(head,
-struct drm_buddy_block,
-link);
- if (!block)
- goto fallback;
-
- while (start >= amdgpu_vram_mgr_block_size(block)) {
- start -= amdgpu_vram_mgr_block_size(block);
-
- next = block->link.next;
- if (next != head)
- block = list_entry(next, struct
drm_buddy_block, link);
- }
-
- cur->start = amdgpu_vram_mgr_block_start(block) + start;
- cur->size = min(amdgpu_vram_mgr_block_size(block) - start,
size);
- cur->remaining = size;
- cur->node = block;
- break;
- case TTM_PL_TT:
- node = to_ttm_range_mgr_node(res)->mm_nodes;
- while (start >= node->size << PAGE_SHIFT)
- start -= node++->size << PAGE_SHIFT;
-
- cur->start = (node->start << PAGE_SHIFT) + start;
- cur->size = min((node->size << PAGE_SHIFT) - start, size);
+ if (!res || res->mem_type == TTM_PL_SYSTEM) {
+ cur->start = start;
+ cur->size = size;
cur->remaining = size;
- cur->node = node;
- break;
- default:
- goto fallback;
+ cur->node = NULL;
+ WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
+ return;
}
- return;
+ BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
-fallback:
- cur->start = start;
- cur->size = size;
+ node = to_ttm_range_mgr_node(res)->mm_nodes;
+ while (start >= node->size << PAGE_SHIFT)
+ start -= node++->size << PAGE_SHIFT;
+
+ cur->start = (node->start << PAGE_SHIFT) + start;
+ cur->size = min((node->size << PAGE_SHIFT) - start, size);
cur->remaining = size;
- cur->node = NULL;
- WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
- return;
+ cur->node = node;
}
/**
@@ -124,9 +85,7 @@ static inline void amdgpu_res_first(struct ttm_resource *res
Re: [PATCH] dma-buf: revert "return only unsignaled fences in dma_fence_unwrap_for_each v3"
Hi Thomas,
Am 14.07.22 um 10:40 schrieb Thomas Zimmermann:
Hi Christian
Am 12.07.22 um 12:28 schrieb Christian König:
This reverts commit 8f61973718485f3e89bc4f408f929048b7b47c83.
I only found this commit in drm-misc-next. Should the revert be
cherry-picked into drm-misc-next-fixes?
yes for all three patches you just pinged me.
I've already tried to push them to drm-misc-next-fixes, but the patches
somehow wouldn't apply. I think the -next-fixes branch was somehow
lagging behind.
Thanks,
Christian.
Best regards
Thomas
It turned out that this is not correct. Especially the sync_file info
IOCTL needs to see even signaled fences to correctly report back their
status to userspace.
Instead add the filter in the merge function again where it makes sense.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-fence-unwrap.c | 3 ++-
include/linux/dma-fence-unwrap.h | 6 +-
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-unwrap.c
b/drivers/dma-buf/dma-fence-unwrap.c
index 502a65ea6d44..7002bca792ff 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -72,7 +72,8 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned
int num_fences,
count = 0;
for (i = 0; i < num_fences; ++i) {
dma_fence_unwrap_for_each(tmp, &iter[i], fences[i])
- ++count;
+ if (!dma_fence_is_signaled(tmp))
+ ++count;
}
if (count == 0)
diff --git a/include/linux/dma-fence-unwrap.h
b/include/linux/dma-fence-unwrap.h
index 390de1ee9d35..66b1e56fbb81 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -43,14 +43,10 @@ struct dma_fence *dma_fence_unwrap_next(struct
dma_fence_unwrap *cursor);
* Unwrap dma_fence_chain and dma_fence_array containers and deep
dive into all
* potential fences in them. If @head is just a normal fence only
that one is
* returned.
- *
- * Note that signalled fences are opportunistically filtered out, which
- * means the iteration is potentially over no fence at all.
*/
#define dma_fence_unwrap_for_each(fence, cursor, head) \
for (fence = dma_fence_unwrap_first(head, cursor); fence; \
- fence = dma_fence_unwrap_next(cursor)) \
- if (!dma_fence_is_signaled(fence))
+ fence = dma_fence_unwrap_next(cursor))
struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
struct dma_fence **fences,
Re: [Intel-gfx] [PATCH v2 01/39] drm/i915/gvt: Fix kernel-doc for intel_gvt_switch_mmio()
On Wed, 13 Jul 2022 18:00:59 -0400 Rodrigo Vivi wrote: > On Wed, Jul 13, 2022 at 05:54:44PM -0400, Rodrigo Vivi wrote: > > On Wed, Jul 13, 2022 at 09:11:49AM +0100, Mauro Carvalho Chehab wrote: > > > From: Jiapeng Chong > > > > > > Fix the following W=1 kernel warnings: > > > > > > drivers/gpu/drm/i915/gvt/mmio_context.c:560: warning: expecting > > > prototype for intel_gvt_switch_render_mmio(). Prototype was for > > > intel_gvt_switch_mmio() instead. > > > > > > Reported-by: Abaci Robot > > > Signed-off-by: Jiapeng Chong > > > Acked-by: Zhenyu Wang > > > Signed-off-by: Mauro Carvalho Chehab > > > > Reviewed-by: Rodrigo Vivi > > I actually changed my mind after seeing that in most cases you use "()" > for the functions and you didn't use for this case... The documentation build system handles both ways equally, and there's no consensus kernel-wide about what would be the preferred way[1]. Also, at the html (or pdf) output, they'll all look the same. So, no difference in practice at the produced documentation. [1] The current count (using drm-tip 2022y-07m-12d-21h-47m-27s) as basis, is: $ git ls-files|xargs grep -Pzo "\/\*\*\n \* [_a-zA-Z0-9]+ -" |wc -l 36680 $ git ls-files|xargs grep -Pzo "\/\*\*\n \* [_a-zA-Z0-9]+\s*\(\) -" |wc -l 12068 So, 48748 documented functions, being ~25% with parenthesis, and ~75% without it. Under drivers/gpu, the numbers are: $ git ls-files|grep drivers/gpu/|xargs grep -Pzo "\/\*\*\n \* [_a-zA-Z0-9]+\s*\(\) -" |wc -l 480 mchehab@sal /new_devel/v4l/tmp $ git ls-files|grep drivers/gpu/|xargs grep -Pzo "\/\*\*\n \* [_a-zA-Z0-9]+ -" |wc -l 4046 > which one should we pick for consistency? Yeah, it is nicer to use the same way everywhere. Btw, on media, I was enforcing one way at the beginning, but I ended giving up doing that as it was too many efforts for too little. Nowadays, half of media function declarations have parenthesis, half doesn't. Anyway, this is what we have at i915 driver, before this series: $ grep -Pzo "\/\*\*\n \* [_a-zA-Z0-9]+\s*\(\) -" $(find drivers/gpu/drm/i915 -type f)|wc -l 53 $ grep -Pzo "\/\*\*\n \* [_a-zA-Z0-9]+ -" $(find drivers/gpu/drm/i915 -type f)|wc -l 542 This series include 3 functions with "()" (on patches 1 and 3, both authored by Jiapeng, and 11 functions without it on my own patches. I'll change those two patches to remove the "()" for consistency. I guess I'll add a patch at the end changing the other 53 functions to drop "()". Regards, Mauro
Re: [PATCH] dma-buf: revert "return only unsignaled fences in dma_fence_unwrap_for_each v3"
Hi
Am 14.07.22 um 10:49 schrieb Christian König:
Hi Thomas,
Am 14.07.22 um 10:40 schrieb Thomas Zimmermann:
Hi Christian
Am 12.07.22 um 12:28 schrieb Christian König:
This reverts commit 8f61973718485f3e89bc4f408f929048b7b47c83.
I only found this commit in drm-misc-next. Should the revert be
cherry-picked into drm-misc-next-fixes?
yes for all three patches you just pinged me.
I've already tried to push them to drm-misc-next-fixes, but the patches
somehow wouldn't apply. I think the -next-fixes branch was somehow
lagging behind.
I just forwarded drm-misc-next-fixes to the latest state of drm-next.
Chances are, these patches will apply now.
Best regards
Thomas
Thanks,
Christian.
Best regards
Thomas
It turned out that this is not correct. Especially the sync_file info
IOCTL needs to see even signaled fences to correctly report back their
status to userspace.
Instead add the filter in the merge function again where it makes sense.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-fence-unwrap.c | 3 ++-
include/linux/dma-fence-unwrap.h | 6 +-
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-unwrap.c
b/drivers/dma-buf/dma-fence-unwrap.c
index 502a65ea6d44..7002bca792ff 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -72,7 +72,8 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned
int num_fences,
count = 0;
for (i = 0; i < num_fences; ++i) {
dma_fence_unwrap_for_each(tmp, &iter[i], fences[i])
- ++count;
+ if (!dma_fence_is_signaled(tmp))
+ ++count;
}
if (count == 0)
diff --git a/include/linux/dma-fence-unwrap.h
b/include/linux/dma-fence-unwrap.h
index 390de1ee9d35..66b1e56fbb81 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -43,14 +43,10 @@ struct dma_fence *dma_fence_unwrap_next(struct
dma_fence_unwrap *cursor);
* Unwrap dma_fence_chain and dma_fence_array containers and deep
dive into all
* potential fences in them. If @head is just a normal fence only
that one is
* returned.
- *
- * Note that signalled fences are opportunistically filtered out, which
- * means the iteration is potentially over no fence at all.
*/
#define dma_fence_unwrap_for_each(fence, cursor, head) \
for (fence = dma_fence_unwrap_first(head, cursor); fence; \
- fence = dma_fence_unwrap_next(cursor)) \
- if (!dma_fence_is_signaled(fence))
+ fence = dma_fence_unwrap_next(cursor))
struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
struct dma_fence **fences,
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
[PATCH v3 0/7] Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation
This patch series fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation, etc. We need to check that we avoid integer overflows when looking up a page, and so fix all the instances where we have mistakenly used a plain integer instead of a more suitable long. And there is an impedance mismatch between the scatterlist API using unsigned int and our memory/page accounting in unsigned long. That is we may try to create a scatterlist for a large object that overflows returning a small table into which we try to fit very many pages. As the object size is under the control of userspace, we have to be prudent and catch the conversion errors. To catch the implicit truncation as we switch from unsigned long into the scatterlist's unsigned int, we use our overflows_type check and report E2BIG prior to the operation. This is already used in our create ioctls to indicate if the uABI request is simply too large for the backing store. And ttm place also has the same problem with scatterlist creation, and we fix the integer truncation problem with the way approached by scatterlist creation. And It corrects the error code to return -E2BIG when creating gem objects using ttm or shmem, if the size is too large in each case. In order to provide a common macro, it moves and adds a few utility macros into drm util header v3: Modify overflows_type() macro to consider signed data types and add is_type_unsigned() macro (Mauro) Make not use the same macro name on a function. (Mauro) For kernel-doc, macros and functions are handled in the same namespace, the same macro name on a function prevents ever adding documentation for it. Not to change execution inside a macro. (Mauro) Fix the problem that safe_conversion() macro always returns true (G.G) Add safe_conversion_gem_bug_on() macro and remove temporal SAFE_CONVERSION() macro. (G.G.) Testcase: igt@gem_create@create-massive Testcase: igt@gem_userptr_blits@input-checking Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4991 Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5411 Cc: Mauro Carvalho Chehab Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Cc: Nirmoy Das Cc: Jani Nikula Cc: David Airlie Cc: Daniel Vetter Chris Wilson (3): drm/i915/gem: Typecheck page lookups drm/i915: Check for integer truncation on scatterlist creation drm/i915: Remove truncation warning for large objects Gwan-gyeong Mun (4): drm: Move and add a few utility macros into drm util header drm/i915: Check for integer truncation on the configuration of ttm place drm/i915: Check if the size is too big while creating shmem file drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 +- drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +- drivers/gpu/drm/i915/gem/i915_gem_object.h| 87 +++ drivers/gpu/drm/i915/gem/i915_gem_pages.c | 27 +++--- drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4 + drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 14 ++- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 23 - drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 +- .../drm/i915/gem/selftests/i915_gem_context.c | 12 +-- .../drm/i915/gem/selftests/i915_gem_mman.c| 8 +- .../drm/i915/gem/selftests/i915_gem_object.c | 8 +- drivers/gpu/drm/i915/gvt/dmabuf.c | 9 +- drivers/gpu/drm/i915/i915_gem.c | 18 +++- drivers/gpu/drm/i915/i915_gem.h | 4 + drivers/gpu/drm/i915/i915_scatterlist.h | 8 ++ drivers/gpu/drm/i915/i915_utils.h | 5 +- drivers/gpu/drm/i915/i915_vma.c | 8 +- drivers/gpu/drm/i915/intel_region_ttm.c | 20 - include/drm/drm_util.h| 77 19 files changed, 257 insertions(+), 93 deletions(-) -- 2.34.1
[PATCH v3 1/7] drm: Move and add a few utility macros into drm util header
It moves overflows_type utility macro into drm util header from i915_utils
header. The overflows_type can be used to catch the truncation between data
types. And it adds safe_conversion() macro which performs a type conversion
(cast) of an source value into a new variable, checking that the
destination is large enough to hold the source value.
And it adds exact_type and exactly_pgoff_t macro to catch type mis-match
while compiling.
v3: Add is_type_unsigned() macro (Mauro)
Modify overflows_type() macro to consider signed data types (Mauro)
Fix the problem that safe_conversion() macro always returns true
Signed-off-by: Gwan-gyeong Mun
Cc: Thomas Hellström
Cc: Matthew Auld
Cc: Nirmoy Das
Cc: Jani Nikula
---
drivers/gpu/drm/i915/i915_utils.h | 5 +-
include/drm/drm_util.h| 77 +++
2 files changed, 78 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_utils.h
b/drivers/gpu/drm/i915/i915_utils.h
index c10d68cdc3ca..345e5b2dc1cd 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_X86
#include
@@ -111,10 +112,6 @@ bool i915_error_injected(void);
#define range_overflows_end_t(type, start, size, max) \
range_overflows_end((type)(start), (type)(size), (type)(max))
-/* Note we don't consider signbits :| */
-#define overflows_type(x, T) \
- (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T))
-
#define ptr_mask_bits(ptr, n) ({ \
unsigned long __v = (unsigned long)(ptr); \
(typeof(ptr))(__v & -BIT(n)); \
diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
index 79952d8c4bba..d594ef34b537 100644
--- a/include/drm/drm_util.h
+++ b/include/drm/drm_util.h
@@ -62,6 +62,83 @@
*/
#define for_each_if(condition) if (!(condition)) {} else
+/**
+ * is_type_unsigned - helper for checking data type which is an unsigned data
+ * type or not
+ * @x: The data type to check
+ *
+ * Returns:
+ * True if the data type is an unsigned data type, false otherwise.
+ */
+#define is_type_unsigned(x) ((typeof(x))-1 >= (typeof(x))0)
+
+/**
+ * overflows_type - helper for checking the truncation between data types
+ * @x: Source for overflow type comparison
+ * @T: Destination for overflow type comparison
+ *
+ * It compares the values and size of each data type between the first and
+ * second argument to check whether truncation can occur when assigning the
+ * first argument to the variable of the second argument.
+ * Source and Destination can be used with or without sign bit.
+ * Composite data structures such as union and structure are not considered.
+ * Enum data types are not considered.
+ * Floating point data types are not considered.
+ *
+ * Returns:
+ * True if truncation can occur, false otherwise.
+ */
+
+#define overflows_type(x, T) \
+ (is_type_unsigned(x) ? \
+ is_type_unsigned(T) ? \
+ (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T)) ? 1
: 0 \
+ : (sizeof(x) >= sizeof(T) && (x) >> (BITS_PER_TYPE(T) -
1)) ? 1 : 0 \
+ : is_type_unsigned(T) ? \
+ ((x) < 0) ? 1 : (sizeof(x) > sizeof(T) && (x) >>
BITS_PER_TYPE(T)) ? 1 : 0 \
+ : (sizeof(x) > sizeof(T)) ? \
+ ((x) < 0) ? (((x) * -1) >> BITS_PER_TYPE(T)) ? 1 : 0 \
+ : ((x) >> BITS_PER_TYPE(T)) ? 1 : 0 \
+ : 0)
+
+/**
+ * exact_type - break compile if source type and destination value's type are
+ * not the same
+ * @T: Source type
+ * @n: Destination value
+ *
+ * It is a helper macro for a poor man's -Wconversion: only allow variables of
+ * an exact type. It determines whether the source type and destination value's
+ * type are the same while compiling, and it breaks compile if two types are
+ * not the same
+ */
+#define exact_type(T, n) \
+ BUILD_BUG_ON(!__builtin_constant_p(n) &&
!__builtin_types_compatible_p(T, typeof(n)))
+
+/**
+ * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
+ * @n: value to compare pgoff_t type
+ *
+ * It breaks compile if the argument value's type is not pgoff_t type.
+ */
+#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
+
+/*
+ * safe_conversion - perform a type conversion (cast) of an source value into
+ * a new variable, checking that the destination is large enough to hold the
+ * source value.
+ * @ptr: Destination pointer address
+ * @value: Source value
+ *
+ * Returns:
+ * If the value would overflow the destination, it returns false.
+ */
+#define safe_conversion(ptr, value) ({ \
+ typeof(value) __v = (value); \
+ typeof(ptr) __ptr = (ptr); \
+ overflows_type(__v, *__ptr) ? 0 : ((*__ptr = (typeof(*__ptr))__v), 1); \
+})
+
/**
* drm_can_sleep - returns true if currently okay to sleep
*
--
2.34.1
[PATCH v3 2/7] drm/i915/gem: Typecheck page lookups
From: Chris Wilson
We need to check that we avoid integer overflows when looking up a page,
and so fix all the instances where we have mistakenly used a plain
integer instead of a more suitable long. Be pedantic and add integer
typechecking to the lookup so that we can be sure that we are safe.
And it also uses pgoff_t as our page lookups must remain compatible with
the page cache, pgoff_t is currently exactly unsigned long.
v2: Move added i915_utils's macro into drm_util header (Jani N)
v3: Make not use the same macro name on a function. (Mauro)
For kernel-doc, macros and functions are handled in the same namespace,
the same macro name on a function prevents ever adding documentation
for it.
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
---
drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +-
drivers/gpu/drm/i915/gem/i915_gem_object.h| 77 +--
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 27 ---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
.../drm/i915/gem/selftests/i915_gem_context.c | 12 +--
.../drm/i915/gem/selftests/i915_gem_mman.c| 8 +-
.../drm/i915/gem/selftests/i915_gem_object.c | 8 +-
drivers/gpu/drm/i915/i915_gem.c | 18 -
drivers/gpu/drm/i915/i915_vma.c | 8 +-
9 files changed, 106 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index ccec4055fde3..90996fe8ad45 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -421,10 +421,11 @@ void __i915_gem_object_invalidate_frontbuffer(struct
drm_i915_gem_object *obj,
static void
i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ pgoff_t idx = offset >> PAGE_SHIFT;
void *src_map;
void *src_ptr;
- src_map = kmap_atomic(i915_gem_object_get_page(obj, offset >>
PAGE_SHIFT));
+ src_map = kmap_atomic(i915_gem_object_get_page(obj, idx));
src_ptr = src_map + offset_in_page(offset);
if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
@@ -437,9 +438,10 @@ i915_gem_object_read_from_page_kmap(struct
drm_i915_gem_object *obj, u64 offset,
static void
i915_gem_object_read_from_page_iomap(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ pgoff_t idx = offset >> PAGE_SHIFT;
+ dma_addr_t dma = i915_gem_object_get_dma_address(obj, idx);
void __iomem *src_map;
void __iomem *src_ptr;
- dma_addr_t dma = i915_gem_object_get_dma_address(obj, offset >>
PAGE_SHIFT);
src_map = io_mapping_map_wc(&obj->mm.region->iomap,
dma - obj->mm.region->region.start,
@@ -468,6 +470,7 @@ i915_gem_object_read_from_page_iomap(struct
drm_i915_gem_object *obj, u64 offset
*/
int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ GEM_BUG_ON(overflows_type(offset >> PAGE_SHIFT, pgoff_t));
GEM_BUG_ON(offset >= obj->base.size);
GEM_BUG_ON(offset_in_page(offset) > PAGE_SIZE - size);
GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 6f0a3ce35567..4b49e0d2890b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -27,8 +27,10 @@ enum intel_region_id;
* spot such a local variable, please consider fixing!
*
* Aside from our own locals (for which we have no excuse!):
- * - sg_table embeds unsigned int for num_pages
- * - get_user_pages*() mixed ints with longs
+ * - sg_table embeds unsigned int for nents
+ *
+ * We can check for invalidly typed locals with typecheck(), see for example
+ * i915_gem_object_get_sg().
*/
#define GEM_CHECK_SIZE_OVERFLOW(sz) \
GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
@@ -364,43 +366,72 @@ int i915_gem_object_set_tiling(struct drm_i915_gem_object
*obj,
unsigned int tiling, unsigned int stride);
struct scatterlist *
-__i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
-struct i915_gem_object_page_iter *iter,
-unsigned int n,
-unsigned int *offset, bool dma);
+__i915_gem_object_page_iter_get_sg(struct drm_i915_gem_object *obj,
+ struct i915_gem_object_page_iter *iter,
+ pgoff_t n,
+ unsigned int *offset);
+
+#define i915_gem_object_page_iter_get_sg(obj, it, n, offset) ({ \
+ exactly_pgoff_t(n); \
+ __i915_gem_object_page_iter_get_sg(obj, it, n, offset); \
+})
static inline struct scatterlist *
-i915_gem_
[PATCH v3 3/7] drm/i915: Check for integer truncation on scatterlist creation
From: Chris Wilson
There is an impedance mismatch between the scatterlist API using unsigned
int and our memory/page accounting in unsigned long. That is we may try
to create a scatterlist for a large object that overflows returning a
small table into which we try to fit very many pages. As the object size
is under control of userspace, we have to be prudent and catch the
conversion errors.
To catch the implicit truncation as we switch from unsigned long into the
scatterlist's unsigned int, we use overflows_type check and report
E2BIG prior to the operation. This is already used in our create ioctls to
indicate if the uABI request is simply too large for the backing store.
Failing that type check, we have a second check at sg_alloc_table time
to make sure the values we are passing into the scatterlist API are not
truncated.
It uses pgoff_t for locals that are dealing with page indices, in this
case, the page count is the limit of the page index.
And it uses safe_conversion() macro which performs a type conversion (cast)
of an integer value into a new variable, checking that the destination is
large enough to hold the source value.
v2: Move added i915_utils's macro into drm_util header (Jani N)
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Brian Welty
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
---
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 --
drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ---
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4
drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 5 -
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 4
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 -
drivers/gpu/drm/i915/gvt/dmabuf.c| 9 +
drivers/gpu/drm/i915/i915_scatterlist.h | 8
8 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
index c698f95af15f..ff2e6e780631 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
@@ -37,10 +37,13 @@ static int i915_gem_object_get_pages_internal(struct
drm_i915_gem_object *obj)
struct sg_table *st;
struct scatterlist *sg;
unsigned int sg_page_sizes;
- unsigned int npages;
+ pgoff_t npages; /* restricted by sg_alloc_table */
int max_order;
gfp_t gfp;
+ if (!safe_conversion(&npages, obj->base.size >> PAGE_SHIFT))
+ return -E2BIG;
+
max_order = MAX_ORDER;
#ifdef CONFIG_SWIOTLB
if (is_swiotlb_active(obj->base.dev->dev)) {
@@ -67,7 +70,6 @@ static int i915_gem_object_get_pages_internal(struct
drm_i915_gem_object *obj)
if (!st)
return -ENOMEM;
- npages = obj->base.size / PAGE_SIZE;
if (sg_alloc_table(st, npages, GFP_KERNEL)) {
kfree(st);
return -ENOMEM;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 4b49e0d2890b..a61d7984f1d1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -26,9 +26,6 @@ enum intel_region_id;
* this and catch if we ever need to fix it. In the meantime, if you do
* spot such a local variable, please consider fixing!
*
- * Aside from our own locals (for which we have no excuse!):
- * - sg_table embeds unsigned int for nents
- *
* We can check for invalidly typed locals with typecheck(), see for example
* i915_gem_object_get_sg().
*/
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 0d0e46dae559..88ba7266a3a5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -28,6 +28,10 @@ static int i915_gem_object_get_pages_phys(struct
drm_i915_gem_object *obj)
void *dst;
int i;
+ /* Contiguous chunk, with a single scatterlist element */
+ if (overflows_type(obj->base.size, sg->length))
+ return -E2BIG;
+
if (GEM_WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 4eed3dd90ba8..604e8829e8ea 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -193,13 +193,16 @@ static int shmem_get_pages(struct drm_i915_gem_object
*obj)
struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct intel_memory_region *mem = obj->mm.region;
struct address_space *mapping = obj->base.filp->f_mapping;
- const unsigned long page_count = obj->base.size / PAGE_SIZE;
unsigned int max_segment = i915_sg_segment_size();
struct sg_table *st;
struct sgt_iter sgt_iter;
+ pgoff_t page_cou
[PATCH v3 4/7] drm/i915: Check for integer truncation on the configuration of ttm place
There is an impedance mismatch between the first/last valid page
frame number of ttm place in unsigned and our memory/page accounting in
unsigned long.
As the object size is under the control of userspace, we have to be prudent
and catch the conversion errors.
To catch the implicit truncation as we switch from unsigned long to
unsigned, we use overflows_type check and report E2BIG or overflow_type
prior to the operation.
v3: Not to change execution inside a macro. (Mauro)
Add safe_conversion_gem_bug_on() macro and remove temporal
SAFE_CONVERSION() macro.
Signed-off-by: Gwan-gyeong Mun
Cc: Chris Wilson
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 6 +++---
drivers/gpu/drm/i915/i915_gem.h | 4
drivers/gpu/drm/i915/intel_region_ttm.c | 20 +---
3 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 9f2be1892b6c..88f2887627dc 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -140,14 +140,14 @@ i915_ttm_place_from_region(const struct
intel_memory_region *mr,
if (flags & I915_BO_ALLOC_CONTIGUOUS)
place->flags |= TTM_PL_FLAG_CONTIGUOUS;
if (offset != I915_BO_INVALID_OFFSET) {
- place->fpfn = offset >> PAGE_SHIFT;
- place->lpfn = place->fpfn + (size >> PAGE_SHIFT);
+ safe_conversion_gem_bug_on(&place->fpfn, offset >> PAGE_SHIFT);
+ safe_conversion_gem_bug_on(&place->lpfn, place->fpfn + (size >>
PAGE_SHIFT));
} else if (mr->io_size && mr->io_size < mr->total) {
if (flags & I915_BO_ALLOC_GPU_ONLY) {
place->flags |= TTM_PL_FLAG_TOPDOWN;
} else {
place->fpfn = 0;
- place->lpfn = mr->io_size >> PAGE_SHIFT;
+ safe_conversion_gem_bug_on(&place->lpfn, mr->io_size >>
PAGE_SHIFT);
}
}
}
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 68d8d52bd541..6b673607abee 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -83,5 +83,9 @@ struct drm_i915_private;
#endif
#define I915_GEM_IDLE_TIMEOUT (HZ / 5)
+#define safe_conversion_gem_bug_on(ptr, value) ({ \
+ safe_conversion(ptr, value) ? 1 \
+ : (({ GEM_BUG_ON(overflows_type(value, *ptr)); }), 0); \
+})
#endif /* __I915_GEM_H__ */
diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
b/drivers/gpu/drm/i915/intel_region_ttm.c
index 575d67bc6ffe..f0d143948725 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -209,14 +209,26 @@ intel_region_ttm_resource_alloc(struct
intel_memory_region *mem,
if (flags & I915_BO_ALLOC_CONTIGUOUS)
place.flags |= TTM_PL_FLAG_CONTIGUOUS;
if (offset != I915_BO_INVALID_OFFSET) {
- place.fpfn = offset >> PAGE_SHIFT;
- place.lpfn = place.fpfn + (size >> PAGE_SHIFT);
+ if (!safe_conversion_gem_bug_on(&place.fpfn,
+ offset >> PAGE_SHIFT)) {
+ ret = -E2BIG;
+ goto out;
+ }
+ if (!safe_conversion_gem_bug_on(&place.lpfn,
+ place.fpfn + (size >>
PAGE_SHIFT))) {
+ ret = -E2BIG;
+ goto out;
+ }
} else if (mem->io_size && mem->io_size < mem->total) {
if (flags & I915_BO_ALLOC_GPU_ONLY) {
place.flags |= TTM_PL_FLAG_TOPDOWN;
} else {
place.fpfn = 0;
- place.lpfn = mem->io_size >> PAGE_SHIFT;
+ if (!safe_conversion_gem_bug_on(&place.lpfn,
+ mem->io_size >>
PAGE_SHIFT)) {
+ ret = -E2BIG;
+ goto out;
+ }
}
}
@@ -224,6 +236,8 @@ intel_region_ttm_resource_alloc(struct intel_memory_region
*mem,
mock_bo.bdev = &mem->i915->bdev;
ret = man->func->alloc(man, &mock_bo, &place, &res);
+
+out:
if (ret == -ENOSPC)
ret = -ENXIO;
if (!ret)
--
2.34.1
[PATCH v3 6/7] drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large
The ttm_bo_init_reserved() functions returns -ENOSPC if the size is too big to add vma. The direct function that returns -ENOSPC is drm_mm_insert_node_in_range(). To handle the same error as other code returning -E2BIG when the size is too large, it converts return value to -E2BIG. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 88f2887627dc..4d478bf325be 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1249,6 +1249,17 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, ret = ttm_bo_init_reserved(&i915->bdev, i915_gem_to_ttm(obj), bo_type, &i915_sys_placement, page_size >> PAGE_SHIFT, &ctx, NULL, NULL, i915_ttm_bo_destroy); + + /* +* XXX: The ttm_bo_init_reserved() functions returns -ENOSPC if the size +* is too big to add vma. The direct function that returns -ENOSPC is +* drm_mm_insert_node_in_range(). To handle the same error as other code +* that returns -E2BIG when the size is too large, it converts -ENOSPC to +* -E2BIG. +*/ + if (size >> PAGE_SHIFT > INT_MAX && ret == -ENOSPC) + ret = -E2BIG; + if (ret) return i915_ttm_err_to_gem(ret); -- 2.34.1
[PATCH v3 5/7] drm/i915: Check if the size is too big while creating shmem file
The __shmem_file_setup() function returns -EINVAL if size is greater than MAX_LFS_FILESIZE. To handle the same error as other code that returns -E2BIG when the size is too large, it add a code that returns -E2BIG when the size is larger than the size that can be handled. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab --- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 604e8829e8ea..8495e87432f6 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -541,6 +541,15 @@ static int __create_shmem(struct drm_i915_private *i915, drm_gem_private_object_init(&i915->drm, obj, size); + /* XXX: The __shmem_file_setup() function returns -EINVAL if size is +* greater than MAX_LFS_FILESIZE. +* To handle the same error as other code that returns -E2BIG when +* the size is too large, we add a code that returns -E2BIG when the +* size is larger than the size that can be handled. +*/ + if (size > MAX_LFS_FILESIZE) + return -E2BIG; + if (i915->mm.gemfs) filp = shmem_file_setup_with_mnt(i915->mm.gemfs, "i915", size, flags); -- 2.34.1
[PATCH v3 7/7] drm/i915: Remove truncation warning for large objects
From: Chris Wilson
Having addressed the issues surrounding incorrect types for local
variables and potential integer truncation in using the scatterlist API,
we have closed all the loop holes we had previously identified with
dangerously large object creation. As such, we can eliminate the warning
put in place to remind us to complete the review.
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Brian Welty
Cc: Matthew Auld
Cc: Thomas Hellström
Testcase: igt@gem_create@create-massive
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4991
Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
---
drivers/gpu/drm/i915/gem/i915_gem_object.h | 15 ---
1 file changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index a61d7984f1d1..eec6ec3ad1a1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -20,25 +20,10 @@
enum intel_region_id;
-/*
- * XXX: There is a prevalence of the assumption that we fit the
- * object's page count inside a 32bit _signed_ variable. Let's document
- * this and catch if we ever need to fix it. In the meantime, if you do
- * spot such a local variable, please consider fixing!
- *
- * We can check for invalidly typed locals with typecheck(), see for example
- * i915_gem_object_get_sg().
- */
-#define GEM_CHECK_SIZE_OVERFLOW(sz) \
- GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
-
static inline bool i915_gem_object_size_2big(u64 size)
{
struct drm_i915_gem_object *obj;
- if (GEM_CHECK_SIZE_OVERFLOW(size))
- return true;
-
if (overflows_type(size, obj->base.size))
return true;
--
2.34.1
[PATCH v3 0/8] drm/vc4: Add generic helpers for HDMI scrambling
Hi, This is a follow-up of the work to support the interactions between the hotplug and the scrambling support for vc4: https://lore.kernel.org/dri-devel/20210507150515.257424-11-max...@cerno.tech/ https://lore.kernel.org/dri-devel/20211025152903.1088803-10-max...@cerno.tech/ https://lore.kernel.org/dri-devel/2028103814.524670-1-max...@cerno.tech/ Ville feedback was that the same discussion happened some time ago for i915, and resulted in a function to do an full disable/enable cycle on reconnection to avoid breaking the HDMI 2.0 spec. While the previous versions of this series was moving the current scrambling related functions into generic helpers to consolidate that logic, it proved to be difficult to rework existing drivers to make use of it without hardware to test it on and thus the code is (for now) private to vc4. I still believe that long term, the code to decide if the scrambler needs to be enabled or not should be moved into a generic helper. This also means that we would need to move the format output decision to a generic helper, which also makes sense to me but it probably going to be controversial. Let me know what you think, Maxime Changes from v2: - Rebased on next-20220713 - Dropped the generic helpers and put them into vc4 Changes from v1: - Dropped the 340MHz define - Make drm_mode_hdmi_requires_scrambling use the bpc - Make more drm_display_mode const in vc4 - Dropped the tegra conversion - Added more comments Maxime Ripard (8): drm/vc4: hdmi: Constify drm_display_mode drm/vc4: hdmi: Remove unused argument in vc4_hdmi_supports_scrambling drm/vc4: hdmi: Remove mutex in detect drm/vc4: hdmi: Simplify the hotplug handling drm/vc4: hdmi: Switch to detect_ctx drm/vc4: hdmi: Move vc4_hdmi_supports_scrambling() around drm/vc4: hdmi: Reset link on hotplug drm/scdc: Document hotplug gotchas drivers/gpu/drm/display/drm_scdc_helper.c | 13 + drivers/gpu/drm/vc4/vc4_hdmi.c| 309 ++ drivers/gpu/drm/vc4/vc4_hdmi.h| 12 +- 3 files changed, 220 insertions(+), 114 deletions(-) -- 2.36.1
[PATCH v3 1/8] drm/vc4: hdmi: Constify drm_display_mode
We don't modify the drm_display_mode pointer we have in the driver in
most places, so let's make them const.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 16
drivers/gpu/drm/vc4/vc4_hdmi.h | 2 +-
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 3b75ac6fa0db..adcc2d3918f1 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -295,7 +295,7 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
if (vc4_hdmi->disable_4kp60) {
struct drm_device *drm = connector->dev;
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
list_for_each_entry(mode, &connector->probed_modes, head) {
if (vc4_hdmi_mode_needs_scrambling(mode, 8,
VC4_HDMI_OUTPUT_RGB)) {
@@ -651,7 +651,7 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
}
static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
-struct drm_display_mode *mode)
+const struct drm_display_mode *mode)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_info *display = &vc4_hdmi->connector.display_info;
@@ -673,7 +673,7 @@ static bool vc4_hdmi_supports_scrambling(struct drm_encoder
*encoder,
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
+ const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
unsigned long flags;
lockdep_assert_held(&vc4_hdmi->mutex);
@@ -975,7 +975,7 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,
static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi,
struct drm_connector_state *state,
-struct drm_display_mode *mode)
+const struct drm_display_mode *mode)
{
bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
@@ -1032,7 +1032,7 @@ static void vc4_hdmi_set_timings(struct vc4_hdmi
*vc4_hdmi,
static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi,
struct drm_connector_state *state,
-struct drm_display_mode *mode)
+const struct drm_display_mode *mode)
{
const struct vc4_hdmi_connector_state *vc4_state =
conn_state_to_vc4_hdmi_conn_state(state);
@@ -1179,7 +1179,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct
drm_encoder *encoder,
drm_atomic_get_new_connector_state(state, connector);
struct vc4_hdmi_connector_state *vc4_conn_state =
conn_state_to_vc4_hdmi_conn_state(conn_state);
- struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
+ const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
unsigned long tmds_char_rate = vc4_conn_state->tmds_char_rate;
unsigned long bvb_rate, hsm_rate;
unsigned long flags;
@@ -1283,7 +1283,7 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct
drm_encoder *encoder,
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_connector *connector = &vc4_hdmi->connector;
- struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
+ const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
struct drm_connector_state *conn_state =
drm_atomic_get_new_connector_state(state, connector);
unsigned long flags;
@@ -1304,7 +1304,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
struct drm_atomic_state *state)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
+ const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
struct drm_display_info *display = &vc4_hdmi->connector.display_info;
bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index c3ed2b07df23..b3f404ca94b6 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -71,7 +71,7 @@ struct vc4_hdmi_variant {
/* Callback to configure the video timings in the HDMI block */
void (*set_timings)(struct vc4_hdmi *vc4_hdmi,
struct drm_connector_state *state,
- struct drm_display_mode *mode);
+
[PATCH v3 2/8] drm/vc4: hdmi: Remove unused argument in vc4_hdmi_supports_scrambling
Even though vc4_hdmi_supports_scrambling takes a mode as an argument, it
never uses it. Let's remove it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index adcc2d3918f1..13407e06846f 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -650,8 +650,7 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
vc4_hdmi_set_hdr_infoframe(encoder);
}
-static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
-const struct drm_display_mode *mode)
+static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_info *display = &vc4_hdmi->connector.display_info;
@@ -678,7 +677,7 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder
*encoder)
lockdep_assert_held(&vc4_hdmi->mutex);
- if (!vc4_hdmi_supports_scrambling(encoder, mode))
+ if (!vc4_hdmi_supports_scrambling(encoder))
return;
if (!vc4_hdmi_mode_needs_scrambling(mode,
--
2.36.1
[PATCH v3 3/8] drm/vc4: hdmi: Remove mutex in detect
We recently introduced a new mutex to protect concurrent execution of
ALSA and KMS hooks, and the concurrent access to some of vc4_hdmi
fields.
However, using it in the detect hook was creating a reentrency issue
with CEC code. Indeed, calling cec_s_phys_addr_from_edid from detect
might call the CEC adap_enable hook with the lock held, eventually
resulting in a deadlock.
Since we didn't really need to protect anything at the moment in the CEC
code, the decision was made to ignore the mutex in those CEC hooks,
working around the issue.
However, we can have the same thing happening if we end up triggering a
mode set from the detect callback, for example using
drm_atomic_helper_connector_hdmi_reset_link().
Since we don't really need to protect anything in detect either, let's
just drop the lock in detect, and add it again in CEC.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 89 +-
drivers/gpu/drm/vc4/vc4_hdmi.h | 10 +---
2 files changed, 36 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 13407e06846f..aacc94d593f0 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -233,7 +233,16 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
bool connected = false;
- mutex_lock(&vc4_hdmi->mutex);
+ /*
+* NOTE: This function should really take vc4_hdmi->mutex, but
+* doing so results in reentrancy issues since
+* cec_s_phys_addr_from_edid might call .adap_enable, which
+* leads to that funtion being called with our mutex held.
+*
+* Concurrency isn't an issue at the moment since we don't share
+* any state with any of the other frameworks so we can ignore
+* the lock for now.
+*/
WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
@@ -258,13 +267,11 @@ vc4_hdmi_connector_detect(struct drm_connector
*connector, bool force)
vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base);
pm_runtime_put(&vc4_hdmi->pdev->dev);
- mutex_unlock(&vc4_hdmi->mutex);
return connector_status_connected;
}
cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
pm_runtime_put(&vc4_hdmi->pdev->dev);
- mutex_unlock(&vc4_hdmi->mutex);
return connector_status_disconnected;
}
@@ -280,14 +287,21 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
int ret = 0;
struct edid *edid;
- mutex_lock(&vc4_hdmi->mutex);
+ /*
+* NOTE: This function should really take vc4_hdmi->mutex, but
+* doing so results in reentrancy issues since
+* cec_s_phys_addr_from_edid might call .adap_enable, which
+* leads to that funtion being called with our mutex held.
+*
+* Concurrency isn't an issue at the moment since we don't share
+* any state with any of the other frameworks so we can ignore
+* the lock for now.
+*/
edid = drm_get_edid(connector, vc4_hdmi->ddc);
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
- if (!edid) {
- ret = -ENODEV;
- goto out;
- }
+ if (!edid)
+ return -ENODEV;
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
@@ -305,9 +319,6 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
}
}
-out:
- mutex_unlock(&vc4_hdmi->mutex);
-
return ret;
}
@@ -2381,21 +2392,12 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
u32 val;
int ret;
- /*
-* NOTE: This function should really take vc4_hdmi->mutex, but doing so
-* results in a reentrancy since cec_s_phys_addr_from_edid() called in
-* .detect or .get_modes might call .adap_enable, which leads to this
-* function being called with that mutex held.
-*
-* Concurrency is not an issue for the moment since we don't share any
-* state with KMS, so we can ignore the lock for now, but we need to
-* keep it in mind if we were to change that assumption.
-*/
-
ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev);
if (ret)
return ret;
+ mutex_lock(&vc4_hdmi->mutex);
+
spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
val = HDMI_READ(HDMI_CEC_CNTRL_5);
@@ -2430,6 +2432,8 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
+ mutex_unlock(&vc4_hdmi->mutex);
+
return 0;
}
@@ -2438,16 +2442,7 @@ static int vc4_hdmi_cec_disable(struct cec_adapter *adap)
struct vc4_hdmi *vc4_hdmi = cec_get_dr
[PATCH v3 4/8] drm/vc4: hdmi: Simplify the hotplug handling
Our detect callback has a bunch of operations to perform depending on
the current and last status of the connector, such a setting the CEC
physical address or enabling the scrambling again.
This is currently dealt with a bunch of if / else statetements that make
it fairly difficult to read and extend.
Let's move all that logic to a function of its own.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 66 ++
1 file changed, 44 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index aacc94d593f0..6d84f3b96654 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -227,17 +227,53 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi
*vc4_hdmi) {}
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
+static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
+ enum drm_connector_status status)
+{
+ struct drm_connector *connector = &vc4_hdmi->connector;
+ struct edid *edid;
+
+ /*
+* NOTE: This function should really be called with
+* vc4_hdmi->mutex held, but doing so results in reentrancy
+* issues since cec_s_phys_addr_from_edid might call
+* .adap_enable, which leads to that funtion being called with
+* our mutex held.
+*
+* Concurrency isn't an issue at the moment since we don't share
+* any state with any of the other frameworks so we can ignore
+* the lock for now.
+*/
+
+ if (status == connector->status)
+ return;
+
+ if (status == connector_status_disconnected) {
+ cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
+ return;
+ }
+
+ edid = drm_get_edid(connector, vc4_hdmi->ddc);
+ if (!edid)
+ return;
+
+ cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
+ kfree(edid);
+
+ vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
+}
+
static enum drm_connector_status
vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
- bool connected = false;
+ enum drm_connector_status status = connector_status_disconnected;
/*
* NOTE: This function should really take vc4_hdmi->mutex, but
* doing so results in reentrancy issues since
-* cec_s_phys_addr_from_edid might call .adap_enable, which
-* leads to that funtion being called with our mutex held.
+* vc4_hdmi_handle_hotplug() can call into other functions that
+* would take the mutex while it's held here.
*
* Concurrency isn't an issue at the moment since we don't share
* any state with any of the other frameworks so we can ignore
@@ -248,31 +284,17 @@ vc4_hdmi_connector_detect(struct drm_connector
*connector, bool force)
if (vc4_hdmi->hpd_gpio) {
if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
- connected = true;
+ status = connector_status_connected;
} else {
if (vc4_hdmi->variant->hp_detect &&
vc4_hdmi->variant->hp_detect(vc4_hdmi))
- connected = true;
+ status = connector_status_connected;
}
- if (connected) {
- if (connector->status != connector_status_connected) {
- struct edid *edid = drm_get_edid(connector,
vc4_hdmi->ddc);
-
- if (edid) {
- cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- kfree(edid);
- }
- }
-
- vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base);
- pm_runtime_put(&vc4_hdmi->pdev->dev);
- return connector_status_connected;
- }
-
- cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
+ vc4_hdmi_handle_hotplug(vc4_hdmi, status);
pm_runtime_put(&vc4_hdmi->pdev->dev);
- return connector_status_disconnected;
+
+ return status;
}
static void vc4_hdmi_connector_destroy(struct drm_connector *connector)
--
2.36.1
Re: [PATCH] dma-buf: revert "return only unsignaled fences in dma_fence_unwrap_for_each v3"
Am 14.07.22 um 11:06 schrieb Thomas Zimmermann:
Hi
Am 14.07.22 um 10:49 schrieb Christian König:
Hi Thomas,
Am 14.07.22 um 10:40 schrieb Thomas Zimmermann:
Hi Christian
Am 12.07.22 um 12:28 schrieb Christian König:
This reverts commit 8f61973718485f3e89bc4f408f929048b7b47c83.
I only found this commit in drm-misc-next. Should the revert be
cherry-picked into drm-misc-next-fixes?
yes for all three patches you just pinged me.
I've already tried to push them to drm-misc-next-fixes, but the
patches somehow wouldn't apply. I think the -next-fixes branch was
somehow lagging behind.
I just forwarded drm-misc-next-fixes to the latest state of drm-next.
Chances are, these patches will apply now.
Thanks, should I cherry pick them or are you going to do it?
And can we somehow make sure that when the drm-misc-next is merged into
drm-next for upstreaming that drm-misc-next-fixes is up to date as well?
That would make things much easier.
Thanks,
Christian.
Best regards
Thomas
Thanks,
Christian.
Best regards
Thomas
It turned out that this is not correct. Especially the sync_file info
IOCTL needs to see even signaled fences to correctly report back their
status to userspace.
Instead add the filter in the merge function again where it makes
sense.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-fence-unwrap.c | 3 ++-
include/linux/dma-fence-unwrap.h | 6 +-
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-unwrap.c
b/drivers/dma-buf/dma-fence-unwrap.c
index 502a65ea6d44..7002bca792ff 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -72,7 +72,8 @@ struct dma_fence
*__dma_fence_unwrap_merge(unsigned int num_fences,
count = 0;
for (i = 0; i < num_fences; ++i) {
dma_fence_unwrap_for_each(tmp, &iter[i], fences[i])
- ++count;
+ if (!dma_fence_is_signaled(tmp))
+ ++count;
}
if (count == 0)
diff --git a/include/linux/dma-fence-unwrap.h
b/include/linux/dma-fence-unwrap.h
index 390de1ee9d35..66b1e56fbb81 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -43,14 +43,10 @@ struct dma_fence *dma_fence_unwrap_next(struct
dma_fence_unwrap *cursor);
* Unwrap dma_fence_chain and dma_fence_array containers and deep
dive into all
* potential fences in them. If @head is just a normal fence only
that one is
* returned.
- *
- * Note that signalled fences are opportunistically filtered out,
which
- * means the iteration is potentially over no fence at all.
*/
#define dma_fence_unwrap_for_each(fence, cursor, head) \
for (fence = dma_fence_unwrap_first(head, cursor); fence; \
- fence = dma_fence_unwrap_next(cursor)) \
- if (!dma_fence_is_signaled(fence))
+ fence = dma_fence_unwrap_next(cursor))
struct dma_fence *__dma_fence_unwrap_merge(unsigned int
num_fences,
struct dma_fence **fences,
[PATCH v3 6/8] drm/vc4: hdmi: Move vc4_hdmi_supports_scrambling() around
We'll need it earlier in the driver, so let's move it next to the other
scrambling-related helpers.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 668addc8c170..2d988c73d12b 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -125,6 +125,23 @@ static unsigned long long
vc4_hdmi_encoder_compute_mode_clock(const struct drm_display_mode *mode,
unsigned int bpc, enum
vc4_hdmi_output_format fmt);
+static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder)
+{
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_display_info *display = &vc4_hdmi->connector.display_info;
+
+ lockdep_assert_held(&vc4_hdmi->mutex);
+
+ if (!display->is_hdmi)
+ return false;
+
+ if (!display->hdmi.scdc.supported ||
+ !display->hdmi.scdc.scrambling.supported)
+ return false;
+
+ return true;
+}
+
static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode,
unsigned int bpc,
enum vc4_hdmi_output_format fmt)
@@ -684,23 +701,6 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
vc4_hdmi_set_hdr_infoframe(encoder);
}
-static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder)
-{
- struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- struct drm_display_info *display = &vc4_hdmi->connector.display_info;
-
- lockdep_assert_held(&vc4_hdmi->mutex);
-
- if (!display->is_hdmi)
- return false;
-
- if (!display->hdmi.scdc.supported ||
- !display->hdmi.scdc.scrambling.supported)
- return false;
-
- return true;
-}
-
#define SCRAMBLING_POLLING_DELAY_MS1000
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
--
2.36.1
[PATCH v3 5/8] drm/vc4: hdmi: Switch to detect_ctx
We'll need the locking context in future patch, so let's convert .detect
to .detect_ctx.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 6d84f3b96654..668addc8c170 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -263,8 +263,9 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi
*vc4_hdmi,
vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
}
-static enum drm_connector_status
-vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
+static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
+struct drm_modeset_acquire_ctx *ctx,
+bool force)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
enum drm_connector_status status = connector_status_disconnected;
@@ -412,7 +413,6 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector
*connector)
}
static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
- .detect = vc4_hdmi_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = vc4_hdmi_connector_destroy,
.reset = vc4_hdmi_connector_reset,
@@ -421,6 +421,7 @@ static const struct drm_connector_funcs
vc4_hdmi_connector_funcs = {
};
static const struct drm_connector_helper_funcs vc4_hdmi_connector_helper_funcs
= {
+ .detect_ctx = vc4_hdmi_connector_detect_ctx,
.get_modes = vc4_hdmi_connector_get_modes,
.atomic_check = vc4_hdmi_connector_atomic_check,
};
--
2.36.1
[PATCH v3 7/8] drm/vc4: hdmi: Reset link on hotplug
During a hotplug cycle (such as a TV going out of suspend, or when the
cable is disconnected and reconnected), the expectation is that the same
state used before the disconnection is reused until the next commit.
However, the HDMI scrambling requires that some flags are set in the
monitor, and those flags are very likely to be reset when the cable has
been disconnected. This will thus result in a blank display, even if the
display pipeline configuration hasn't been modified or is in the exact
same state.
The solution we've had so far is to enable the scrambling-related bits
again on reconnection, but the HDMI 2.0 specification (Section 6.1.3.1 -
Scrambling Control) requires that the scrambling enable bit is set
before sending any scrambled video signal. Using that solution thus
breaks that expectation.
The solution used by i915 is to do a full modeset on the connector so
that we disable the video signal, enable the scrambling bit, and enable
the video signal again.
As such, we took that code and plugged it into vc4. It probably could
have been turned into an helper, but it proved to be difficult for
several reasons:
* i915 has fairly different structures than simpler KMS drivers such
as vc4, so doing some code that works with both proved to be
difficult;
* Other simpler drivers could reuse some of it (tegra, dw-hdmi), but
it would still require to move some parameters currently stored in
private structure that are needed to compute whether the scrambling
is needed or not, and then inform the driver that it needs to be
enabled. Some of those parameters are already in core structures
(drm_display_mode, drm_display_info, bpc), but the output format
isnt't. Adding it is fairly challenging since unlike the TMDS char
rate or mode, there's no consensus on what format to pick in
drivers, so it's not possible to write some generic code that can
depend on it.
For these reasons, we chose to duplicate the code for now, until someone
else really needs it as well, in which case we will be able to convert
it into a generic helper.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 104 -
1 file changed, 101 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 2d988c73d12b..f136efe32047 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -242,9 +242,103 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi
*vc4_hdmi)
static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
#endif
-static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
+static int reset_pipe(struct drm_crtc *crtc,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_atomic_state *state;
+ struct drm_crtc_state *crtc_state;
+ int ret;
+
+ state = drm_atomic_state_alloc(crtc->dev);
+ if (!state)
+ return -ENOMEM;
+
+ state->acquire_ctx = ctx;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(crtc_state);
+ goto out;
+ }
+
+ crtc_state->connectors_changed = true;
+
+ ret = drm_atomic_commit(state);
+out:
+ drm_atomic_state_put(state);
+
+ return ret;
+}
+
+static int vc4_hdmi_reset_link(struct drm_connector *connector,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_device *drm = connector->dev;
+ struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
+ struct drm_encoder *encoder = &vc4_hdmi->encoder.base;
+ struct drm_connector_state *conn_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+ bool scrambling_needed;
+ u8 config;
+ int ret;
+
+ if (!connector)
+ return 0;
+
+ ret = drm_modeset_lock(&drm->mode_config.connection_mutex, ctx);
+ if (ret)
+ return ret;
+
+ conn_state = connector->state;
+ crtc = conn_state->crtc;
+ if (!crtc)
+ return 0;
+
+ ret = drm_modeset_lock(&crtc->mutex, ctx);
+ if (ret)
+ return ret;
+
+ crtc_state = crtc->state;
+ if (!crtc_state->active)
+ return 0;
+
+ if (!vc4_hdmi_supports_scrambling(encoder))
+ return 0;
+
+ scrambling_needed =
vc4_hdmi_mode_needs_scrambling(&vc4_hdmi->saved_adjusted_mode,
+ vc4_hdmi->output_bpc,
+
vc4_hdmi->output_format);
+ if (!scrambling_needed)
+ return 0;
+
+ if (conn_state->commit &&
+ !try_wait_for_completion(&conn_state->commit->hw_done))
+ return 0;
+
+ ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
+ if (ret <
[PATCH v3 8/8] drm/scdc: Document hotplug gotchas
There's some interactions between the SCDC setup and the disconnection / reconnection of displays. Let's document it and a solution. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/display/drm_scdc_helper.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c index 81881e81ceae..c3ad4ab2b456 100644 --- a/drivers/gpu/drm/display/drm_scdc_helper.c +++ b/drivers/gpu/drm/display/drm_scdc_helper.c @@ -35,6 +35,19 @@ * HDMI 2.0 specification. It is a point-to-point protocol that allows the * HDMI source and HDMI sink to exchange data. The same I2C interface that * is used to access EDID serves as the transport mechanism for SCDC. + * + * Note: The SCDC status is going to be lost when the display is + * disconnected. This can happen physically when the user disconnects + * the cable, but also when a display is switched on (such as waking up + * a TV). + * + * This is further complicated by the fact that, upon a disconnection / + * reconnection, KMS won't change the mode on its own. This means that + * one can't just rely on setting the SCDC status on enable, but also + * has to track the connector status changes using interrupts and + * restore the SCDC status. The typical solution for this is to trigger an + * empty modeset in drm_connector_helper_funcs.detect_ctx(), like what vc4 does + * in vc4_hdmi_reset_link(). */ #define SCDC_I2C_SLAVE_ADDRESS 0x54 -- 2.36.1
Re: [PATCH v4 13/13] video: backlight: mt6370: Add Mediatek MT6370 support
On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu wrote: > Andy Shevchenko 於 2022年7月13日 週三 晚上8:07寫道： > > On Wed, Jul 13, 2022 at 12:53 PM ChiaEn Wu wrote: > > > Andy Shevchenko 於 2022年7月5日 週二 清晨5:14寫道： > > > > On Mon, Jul 4, 2022 at 7:43 AM ChiaEn Wu wrote: Please, once again, remove unneeded context when replying! ^^^ ... > > > > > + prop_val = (ilog2(roundup_pow_of_two(prop_val)) + 1) > > > > > >> 1; > > > > > > > > Isn't something closer to get_order() or fls()? > > > > > > I will revise it to "(get_order(prop_va * PAGE_SIZE) + 1) / 2" and > > > this change is meet your expectations?? > > > > Nope. Try again. What about fls()? > > I have tried two methods so far, as follows > - > /* > * prop_val = 1 --> 1 steps --> b'00 > * prop_val = 2 ~ 4 --> 4 steps --> b'01 > * prop_val = 5 ~ 16 --> 16 steps --> b'10 > * prop_val = 17 ~ 64 --> 64 steps --> b'11 > */ So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3. Now, consider x - 1: 0 ( 0 ) --> 0 1 (2^0) --> 1 4 (2^2) --> 2 16 (2^4) --> 3 64 (2^6) --> ? (but let's consider that the range has been checked already) Since we take the lower limit, it means ffs(): y = (ffs(x - 1) + 1) / 2; Does it work for you? > // 1. use fls() and ffs() combination > prop_val = ffs(prop_val) == fls(prop_val) ? fls(prop_val) >> 1 : > (fls(prop_val) + 1) >> 1; > > // 2. use one line for-loop, but without fls() > for (i = --prop_val, prop_val = 0; i >> 2 * prop_val != 0; prop_val++); > - > Do these changes meet your expectations?? No, this is ugly. Yes, I understand that a bit arithmetics is hard... -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 7/9] dt-bindings: msm/dp: mark vdda supplies as deprecated
On 10/07/2022 10:41, Dmitry Baryshkov wrote:
> The commit fa384dd8b9b8 ("drm/msm/dp: delete vdda regulator related
> functions from eDP/DP controller") removed support for VDDA supplies
No such commit exists in next. Do not reference unpublished commits. If
this is your tree, be sure that it is in next.
> from the DP controller driver. These supplies are now handled by the eDP
> or QMP PHYs. Mark these properties as deprecated and drop them from the
> example.
Right now I cannot judge whether this is correct or not. I don't know
what's in that commit, but in general driver implementation changes do
not warrant changes in the binding.
Best regards,
Krzysztof
[PATCH v2] drm/bridge: it6505: Power on downstream device in .atomic_enable
Send DPCD DP_SET_POWER_D0 command to the monitor in .atomic_enable
callback. Without this command, some monitors won't show up again after
changing the resolution.
Fixes: 46ca7da7f1e8 ("drm/bridge: it6505: Send DPCD SET_POWER to downstream")
Signed-off-by: Pin-Yen Lin
---
Changes in v2:
- Update the typo in the summary (power on --> power off)
- Re-write the commit message to make it clearer.
drivers/gpu/drm/bridge/ite-it6505.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
b/drivers/gpu/drm/bridge/ite-it6505.c
index 4b673c4792d7..e5626035f311 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2945,6 +2945,9 @@ static void it6505_bridge_atomic_enable(struct drm_bridge
*bridge,
if (ret)
dev_err(dev, "Failed to setup AVI infoframe: %d", ret);
+ it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link,
+DP_SET_POWER_D0);
+
it6505_update_video_parameter(it6505, mode);
ret = it6505_send_video_infoframe(it6505, &frame);
--
2.37.0.144.g8ac04bfd2-goog
Re: [PATCH v2 8/9] dt-bindings: msm/dp: add missing properties
On 10/07/2022 10:41, Dmitry Baryshkov wrote: > Document missing definitions for opp-table (DP controller OPPs), aux-bus > (DP AUX BUS) and data-lanes (DP/eDP lanes mapping) properties. > > Reviewed-by: Stephen Boyd > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/dp-controller.yaml | 12 > 1 file changed, 12 insertions(+) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 13/13] video: backlight: mt6370: Add Mediatek MT6370 support
On Thu, Jul 14, 2022 at 11:27 AM Andy Shevchenko wrote: > > On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu wrote: > > Andy Shevchenko 於 2022年7月13日 週三 晚上8:07寫道： ... > > I have tried two methods so far, as follows > > - > > /* > > * prop_val = 1 --> 1 steps --> b'00 > > * prop_val = 2 ~ 4 --> 4 steps --> b'01 > > * prop_val = 5 ~ 16 --> 16 steps --> b'10 > > * prop_val = 17 ~ 64 --> 64 steps --> b'11 > > */ > > So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3. > Now, consider x - 1: > 0 ( 0 ) --> 0 > 1 (2^0) --> 1 > 4 (2^2) --> 2 > 16 (2^4) --> 3 > 64 (2^6) --> ? (but let's consider that the range has been checked already) > > Since we take the lower limit, it means ffs(): > > y = (ffs(x - 1) + 1) / 2; > > Does it work for you? It wouldn't, because we need to use fls() against it actually. So, 0..1 (-1..0) --> 0 2..4 (1..3) --> 1 5..16 (4..15) --> 2 17..64 (16..63) --> 3 y = x ? ((fls(x - 1) + 1) / 2 : 0; -- With Best Regards, Andy Shevchenko
Re: [PATCH v4 13/13] video: backlight: mt6370: Add Mediatek MT6370 support
On Thu, Jul 14, 2022 at 11:27:07AM +0200, Andy Shevchenko wrote: > On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu wrote: > > I have tried two methods so far, as follows > > - > > /* > > * prop_val = 1 --> 1 steps --> b'00 > > * prop_val = 2 ~ 4 --> 4 steps --> b'01 > > * prop_val = 5 ~ 16 --> 16 steps --> b'10 > > * prop_val = 17 ~ 64 --> 64 steps --> b'11 > > */ > > So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3. > Now, consider x - 1: > 0 ( 0 ) --> 0 > 1 (2^0) --> 1 > 4 (2^2) --> 2 > 16 (2^4) --> 3 > 64 (2^6) --> ? (but let's consider that the range has been checked already) > > Since we take the lower limit, it means ffs(): > > y = (ffs(x - 1) + 1) / 2; > > Does it work for you? To be honest, for this tiny table, writing code that *doesn't* require intricate deciphering together with a huge comment saying what is does would probably be better: prop_val = (prop_val <= 1 ? 0 : prop_val <= 4 ? 1 : prop_val <= 16 ? 2 : 3); This would be "obviously correct" and require no comment. Daniel.
Re: [PATCH v4 13/13] video: backlight: mt6370: Add Mediatek MT6370 support
On Thu, Jul 14, 2022 at 11:47 AM Daniel Thompson wrote: > > On Thu, Jul 14, 2022 at 11:27:07AM +0200, Andy Shevchenko wrote: > > On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu wrote: > > > I have tried two methods so far, as follows > > > - > > > /* > > > * prop_val = 1 --> 1 steps --> b'00 > > > * prop_val = 2 ~ 4 --> 4 steps --> b'01 > > > * prop_val = 5 ~ 16 --> 16 steps --> b'10 > > > * prop_val = 17 ~ 64 --> 64 steps --> b'11 > > > */ > > > > So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3. > > Now, consider x - 1: > > 0 ( 0 ) --> 0 > > 1 (2^0) --> 1 > > 4 (2^2) --> 2 > > 16 (2^4) --> 3 > > 64 (2^6) --> ? (but let's consider that the range has been checked already) > > > > Since we take the lower limit, it means ffs(): > > > > y = (ffs(x - 1) + 1) / 2; > > > > Does it work for you? > > To be honest, for this tiny table, writing code that *doesn't* require > intricate > deciphering together with a huge comment saying what is does would probably be > better: > > prop_val = (prop_val <= 1 ? 0 : > prop_val <= 4 ? 1 : > prop_val <= 16 ? 2 : > 3); > > This would be "obviously correct" and require no comment. Agree. It will also limit checking (and whatever needed for that). -- With Best Regards, Andy Shevchenko
Re: [PATCH] dma-buf: revert "return only unsignaled fences in dma_fence_unwrap_for_each v3"
Hi
Am 14.07.22 um 11:13 schrieb Christian König:
Am 14.07.22 um 11:06 schrieb Thomas Zimmermann:
Hi
Am 14.07.22 um 10:49 schrieb Christian König:
Hi Thomas,
Am 14.07.22 um 10:40 schrieb Thomas Zimmermann:
Hi Christian
Am 12.07.22 um 12:28 schrieb Christian König:
This reverts commit 8f61973718485f3e89bc4f408f929048b7b47c83.
I only found this commit in drm-misc-next. Should the revert be
cherry-picked into drm-misc-next-fixes?
yes for all three patches you just pinged me.
I've already tried to push them to drm-misc-next-fixes, but the
patches somehow wouldn't apply. I think the -next-fixes branch was
somehow lagging behind.
I just forwarded drm-misc-next-fixes to the latest state of drm-next.
Chances are, these patches will apply now.
Thanks, should I cherry pick them or are you going to do it?
Please go ahead. You know best what needs to be reverted.
And can we somehow make sure that when the drm-misc-next is merged into
drm-next for upstreaming that drm-misc-next-fixes is up to date as well?
That would make things much easier.
It's the drm-misc maintainer's job; me in this case. I simply was late.
I don't know if this update can be automated. For example, such that it
automatically merges drm-next back into drm-misc-next-fixes afer -rc6.
But agree that the transition period around -rc6 is a bit of a problem
each time.
Best regards
Thomas
Thanks,
Christian.
Best regards
Thomas
Thanks,
Christian.
Best regards
Thomas
It turned out that this is not correct. Especially the sync_file info
IOCTL needs to see even signaled fences to correctly report back their
status to userspace.
Instead add the filter in the merge function again where it makes
sense.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-fence-unwrap.c | 3 ++-
include/linux/dma-fence-unwrap.h | 6 +-
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-unwrap.c
b/drivers/dma-buf/dma-fence-unwrap.c
index 502a65ea6d44..7002bca792ff 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -72,7 +72,8 @@ struct dma_fence
*__dma_fence_unwrap_merge(unsigned int num_fences,
count = 0;
for (i = 0; i < num_fences; ++i) {
dma_fence_unwrap_for_each(tmp, &iter[i], fences[i])
- ++count;
+ if (!dma_fence_is_signaled(tmp))
+ ++count;
}
if (count == 0)
diff --git a/include/linux/dma-fence-unwrap.h
b/include/linux/dma-fence-unwrap.h
index 390de1ee9d35..66b1e56fbb81 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -43,14 +43,10 @@ struct dma_fence *dma_fence_unwrap_next(struct
dma_fence_unwrap *cursor);
* Unwrap dma_fence_chain and dma_fence_array containers and deep
dive into all
* potential fences in them. If @head is just a normal fence only
that one is
* returned.
- *
- * Note that signalled fences are opportunistically filtered out,
which
- * means the iteration is potentially over no fence at all.
*/
#define dma_fence_unwrap_for_each(fence, cursor, head) \
for (fence = dma_fence_unwrap_first(head, cursor); fence; \
- fence = dma_fence_unwrap_next(cursor)) \
- if (!dma_fence_is_signaled(fence))
+ fence = dma_fence_unwrap_next(cursor))
struct dma_fence *__dma_fence_unwrap_merge(unsigned int
num_fences,
struct dma_fence **fences,
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/fbdev: suspend HPD before fbdev unregistration
On 14.07.2022 05:09, Murthy, Arun R wrote: -Original Message- From: Hajda, Andrzej Sent: Wednesday, July 13, 2022 8:50 PM To: Jani Nikula ; Ville Syrjälä ; Murthy, Arun R Cc: Hajda, Andrzej ; Joonas Lahtinen ; Vivi, Rodrigo ; Tvrtko Ursulin ; Daniel Vetter ; intel-...@lists.freedesktop.org; dri- de...@lists.freedesktop.org Subject: [PATCH v3 2/3] drm/i915/fbdev: suspend HPD before fbdev unregistration HPD event after fbdev unregistration can cause registration of deferred fbdev which will not be unregistered later, causing use-after-free. To avoid it HPD handling should be suspended before fbdev unregistration. It should fix following GPF: [272.634530] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b6b: [#1] PREEMPT SMP NOPTI [272.634536] CPU: 0 PID: 6030 Comm: i915_selftest Tainted: G U 5.18.0-rc5-CI_DRM_11603-g12dccf4f5eef+ #1 [272.634541] Hardware name: Intel Corporation Raptor Lake Client Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.2397.A01.2109300731 09/30/2021 [272.634545] RIP: 0010:fb_do_apertures_overlap.part.14+0x26/0x60 ... [272.634582] Call Trace: [272.634583] [272.634585] do_remove_conflicting_framebuffers+0x59/0xa0 [272.634589] remove_conflicting_framebuffers+0x2d/0xc0 [272.634592] remove_conflicting_pci_framebuffers+0xc8/0x110 [272.634595] drm_aperture_remove_conflicting_pci_framebuffers+0x52/0x70 [272.634604] i915_driver_probe+0x63a/0xdd0 [i915] Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5329 Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5510 Signed-off-by: Andrzej Hajda --- Reviewed-by: Arun R Murthy Thanks and Regards, Arun R Murthy Ups, I forgot to add your r-b. Anyway, thanks for both r-b. Regards Andrzej
Re: [Intel-gfx] [PATCH v2 05/39] drm/i915: display: fix kernel-doc markup warnings
On Wed, 13 Jul 2022 18:05:06 -0400 Rodrigo Vivi wrote: > On Wed, Jul 13, 2022 at 09:11:53AM +0100, Mauro Carvalho Chehab wrote: > > There are a couple of issues at i915 display kernel-doc markups: > > > > drivers/gpu/drm/i915/display/intel_display_debugfs.c:2238: warning: > > Function parameter or member 'intel_connector' not described in > > 'intel_connector_debugfs_add' > > drivers/gpu/drm/i915/display/intel_display_debugfs.c:2238: warning: > > Excess function parameter 'connector' description in > > 'intel_connector_debugfs_add' > > drivers/gpu/drm/i915/display/intel_display_power.c:700: warning: > > expecting prototype for intel_display_power_put_async(). Prototype was for > > __intel_display_power_put_async() instead > > drivers/gpu/drm/i915/display/intel_tc.c:807: warning: Function > > parameter or member 'work' not described in > > 'intel_tc_port_disconnect_phy_work' > > drivers/gpu/drm/i915/display/intel_tc.c:807: warning: Excess function > > parameter 'dig_port' description in 'intel_tc_port_disconnect_phy_work' > > > > Those are due to wrong parameter of function name. Address them. > > > > Signed-off-by: Mauro Carvalho Chehab > > --- > > > > To avoid mailbombing on a large number of people, only mailing lists were > > C/C on the cover. > > See [PATCH v2 00/39] at: > > https://lore.kernel.org/all/cover.1657699522.git.mche...@kernel.org/ > > > > drivers/gpu/drm/i915/display/intel_display_debugfs.c | 2 +- > > drivers/gpu/drm/i915/display/intel_display_power.c | 2 +- > > drivers/gpu/drm/i915/display/intel_tc.c | 2 +- > > 3 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > index 6c3954479047..1e35eb01742b 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > @@ -2229,7 +2229,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_current_bpc); > > > > /** > > * intel_connector_debugfs_add - add i915 specific connector debugfs files > > - * @connector: pointer to a registered drm_connector > > + * @intel_connector: pointer to a registered drm_connector > > * > > * Cleanup will be done by drm_connector_unregister() through a call to > > * drm_debugfs_connector_remove(). > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > index 589af257edeb..fd6b71160a06 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > @@ -685,7 +685,7 @@ intel_display_power_put_async_work(struct work_struct > > *work) > > } > > > > /** > > - * intel_display_power_put_async - release a power domain reference > > asynchronously > > + * __intel_display_power_put_async - release a power domain reference > > asynchronously > > oh, we are really using __ prefix for non-static functions?! o.O Yeah... Btw, this is actually a common practice to have __ prefix on non-static and even on exported functions. Usually, the __ variant assumes that a spinlock/mutex were already taken previously. However, that's not the case here, as it starts holding a mutex. In this specific case, the __ variant is called by an inline function on a different way, depending if CONFIG_DRM_I915_DEBUG_RUNTIME_PM is true. On such case, it passes the runtime PM wakeref, otherwise it passes a -1. Funny enough, intel_display_power_put() ifdef is inside the C file, using a different implementation: #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM) ... void intel_display_power_put(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain, intel_wakeref_t wakeref) ... #else ... void intel_display_power_put_unchecked(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain) ... #endif For consistency, I would use the same solution for both, probably at the C file, and avoiding use a __ prefix for the async put version. > anyway, with that ditto "()" consistency, > > Reviewed-by: Rodrigo Vivi Thanks! Btw, I'm removing "()" from patches 1-3 (both at descriptions and internally), keeping your R-B on them too. Regards, Mauro
Re: [PATCH v1] drm/scheduler: Don't kill jobs in interrupt context
On 7/12/22 11:56, Dmitry Osipenko wrote: > On 7/6/22 18:46, Alex Deucher wrote: >> On Wed, Jul 6, 2022 at 9:49 AM Andrey Grodzovsky >> wrote: >>> >>> On 2022-07-06 03:07, Dmitry Osipenko wrote: >>> Hello Andrey, On 5/17/22 17:48, Dmitry Osipenko wrote: > On 5/17/22 17:13, Andrey Grodzovsky wrote: >> Done. >> >> Andrey > Awesome, thank you! > Given that this drm-scheduler issue needs to be fixed in the 5.19-RC and earlier, shouldn't it be in the drm-fixes and not in drm-next? >>> >>> >>> I pushed it into drm-misc from where it got into drm-next. I don't have >>> permission for drm-fixes. >> >> The -fixes branch of drm-misc. > > Now I don't see the scheduler bugfix neither in the -fixes branch nor in > the -next and today Dave sent out 5.19-rc7 pull request without the > scheduler fix. Could anyone please check what is going on with the DRM > patches? Thanks! > > https://github.com/freedesktop/drm-misc/commits/drm-misc-fixes > https://cgit.freedesktop.org/drm/drm-misc/log/?h=drm-misc-fixes The patch is in the drm-misc-next-fixes, so it wasn't moved to the drm-misc-fixes. Andrey, don't you have access to drm-misc-fixes? Or you meant drm-fixes=drm-misc-fixes? -- Best regards, Dmitry
Re: [Intel-gfx] [PATCH v2 06/39] drm/i915: gt: fix some Kernel-doc issues
On Wed, 13 Jul 2022 18:07:44 -0400 Rodrigo Vivi wrote: > On Wed, Jul 13, 2022 at 09:11:54AM +0100, Mauro Carvalho Chehab wrote: > > There are several trivial warnings there, due to trivial things: > > - lack of function name at the kerneldoc markup; > > - undocumented structs with kernel-doc markups; > > - wrong parameter syntax. > > > > Fix such warnings: > > > > drivers/gpu/drm/i915/gt/intel_context.h:107: warning: Function > > parameter or member 'ce' not described in 'intel_context_lock_pinned' > > drivers/gpu/drm/i915/gt/intel_context.h:122: warning: Function > > parameter or member 'ce' not described in 'intel_context_is_pinned' > > drivers/gpu/drm/i915/gt/intel_context.h:141: warning: Function > > parameter or member 'ce' not described in 'intel_context_unlock_pinned' > > drivers/gpu/drm/i915/gt/intel_gtt.h:510: warning: Function parameter or > > member 'vm' not described in 'i915_vm_resv_put' > > drivers/gpu/drm/i915/gt/intel_gtt.h:510: warning: Excess function > > parameter 'resv' description in 'i915_vm_resv_put' > > drivers/gpu/drm/i915/gt/intel_gtt.h:615: warning: Function parameter or > > member 'i915' not described in 'i915_ggtt_mark_pte_lost' > > drivers/gpu/drm/i915/gt/intel_gtt.h:615: warning: Function parameter or > > member 'val' not described in 'i915_ggtt_mark_pte_lost' > > drivers/gpu/drm/i915/gt/intel_rps.c:2343: warning: This comment starts > > with '/**', but isn't a kernel-doc comment. Refer > > Documentation/doc-guide/kernel-doc.rst > > * Tells the intel_ips driver that the i915 driver is now loaded, if > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:28: warning: Function > > parameter or member 'size' not described in '__guc_capture_bufstate' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:28: warning: Function > > parameter or member 'data' not described in '__guc_capture_bufstate' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:28: warning: Function > > parameter or member 'rd' not described in '__guc_capture_bufstate' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:28: warning: Function > > parameter or member 'wr' not described in '__guc_capture_bufstate' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:60: warning: Function > > parameter or member 'link' not described in '__guc_capture_parsed_output' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:60: warning: Function > > parameter or member 'is_partial' not described in > > '__guc_capture_parsed_output' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:60: warning: Function > > parameter or member 'eng_class' not described in > > '__guc_capture_parsed_output' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:60: warning: Function > > parameter or member 'eng_inst' not described in > > '__guc_capture_parsed_output' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:60: warning: Function > > parameter or member 'guc_id' not described in '__guc_capture_parsed_output' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:60: warning: Function > > parameter or member 'lrca' not described in '__guc_capture_parsed_output' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:60: warning: Function > > parameter or member 'reginfo' not described in '__guc_capture_parsed_output' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:63: warning: wrong > > kernel-doc identifier on line: > > * struct guc_debug_capture_list_header / struct guc_debug_capture_list > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:81: warning: wrong > > kernel-doc identifier on line: > > * struct __guc_mmio_reg_descr / struct __guc_mmio_reg_descr_group > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:106: warning: wrong > > kernel-doc identifier on line: > > * struct guc_state_capture_header_t / struct guc_state_capture_t / > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:164: warning: Function > > parameter or member 'is_valid' not described in '__guc_capture_ads_cache' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:164: warning: Function > > parameter or member 'ptr' not described in '__guc_capture_ads_cache' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:164: warning: Function > > parameter or member 'size' not described in '__guc_capture_ads_cache' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:164: warning: Function > > parameter or member 'status' not described in '__guc_capture_ads_cache' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:217: warning: Function > > parameter or member 'ads_null_cache' not described in > > 'intel_guc_state_capture' > > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h:217: warning: Function > > parameter or member 'max_mmio_per_node' not described in > > 'intel_guc_state_capture' > > drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h:401: warning: Function > > parameter or member 'marker' not described in 'guc_log_buffer_state' > >
[PATCH] drm/amdgpu: Fix for drm buddy memory corruption
User reported gpu page fault when running graphics applications
and in some cases garbaged graphics are observed as soon as X
starts. This patch fixes all the issues.
Fixed the typecast issue for fpfn and lpfn variables, thus
preventing the overflow problem which resolves the memory
corruption.
Signed-off-by: Arunpravin Paneer Selvam
Reported-by: Mike Lothian
Tested-by: Mike Lothian
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 16
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 2 +-
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 49e4092f447f..34d789054ec8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -366,11 +366,11 @@ static int amdgpu_vram_mgr_new(struct
ttm_resource_manager *man,
unsigned long pages_per_block;
int r;
- lpfn = place->lpfn << PAGE_SHIFT;
+ lpfn = (u64)place->lpfn << PAGE_SHIFT;
if (!lpfn)
lpfn = man->size;
- fpfn = place->fpfn << PAGE_SHIFT;
+ fpfn = (u64)place->fpfn << PAGE_SHIFT;
max_bytes = adev->gmc.mc_vram_size;
if (tbo->type != ttm_bo_type_kernel)
@@ -410,12 +410,12 @@ static int amdgpu_vram_mgr_new(struct
ttm_resource_manager *man,
/* Allocate blocks in desired range */
vres->flags |= DRM_BUDDY_RANGE_ALLOCATION;
- remaining_size = vres->base.num_pages << PAGE_SHIFT;
+ remaining_size = (u64)vres->base.num_pages << PAGE_SHIFT;
mutex_lock(&mgr->lock);
while (remaining_size) {
if (tbo->page_alignment)
- min_block_size = tbo->page_alignment << PAGE_SHIFT;
+ min_block_size = (u64)tbo->page_alignment << PAGE_SHIFT;
else
min_block_size = mgr->default_page_size;
@@ -424,12 +424,12 @@ static int amdgpu_vram_mgr_new(struct
ttm_resource_manager *man,
/* Limit maximum size to 2GiB due to SG table limitations */
size = min(remaining_size, 2ULL << 30);
- if (size >= pages_per_block << PAGE_SHIFT)
- min_block_size = pages_per_block << PAGE_SHIFT;
+ if (size >= (u64)pages_per_block << PAGE_SHIFT)
+ min_block_size = (u64)pages_per_block << PAGE_SHIFT;
cur_size = size;
- if (fpfn + size != place->lpfn << PAGE_SHIFT) {
+ if (fpfn + size != (u64)place->lpfn << PAGE_SHIFT) {
/*
* Except for actual range allocation, modify the size
and
* min_block_size conforming to continuous flag
enablement
@@ -469,7 +469,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager
*man,
LIST_HEAD(temp);
trim_list = &vres->blocks;
- original_size = vres->base.num_pages << PAGE_SHIFT;
+ original_size = (u64)vres->base.num_pages << PAGE_SHIFT;
/*
* If size value is rounded up to min_block_size, trim the last
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
index 9a2db87186c7..bef0f561ba60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
@@ -50,7 +50,7 @@ static inline u64 amdgpu_vram_mgr_block_start(struct
drm_buddy_block *block)
static inline u64 amdgpu_vram_mgr_block_size(struct drm_buddy_block *block)
{
- return PAGE_SIZE << drm_buddy_block_order(block);
+ return (u64)PAGE_SIZE << drm_buddy_block_order(block);
}
static inline struct drm_buddy_block *
--
2.25.1
Re: [PATCH v2 7/9] dt-bindings: msm/dp: mark vdda supplies as deprecated
On 14/07/2022 12:38, Krzysztof Kozlowski wrote:
On 10/07/2022 10:41, Dmitry Baryshkov wrote:
The commit fa384dd8b9b8 ("drm/msm/dp: delete vdda regulator related
functions from eDP/DP controller") removed support for VDDA supplies
No such commit exists in next. Do not reference unpublished commits. If
this is your tree, be sure that it is in next.
Excuse me. It might have changed at some point. I will update the patch
description in the next revision. The commit in question is 7516351bebc1
("drm/msm/dp: delete vdda regulator related functions from eDP/DP
controller")
from the DP controller driver. These supplies are now handled by the eDP
or QMP PHYs. Mark these properties as deprecated and drop them from the
example.
Right now I cannot judge whether this is correct or not. I don't know
what's in that commit, but in general driver implementation changes do
not warrant changes in the binding.
The vdda supplies were initially made a part of DP controller binding,
however lately they were moved to be a part of eDP/DP PHY binding (as
this better reflects the hardware). DP driver dropped support for these
supplies too. Thus I wanted to mark these supplies as deprecated to
discourage using them in the DTS files.
--
With best wishes
Dmitry
Re: [PATCH v4 13/13] video: backlight: mt6370: Add Mediatek MT6370 support
On Thu, Jul 14, 2022 at 11:43 AM Andy Shevchenko wrote: > On Thu, Jul 14, 2022 at 11:27 AM Andy Shevchenko > wrote: > > On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu wrote: > > > Andy Shevchenko 於 2022年7月13日 週三 晚上8:07寫道： ... > > > * prop_val = 1 --> 1 steps --> b'00 > > > * prop_val = 2 ~ 4 --> 4 steps --> b'01 > > > * prop_val = 5 ~ 16 --> 16 steps --> b'10 > > > * prop_val = 17 ~ 64 --> 64 steps --> b'11 > > > > So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3. > > Now, consider x - 1: > > 0 ( 0 ) --> 0 > > 1 (2^0) --> 1 > > 4 (2^2) --> 2 > > 16 (2^4) --> 3 > > 64 (2^6) --> ? (but let's consider that the range has been checked already) > > > > Since we take the lower limit, it means ffs(): > > > > y = (ffs(x - 1) + 1) / 2; > > > > Does it work for you? > > It wouldn't, because we need to use fls() against it actually. > > So, > 0..1 (-1..0) --> 0 > 2..4 (1..3) --> 1 > 5..16 (4..15) --> 2 > 17..64 (16..63) --> 3 > > y = x ? ((fls(x - 1) + 1) / 2 : 0; Okay, I nailed it down, but Daniel is right, it's simpler to have just conditionals. y = x >=2 ? __fls(x - 1) / 2 + 1 : 0; -- With Best Regards, Andy Shevchenko
[Bug 201957] amdgpu: ring gfx timeout
https://bugzilla.kernel.org/show_bug.cgi?id=201957 --- Comment #81 from Danil (s48g...@gmail.com) --- Nvidia released 515.57 drivers that fix "Nvidia being broken when used as second GPU in Linux", my bug above. Nvidia GPU works again when AMD GPU main. -- 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 v3 00/11] drm/msm/dsi_phy: Replace parent names with clk_hw pointers
On 30/06/2022 01:53, Marijn Suijten wrote: parent_hw pointers are easier to manage and cheaper to use than repeatedly formatting the parent name and subsequently leaving the clk framework to perform lookups based on that name. This series starts out by adding extra constructors for divider, mux and fixed-factor clocks that have parent_hw(s) pointer argument(s) instead of some DT index or name. Followed by individual patches performing the conversion, one DSI PHY at a time. dsi_phy_28nm_8960 includes an extra fixup to replace "eternal" devm_kzalloc allocations (for the lifetime of the device) with stack-local char arrays, like all the other DSI PHY drivers. (Questions from v1 cover letter regarding the future of these drivers is omitted for brevity.) And with enough future improvements out of the way, let's round out this patch-series by stating that it has been successfully tested on: - Sony Nile Discovery (Xperia XA2 Ultra): 14nm; - Sony Seine PDX201 (Xperia 10II): 14nm; - Sony Loire Suzu (Xperia X): 28nm. And no diff is observed in debugfs's clk_summary. Unfortunately all other devices in my collection with a 7/10nm DSI PHY have a DSC panel which we have yet to get working. Changes since v2: - in fixed-factor: - Reorder if - else if change to consume less diff; - Go over 80-char column lint when adding new arguments to function calls, instead of reflowing all arguments to adhere to this limit; also consuming less diff. v2: https://lore.kernel.org/linux-arm-msm/20220601220747.1145095-1-marijn.suij...@somainline.org/ Changes since v1: - Moved indentation changes to separate patch (Dmitry); - dsi_phy_28nm_8960: move clock name allocation removal prior to parent_hw refactor; - Remove vco_name stack-local char array in favour of reusing clk_name (Dmitry); - Inserted additional patch to replace hardcoded char-array length constant 32 with sizeof(clk_name). v1: https://lore.kernel.org/linux-arm-msm/20220523213837.1016542-1-marijn.suij...@somainline.org/T/#u Marijn Suijten (11): clk: divider: Introduce devm_clk_hw_register_divider_parent_hw() clk: mux: Introduce devm_clk_hw_register_mux_parent_hws() clk: fixed-factor: Introduce *clk_hw_register_fixed_factor_parent_hw() Stephen, do we stand a chance of landing patches 1-3 into 5.20? We would like to merge the series into 5.21 through the msm-next. Landing clk patches in 5.20 would save us from using immutable branches, etc. drm/msm/dsi/phy: Reindent and reflow multiline function calls drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names drm/msm/dsi/phy: Replace hardcoded char-array length with sizeof() drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers drivers/clk/clk-fixed-factor.c| 45 - drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c| 165 +- drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c| 55 +++--- drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c| 117 ++--- .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 90 +- drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 156 - include/linux/clk-provider.h | 34 7 files changed, 351 insertions(+), 311 deletions(-) -- With best wishes Dmitry
[PATCH v2 5/5] drm/modes: parse_cmdline: Add support for named modes containing dashes
It is fairly common for named video modes to contain dashes (e.g. "tt-mid" on Atari, "dblntsc-ff" on Amiga). Currently such mode names are not recognized, as the dash is considered to be a separator between mode name and bpp. Fix this by skipping any dashes that are not followed immediately by a digit when looking for the separator. Signed-off-by: Geert Uytterhoeven Reviewed-by: Hans de Goede --- v2: - Add Reviewed-by. --- drivers/gpu/drm/drm_modes.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index bfc3a08614522689..c5f490de70314176 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1827,6 +1827,8 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, /* Try to locate the bpp and refresh specifiers, if any */ bpp_ptr = strchr(name, '-'); + while (bpp_ptr && !isdigit(bpp_ptr[1])) + bpp_ptr = strchr(bpp_ptr + 1, '-'); if (bpp_ptr) bpp_off = bpp_ptr - name; -- 2.25.1
[PATCH v2 4/5] drm/modes: Add support for driver-specific named modes
The mode parsing code recognizes named modes only if they are explicitly
listed in the internal whitelist, which is currently limited to "NTSC"
and "PAL".
Provide a mechanism for drivers to override this list to support custom
mode names.
Ideally, this list should just come from the driver's actual list of
modes, but connector->probed_modes is not yet populated at the time of
parsing.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Hans de Goede
---
v2:
- Add Reviewed-by.
---
drivers/gpu/drm/drm_modes.c | 15 +++
include/drm/drm_connector.h | 10 ++
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 0cbf0467f263b30a..bfc3a08614522689 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1748,24 +1748,30 @@ static int drm_mode_parse_cmdline_options(const char
*str,
static const char * const drm_named_modes_whitelist[] = {
"NTSC",
"PAL",
+ NULL
};
static int drm_mode_parse_cmdline_named_mode(const char *name,
unsigned int length, bool refresh,
+const struct drm_connector
*connector,
struct drm_cmdline_mode *mode)
{
+ const char * const *named_modes_whitelist;
unsigned int i;
int ret;
- for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
- ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
+ named_modes_whitelist = connector->named_modes_whitelist ? :
+ drm_named_modes_whitelist;
+
+ for (i = 0; named_modes_whitelist[i]; i++) {
+ ret = str_has_prefix(name, named_modes_whitelist[i]);
if (ret != length)
continue;
if (refresh)
return -EINVAL; /* named + refresh is invalid */
- strcpy(mode->name, drm_named_modes_whitelist[i]);
+ strcpy(mode->name, named_modes_whitelist[i]);
mode->specified = true;
return 0;
}
@@ -1849,7 +1855,8 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
/* First check for a named mode */
if (mode_end) {
ret = drm_mode_parse_cmdline_named_mode(name, mode_end,
- refresh_ptr, mode);
+ refresh_ptr, connector,
+ mode);
if (ret)
return false;
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 3ac4bf87f2571c4c..6361f8a596c01107 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1659,6 +1659,16 @@ struct drm_connector {
/** @hdr_sink_metadata: HDR Metadata Information read from sink */
struct hdr_sink_metadata hdr_sink_metadata;
+
+ /**
+* @named_modes_whitelist:
+*
+* Optional NULL-terminated array of names to be considered valid mode
+* names. This lets the command line option parser distinguish between
+* mode names and freestanding extras and/or options.
+* If not set, a set of defaults will be used.
+*/
+ const char * const *named_modes_whitelist;
};
#define obj_to_connector(x) container_of(x, struct drm_connector, base)
--
2.25.1
[PATCH 2/2] drm/etnaviv: reap idle mapping if it doesn't match the softpin address
When a idle BO, which is held open by another process, gets freed by
userspace and subsequently referenced again by e.g. importing it again,
userspace may assign a different softpin VA than the last time around.
As the kernel GEM object still exists, we likely have a idle mapping
with the old VA still cached, if it hasn't been reaped in the meantime.
As the context matches, we then simply try to resurrect this mapping by
increasing the refcount. As the VA in this mapping does not match the
new softpin address, we consequently fail the otherwise valid submit.
Instead of failing, reap the idle mapping.
Cc: sta...@vger.kernel.org # 5.19
Signed-off-by: Lucas Stach
---
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index cc386f8a7116..5cf13e52f7c9 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -258,7 +258,12 @@ struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
if (mapping->use == 0) {
mutex_lock(&mmu_context->lock);
if (mapping->context == mmu_context)
- mapping->use += 1;
+ if (va && mapping->iova != va) {
+ etnaviv_iommu_reap_mapping(mapping);
+ mapping = NULL;
+ } else {
+ mapping->use += 1;
+ }
else
mapping = NULL;
mutex_unlock(&mmu_context->lock);
--
2.30.2
[PATCH 1/2] drm/etnaviv: move idle mapping reaping into separate function
The same logic is already used in two different places and now
it will also be needed outside of the compilation unit, so split
it into a separate function.
Cc: sta...@vger.kernel.org # 5.19
Signed-off-by: Lucas Stach
---
drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 23 +++
drivers/gpu/drm/etnaviv/etnaviv_mmu.h | 1 +
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
index dc1aa738c4f1..55479cb8b1ac 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
@@ -135,6 +135,19 @@ static void etnaviv_iommu_remove_mapping(struct
etnaviv_iommu_context *context,
drm_mm_remove_node(&mapping->vram_node);
}
+void etnaviv_iommu_reap_mapping(struct etnaviv_vram_mapping *mapping)
+{
+ struct etnaviv_iommu_context *context = mapping->context;
+
+ lockdep_assert_held(&context->lock);
+ WARN_ON(mapping->use);
+
+ etnaviv_iommu_remove_mapping(context, mapping);
+ etnaviv_iommu_context_put(mapping->context);
+ mapping->context = NULL;
+ list_del_init(&mapping->mmu_node);
+}
+
static int etnaviv_iommu_find_iova(struct etnaviv_iommu_context *context,
struct drm_mm_node *node, size_t size)
{
@@ -202,10 +215,7 @@ static int etnaviv_iommu_find_iova(struct
etnaviv_iommu_context *context,
* this mapping.
*/
list_for_each_entry_safe(m, n, &list, scan_node) {
- etnaviv_iommu_remove_mapping(context, m);
- etnaviv_iommu_context_put(m->context);
- m->context = NULL;
- list_del_init(&m->mmu_node);
+ etnaviv_iommu_reap_mapping(m);
list_del_init(&m->scan_node);
}
@@ -257,10 +267,7 @@ static int etnaviv_iommu_insert_exact(struct
etnaviv_iommu_context *context,
}
list_for_each_entry_safe(m, n, &scan_list, scan_node) {
- etnaviv_iommu_remove_mapping(context, m);
- etnaviv_iommu_context_put(m->context);
- m->context = NULL;
- list_del_init(&m->mmu_node);
+ etnaviv_iommu_reap_mapping(m);
list_del_init(&m->scan_node);
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.h
b/drivers/gpu/drm/etnaviv/etnaviv_mmu.h
index e4a0b7d09c2e..c01a147f0dfd 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.h
@@ -91,6 +91,7 @@ int etnaviv_iommu_map_gem(struct etnaviv_iommu_context
*context,
struct etnaviv_vram_mapping *mapping, u64 va);
void etnaviv_iommu_unmap_gem(struct etnaviv_iommu_context *context,
struct etnaviv_vram_mapping *mapping);
+void etnaviv_iommu_reap_mapping(struct etnaviv_vram_mapping *mapping);
int etnaviv_iommu_get_suballoc_va(struct etnaviv_iommu_context *ctx,
struct etnaviv_vram_mapping *mapping,
--
2.30.2
[PATCH][next] drm/amd/display: Fix spelling mistake "supporing" -> "supporting"
There is a spelling mistake in a dml_print message. Fix it.
Signed-off-by: Colin Ian King
---
.../gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c| 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
b/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
index 6101c962ab0a..fc4d7474c111 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
@@ -2994,7 +2994,7 @@ static void
DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerforman
for (k = 0; k < v->NumberOfActivePlanes; ++k) {
if (v->ImmediateFlipSupportedForPipe[k] ==
false) {
#ifdef __DML_VBA_DEBUG__
- dml_print("DML::%s: Pipe %0d not
supporing iflip\n", __func__, k);
+ dml_print("DML::%s: Pipe %0d not
supporting iflip\n", __func__, k);
#endif
v->ImmediateFlipSupported = false;
}
--
2.35.3
Re: [PATCH v2 7/9] dt-bindings: msm/dp: mark vdda supplies as deprecated
On 14/07/2022 12:15, Dmitry Baryshkov wrote:
> On 14/07/2022 12:38, Krzysztof Kozlowski wrote:
>> On 10/07/2022 10:41, Dmitry Baryshkov wrote:
>>> The commit fa384dd8b9b8 ("drm/msm/dp: delete vdda regulator related
>>> functions from eDP/DP controller") removed support for VDDA supplies
>>
>> No such commit exists in next. Do not reference unpublished commits. If
>> this is your tree, be sure that it is in next.
>
> Excuse me. It might have changed at some point. I will update the patch
> description in the next revision. The commit in question is 7516351bebc1
> ("drm/msm/dp: delete vdda regulator related functions from eDP/DP
> controller")
>
>>
>>> from the DP controller driver. These supplies are now handled by the eDP
>>> or QMP PHYs. Mark these properties as deprecated and drop them from the
>>> example.
>>
>> Right now I cannot judge whether this is correct or not. I don't know
>> what's in that commit, but in general driver implementation changes do
>> not warrant changes in the binding.
>
> The vdda supplies were initially made a part of DP controller binding,
> however lately they were moved to be a part of eDP/DP PHY binding (as
> this better reflects the hardware). DP driver dropped support for these
> supplies too. Thus I wanted to mark these supplies as deprecated to
> discourage using them in the DTS files.
OK. Just better to reference the commit which adds them to PHY binding.
Best regards,
Krzysztof
[PATCH v2 2/5] drm/modes: Extract drm_mode_parse_cmdline_named_mode()
Extract the code to check for a named mode parameter into its own
function, to streamline the main parsing flow.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Hans de Goede
Acked-by: Thomas Zimmermann
---
v2:
- Add Reviewed-by, Acked-by,
- Fix length check.
---
drivers/gpu/drm/drm_modes.c | 40 +++--
1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 67773740c74c9ba0..a3df18fccb31fa77 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1749,6 +1749,29 @@ static const char * const drm_named_modes_whitelist[] = {
"PAL",
};
+static int drm_mode_parse_cmdline_named_mode(const char *name,
+unsigned int length, bool refresh,
+struct drm_cmdline_mode *mode)
+{
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
+ ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
+ if (ret != length)
+ continue;
+
+ if (refresh)
+ return -EINVAL; /* named + refresh is invalid */
+
+ strcpy(mode->name, drm_named_modes_whitelist[i]);
+ mode->specified = true;
+ return 0;
+ }
+
+ return 0;
+}
+
/**
* drm_mode_parse_command_line_for_connector - parse command line modeline for
connector
* @mode_option: optional per connector mode option
@@ -1785,7 +1808,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
const char *options_ptr = NULL;
char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
- int i, len, ret;
+ int len, ret;
memset(mode, 0, sizeof(*mode));
mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
@@ -1823,16 +1846,11 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
}
/* First check for a named mode */
- for (i = 0; mode_end && i < ARRAY_SIZE(drm_named_modes_whitelist); i++)
{
- ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
- if (ret == mode_end) {
- if (refresh_ptr)
- return false; /* named + refresh is invalid */
-
- strcpy(mode->name, drm_named_modes_whitelist[i]);
- mode->specified = true;
- break;
- }
+ if (mode_end) {
+ ret = drm_mode_parse_cmdline_named_mode(name, mode_end,
+ refresh_ptr, mode);
+ if (ret)
+ return false;
}
/* No named mode? Check for a normal mode argument, e.g. 1024x768 */
--
2.25.1
[PATCH v2 3/5] drm/modes: parse_cmdline: Make mode->*specified handling more uniform
The various mode->*specified flags are not handled in an uniform way:
some flags are set by the corresponding drm_mode_parse_cmdline_*()
function, some flags by the caller of the function, and some flags by
both.
Make this uniform by making this the responsibility of the various
parsing helpers, i.e.
- Move the setting of mode->specified from caller to callee,
- Drop the duplicate setting of mode->bpp_specified and
mode->refresh_specified from callers.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Hans de Goede
Acked-by: Thomas Zimmermann
---
v2:
- Add Reviewed-by, Acked-by.
---
drivers/gpu/drm/drm_modes.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index a3df18fccb31fa77..0cbf0467f263b30a 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1599,6 +1599,7 @@ static int drm_mode_parse_cmdline_res_mode(const char
*str, unsigned int length,
mode->yres = yres;
mode->cvt = cvt;
mode->rb = rb;
+ mode->specified = true;
return 0;
}
@@ -1861,8 +1862,6 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
mode);
if (ret)
return false;
-
- mode->specified = true;
}
/* No mode? Check for freestanding extras and/or options */
@@ -1884,8 +1883,6 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
ret = drm_mode_parse_cmdline_bpp(bpp_ptr, &bpp_end_ptr, mode);
if (ret)
return false;
-
- mode->bpp_specified = true;
}
if (refresh_ptr) {
@@ -1893,8 +1890,6 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
&refresh_end_ptr, mode);
if (ret)
return false;
-
- mode->refresh_specified = true;
}
/*
--
2.25.1
[PATCH 01/10] drm/sched: move calling drm_sched_entity_select_rq
We already discussed that the call to drm_sched_entity_select_rq() needs
to move to drm_sched_job_arm() to be able to set a new scheduler list
between _init() and _arm(). This was just not applied for some reason.
Signed-off-by: Christian König
CC: Andrey Grodzovsky
CC: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/scheduler/sched_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 68317d3a7a27..e0ab14e0fb6b 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -592,7 +592,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
struct drm_sched_entity *entity,
void *owner)
{
- drm_sched_entity_select_rq(entity);
if (!entity->rq)
return -ENOENT;
@@ -628,7 +627,7 @@ void drm_sched_job_arm(struct drm_sched_job *job)
struct drm_sched_entity *entity = job->entity;
BUG_ON(!entity);
-
+ drm_sched_entity_select_rq(entity);
sched = entity->rq->sched;
job->sched = sched;
--
2.25.1
[PATCH v2 1/5] drm/modes: parse_cmdline: Handle empty mode name part
If no mode name part was specified, mode_end is zero, and the "ret ==
mode_end" check does the wrong thing.
Fix this by skipping all named mode handling when mode_end is zero.
Fixes: 7b1cce760afe38b4 ("drm/modes: parse_cmdline: Allow specifying
stand-alone options")
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Hans de Goede
Acked-by: Thomas Zimmermann
---
v2:
- Add Reviewed-by, Acked-by,
- Keep "ret == mode_end" check.
---
drivers/gpu/drm/drm_modes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 14b746f7ba975954..67773740c74c9ba0 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1823,7 +1823,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
}
/* First check for a named mode */
- for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
+ for (i = 0; mode_end && i < ARRAY_SIZE(drm_named_modes_whitelist); i++)
{
ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
if (ret == mode_end) {
if (refresh_ptr)
--
2.25.1
[PATCH v2 0/5] drm/modes: Command line mode selection fixes and improvements
Hi all,
This patch series contains fixes and improvements for specifying video
modes on the kernel command line.
Changes compared to v1[1]:
- Add Reviewed-by, Acked-by,
- Keep length check.
This has been tested on ARAnyM using a work-in-progress Atari DRM driver
(more info and related patches can be found in [2]).
Thanks for your comments!
[1] "[PATCH 0/5] drm/modes: Command line mode selection fixes and
improvements"
https://lore.kernel.org/r/cover.1657301107.git.ge...@linux-m68k.org
[2] "[PATCH v3 00/10] drm: Add support for low-color frame buffer formats"
https://lore.kernel.org/r/cover.1657294931.git.ge...@linux-m68k.org
Geert Uytterhoeven (5):
drm/modes: parse_cmdline: Handle empty mode name part
drm/modes: Extract drm_mode_parse_cmdline_named_mode()
drm/modes: parse_cmdline: Make mode->*specified handling more uniform
drm/modes: Add support for driver-specific named modes
drm/modes: parse_cmdline: Add support for named modes containing
dashes
drivers/gpu/drm/drm_modes.c | 56 ++---
include/drm/drm_connector.h | 10 +++
2 files changed, 49 insertions(+), 17 deletions(-)
--
2.25.1
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 v2] drm/bridge: it6505: Power on downstream device in .atomic_enable
Reviewed-by: Allen Chen
-Original Message-
From: Pin-Yen Lin
Sent: Thursday, July 14, 2022 5:39 PM
To: Andrzej Hajda ; Neil Armstrong
; Robert Foss ; Laurent
Pinchart ; Jonas Karlman ;
Jernej Skrabec ; David Airlie ;
Daniel Vetter
Cc: Hsin-Yi Wang ; Allen Chen (陳柏宇)
; Pin-Yen Lin ;
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: [PATCH v2] drm/bridge: it6505: Power on downstream device in
.atomic_enable
Send DPCD DP_SET_POWER_D0 command to the monitor in .atomic_enable callback.
Without this command, some monitors won't show up again after changing the
resolution.
Fixes: 46ca7da7f1e8 ("drm/bridge: it6505: Send DPCD SET_POWER to downstream")
Signed-off-by: Pin-Yen Lin
---
Changes in v2:
- Update the typo in the summary (power on --> power off)
- Re-write the commit message to make it clearer.
drivers/gpu/drm/bridge/ite-it6505.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
b/drivers/gpu/drm/bridge/ite-it6505.c
index 4b673c4792d7..e5626035f311 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2945,6 +2945,9 @@ static void it6505_bridge_atomic_enable(struct drm_bridge
*bridge,
if (ret)
dev_err(dev, "Failed to setup AVI infoframe: %d", ret);
+ it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link,
+DP_SET_POWER_D0);
+
it6505_update_video_parameter(it6505, mode);
ret = it6505_send_video_infoframe(it6505, &frame);
--
2.37.0.144.g8ac04bfd2-goog
Re: [PATCH v14 03/10] drm/edid: Add cea_sad helpers for freq/length
Il 12/07/22 13:12, Bo-Chen Chen ha scritto:
From: Guillaume Ranquet
This patch adds two helper functions that extract the frequency and word
length from a struct cea_sad.
For these helper functions new defines are added that help translate the
'freq' and 'byte2' fields into real numbers.
Signed-off-by: Markus Schneider-Pargmann
Signed-off-by: Guillaume Ranquet
Signed-off-by: Bo-Chen Chen
---
drivers/gpu/drm/drm_edid.c | 73 ++
include/drm/drm_edid.h | 14
2 files changed, 87 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bc43e1b32092..79316d7f1fd8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4916,6 +4916,79 @@ int drm_edid_to_speaker_allocation(const struct edid
*edid, u8 **sadb)
}
EXPORT_SYMBOL(drm_edid_to_speaker_allocation);
+/**
+ * drm_cea_sad_get_sample_rate - Extract the sample rate from cea_sad
+ * @sad: Pointer to the cea_sad struct
+ *
+ * Extracts the cea_sad frequency field and returns the sample rate in Hz.
+ *
+ * Return: Sample rate in Hz or a negative errno if parsing failed.
+ */
+int drm_cea_sad_get_sample_rate(const struct cea_sad *sad)
+{
+ switch (sad->freq) {
+ case DRM_CEA_SAD_FREQ_32KHZ:
+ return 32000;
+ case DRM_CEA_SAD_FREQ_44KHZ:
+ return 44100;
+ case DRM_CEA_SAD_FREQ_48KHZ:
+ return 48000;
+ case DRM_CEA_SAD_FREQ_88KHZ:
+ return 88200;
+ case DRM_CEA_SAD_FREQ_96KHZ:
+ return 96000;
+ case DRM_CEA_SAD_FREQ_176KHZ:
+ return 176400;
+ case DRM_CEA_SAD_FREQ_192KHZ:
+ return 192000;
+ default:
+ return -EINVAL;
+ }
+}
+EXPORT_SYMBOL(drm_cea_sad_get_sample_rate);
+
+static bool drm_cea_sad_is_pcm(const struct cea_sad *sad)
+{
+ switch (sad->format) {
+ case HDMI_AUDIO_CODING_TYPE_PCM:
+ return true;
+ default:
+ return false;
+ }
Are you sure that you need this helper? That's used only in one function...
...if you really need this one, though, I don't think that using a switch
is the best option here.
Unless anyone is against that (please, reason?), I would be for doing it like:
return sad->format == HDMI_AUDIO_CODING_TYPE_PCM;
Everything else looks good to me (and working, too).
Cheers,
Angelo
Re: [PATCH v3 1/7] drm: Move and add a few utility macros into drm util header
On Thu, 14 Jul 2022 12:08:01 +0300
Gwan-gyeong Mun wrote:
> It moves overflows_type utility macro into drm util header from i915_utils
> header. The overflows_type can be used to catch the truncation between data
> types. And it adds safe_conversion() macro which performs a type conversion
> (cast) of an source value into a new variable, checking that the
> destination is large enough to hold the source value.
> And it adds exact_type and exactly_pgoff_t macro to catch type mis-match
> while compiling.
>
> v3: Add is_type_unsigned() macro (Mauro)
> Modify overflows_type() macro to consider signed data types (Mauro)
> Fix the problem that safe_conversion() macro always returns true
>
> Signed-off-by: Gwan-gyeong Mun
> Cc: Thomas Hellström
> Cc: Matthew Auld
> Cc: Nirmoy Das
> Cc: Jani Nikula
LGTM.
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/gpu/drm/i915/i915_utils.h | 5 +-
> include/drm/drm_util.h| 77 +++
> 2 files changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_utils.h
> b/drivers/gpu/drm/i915/i915_utils.h
> index c10d68cdc3ca..345e5b2dc1cd 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -32,6 +32,7 @@
> #include
> #include
> #include
> +#include
>
> #ifdef CONFIG_X86
> #include
> @@ -111,10 +112,6 @@ bool i915_error_injected(void);
> #define range_overflows_end_t(type, start, size, max) \
> range_overflows_end((type)(start), (type)(size), (type)(max))
>
> -/* Note we don't consider signbits :| */
> -#define overflows_type(x, T) \
> - (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T))
> -
> #define ptr_mask_bits(ptr, n) ({ \
> unsigned long __v = (unsigned long)(ptr); \
> (typeof(ptr))(__v & -BIT(n)); \
> diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
> index 79952d8c4bba..d594ef34b537 100644
> --- a/include/drm/drm_util.h
> +++ b/include/drm/drm_util.h
> @@ -62,6 +62,83 @@
> */
> #define for_each_if(condition) if (!(condition)) {} else
>
> +/**
> + * is_type_unsigned - helper for checking data type which is an unsigned data
> + * type or not
> + * @x: The data type to check
> + *
> + * Returns:
> + * True if the data type is an unsigned data type, false otherwise.
> + */
> +#define is_type_unsigned(x) ((typeof(x))-1 >= (typeof(x))0)
> +
> +/**
> + * overflows_type - helper for checking the truncation between data types
> + * @x: Source for overflow type comparison
> + * @T: Destination for overflow type comparison
> + *
> + * It compares the values and size of each data type between the first and
> + * second argument to check whether truncation can occur when assigning the
> + * first argument to the variable of the second argument.
> + * Source and Destination can be used with or without sign bit.
> + * Composite data structures such as union and structure are not considered.
> + * Enum data types are not considered.
> + * Floating point data types are not considered.
> + *
> + * Returns:
> + * True if truncation can occur, false otherwise.
> + */
> +
> +#define overflows_type(x, T) \
> + (is_type_unsigned(x) ? \
> + is_type_unsigned(T) ? \
> + (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T)) ? 1
> : 0 \
> + : (sizeof(x) >= sizeof(T) && (x) >> (BITS_PER_TYPE(T) -
> 1)) ? 1 : 0 \
> + : is_type_unsigned(T) ? \
> + ((x) < 0) ? 1 : (sizeof(x) > sizeof(T) && (x) >>
> BITS_PER_TYPE(T)) ? 1 : 0 \
> + : (sizeof(x) > sizeof(T)) ? \
> + ((x) < 0) ? (((x) * -1) >> BITS_PER_TYPE(T)) ? 1 : 0 \
> + : ((x) >> BITS_PER_TYPE(T)) ? 1 : 0 \
> + : 0)
> +
> +/**
> + * exact_type - break compile if source type and destination value's type are
> + * not the same
> + * @T: Source type
> + * @n: Destination value
> + *
> + * It is a helper macro for a poor man's -Wconversion: only allow variables
> of
> + * an exact type. It determines whether the source type and destination
> value's
> + * type are the same while compiling, and it breaks compile if two types are
> + * not the same
> + */
> +#define exact_type(T, n) \
> + BUILD_BUG_ON(!__builtin_constant_p(n) &&
> !__builtin_types_compatible_p(T, typeof(n)))
> +
> +/**
> + * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
> + * @n: value to compare pgoff_t type
> + *
> + * It breaks compile if the argument value's type is not pgoff_t type.
> + */
> +#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
> +
> +/*
> + * safe_conversion - perform a type conversion (cast) of an source value into
> + * a new variable, checking that the destination is large enough to hold the
> + * source value.
> + * @ptr: Destination pointer address
> + * @value: Source value
> + *
> + * Returns:
> + * If the value woul
Re: [Intel-gfx] [PATCH v3 2/7] drm: Add drm_memcpy_from_wc() variant which accepts destination address
Am 13.07.22 um 17:47 schrieb Lucas De Marchi:
On Tue, Apr 26, 2022 at 10:21:43PM +0530, Balasubramani Vivekanandan
wrote:
Fast copy using non-temporal instructions for x86 currently exists at
two
locations. One is implemented in i915 driver at i915/i915_memcpy.c and
another copy at drm_cache.c. The plan is to remove the duplicate
implementation in i915 driver and use the functions from drm_cache.c.
A variant of drm_memcpy_from_wc() is added in drm_cache.c which accepts
address as argument instead of iosys_map for destination. It is a very
common scenario in i915 to copy from a WC memory type, which may be an
io memory or a system memory to a destination address pointing to system
memory. To avoid the overhead of creating iosys_map type for the
destination, new variant is created to accept the address directly.
Well that is pretty much a NAK. iosys_map was created to avoid exactly
this kind of usage.
Creating a temporary iosys_map instance on the stack should have
basically no overhead at all.
Also a new function is exported in drm_cache.c to find if the fast copy
is supported by the platform or not. It is required for i915.
v2: Added a new argument to drm_memcpy_from_wc_vaddr() which provides
the offset into the src address to start copy from.
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
Cc: Daniel Vetter
Cc: Thomas Hellstr_m
^ utf-8 error?
Signed-off-by: Balasubramani Vivekanandan
Reviewed-by: Lucas De Marchi
Reviewed-by: Nirmoy Das
---
drivers/gpu/drm/drm_cache.c | 55 +
include/drm/drm_cache.h | 3 ++
2 files changed, 58 insertions(+)
diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 2e2545df3310..8c7af755f7bc 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -358,6 +358,55 @@ void drm_memcpy_from_wc(struct iosys_map *dst,
}
EXPORT_SYMBOL(drm_memcpy_from_wc);
+/**
+ * drm_memcpy_from_wc_vaddr - Perform the fastest available memcpy
from a source
+ * that may be WC to a destination in system memory.
+ * @dst: The destination pointer
+ * @src: The source pointer
+ * @src_offset: The offset from which to copy
+ * @len: The size of the area to transfer in bytes
+ *
+ * Same as drm_memcpy_from_wc except destination is accepted as
system memory
+ * address. Useful in situations where passing destination address
as iosys_map
+ * is simply an overhead and can be avoided.
+ */
+void drm_memcpy_from_wc_vaddr(void *dst, const struct iosys_map *src,
+ size_t src_offset, unsigned long len)
+{
+ const void *src_addr = src->is_iomem ?
+ (void const __force *)src->vaddr_iomem :
+ src->vaddr;
checking this again... this is a layer violation. We do have some in
the codebase, but it'd be good not to add more.
Yes, agree completely. That here is something we should try hard to avoid.
later I'd like these memcpy variants to be moved iosys-map. I'm not sure
if Thomas Zimmermann or Christian agree. Cc'ing them here.
Again yes, that sounds like a good idea to me as well.
Maybe we could scan the codebase and add a function in iosys-map like:
void *iosys_map_get_ptr(struct iosys_map *map, bool is_lmem)
... which is similar to the ttm_kmap_obj_virtual() function we have in
ttm.
so the cases where the layer is violated at least they come from a
single function call in iosys_map API rather than directly accessing
is_iomem/vaddr_iomem/vaddr. Thomas/Christian, any opinion here?
As noted above creating an iosys_map instance on the stack has basically
no overhead at all.
So we should really switch those functions over to using the structure
directly.
Regards,
Christian.
Lucas De Marchi
+
+ if (WARN_ON(in_interrupt())) {
+ iosys_map_memcpy_from(dst, src, src_offset, len);
+ return;
+ }
+
+ if (static_branch_likely(&has_movntdqa)) {
+ __drm_memcpy_from_wc(dst, src_addr + src_offset, len);
+ return;
+ }
+
+ iosys_map_memcpy_from(dst, src, src_offset, len);
+}
+EXPORT_SYMBOL(drm_memcpy_from_wc_vaddr);
+
+/*
+ * drm_memcpy_fastcopy_supported - Returns if fast copy using
non-temporal
+ * instructions is supported
+ *
+ * Returns true if platform has support for fast copying from wc
memory type
+ * using non-temporal instructions. Else false.
+ */
+bool drm_memcpy_fastcopy_supported(void)
+{
+ if (static_branch_likely(&has_movntdqa))
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(drm_memcpy_fastcopy_supported);
+
/*
* drm_memcpy_init_early - One time initialization of the WC memcpy code
*/
@@ -382,6 +431,12 @@ void drm_memcpy_from_wc(struct iosys_map *dst,
}
EXPORT_SYMBOL(drm_memcpy_from_wc);
+bool drm_memcpy_fastcopy_supported(void)
+{
+ return false;
+}
+EXPORT_SYMBOL(drm_memcpy_fastcopy_supported);
+
void drm_memcpy_init_early(void)
{
}
diff --git a/include/drm/drm_cache.h b/include/drm/drm_cache.h
index 22deb216b59
Re: [PATCH v3 2/7] drm/i915/gem: Typecheck page lookups
On Thu, 14 Jul 2022 12:08:02 +0300
Gwan-gyeong Mun wrote:
> From: Chris Wilson
>
> We need to check that we avoid integer overflows when looking up a page,
> and so fix all the instances where we have mistakenly used a plain
> integer instead of a more suitable long. Be pedantic and add integer
> typechecking to the lookup so that we can be sure that we are safe.
> And it also uses pgoff_t as our page lookups must remain compatible with
> the page cache, pgoff_t is currently exactly unsigned long.
>
> v2: Move added i915_utils's macro into drm_util header (Jani N)
> v3: Make not use the same macro name on a function. (Mauro)
> For kernel-doc, macros and functions are handled in the same namespace,
> the same macro name on a function prevents ever adding documentation
> for it.
>
> Signed-off-by: Chris Wilson
> Signed-off-by: Gwan-gyeong Mun
> Cc: Tvrtko Ursulin
> Cc: Matthew Auld
> Cc: Thomas Hellström
> Reviewed-by: Nirmoy Das
LGTM, so:
> Reviewed-by: Mauro Carvalho Chehab
Yet, it would make sense to have some patch adding kernel-doc
markups to the kAPI functions declared at the header files here.
Regards,
Mauro
> ---
> drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +-
> drivers/gpu/drm/i915/gem/i915_gem_object.h| 77 +--
> drivers/gpu/drm/i915/gem/i915_gem_pages.c | 27 ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
> .../drm/i915/gem/selftests/i915_gem_context.c | 12 +--
> .../drm/i915/gem/selftests/i915_gem_mman.c| 8 +-
> .../drm/i915/gem/selftests/i915_gem_object.c | 8 +-
> drivers/gpu/drm/i915/i915_gem.c | 18 -
> drivers/gpu/drm/i915/i915_vma.c | 8 +-
> 9 files changed, 106 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index ccec4055fde3..90996fe8ad45 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -421,10 +421,11 @@ void __i915_gem_object_invalidate_frontbuffer(struct
> drm_i915_gem_object *obj,
> static void
> i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64
> offset, void *dst, int size)
> {
> + pgoff_t idx = offset >> PAGE_SHIFT;
> void *src_map;
> void *src_ptr;
>
> - src_map = kmap_atomic(i915_gem_object_get_page(obj, offset >>
> PAGE_SHIFT));
> + src_map = kmap_atomic(i915_gem_object_get_page(obj, idx));
>
> src_ptr = src_map + offset_in_page(offset);
> if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
> @@ -437,9 +438,10 @@ i915_gem_object_read_from_page_kmap(struct
> drm_i915_gem_object *obj, u64 offset,
> static void
> i915_gem_object_read_from_page_iomap(struct drm_i915_gem_object *obj, u64
> offset, void *dst, int size)
> {
> + pgoff_t idx = offset >> PAGE_SHIFT;
> + dma_addr_t dma = i915_gem_object_get_dma_address(obj, idx);
> void __iomem *src_map;
> void __iomem *src_ptr;
> - dma_addr_t dma = i915_gem_object_get_dma_address(obj, offset >>
> PAGE_SHIFT);
>
> src_map = io_mapping_map_wc(&obj->mm.region->iomap,
> dma - obj->mm.region->region.start,
> @@ -468,6 +470,7 @@ i915_gem_object_read_from_page_iomap(struct
> drm_i915_gem_object *obj, u64 offset
> */
> int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64
> offset, void *dst, int size)
> {
> + GEM_BUG_ON(overflows_type(offset >> PAGE_SHIFT, pgoff_t));
> GEM_BUG_ON(offset >= obj->base.size);
> GEM_BUG_ON(offset_in_page(offset) > PAGE_SIZE - size);
> GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index 6f0a3ce35567..4b49e0d2890b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -27,8 +27,10 @@ enum intel_region_id;
> * spot such a local variable, please consider fixing!
> *
> * Aside from our own locals (for which we have no excuse!):
> - * - sg_table embeds unsigned int for num_pages
> - * - get_user_pages*() mixed ints with longs
> + * - sg_table embeds unsigned int for nents
> + *
> + * We can check for invalidly typed locals with typecheck(), see for example
> + * i915_gem_object_get_sg().
> */
> #define GEM_CHECK_SIZE_OVERFLOW(sz) \
> GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
> @@ -364,43 +366,72 @@ int i915_gem_object_set_tiling(struct
> drm_i915_gem_object *obj,
> unsigned int tiling, unsigned int stride);
>
> struct scatterlist *
> -__i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
> - struct i915_gem_object_page_iter *iter,
> - unsigned int n,
> - unsigned int *offset, bool dma);
> +__i915_gem_object_page_iter_get_sg(struct drm_i915_gem_object *obj,
> +
Re: [PATCH v14 04/10] video/hdmi: Add audio_infoframe packing for DP
Il 12/07/22 13:12, Bo-Chen Chen ha scritto:
From: Markus Schneider-Pargmann
Similar to HDMI, DP uses audio infoframes as well which are structured
very similar to the HDMI ones.
This patch adds a helper function to pack the HDMI audio infoframe for
DP, called hdmi_audio_infoframe_pack_for_dp().
hdmi_audio_infoframe_pack_only() is split into two parts. One of them
packs the payload only and can be used for HDMI and DP.
Also constify the frame parameter in hdmi_audio_infoframe_check() as
it is passed to hdmi_audio_infoframe_check_only() which expects a const.
Signed-off-by: Markus Schneider-Pargmann
Signed-off-by: Guillaume Ranquet
Signed-off-by: Bo-Chen Chen
---
drivers/video/hdmi.c | 82 +++-
include/drm/display/drm_dp.h | 2 +
include/linux/hdmi.h | 7 ++-
3 files changed, 71 insertions(+), 20 deletions(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 947be761dfa4..86805d77cc86 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -21,6 +21,7 @@
* DEALINGS IN THE SOFTWARE.
*/
+#include
#include
#include
#include
@@ -381,12 +382,34 @@ static int hdmi_audio_infoframe_check_only(const struct
hdmi_audio_infoframe *fr
*
* Returns 0 on success or a negative error code on failure.
*/
-int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame)
+int hdmi_audio_infoframe_check(const struct hdmi_audio_infoframe *frame)
{
return hdmi_audio_infoframe_check_only(frame);
}
EXPORT_SYMBOL(hdmi_audio_infoframe_check);
+static void
+hdmi_audio_infoframe_pack_payload(const struct hdmi_audio_infoframe *frame,
+ u8 *buffer)
+{
+ u8 channels;
+
+ if (frame->channels >= 2)
+ channels = frame->channels - 1;
+ else
+ channels = 0;
+
+ buffer[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
+ buffer[1] = ((frame->sample_frequency & 0x7) << 2) |
+(frame->sample_size & 0x3);
+ buffer[2] = frame->coding_type_ext & 0x1f;
+ buffer[3] = frame->channel_allocation;
+ buffer[4] = (frame->level_shift_value & 0xf) << 3;
+
+ if (frame->downmix_inhibit)
+ buffer[4] |= BIT(7);
+}
+
/**
* hdmi_audio_infoframe_pack_only() - write HDMI audio infoframe to binary
buffer
* @frame: HDMI audio infoframe
@@ -404,7 +427,6 @@ EXPORT_SYMBOL(hdmi_audio_infoframe_check);
ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe
*frame,
void *buffer, size_t size)
{
- unsigned char channels;
u8 *ptr = buffer;
size_t length;
int ret;
@@ -420,28 +442,13 @@ ssize_t hdmi_audio_infoframe_pack_only(const struct
hdmi_audio_infoframe *frame,
memset(buffer, 0, size);
- if (frame->channels >= 2)
- channels = frame->channels - 1;
- else
- channels = 0;
-
ptr[0] = frame->type;
ptr[1] = frame->version;
ptr[2] = frame->length;
ptr[3] = 0; /* checksum */
- /* start infoframe payload */
- ptr += HDMI_INFOFRAME_HEADER_SIZE;
-
- ptr[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7);
- ptr[1] = ((frame->sample_frequency & 0x7) << 2) |
-(frame->sample_size & 0x3);
- ptr[2] = frame->coding_type_ext & 0x1f;
- ptr[3] = frame->channel_allocation;
- ptr[4] = (frame->level_shift_value & 0xf) << 3;
-
- if (frame->downmix_inhibit)
- ptr[4] |= BIT(7);
+ hdmi_audio_infoframe_pack_payload(frame,
+ ptr + HDMI_INFOFRAME_HEADER_SIZE);
hdmi_infoframe_set_checksum(buffer, length);
@@ -479,6 +486,43 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
}
EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
+/**
+ * hdmi_audio_infoframe_pack_for_dp - Pack a HDMI Audio infoframe for
DisplayPort
+ *
+ * @frame: HDMI Audio infoframe
+ * @sdp:secondary data packet for display port. This is filled with the
+ * appropriate: data
"This is filled with the appropriate data"
... well, that's pretty obvious, isn't it?
You're describing that this function is filling sdp in the description, so you
can just remove that part.
Also, "Secondary data packet for DisplayPort", please.
+ * @dp_version: Display Port version to be encoded in the header
We're not meaning "a display port", but really "DisplayPort": please remove
the space between "Display" and "Port" :-)
(here and in the description below)
+ *
+ * Packs a HDMI Audio Infoframe to be sent over Display Port. This function
+ * fills the secondary data packet to be used for Display Port.
+ *
+ * Return: Number of total written bytes or a negative errno on failure.
+ */
+ssize_t
+hdmi_audio_infoframe_pack_for_dp(const struct hdmi_audio_infoframe *frame,
+struct dp_sdp *sdp, u8 dp_ver
Re: [PATCH v3 4/7] drm/i915: Check for integer truncation on the configuration of ttm place
On Thu, 14 Jul 2022 12:08:04 +0300
Gwan-gyeong Mun wrote:
> There is an impedance mismatch between the first/last valid page
> frame number of ttm place in unsigned and our memory/page accounting in
> unsigned long.
> As the object size is under the control of userspace, we have to be prudent
> and catch the conversion errors.
> To catch the implicit truncation as we switch from unsigned long to
> unsigned, we use overflows_type check and report E2BIG or overflow_type
> prior to the operation.
>
> v3: Not to change execution inside a macro. (Mauro)
> Add safe_conversion_gem_bug_on() macro and remove temporal
> SAFE_CONVERSION() macro.
>
> Signed-off-by: Gwan-gyeong Mun
> Cc: Chris Wilson
> Cc: Matthew Auld
> Cc: Thomas Hellström
> Reviewed-by: Nirmoy Das
LGTM.
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 6 +++---
> drivers/gpu/drm/i915/i915_gem.h | 4
> drivers/gpu/drm/i915/intel_region_ttm.c | 20 +---
> 3 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index 9f2be1892b6c..88f2887627dc 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -140,14 +140,14 @@ i915_ttm_place_from_region(const struct
> intel_memory_region *mr,
> if (flags & I915_BO_ALLOC_CONTIGUOUS)
> place->flags |= TTM_PL_FLAG_CONTIGUOUS;
> if (offset != I915_BO_INVALID_OFFSET) {
> - place->fpfn = offset >> PAGE_SHIFT;
> - place->lpfn = place->fpfn + (size >> PAGE_SHIFT);
> + safe_conversion_gem_bug_on(&place->fpfn, offset >> PAGE_SHIFT);
> + safe_conversion_gem_bug_on(&place->lpfn, place->fpfn + (size >>
> PAGE_SHIFT));
> } else if (mr->io_size && mr->io_size < mr->total) {
> if (flags & I915_BO_ALLOC_GPU_ONLY) {
> place->flags |= TTM_PL_FLAG_TOPDOWN;
> } else {
> place->fpfn = 0;
> - place->lpfn = mr->io_size >> PAGE_SHIFT;
> + safe_conversion_gem_bug_on(&place->lpfn, mr->io_size >>
> PAGE_SHIFT);
> }
> }
> }
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 68d8d52bd541..6b673607abee 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -83,5 +83,9 @@ struct drm_i915_private;
> #endif
>
> #define I915_GEM_IDLE_TIMEOUT (HZ / 5)
> +#define safe_conversion_gem_bug_on(ptr, value) ({ \
> + safe_conversion(ptr, value) ? 1 \
> + : (({ GEM_BUG_ON(overflows_type(value, *ptr)); }), 0); \
> +})
>
> #endif /* __I915_GEM_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
> b/drivers/gpu/drm/i915/intel_region_ttm.c
> index 575d67bc6ffe..f0d143948725 100644
> --- a/drivers/gpu/drm/i915/intel_region_ttm.c
> +++ b/drivers/gpu/drm/i915/intel_region_ttm.c
> @@ -209,14 +209,26 @@ intel_region_ttm_resource_alloc(struct
> intel_memory_region *mem,
> if (flags & I915_BO_ALLOC_CONTIGUOUS)
> place.flags |= TTM_PL_FLAG_CONTIGUOUS;
> if (offset != I915_BO_INVALID_OFFSET) {
> - place.fpfn = offset >> PAGE_SHIFT;
> - place.lpfn = place.fpfn + (size >> PAGE_SHIFT);
> + if (!safe_conversion_gem_bug_on(&place.fpfn,
> + offset >> PAGE_SHIFT)) {
> + ret = -E2BIG;
> + goto out;
> + }
> + if (!safe_conversion_gem_bug_on(&place.lpfn,
> + place.fpfn + (size >>
> PAGE_SHIFT))) {
> + ret = -E2BIG;
> + goto out;
> + }
> } else if (mem->io_size && mem->io_size < mem->total) {
> if (flags & I915_BO_ALLOC_GPU_ONLY) {
> place.flags |= TTM_PL_FLAG_TOPDOWN;
> } else {
> place.fpfn = 0;
> - place.lpfn = mem->io_size >> PAGE_SHIFT;
> + if (!safe_conversion_gem_bug_on(&place.lpfn,
> + mem->io_size >>
> PAGE_SHIFT)) {
> + ret = -E2BIG;
> + goto out;
> + }
> }
> }
>
> @@ -224,6 +236,8 @@ intel_region_ttm_resource_alloc(struct
> intel_memory_region *mem,
> mock_bo.bdev = &mem->i915->bdev;
>
> ret = man->func->alloc(man, &mock_bo, &place, &res);
> +
> +out:
> if (ret == -ENOSPC)
> ret = -ENXIO;
> if (!ret)
[PATCH 6/8] dt-bindings: backlight: Update Lee Jones' email address
Going forward, I'll be using my kernel.org for upstream work. Cc: Daniel Thompson Cc: Jingoo Han Cc: Rob Herring Cc: Krzysztof Kozlowski Cc: dri-devel@lists.freedesktop.org Cc: linux-l...@vger.kernel.org Cc: devicet...@vger.kernel.org Signed-off-by: Lee Jones Signed-off-by: Lee Jones --- Documentation/devicetree/bindings/leds/backlight/common.yaml| 2 +- .../devicetree/bindings/leds/backlight/gpio-backlight.yaml | 2 +- .../devicetree/bindings/leds/backlight/led-backlight.yaml | 2 +- .../devicetree/bindings/leds/backlight/lm3630a-backlight.yaml | 2 +- .../devicetree/bindings/leds/backlight/pwm-backlight.yaml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/backlight/common.yaml b/Documentation/devicetree/bindings/leds/backlight/common.yaml index 702ba350d8698..3b60afbab68ba 100644 --- a/Documentation/devicetree/bindings/leds/backlight/common.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/common.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Common backlight properties maintainers: - - Lee Jones + - Lee Jones - Daniel Thompson - Jingoo Han diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml index 75cc569b9c558..3300451fcfd5e 100644 --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: gpio-backlight bindings maintainers: - - Lee Jones + - Lee Jones - Daniel Thompson - Jingoo Han diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml index f5822f4ea6679..0793d0adc4ec9 100644 --- a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: led-backlight bindings maintainers: - - Lee Jones + - Lee Jones - Daniel Thompson - Jingoo Han diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml index 08fe5cf8614a8..3c9b4054ed9a5 100644 --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: TI LM3630A High-Efficiency Dual-String White LED maintainers: - - Lee Jones + - Lee Jones - Daniel Thompson - Jingoo Han diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml index fcb8429f3088c..78fbe20a17580 100644 --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: pwm-backlight bindings maintainers: - - Lee Jones + - Lee Jones - Daniel Thompson - Jingoo Han -- 2.37.0.144.g8ac04bfd2-goog
[Bug 216120] [BISECTED][REGRESSION] drm/amdgpu: add drm buddy support to amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=216120 --- Comment #4 from mat2003...@gmail.com --- this patch: https://gitlab.freedesktop.org/drm/amd/uploads/564b2cc2b5ea87357f39e45c3a1a44e2/0001-drm-amdgpu-Fix-for-drm-buddy-memory-corruption.patch on top of 5.19.0-rc6 seems to fix the problem tested: 5.19.0-rc6: NOT OK 5.19.0-rc6+0001-drm-amdgpu-Fix-for-drm-buddy-memory-corruption.patch: OK -- 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 v14 09/10] drm/mediatek: DP audio support for MT8195
Il 12/07/22 13:12, Bo-Chen Chen ha scritto:
From: Guillaume Ranquet
This patch adds audio support to the DP driver for MT8195 with up to 8
channels.
Signed-off-by: Guillaume Ranquet
Signed-off-by: Bo-Chen Chen
---
drivers/gpu/drm/mediatek/mtk_dp.c | 723 ++
drivers/gpu/drm/mediatek/mtk_dp_reg.h | 2 +
2 files changed, 725 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
b/drivers/gpu/drm/mediatek/mtk_dp.c
index 5ab646bd2bc4..fa7bb102a105 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
..snip..
@@ -2082,6 +2693,104 @@ static void mtk_dp_debounce_timer(struct timer_list *t)
mtk_dp->need_debounce = true;
}
+/*
+ * HDMI audio codec callbacks
+ */
+static int mtk_dp_audio_hw_params(struct device *dev, void *data,
+ struct hdmi_codec_daifmt *daifmt,
+ struct hdmi_codec_params *params)
+{
+ struct mtk_dp *mtk_dp = dev_get_drvdata(dev);
+ struct mtk_dp_audio_cfg cfg;
+
+ if (!mtk_dp->enabled) {
+ pr_err("%s, DP is not ready!\n", __func__);
drm_err() here please.
+ return -ENODEV;
+ }
+
+ cfg.channels = params->cea.channels;
+ cfg.sample_rate = params->sample_rate;
+ cfg.word_length_bits = 24;
+
+ mtk_dp_audio_setup(mtk_dp, &cfg);
+
+ return 0;
+}
+
..snip..
+
+static int mtk_dp_register_audio_driver(struct device *dev)
+{
+ struct mtk_dp *mtk_dp = dev_get_drvdata(dev);
+ struct hdmi_codec_pdata codec_data = {
+ .ops = &mtk_dp_audio_codec_ops,
+ .max_i2s_channels = 8,
+ .i2s = 1,
+ .data = mtk_dp,
+ };
+ struct platform_device *pdev;
+
+ pdev = platform_device_register_data(dev, HDMI_CODEC_DRV_NAME,
+PLATFORM_DEVID_AUTO, &codec_data,
+sizeof(codec_data));
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
You're never unregistering this device, which is unacceptable.
Please add a platform device pointer in struct mtk_dp, so that this function
becomes, simply:
mtk_dp->audio_pdev = platform_device_register_data(dev,
HDMI_CODEC_DRV_NAME,
PLATFORM_DEVID_AUTO,
&codec_data,
sizeof(codec_data));
return PTR_ERR_OR_ZERO(mtk_dp->audio_pdev);
+
+ return 0;
+}
+
static int mtk_dp_probe(struct platform_device *pdev)
{
struct mtk_dp *mtk_dp;
@@ -2127,8 +2836,21 @@ static int mtk_dp_probe(struct platform_device *pdev)
return dev_err_probe(dev, -EPROBE_DEFER,
"failed to request mediatek dptx irq\n");
+ mutex_init(&mtk_dp->edid_lock);
+ mutex_init(&mtk_dp->eld_lock);
+ mutex_init(&mtk_dp->update_plugged_status_lock);
+
platform_set_drvdata(pdev, mtk_dp);
+ if (!mtk_dp_is_edp(mtk_dp)) {
+ ret = mtk_dp_register_audio_driver(dev);
+ if (ret) {
+ dev_err(dev, "Failed to register audio driver: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
mtk_dp->phy_dev = platform_device_register_data(dev, "mediatek-dp-phy",
PLATFORM_DEVID_AUTO,
&mtk_dp->regs,
@@ -2174,6 +2896,7 @@ static int mtk_dp_remove(struct platform_device *pdev)
platform_device_unregister(mtk_dp->phy_dev);
... and unregister it here:
if (mtk_dp->audio_pdev)
platform_device_unregister(mtk_dp->audio_pdev);
mtk_dp_video_mute(mtk_dp, true);
+ mtk_dp_audio_mute(mtk_dp, true);
del_timer_sync(&mtk_dp->debounce_timer);
pm_runtime_disable(&pdev->dev);
Regards,
Angelo
Re: [PATCH v2 1/5] drm/modes: parse_cmdline: Handle empty mode name part
Hi,
On Thu, Jul 14, 2022 at 11:04:06AM +0200, Geert Uytterhoeven wrote:
> If no mode name part was specified, mode_end is zero, and the "ret ==
> mode_end" check does the wrong thing.
>
> Fix this by skipping all named mode handling when mode_end is zero.
>
> Fixes: 7b1cce760afe38b4 ("drm/modes: parse_cmdline: Allow specifying
> stand-alone options")
> Signed-off-by: Geert Uytterhoeven
> Reviewed-by: Hans de Goede
> Acked-by: Thomas Zimmermann
We should add a test for that in drivers/gpu/drm/tests/drm_cmdline_parser_test.c
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v2 2/5] drm/modes: Extract drm_mode_parse_cmdline_named_mode()
On Thu, Jul 14, 2022 at 11:04:07AM +0200, Geert Uytterhoeven wrote: > Extract the code to check for a named mode parameter into its own > function, to streamline the main parsing flow. > > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Hans de Goede > Acked-by: Thomas Zimmermann Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH v2 3/5] drm/modes: parse_cmdline: Make mode->*specified handling more uniform
On Thu, Jul 14, 2022 at 11:04:08AM +0200, Geert Uytterhoeven wrote: > The various mode->*specified flags are not handled in an uniform way: > some flags are set by the corresponding drm_mode_parse_cmdline_*() > function, some flags by the caller of the function, and some flags by > both. > > Make this uniform by making this the responsibility of the various > parsing helpers, i.e. > - Move the setting of mode->specified from caller to callee, > - Drop the duplicate setting of mode->bpp_specified and > mode->refresh_specified from callers. > > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Hans de Goede > Acked-by: Thomas Zimmermann Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH v2 5/5] drm/modes: parse_cmdline: Add support for named modes containing dashes
On Thu, Jul 14, 2022 at 11:04:10AM +0200, Geert Uytterhoeven wrote: > It is fairly common for named video modes to contain dashes (e.g. > "tt-mid" on Atari, "dblntsc-ff" on Amiga). Currently such mode names > are not recognized, as the dash is considered to be a separator between > mode name and bpp. > > Fix this by skipping any dashes that are not followed immediately by a > digit when looking for the separator. > > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Hans de Goede Ditto, we should have a test for that Maxime signature.asc Description: PGP signature
[PATCH v2 07/21] drm/i915/gt: describe the new tlb parameter at i915_vma_resource
TLB cache invalidation can happen on two different situations: 1. synchronously, at __vma_put_pages(); 2. asynchronously. On the first case, TLB cache invalidation happens inside __vma_put_pages(). So, no need to do it later on. However, on the second case, the pages will keep in memory until __i915_vma_evict() is called. So, we need to store the TLB data at struct i915_vma_resource, in order to do a TLB cache invalidation before allowing userspace to re-use the same memory. So, i915_vma_resource_unbind() has gained a new parameter in order to store the TLB data at the second case. Document it. Signed-off-by: Mauro Carvalho Chehab --- To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover. See [PATCH v2 00/21] at: https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/ drivers/gpu/drm/i915/i915_vma_resource.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_vma_resource.c b/drivers/gpu/drm/i915/i915_vma_resource.c index 5a67995ea5fe..4fe09ea0a825 100644 --- a/drivers/gpu/drm/i915/i915_vma_resource.c +++ b/drivers/gpu/drm/i915/i915_vma_resource.c @@ -216,6 +216,10 @@ i915_vma_resource_fence_notify(struct i915_sw_fence *fence, /** * i915_vma_resource_unbind - Unbind a vma resource * @vma_res: The vma resource to unbind. + * @tlb: pointer to vma->obj->mm.tlb associated with the resource + * to be stored at vma_res->tlb. When not-NULL, it will be used + * to do TLB cache invalidation before freeing a VMA resource. + * used only for async unbind. * * At this point this function does little more than publish a fence that * signals immediately unless signaling is held back. -- 2.36.1
[PATCH v2 05/21] drm/i915/gt: Skip TLB invalidations once wedged
From: Chris Wilson
Skip all further TLB invalidations once the device is wedged and
had been reset, as, on such cases, it can no longer process instructions
on the GPU and the user no longer has access to the TLB's in each engine.
That helps to reduce the performance regression introduced by TLB
invalidate logic.
Cc: sta...@vger.kernel.org
Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
Signed-off-by: Chris Wilson
Cc: Fei Yang
Cc: Andi Shyti
Acked-by: Thomas Hellström
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/intel_gt.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 1d84418e8676..5c55a90672f4 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -934,6 +934,9 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt)
if (I915_SELFTEST_ONLY(gt->awake == -ENODEV))
return;
+ if (intel_gt_is_wedged(gt))
+ return;
+
if (GRAPHICS_VER(i915) == 12) {
regs = gen12_regs;
num = ARRAY_SIZE(gen12_regs);
--
2.36.1
[PATCH v2 12/21] drm/i915/guc: Introduce TLB_INVALIDATION_ALL action
From: Piotr Piórkowski
Add a new way to invalidate TLB via GuC using actions 0x7002
(TLB_INVALIDATION_ALL).
Those actions will be used on upcoming patches.
Signed-off-by: Piotr Piórkowski
Cc: Michal Wajdeczko
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 14 ++
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 1 +
3 files changed, 16 insertions(+)
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 14e35a2f8306..fb0af33e43cc 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
@@ -138,6 +138,7 @@ enum intel_guc_action {
INTEL_GUC_ACTION_PAGE_FAULT_NOTIFICATION = 0x6001,
INTEL_GUC_ACTION_TLB_INVALIDATION = 0x7000,
INTEL_GUC_ACTION_TLB_INVALIDATION_DONE = 0x7001,
+ INTEL_GUC_ACTION_TLB_INVALIDATION_ALL = 0x7002,
INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002,
INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003,
INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 5c59f9b144a3..8a104a292598 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -945,6 +945,20 @@ int intel_guc_invalidate_tlb_guc(struct intel_guc *guc,
return guc_send_invalidate_tlb(guc, action, ARRAY_SIZE(action));
}
+int intel_guc_invalidate_tlb_all(struct intel_guc *guc)
+{
+ u32 action[] = {
+ INTEL_GUC_ACTION_TLB_INVALIDATION_ALL,
+ 0,
+ INTEL_GUC_TLB_INVAL_MODE_HEAVY <<
INTEL_GUC_TLB_INVAL_MODE_SHIFT |
+ INTEL_GUC_TLB_INVAL_FLUSH_CACHE,
+ };
+
+ GEM_BUG_ON(!INTEL_GUC_SUPPORTS_TLB_INVALIDATION(guc));
+
+ return guc_send_invalidate_tlb(guc, action, ARRAY_SIZE(action));
+}
+
/**
* intel_guc_load_status - dump information about GuC load status
* @guc: the GuC
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 73c46d405dc4..01c6478451cc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -386,6 +386,7 @@ int intel_guc_self_cfg64(struct intel_guc *guc, u16 key,
u64 value);
int intel_guc_invalidate_tlb_guc(struct intel_guc *guc,
enum intel_guc_tlb_inval_mode mode);
+int intel_guc_invalidate_tlb_all(struct intel_guc *guc);
static inline bool intel_guc_is_supported(struct intel_guc *guc)
{
--
2.36.1
[PATCH v2 21/21] drm/i915/guc: document TLB cache invalidation functions
Add documentation for the kAPI functions that do TLB cache
invalidation via GuC.
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 52 ++
1 file changed, 45 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 98260a7bc90b..173833bc3a62 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -923,7 +923,14 @@ static int guc_send_invalidate_tlb(struct intel_guc *guc,
u32 *action, u32 size)
return err;
}
- /* Full TLB invalidation */
+/**
+ * intel_guc_invalidate_tlb_full - GuC full TLB invalidation
+ *
+ * @guc: the guc
+ * @mode: mode of TLB cache invalidation (heavy or lite)
+ *
+ * Use GuC to do a full TLB cache invalidation if supported.
+ */
int intel_guc_invalidate_tlb_full(struct intel_guc *guc,
enum intel_guc_tlb_inval_mode mode)
{
@@ -943,8 +950,17 @@ int intel_guc_invalidate_tlb_full(struct intel_guc *guc,
return guc_send_invalidate_tlb(guc, action, ARRAY_SIZE(action));
}
-/*
- * Selective TLB Invalidation for Address Range:
+/**
+ * intel_guc_invalidate_tlb_page_selective - GuC selective TLB invalidation
+ * for an address range
+ *
+ * @guc: the guc
+ * @mode: mode of TLB cache invalidation (heavy or lite)
+ * @start: range start
+ * @length: range length
+ *
+ * Use GuC to do a selective TLB invalidation if supported.
+ *
* TLB's in the Address Range is Invalidated across all engines.
*/
int intel_guc_invalidate_tlb_page_selective(struct intel_guc *guc,
@@ -978,8 +994,18 @@ int intel_guc_invalidate_tlb_page_selective(struct
intel_guc *guc,
return guc_send_invalidate_tlb(guc, action, ARRAY_SIZE(action));
}
-/*
- * Selective TLB Invalidation for Context:
+/**
+ * intel_guc_invalidate_tlb_page_selective_ctx - GuC selective TLB
+ * invalidation for a context
+ *
+ * @guc: the guc
+ * @mode: mode of TLB cache invalidation (heavy or lite)
+ * @start: range start
+ * @length: range length
+ * @ctxid: context ID
+ *
+ * Use GuC to do a selective TLB invalidation on a context if supported.
+ *
* Invalidates all TLB's for a specific context across all engines.
*/
int intel_guc_invalidate_tlb_page_selective_ctx(struct intel_guc *guc,
@@ -1013,8 +1039,13 @@ int intel_guc_invalidate_tlb_page_selective_ctx(struct
intel_guc *guc,
return guc_send_invalidate_tlb(guc, action, ARRAY_SIZE(action));
}
-/*
- * Guc TLB Invalidation: Invalidate the TLB's of GuC itself.
+/**
+ * intel_guc_invalidate_tlb_guc - GuC self TLB invalidation
+ *
+ * @guc: the guc
+ * @mode: mode of TLB cache invalidation (heavy or lite)
+ *
+ * Use GuC to invalidate the TLB's of GuC itself.
*/
int intel_guc_invalidate_tlb_guc(struct intel_guc *guc,
enum intel_guc_tlb_inval_mode mode)
@@ -1035,6 +1066,13 @@ int intel_guc_invalidate_tlb_guc(struct intel_guc *guc,
return guc_send_invalidate_tlb(guc, action, ARRAY_SIZE(action));
}
+/**
+ * intel_guc_invalidate_tlb_all - GuC global TLB invalidation
+ *
+ * @guc: the guc
+ *
+ * Use GuC to do a complete TLB invalidation on all tables
+ */
int intel_guc_invalidate_tlb_all(struct intel_guc *guc)
{
u32 action[] = {
--
2.36.1
[PATCH v2 10/21] drm/i915/guc: use kernel-doc for enum intel_guc_tlb_inval_mode
Transform the comments for intel_guc_tlb_inval_mode into a
kernel-doc markup.
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
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 2e39d8df4c82..14e35a2f8306 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
@@ -190,15 +190,18 @@ enum intel_guc_tlb_invalidation_type {
INTEL_GUC_TLB_INVAL_GUC = 0x3,
};
-/*
- * 0: Heavy mode of Invalidation:
+/**
+ * enum intel_guc_tlb_inval_mode - define the mode for TLB cache invlidation
+ *
+ * @INTEL_GUC_TLB_INVAL_MODE_HEAVY: Heavy Invalidation Mode.
* The pipeline of the engine(s) for which the invalidation is targeted to is
* blocked, and all the in-flight transactions are guaranteed to be Globally
- * Observed before completing the TLB invalidation
- * 1: Lite mode of Invalidation:
+ * Observed before completing the TLB invalidation.
+ * @INTEL_GUC_TLB_INVAL_MODE_LITE: Light Invalidation Mode.
* TLBs of the targeted engine(s) are immediately invalidated.
* In-flight transactions are NOT guaranteed to be Globally Observed before
* completing TLB invalidation.
+ *
* Light Invalidation Mode is to be used only when
* it can be guaranteed (by SW) that the address translations remain invariant
* for the in-flight transactions across the TLB invalidation. In other words,
--
2.36.1
[PATCH v2 20/21] drm/i915/guc: describe enum intel_guc_tlb_invalidation_type
Add a description for intel_guc_tlb_invalidation_type enum.
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 12
1 file changed, 12 insertions(+)
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 5c019856a269..e97065c62d28 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
@@ -187,6 +187,18 @@ enum intel_guc_state_capture_event_status {
/* Flush PPC or SMRO caches along with TLB invalidation request */
#define INTEL_GUC_TLB_INVAL_FLUSH_CACHE (1 << 31)
+/**
+ * enum intel_guc_tlb_invalidation_type - type of TLB cache invalidation
+ *
+ * @INTEL_GUC_TLB_INVAL_FULL:
+ * Global TLB invalidation
+ * @INTEL_GUC_TLB_INVAL_PAGE_SELECTIVE:
+ * Page-selective TLB cache invalidation
+ * @INTEL_GUC_TLB_INVAL_PAGE_SELECTIVE_CTX:
+ * Context-selective TLB cache invalidation
+ * @INTEL_GUC_TLB_INVAL_GUC:
+ * Invalidate TLB on GuC itself
+ */
enum intel_guc_tlb_invalidation_type {
INTEL_GUC_TLB_INVAL_FULL = 0x0,
INTEL_GUC_TLB_INVAL_PAGE_SELECTIVE = 0x1,
--
2.36.1
[PATCH v2 16/21] drm/i915: Define GuC Based TLB invalidation routines
From: Prathap Kumar Valsan
Add routines to interface with GuC firmware for selective TLB invalidation
supported on XeHP.
Signed-off-by: Prathap Kumar Valsan
Cc: Matthew Brost
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 3 +
drivers/gpu/drm/i915/gt/uc/intel_guc.c| 90 +++
drivers/gpu/drm/i915/gt/uc/intel_guc.h| 10 +++
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 3 +
4 files changed, 106 insertions(+)
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 fb0af33e43cc..5c019856a269 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
@@ -188,6 +188,9 @@ enum intel_guc_state_capture_event_status {
#define INTEL_GUC_TLB_INVAL_FLUSH_CACHE (1 << 31)
enum intel_guc_tlb_invalidation_type {
+ INTEL_GUC_TLB_INVAL_FULL = 0x0,
+ INTEL_GUC_TLB_INVAL_PAGE_SELECTIVE = 0x1,
+ INTEL_GUC_TLB_INVAL_PAGE_SELECTIVE_CTX = 0x2,
INTEL_GUC_TLB_INVAL_GUC = 0x3,
};
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 8a104a292598..98260a7bc90b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -923,6 +923,96 @@ static int guc_send_invalidate_tlb(struct intel_guc *guc,
u32 *action, u32 size)
return err;
}
+ /* Full TLB invalidation */
+int intel_guc_invalidate_tlb_full(struct intel_guc *guc,
+ enum intel_guc_tlb_inval_mode mode)
+{
+ u32 action[] = {
+ INTEL_GUC_ACTION_TLB_INVALIDATION,
+ 0,
+ INTEL_GUC_TLB_INVAL_FULL << INTEL_GUC_TLB_INVAL_TYPE_SHIFT |
+ mode << INTEL_GUC_TLB_INVAL_MODE_SHIFT |
+ INTEL_GUC_TLB_INVAL_FLUSH_CACHE,
+ };
+
+ if (!INTEL_GUC_SUPPORTS_TLB_INVALIDATION(guc)) {
+ DRM_ERROR("Tlb invalidation: Operation not supported in this
platform!\n");
+ return 0;
+ }
+
+ return guc_send_invalidate_tlb(guc, action, ARRAY_SIZE(action));
+}
+
+/*
+ * Selective TLB Invalidation for Address Range:
+ * TLB's in the Address Range is Invalidated across all engines.
+ */
+int intel_guc_invalidate_tlb_page_selective(struct intel_guc *guc,
+ enum intel_guc_tlb_inval_mode mode,
+ u64 start, u64 length)
+{
+ u64 vm_total = BIT_ULL(INTEL_INFO(guc_to_gt(guc)->i915)->ppgtt_size);
+ u32 address_mask = (ilog2(length) - ilog2(I915_GTT_PAGE_SIZE_4K));
+ u32 full_range = vm_total == length;
+ u32 action[] = {
+ INTEL_GUC_ACTION_TLB_INVALIDATION,
+ 0,
+ INTEL_GUC_TLB_INVAL_PAGE_SELECTIVE <<
INTEL_GUC_TLB_INVAL_TYPE_SHIFT |
+ mode << INTEL_GUC_TLB_INVAL_MODE_SHIFT |
+ INTEL_GUC_TLB_INVAL_FLUSH_CACHE,
+ 0,
+ full_range ? full_range : lower_32_bits(start),
+ full_range ? 0 : upper_32_bits(start),
+ full_range ? 0 : address_mask,
+ };
+
+ if (!INTEL_GUC_SUPPORTS_TLB_INVALIDATION_SELECTIVE(guc)) {
+ DRM_ERROR("Tlb invalidation: Operation not supported in this
platform!\n");
+ return 0;
+ }
+
+ GEM_BUG_ON(!IS_ALIGNED(start, I915_GTT_PAGE_SIZE_4K));
+ GEM_BUG_ON(!IS_ALIGNED(length, I915_GTT_PAGE_SIZE_4K));
+ GEM_BUG_ON(range_overflows(start, length, vm_total));
+
+ return guc_send_invalidate_tlb(guc, action, ARRAY_SIZE(action));
+}
+
+/*
+ * Selective TLB Invalidation for Context:
+ * Invalidates all TLB's for a specific context across all engines.
+ */
+int intel_guc_invalidate_tlb_page_selective_ctx(struct intel_guc *guc,
+ enum intel_guc_tlb_inval_mode
mode,
+ u64 start, u64 length, u32
ctxid)
+{
+ u64 vm_total = BIT_ULL(INTEL_INFO(guc_to_gt(guc)->i915)->ppgtt_size);
+ u32 address_mask = (ilog2(length) - ilog2(I915_GTT_PAGE_SIZE_4K));
+ u32 full_range = vm_total == length;
+ u32 action[] = {
+ INTEL_GUC_ACTION_TLB_INVALIDATION,
+ 0,
+ INTEL_GUC_TLB_INVAL_PAGE_SELECTIVE_CTX <<
INTEL_GUC_TLB_INVAL_TYPE_SHIFT |
+ mode << INTEL_GUC_TLB_INVAL_MODE_SHIFT |
+ INTEL_GUC_TLB_INVAL_FLUSH_CACHE,
+ ctxid,
+ full_range ? full_range : lower_32_bits(start),
+ full_range ? 0 : upper_32_bits(start),
+ full_range ? 0 : address_mask,
+ };
+
+ if (!INTEL_GUC_SUPPORTS_T
[PATCH v2 17/21] drm/i915: Add generic interface for tlb invalidation for XeHP
From: Prathap Kumar Valsan
Add an interface for GuC TLB actions, supporting both selective and
full TLB invalidations. After this change, when GuC is enabled,
tlb invalidations use GuC ct. Otherwise, use mmio interface.
Signed-off-by: Prathap Kumar Valsan
Cc: Niranjana Vishwanathapura
Cc: Fei Yang
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 +++
drivers/gpu/drm/i915/gt/intel_tlb.c | 78 -
drivers/gpu/drm/i915/gt/intel_tlb.h | 1 +
3 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 60d6eb5f245b..52508a9c23e5 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1054,6 +1054,14 @@
#define GEN12_GAM_DONE _MMIO(0xcf68)
+#define XEHP_TLB_INV_DESC0 _MMIO(0xcf7c)
+#define XEHP_TLB_INV_DESC0_ADDR_LO REG_GENMASK(31, 12)
+#define XEHP_TLB_INV_DESC0_ADDR_MASK REG_GENMASK(8, 3)
+#define XEHP_TLB_INV_DESC0_G REG_GENMASK(2, 1)
+#define XEHP_TLB_INV_DESC0_VALID REG_BIT(0)
+#define XEHP_TLB_INV_DESC1 _MMIO(0xcf80)
+#define XEHP_TLB_INV_DESC0_ADDR_HI REG_GENMASK(31, 0)
+
#define GEN7_HALF_SLICE_CHICKEN1 _MMIO(0xe100) /* IVB GT1 + VLV
*/
#define GEN7_MAX_PS_THREAD_DEP (8 << 12)
#define GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE (1 << 10)
diff --git a/drivers/gpu/drm/i915/gt/intel_tlb.c
b/drivers/gpu/drm/i915/gt/intel_tlb.c
index af8cae979489..15ed83226676 100644
--- a/drivers/gpu/drm/i915/gt/intel_tlb.c
+++ b/drivers/gpu/drm/i915/gt/intel_tlb.c
@@ -10,6 +10,7 @@
#include "intel_gt_pm.h"
#include "intel_gt_regs.h"
#include "intel_tlb.h"
+#include "uc/intel_guc.h"
struct reg_and_bit {
i915_reg_t reg;
@@ -159,11 +160,16 @@ void intel_gt_invalidate_tlb_full(struct intel_gt *gt,
u32 seqno)
return;
with_intel_gt_pm_if_awake(gt, wakeref) {
+ struct intel_guc *guc = >->uc.guc;
+
mutex_lock(>->tlb.invalidate_lock);
if (tlb_seqno_passed(gt, seqno))
goto unlock;
- mmio_invalidate_full(gt);
+ if (INTEL_GUC_SUPPORTS_TLB_INVALIDATION(guc))
+ intel_guc_invalidate_tlb_full(guc,
INTEL_GUC_TLB_INVAL_MODE_HEAVY);
+ else
+ mmio_invalidate_full(gt);
write_seqcount_invalidate(>->tlb.seqno);
unlock:
@@ -171,6 +177,76 @@ void intel_gt_invalidate_tlb_full(struct intel_gt *gt, u32
seqno)
}
}
+static bool mmio_invalidate_range(struct intel_gt *gt, u64 start, u64 length)
+{
+ u32 address_mask = (ilog2(length) - ilog2(I915_GTT_PAGE_SIZE_4K));
+ u64 vm_total = BIT_ULL(INTEL_INFO(gt->i915)->ppgtt_size);
+ intel_wakeref_t wakeref;
+ u32 dw0, dw1;
+ int err;
+
+ GEM_BUG_ON(!IS_ALIGNED(start, I915_GTT_PAGE_SIZE_4K));
+ GEM_BUG_ON(!IS_ALIGNED(length, I915_GTT_PAGE_SIZE_4K));
+ GEM_BUG_ON(range_overflows(start, length, vm_total));
+
+ dw0 = FIELD_PREP(XEHP_TLB_INV_DESC0_ADDR_LO, (lower_32_bits(start) >>
12)) |
+ FIELD_PREP(XEHP_TLB_INV_DESC0_ADDR_MASK, address_mask) |
+ FIELD_PREP(XEHP_TLB_INV_DESC0_G, 0x3) |
+ FIELD_PREP(XEHP_TLB_INV_DESC0_VALID, 0x1);
+ dw1 = upper_32_bits(start);
+
+ err = 0;
+ with_intel_gt_pm_if_awake(gt, wakeref) {
+ struct intel_uncore *uncore = gt->uncore;
+
+ intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
+
+ mutex_lock(>->tlb.invalidate_lock);
+ intel_uncore_write_fw(uncore, XEHP_TLB_INV_DESC1, dw1);
+ intel_uncore_write_fw(uncore, XEHP_TLB_INV_DESC0, dw0);
+ err = __intel_wait_for_register_fw(uncore,
+ XEHP_TLB_INV_DESC0,
+ XEHP_TLB_INV_DESC0_VALID,
+ 0, 100, 10, NULL);
+ mutex_unlock(>->tlb.invalidate_lock);
+
+ intel_uncore_forcewake_put_delayed(uncore, FORCEWAKE_ALL);
+ }
+
+ if (err)
+ drm_err_ratelimited(>->i915->drm,
+ "TLB invalidation response timed out\n");
+
+ return err == 0;
+}
+
+bool intel_gt_invalidate_tlb_range(struct intel_gt *gt,
+ u64 start, u64 length)
+{
+ struct intel_guc *guc = >->uc.guc;
+ intel_wakeref_t wakeref;
+
+ if (intel_gt_is_wedged(gt))
+ return true;
+
+ if (!INTEL_GUC_SUPPORTS_TLB_INVALID
[PATCH v2 03/21] drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
From: Chris Wilson
Ensure that the TLB of the OA unit is also invalidated
on gen12 HW, as just invalidating the TLB of an engine is not
enough.
Cc: sta...@vger.kernel.org
Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
Signed-off-by: Chris Wilson
Cc: Fei Yang
Cc: Andi Shyti
Acked-by: Thomas Hellström
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/intel_gt.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index c4d43da84d8e..1d84418e8676 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -11,6 +11,7 @@
#include "pxp/intel_pxp.h"
#include "i915_drv.h"
+#include "i915_perf_oa_regs.h"
#include "intel_context.h"
#include "intel_engine_pm.h"
#include "intel_engine_regs.h"
@@ -969,6 +970,15 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt)
awake |= engine->mask;
}
+ /* Wa_2207587034:tgl,dg1,rkl,adl-s,adl-p */
+ if (awake &&
+ (IS_TIGERLAKE(i915) ||
+IS_DG1(i915) ||
+IS_ROCKETLAKE(i915) ||
+IS_ALDERLAKE_S(i915) ||
+IS_ALDERLAKE_P(i915)))
+ intel_uncore_write_fw(uncore, GEN12_OA_TLB_INV_CR, 1);
+
spin_unlock_irq(&uncore->lock);
for_each_engine_masked(engine, gt, awake, tmp) {
--
2.36.1
[PATCH v2 13/21] drm/i915: Invalidate the TLBs on each GT
From: Chris Wilson
With multi-GT devices, the object may have been bound on each GT.
Invalidate the TLBs across all GT before releasing the pages
back to the system.
Signed-off-by: Chris Wilson
Cc: Fei Yang
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 4 +++-
drivers/gpu/drm/i915/gem/i915_gem_pages.c| 13 -
drivers/gpu/drm/i915/gt/intel_engine.h | 1 +
drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.h | 3 ++-
drivers/gpu/drm/i915/gt/intel_gt_defines.h | 11 +++
drivers/gpu/drm/i915/gt/intel_gt_types.h | 4 +++-
drivers/gpu/drm/i915/gt/intel_ppgtt.c| 4 ++--
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/i915_vma.c | 14 +++---
drivers/gpu/drm/i915/i915_vma.h | 2 +-
10 files changed, 42 insertions(+), 15 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_defines.h
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 9f6b14ec189a..3c1d0b750a67 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -17,6 +17,8 @@
#include "i915_selftest.h"
#include "i915_vma_resource.h"
+#include "gt/intel_gt_defines.h"
+
struct drm_i915_gem_object;
struct intel_fronbuffer;
struct intel_memory_region;
@@ -616,7 +618,7 @@ struct drm_i915_gem_object {
*/
bool dirty:1;
- u32 tlb;
+ u32 tlb[I915_MAX_GT];
} mm;
struct {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 1cd76cc5d9f3..4a6a2f2e8148 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -194,13 +194,16 @@ static void unmap_object(struct drm_i915_gem_object *obj,
void *ptr)
static void flush_tlb_invalidate(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
- struct intel_gt *gt = to_gt(i915);
+ struct intel_gt *gt;
+ int id;
- if (!obj->mm.tlb)
- return;
+ for_each_gt(gt, i915, id) {
+ if (!obj->mm.tlb[id])
+ continue;
- intel_gt_invalidate_tlb_full(gt, obj->mm.tlb);
- obj->mm.tlb = 0;
+ intel_gt_invalidate_tlb_full(gt, obj->mm.tlb[id]);
+ obj->mm.tlb[id] = 0;
+ }
}
struct sg_table *
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h
b/drivers/gpu/drm/i915/gt/intel_engine.h
index 04e435bce79b..fe1dc55bf8f7 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -18,6 +18,7 @@
#include "intel_gt_types.h"
#include "intel_timeline.h"
#include "intel_workarounds.h"
+#include "uc/intel_guc_submission.h"
struct drm_printer;
struct intel_context;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.h
b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.h
index 487b8a5520f1..8d41cf0c937a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.h
@@ -11,8 +11,9 @@
#include "i915_active.h"
#include "intel_gt_buffer_pool_types.h"
-struct intel_gt;
+enum i915_map_type;
struct i915_request;
+struct intel_gt;
struct intel_gt_buffer_pool_node *
intel_gt_get_buffer_pool(struct intel_gt *gt, size_t size,
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_defines.h
b/drivers/gpu/drm/i915/gt/intel_gt_defines.h
new file mode 100644
index ..7c711726d663
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_gt_defines.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __INTEL_GT_DEFINES__
+#define __INTEL_GT_DEFINES__
+
+#define I915_MAX_GT 4
+
+#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h
b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index 3804a583382b..b857c3972251 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -19,7 +19,6 @@
#include "uc/intel_uc.h"
#include "intel_gsc.h"
-#include "i915_vma.h"
#include "intel_engine_types.h"
#include "intel_gt_buffer_pool_types.h"
#include "intel_hwconfig.h"
@@ -31,8 +30,11 @@
#include "intel_wakeref.h"
#include "pxp/intel_pxp_types.h"
+#include "intel_gt_defines.h"
+
struct drm_i915_private;
struct i915_ggtt;
+struct i915_vma;
struct intel_engine_cs;
struct intel_uncore;
diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c
b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
index 2da6c82a8bd2..f764d250e929 100644
--- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
@
[PATCH v2 06/21] drm/i915/gt: Batch TLB invalidations
From: Chris Wilson
Invalidate TLB in patch, in order to reduce performance regressions.
Currently, every caller performs a full barrier around a TLB
invalidation, ignoring all other invalidations that may have already
removed their PTEs from the cache. As this is a synchronous operation
and can be quite slow, we cause multiple threads to contend on the TLB
invalidate mutex blocking userspace.
We only need to invalidate the TLB once after replacing our PTE to
ensure that there is no possible continued access to the physical
address before releasing our pages. By tracking a seqno for each full
TLB invalidate we can quickly determine if one has been performed since
rewriting the PTE, and only if necessary trigger one for ourselves.
That helps to reduce the performance regression introduced by TLB
invalidate logic.
[mchehab: rebased to not require moving the code to a separate file]
Cc: sta...@vger.kernel.org
Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
Suggested-by: Tvrtko Ursulin
Signed-off-by: Chris Wilson
Cc: Fei Yang
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
.../gpu/drm/i915/gem/i915_gem_object_types.h | 3 +-
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 21 +---
drivers/gpu/drm/i915/gt/intel_gt.c| 53 ++-
drivers/gpu/drm/i915/gt/intel_gt.h| 12 -
drivers/gpu/drm/i915/gt/intel_gt_types.h | 18 ++-
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 8 ++-
drivers/gpu/drm/i915/i915_vma.c | 34 +---
drivers/gpu/drm/i915/i915_vma.h | 1 +
drivers/gpu/drm/i915/i915_vma_resource.c | 5 +-
drivers/gpu/drm/i915/i915_vma_resource.h | 6 ++-
10 files changed, 125 insertions(+), 36 deletions(-)
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 5cf36a130061..9f6b14ec189a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -335,7 +335,6 @@ struct drm_i915_gem_object {
#define I915_BO_READONLY BIT(7)
#define I915_TILING_QUIRK_BIT 8 /* unknown swizzling; do not release! */
#define I915_BO_PROTECTED BIT(9)
-#define I915_BO_WAS_BOUND_BIT 10
/**
* @mem_flags - Mutable placement-related flags
*
@@ -616,6 +615,8 @@ struct drm_i915_gem_object {
* pages were last acquired.
*/
bool dirty:1;
+
+ u32 tlb;
} mm;
struct {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 6835279943df..8357dbdcab5c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -191,6 +191,18 @@ static void unmap_object(struct drm_i915_gem_object *obj,
void *ptr)
vunmap(ptr);
}
+static void flush_tlb_invalidate(struct drm_i915_gem_object *obj)
+{
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ struct intel_gt *gt = to_gt(i915);
+
+ if (!obj->mm.tlb)
+ return;
+
+ intel_gt_invalidate_tlb(gt, obj->mm.tlb);
+ obj->mm.tlb = 0;
+}
+
struct sg_table *
__i915_gem_object_unset_pages(struct drm_i915_gem_object *obj)
{
@@ -216,14 +228,7 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object
*obj)
__i915_gem_object_reset_page_iter(obj);
obj->mm.page_sizes.phys = obj->mm.page_sizes.sg = 0;
- if (test_and_clear_bit(I915_BO_WAS_BOUND_BIT, &obj->flags)) {
- struct drm_i915_private *i915 = to_i915(obj->base.dev);
- struct intel_gt *gt = to_gt(i915);
- intel_wakeref_t wakeref;
-
- with_intel_gt_pm_if_awake(gt, wakeref)
- intel_gt_invalidate_tlbs(gt);
- }
+ flush_tlb_invalidate(obj);
return pages;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 5c55a90672f4..f435e06125aa 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -38,8 +38,6 @@ static void __intel_gt_init_early(struct intel_gt *gt)
{
spin_lock_init(>->irq_lock);
- mutex_init(>->tlb_invalidate_lock);
-
INIT_LIST_HEAD(>->closed_vma);
spin_lock_init(>->closed_lock);
@@ -50,6 +48,8 @@ static void __intel_gt_init_early(struct intel_gt *gt)
intel_gt_init_reset(gt);
intel_gt_init_requests(gt);
intel_gt_init_timelines(gt);
+ mutex_init(>->tlb.invalidate_lock);
+ seqcount_mutex_init(>->tlb.seqno, >->tlb.invalidate_lock);
intel_gt_pm_init_early(gt);
intel_uc_init_early(>->uc);
@@ -770,6 +770,7 @@ void intel_gt_driver_late_release_all(struct
[PATCH v2 00/21] Fix performance regressions with TLB and add GuC support
TLB invalidation is a slow operation. It should not be doing lightly, as it causes performance regressions, like this: [178.821002] i915 :00:02.0: [drm] *ERROR* rcs0 TLB invalidation did not complete in 4ms! This series contain 1) some patches that makes TLB invalidation to happen only on active, non-wedged engines, doing cache invalidation in batch and only when GT objects are exposed to userspace: drm/i915/gt: Ignore TLB invalidations on idle engines drm/i915/gt: Only invalidate TLBs exposed to user manipulation drm/i915/gt: Skip TLB invalidations once wedged drm/i915/gt: Batch TLB invalidations drm/i915/gt: Move TLB invalidation to its own file 2) It fixes two bugs, being the first a workaround: drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations drm/i915: Invalidate the TLBs on each GT drm/i915/guc: Introduce TLB_INVALIDATION_ALL action 3) It adds GuC support. Besides providing TLB invalidation on some additional hardware, this should also help serializing GuC operations with TLB invalidation: drm/i915/guc: Introduce TLB_INVALIDATION_ALL action drm/i915/guc: Define CTB based TLB invalidation routines drm/i915: Add platform macro for selective tlb flush drm/i915: Define GuC Based TLB invalidation routines drm/i915: Add generic interface for tlb invalidation for XeHP drm/i915: Use selective tlb invalidations where supported 4) It adds the corresponding kernel-doc markups for the kAPI used for TLB invalidation. While I could have split this into smaller pieces, I'm opting to send them altogether, in order for CI trybot to better verify what issues will be closed with this series. --- v2: - no changes. Just rebased on the top of drm-tip: 2022y-07m-14d-08h-35m-36s, as CI trybot was having troubles applying it. Hopefully, it will now work. Chris Wilson (7): drm/i915/gt: Ignore TLB invalidations on idle engines drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations drm/i915/gt: Only invalidate TLBs exposed to user manipulation drm/i915/gt: Skip TLB invalidations once wedged drm/i915/gt: Batch TLB invalidations drm/i915/gt: Move TLB invalidation to its own file drm/i915: Invalidate the TLBs on each GT Mauro Carvalho Chehab (8): drm/i915/gt: document with_intel_gt_pm_if_awake() drm/i915/gt: describe the new tlb parameter at i915_vma_resource drm/i915/guc: use kernel-doc for enum intel_guc_tlb_inval_mode drm/i915/guc: document the TLB invalidation struct members drm/i915: document tlb field at struct drm_i915_gem_object drm/i915/gt: document TLB cache invalidation functions drm/i915/guc: describe enum intel_guc_tlb_invalidation_type drm/i915/guc: document TLB cache invalidation functions Piotr Piórkowski (1): drm/i915/guc: Introduce TLB_INVALIDATION_ALL action Prathap Kumar Valsan (5): drm/i915/guc: Define CTB based TLB invalidation routines drm/i915: Add platform macro for selective tlb flush drm/i915: Define GuC Based TLB invalidation routines drm/i915: Add generic interface for tlb invalidation for XeHP drm/i915: Use selective tlb invalidations where supported drivers/gpu/drm/i915/Makefile | 1 + .../gpu/drm/i915/gem/i915_gem_object_types.h | 6 +- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 28 +- drivers/gpu/drm/i915/gt/intel_engine.h| 1 + drivers/gpu/drm/i915/gt/intel_gt.c| 125 +--- drivers/gpu/drm/i915/gt/intel_gt.h| 2 - .../gpu/drm/i915/gt/intel_gt_buffer_pool.h| 3 +- drivers/gpu/drm/i915/gt/intel_gt_defines.h| 11 + drivers/gpu/drm/i915/gt/intel_gt_pm.h | 10 + drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 + drivers/gpu/drm/i915/gt/intel_gt_types.h | 22 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 8 +- drivers/gpu/drm/i915/gt/intel_tlb.c | 295 ++ drivers/gpu/drm/i915/gt/intel_tlb.h | 30 ++ .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 54 drivers/gpu/drm/i915/gt/uc/intel_guc.c| 232 ++ drivers/gpu/drm/i915/gt/uc/intel_guc.h| 36 +++ drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 24 +- drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 9 + .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 91 +- drivers/gpu/drm/i915/i915_drv.h | 4 +- drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/i915_vma.c | 46 ++- drivers/gpu/drm/i915/i915_vma.h | 2 + drivers/gpu/drm/i915/i915_vma_resource.c | 9 +- drivers/gpu/drm/i915/i915_vma_resource.h | 6 +- drivers/gpu/drm/i915/intel_device_info.h | 1 + 27 files changed, 910 insertions(+), 155 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_defines.h create mode 100644 drivers/gpu/drm/i915/gt/intel_tlb.c create mode 100644 drivers/gpu/drm/i915/gt/intel_tlb.h -- 2.36.1
[PATCH v2 04/21] drm/i915/gt: Only invalidate TLBs exposed to user manipulation
From: Chris Wilson
Don't flush TLBs when the buffer is only used in the GGTT under full
control of the kernel, as there's no risk of concurrent access
and stale access from prefetch.
We only need to invalidate the TLB if they are accessible by the user.
That helps to reduce the performance regression introduced by TLB
invalidate logic.
Cc: sta...@vger.kernel.org
Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
Signed-off-by: Chris Wilson
Cc: Fei Yang
Cc: Andi Shyti
Acked-by: Thomas Hellström
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/i915_vma.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index ef3b04c7e153..646f419b2035 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -538,7 +538,8 @@ int i915_vma_bind(struct i915_vma *vma,
bind_flags);
}
- set_bit(I915_BO_WAS_BOUND_BIT, &vma->obj->flags);
+ if (bind_flags & I915_VMA_LOCAL_BIND)
+ set_bit(I915_BO_WAS_BOUND_BIT, &vma->obj->flags);
atomic_or(bind_flags, &vma->flags);
return 0;
--
2.36.1
[PATCH v2 11/21] drm/i915/guc: document the TLB invalidation struct members
Add documentation for the 3 new members of struct intel_guc
that are used to handle TLB cache invalidation logic.
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index f82a121b0838..73c46d405dc4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -76,11 +76,23 @@ struct intel_guc {
*/
atomic_t outstanding_submission_g2h;
- /** @interrupts: pointers to GuC interrupt-managing functions. */
+ /**
+* @tlb_lookup: TLB cache invalidation lookup table.
+*/
struct xarray tlb_lookup;
+
+ /**
+* @serial_slot: index to the latest allocated element at the
+* @tlb_lookup xarray.
+*/
u32 serial_slot;
+
+ /**
+* @next_seqno: next index to be allocated at the @tlb_lookup xarray.
+*/
u32 next_seqno;
+ /** @interrupts: pointers to GuC interrupt-managing functions. */
struct {
void (*reset)(struct intel_guc *guc);
void (*enable)(struct intel_guc *guc);
--
2.36.1
[PATCH v2 09/21] drm/i915/guc: Define CTB based TLB invalidation routines
From: Prathap Kumar Valsan
Add routines to interface with GuC firmware for TLB invalidation.
Signed-off-by: Prathap Kumar Valsan
Cc: Bruce Chang
Cc: Michal Wajdeczko
Cc: Matthew Brost
Cc: Chris Wilson
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 35 +++
drivers/gpu/drm/i915/gt/uc/intel_guc.c| 90 ++
drivers/gpu/drm/i915/gt/uc/intel_guc.h| 13 +++
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 24 -
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 6 ++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 91 ++-
6 files changed, 253 insertions(+), 6 deletions(-)
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 4ef9990ed7f8..2e39d8df4c82 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
@@ -134,6 +134,10 @@ enum intel_guc_action {
INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601,
INTEL_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507,
INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A,
+ INTEL_GUC_ACTION_NOTIFY_MEMORY_CAT_ERROR = 0x6000,
+ INTEL_GUC_ACTION_PAGE_FAULT_NOTIFICATION = 0x6001,
+ INTEL_GUC_ACTION_TLB_INVALIDATION = 0x7000,
+ INTEL_GUC_ACTION_TLB_INVALIDATION_DONE = 0x7001,
INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002,
INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003,
INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004,
@@ -177,4 +181,35 @@ enum intel_guc_state_capture_event_status {
#define INTEL_GUC_STATE_CAPTURE_EVENT_STATUS_MASK 0x00FF
+#define INTEL_GUC_TLB_INVAL_TYPE_SHIFT 0
+#define INTEL_GUC_TLB_INVAL_MODE_SHIFT 8
+/* Flush PPC or SMRO caches along with TLB invalidation request */
+#define INTEL_GUC_TLB_INVAL_FLUSH_CACHE (1 << 31)
+
+enum intel_guc_tlb_invalidation_type {
+ INTEL_GUC_TLB_INVAL_GUC = 0x3,
+};
+
+/*
+ * 0: Heavy mode of Invalidation:
+ * The pipeline of the engine(s) for which the invalidation is targeted to is
+ * blocked, and all the in-flight transactions are guaranteed to be Globally
+ * Observed before completing the TLB invalidation
+ * 1: Lite mode of Invalidation:
+ * TLBs of the targeted engine(s) are immediately invalidated.
+ * In-flight transactions are NOT guaranteed to be Globally Observed before
+ * completing TLB invalidation.
+ * Light Invalidation Mode is to be used only when
+ * it can be guaranteed (by SW) that the address translations remain invariant
+ * for the in-flight transactions across the TLB invalidation. In other words,
+ * this mode can be used when the TLB invalidation is intended to clear out the
+ * stale cached translations that are no longer in use. Light Invalidation Mode
+ * is much faster than the Heavy Invalidation Mode, as it does not wait for the
+ * in-flight transactions to be GOd.
+ */
+enum intel_guc_tlb_inval_mode {
+ INTEL_GUC_TLB_INVAL_MODE_HEAVY = 0x0,
+ INTEL_GUC_TLB_INVAL_MODE_LITE = 0x1,
+};
+
#endif /* _ABI_GUC_ACTIONS_ABI_H */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 2706a8c65090..5c59f9b144a3 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -855,6 +855,96 @@ int intel_guc_self_cfg64(struct intel_guc *guc, u16 key,
u64 value)
return __guc_self_cfg(guc, key, 2, value);
}
+static int guc_send_invalidate_tlb(struct intel_guc *guc, u32 *action, u32
size)
+{
+ struct intel_guc_tlb_wait _wq, *wq = &_wq;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
+ int err = 0;
+ u32 seqno;
+
+ init_waitqueue_head(&_wq.wq);
+
+ if (xa_alloc_cyclic_irq(&guc->tlb_lookup, &seqno, wq,
+ xa_limit_32b, &guc->next_seqno,
+ GFP_ATOMIC | __GFP_NOWARN) < 0) {
+ /* Under severe memory pressure? Serialise TLB allocations */
+ xa_lock_irq(&guc->tlb_lookup);
+ wq = xa_load(&guc->tlb_lookup, guc->serial_slot);
+ wait_event_lock_irq(wq->wq,
+ !READ_ONCE(wq->status),
+ guc->tlb_lookup.xa_lock);
+ /*
+* Update wq->status under lock to ensure only one waiter can
+* issue the tlb invalidation command using the serial slot at a
+* time. The condition is set to false before releasing the lock
+* so that other caller continue to wait until woken up again.
+*/
+ wq->status = 1;
+ xa_unlock_irq(&guc->tlb_lookup);
+
+ seqno = guc->serial_slot;
+ }
+
+
[PATCH v2 18/21] drm/i915: Use selective tlb invalidations where supported
From: Prathap Kumar Valsan
For platforms supporting selective tlb invalidations, we don't need to
do a full tlb invalidation. Rather do a range based tlb invalidation for
every unbind of purged vma belongs to an active vm.
[mchehab: change moved from intel_ppgtt.c to i915_vma.c]
Signed-off-by: Prathap Kumar Valsan
Cc: Niranjana Vishwanathapura
Cc: Fei Yang
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 14 +-
drivers/gpu/drm/i915/i915_vma.h | 3 ++-
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c
b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
index f764d250e929..74782fb2ccbd 100644
--- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
@@ -211,7 +211,7 @@ void ppgtt_unbind_vma(struct i915_address_space *vm,
return;
vm->clear_range(vm, vma_res->start, vma_res->vma_size);
- vma_invalidate_tlb(vm, vma_res->tlb);
+ vma_invalidate_tlb(vm, vma_res->tlb, vma_res->start, vma_res->vma_size);
}
static unsigned long pd_count(u64 size, int shift)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 5edc745dcc51..6d881a6b403a 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1309,7 +1309,8 @@ I915_SELFTEST_EXPORT int i915_vma_get_pages(struct
i915_vma *vma)
return err;
}
-void vma_invalidate_tlb(struct i915_address_space *vm, u32 *tlb)
+void vma_invalidate_tlb(struct i915_address_space *vm, u32 *tlb,
+ u64 start, u64 size)
{
struct intel_gt *gt;
int id;
@@ -1325,9 +1326,11 @@ void vma_invalidate_tlb(struct i915_address_space *vm,
u32 *tlb)
* the most recent TLB invalidation seqno, and if we have not yet
* flushed the TLBs upon release, perform a full invalidation.
*/
- for_each_gt(gt, vm->i915, id)
- WRITE_ONCE(tlb[id],
- intel_gt_next_invalidate_tlb_full(vm->gt));
+ for_each_gt(gt, vm->i915, id) {
+ if (!intel_gt_invalidate_tlb_range(gt, start, size))
+ WRITE_ONCE(tlb[id],
+ intel_gt_next_invalidate_tlb_full(vm->gt));
+ }
}
static void __vma_put_pages(struct i915_vma *vma, unsigned int count)
@@ -1980,7 +1983,8 @@ struct dma_fence *__i915_vma_evict(struct i915_vma *vma,
bool async)
dma_fence_put(unbind_fence);
unbind_fence = NULL;
}
- vma_invalidate_tlb(vma->vm, vma->obj->mm.tlb);
+ vma_invalidate_tlb(vma->vm, vma->obj->mm.tlb,
+ vma->node.start, vma->size);
}
/*
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 33a58f605d75..3f0af9595e59 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -213,7 +213,8 @@ bool i915_vma_misplaced(const struct i915_vma *vma,
u64 size, u64 alignment, u64 flags);
void __i915_vma_set_map_and_fenceable(struct i915_vma *vma);
void i915_vma_revoke_mmap(struct i915_vma *vma);
-void vma_invalidate_tlb(struct i915_address_space *vm, u32 *tlb);
+void vma_invalidate_tlb(struct i915_address_space *vm, u32 *tlb,
+ u64 start, u64 size);
struct dma_fence *__i915_vma_evict(struct i915_vma *vma, bool async);
int __i915_vma_unbind(struct i915_vma *vma);
int __must_check i915_vma_unbind(struct i915_vma *vma);
--
2.36.1
[PATCH v2 02/21] drm/i915/gt: document with_intel_gt_pm_if_awake()
Add a kernel-doc markup to document this new macro. Signed-off-by: Mauro Carvalho Chehab --- To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover. See [PATCH v2 00/21] at: https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/ drivers/gpu/drm/i915/gt/intel_gt_pm.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index a334787a4939..4d4caf612fdc 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -55,6 +55,13 @@ static inline void intel_gt_pm_might_put(struct intel_gt *gt) for (tmp = 1, intel_gt_pm_get(gt); tmp; \ intel_gt_pm_put(gt), tmp = 0) +/** + * with_intel_gt_pm_if_awake - if GT is PM awake, get a reference to prevent + * it to sleep, run some code and then put the reference away. + * + * @gt: pointer to the gt + * @wf: pointer to a temporary wakeref. + */ #define with_intel_gt_pm_if_awake(gt, wf) \ for (wf = intel_gt_pm_get_if_awake(gt); wf; intel_gt_pm_put_async(gt), wf = 0) -- 2.36.1
[PATCH v2 14/21] drm/i915: document tlb field at struct drm_i915_gem_object
Add documentation to the TLB field inside
struct drm_i915_gem_object.
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 1 +
1 file changed, 1 insertion(+)
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 3c1d0b750a67..6f5b9e34a4d7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -618,6 +618,7 @@ struct drm_i915_gem_object {
*/
bool dirty:1;
+ /** @mm.tlb: array with TLB invalidate IDs */
u32 tlb[I915_MAX_GT];
} mm;
--
2.36.1
[PATCH v2 15/21] drm/i915: Add platform macro for selective tlb flush
From: Prathap Kumar Valsan
Add support for selective TLB invalidation, which is a platform
feature supported on XeHP.
Signed-off-by: Prathap Kumar Valsan
Cc: Niranjana Vishwanathapura
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/i915_pci.c | 1 +
drivers/gpu/drm/i915/intel_device_info.h | 1 +
3 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f1f70257dbe0..73494960a3a8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1312,6 +1312,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_GT_UC(dev_priv)(INTEL_INFO(dev_priv)->has_gt_uc)
+#define HAS_SELECTIVE_TLB_INVALIDATION(dev_priv) \
+ (INTEL_INFO(dev_priv)->has_selective_tlb_invalidation)
+
#define HAS_POOLED_EU(dev_priv)(INTEL_INFO(dev_priv)->has_pooled_eu)
#define HAS_GLOBAL_MOCS_REGISTERS(dev_priv)
(INTEL_INFO(dev_priv)->has_global_mocs)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index aacc10f2e73f..30d945fe384b 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1022,6 +1022,7 @@ static const struct intel_device_info adl_p_info = {
.has_reset_engine = 1, \
.has_rps = 1, \
.has_runtime_pm = 1, \
+ .has_selective_tlb_invalidation = 1, \
.ppgtt_size = 48, \
.ppgtt_type = INTEL_PPGTT_FULL
diff --git a/drivers/gpu/drm/i915/intel_device_info.h
b/drivers/gpu/drm/i915/intel_device_info.h
index 23bf230aa104..92a38b8f7c47 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -170,6 +170,7 @@ enum intel_ppgtt_type {
func(has_rc6p); \
func(has_rps); \
func(has_runtime_pm); \
+ func(has_selective_tlb_invalidation); \
func(has_snoop); \
func(has_coherent_ggtt); \
func(unfenced_needs_alignment); \
--
2.36.1
[PATCH v2 01/21] drm/i915/gt: Ignore TLB invalidations on idle engines
From: Chris Wilson
Check if the device is powered down prior to any engine activity,
as, on such cases, all the TLBs were already invalidated, so an
explicit TLB invalidation is not needed, thus reducing the
performance regression impact due to it.
This becomes more significant with GuC, as it can only do so when
the connection to the GuC is awake.
Cc: sta...@vger.kernel.org
Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
Signed-off-by: Chris Wilson
Cc: Fei Yang
Cc: Andi Shyti
Cc: Thomas Hellström
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 10 ++
drivers/gpu/drm/i915/gt/intel_gt.c| 17 ++---
drivers/gpu/drm/i915/gt/intel_gt_pm.h | 3 +++
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 97c820eee115..6835279943df 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -6,14 +6,15 @@
#include
+#include "gt/intel_gt.h"
+#include "gt/intel_gt_pm.h"
+
#include "i915_drv.h"
#include "i915_gem_object.h"
#include "i915_scatterlist.h"
#include "i915_gem_lmem.h"
#include "i915_gem_mman.h"
-#include "gt/intel_gt.h"
-
void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
struct sg_table *pages,
unsigned int sg_page_sizes)
@@ -217,10 +218,11 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object
*obj)
if (test_and_clear_bit(I915_BO_WAS_BOUND_BIT, &obj->flags)) {
struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ struct intel_gt *gt = to_gt(i915);
intel_wakeref_t wakeref;
- with_intel_runtime_pm_if_active(&i915->runtime_pm, wakeref)
- intel_gt_invalidate_tlbs(to_gt(i915));
+ with_intel_gt_pm_if_awake(gt, wakeref)
+ intel_gt_invalidate_tlbs(gt);
}
return pages;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 68c2b0d8f187..c4d43da84d8e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -12,6 +12,7 @@
#include "i915_drv.h"
#include "intel_context.h"
+#include "intel_engine_pm.h"
#include "intel_engine_regs.h"
#include "intel_ggtt_gmch.h"
#include "intel_gt.h"
@@ -924,6 +925,7 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt)
struct drm_i915_private *i915 = gt->i915;
struct intel_uncore *uncore = gt->uncore;
struct intel_engine_cs *engine;
+ intel_engine_mask_t awake, tmp;
enum intel_engine_id id;
const i915_reg_t *regs;
unsigned int num = 0;
@@ -947,26 +949,31 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt)
GEM_TRACE("\n");
- assert_rpm_wakelock_held(&i915->runtime_pm);
-
mutex_lock(>->tlb_invalidate_lock);
intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
spin_lock_irq(&uncore->lock); /* serialise invalidate with GT reset */
+ awake = 0;
for_each_engine(engine, gt, id) {
struct reg_and_bit rb;
+ if (!intel_engine_pm_is_awake(engine))
+ continue;
+
rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
if (!i915_mmio_reg_offset(rb.reg))
continue;
intel_uncore_write_fw(uncore, rb.reg, rb.bit);
+ awake |= engine->mask;
}
spin_unlock_irq(&uncore->lock);
- for_each_engine(engine, gt, id) {
+ for_each_engine_masked(engine, gt, awake, tmp) {
+ struct reg_and_bit rb;
+
/*
* HW architecture suggest typical invalidation time at 40us,
* with pessimistic cases up to 100us and a recommendation to
@@ -974,12 +981,8 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt)
*/
const unsigned int timeout_us = 100;
const unsigned int timeout_ms = 4;
- struct reg_and_bit rb;
rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
- if (!i915_mmio_reg_offset(rb.reg))
- continue;
-
if (__intel_wait_for_register_fw(uncore,
rb.reg, rb.bit, 0,
timeout_us, timeout_ms,
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
index bc898df7a48c..a334787a4939 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
+++ b/drivers/gpu/drm/i915/
[PATCH v2 19/21] drm/i915/gt: document TLB cache invalidation functions
Add a description for the kAPI functions inside intel_tlb.c.
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/gt/intel_tlb.c | 36 +
1 file changed, 36 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_tlb.c
b/drivers/gpu/drm/i915/gt/intel_tlb.c
index 15ed83226676..aa2e0086ae88 100644
--- a/drivers/gpu/drm/i915/gt/intel_tlb.c
+++ b/drivers/gpu/drm/i915/gt/intel_tlb.c
@@ -146,6 +146,18 @@ static void mmio_invalidate_full(struct intel_gt *gt)
intel_uncore_forcewake_put_delayed(uncore, FORCEWAKE_ALL);
}
+/**
+ * intel_gt_invalidate_tlb_full - do full TLB cache invalidation
+ * @gt: GT structure
+ * @seqno: sequence number
+ *
+ * Do a full TLB cache invalidation if the @seqno is bigger than the last
+ * full TLB cache invalidation.
+ *
+ * Note:
+ * The TLB cache invalidation logic depends on GEN-specific registers.
+ * It currently supports GEN8 to GEN12 and GuC-based TLB cache invalidation.
+ */
void intel_gt_invalidate_tlb_full(struct intel_gt *gt, u32 seqno)
{
intel_wakeref_t wakeref;
@@ -220,6 +232,17 @@ static bool mmio_invalidate_range(struct intel_gt *gt, u64
start, u64 length)
return err == 0;
}
+/**
+ * intel_gt_invalidate_tlb_range - do full TLB cache invalidation
+ * @gt: GT structure
+ * @start: range start
+ * @length: range length
+ *
+ * Do a selected TLB cache invalidation on a range pointed by @start
+ * with @length size.
+ *
+ * Only some GuC-based GPUs can do a selective cache invalidation.
+ */
bool intel_gt_invalidate_tlb_range(struct intel_gt *gt,
u64 start, u64 length)
{
@@ -247,12 +270,25 @@ bool intel_gt_invalidate_tlb_range(struct intel_gt *gt,
return true;
}
+/**
+ * intel_gt_init_tlb - initialize TLB-specific vars
+ * @gt: GT structure
+ *
+ * TLB cache invalidation logic internally uses some resources that require
+ * initialization. Should be called before doing any TLB cache invalidation.
+ */
void intel_gt_init_tlb(struct intel_gt *gt)
{
mutex_init(>->tlb.invalidate_lock);
seqcount_mutex_init(>->tlb.seqno, >->tlb.invalidate_lock);
}
+/**
+ * intel_gt_fini_tlb - initialize TLB-specific vars
+ * @gt: GT structure
+ *
+ * Frees any resources needed by TLB cache invalidation logic.
+ */
void intel_gt_fini_tlb(struct intel_gt *gt)
{
mutex_destroy(>->tlb.invalidate_lock);
--
2.36.1
[PATCH v2 08/21] drm/i915/gt: Move TLB invalidation to its own file
From: Chris Wilson
Prepare for supporting more TLB invalidation scenarios by moving
the current MMIO invalidation to its own file.
Signed-off-by: Chris Wilson
Cc: Fei Yang
Signed-off-by: Mauro Carvalho Chehab
---
To avoid mailbombing on a large number of people, only mailing lists were C/C
on the cover.
See [PATCH v2 00/21] at:
https://lore.kernel.org/all/cover.1657800199.git.mche...@kernel.org/
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 4 +-
drivers/gpu/drm/i915/gt/intel_gt.c| 168 +---
drivers/gpu/drm/i915/gt/intel_gt.h| 12 --
drivers/gpu/drm/i915/gt/intel_tlb.c | 183 ++
drivers/gpu/drm/i915/gt/intel_tlb.h | 29
drivers/gpu/drm/i915/i915_vma.c | 1 +
7 files changed, 219 insertions(+), 179 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/intel_tlb.c
create mode 100644 drivers/gpu/drm/i915/gt/intel_tlb.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 522ef9b4aff3..d3df9832d1f7 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -126,6 +126,7 @@ gt-y += \
gt/intel_sseu.o \
gt/intel_sseu_debugfs.o \
gt/intel_timeline.o \
+ gt/intel_tlb.o \
gt/intel_workarounds.o \
gt/shmem_utils.o \
gt/sysfs_engines.o
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 8357dbdcab5c..1cd76cc5d9f3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -7,7 +7,7 @@
#include
#include "gt/intel_gt.h"
-#include "gt/intel_gt_pm.h"
+#include "gt/intel_tlb.h"
#include "i915_drv.h"
#include "i915_gem_object.h"
@@ -199,7 +199,7 @@ static void flush_tlb_invalidate(struct drm_i915_gem_object
*obj)
if (!obj->mm.tlb)
return;
- intel_gt_invalidate_tlb(gt, obj->mm.tlb);
+ intel_gt_invalidate_tlb_full(gt, obj->mm.tlb);
obj->mm.tlb = 0;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index f435e06125aa..18d82cd620bd 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -11,9 +11,7 @@
#include "pxp/intel_pxp.h"
#include "i915_drv.h"
-#include "i915_perf_oa_regs.h"
#include "intel_context.h"
-#include "intel_engine_pm.h"
#include "intel_engine_regs.h"
#include "intel_ggtt_gmch.h"
#include "intel_gt.h"
@@ -31,6 +29,7 @@
#include "intel_renderstate.h"
#include "intel_rps.h"
#include "intel_gt_sysfs.h"
+#include "intel_tlb.h"
#include "intel_uncore.h"
#include "shmem_utils.h"
@@ -48,8 +47,7 @@ static void __intel_gt_init_early(struct intel_gt *gt)
intel_gt_init_reset(gt);
intel_gt_init_requests(gt);
intel_gt_init_timelines(gt);
- mutex_init(>->tlb.invalidate_lock);
- seqcount_mutex_init(>->tlb.seqno, >->tlb.invalidate_lock);
+ intel_gt_init_tlb(gt);
intel_gt_pm_init_early(gt);
intel_uc_init_early(>->uc);
@@ -770,7 +768,7 @@ void intel_gt_driver_late_release_all(struct
drm_i915_private *i915)
intel_gt_fini_requests(gt);
intel_gt_fini_reset(gt);
intel_gt_fini_timelines(gt);
- mutex_destroy(>->tlb.invalidate_lock);
+ intel_gt_fini_tlb(gt);
intel_engines_free(gt);
}
}
@@ -881,163 +879,3 @@ void intel_gt_info_print(const struct intel_gt_info *info,
intel_sseu_dump(&info->sseu, p);
}
-
-struct reg_and_bit {
- i915_reg_t reg;
- u32 bit;
-};
-
-static struct reg_and_bit
-get_reg_and_bit(const struct intel_engine_cs *engine, const bool gen8,
- const i915_reg_t *regs, const unsigned int num)
-{
- const unsigned int class = engine->class;
- struct reg_and_bit rb = { };
-
- if (drm_WARN_ON_ONCE(&engine->i915->drm,
-class >= num || !regs[class].reg))
- return rb;
-
- rb.reg = regs[class];
- if (gen8 && class == VIDEO_DECODE_CLASS)
- rb.reg.reg += 4 * engine->instance; /* GEN8_M2TCR */
- else
- rb.bit = engine->instance;
-
- rb.bit = BIT(rb.bit);
-
- return rb;
-}
-
-static void mmio_invalidate_full(struct intel_gt *gt)
-{
- static const i915_reg_t gen8_regs[] = {
- [RENDER_CLASS] = GEN8_RTCR,
- [VIDEO_DECODE_CLASS]= GEN8_M1TCR, /* , GEN8_M2TCR */
- [VIDEO_ENHANCEMENT_CLASS] = GEN8_VTCR,
- [COPY_ENGINE_CLASS] = GEN8_BTCR,
- };
- static const i915_reg_t gen12_regs[] = {
- [RENDER_CLASS] = GEN12_GFX_TLB_INV_CR,
- [VIDEO_DECODE_CLASS]= GEN12_VD_TLB_INV_CR,
- [VIDEO_ENHANCEMENT_CLASS] = GEN12_VE_TLB_INV_CR,
- [COPY_ENGIN
Re: [PATCH v2 4/5] drm/modes: Add support for driver-specific named modes
On Thu, Jul 14, 2022 at 11:04:09AM +0200, Geert Uytterhoeven wrote: > The mode parsing code recognizes named modes only if they are explicitly > listed in the internal whitelist, which is currently limited to "NTSC" > and "PAL". > > Provide a mechanism for drivers to override this list to support custom > mode names. > > Ideally, this list should just come from the driver's actual list of > modes, but connector->probed_modes is not yet populated at the time of > parsing. > > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Hans de Goede Like we discussed on IRC, I'm not sure allowing drivers to handle named modes is the right thing to do. Named modes in general were a workaround the fact that we were missing infos in drm_display_mode to describe all the modes. I think we really should focus on addressing that first, and then creating some kind of backward compat layer to create an initial DRM state from a named mode provided on the command line. Maxime signature.asc Description: PGP signature
Re: [PATCH 3/3] drm/komeda - Fix handling of pending crtc state commit to avoid lock-up
On 2022-07-11 11:13, Liviu Dudau wrote:
[...]
But nothing worrying. It does work, though doesn't compile due to:
drivers/gpu/drm/arm/display/komeda/komeda_kms.c: In function
‘komeda_kms_atomic_commit_hw_done’:
drivers/gpu/drm/arm/display/komeda/komeda_kms.c:77:9: error: ‘for’ loop
initial declarations are only allowed in C99 or C11 mode
77 | for (int i = 0; i < kms->n_crtcs; i++) {
| ^~~
drivers/gpu/drm/arm/display/komeda/komeda_kms.c:77:9: note: use option
‘-std=c9
’, ‘-std=gnu99’, ‘-std=c11’ or ‘-std=gnu11’ to compile your code
but that was a trivial fixup.
Interesting that I'm not seeing that, probably due to using GCC12? Anyway, I'll
fix
that and send a proper patch.
FWIW we do use -std=gnu11 since 5.18 (see e8c07082a810), but I'm not
entirely sure what the status quo is for using the new features in fixes
which might also warrant backporting to stable. I believe Carsten's
stuck on an older kernel thanks to constraints of the rest of that
project ;)
Cheers,
Robin.
