Re: [PATCH v5] dma-buf: Add DmaBufTotal counter in meminfo
On Wed, Apr 21, 2021 at 05:35:57PM +, peter.enderb...@sony.com wrote: > On 4/21/21 5:31 PM, Mike Rapoport wrote: > > On Wed, Apr 21, 2021 at 10:37:11AM +, peter.enderb...@sony.com wrote: > >> On 4/21/21 11:15 AM, Daniel Vetter wrote: > >>> We need to understand what the "correct" value is. Not in terms of kernel > >>> code, but in terms of semantics. Like if userspace allocates a GL texture, > >>> is this supposed to show up in your metric or not. Stuff like that. > >> That it like that would like to only one pointer type. You need to know > >> what > >> > >> you pointing at to know what it is. it might be a hardware or a other > >> pointer. > >> > >> If there is a limitation on your pointers it is a good metric to count them > >> even if you don't know what they are. Same goes for dma-buf, they > >> are generic, but they consume some resources that are counted in pages. > >> > >> It would be very good if there a sub division where you could measure > >> all possible types separately. We have the detailed in debugfs, but > >> nothing > >> for the user. A summary in meminfo seems to be the best place for such > >> metric. > > > > Let me try to summarize my understanding of the problem, maybe it'll help > > others as well. > > Thanks! > > > > A device driver allocates memory and exports this memory via dma-buf so > > that this memory will be accessible for userspace via a file descriptor. > > > > The allocated memory can be either allocated with alloc_page() from system > > RAM or by other means from dedicated VRAM (that is not managed by Linux mm) > > or even from on-device memory. > > > > The dma-buf driver tracks the amount of the memory it was requested to > > export and the size it sees is available at debugfs and fdinfo. > > > > The debugfs is not available to user and maybe entirely disabled in > > production systems. > > > > There could be quite a few open dma-bufs so there is no overall summary, > > plus fdinfo in production systems your refer to is also unavailable to the > > user because of selinux policy. > > > > And there are a few details that are not clear to me: > > > > * Since DRM device drivers seem to be the major user of dma-buf exports, > > why cannot we add information about their memory consumption to, say, > > /sys/class/graphics/drm/cardX/memory-usage? > > Android is using it for binder that connect more or less everything > internally. Ok, then it rules out /sys/class/graphics indeed. > > * How exactly user generates reports that would include the new counters? > > From my (mostly outdated) experience Android users won't open a terminal > > and type 'cat /proc/meminfo' there. I'd presume there is a vendor agent > > that collects the data and sends it for analysis. In this case what is > > the reason the vendor is unable to adjust selinix policy so that the > > agent will be able to access fdinfo? > > When you turn on developer mode on android you can use > usb with a program called adb. And there you get a normal shell. > > (not root though) > > There is applications that non developers can use to get > information. It is very limited though and there are API's > provide it. > > > > > > * And, as others already mentioned, it is not clear what are the problems > > that can be detected by examining DmaBufTotal except saying "oh, there is > > too much/too little memory exported via dma-buf". What would be user > > visible effects of these problems? What are the next steps to investigate > > them? What other data will be probably required to identify root cause? > > > When you debug thousands of devices it is quite nice to have > ways to classify what the problem it is not. The normal user does not > see anything of this. However they can generate bug-reports that > collect information about as much they can. Then the user have > to provide this bug-report to the manufacture or mostly the > application developer. And when the problem is > system related we need to reproduce the issue on a full > debug enabled unit. So the flow is like this: * a user has a problem and reports it to an application developer; at best the user runs simple and limited app to collect some data * if the application developer considers this issue as a system related they can open adb and collect some more information about the system using non-root shell with selinux policy restrictions and send this information to the device manufacturer. * the manufacturer continues to debug the issue and at this point as much information is possible would have been useful. In this flow I still fail to understand why the manufacturer cannot provide userspace tools that will be able to collect the required information. These tools not necessarily need to target the end user, they may be only intended for the application developers, e.g. policy could allow such tool to access some of the system data only when the system is in developer mode. -- Sincerely yours, Mik
Re: [PATCH v3 1/2] dt-bindings: display: bridge: Add bindings for SN65DSI83/84/85
On Thu, Apr 22, 2021 at 4:04 AM Marek Vasut wrote: > > On 4/8/21 4:45 PM, Jagan Teki wrote: > > On Wed, Mar 24, 2021 at 7:26 PM Claudius Heine wrote: > >> > >> Hi Jagan, > >> > >> On 2021-02-14 18:44, Jagan Teki wrote: > >>> SN65DSI83/84/85 devices are MIPI DSI to LVDS based bridge > >>> controller IC's from Texas Instruments. > >>> > >>> SN65DSI83 - Single Channel DSI to Single-link LVDS bridge > >>> SN65DSI84 - Single Channel DSI to Dual-link LVDS bridge > >>> SN65DSI85 - Dual Channel DSI to Dual-link LVDS bridge > >>> > >>> Right now the bridge driver is supporting Channel A with single > >>> link, so dt-bindings documented according to it. > >> > >> Do you know when we can expect a v4 for this? > >> > >> I am currently working on top of your patch set to setup a dual-link > >> LVDS bridge of SN65DSI84. > > > > Yes, I'm planning to send v4 this week. will keep you in CC. thanks! > > I haven't seen any activity here for over two weeks, so I decided to > send V2 of the driver I wrote, now tested on both DSI83 and DSI84. It delayed me since I have considered several comments from the Mailing list to wrote Dual Link-LVDS configuration support. I have a plan to send v4 in the coming weekend with these changes, I thought it would be the possible driver to support 1 and 2 links LVDS. Jagan. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] coda: fix reference counting in coda_file_mmap error path
On Wed, Apr 21, 2021 at 03:20:11PM +0200, Christian König wrote: > mmap_region() now calls fput() on the vma->vm_file. > > So we need to drop the extra reference on the coda file instead of the > host file. > > Signed-off-by: Christian König > Fixes: 1527f926fd04 ("mm: mmap: fix fput in error path v2") > CC: sta...@vger.kernel.org # 5.11+ Reviewed-by: Daniel Vetter > --- > fs/coda/file.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/coda/file.c b/fs/coda/file.c > index 128d63df5bfb..ef5ca22bfb3e 100644 > --- a/fs/coda/file.c > +++ b/fs/coda/file.c > @@ -175,10 +175,10 @@ coda_file_mmap(struct file *coda_file, struct > vm_area_struct *vma) > ret = call_mmap(vma->vm_file, vma); > > if (ret) { > - /* if call_mmap fails, our caller will put coda_file so we > - * should drop the reference to the host_file that we got. > + /* if call_mmap fails, our caller will put host_file so we > + * should drop the reference to the coda_file that we got. >*/ > - fput(host_file); > + fput(coda_file); > kfree(cvm_ops); > } else { > /* here we add redirects for the open/close vm_operations */ > -- > 2.25.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] ovl: fix reference counting in ovl_mmap error path
On Wed, Apr 21, 2021 at 03:20:12PM +0200, Christian König wrote: > mmap_region() now calls fput() on the vma->vm_file. > > Fix this by using vma_set_file() so it doesn't need to be > handled manually here any more. > > Signed-off-by: Christian König > Fixes: 1527f926fd04 ("mm: mmap: fix fput in error path v2") > CC: sta...@vger.kernel.org # 5.11+ > --- > fs/overlayfs/file.c | 11 +-- > 1 file changed, 1 insertion(+), 10 deletions(-) > > diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c > index dbfb35fb0ff7..3847cdc069b5 100644 > --- a/fs/overlayfs/file.c > +++ b/fs/overlayfs/file.c > @@ -430,20 +430,11 @@ static int ovl_mmap(struct file *file, struct > vm_area_struct *vma) > if (WARN_ON(file != vma->vm_file)) > return -EIO; > > - vma->vm_file = get_file(realfile); > + vma_set_file(vma, realfile); Reviewed-by: Daniel Vetter > > old_cred = ovl_override_creds(file_inode(file)->i_sb); > ret = call_mmap(vma->vm_file, vma); > revert_creds(old_cred); > - > - if (ret) { > - /* Drop reference count from new vm_file value */ > - fput(realfile); > - } else { > - /* Drop reference count from previous vm_file value */ > - fput(file); > - } > - > ovl_file_accessed(file); > > return ret; > -- > 2.25.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Fix docbook descriptions for i915_cmd_parser
On Wed, Apr 21, 2021 at 04:39:10PM +0200, Maarten Lankhorst wrote: > Op 21-04-2021 om 16:32 schreef Daniel Vetter: > > On Wed, Apr 21, 2021 at 2:03 PM Maarten Lankhorst > > wrote: > >> Fixes the following htmldocs warnings: > >> drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function > >> parameter 'trampoline' description in 'intel_engine_cmd_parser' > >> drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter > >> or member 'jump_whitelist' not described in 'intel_engine_cmd_parser' > >> drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter > >> or member 'shadow_map' not described in 'intel_engine_cmd_parser' > >> drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter > >> or member 'batch_map' not described in 'intel_engine_cmd_parser' > >> drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function > >> parameter 'trampoline' description in 'intel_engine_cmd_parser' > >> > >> Reported-by: Stephen Rothwell > >> Signed-off-by: Maarten Lankhorst > >> --- > >> drivers/gpu/drm/i915/i915_cmd_parser.c | 16 +++- > >> 1 file changed, 15 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c > >> b/drivers/gpu/drm/i915/i915_cmd_parser.c > >> index e6f1e93a..afb9b7516999 100644 > >> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c > >> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c > >> @@ -1369,6 +1369,18 @@ static int check_bbstart(u32 *cmd, u32 offset, u32 > >> length, > >> return 0; > >> } > >> > >> +/** > >> + * intel_engine_cmd_parser_alloc_jump_whitelist() - preallocate jump > >> whitelist for intel_engine_cmd_parser() > >> + * @batch_length: length of the commands in batch_obj > >> + * @trampoline: Whether jump trampolines are used. > >> + * > >> + * Preallocates a jump whitelist for parsing the cmd buffer in > >> intel_engine_cmd_parser(). > >> + * This has to be preallocated, because the command parser runs in > >> signaling context, > >> + * and may not allocate any memory. > >> + * > >> + * Return: NULL or pointer to a jump whitelist, or ERR_PTR() on failure. > >> Use > >> + * IS_ERR() to check for errors. Must bre freed() with kfree(). > > IS_ERR_OR_NULL or needs an actual bugfix in the code since we're not > > consistent. Also s/bre/be/ > We're sort of consistent, NULL is a valid return code. IS_ERR is only on > faliure. :) Maybe explain that and then Reviewed-by: Daniel Vetter Cheers, Daniel > > -Daniel > > > >> + */ > >> unsigned long *intel_engine_cmd_parser_alloc_jump_whitelist(u32 > >> batch_length, > >> bool > >> trampoline) > >> { > >> @@ -1401,7 +1413,9 @@ unsigned long > >> *intel_engine_cmd_parser_alloc_jump_whitelist(u32 batch_length, > >> * @batch_offset: byte offset in the batch at which execution starts > >> * @batch_length: length of the commands in batch_obj > >> * @shadow: validated copy of the batch buffer in question > >> - * @trampoline: whether to emit a conditional trampoline at the end of > >> the batch > >> + * @jump_whitelist: buffer preallocated with > >> intel_engine_cmd_parser_alloc_jump_whitelist() > >> + * @shadow_map: mapping to @shadow vma > >> + * @batch_map: mapping to @batch vma > >> * > >> * Parses the specified batch buffer looking for privilege violations as > >> * described in the overview. > >> -- > >> 2.31.0 > >> > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/ttm: fix error handling if no BO can be swapped out
Signed-off-by: Shiwu Zhang --- drivers/gpu/drm/ttm/ttm_device.c | 2 +- drivers/gpu/drm/ttm/ttm_tt.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 1f2024164d72..5dc92b056f0a 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -112,7 +112,7 @@ int ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { struct ttm_global *glob = &ttm_glob; struct ttm_device *bdev; - int ret = -EBUSY; + int ret = 0; mutex_lock(&ttm_global_mutex); list_for_each_entry(bdev, &glob->device_list, device_list) { diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 48c407cff112..539e0232cb3b 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -329,6 +329,8 @@ int ttm_tt_populate(struct ttm_device *bdev, ttm_dma32_pages_limit) { ret = ttm_global_swapout(ctx, GFP_KERNEL); + if (ret == 0) + break; if (ret < 0) goto error; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings
On 22/04/2021 00:31, Marek Vasut wrote: > Add DT binding document for TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge. > > Signed-off-by: Marek Vasut > Cc: Douglas Anderson > Cc: Jagan Teki > Cc: Laurent Pinchart > Cc: Linus Walleij > Cc: Rob Herring > Cc: Sam Ravnborg > Cc: Stephen Boyd > Cc: devicet...@vger.kernel.org > To: dri-devel@lists.freedesktop.org > --- > V2: Add compatible string for SN65DSI84, since this is now tested on it > --- > .../bindings/display/bridge/ti,sn65dsi83.yaml | 134 ++ > 1 file changed, 134 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > new file mode 100644 > index ..42d11b46a1eb > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > @@ -0,0 +1,134 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: SN65DSI83 and SN65DSI84 DSI to LVDS bridge chip > + > +maintainers: > + - Marek Vasut > + > +description: | > + Texas Instruments SN65DSI83 1x Single-link MIPI DSI > + to 1x Single-link LVDS > + https://www.ti.com/lit/gpn/sn65dsi83 > + Texas Instruments SN65DSI84 1x Single-link MIPI DSI > + to 1x Dual-link or 2x Single-link LVDS > + https://www.ti.com/lit/gpn/sn65dsi84 > + > +properties: > + compatible: > +oneOf: > + - const: ti,sn65dsi83 > + - const: ti,sn65dsi84 > + > + reg: > +const: 0x2d > + > + enable-gpios: > +maxItems: 1 > +description: GPIO specifier for bridge_en pin (active high). > + > + ports: > +type: object > +additionalProperties: false > + > +properties: > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 0 > + > + port@0: > +type: object > +additionalProperties: false > + > +description: > + Video port for MIPI DSI input > + > +properties: > + reg: > +const: 0 > + > + endpoint: > +type: object > +additionalProperties: false > +properties: > + remote-endpoint: true > + data-lanes: > +description: array of physical DSI data lane indexes. > + > +required: > + - reg > + > + port@1: > +type: object > +additionalProperties: false > + > +description: > + Video port for LVDS output (panel or bridge). > + > +properties: > + reg: > +const: 1 > + > + endpoint: > +type: object > +additionalProperties: false > +properties: > + remote-endpoint: true Similar to Jagan's serie, would be great to add bindings for the dual-link LVDS even if not supported by the driver (the driver can fails with a verbose error). Neil > + > +required: > + - reg > + > +required: > + - "#address-cells" > + - "#size-cells" > + - port@0 > + - port@1 > + > +required: > + - compatible > + - reg > + - enable-gpios > + - ports > + > +additionalProperties: false > + > +examples: > + - | > +#include > + > +i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + bridge@2d { > +compatible = "ti,sn65dsi83"; > +reg = <0x2d>; > + > +enable-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>; > + > +ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > +reg = <0>; > +endpoint { > + remote-endpoint = <&dsi0_out>; > + data-lanes = <1 2 3 4>; > +}; > + }; > + > + port@1 { > +reg = <1>; > +endpoint { > + remote-endpoint = <&panel_in_lvds>; > +}; > + }; > +}; > + }; > +}; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm/ttm: fix error handling if no BO can be swapped out
[AMD Official Use Only - Internal Distribution Only] Hi Chris, Yes. I'll rework the patch. Thanks for your comments. --Brs, Morris Zhang MLSE Linux ML SRDC Ext. 25147 -Original Message- From: Koenig, Christian Sent: Wednesday, April 21, 2021 9:34 PM To: Zhang, Morris ; dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/ttm: fix error handling if no BO can be swapped out Am 20.04.21 um 16:32 schrieb Shiwu Zhang: > In case that all pre-allocated BOs are busy, just continue to populate > BOs since likely half of system memory in total is still free. > > Signed-off-by: Shiwu Zhang > --- > drivers/gpu/drm/ttm/ttm_device.c | 4 ++-- > drivers/gpu/drm/ttm/ttm_tt.c | 2 ++ > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_device.c > b/drivers/gpu/drm/ttm/ttm_device.c > index 1f2024164d72..0200709db9be 100644 > --- a/drivers/gpu/drm/ttm/ttm_device.c > +++ b/drivers/gpu/drm/ttm/ttm_device.c > @@ -133,7 +133,7 @@ int ttm_device_swapout(struct ttm_device *bdev, struct > ttm_operation_ctx *ctx, > struct ttm_resource_manager *man; > struct ttm_buffer_object *bo; > unsigned i, j; > - int ret; > + int ret=-EBUSY; > > spin_lock(&bdev->lru_lock); > for (i = TTM_PL_SYSTEM; i < TTM_NUM_MEM_TYPES; ++i) { @@ -161,7 > +161,7 @@ int ttm_device_swapout(struct ttm_device *bdev, struct > ttm_operation_ctx *ctx, > } > } > spin_unlock(&bdev->lru_lock); > - return 0; > + return ret; The function should return the number of pages swapped out. Returning 0 here is already perfectly ok. > } > EXPORT_SYMBOL(ttm_device_swapout); > > diff --git a/drivers/gpu/drm/ttm/ttm_tt.c > b/drivers/gpu/drm/ttm/ttm_tt.c index 48c407cff112..4e1e06a04428 100644 > --- a/drivers/gpu/drm/ttm/ttm_tt.c > +++ b/drivers/gpu/drm/ttm/ttm_tt.c > @@ -329,6 +329,8 @@ int ttm_tt_populate(struct ttm_device *bdev, > ttm_dma32_pages_limit) { > > ret = ttm_global_swapout(ctx, GFP_KERNEL); > + if (ret == -EBUSY) > + break; > if (ret < 0) > goto error; Here we should just have a check for ret == 0 instead of testing for -EBUSY. Regards, Christian. > } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings
On Thu, Apr 22, 2021 at 4:01 AM Marek Vasut wrote: > > Add DT binding document for TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge. > > Signed-off-by: Marek Vasut > Cc: Douglas Anderson > Cc: Jagan Teki > Cc: Laurent Pinchart > Cc: Linus Walleij > Cc: Rob Herring > Cc: Sam Ravnborg > Cc: Stephen Boyd > Cc: devicet...@vger.kernel.org > To: dri-devel@lists.freedesktop.org > --- > V2: Add compatible string for SN65DSI84, since this is now tested on it > --- > .../bindings/display/bridge/ti,sn65dsi83.yaml | 134 ++ > 1 file changed, 134 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > new file mode 100644 > index ..42d11b46a1eb > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > @@ -0,0 +1,134 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: SN65DSI83 and SN65DSI84 DSI to LVDS bridge chip Can it possible to wait for my v4 to have dual-link LVDS supported which is quite discussing points on previous versions? Jagan. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm: drm_connector.c: Use tabs for code indents
On Wed, Apr 21, 2021 at 08:42:49PM +0100, Beatriz Martins de Carvalho wrote: > Remove space and use tabs for indent the code to follow the > Linux kernel coding conventions. > Problem found by checkpatch > > Signed-off-by: Beatriz Martins de Carvalho > Both of your patch sets applied to drm-misc-next for 5.14, thanks a lot. -Daniel > --- > drivers/gpu/drm/drm_connector.c | 38 - > 1 file changed, 19 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 7631f76e7f34..38600c3a6ab2 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1958,11 +1958,11 @@ int drm_connector_set_path_property(struct > drm_connector *connector, > int ret; > > ret = drm_property_replace_global_blob(dev, > -&connector->path_blob_ptr, > -strlen(path) + 1, > -path, > -&connector->base, > -dev->mode_config.path_property); > +&connector->path_blob_ptr, > +strlen(path) + 1, > +path, > +&connector->base, > +dev->mode_config.path_property); > return ret; > } > EXPORT_SYMBOL(drm_connector_set_path_property); > @@ -1988,11 +1988,11 @@ int drm_connector_set_tile_property(struct > drm_connector *connector) > > if (!connector->has_tile) { > ret = drm_property_replace_global_blob(dev, > - > &connector->tile_blob_ptr, > - 0, > - NULL, > - &connector->base, > - > dev->mode_config.tile_property); > + > &connector->tile_blob_ptr, > + 0, > + NULL, > + &connector->base, > + > dev->mode_config.tile_property); > return ret; > } > > @@ -2003,11 +2003,11 @@ int drm_connector_set_tile_property(struct > drm_connector *connector) >connector->tile_h_size, connector->tile_v_size); > > ret = drm_property_replace_global_blob(dev, > -&connector->tile_blob_ptr, > -strlen(tile) + 1, > -tile, > -&connector->base, > -dev->mode_config.tile_property); > +&connector->tile_blob_ptr, > +strlen(tile) + 1, > +tile, > +&connector->base, > +dev->mode_config.tile_property); > return ret; > } > EXPORT_SYMBOL(drm_connector_set_tile_property); > @@ -2076,10 +2076,10 @@ int drm_connector_update_edid_property(struct > drm_connector *connector, > > ret = drm_property_replace_global_blob(dev, > &connector->edid_blob_ptr, > -size, > -edid, > -&connector->base, > -dev->mode_config.edid_property); > +size, > +edid, > +&connector->base, > +dev->mode_config.edid_property); > if (ret) > return ret; > return drm_connector_set_tile_property(connector); > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/bridge: nwl-dsi: Get MIPI DSI controller and PHY ready in ->mode_set()
Hi, On 22/04/2021 07:14, Liu Ying wrote: > Some MIPI DSI panel drivers like 'raydium,rm68200' send > MIPI_DCS_SET_DISPLAY_ON commands in panel_funcs->prepare(), which > requires the MIPI DSI controller and PHY to be ready beforehand. > Without this patch, the nwl-dsi driver gets the MIPI DSI controller > and PHY ready in bridge_funcs->pre_enable(), which happens after > the panel_funcs->prepare(). So, this patch shifts the bridge operation > ealier from bridge_funcs->pre_enable() to bridge_funcs->mode_set(). This makes sense, this is how we do on the synopsys dw mipi dsi driver. > This way, more MIPI DSI panels can connect to this nwl-dsi bridge. > Care is taken to make sure bridge_funcs->mode_set()/atomic_disable() > are called in pairs, which includes removing a check on unchanged HS > clock rate and forcing a full modeset when only connector's DPMS is > brought out of "Off" status. I would split the changes in multiple patches to clarify the changes. Neil > > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: David Airlie > Cc: Daniel Vetter > Cc: Guido Günther > Cc: Robert Chiras > Cc: NXP Linux Team > Signed-off-by: Liu Ying > --- > v1->v2: > * Fix commit message typo - s/unchange/unchanged/ > > drivers/gpu/drm/bridge/nwl-dsi.c | 86 > +--- > 1 file changed, 46 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c > b/drivers/gpu/drm/bridge/nwl-dsi.c > index be6bfc5..229f0b1 100644 > --- a/drivers/gpu/drm/bridge/nwl-dsi.c > +++ b/drivers/gpu/drm/bridge/nwl-dsi.c > @@ -21,6 +21,7 @@ > #include > #include > > +#include > #include > #include > #include > @@ -661,7 +662,7 @@ static irqreturn_t nwl_dsi_irq_handler(int irq, void > *data) > return IRQ_HANDLED; > } > > -static int nwl_dsi_enable(struct nwl_dsi *dsi) > +static int nwl_dsi_mode_set(struct nwl_dsi *dsi) > { > struct device *dev = dsi->dev; > union phy_configure_opts *phy_cfg = &dsi->phy_cfg; > @@ -748,7 +749,9 @@ static int nwl_dsi_disable(struct nwl_dsi *dsi) > return 0; > } > > -static void nwl_dsi_bridge_disable(struct drm_bridge *bridge) > +static void > +nwl_dsi_bridge_atomic_disable(struct drm_bridge *bridge, > + struct drm_bridge_state *old_bridge_state) > { > struct nwl_dsi *dsi = bridge_to_dsi(bridge); > int ret; > @@ -809,17 +812,6 @@ static int nwl_dsi_get_dphy_params(struct nwl_dsi *dsi, > return 0; > } > > -static bool nwl_dsi_bridge_mode_fixup(struct drm_bridge *bridge, > - const struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode) > -{ > - /* At least LCDIF + NWL needs active high sync */ > - adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); > - adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC); > - > - return true; > -} > - > static enum drm_mode_status > nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge, > const struct drm_display_info *info, > @@ -837,6 +829,29 @@ nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge, > return MODE_OK; > } > > +static int nwl_dsi_bridge_atomic_check(struct drm_bridge *bridge, > +struct drm_bridge_state *bridge_state, > +struct drm_crtc_state *crtc_state, > +struct drm_connector_state *conn_state) > +{ > + struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; > + > + /* At least LCDIF + NWL needs active high sync */ > + adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); > + adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC); > + > + /* > + * Do a full modeset if crtc_state->active is changed to be true. > + * This ensures our ->mode_set() is called to get the DSI controller > + * and the PHY ready to send DCS commands, when only the connector's > + * DPMS is brought out of "Off" status. > + */ > + if (crtc_state->active_changed && crtc_state->active) > + crtc_state->mode_changed = true; > + > + return 0; > +} > + > static void > nwl_dsi_bridge_mode_set(struct drm_bridge *bridge, > const struct drm_display_mode *mode, > @@ -852,13 +867,6 @@ nwl_dsi_bridge_mode_set(struct drm_bridge *bridge, > if (ret < 0) > return; > > - /* > - * If hs clock is unchanged, we're all good - all parameters are > - * derived from it atm. > - */ > - if (new_cfg.mipi_dphy.hs_clk_rate == dsi->phy_cfg.mipi_dphy.hs_clk_rate) > - return; > - > phy_ref_rate = clk_get_rate(dsi->phy_ref_clk); > DRM_DEV_DEBUG_DRIVER(dev, "PHY at ref rate: %lu\n", phy_ref_rate); > /* Save the
Re: [PATCH v2 0/9] drm: Add privacy-screen class and connector properties
Hi, On Wednesday, April 21st, 2021 at 10:47 PM, Hans de Goede wrote: > There now is GNOME userspace code using the new properties: > https://hackmd.io/@3v1n0/rkyIy3BOw Thanks for working on this. Can these patches be submitted as merge requests against the upstream projects? It would be nice to get some feedback from the maintainers, and be able to easily leave some comments there as well. Thanks, Simon ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/9] drm: Add privacy-screen class and connector properties
Hi, On 4/22/21 10:51 AM, Simon Ser wrote: > Hi, > > On Wednesday, April 21st, 2021 at 10:47 PM, Hans de Goede > wrote: > >> There now is GNOME userspace code using the new properties: >> https://hackmd.io/@3v1n0/rkyIy3BOw > > Thanks for working on this. > > Can these patches be submitted as merge requests against the upstream > projects? It would be nice to get some feedback from the maintainers, > and be able to easily leave some comments there as well. I guess Marco was waiting for the kernel bits too land before submitting these, but I agree that it would probably be good to have these submitted now, we can mark them as WIP to avoid them getting merged before the kernel side is finalized. Marco, can you take care of submitting WIP merge-reqs for these? Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/9] drm: Add privacy-screen class and connector properties
On Thursday, April 22nd, 2021 at 10:54 AM, Hans de Goede wrote: > I guess Marco was waiting for the kernel bits too land before submitting > these, > but I agree that it would probably be good to have these submitted now, we > can mark them as WIP to avoid them getting merged before the kernel side > is finalized. Yes, this is how it should be done, see [1]. In particular: > The userspace side must be fully reviewed and tested to the standards > of that userspace project. And yeah, the user-space side still can't be merged before the kernel side: > The kernel patch can only be merged after all the above requirements > are met, but it must be merged to either drm-next or drm-misc-next > before the userspace patches land. [1]: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 03/16] swiotlb: Refactor swiotlb_create_debugfs
Split the debugfs creation to make the code reusable for supporting different bounce buffer pools, e.g. restricted DMA pool. Signed-off-by: Claire Chang --- kernel/dma/swiotlb.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 3f1adee35097..57a9adb920bf 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -660,18 +660,24 @@ EXPORT_SYMBOL_GPL(is_swiotlb_active); #ifdef CONFIG_DEBUG_FS -static int __init swiotlb_create_debugfs(void) +static void swiotlb_create_debugfs(struct io_tlb_mem *mem, const char *name, + struct dentry *node) { - struct io_tlb_mem *mem = io_tlb_default_mem; - if (!mem) - return 0; - mem->debugfs = debugfs_create_dir("swiotlb", NULL); + return; + + mem->debugfs = debugfs_create_dir(name, node); debugfs_create_ulong("io_tlb_nslabs", 0400, mem->debugfs, &mem->nslabs); debugfs_create_ulong("io_tlb_used", 0400, mem->debugfs, &mem->used); +} + +static int __init swiotlb_create_default_debugfs(void) +{ + swiotlb_create_debugfs(io_tlb_default_mem, "swiotlb", NULL); + return 0; } -late_initcall(swiotlb_create_debugfs); +late_initcall(swiotlb_create_default_debugfs); #endif -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 11/16] swiotlb: Refactor swiotlb_tbl_unmap_single
Add a new function, release_slots, to make the code reusable for supporting different bounce buffer pools, e.g. restricted DMA pool. Signed-off-by: Claire Chang --- kernel/dma/swiotlb.c | 35 --- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index b7d634d7a7eb..af0feb8eaead 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -547,27 +547,15 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, return tlb_addr; } -/* - * tlb_addr is the physical address of the bounce buffer to unmap. - */ -void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr, - size_t mapping_size, enum dma_data_direction dir, - unsigned long attrs) +static void release_slots(struct device *dev, phys_addr_t tlb_addr) { - struct io_tlb_mem *mem = get_io_tlb_mem(hwdev); + struct io_tlb_mem *mem = get_io_tlb_mem(dev); unsigned long flags; - unsigned int offset = swiotlb_align_offset(hwdev, tlb_addr); + unsigned int offset = swiotlb_align_offset(dev, tlb_addr); int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT; int nslots = nr_slots(mem->slots[index].alloc_size + offset); int count, i; - /* -* First, sync the memory before unmapping the entry -*/ - if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) && - (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL)) - swiotlb_bounce(hwdev, tlb_addr, mapping_size, DMA_FROM_DEVICE); - /* * Return the buffer to the free list by setting the corresponding * entries to indicate the number of contiguous entries available. @@ -602,6 +590,23 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr, spin_unlock_irqrestore(&mem->lock, flags); } +/* + * tlb_addr is the physical address of the bounce buffer to unmap. + */ +void swiotlb_tbl_unmap_single(struct device *dev, phys_addr_t tlb_addr, + size_t mapping_size, enum dma_data_direction dir, + unsigned long attrs) +{ + /* +* First, sync the memory before unmapping the entry +*/ + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) && + (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL)) + swiotlb_bounce(dev, tlb_addr, mapping_size, DMA_FROM_DEVICE); + + release_slots(dev, tlb_addr); +} + void swiotlb_sync_single_for_device(struct device *dev, phys_addr_t tlb_addr, size_t size, enum dma_data_direction dir) { -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/msm/dp: service only one irq_hpd if there are multiple irq_hpd pending
On 2021-04-21 10:26, khs...@codeaurora.org wrote: On 2021-04-20 15:01, Stephen Boyd wrote: Quoting Kuogee Hsieh (2021-04-16 13:27:57) Some dongle may generate more than one irq_hpd events in a short period of time. This patch will treat those irq_hpd events as single one and service only one irq_hpd event. Why is it bad to get multiple irq_hpd events in a short period of time? Please tell us here in the commit text. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/dp/dp_display.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 5a39da6..0a7d383 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -707,6 +707,9 @@ static int dp_irq_hpd_handle(struct dp_display_private *dp, u32 data) return 0; } + /* only handle first irq_hpd in case of multiple irs_hpd pending */ + dp_del_event(dp, EV_IRQ_HPD_INT); + ret = dp_display_usbpd_attention_cb(&dp->pdev->dev); if (ret == -ECONNRESET) { /* cable unplugged */ dp->core_initialized = false; @@ -1300,6 +1303,9 @@ static int dp_pm_suspend(struct device *dev) /* host_init will be called at pm_resume */ dp->core_initialized = false; + /* system suspended, delete pending irq_hdps */ + dp_del_event(dp, EV_IRQ_HPD_INT); What happens if I suspend my device and when this function is running I toggle my monitor to use the HDMI input that is connected instead of some other input, maybe the second HDMI input? Wouldn't that generate an HPD interrupt to grab the attention of this device? no, At this time display is off. this mean dp controller is off and mainlink has teared down. it will start with plug in interrupt to bring dp controller up and start link training. irq_hpd can be generated only panel is at run time of operation mode and need attention from host. If host is shutting down, then no need to service pending irq_hpd. + mutex_unlock(&dp->event_mutex); return 0; @@ -1496,6 +1502,9 @@ int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder) /* stop sentinel checking */ dp_del_event(dp_display, EV_DISCONNECT_PENDING_TIMEOUT); + /* link is down, delete pending irq_hdps */ + dp_del_event(dp_display, EV_IRQ_HPD_INT); + I'm becoming convinced that the whole kthread design and event queue is broken. These sorts of patches are working around the larger problem that the kthread is running independently of the driver and irqs can come in at any time but the event queue is not checked from the irq handler to debounce the irq event. Is the event queue necessary at all? I wonder if it would be simpler to just use an irq thread and process the hpd signal from there. Then we're guaranteed to not get an irq again until the irq thread is done processing the event. This would naturally debounce the irq hpd event that way. event q just like bottom half of irq handler. it turns irq into event and handle them sequentially. irq_hpd is asynchronous event from panel to bring up attention of hsot during run time of operation. Here, the dongle is unplugged and main link had teared down so that no need to service pending irq_hpd if any. As Kuogee mentioned, IRQ_HPD is a message received from the panel and is not like your typical HW generated IRQ. There is no guarantee that we will not receive an IRQ_HPD until we are finished with processing of an earlier HPD message or an IRQ_HPD message. For example - when you run the protocol compliance, when we get a HPD from the sink, we are expected to start reading DPCD, EDID and proceed with link training. As soon as link training is finished (which is marked by a specific DPCD register write), the sink is going to issue an IRQ_HPD. At this point, we may not done with processing the HPD high as after link training we would typically notify the user mode of the newly connected display, etc. dp_display_disable(dp_display, 0); rc = dp_display_unprepare(dp); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 15/16] dt-bindings: of: Add restricted DMA pool
Introduce the new compatible string, restricted-dma-pool, for restricted DMA. One can specify the address and length of the restricted DMA memory region by restricted-dma-pool in the reserved-memory node. Signed-off-by: Claire Chang --- .../reserved-memory/reserved-memory.txt | 24 +++ 1 file changed, 24 insertions(+) diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt index e8d3096d922c..fc9a12c2f679 100644 --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt @@ -51,6 +51,20 @@ compatible (optional) - standard definition used as a shared pool of DMA buffers for a set of devices. It can be used by an operating system to instantiate the necessary pool management subsystem if necessary. +- restricted-dma-pool: This indicates a region of memory meant to be + used as a pool of restricted DMA buffers for a set of devices. The + memory region would be the only region accessible to those devices. + When using this, the no-map and reusable properties must not be set, + so the operating system can create a virtual mapping that will be used + for synchronization. The main purpose for restricted DMA is to + mitigate the lack of DMA access control on systems without an IOMMU, + which could result in the DMA accessing the system memory at + unexpected times and/or unexpected addresses, possibly leading to data + leakage or corruption. The feature on its own provides a basic level + of protection against the DMA overwriting buffer contents at + unexpected times. However, to protect against general data leakage and + system memory corruption, the system needs to provide way to lock down + the memory access, e.g., MPU. - vendor specific string in the form ,[-] no-map (optional) - empty property - Indicates the operating system must not create a virtual mapping @@ -120,6 +134,11 @@ one for multimedia processing (named multimedia-memory@7700, 64MiB). compatible = "acme,multimedia-memory"; reg = <0x7700 0x400>; }; + + restricted_dma_mem_reserved: restricted_dma_mem_reserved { + compatible = "restricted-dma-pool"; + reg = <0x5000 0x40>; + }; }; /* ... */ @@ -138,4 +157,9 @@ one for multimedia processing (named multimedia-memory@7700, 64MiB). memory-region = <&multimedia_reserved>; /* ... */ }; + + pcie_device: pcie_device@0,0 { + memory-region = <&restricted_dma_mem_reserved>; + /* ... */ + }; }; -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 14/16] dma-direct: Allocate memory from restricted DMA pool if available
The restricted DMA pool is preferred if available. The restricted DMA pools provide a basic level of protection against the DMA overwriting buffer contents at unexpected times. However, to protect against general data leakage and system memory corruption, the system needs to provide a way to lock down the memory access, e.g., MPU. Signed-off-by: Claire Chang --- kernel/dma/direct.c | 35 ++- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index 7a27f0510fcc..29523d2a9845 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -78,6 +78,10 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size) static void __dma_direct_free_pages(struct device *dev, struct page *page, size_t size) { +#ifdef CONFIG_DMA_RESTRICTED_POOL + if (swiotlb_free(dev, page, size)) + return; +#endif dma_free_contiguous(dev, page, size); } @@ -92,7 +96,17 @@ static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size, gfp |= dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask, &phys_limit); - page = dma_alloc_contiguous(dev, size, gfp); + +#ifdef CONFIG_DMA_RESTRICTED_POOL + page = swiotlb_alloc(dev, size); + if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) { + __dma_direct_free_pages(dev, page, size); + page = NULL; + } +#endif + + if (!page) + page = dma_alloc_contiguous(dev, size, gfp); if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) { dma_free_contiguous(dev, page, size); page = NULL; @@ -148,7 +162,7 @@ void *dma_direct_alloc(struct device *dev, size_t size, gfp |= __GFP_NOWARN; if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) && - !force_dma_unencrypted(dev)) { + !force_dma_unencrypted(dev) && !is_dev_swiotlb_force(dev)) { page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO); if (!page) return NULL; @@ -161,8 +175,8 @@ void *dma_direct_alloc(struct device *dev, size_t size, } if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) && - !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && - !dev_is_dma_coherent(dev)) + !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && !dev_is_dma_coherent(dev) && + !is_dev_swiotlb_force(dev)) return arch_dma_alloc(dev, size, dma_handle, gfp, attrs); /* @@ -172,7 +186,9 @@ void *dma_direct_alloc(struct device *dev, size_t size, if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) && !gfpflags_allow_blocking(gfp) && (force_dma_unencrypted(dev) || -(IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && !dev_is_dma_coherent(dev +(IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && + !dev_is_dma_coherent(dev))) && + !is_dev_swiotlb_force(dev)) return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp); /* we always manually zero the memory once we are done */ @@ -253,15 +269,15 @@ void dma_direct_free(struct device *dev, size_t size, unsigned int page_order = get_order(size); if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) && - !force_dma_unencrypted(dev)) { + !force_dma_unencrypted(dev) && !is_dev_swiotlb_force(dev)) { /* cpu_addr is a struct page cookie, not a kernel address */ dma_free_contiguous(dev, cpu_addr, size); return; } if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) && - !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && - !dev_is_dma_coherent(dev)) { + !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && !dev_is_dma_coherent(dev) && + !is_dev_swiotlb_force(dev)) { arch_dma_free(dev, size, cpu_addr, dma_addr, attrs); return; } @@ -289,7 +305,8 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size, void *ret; if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) && - force_dma_unencrypted(dev) && !gfpflags_allow_blocking(gfp)) + force_dma_unencrypted(dev) && !gfpflags_allow_blocking(gfp) && + !is_dev_swiotlb_force(dev)) return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp); page = __dma_direct_alloc_pages(dev, size, gfp); -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 08/16] swiotlb: Update is_swiotlb_active to add a struct device argument
Update is_swiotlb_active to add a struct device argument. This will be useful later to allow for restricted DMA pool. Signed-off-by: Claire Chang --- drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +- drivers/gpu/drm/nouveau/nouveau_ttm.c| 2 +- drivers/pci/xen-pcifront.c | 2 +- include/linux/swiotlb.h | 4 ++-- kernel/dma/direct.c | 2 +- kernel/dma/swiotlb.c | 4 ++-- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c b/drivers/gpu/drm/i915/gem/i915_gem_internal.c index ce6b664b10aa..7d48c433446b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c @@ -42,7 +42,7 @@ static int i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj) max_order = MAX_ORDER; #ifdef CONFIG_SWIOTLB - if (is_swiotlb_active()) { + if (is_swiotlb_active(NULL)) { unsigned int max_segment; max_segment = swiotlb_max_segment(); diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c index e8b506a6685b..2a2ae6d6cf6d 100644 --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c @@ -321,7 +321,7 @@ nouveau_ttm_init(struct nouveau_drm *drm) } #if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86) - need_swiotlb = is_swiotlb_active(); + need_swiotlb = is_swiotlb_active(NULL); #endif ret = ttm_device_init(&drm->ttm.bdev, &nouveau_bo_driver, drm->dev->dev, diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c index b7a8f3a1921f..6d548ce53ce7 100644 --- a/drivers/pci/xen-pcifront.c +++ b/drivers/pci/xen-pcifront.c @@ -693,7 +693,7 @@ static int pcifront_connect_and_init_dma(struct pcifront_device *pdev) spin_unlock(&pcifront_dev_lock); - if (!err && !is_swiotlb_active()) { + if (!err && !is_swiotlb_active(NULL)) { err = pci_xen_swiotlb_init_late(); if (err) dev_err(&pdev->xdev->dev, "Could not setup SWIOTLB!\n"); diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 2a6cca07540b..c530c976d18b 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -123,7 +123,7 @@ static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) void __init swiotlb_exit(void); unsigned int swiotlb_max_segment(void); size_t swiotlb_max_mapping_size(struct device *dev); -bool is_swiotlb_active(void); +bool is_swiotlb_active(struct device *dev); void __init swiotlb_adjust_size(unsigned long size); #else #define swiotlb_force SWIOTLB_NO_FORCE @@ -143,7 +143,7 @@ static inline size_t swiotlb_max_mapping_size(struct device *dev) return SIZE_MAX; } -static inline bool is_swiotlb_active(void) +static inline bool is_swiotlb_active(struct device *dev) { return false; } diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index 84c9feb5474a..7a88c34d0867 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -495,7 +495,7 @@ int dma_direct_supported(struct device *dev, u64 mask) size_t dma_direct_max_mapping_size(struct device *dev) { /* If SWIOTLB is active, use its maximum mapping size */ - if (is_swiotlb_active() && + if (is_swiotlb_active(dev) && (dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE)) return swiotlb_max_mapping_size(dev); return SIZE_MAX; diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index ffbb8724e06c..1d221343f1c8 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -659,9 +659,9 @@ size_t swiotlb_max_mapping_size(struct device *dev) return ((size_t)IO_TLB_SIZE) * IO_TLB_SEGSIZE; } -bool is_swiotlb_active(void) +bool is_swiotlb_active(struct device *dev) { - return io_tlb_default_mem != NULL; + return get_io_tlb_mem(dev) != NULL; } EXPORT_SYMBOL_GPL(is_swiotlb_active); -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 09/16] swiotlb: Bounce data from/to restricted DMA pool if available
Regardless of swiotlb setting, the restricted DMA pool is preferred if available. The restricted DMA pools provide a basic level of protection against the DMA overwriting buffer contents at unexpected times. However, to protect against general data leakage and system memory corruption, the system needs to provide a way to lock down the memory access, e.g., MPU. Note that is_dev_swiotlb_force doesn't check if swiotlb_force == SWIOTLB_FORCE. Otherwise the memory allocation behavior with default swiotlb will be changed by the following patche ("dma-direct: Allocate memory from restricted DMA pool if available"). Signed-off-by: Claire Chang --- include/linux/swiotlb.h | 13 + kernel/dma/direct.h | 3 ++- kernel/dma/swiotlb.c| 8 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index c530c976d18b..0c5a18d9cf89 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -120,6 +120,15 @@ static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) return mem && paddr >= mem->start && paddr < mem->end; } +static inline bool is_dev_swiotlb_force(struct device *dev) +{ +#ifdef CONFIG_DMA_RESTRICTED_POOL + if (dev->dma_io_tlb_mem) + return true; +#endif /* CONFIG_DMA_RESTRICTED_POOL */ + return false; +} + void __init swiotlb_exit(void); unsigned int swiotlb_max_segment(void); size_t swiotlb_max_mapping_size(struct device *dev); @@ -131,6 +140,10 @@ static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) { return false; } +static inline bool is_dev_swiotlb_force(struct device *dev) +{ + return false; +} static inline void swiotlb_exit(void) { } diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h index 13e9e7158d94..f94813674e23 100644 --- a/kernel/dma/direct.h +++ b/kernel/dma/direct.h @@ -87,7 +87,8 @@ static inline dma_addr_t dma_direct_map_page(struct device *dev, phys_addr_t phys = page_to_phys(page) + offset; dma_addr_t dma_addr = phys_to_dma(dev, phys); - if (unlikely(swiotlb_force == SWIOTLB_FORCE)) + if (unlikely(swiotlb_force == SWIOTLB_FORCE) || + is_dev_swiotlb_force(dev)) return swiotlb_map(dev, phys, size, dir, attrs); if (unlikely(!dma_capable(dev, dma_addr, size, true))) { diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 1d221343f1c8..96ff36d8ec53 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -344,7 +344,7 @@ void __init swiotlb_exit(void) static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size, enum dma_data_direction dir) { - struct io_tlb_mem *mem = io_tlb_default_mem; + struct io_tlb_mem *mem = get_io_tlb_mem(dev); int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT; phys_addr_t orig_addr = mem->slots[index].orig_addr; size_t alloc_size = mem->slots[index].alloc_size; @@ -426,7 +426,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index) static int find_slots(struct device *dev, phys_addr_t orig_addr, size_t alloc_size) { - struct io_tlb_mem *mem = io_tlb_default_mem; + struct io_tlb_mem *mem = get_io_tlb_mem(dev); unsigned long boundary_mask = dma_get_seg_boundary(dev); dma_addr_t tbl_dma_addr = phys_to_dma_unencrypted(dev, mem->start) & boundary_mask; @@ -503,7 +503,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, size_t mapping_size, size_t alloc_size, enum dma_data_direction dir, unsigned long attrs) { - struct io_tlb_mem *mem = io_tlb_default_mem; + struct io_tlb_mem *mem = get_io_tlb_mem(dev); unsigned int offset = swiotlb_align_offset(dev, orig_addr); unsigned int i; int index; @@ -554,7 +554,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr, size_t mapping_size, enum dma_data_direction dir, unsigned long attrs) { - struct io_tlb_mem *mem = io_tlb_default_mem; + struct io_tlb_mem *mem = get_io_tlb_mem(hwdev); unsigned long flags; unsigned int offset = swiotlb_align_offset(hwdev, tlb_addr); int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT; -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 02/16] swiotlb: Refactor swiotlb init functions
Add a new function, swiotlb_init_io_tlb_mem, for the io_tlb_mem struct initialization to make the code reusable. Note that we now also call set_memory_decrypted in swiotlb_init_with_tbl. Signed-off-by: Claire Chang --- kernel/dma/swiotlb.c | 51 ++-- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 8635a57f88e9..3f1adee35097 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -166,9 +166,30 @@ void __init swiotlb_update_mem_attributes(void) memset(vaddr, 0, bytes); } -int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) +static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start, + unsigned long nslabs, bool late_alloc) { + void *vaddr = phys_to_virt(start); unsigned long bytes = nslabs << IO_TLB_SHIFT, i; + + mem->nslabs = nslabs; + mem->start = start; + mem->end = mem->start + bytes; + mem->index = 0; + mem->late_alloc = late_alloc; + spin_lock_init(&mem->lock); + for (i = 0; i < mem->nslabs; i++) { + mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i); + mem->slots[i].orig_addr = INVALID_PHYS_ADDR; + mem->slots[i].alloc_size = 0; + } + + set_memory_decrypted((unsigned long)vaddr, bytes >> PAGE_SHIFT); + memset(vaddr, 0, bytes); +} + +int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) +{ struct io_tlb_mem *mem; size_t alloc_size; @@ -184,16 +205,8 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) if (!mem) panic("%s: Failed to allocate %zu bytes align=0x%lx\n", __func__, alloc_size, PAGE_SIZE); - mem->nslabs = nslabs; - mem->start = __pa(tlb); - mem->end = mem->start + bytes; - mem->index = 0; - spin_lock_init(&mem->lock); - for (i = 0; i < mem->nslabs; i++) { - mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i); - mem->slots[i].orig_addr = INVALID_PHYS_ADDR; - mem->slots[i].alloc_size = 0; - } + + swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, false); io_tlb_default_mem = mem; if (verbose) @@ -280,7 +293,6 @@ swiotlb_late_init_with_default_size(size_t default_size) int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) { - unsigned long bytes = nslabs << IO_TLB_SHIFT, i; struct io_tlb_mem *mem; if (swiotlb_force == SWIOTLB_NO_FORCE) @@ -295,20 +307,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) if (!mem) return -ENOMEM; - mem->nslabs = nslabs; - mem->start = virt_to_phys(tlb); - mem->end = mem->start + bytes; - mem->index = 0; - mem->late_alloc = 1; - spin_lock_init(&mem->lock); - for (i = 0; i < mem->nslabs; i++) { - mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i); - mem->slots[i].orig_addr = INVALID_PHYS_ADDR; - mem->slots[i].alloc_size = 0; - } - - set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT); - memset(tlb, 0, bytes); + swiotlb_init_io_tlb_mem(mem, virt_to_phys(tlb), nslabs, true); io_tlb_default_mem = mem; swiotlb_print_info(); -- I'm not 100% sure if set_memory_decrypted is safe to call in swiotlb_init_with_tbl, but I didn't hit any issue booting with this. 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 05/16] swiotlb: Add restricted DMA pool initialization
Add the initialization function to create restricted DMA pools from matching reserved-memory nodes. Signed-off-by: Claire Chang --- include/linux/device.h | 4 +++ include/linux/swiotlb.h | 3 +- kernel/dma/swiotlb.c| 80 + 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/include/linux/device.h b/include/linux/device.h index 38a2071cf776..4987608ea4ff 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -416,6 +416,7 @@ struct dev_links_info { * @dma_pools: Dma pools (if dma'ble device). * @dma_mem: Internal for coherent mem override. * @cma_area: Contiguous memory area for dma allocations + * @dma_io_tlb_mem: Internal for swiotlb io_tlb_mem override. * @archdata: For arch-specific additions. * @of_node: Associated device tree node. * @fwnode:Associated device node supplied by platform firmware. @@ -521,6 +522,9 @@ struct device { #ifdef CONFIG_DMA_CMA struct cma *cma_area; /* contiguous memory area for dma allocations */ +#endif +#ifdef CONFIG_DMA_RESTRICTED_POOL + struct io_tlb_mem *dma_io_tlb_mem; #endif /* arch specific additions */ struct dev_archdata archdata; diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 216854a5e513..03ad6e3b4056 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -72,7 +72,8 @@ extern enum swiotlb_force swiotlb_force; * range check to see if the memory was in fact allocated by this * API. * @nslabs:The number of IO TLB blocks (in groups of 64) between @start and - * @end. This is command line adjustable via setup_io_tlb_npages. + * @end. For default swiotlb, this is command line adjustable via + * setup_io_tlb_npages. * @used: The number of used IO TLB block. * @list: The free list describing the number of free entries available * from each index. diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 57a9adb920bf..ffbb8724e06c 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -39,6 +39,13 @@ #ifdef CONFIG_DEBUG_FS #include #endif +#ifdef CONFIG_DMA_RESTRICTED_POOL +#include +#include +#include +#include +#include +#endif #include #include @@ -681,3 +688,76 @@ static int __init swiotlb_create_default_debugfs(void) late_initcall(swiotlb_create_default_debugfs); #endif + +#ifdef CONFIG_DMA_RESTRICTED_POOL +static int rmem_swiotlb_device_init(struct reserved_mem *rmem, + struct device *dev) +{ + struct io_tlb_mem *mem = rmem->priv; + unsigned long nslabs = rmem->size >> IO_TLB_SHIFT; + + if (dev->dma_io_tlb_mem) + return 0; + + /* Since multiple devices can share the same pool, the private data, +* io_tlb_mem struct, will be initialized by the first device attached +* to it. +*/ + if (!mem) { + mem = kzalloc(struct_size(mem, slots, nslabs), GFP_KERNEL); + if (!mem) + return -ENOMEM; +#ifdef CONFIG_ARM + if (!PageHighMem(pfn_to_page(PHYS_PFN(rmem->base { + kfree(mem); + return -EINVAL; + } +#endif /* CONFIG_ARM */ + swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false); + + rmem->priv = mem; + } + +#ifdef CONFIG_DEBUG_FS + if (!io_tlb_default_mem->debugfs) + io_tlb_default_mem->debugfs = + debugfs_create_dir("swiotlb", NULL); + + swiotlb_create_debugfs(mem, rmem->name, io_tlb_default_mem->debugfs); +#endif /* CONFIG_DEBUG_FS */ + + dev->dma_io_tlb_mem = mem; + + return 0; +} + +static void rmem_swiotlb_device_release(struct reserved_mem *rmem, + struct device *dev) +{ + if (dev) + dev->dma_io_tlb_mem = NULL; +} + +static const struct reserved_mem_ops rmem_swiotlb_ops = { + .device_init = rmem_swiotlb_device_init, + .device_release = rmem_swiotlb_device_release, +}; + +static int __init rmem_swiotlb_setup(struct reserved_mem *rmem) +{ + unsigned long node = rmem->fdt_node; + + if (of_get_flat_dt_prop(node, "reusable", NULL) || + of_get_flat_dt_prop(node, "linux,cma-default", NULL) || + of_get_flat_dt_prop(node, "linux,dma-default", NULL) || + of_get_flat_dt_prop(node, "no-map", NULL)) + return -EINVAL; + + rmem->ops = &rmem_swiotlb_ops; + pr_info("Reserved memory: created device swiotlb memory pool at %pa, size %ld MiB\n", + &rmem->base, (unsigned long)rmem->size / SZ_1M); + return 0; +} + +RESERVEDMEM_OF_DECLARE(dma, "restricted-dma-pool", rmem_swiotlb_setup); +#endif /* CONFIG_DMA_RESTRICTED_POOL */ -- 2.31.1.368.gbe11c130af-goog ___
[PATCH v5 01/16] swiotlb: Fix the type of index
Fix the type of index from unsigned int to int since find_slots() might return -1. Fixes: 0774983bc923 ("swiotlb: refactor swiotlb_tbl_map_single") Signed-off-by: Claire Chang --- kernel/dma/swiotlb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 0a5b6f7e75bc..8635a57f88e9 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -499,7 +499,8 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, { struct io_tlb_mem *mem = io_tlb_default_mem; unsigned int offset = swiotlb_align_offset(dev, orig_addr); - unsigned int index, i; + unsigned int i; + int index; phys_addr_t tlb_addr; if (!mem) -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 10/16] swiotlb: Move alloc_size to find_slots
Move the maintenance of alloc_size to find_slots for better code reusability later. Signed-off-by: Claire Chang --- kernel/dma/swiotlb.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 96ff36d8ec53..b7d634d7a7eb 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -479,8 +479,11 @@ static int find_slots(struct device *dev, phys_addr_t orig_addr, return -1; found: - for (i = index; i < index + nslots; i++) + for (i = index; i < index + nslots; i++) { mem->slots[i].list = 0; + mem->slots[i].alloc_size = + alloc_size - ((i - index) << IO_TLB_SHIFT); + } for (i = index - 1; io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 && mem->slots[i].list; i--) @@ -535,11 +538,8 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, * This is needed when we sync the memory. Then we sync the buffer if * needed. */ - for (i = 0; i < nr_slots(alloc_size + offset); i++) { + for (i = 0; i < nr_slots(alloc_size + offset); i++) mem->slots[index + i].orig_addr = slot_addr(orig_addr, i); - mem->slots[index + i].alloc_size = - alloc_size - (i << IO_TLB_SHIFT); - } tlb_addr = slot_addr(mem->start, index) + offset; if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) && (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL)) -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 04/16] swiotlb: Add DMA_RESTRICTED_POOL
Add a new kconfig symbol, DMA_RESTRICTED_POOL, for restricted DMA pool. Signed-off-by: Claire Chang --- kernel/dma/Kconfig | 14 ++ 1 file changed, 14 insertions(+) diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig index 77b405508743..3e961dc39634 100644 --- a/kernel/dma/Kconfig +++ b/kernel/dma/Kconfig @@ -80,6 +80,20 @@ config SWIOTLB bool select NEED_DMA_MAP_STATE +config DMA_RESTRICTED_POOL + bool "DMA Restricted Pool" + depends on OF && OF_RESERVED_MEM + select SWIOTLB + help + This enables support for restricted DMA pools which provide a level of + DMA memory protection on systems with limited hardware protection + capabilities, such as those lacking an IOMMU. + + For more information see + + and . + If unsure, say "n". + # # Should be selected if we can mmap non-coherent mappings to userspace. # The only thing that is really required is a way to set an uncached bit -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 13/16] swiotlb: Add restricted DMA alloc/free support.
Add the functions, swiotlb_{alloc,free} to support the memory allocation from restricted DMA pool. Signed-off-by: Claire Chang --- include/linux/swiotlb.h | 4 kernel/dma/swiotlb.c| 35 +-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 0c5a18d9cf89..e8cf49bd90c5 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -134,6 +134,10 @@ unsigned int swiotlb_max_segment(void); size_t swiotlb_max_mapping_size(struct device *dev); bool is_swiotlb_active(struct device *dev); void __init swiotlb_adjust_size(unsigned long size); +#ifdef CONFIG_DMA_RESTRICTED_POOL +struct page *swiotlb_alloc(struct device *dev, size_t size); +bool swiotlb_free(struct device *dev, struct page *page, size_t size); +#endif /* CONFIG_DMA_RESTRICTED_POOL */ #else #define swiotlb_force SWIOTLB_NO_FORCE static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index af0feb8eaead..274272c79080 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -454,8 +454,9 @@ static int find_slots(struct device *dev, phys_addr_t orig_addr, index = wrap = wrap_index(mem, ALIGN(mem->index, stride)); do { - if ((slot_addr(tbl_dma_addr, index) & iotlb_align_mask) != - (orig_addr & iotlb_align_mask)) { + if (orig_addr && + (slot_addr(tbl_dma_addr, index) & iotlb_align_mask) != + (orig_addr & iotlb_align_mask)) { index = wrap_index(mem, index + 1); continue; } @@ -695,6 +696,36 @@ late_initcall(swiotlb_create_default_debugfs); #endif #ifdef CONFIG_DMA_RESTRICTED_POOL +struct page *swiotlb_alloc(struct device *dev, size_t size) +{ + struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + phys_addr_t tlb_addr; + int index; + + if (!mem) + return NULL; + + index = find_slots(dev, 0, size); + if (index == -1) + return NULL; + + tlb_addr = slot_addr(mem->start, index); + + return pfn_to_page(PFN_DOWN(tlb_addr)); +} + +bool swiotlb_free(struct device *dev, struct page *page, size_t size) +{ + phys_addr_t tlb_addr = page_to_phys(page); + + if (!is_swiotlb_buffer(dev, tlb_addr)) + return false; + + release_slots(dev, tlb_addr); + + return true; +} + static int rmem_swiotlb_device_init(struct reserved_mem *rmem, struct device *dev) { -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 07/16] swiotlb: Update is_swiotlb_buffer to add a struct device argument
Update is_swiotlb_buffer to add a struct device argument. This will be useful later to allow for restricted DMA pool. Signed-off-by: Claire Chang --- drivers/iommu/dma-iommu.c | 12 ++-- drivers/xen/swiotlb-xen.c | 2 +- include/linux/swiotlb.h | 6 +++--- kernel/dma/direct.c | 6 +++--- kernel/dma/direct.h | 6 +++--- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 7bcdd1205535..a5df35bfd150 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -504,7 +504,7 @@ static void __iommu_dma_unmap_swiotlb(struct device *dev, dma_addr_t dma_addr, __iommu_dma_unmap(dev, dma_addr, size); - if (unlikely(is_swiotlb_buffer(phys))) + if (unlikely(is_swiotlb_buffer(dev, phys))) swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs); } @@ -575,7 +575,7 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys, } iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask); - if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys)) + if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(dev, phys)) swiotlb_tbl_unmap_single(dev, phys, org_size, dir, attrs); return iova; } @@ -781,7 +781,7 @@ static void iommu_dma_sync_single_for_cpu(struct device *dev, if (!dev_is_dma_coherent(dev)) arch_sync_dma_for_cpu(phys, size, dir); - if (is_swiotlb_buffer(phys)) + if (is_swiotlb_buffer(dev, phys)) swiotlb_sync_single_for_cpu(dev, phys, size, dir); } @@ -794,7 +794,7 @@ static void iommu_dma_sync_single_for_device(struct device *dev, return; phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle); - if (is_swiotlb_buffer(phys)) + if (is_swiotlb_buffer(dev, phys)) swiotlb_sync_single_for_device(dev, phys, size, dir); if (!dev_is_dma_coherent(dev)) @@ -815,7 +815,7 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev, if (!dev_is_dma_coherent(dev)) arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir); - if (is_swiotlb_buffer(sg_phys(sg))) + if (is_swiotlb_buffer(dev, sg_phys(sg))) swiotlb_sync_single_for_cpu(dev, sg_phys(sg), sg->length, dir); } @@ -832,7 +832,7 @@ static void iommu_dma_sync_sg_for_device(struct device *dev, return; for_each_sg(sgl, sg, nelems, i) { - if (is_swiotlb_buffer(sg_phys(sg))) + if (is_swiotlb_buffer(dev, sg_phys(sg))) swiotlb_sync_single_for_device(dev, sg_phys(sg), sg->length, dir); diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 4c89afc0df62..0c6ed09f8513 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -100,7 +100,7 @@ static int is_xen_swiotlb_buffer(struct device *dev, dma_addr_t dma_addr) * in our domain. Therefore _only_ check address within our domain. */ if (pfn_valid(PFN_DOWN(paddr))) - return is_swiotlb_buffer(paddr); + return is_swiotlb_buffer(dev, paddr); return 0; } diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index b469f04cca26..2a6cca07540b 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -113,9 +113,9 @@ static inline struct io_tlb_mem *get_io_tlb_mem(struct device *dev) return io_tlb_default_mem; } -static inline bool is_swiotlb_buffer(phys_addr_t paddr) +static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) { - struct io_tlb_mem *mem = io_tlb_default_mem; + struct io_tlb_mem *mem = get_io_tlb_mem(dev); return mem && paddr >= mem->start && paddr < mem->end; } @@ -127,7 +127,7 @@ bool is_swiotlb_active(void); void __init swiotlb_adjust_size(unsigned long size); #else #define swiotlb_force SWIOTLB_NO_FORCE -static inline bool is_swiotlb_buffer(phys_addr_t paddr) +static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) { return false; } diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index f737e3347059..84c9feb5474a 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -343,7 +343,7 @@ void dma_direct_sync_sg_for_device(struct device *dev, for_each_sg(sgl, sg, nents, i) { phys_addr_t paddr = dma_to_phys(dev, sg_dma_address(sg)); - if (unlikely(is_swiotlb_buffer(paddr))) + if (unlikely(is_swiotlb_buffer(dev, paddr))) swiotlb_sync_single_for_device(dev, paddr, sg->length, dir); @@ -369,7 +369,7 @@ void dma_direct_sync_sg_for_cpu(struct device
[PATCH v5 00/16] Restricted DMA
This series implements mitigations for lack of DMA access control on systems without an IOMMU, which could result in the DMA accessing the system memory at unexpected times and/or unexpected addresses, possibly leading to data leakage or corruption. For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus is not behind an IOMMU. As PCI-e, by design, gives the device full access to system memory, a vulnerability in the Wi-Fi firmware could easily escalate to a full system exploit (remote wifi exploits: [1a], [1b] that shows a full chain of exploits; [2], [3]). To mitigate the security concerns, we introduce restricted DMA. Restricted DMA utilizes the existing swiotlb to bounce streaming DMA in and out of a specially allocated region and does memory allocation from the same region. The feature on its own provides a basic level of protection against the DMA overwriting buffer contents at unexpected times. However, to protect against general data leakage and system memory corruption, the system needs to provide a way to restrict the DMA to a predefined memory region (this is usually done at firmware level, e.g. MPU in ATF on some ARM platforms [4]). [1a] https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html [1b] https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html [2] https://blade.tencent.com/en/advisories/qualpwn/ [3] https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/ [4] https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8183/drivers/emi_mpu/emi_mpu.c#L132 v5: Rebase on latest linux-next v4: - Fix spinlock bad magic - Use rmem->name for debugfs entry - Address the comments in v3 v3: Using only one reserved memory region for both streaming DMA and memory allocation. https://lore.kernel.org/patchwork/cover/1360992/ v2: Building on top of swiotlb. https://lore.kernel.org/patchwork/cover/1280705/ v1: Using dma_map_ops. https://lore.kernel.org/patchwork/cover/1271660/ Claire Chang (16): swiotlb: Fix the type of index swiotlb: Refactor swiotlb init functions swiotlb: Refactor swiotlb_create_debugfs swiotlb: Add DMA_RESTRICTED_POOL swiotlb: Add restricted DMA pool initialization swiotlb: Add a new get_io_tlb_mem getter swiotlb: Update is_swiotlb_buffer to add a struct device argument swiotlb: Update is_swiotlb_active to add a struct device argument swiotlb: Bounce data from/to restricted DMA pool if available swiotlb: Move alloc_size to find_slots swiotlb: Refactor swiotlb_tbl_unmap_single dma-direct: Add a new wrapper __dma_direct_free_pages() swiotlb: Add restricted DMA alloc/free support. dma-direct: Allocate memory from restricted DMA pool if available dt-bindings: of: Add restricted DMA pool of: Add plumbing for restricted DMA pool .../reserved-memory/reserved-memory.txt | 24 ++ drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +- drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 +- drivers/iommu/dma-iommu.c | 12 +- drivers/of/address.c | 25 ++ drivers/of/device.c | 3 + drivers/of/of_private.h | 5 + drivers/pci/xen-pcifront.c| 2 +- drivers/xen/swiotlb-xen.c | 2 +- include/linux/device.h| 4 + include/linux/swiotlb.h | 41 ++- kernel/dma/Kconfig| 14 + kernel/dma/direct.c | 57 +++-- kernel/dma/direct.h | 9 +- kernel/dma/swiotlb.c | 242 +- 15 files changed, 347 insertions(+), 97 deletions(-) -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 12/16] dma-direct: Add a new wrapper __dma_direct_free_pages()
Add a new wrapper __dma_direct_free_pages() that will be useful later for swiotlb_free(). Signed-off-by: Claire Chang --- kernel/dma/direct.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index 7a88c34d0867..7a27f0510fcc 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -75,6 +75,12 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size) min_not_zero(dev->coherent_dma_mask, dev->bus_dma_limit); } +static void __dma_direct_free_pages(struct device *dev, struct page *page, + size_t size) +{ + dma_free_contiguous(dev, page, size); +} + static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size, gfp_t gfp) { @@ -237,7 +243,7 @@ void *dma_direct_alloc(struct device *dev, size_t size, return NULL; } out_free_pages: - dma_free_contiguous(dev, page, size); + __dma_direct_free_pages(dev, page, size); return NULL; } @@ -273,7 +279,7 @@ void dma_direct_free(struct device *dev, size_t size, else if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED)) arch_dma_clear_uncached(cpu_addr, size); - dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size); + __dma_direct_free_pages(dev, dma_direct_to_page(dev, dma_addr), size); } struct page *dma_direct_alloc_pages(struct device *dev, size_t size, @@ -310,7 +316,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size, *dma_handle = phys_to_dma_direct(dev, page_to_phys(page)); return page; out_free_pages: - dma_free_contiguous(dev, page, size); + __dma_direct_free_pages(dev, page, size); return NULL; } @@ -329,7 +335,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, if (force_dma_unencrypted(dev)) set_memory_encrypted((unsigned long)vaddr, 1 << page_order); - dma_free_contiguous(dev, page, size); + __dma_direct_free_pages(dev, page, size); } #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \ -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 06/16] swiotlb: Add a new get_io_tlb_mem getter
Add a new getter, get_io_tlb_mem, to help select the io_tlb_mem struct. The restricted DMA pool is preferred if available. Signed-off-by: Claire Chang --- include/linux/swiotlb.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 03ad6e3b4056..b469f04cca26 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -2,6 +2,7 @@ #ifndef __LINUX_SWIOTLB_H #define __LINUX_SWIOTLB_H +#include #include #include #include @@ -102,6 +103,16 @@ struct io_tlb_mem { }; extern struct io_tlb_mem *io_tlb_default_mem; +static inline struct io_tlb_mem *get_io_tlb_mem(struct device *dev) +{ +#ifdef CONFIG_DMA_RESTRICTED_POOL + if (dev && dev->dma_io_tlb_mem) + return dev->dma_io_tlb_mem; +#endif /* CONFIG_DMA_RESTRICTED_POOL */ + + return io_tlb_default_mem; +} + static inline bool is_swiotlb_buffer(phys_addr_t paddr) { struct io_tlb_mem *mem = io_tlb_default_mem; -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 16/16] of: Add plumbing for restricted DMA pool
If a device is not behind an IOMMU, we look up the device node and set up the restricted DMA when the restricted-dma-pool is presented. Signed-off-by: Claire Chang --- drivers/of/address.c| 25 + drivers/of/device.c | 3 +++ drivers/of/of_private.h | 5 + 3 files changed, 33 insertions(+) diff --git a/drivers/of/address.c b/drivers/of/address.c index 54f221dde267..fff3adfe4986 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -1109,6 +1110,30 @@ bool of_dma_is_coherent(struct device_node *np) } EXPORT_SYMBOL_GPL(of_dma_is_coherent); +int of_dma_set_restricted_buffer(struct device *dev) +{ + struct device_node *node; + int count, i; + + if (!dev->of_node) + return 0; + + count = of_property_count_elems_of_size(dev->of_node, "memory-region", + sizeof(phandle)); + for (i = 0; i < count; i++) { + node = of_parse_phandle(dev->of_node, "memory-region", i); + /* There might be multiple memory regions, but only one +* restriced-dma-pool region is allowed. +*/ + if (of_device_is_compatible(node, "restricted-dma-pool") && + of_device_is_available(node)) + return of_reserved_mem_device_init_by_idx( + dev, dev->of_node, i); + } + + return 0; +} + /** * of_mmio_is_nonposted - Check if device uses non-posted MMIO * @np:device node diff --git a/drivers/of/device.c b/drivers/of/device.c index c5a9473a5fb1..d8d865223e51 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -165,6 +165,9 @@ int of_dma_configure_id(struct device *dev, struct device_node *np, arch_setup_dma_ops(dev, dma_start, size, iommu, coherent); + if (!iommu) + return of_dma_set_restricted_buffer(dev); + return 0; } EXPORT_SYMBOL_GPL(of_dma_configure_id); diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index d717efbd637d..e9237f5eff48 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -163,12 +163,17 @@ struct bus_dma_region; #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_HAS_DMA) int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map); +int of_dma_set_restricted_buffer(struct device *dev); #else static inline int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map) { return -ENODEV; } +static inline int of_dma_get_restricted_buffer(struct device *dev) +{ + return -ENODEV; +} #endif #endif /* _LINUX_OF_PRIVATE_H */ -- 2.31.1.368.gbe11c130af-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/ttm: fix error handling if no BO can be swapped out
Am 22.04.21 um 10:37 schrieb Shiwu Zhang: Signed-off-by: Shiwu Zhang --- drivers/gpu/drm/ttm/ttm_device.c | 2 +- drivers/gpu/drm/ttm/ttm_tt.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 1f2024164d72..5dc92b056f0a 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -112,7 +112,7 @@ int ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { struct ttm_global *glob = &ttm_glob; struct ttm_device *bdev; - int ret = -EBUSY; + int ret = 0; Please don't initialize the return value if we don't need that. Apart from that looks good to me now. Thanks, Christian. mutex_lock(&ttm_global_mutex); list_for_each_entry(bdev, &glob->device_list, device_list) { diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 48c407cff112..539e0232cb3b 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -329,6 +329,8 @@ int ttm_tt_populate(struct ttm_device *bdev, ttm_dma32_pages_limit) { ret = ttm_global_swapout(ctx, GFP_KERNEL); + if (ret == 0) + break; if (ret < 0) goto error; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/bridge: nwl-dsi: Get MIPI DSI controller and PHY ready in ->mode_set()
Hi Neil, On Thu, 2021-04-22 at 10:48 +0200, Neil Armstrong wrote: > Hi, > > On 22/04/2021 07:14, Liu Ying wrote: > > Some MIPI DSI panel drivers like 'raydium,rm68200' send > > MIPI_DCS_SET_DISPLAY_ON commands in panel_funcs->prepare(), which > > requires the MIPI DSI controller and PHY to be ready beforehand. > > Without this patch, the nwl-dsi driver gets the MIPI DSI controller > > and PHY ready in bridge_funcs->pre_enable(), which happens after > > the panel_funcs->prepare(). So, this patch shifts the bridge operation > > ealier from bridge_funcs->pre_enable() to bridge_funcs->mode_set(). > > This makes sense, this is how we do on the synopsys dw mipi dsi driver. > > > This way, more MIPI DSI panels can connect to this nwl-dsi bridge. > > Care is taken to make sure bridge_funcs->mode_set()/atomic_disable() > > are called in pairs, which includes removing a check on unchanged HS > > clock rate and forcing a full modeset when only connector's DPMS is > > brought out of "Off" status. > > I would split the changes in multiple patches to clarify the changes. Maybe, I can split this into the below 3 patches: 1) Remove a check on unchanged HS clock rate from ->mode_set(). 2) Force a full modeset when only connector's DPMS is brought out of "Off" status. This will be done in ->atomic_check() together with the fixup for H/VSYNC polarities. 3) Shift the bridge operation as the last step. I'll mention that 1) & 2) are needed by 3) in their commit message. Does this split-up sound reasonable? Thanks, Liu Ying > > Neil > > > Cc: Andrzej Hajda > > Cc: Neil Armstrong > > Cc: Robert Foss > > Cc: Laurent Pinchart > > Cc: Jonas Karlman > > Cc: Jernej Skrabec > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Guido Günther > > Cc: Robert Chiras > > Cc: NXP Linux Team > > Signed-off-by: Liu Ying > > --- > > v1->v2: > > * Fix commit message typo - s/unchange/unchanged/ > > > > drivers/gpu/drm/bridge/nwl-dsi.c | 86 > > +--- > > 1 file changed, 46 insertions(+), 40 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c > > b/drivers/gpu/drm/bridge/nwl-dsi.c > > index be6bfc5..229f0b1 100644 > > --- a/drivers/gpu/drm/bridge/nwl-dsi.c > > +++ b/drivers/gpu/drm/bridge/nwl-dsi.c > > @@ -21,6 +21,7 @@ > > #include > > #include > > > > +#include > > #include > > #include > > #include > > @@ -661,7 +662,7 @@ static irqreturn_t nwl_dsi_irq_handler(int irq, void > > *data) > > return IRQ_HANDLED; > > } > > > > -static int nwl_dsi_enable(struct nwl_dsi *dsi) > > +static int nwl_dsi_mode_set(struct nwl_dsi *dsi) > > { > > struct device *dev = dsi->dev; > > union phy_configure_opts *phy_cfg = &dsi->phy_cfg; > > @@ -748,7 +749,9 @@ static int nwl_dsi_disable(struct nwl_dsi *dsi) > > return 0; > > } > > > > -static void nwl_dsi_bridge_disable(struct drm_bridge *bridge) > > +static void > > +nwl_dsi_bridge_atomic_disable(struct drm_bridge *bridge, > > + struct drm_bridge_state *old_bridge_state) > > { > > struct nwl_dsi *dsi = bridge_to_dsi(bridge); > > int ret; > > @@ -809,17 +812,6 @@ static int nwl_dsi_get_dphy_params(struct nwl_dsi *dsi, > > return 0; > > } > > > > -static bool nwl_dsi_bridge_mode_fixup(struct drm_bridge *bridge, > > - const struct drm_display_mode *mode, > > - struct drm_display_mode *adjusted_mode) > > -{ > > - /* At least LCDIF + NWL needs active high sync */ > > - adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); > > - adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC); > > - > > - return true; > > -} > > - > > static enum drm_mode_status > > nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge, > > const struct drm_display_info *info, > > @@ -837,6 +829,29 @@ nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge, > > return MODE_OK; > > } > > > > +static int nwl_dsi_bridge_atomic_check(struct drm_bridge *bridge, > > + struct drm_bridge_state *bridge_state, > > + struct drm_crtc_state *crtc_state, > > + struct drm_connector_state *conn_state) > > +{ > > + struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; > > + > > + /* At least LCDIF + NWL needs active high sync */ > > + adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); > > + adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC); > > + > > + /* > > +* Do a full modeset if crtc_state->active is changed to be true. > > +* This ensures our ->mode_set() is called to get the DSI controller > > +* and the PHY ready to send DCS commands, when only the connector's > > +* DPMS is brought out of "Off" status. > > +*/ > > + if (crtc_state->active_changed && crtc_state->active) > > + crtc_sta
Re: [PATCH v3 1/2] drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_lock_all() with drm_modeset_lock_all_ctx()
On Wed, Apr 21, 2021 at 01:37:15PM +0200, Fabio M. De Francesco wrote: > Replace the deprecated API with new helpers, according to the > TODO list of the DRM subsystem. > > In this first patch drm_modeset_lock_all() is replaced with > drm_modeset_lock_all_ctx(). Unlike drm_modeset_lock_all(), > the latter doesn’t take the global drm_mode_config.mutex > since that lock isn’t required for modeset state changes. So this is only true for core modeset code, not necessarily for drivers. So needs to be audited in each case. Now loocking at the precise code you're touching here the situation is a lot more specific: - We are looping over all the crtc. This list never changes, so no locks needed. - Then we look at crtc->state, which is proteced by crtc->mutex. So that's the only lock we need, and we only need to hold a single one (so no EDEADLCK retry loop needed due to multiple lock acquisition). So I think the right fix here is to just grab the crtc->mutex lock right around the access to crtc->state with drm_modeset_lock(). And ditch the lock_all and all its complexity completely. tldr; locking is complicated :-) Can you pls respin? Thanks, Daniel > Signed-off-by: Fabio M. De Francesco > --- > > Changes from v2: The work is split in two consecutive logical steps. > Changes from v1: Added further information to the commit message. > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 13 ++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index 671ec1002230..856903db34c5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -1438,8 +1438,15 @@ static int amdgpu_pmops_runtime_idle(struct device > *dev) > > if (amdgpu_device_has_dc_support(adev)) { > struct drm_crtc *crtc; > - > - drm_modeset_lock_all(drm_dev); > + struct drm_modeset_acquire_ctx ctx; > + int ret_lock = 0; > + > +retry: > + drm_modeset_lock_all_ctx(drm_dev, &ctx); > + if(ret_lock == -EDEADLK) { > + drm_modeset_backoff(&ctx); > + goto retry; > + } > > drm_for_each_crtc(crtc, drm_dev) { > if (crtc->state->active) { > @@ -1448,7 +1455,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev) > } > } > > - drm_modeset_unlock_all(drm_dev); > + drm_modeset_drop_locks(&ctx); > > } else { > struct drm_connector *list_connector; > -- > 2.31.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Rewrite CL/CTG L-shaped memory detection
On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Currently we try to detect a symmetric memory configurations > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > either only set on a very specific subset of machines or it > just does not exist (it's not mentioned in any public chipset > datasheets I've found). As it happens my CL/CTG machines never > set said bit, even if I populate the channels with identical > sticks. > > So let's do the L-shaped memory detection the same way as the > desktop variants, ie. just look at the DRAM rank boundary > registers to see if both channels have an identical size. > > With this my CL/CTG no longer claim L-shaped memory when I use > identical sticks. Also tested with non-matching sticks just to > make sure the L-shaped memory is still properly detected. > > And for completeness let's update the debugfs code to dump > the correct set of registers on each platform. > > Cc: Chris Wilson > Signed-off-by: Ville Syrjälä Did you check this with the swapping igt? I have some vague memories of bug reports where somehow the machine was acting like it's L-shaped memory despite that banks were populated equally. I've iirc tried all kinds of tricks to figure it out, all to absolutely no avail. tbh I'd just not touch this, not really worth it. -Daniel > --- > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 15 --- > drivers/gpu/drm/i915/i915_debugfs.c | 16 > drivers/gpu/drm/i915/i915_reg.h | 4 > 3 files changed, 24 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > index 0fa6c38893f7..754f20768de5 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > @@ -693,14 +693,15 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt) > swizzle_x = I915_BIT_6_SWIZZLE_9_10_17; > swizzle_y = I915_BIT_6_SWIZZLE_9_17; > } > - break; > - } > > - /* check for L-shaped memory aka modified enhanced addressing */ > - if (IS_GEN(i915, 4) && > - !(intel_uncore_read(uncore, DCC2) & > DCC2_MODIFIED_ENHANCED_DISABLE)) { > - swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > - swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > + /* check for L-shaped memory aka modified enhanced > addressing */ > + if (IS_GEN(i915, 4) && > + intel_uncore_read16(uncore, C0DRB3_CL) != > + intel_uncore_read16(uncore, C1DRB3_CL)) { > + swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > + swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > + } > + break; > } > > if (dcc == 0x) { > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 8dd374691102..6de11ffcde38 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -621,10 +621,18 @@ static int i915_swizzle_info(struct seq_file *m, void > *data) > intel_uncore_read(uncore, DCC)); > seq_printf(m, "DDC2 = 0x%08x\n", > intel_uncore_read(uncore, DCC2)); > - seq_printf(m, "C0DRB3 = 0x%04x\n", > -intel_uncore_read16(uncore, C0DRB3_BW)); > - seq_printf(m, "C1DRB3 = 0x%04x\n", > -intel_uncore_read16(uncore, C1DRB3_BW)); > + > + if (IS_G45(dev_priv) || IS_I965G(dev_priv) || IS_G33(dev_priv)) > { > + seq_printf(m, "C0DRB3 = 0x%04x\n", > +intel_uncore_read16(uncore, C0DRB3_BW)); > + seq_printf(m, "C1DRB3 = 0x%04x\n", > +intel_uncore_read16(uncore, C1DRB3_BW)); > + } else if (IS_GEN(dev_priv, 4)) { > + seq_printf(m, "C0DRB3 = 0x%04x\n", > +intel_uncore_read16(uncore, C0DRB3_CL)); > + seq_printf(m, "C1DRB3 = 0x%04x\n", > +intel_uncore_read16(uncore, C1DRB3_CL)); > + } > } else if (INTEL_GEN(dev_priv) >= 6) { > seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n", > intel_uncore_read(uncore, MAD_DIMM_C0)); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 0587b2455ea1..055c258179a1 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3790,6 +3790,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define C0DRB3_BW_MMIO(MCHBAR_MIRROR_BASE + 0x206)
Re: [PATCH v2] drm/ttm: fix error handling if no BO can be swapped out
On Thu, Apr 22, 2021 at 04:37:49PM +0800, Shiwu Zhang wrote: > Signed-off-by: Shiwu Zhang Cc: sta...@vger.kernel.org or at least Fixes: tag? Zero length commit message for something that claims to be a bugfix is probably too short :-) -Daniel > --- > drivers/gpu/drm/ttm/ttm_device.c | 2 +- > drivers/gpu/drm/ttm/ttm_tt.c | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_device.c > b/drivers/gpu/drm/ttm/ttm_device.c > index 1f2024164d72..5dc92b056f0a 100644 > --- a/drivers/gpu/drm/ttm/ttm_device.c > +++ b/drivers/gpu/drm/ttm/ttm_device.c > @@ -112,7 +112,7 @@ int ttm_global_swapout(struct ttm_operation_ctx *ctx, > gfp_t gfp_flags) > { > struct ttm_global *glob = &ttm_glob; > struct ttm_device *bdev; > - int ret = -EBUSY; > + int ret = 0; > > mutex_lock(&ttm_global_mutex); > list_for_each_entry(bdev, &glob->device_list, device_list) { > diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c > index 48c407cff112..539e0232cb3b 100644 > --- a/drivers/gpu/drm/ttm/ttm_tt.c > +++ b/drivers/gpu/drm/ttm/ttm_tt.c > @@ -329,6 +329,8 @@ int ttm_tt_populate(struct ttm_device *bdev, > ttm_dma32_pages_limit) { > > ret = ttm_global_swapout(ctx, GFP_KERNEL); > + if (ret == 0) > + break; > if (ret < 0) > goto error; > } > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v2] drm/ttm: fix error handling if no BO can be swapped out
[AMD Official Use Only - Internal Distribution Only] Alright, Thanks again! --Brs, Morris Zhang MLSE Linux ML SRDC Ext. 25147 -Original Message- From: Koenig, Christian Sent: Thursday, April 22, 2021 5:22 PM To: Zhang, Morris ; dri-devel@lists.freedesktop.org Subject: Re: [PATCH v2] drm/ttm: fix error handling if no BO can be swapped out Am 22.04.21 um 10:37 schrieb Shiwu Zhang: > Signed-off-by: Shiwu Zhang > --- > drivers/gpu/drm/ttm/ttm_device.c | 2 +- > drivers/gpu/drm/ttm/ttm_tt.c | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_device.c > b/drivers/gpu/drm/ttm/ttm_device.c > index 1f2024164d72..5dc92b056f0a 100644 > --- a/drivers/gpu/drm/ttm/ttm_device.c > +++ b/drivers/gpu/drm/ttm/ttm_device.c > @@ -112,7 +112,7 @@ int ttm_global_swapout(struct ttm_operation_ctx *ctx, > gfp_t gfp_flags) > { > struct ttm_global *glob = &ttm_glob; > struct ttm_device *bdev; > - int ret = -EBUSY; > + int ret = 0; Please don't initialize the return value if we don't need that. Apart from that looks good to me now. Thanks, Christian. > > mutex_lock(&ttm_global_mutex); > list_for_each_entry(bdev, &glob->device_list, device_list) { diff > --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c > index 48c407cff112..539e0232cb3b 100644 > --- a/drivers/gpu/drm/ttm/ttm_tt.c > +++ b/drivers/gpu/drm/ttm/ttm_tt.c > @@ -329,6 +329,8 @@ int ttm_tt_populate(struct ttm_device *bdev, > ttm_dma32_pages_limit) { > > ret = ttm_global_swapout(ctx, GFP_KERNEL); > + if (ret == 0) > + break; > if (ret < 0) > goto error; > } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drm/ttm: fix error handling if no BO can be swapped out
In case that all pre-allocated BOs are busy, just continue to populate BOs since likely half of system memory in total is still free. Signed-off-by: Shiwu Zhang --- drivers/gpu/drm/ttm/ttm_device.c | 2 +- drivers/gpu/drm/ttm/ttm_tt.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 1f2024164d72..a48fe4dccd61 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -112,7 +112,7 @@ int ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { struct ttm_global *glob = &ttm_glob; struct ttm_device *bdev; - int ret = -EBUSY; + int ret; mutex_lock(&ttm_global_mutex); list_for_each_entry(bdev, &glob->device_list, device_list) { diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 48c407cff112..539e0232cb3b 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -329,6 +329,8 @@ int ttm_tt_populate(struct ttm_device *bdev, ttm_dma32_pages_limit) { ret = ttm_global_swapout(ctx, GFP_KERNEL); + if (ret == 0) + break; if (ret < 0) goto error; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] efifb: Check efifb_pci_dev before using it
On Wed, Apr 14, 2021 at 8:20 AM Alex Deucher wrote: > > On Tue, Apr 13, 2021 at 2:37 PM Daniel Vetter wrote: > > > > On Tue, Apr 13, 2021 at 8:02 PM Alex Deucher wrote: > > > > > > On Tue, Apr 13, 2021 at 1:05 PM Kai-Heng Feng > > > wrote: > > > > > > > > On some platforms like Hyper-V and RPi4 with UEFI firmware, efifb is not > > > > a PCI device. > > > > > > > > So make sure efifb_pci_dev is found before using it. > > > > > > > > Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at > > > > PCI D0") > > > > BugLink: https://bugs.launchpad.net/bugs/1922403 > > > > Signed-off-by: Kai-Heng Feng > > > > > > Reviewed-by: Alex Deucher > > > > fbdev is in drm-misc, so maybe you can push this one too? > > Yes, pushed. Thanks! > Can we have this pushed into the branch that gets merged into linux-next. I still don't see this fix in -next and we are unable to do testing on our platform as we hit a boot crash without this as reported in [1]. We prefer running tests on -next without any additional patches or reverts, hence the nagging, sorry for that. Regards, Sudeep [1] https://lore.kernel.org/dri-devel/20210415102224.2764054-1-sudeep.ho...@arm.com/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: Rename DP_PSR_SELECTIVE_UPDATE to better mach eDP spec
The changed name looks more accurate to the edp 1.4b spec. Looks good to me. Reviewed-by: Gwan-gyeong Mun On Wed, 2021-04-21 at 15:02 -0700, José Roberto de Souza wrote: > DP_PSR_EN_CFG bit 5 aka "Selective Update Region Scan Line Capture > Indication" in eDP spec has a ambiguous name, so renaming to better > match specification. > > While at it, replacing bit shit by BIT() macro and adding the version > some registers were added to eDP specification. > > Cc: > Cc: Rodrigo Vivi > Cc: Jani Nikula > Cc: Gwan-gyeong Mun > Signed-off-by: José Roberto de Souza > --- > include/drm/drm_dp_helper.h | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 1e85c2021f2f..d6f6a084a190 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -687,14 +687,14 @@ struct drm_device; > #define DP_DSC_ENABLE 0x160 /* DP 1.4 */ > # define DP_DECOMPRESSION_EN (1 << 0) > > -#define DP_PSR_EN_CFG 0x170 /* XXX 1.2? */ > -# define DP_PSR_ENABLE (1 << 0) > -# define DP_PSR_MAIN_LINK_ACTIVE (1 << 1) > -# define DP_PSR_CRC_VERIFICATION (1 << 2) > -# define DP_PSR_FRAME_CAPTURE (1 << 3) > -# define DP_PSR_SELECTIVE_UPDATE (1 << 4) > -# define DP_PSR_IRQ_HPD_WITH_CRC_ERRORS (1 << 5) > -# define DP_PSR_ENABLE_PSR2 (1 << 6) /* eDP 1.4a */ > +#define DP_PSR_EN_CFG 0x170 /* XXX 1.2? */ > +# define DP_PSR_ENABLE BIT(0) > +# define DP_PSR_MAIN_LINK_ACTIVE BIT(1) > +# define DP_PSR_CRC_VERIFICATION BIT(2) > +# define DP_PSR_FRAME_CAPTURE BIT(3) > +# define DP_PSR_SU_REGION_SCANLINE_CAPTURE BIT(4) /* eDP 1.4a */ > +# define DP_PSR_IRQ_HPD_WITH_CRC_ERRORSBIT(5) /* eDP > 1.4a */ > +# define DP_PSR_ENABLE_PSR2BIT(6) /* eDP 1.4a */ > > #define DP_ADAPTER_CTRL 0x1a0 > # define DP_ADAPTER_CTRL_FORCE_LOAD_SENSE (1 << 0) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/ttm: fix error handling if no BO can be swapped out
Am 22.04.21 um 11:52 schrieb Daniel Vetter: On Thu, Apr 22, 2021 at 04:37:49PM +0800, Shiwu Zhang wrote: Signed-off-by: Shiwu Zhang Cc: sta...@vger.kernel.org or at least Fixes: tag? The code in question is only in drm-misc-next currently and won't go upstream before 5.14. Zero length commit message for something that claims to be a bugfix is probably too short :-) Well that is indeed a good point :) Christian. -Daniel --- drivers/gpu/drm/ttm/ttm_device.c | 2 +- drivers/gpu/drm/ttm/ttm_tt.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 1f2024164d72..5dc92b056f0a 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -112,7 +112,7 @@ int ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { struct ttm_global *glob = &ttm_glob; struct ttm_device *bdev; - int ret = -EBUSY; + int ret = 0; mutex_lock(&ttm_global_mutex); list_for_each_entry(bdev, &glob->device_list, device_list) { diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 48c407cff112..539e0232cb3b 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -329,6 +329,8 @@ int ttm_tt_populate(struct ttm_device *bdev, ttm_dma32_pages_limit) { ret = ttm_global_swapout(ctx, GFP_KERNEL); + if (ret == 0) + break; if (ret < 0) goto error; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cchristian.koenig%40amd.com%7C61e953fa35114f9d69b508d905744ac9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637546819302622693%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=kYfLVPAIa%2FalK8g4xyI9pV7LcIH5Y7VRApNPdUtei5c%3D&reserved=0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/mediatek: clear pending flag when cmdq packet is done.
From: CK Hu In cmdq mode, packet may be flushed before it is executed, so the pending flag should be cleared after cmdq packet is done. Signed-off-by: CK Hu Signed-off-by: Hsin-Yi Wang --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 56 +--- include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 40df2c823187..051bf0eb00d3 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -224,6 +224,45 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc, #if IS_REACHABLE(CONFIG_MTK_CMDQ) static void ddp_cmdq_cb(struct cmdq_cb_data data) { + struct cmdq_pkt *pkt = (struct cmdq_pkt *)data.data; + struct mtk_drm_crtc *mtk_crtc = (struct mtk_drm_crtc *)pkt->crtc; + struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state); + unsigned int i; + + if (data.sta == CMDQ_CB_ERROR) + goto destroy_pkt; + + if (state->pending_config) { + state->pending_config = false; + } + + if (mtk_crtc->pending_planes) { + for (i = 0; i < mtk_crtc->layer_nr; i++) { + struct drm_plane *plane = &mtk_crtc->planes[i]; + struct mtk_plane_state *plane_state; + + plane_state = to_mtk_plane_state(plane->state); + + if (plane_state->pending.config) + plane_state->pending.config = false; + } + mtk_crtc->pending_planes = false; + } + + if (mtk_crtc->pending_async_planes) { + for (i = 0; i < mtk_crtc->layer_nr; i++) { + struct drm_plane *plane = &mtk_crtc->planes[i]; + struct mtk_plane_state *plane_state; + + plane_state = to_mtk_plane_state(plane->state); + + if (plane_state->pending.async_config) + plane_state->pending.async_config = false; + } + mtk_crtc->pending_async_planes = false; + } + +destroy_pkt: cmdq_pkt_destroy(data.data); } #endif @@ -377,8 +416,8 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc, state->pending_height, state->pending_vrefresh, 0, cmdq_handle); - - state->pending_config = false; + if (!cmdq_handle) + state->pending_config = false; } if (mtk_crtc->pending_planes) { @@ -398,9 +437,11 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc, mtk_ddp_comp_layer_config(comp, local_layer, plane_state, cmdq_handle); - plane_state->pending.config = false; + if (!cmdq_handle) + plane_state->pending.config = false; } - mtk_crtc->pending_planes = false; + if (!cmdq_handle) + mtk_crtc->pending_planes = false; } if (mtk_crtc->pending_async_planes) { @@ -420,9 +461,11 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc, mtk_ddp_comp_layer_config(comp, local_layer, plane_state, cmdq_handle); - plane_state->pending.async_config = false; + if (!cmdq_handle) + plane_state->pending.async_config = false; } - mtk_crtc->pending_async_planes = false; + if (!cmdq_handle) + mtk_crtc->pending_async_planes = false; } } @@ -475,6 +518,7 @@ static void mtk_drm_crtc_update_config(struct mtk_drm_crtc *mtk_crtc, cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false); mtk_crtc_ddp_config(crtc, cmdq_handle); cmdq_pkt_finalize(cmdq_handle); + cmdq_handle->crtc = mtk_crtc; cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle); } #endif diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h index d5a983d65f05..c06b14ec03e5 100644 --- a/include/linux/mailbox/mtk-cmdq-mailbox.h +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h @@ -90,6 +90,7 @@ struct cmdq_pkt { struct cmdq_task_cb cb; struct cmdq_task_cb async_cb; void*cl; + void*crtc; }; u8 cmdq_get_shift_pa(struct mbox_chan *chan); -- 2.31.1.498.g6c1eba
Re: [PATCH v3] drm/ttm: fix error handling if no BO can be swapped out
Am 22.04.21 um 12:25 schrieb Shiwu Zhang: In case that all pre-allocated BOs are busy, just continue to populate I'm not a native speaker of English either, but I think that should read "previously allocated". BOs since likely half of system memory in total is still free. Signed-off-by: Shiwu Zhang --- drivers/gpu/drm/ttm/ttm_device.c | 2 +- drivers/gpu/drm/ttm/ttm_tt.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 1f2024164d72..a48fe4dccd61 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -112,7 +112,7 @@ int ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { struct ttm_global *glob = &ttm_glob; struct ttm_device *bdev; - int ret = -EBUSY; + int ret; Oh, we have been inconsistent here? In this case we should indeed zero initialize the variable and also fix the only other user in vmwgfx! Going to take care of this myself. Thanks for the help, Christian. mutex_lock(&ttm_global_mutex); list_for_each_entry(bdev, &glob->device_list, device_list) { diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 48c407cff112..539e0232cb3b 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -329,6 +329,8 @@ int ttm_tt_populate(struct ttm_device *bdev, ttm_dma32_pages_limit) { ret = ttm_global_swapout(ctx, GFP_KERNEL); + if (ret == 0) + break; if (ret < 0) goto error; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/10] drm/amdgpu: Move dmabuf attach/detach to backend_(un)bind
Am 22.04.21 um 03:30 schrieb Felix Kuehling: The dmabuf attachment should be updated by moving the SG BO to DOMAIN_CPU and back to DOMAIN_GTT. This does not necessarily invoke the populate/unpopulate callbacks. Do this in backend_bind/unbind instead. Signed-off-by: Felix Kuehling Reviewed-by: Christian König --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 3 -- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 51 +-- 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 18a1f9222a59..68e6ce8dcf33 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -582,9 +582,6 @@ kfd_mem_dmaunmap_dmabuf(struct kfd_mem_attachment *attachment) amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU); ttm_bo_validate(&bo->tbo, &bo->placement, &ctx); - /* FIXME: This does not guarantee that amdgpu_ttm_tt_unpopulate is -* called -*/ } static void diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 7e7d8330d64b..fc2a8d681dbc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -910,7 +910,23 @@ static int amdgpu_ttm_backend_bind(struct ttm_device *bdev, DRM_ERROR("failed to pin userptr\n"); return r; } + } else if (ttm->page_flags & TTM_PAGE_FLAG_SG) { + if (!ttm->sg) { + struct dma_buf_attachment *attach; + struct sg_table *sgt; + + attach = gtt->gobj->import_attach; + sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); + if (IS_ERR(sgt)) + return PTR_ERR(sgt); + + ttm->sg = sgt; + } + + drm_prime_sg_to_dma_addr_array(ttm->sg, gtt->ttm.dma_address, + ttm->num_pages); } + if (!ttm->num_pages) { WARN(1, "nothing to bind %u pages for mreg %p back %p!\n", ttm->num_pages, bo_mem, ttm); @@ -1037,8 +1053,15 @@ static void amdgpu_ttm_backend_unbind(struct ttm_device *bdev, int r; /* if the pages have userptr pinning then clear that first */ - if (gtt->userptr) + if (gtt->userptr) { amdgpu_ttm_tt_unpin_userptr(bdev, ttm); + } else if (ttm->sg && gtt->gobj->import_attach) { + struct dma_buf_attachment *attach; + + attach = gtt->gobj->import_attach; + dma_buf_unmap_attachment(attach, ttm->sg, DMA_BIDIRECTIONAL); + ttm->sg = NULL; + } if (!gtt->bound) return; @@ -1125,23 +1148,8 @@ static int amdgpu_ttm_tt_populate(struct ttm_device *bdev, return 0; } - if (ttm->page_flags & TTM_PAGE_FLAG_SG) { - if (!ttm->sg) { - struct dma_buf_attachment *attach; - struct sg_table *sgt; - - attach = gtt->gobj->import_attach; - sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); - if (IS_ERR(sgt)) - return PTR_ERR(sgt); - - ttm->sg = sgt; - } - - drm_prime_sg_to_dma_addr_array(ttm->sg, gtt->ttm.dma_address, - ttm->num_pages); + if (ttm->page_flags & TTM_PAGE_FLAG_SG) return 0; - } return ttm_pool_alloc(&adev->mman.bdev.pool, ttm, ctx); } @@ -1165,15 +1173,6 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device *bdev, return; } - if (ttm->sg && gtt->gobj->import_attach) { - struct dma_buf_attachment *attach; - - attach = gtt->gobj->import_attach; - dma_buf_unmap_attachment(attach, ttm->sg, DMA_BIDIRECTIONAL); - ttm->sg = NULL; - return; - } - if (ttm->page_flags & TTM_PAGE_FLAG_SG) return; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: fix error handling if no BO can be swapped out v4
From: Shiwu Zhang In case that all pre-allocated BOs are busy, just continue to populate BOs since likely half of system memory in total is still free. v4 (chk): fix code moved to VMWGFX as well Signed-off-by: Shiwu Zhang Reviewed-by: Christian König Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_device.c| 2 +- drivers/gpu/drm/ttm/ttm_tt.c| 2 ++ drivers/gpu/drm/vmwgfx/ttm_memory.c | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 1f2024164d72..5dc92b056f0a 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -112,7 +112,7 @@ int ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { struct ttm_global *glob = &ttm_glob; struct ttm_device *bdev; - int ret = -EBUSY; + int ret = 0; mutex_lock(&ttm_global_mutex); list_for_each_entry(bdev, &glob->device_list, device_list) { diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 48c407cff112..539e0232cb3b 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -329,6 +329,8 @@ int ttm_tt_populate(struct ttm_device *bdev, ttm_dma32_pages_limit) { ret = ttm_global_swapout(ctx, GFP_KERNEL); + if (ret == 0) + break; if (ret < 0) goto error; } diff --git a/drivers/gpu/drm/vmwgfx/ttm_memory.c b/drivers/gpu/drm/vmwgfx/ttm_memory.c index 104b95a8c7a2..aeb0a22a2c34 100644 --- a/drivers/gpu/drm/vmwgfx/ttm_memory.c +++ b/drivers/gpu/drm/vmwgfx/ttm_memory.c @@ -280,7 +280,7 @@ static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq, spin_unlock(&glob->lock); ret = ttm_global_swapout(ctx, GFP_KERNEL); spin_lock(&glob->lock); - if (unlikely(ret < 0)) + if (unlikely(ret <= 0)) break; } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: remove special handling for non GEM drivers
Zack or Roland any objections to this? There shouldn't be any functional change. Thanks, Christian. Am 19.04.21 um 11:28 schrieb Christian König: vmwgfx is the only driver actually using this. Move the handling into the driver instead. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 11 --- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 10 ++ include/drm/ttm/ttm_bo_api.h | 19 --- 3 files changed, 10 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 80831df0ef61..38183e227116 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -460,8 +460,6 @@ static void ttm_bo_release(struct kref *kref) atomic_dec(&ttm_glob.bo_count); dma_fence_put(bo->moving); - if (!ttm_bo_uses_embedded_gem_object(bo)) - dma_resv_fini(&bo->base._resv); bo->destroy(bo); } @@ -1056,15 +1054,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, } else { bo->base.resv = &bo->base._resv; } - if (!ttm_bo_uses_embedded_gem_object(bo)) { - /* -* bo.base is not initialized, so we have to setup the -* struct elements we want use regardless. -*/ - bo->base.size = size; - dma_resv_init(&bo->base._resv); - drm_vma_node_reset(&bo->base.vma_node); - } atomic_inc(&ttm_glob.bo_count); /* diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 50e529a01677..587314d57991 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -460,6 +460,7 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo) WARN_ON(vmw_bo->dirty); WARN_ON(!RB_EMPTY_ROOT(&vmw_bo->res_tree)); vmw_bo_unmap(vmw_bo); + dma_resv_fini(&bo->base._resv); kfree(vmw_bo); } @@ -512,6 +513,11 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size, if (unlikely(ret)) goto error_free; + + bo->base.size = size; + dma_resv_init(&bo->base._resv); + drm_vma_node_reset(&bo->base.vma_node); + ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size, ttm_bo_type_device, placement, 0, &ctx, NULL, NULL, NULL); @@ -570,6 +576,10 @@ int vmw_bo_init(struct vmw_private *dev_priv, if (unlikely(ret)) return ret; + vmw_bo->base.base.size = size; + dma_resv_init(&vmw_bo->base.base._resv); + drm_vma_node_reset(&vmw_bo->base.base.vma_node); + ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size, ttm_bo_type_device, placement, 0, &ctx, NULL, NULL, bo_free); diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 3587f660e8f4..e88da481a976 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -562,25 +562,6 @@ ssize_t ttm_bo_io(struct ttm_device *bdev, struct file *filp, int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, gfp_t gfp_flags); -/** - * ttm_bo_uses_embedded_gem_object - check if the given bo uses the - * embedded drm_gem_object. - * - * Most ttm drivers are using gem too, so the embedded - * ttm_buffer_object.base will be initialized by the driver (before - * calling ttm_bo_init). It is also possible to use ttm without gem - * though (vmwgfx does that). - * - * This helper will figure whenever a given ttm bo is a gem object too - * or not. - * - * @bo: The bo to check. - */ -static inline bool ttm_bo_uses_embedded_gem_object(struct ttm_buffer_object *bo) -{ - return bo->base.dev != NULL; -} - /** * ttm_bo_pin - Pin the buffer object. * @bo: The buffer object to pin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/bridge: nwl-dsi: Get MIPI DSI controller and PHY ready in ->mode_set()
Hi, On 22/04/2021 11:31, Liu Ying wrote: > Hi Neil, > > On Thu, 2021-04-22 at 10:48 +0200, Neil Armstrong wrote: >> Hi, >> >> On 22/04/2021 07:14, Liu Ying wrote: >>> Some MIPI DSI panel drivers like 'raydium,rm68200' send >>> MIPI_DCS_SET_DISPLAY_ON commands in panel_funcs->prepare(), which >>> requires the MIPI DSI controller and PHY to be ready beforehand. >>> Without this patch, the nwl-dsi driver gets the MIPI DSI controller >>> and PHY ready in bridge_funcs->pre_enable(), which happens after >>> the panel_funcs->prepare(). So, this patch shifts the bridge operation >>> ealier from bridge_funcs->pre_enable() to bridge_funcs->mode_set(). >> >> This makes sense, this is how we do on the synopsys dw mipi dsi driver. >> >>> This way, more MIPI DSI panels can connect to this nwl-dsi bridge. >>> Care is taken to make sure bridge_funcs->mode_set()/atomic_disable() >>> are called in pairs, which includes removing a check on unchanged HS >>> clock rate and forcing a full modeset when only connector's DPMS is >>> brought out of "Off" status. >> >> I would split the changes in multiple patches to clarify the changes. > > Maybe, I can split this into the below 3 patches: > 1) Remove a check on unchanged HS clock rate from ->mode_set(). > 2) Force a full modeset when only connector's DPMS is brought out of > "Off" status. This will be done in ->atomic_check() together with the > fixup for H/VSYNC polarities. > 3) Shift the bridge operation as the last step. > > I'll mention that 1) & 2) are needed by 3) in their commit message. Sure, you can also put this split explanation in the cover letter. > > Does this split-up sound reasonable? Yes makes sense, thanks Neil > > Thanks, > Liu Ying > >> >> Neil >> >>> Cc: Andrzej Hajda >>> Cc: Neil Armstrong >>> Cc: Robert Foss >>> Cc: Laurent Pinchart >>> Cc: Jonas Karlman >>> Cc: Jernej Skrabec >>> Cc: David Airlie >>> Cc: Daniel Vetter >>> Cc: Guido Günther >>> Cc: Robert Chiras >>> Cc: NXP Linux Team >>> Signed-off-by: Liu Ying >>> --- >>> v1->v2: >>> * Fix commit message typo - s/unchange/unchanged/ >>> >>> drivers/gpu/drm/bridge/nwl-dsi.c | 86 >>> +--- >>> 1 file changed, 46 insertions(+), 40 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c >>> b/drivers/gpu/drm/bridge/nwl-dsi.c >>> index be6bfc5..229f0b1 100644 >>> --- a/drivers/gpu/drm/bridge/nwl-dsi.c >>> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c >>> @@ -21,6 +21,7 @@ >>> #include >>> #include >>> >>> +#include >>> #include >>> #include >>> #include >>> @@ -661,7 +662,7 @@ static irqreturn_t nwl_dsi_irq_handler(int irq, void >>> *data) >>> return IRQ_HANDLED; >>> } >>> >>> -static int nwl_dsi_enable(struct nwl_dsi *dsi) >>> +static int nwl_dsi_mode_set(struct nwl_dsi *dsi) >>> { >>> struct device *dev = dsi->dev; >>> union phy_configure_opts *phy_cfg = &dsi->phy_cfg; >>> @@ -748,7 +749,9 @@ static int nwl_dsi_disable(struct nwl_dsi *dsi) >>> return 0; >>> } >>> >>> -static void nwl_dsi_bridge_disable(struct drm_bridge *bridge) >>> +static void >>> +nwl_dsi_bridge_atomic_disable(struct drm_bridge *bridge, >>> + struct drm_bridge_state *old_bridge_state) >>> { >>> struct nwl_dsi *dsi = bridge_to_dsi(bridge); >>> int ret; >>> @@ -809,17 +812,6 @@ static int nwl_dsi_get_dphy_params(struct nwl_dsi *dsi, >>> return 0; >>> } >>> >>> -static bool nwl_dsi_bridge_mode_fixup(struct drm_bridge *bridge, >>> - const struct drm_display_mode *mode, >>> - struct drm_display_mode *adjusted_mode) >>> -{ >>> - /* At least LCDIF + NWL needs active high sync */ >>> - adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); >>> - adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC); >>> - >>> - return true; >>> -} >>> - >>> static enum drm_mode_status >>> nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge, >>> const struct drm_display_info *info, >>> @@ -837,6 +829,29 @@ nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge, >>> return MODE_OK; >>> } >>> >>> +static int nwl_dsi_bridge_atomic_check(struct drm_bridge *bridge, >>> + struct drm_bridge_state *bridge_state, >>> + struct drm_crtc_state *crtc_state, >>> + struct drm_connector_state *conn_state) >>> +{ >>> + struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; >>> + >>> + /* At least LCDIF + NWL needs active high sync */ >>> + adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); >>> + adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC); >>> + >>> + /* >>> +* Do a full modeset if crtc_state->active is changed to be true. >>> +* This ensures our ->mode_set() is called to get the DSI controller >>> +* and the PHY ready to send DCS
Re: [PATCH 27/40] drm/ttm/ttm_device: Demote kernel-doc abuses
Am 20.04.21 um 10:53 schrieb Daniel Vetter: On Fri, Apr 16, 2021 at 05:32:52PM +0200, Christian König wrote: Am 16.04.21 um 16:37 schrieb Lee Jones: Fixes the following W=1 kernel build warning(s): drivers/gpu/drm/ttm/ttm_device.c:42: warning: Function parameter or member 'ttm_global_mutex' not described in 'DEFINE_MUTEX' drivers/gpu/drm/ttm/ttm_device.c:42: warning: expecting prototype for ttm_global_mutex(). Prototype was for DEFINE_MUTEX() instead drivers/gpu/drm/ttm/ttm_device.c:112: warning: Function parameter or member 'ctx' not described in 'ttm_global_swapout' drivers/gpu/drm/ttm/ttm_device.c:112: warning: Function parameter or member 'gfp_flags' not described in 'ttm_global_swapout' drivers/gpu/drm/ttm/ttm_device.c:112: warning: expecting prototype for A buffer object shrink method that tries to swap out the first(). Prototype was for ttm_global_swapout() instead Cc: Christian Koenig Cc: Huang Rui Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Signed-off-by: Lee Jones Reviewed-by: Christian König Can you pls also land all the patches you reviewed from Lee into drm-misc-next? Just so they wont' get lost. Will all head in for 5.14. Alex took care of the amdgpu patches and I've just pushed the TTM and the scheduler patch to drm-misc-next. Christian. -Daniel --- drivers/gpu/drm/ttm/ttm_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 9b787b3caeb50..a8bec8358350d 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -36,7 +36,7 @@ #include "ttm_module.h" -/** +/* * ttm_global_mutex - protecting the global state */ DEFINE_MUTEX(ttm_global_mutex); @@ -104,7 +104,7 @@ static int ttm_global_init(void) return ret; } -/** +/* * A buffer object shrink method that tries to swap out the first * buffer object on the global::swap_lru list. */ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/amdkfd: fix uint32 variable compared to less than zero
From: Colin Ian King Currently the call to kfd_process_gpuidx_from_gpuid is returning an int value and this is being assigned to a uint32_t variable gpuidx and this is being checked for a negative error return which is always going to be false. Fix this by making gpuidx an int32_t. This makes gpuidx also type consistent with the use of gpuidx from the callers. Addresses-Coverity: ("Unsigned compared against 0") Fixes: cda0f85bfa5e ("drm/amdkfd: refine migration policy with xnack on") Signed-off-by: Colin Ian King --- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c index 0e0b4ffd20ab..bf3c8de85b4a 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c @@ -1330,7 +1330,7 @@ static void svm_range_unreserve_bos(struct svm_validate_context *ctx) */ static int svm_range_validate_and_map(struct mm_struct *mm, struct svm_range *prange, - uint32_t gpuidx, bool intr, bool wait) + int32_t gpuidx, bool intr, bool wait) { struct svm_validate_context ctx; struct hmm_range *hmm_range; -- 2.30.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] coda: fix reference counting in coda_file_mmap error path
Hi Jan, Am 22.04.21 um 14:27 schrieb Jan Harkes: Looks good to me. I'm also maintaining an out of tree coda module build that people sometimes use, which has workarounds for differences between the various kernel versions. Do you have a reference to the corresponding mmap_region change? If it is merged already I'll probably be able to find it. Is this mmap_region change expected to be backported to any lts kernels? That is the following upstream commit in Linus tree: commit 1527f926fd04490f648c42f42b45218a04754f87 Author: Christian König Date: Fri Oct 9 15:08:55 2020 +0200 mm: mmap: fix fput in error path v2 But I don't think we should backport that. And sorry for the noise. We had so many places which expected different behavior that I didn't noticed that two occasions in the fs code actually rely on the current behavior. For your out of tree module you could make the code version independent by setting the vma back to the original file in case of an error. That should work with both behaviors in mmap_region. Thanks, Christian. Jan On April 21, 2021 9:20:11 AM EDT, "Christian König" wrote: mmap_region() now calls fput() on the vma->vm_file. So we need to drop the extra reference on the coda file instead of the host file. Signed-off-by: Christian König Fixes: 1527f926fd04 ("mm: mmap: fix fput in error path v2") CC: sta...@vger.kernel.org # 5.11+ --- fs/coda/file.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/coda/file.c b/fs/coda/file.c index 128d63df5bfb..ef5ca22bfb3e 100644 --- a/fs/coda/file.c +++ b/fs/coda/file.c @@ -175,10 +175,10 @@ coda_file_mmap(struct file *coda_file, struct vm_area_struct *vma) ret = call_mmap(vma->vm_file, vma); if (ret) { - /* if call_mmap fails, our caller will put coda_file so we -* should drop the reference to the host_file that we got. + /* if call_mmap fails, our caller will put host_file so we +* should drop the reference to the coda_file that we got. */ - fput(host_file); + fput(coda_file); kfree(cvm_ops); } else { /* here we add redirects for the open/close vm_operations */ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/amdkfd: remove redundant initialization to variable r
From: Colin Ian King The variable r is being initialized with a value that is never read and it is being updated later with a new value. The initialization is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King --- drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c index d44a46eb00d6..a66b67083d83 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c @@ -303,7 +303,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, uint64_t vram_addr; uint64_t offset; uint64_t i, j; - int r = -ENOMEM; + int r; pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms, prange->start, prange->last); -- 2.30.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] drm/msm/dpu: simplify dpu_hw_blk handling
Drop most of "extra" features of dpu_hw_blk. Changes since v1: - Make dpu_hw_blk an empty structure - Split this into separate patchset Dmitry Baryshkov (3): drm/msm/dpu: remove unused dpu_hw_blk features drm/msm/dpu: drop dpu_hw_blk_destroy function drm/msm/dpu: hw_blk: make dpu_hw_blk empty opaque structure drivers/gpu/drm/msm/Makefile| 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c | 139 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h | 22 +--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 6 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 7 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 6 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 6 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c | 6 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 6 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 6 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 6 - drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 +- 12 files changed, 2 insertions(+), 211 deletions(-) delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] drm/msm/dpu: hw_blk: make dpu_hw_blk empty opaque structure
The code does not really use dpu_hw_blk fields, so drop them, making dpu_hw_blk empty structure. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/Makefile | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c| 24 --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h| 4 +--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c| 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 2 -- .../gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c| 2 -- .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c| 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 2 +- 12 files changed, 2 insertions(+), 45 deletions(-) delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 610d630326bb..55dbde30c2a2 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -58,7 +58,6 @@ msm-y := \ disp/dpu1/dpu_encoder_phys_cmd.o \ disp/dpu1/dpu_encoder_phys_vid.o \ disp/dpu1/dpu_formats.o \ - disp/dpu1/dpu_hw_blk.o \ disp/dpu1/dpu_hw_catalog.o \ disp/dpu1/dpu_hw_ctl.o \ disp/dpu1/dpu_hw_interrupts.o \ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c deleted file mode 100644 index 1f2b74b9eb65.. --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c +++ /dev/null @@ -1,24 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. - */ - -#define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__ - -#include -#include -#include - -#include "dpu_hw_mdss.h" -#include "dpu_hw_blk.h" - -/** - * dpu_hw_blk_init - initialize hw block object - * @hw_blk: pointer to hw block object - * @type: hw block type - enum dpu_hw_blk_type - * @id: instance id of the hw block - */ -void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id) -{ - hw_blk->type = type; - hw_blk->id = id; -} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h index 7768694b558a..52e92f37eda4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h @@ -19,9 +19,7 @@ struct dpu_hw_blk; * @refcount: reference/usage count */ struct dpu_hw_blk { - u32 type; - int id; + /* opaque */ }; -void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id); #endif /*_DPU_HW_BLK_H */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c index 441f66a4fb37..f8a74f6cdc4c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -613,8 +613,6 @@ struct dpu_hw_ctl *dpu_hw_ctl_init(enum dpu_ctl idx, c->mixer_count = m->mixer_count; c->mixer_hw_caps = m->mixer; - dpu_hw_blk_init(&c->base, DPU_HW_BLK_CTL, idx); - return c; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c index 977b25968f34..a98e964c3b6f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c @@ -110,8 +110,6 @@ struct dpu_hw_dspp *dpu_hw_dspp_init(enum dpu_dspp idx, c->cap = cfg; _setup_dspp_ops(c, c->cap->features); - dpu_hw_blk_init(&c->base, DPU_HW_BLK_DSPP, idx); - return c; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c index 17224556d5a8..116e2b5b1a90 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c @@ -325,8 +325,6 @@ struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx, c->mdss = m; _setup_intf_ops(&c->ops, c->cap->features); - dpu_hw_blk_init(&c->base, DPU_HW_BLK_INTF, idx); - return c; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c index 76f8b8f75b82..cb6bb7a22c15 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c @@ -182,8 +182,6 @@ struct dpu_hw_mixer *dpu_hw_lm_init(enum dpu_lm idx, c->cap = cfg; _setup_mixer_ops(m, &c->ops, c->cap->features); - dpu_hw_blk_init(&c->base, DPU_HW_BLK_LM, idx); - return c; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c index 406ba950a066..c06d595d5df0 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c @@ -79,8 +79,6 @@ struct dpu_hw_merge_3d *dpu_hw_merge_3d_init(enum dpu_merge_3d idx, c->caps = cfg; _setup_m
[PATCH v2 1/3] drm/msm/dpu: remove unused dpu_hw_blk features
Remove all unused dpu_hw_blk features and functions: - dpu_hw_blk_get()/_put() and respective refcounting, - global list of all dpu_hw_blk instances, - dpu_hw_blk_ops and empty implementation inside each hw_blk subdriver. This leaves dpu_hw_blk as a placeholder with just type and index. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c| 104 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h| 19 +--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c| 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 4 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c| 4 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c| 4 +- 10 files changed, 10 insertions(+), 145 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c index 819b26e660b9..abad043f35f5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c @@ -11,33 +11,16 @@ #include "dpu_hw_mdss.h" #include "dpu_hw_blk.h" -/* Serialization lock for dpu_hw_blk_list */ -static DEFINE_MUTEX(dpu_hw_blk_lock); - -/* List of all hw block objects */ -static LIST_HEAD(dpu_hw_blk_list); - /** * dpu_hw_blk_init - initialize hw block object * @hw_blk: pointer to hw block object * @type: hw block type - enum dpu_hw_blk_type * @id: instance id of the hw block - * @ops: Pointer to block operations */ -void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id, - struct dpu_hw_blk_ops *ops) +void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id) { - INIT_LIST_HEAD(&hw_blk->list); hw_blk->type = type; hw_blk->id = id; - atomic_set(&hw_blk->refcount, 0); - - if (ops) - hw_blk->ops = *ops; - - mutex_lock(&dpu_hw_blk_lock); - list_add(&hw_blk->list, &dpu_hw_blk_list); - mutex_unlock(&dpu_hw_blk_lock); } /** @@ -51,89 +34,4 @@ void dpu_hw_blk_destroy(struct dpu_hw_blk *hw_blk) pr_err("invalid parameters\n"); return; } - - if (atomic_read(&hw_blk->refcount)) - pr_err("hw_blk:%d.%d invalid refcount\n", hw_blk->type, - hw_blk->id); - - mutex_lock(&dpu_hw_blk_lock); - list_del(&hw_blk->list); - mutex_unlock(&dpu_hw_blk_lock); -} - -/** - * dpu_hw_blk_get - get hw_blk from free pool - * @hw_blk: if specified, increment reference count only - * @type: if hw_blk is not specified, allocate the next available of this type - * @id: if specified (>= 0), allocate the given instance of the above type - * return: pointer to hw block object - */ -struct dpu_hw_blk *dpu_hw_blk_get(struct dpu_hw_blk *hw_blk, u32 type, int id) -{ - struct dpu_hw_blk *curr; - int rc, refcount; - - if (!hw_blk) { - mutex_lock(&dpu_hw_blk_lock); - list_for_each_entry(curr, &dpu_hw_blk_list, list) { - if ((curr->type != type) || - (id >= 0 && curr->id != id) || - (id < 0 && - atomic_read(&curr->refcount))) - continue; - - hw_blk = curr; - break; - } - mutex_unlock(&dpu_hw_blk_lock); - } - - if (!hw_blk) { - pr_debug("no hw_blk:%d\n", type); - return NULL; - } - - refcount = atomic_inc_return(&hw_blk->refcount); - - if (refcount == 1 && hw_blk->ops.start) { - rc = hw_blk->ops.start(hw_blk); - if (rc) { - pr_err("failed to start hw_blk:%d rc:%d\n", type, rc); - goto error_start; - } - } - - pr_debug("hw_blk:%d.%d refcount:%d\n", hw_blk->type, - hw_blk->id, refcount); - return hw_blk; - -error_start: - dpu_hw_blk_put(hw_blk); - return ERR_PTR(rc); -} - -/** - * dpu_hw_blk_put - put hw_blk to free pool if decremented refcount is zero - * @hw_blk: hw block to be freed - */ -void dpu_hw_blk_put(struct dpu_hw_blk *hw_blk) -{ - if (!hw_blk) { - pr_err("invalid parameters\n"); - return; - } - - pr_debug("hw_blk:%d.%d refcount:%d\n", hw_blk->type, hw_blk->id, - atomic_read(&hw_blk->refcount)); - - if (!atomic_read(&hw_blk->refcount)) { - pr_err("hw_blk:%d.%d invalid put\n", hw_blk->type, hw_blk->id); - return; - } - - if (atomic_dec_return(&hw_blk->refcount)) - return; - - if (hw_blk->ops.stop) - hw_bl
[PATCH v2 2/3] drm/msm/dpu: drop dpu_hw_blk_destroy function
The dpu_hw_blk_destroy() function is empty, so we can drop it now. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c | 13 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 3 --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 2 -- 10 files changed, 31 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c index abad043f35f5..1f2b74b9eb65 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c @@ -22,16 +22,3 @@ void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id) hw_blk->type = type; hw_blk->id = id; } - -/** - * dpu_hw_blk_destroy - destroy hw block object. - * @hw_blk: pointer to hw block object - * return: none - */ -void dpu_hw_blk_destroy(struct dpu_hw_blk *hw_blk) -{ - if (!hw_blk) { - pr_err("invalid parameters\n"); - return; - } -} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h index fb3be9a36a50..7768694b558a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h @@ -24,5 +24,4 @@ struct dpu_hw_blk { }; void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id); -void dpu_hw_blk_destroy(struct dpu_hw_blk *hw_blk); #endif /*_DPU_HW_BLK_H */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c index 04a2c4b9a357..441f66a4fb37 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -620,7 +620,5 @@ struct dpu_hw_ctl *dpu_hw_ctl_init(enum dpu_ctl idx, void dpu_hw_ctl_destroy(struct dpu_hw_ctl *ctx) { - if (ctx) - dpu_hw_blk_destroy(&ctx->base); kfree(ctx); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c index d2f1045a736a..977b25968f34 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c @@ -117,9 +117,6 @@ struct dpu_hw_dspp *dpu_hw_dspp_init(enum dpu_dspp idx, void dpu_hw_dspp_destroy(struct dpu_hw_dspp *dspp) { - if (dspp) - dpu_hw_blk_destroy(&dspp->base); - kfree(dspp); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c index 6ffe97601716..17224556d5a8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c @@ -332,8 +332,6 @@ struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx, void dpu_hw_intf_destroy(struct dpu_hw_intf *intf) { - if (intf) - dpu_hw_blk_destroy(&intf->base); kfree(intf); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c index 554bb881de3a..76f8b8f75b82 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c @@ -189,7 +189,5 @@ struct dpu_hw_mixer *dpu_hw_lm_init(enum dpu_lm idx, void dpu_hw_lm_destroy(struct dpu_hw_mixer *lm) { - if (lm) - dpu_hw_blk_destroy(&lm->base); kfree(lm); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c index 863229dd0140..406ba950a066 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c @@ -86,7 +86,5 @@ struct dpu_hw_merge_3d *dpu_hw_merge_3d_init(enum dpu_merge_3d idx, void dpu_hw_merge_3d_destroy(struct dpu_hw_merge_3d *hw) { - if (hw) - dpu_hw_blk_destroy(&hw->base); kfree(hw); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c index 334d5b28f533..92cd724263ce 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c @@ -289,7 +289,5 @@ struct dpu_hw_pingpong *dpu_hw_pingpong_init(enum dpu_pingpong idx, void dpu_hw_pingpong_destroy(struct dpu_hw_pingpong *pp) { - if (pp) - dpu_hw_blk_destroy(&pp->base); kfree(pp); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c index ceb2488ea270..8734a47040aa 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -740,8 +740,6 @@ struct dpu_hw_pipe *dpu_hw_sspp_init(enum dpu_sspp idx, void dpu_hw_sspp_destro
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Rewrite CL/CTG L-shaped memory detection
On Thu, Apr 22, 2021 at 11:49:43AM +0200, Daniel Vetter wrote: > On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Currently we try to detect a symmetric memory configurations > > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > > either only set on a very specific subset of machines or it > > just does not exist (it's not mentioned in any public chipset > > datasheets I've found). As it happens my CL/CTG machines never > > set said bit, even if I populate the channels with identical > > sticks. > > > > So let's do the L-shaped memory detection the same way as the > > desktop variants, ie. just look at the DRAM rank boundary > > registers to see if both channels have an identical size. > > > > With this my CL/CTG no longer claim L-shaped memory when I use > > identical sticks. Also tested with non-matching sticks just to > > make sure the L-shaped memory is still properly detected. > > > > And for completeness let's update the debugfs code to dump > > the correct set of registers on each platform. > > > > Cc: Chris Wilson > > Signed-off-by: Ville Syrjälä > > Did you check this with the swapping igt? I have some vague memories of > bug reports where somehow the machine was acting like it's L-shaped memory > despite that banks were populated equally. I've iirc tried all kinds of > tricks to figure it out, all to absolutely no avail. Did you have a specific test in mind? I ran a bunch of things that seemed swizzle related. All passed just fine. Chris did have similar concerns and suggested we should have better tests. I guess what I should try to do is some selftests which make sure we test both high and low physical addresses and check the swizzle pattern is as expected. But haven't found the time to do that yet. > > tbh I'd just not touch this, not really worth it. It's totally worth it to get gen4 machines working again. > -Daniel > > --- > > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 15 --- > > drivers/gpu/drm/i915/i915_debugfs.c | 16 > > drivers/gpu/drm/i915/i915_reg.h | 4 > > 3 files changed, 24 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > index 0fa6c38893f7..754f20768de5 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > @@ -693,14 +693,15 @@ static void detect_bit_6_swizzle(struct i915_ggtt > > *ggtt) > > swizzle_x = I915_BIT_6_SWIZZLE_9_10_17; > > swizzle_y = I915_BIT_6_SWIZZLE_9_17; > > } > > - break; > > - } > > > > - /* check for L-shaped memory aka modified enhanced addressing */ > > - if (IS_GEN(i915, 4) && > > - !(intel_uncore_read(uncore, DCC2) & > > DCC2_MODIFIED_ENHANCED_DISABLE)) { > > - swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > > - swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > > + /* check for L-shaped memory aka modified enhanced > > addressing */ > > + if (IS_GEN(i915, 4) && > > + intel_uncore_read16(uncore, C0DRB3_CL) != > > + intel_uncore_read16(uncore, C1DRB3_CL)) { > > + swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > > + swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > > + } > > + break; > > } > > > > if (dcc == 0x) { > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index 8dd374691102..6de11ffcde38 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -621,10 +621,18 @@ static int i915_swizzle_info(struct seq_file *m, void > > *data) > >intel_uncore_read(uncore, DCC)); > > seq_printf(m, "DDC2 = 0x%08x\n", > >intel_uncore_read(uncore, DCC2)); > > - seq_printf(m, "C0DRB3 = 0x%04x\n", > > - intel_uncore_read16(uncore, C0DRB3_BW)); > > - seq_printf(m, "C1DRB3 = 0x%04x\n", > > - intel_uncore_read16(uncore, C1DRB3_BW)); > > + > > + if (IS_G45(dev_priv) || IS_I965G(dev_priv) || IS_G33(dev_priv)) > > { > > + seq_printf(m, "C0DRB3 = 0x%04x\n", > > + intel_uncore_read16(uncore, C0DRB3_BW)); > > + seq_printf(m, "C1DRB3 = 0x%04x\n", > > + intel_uncore_read16(uncore, C1DRB3_BW)); > > + } else if (IS_GEN(dev_priv, 4)) { > > + seq_printf(m, "C0DRB3 = 0x%04x\n", > > + intel_uncore_read16(uncore, C0DRB3_CL)); > > + seq_printf(m,
[PATCH] vfio/gvt: fix DRM_I915_GVT dependency on VFIO_MDEV
From: Arnd Bergmann The Kconfig dependency is incomplete since DRM_I915_GVT is a 'bool' symbol that depends on the 'tristate' VFIO_MDEV. This allows a configuration with VFIO_MDEV=m, DRM_I915_GVT=y and DRM_I915=y that causes a link failure: x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function `available_instances_show': gvt.c:(.text+0x67a): undefined reference to `mtype_get_parent_dev' x86_64-linux-ld: gvt.c:(.text+0x6a5): undefined reference to `mtype_get_type_group_id' x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function `description_show': gvt.c:(.text+0x76e): undefined reference to `mtype_get_parent_dev' x86_64-linux-ld: gvt.c:(.text+0x799): undefined reference to `mtype_get_type_group_id' Clarify the dependency by specifically disallowing the broken configuration. If VFIO_MDEV is built-in, it will work, but if VFIO_MDEV=m, the i915 driver cannot be built-in here. Fixes: 07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV") Fixes: 9169cff168ff ("vfio/mdev: Correct the function signatures for the mdev_type_attributes") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/i915/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 7a5b7a93d33e..791cc9556863 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -111,7 +111,7 @@ config DRM_I915_GVT bool "Enable Intel GVT-g graphics virtualization host support" depends on DRM_I915 depends on 64BIT - depends on VFIO_MDEV + depends on VFIO_MDEV=y || VFIO_MDEV=DRM_I915 default n help Choose this option if you want to enable Intel GVT-g graphics -- 2.29.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2
Alex Williamson writes: > On Mon, 12 Apr 2021 19:41:41 +1000 > Michael Ellerman wrote: > >> Alex Williamson writes: >> > On Fri, 26 Mar 2021 07:13:10 +0100 >> > Christoph Hellwig wrote: >> > >> >> This driver never had any open userspace (which for VFIO would include >> >> VM kernel drivers) that use it, and thus should never have been added >> >> by our normal userspace ABI rules. >> >> >> >> Signed-off-by: Christoph Hellwig >> >> Acked-by: Greg Kroah-Hartman >> >> --- >> >> drivers/vfio/pci/Kconfig| 6 - >> >> drivers/vfio/pci/Makefile | 1 - >> >> drivers/vfio/pci/vfio_pci.c | 18 - >> >> drivers/vfio/pci/vfio_pci_nvlink2.c | 490 >> >> drivers/vfio/pci/vfio_pci_private.h | 14 - >> >> include/uapi/linux/vfio.h | 38 +-- >> >> 6 files changed, 4 insertions(+), 563 deletions(-) >> >> delete mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c >> > >> > Hearing no objections, applied to vfio next branch for v5.13. Thanks, >> >> Looks like you only took patch 1? >> >> I can't take patch 2 on its own, that would break the build. >> >> Do you want to take both patches? There's currently no conflicts against >> my tree. It's possible one could appear before the v5.13 merge window, >> though it would probably just be something minor. >> >> Or I could apply both patches to my tree, which means patch 1 would >> appear as two commits in the git history, but that's not a big deal. > > I've already got a conflict in my next branch with patch 1, so it's > best to go through my tree. Seems like a shared branch would be > easiest to allow you to merge and manage potential conflicts against > patch 2, I've pushed a branch here: > > https://github.com/awilliam/linux-vfio.git v5.13/vfio/nvlink Thanks. My next is based on rc2, so I won't pull that in directly, because I don't want to pull all of rc6 in with it. I'll put it in a topic branch and merge it into my next after my first pull has gone to Linus. cheers ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2
On Thu, Apr 22, 2021 at 11:49:31PM +1000, Michael Ellerman wrote: > Alex Williamson writes: > > On Mon, 12 Apr 2021 19:41:41 +1000 > > Michael Ellerman wrote: > > > >> Alex Williamson writes: > >> > On Fri, 26 Mar 2021 07:13:10 +0100 > >> > Christoph Hellwig wrote: > >> > > >> >> This driver never had any open userspace (which for VFIO would include > >> >> VM kernel drivers) that use it, and thus should never have been added > >> >> by our normal userspace ABI rules. > >> >> > >> >> Signed-off-by: Christoph Hellwig > >> >> Acked-by: Greg Kroah-Hartman > >> >> drivers/vfio/pci/Kconfig| 6 - > >> >> drivers/vfio/pci/Makefile | 1 - > >> >> drivers/vfio/pci/vfio_pci.c | 18 - > >> >> drivers/vfio/pci/vfio_pci_nvlink2.c | 490 > >> >> drivers/vfio/pci/vfio_pci_private.h | 14 - > >> >> include/uapi/linux/vfio.h | 38 +-- > >> >> 6 files changed, 4 insertions(+), 563 deletions(-) > >> >> delete mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c > >> > > >> > Hearing no objections, applied to vfio next branch for v5.13. Thanks, > >> > >> Looks like you only took patch 1? > >> > >> I can't take patch 2 on its own, that would break the build. > >> > >> Do you want to take both patches? There's currently no conflicts against > >> my tree. It's possible one could appear before the v5.13 merge window, > >> though it would probably just be something minor. > >> > >> Or I could apply both patches to my tree, which means patch 1 would > >> appear as two commits in the git history, but that's not a big deal. > > > > I've already got a conflict in my next branch with patch 1, so it's > > best to go through my tree. Seems like a shared branch would be > > easiest to allow you to merge and manage potential conflicts against > > patch 2, I've pushed a branch here: > > > > https://github.com/awilliam/linux-vfio.git v5.13/vfio/nvlink > > Thanks. > > My next is based on rc2, so I won't pull that in directly, because I > don't want to pull all of rc6 in with it. Linus is fine if you merge in rc's for development reasons. He doesn't like it when people just merge rc's without a purpose. Merge rc7 to your tree then pull the nvlink topic is acceptable. Or just do nothing because Alex will send it through his tree - this extra co-ordination is really only necessary if there are conflicts. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vfio/gvt: fix DRM_I915_GVT dependency on VFIO_MDEV
On Thu, Apr 22, 2021 at 03:35:33PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann > > The Kconfig dependency is incomplete since DRM_I915_GVT is a 'bool' > symbol that depends on the 'tristate' VFIO_MDEV. This allows a > configuration with VFIO_MDEV=m, DRM_I915_GVT=y and DRM_I915=y that > causes a link failure: > > x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function > `available_instances_show': > gvt.c:(.text+0x67a): undefined reference to `mtype_get_parent_dev' > x86_64-linux-ld: gvt.c:(.text+0x6a5): undefined reference to > `mtype_get_type_group_id' > x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function > `description_show': > gvt.c:(.text+0x76e): undefined reference to `mtype_get_parent_dev' > x86_64-linux-ld: gvt.c:(.text+0x799): undefined reference to > `mtype_get_type_group_id' > > Clarify the dependency by specifically disallowing the broken > configuration. If VFIO_MDEV is built-in, it will work, but if > VFIO_MDEV=m, the i915 driver cannot be built-in here. > > Fixes: 07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV") > Fixes: 9169cff168ff ("vfio/mdev: Correct the function signatures for the > mdev_type_attributes") > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/drm/i915/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Oh kconfig stuff like this makes my head hurt, thanks for finding it I also can't see an alternative to this ugly thing, besides having the i915 guys properly modularize this code someday Reviewed-by: Jason Gunthorpe Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5] dma-buf: Add DmaBufTotal counter in meminfo
On 4/22/21 10:06 AM, Mike Rapoport wrote: > On Wed, Apr 21, 2021 at 05:35:57PM +, peter.enderb...@sony.com wrote: >> On 4/21/21 5:31 PM, Mike Rapoport wrote: >>> On Wed, Apr 21, 2021 at 10:37:11AM +, peter.enderb...@sony.com wrote: On 4/21/21 11:15 AM, Daniel Vetter wrote: > We need to understand what the "correct" value is. Not in terms of kernel > code, but in terms of semantics. Like if userspace allocates a GL texture, > is this supposed to show up in your metric or not. Stuff like that. That it like that would like to only one pointer type. You need to know what you pointing at to know what it is. it might be a hardware or a other pointer. If there is a limitation on your pointers it is a good metric to count them even if you don't know what they are. Same goes for dma-buf, they are generic, but they consume some resources that are counted in pages. It would be very good if there a sub division where you could measure all possible types separately. We have the detailed in debugfs, but nothing for the user. A summary in meminfo seems to be the best place for such metric. >>> >>> Let me try to summarize my understanding of the problem, maybe it'll help >>> others as well. >> Thanks! >> >> >>> A device driver allocates memory and exports this memory via dma-buf so >>> that this memory will be accessible for userspace via a file descriptor. >>> >>> The allocated memory can be either allocated with alloc_page() from system >>> RAM or by other means from dedicated VRAM (that is not managed by Linux mm) >>> or even from on-device memory. >>> >>> The dma-buf driver tracks the amount of the memory it was requested to >>> export and the size it sees is available at debugfs and fdinfo. >>> >>> The debugfs is not available to user and maybe entirely disabled in >>> production systems. >>> >>> There could be quite a few open dma-bufs so there is no overall summary, >>> plus fdinfo in production systems your refer to is also unavailable to the >>> user because of selinux policy. >>> >>> And there are a few details that are not clear to me: >>> >>> * Since DRM device drivers seem to be the major user of dma-buf exports, >>> why cannot we add information about their memory consumption to, say, >>> /sys/class/graphics/drm/cardX/memory-usage? >> Android is using it for binder that connect more or less everything >> internally. > > Ok, then it rules out /sys/class/graphics indeed. > >>> * How exactly user generates reports that would include the new counters? >>> From my (mostly outdated) experience Android users won't open a terminal >>> and type 'cat /proc/meminfo' there. I'd presume there is a vendor agent >>> that collects the data and sends it for analysis. In this case what is >>> the reason the vendor is unable to adjust selinix policy so that the >>> agent will be able to access fdinfo? >> When you turn on developer mode on android you can use >> usb with a program called adb. And there you get a normal shell. >> >> (not root though) >> >> There is applications that non developers can use to get >> information. It is very limited though and there are API's >> provide it. >> >> >>> * And, as others already mentioned, it is not clear what are the problems >>> that can be detected by examining DmaBufTotal except saying "oh, there is >>> too much/too little memory exported via dma-buf". What would be user >>> visible effects of these problems? What are the next steps to investigate >>> them? What other data will be probably required to identify root cause? >>> >> When you debug thousands of devices it is quite nice to have >> ways to classify what the problem it is not. The normal user does not >> see anything of this. However they can generate bug-reports that >> collect information about as much they can. Then the user have >> to provide this bug-report to the manufacture or mostly the >> application developer. And when the problem is >> system related we need to reproduce the issue on a full >> debug enabled unit. > So the flow is like this: > > * a user has a problem and reports it to an application developer; at best > the user runs simple and limited app to collect some data > * if the application developer considers this issue as a system related > they can open adb and collect some more information about the system > using non-root shell with selinux policy restrictions and send this > information to the device manufacturer. > * the manufacturer continues to debug the issue and at this point as much > information is possible would have been useful. > > In this flow I still fail to understand why the manufacturer cannot provide > userspace tools that will be able to collect the required information. > These tools not necessarily need to target the end user, they may be only > intended for the application developers, e.g. policy could allow such tool > to access some of t
Re: [PATCH] vfio/gvt: fix DRM_I915_GVT dependency on VFIO_MDEV
Cc: gvt list & maintainers On Thu, 22 Apr 2021, Jason Gunthorpe wrote: > On Thu, Apr 22, 2021 at 03:35:33PM +0200, Arnd Bergmann wrote: >> From: Arnd Bergmann >> >> The Kconfig dependency is incomplete since DRM_I915_GVT is a 'bool' >> symbol that depends on the 'tristate' VFIO_MDEV. This allows a >> configuration with VFIO_MDEV=m, DRM_I915_GVT=y and DRM_I915=y that >> causes a link failure: >> >> x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function >> `available_instances_show': >> gvt.c:(.text+0x67a): undefined reference to `mtype_get_parent_dev' >> x86_64-linux-ld: gvt.c:(.text+0x6a5): undefined reference to >> `mtype_get_type_group_id' >> x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function >> `description_show': >> gvt.c:(.text+0x76e): undefined reference to `mtype_get_parent_dev' >> x86_64-linux-ld: gvt.c:(.text+0x799): undefined reference to >> `mtype_get_type_group_id' >> >> Clarify the dependency by specifically disallowing the broken >> configuration. If VFIO_MDEV is built-in, it will work, but if >> VFIO_MDEV=m, the i915 driver cannot be built-in here. >> >> Fixes: 07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV") >> Fixes: 9169cff168ff ("vfio/mdev: Correct the function signatures for the >> mdev_type_attributes") >> Signed-off-by: Arnd Bergmann >> --- >> drivers/gpu/drm/i915/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Oh kconfig stuff like this makes my head hurt, thanks for finding it > > I also can't see an alternative to this ugly thing, besides having the > i915 guys properly modularize this code someday > > Reviewed-by: Jason Gunthorpe > > Jason -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: remove special handling for non GEM drivers
On 4/22/21 7:59 AM, Christian König wrote: Zack or Roland any objections to this? There shouldn't be any functional change. Sorry, that looks good. I wanted us to add gem support for a while now, this just adds more reasons to do it for next merge window. Reviewed-by: Zack Rusin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: Make preempt timeout for banned contexts configurable
From: Tvrtko Ursulin When we ban a context, for instance when userspace marked itself as non- persistent and has exited, we apply a 1ms grace period after which any belonging workload still on the GPU (did not preempt) will be forcibly terminated (using engine reset). For some workloads period between preemptible points can be longer than this default which results in preempt timeout reset messages, even though the process would actually gracefully exit if given enough time. Coupled with the fact that error messages during normal operation, even if harmless, can be frowned upon, we are left with two options. We could either increase the timeout for everyone, or we could allow it to be configured. Given how rapid response to banned contexts (which can be for many reasons including deliberate denial of service attempts) is preferrable this patch opts for the latter. The default timeout is left at 1ms but can now be configured either at kernel build time via CONFIG_DRM_I915_BANNED_CONTEXT_TIMEOUT, or at runtime using engine sysfs controls, with the added one being named banned_context_timeout_ms. Signed-off-by: Tvrtko Ursulin Cc: Zhen Han --- drivers/gpu/drm/i915/Kconfig.profile | 13 + drivers/gpu/drm/i915/gt/intel_engine_cs.c | 3 ++ drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 + .../drm/i915/gt/intel_execlists_submission.c | 2 +- drivers/gpu/drm/i915/gt/sysfs_engines.c | 54 +++ 5 files changed, 72 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile index 39328567c200..0be56c7084ff 100644 --- a/drivers/gpu/drm/i915/Kconfig.profile +++ b/drivers/gpu/drm/i915/Kconfig.profile @@ -119,3 +119,16 @@ config DRM_I915_TIMESLICE_DURATION /sys/class/drm/card?/engine/*/timeslice_duration_ms May be 0 to disable timeslicing. + +config DRM_I915_BANNED_CONTEXT_TIMEOUT + int "Banned context timeout (ms)" + default 1 # milliseconds + help + How long to wait (in milliseconds) for a banned context to cleanly + terminate their workloads. If the context does not yield inside the + configured time it will be forcibly reset. + + This is adjustable via + /sys/class/drm/card?/engine/*/banned_context_timeout_ms + + If configured to zero a 1ms minimum will still apply. diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 6dbdbde00f14..80b973367db8 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -306,6 +306,8 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id) CONFIG_DRM_I915_STOP_TIMEOUT; engine->props.timeslice_duration_ms = CONFIG_DRM_I915_TIMESLICE_DURATION; + engine->props.banned_context_timeout_ms = + min(1, CONFIG_DRM_I915_BANNED_CONTEXT_TIMEOUT); /* Override to uninterruptible for OpenCL workloads. */ if (INTEL_GEN(i915) == 12 && engine->class == RENDER_CLASS) @@ -1623,6 +1625,7 @@ static void print_properties(struct intel_engine_cs *engine, P(preempt_timeout_ms), P(stop_timeout_ms), P(timeslice_duration_ms), + P(banned_context_timeout_ms), {}, #undef P diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 883bafc44902..ec4f16b0308a 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -549,6 +549,7 @@ struct intel_engine_cs { unsigned long preempt_timeout_ms; unsigned long stop_timeout_ms; unsigned long timeslice_duration_ms; + unsigned long banned_context_timeout_ms; } props, defaults; I915_SELFTEST_DECLARE(struct fault_attr reset_timeout); diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index de124870af44..e2c4e4230979 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -1209,7 +1209,7 @@ static unsigned long active_preempt_timeout(struct intel_engine_cs *engine, /* Force a fast reset for terminated contexts (ignoring sysfs!) */ if (unlikely(intel_context_is_banned(rq->context) || bad_request(rq))) - return 1; + return engine->props.banned_context_timeout_ms; return READ_ONCE(engine->props.preempt_timeout_ms); } diff --git a/drivers/gpu/drm/i915/gt/sysfs_engines.c b/drivers/gpu/drm/i915/gt/sysfs_engines.c index 967031056202..cce9ab8a3309 100644 --- a/drivers/gpu/drm/i915/gt/sysfs_engines.c +++ b/drivers/gpu/drm/i915/gt/sysfs_engines.c @@ -411,6 +411,52 @@ heartbeat_default(struct kobject *kobj, struct kobj_attribute *att
[PULL] drm-intel-fixes
Hi Dave and Daniel, One GVT fix and one display link training fix targeting stable 5.11. Here goes drm-intel-fixes-2021-04-22: - GVT's BDW regression fix for cmd parser (Zhenyu) - Fix modesetting in case of unexpected AUX timeouts (Imre) Thanks, Rodrigo. The following changes since commit bf05bf16c76bb44ab5156223e1e58e26dfe30a88: Linux 5.12-rc8 (2021-04-18 14:45:32 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2021-04-22 for you to fetch changes up to 2d292995bb8f49a2596bef522679c1e1454f3230: Merge tag 'gvt-fixes-2021-04-20' of https://github.com/intel/gvt-linux into drm-intel-fixes (2021-04-20 09:41:32 -0400) - GVT's BDW regression fix for cmd parser (Zhenyu) - Fix modesetting in case of unexpected AUX timeouts (Imre) Imre Deak (1): drm/i915: Fix modesetting in case of unexpected AUX timeouts Rodrigo Vivi (1): Merge tag 'gvt-fixes-2021-04-20' of https://github.com/intel/gvt-linux into drm-intel-fixes Zhenyu Wang (1): drm/i915/gvt: Fix BDW command parser regression drivers/gpu/drm/i915/display/intel_dp_link_training.c | 3 ++- drivers/gpu/drm/i915/gvt/cmd_parser.c | 19 +-- 2 files changed, 15 insertions(+), 7 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_lock_all with drm_modeset_lock
drm_modeset_lock_all() is not needed here, so it is replaced with drm_modeset_lock(). The crtc list around which we are looping never changes, therefore the only lock we need is to protect access to crtc->state. Suggested-by: Daniel Vetter Suggested-by: Matthew Wilcox Signed-off-by: Fabio M. De Francesco --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 671ec1002230..bce8f6793d8e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1439,17 +1439,16 @@ static int amdgpu_pmops_runtime_idle(struct device *dev) if (amdgpu_device_has_dc_support(adev)) { struct drm_crtc *crtc; - drm_modeset_lock_all(drm_dev); - drm_for_each_crtc(crtc, drm_dev) { + drm_modeset_lock(&crtc->mutex, NULL); if (crtc->state->active) { ret = -EBUSY; - break; } + drm_modeset_unlock(&crtc->mutex); + if (ret < 0) + break; } - drm_modeset_unlock_all(drm_dev); - } else { struct drm_connector *list_connector; struct drm_connector_list_iter iter; -- 2.31.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: Invoke BXT _DSM to enable MUX on HP Workstation laptops
On HP Fury G7 Workstations, graphics output is re-routed from Intel GFX to discrete GFX after S3. This is not desirable, because userspace will treat connected display as a new one, losing display settings. The expected behavior is to let discrete GFX drives all external displays. The platform in question uses ACPI method \_SB.PCI0.HGME to enable MUX. The method is inside the BXT _DSM, so add the _DSM and call it accordingly. I also tested some MUX-less and iGPU only laptops with the BXT _DSM, no regression was found. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3113 References: https://lore.kernel.org/intel-gfx/1460040732-31417-4-git-send-email-animesh.ma...@intel.com/ Signed-off-by: Kai-Heng Feng --- drivers/gpu/drm/i915/display/intel_acpi.c | 17 + drivers/gpu/drm/i915/display/intel_acpi.h | 2 ++ drivers/gpu/drm/i915/i915_drv.c | 5 + 3 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c index 833d0c1be4f1..c7b57c22dce3 100644 --- a/drivers/gpu/drm/i915/display/intel_acpi.c +++ b/drivers/gpu/drm/i915/display/intel_acpi.c @@ -14,11 +14,16 @@ #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */ #define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */ +#define INTEL_DSM_FN_PLATFORM_BXT_MUX_INFO 0 /* No args */ static const guid_t intel_dsm_guid = GUID_INIT(0x7ed873d3, 0xc2d0, 0x4e4f, 0xa8, 0x54, 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c); +static const guid_t intel_bxt_dsm_guid = + GUID_INIT(0x3e5b41c6, 0xeb1d, 0x4260, + 0x9d, 0x15, 0xc7, 0x1f, 0xba, 0xda, 0xe4, 0x14); + static char *intel_dsm_port_name(u8 id) { switch (id) { @@ -176,6 +181,18 @@ void intel_unregister_dsm_handler(void) { } +void intel_bxt_dsm_detect(struct pci_dev *pdev) +{ + acpi_handle dhandle; + + dhandle = ACPI_HANDLE(&pdev->dev); + if (!dhandle) + return; + + acpi_evaluate_dsm(dhandle, &intel_bxt_dsm_guid, INTEL_DSM_REVISION_ID, + INTEL_DSM_FN_PLATFORM_BXT_MUX_INFO, NULL); +} + /* * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All Devices * Attached to the Display Adapter). diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h b/drivers/gpu/drm/i915/display/intel_acpi.h index e8b068661d22..0dd456335bd0 100644 --- a/drivers/gpu/drm/i915/display/intel_acpi.h +++ b/drivers/gpu/drm/i915/display/intel_acpi.h @@ -11,10 +11,12 @@ struct drm_i915_private; #ifdef CONFIG_ACPI void intel_register_dsm_handler(void); void intel_unregister_dsm_handler(void); +void intel_bxt_dsm_detect(struct pci_dev *pdev); void intel_acpi_device_id_update(struct drm_i915_private *i915); #else static inline void intel_register_dsm_handler(void) { return; } static inline void intel_unregister_dsm_handler(void) { return; } +static inline void intel_bxt_dsm_detect(struct pci_dev *pdev) { return; } static inline void intel_acpi_device_id_update(struct drm_i915_private *i915) { return; } #endif /* CONFIG_ACPI */ diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 785dcf20c77b..57b12068aab4 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -853,6 +853,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_cleanup_gem; + intel_bxt_dsm_detect(pdev); + i915_driver_register(i915); enable_rpm_wakeref_asserts(&i915->runtime_pm); @@ -1215,6 +1217,7 @@ int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state) static int i915_drm_resume(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); int ret; disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); @@ -1271,6 +1274,8 @@ static int i915_drm_resume(struct drm_device *dev) intel_gvt_resume(dev_priv); + intel_bxt_dsm_detect(pdev); + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); return 0; -- 2.30.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings
On 4/22/21 10:38 AM, Neil Armstrong wrote: [...] + port@1: +type: object +additionalProperties: false + +description: + Video port for LVDS output (panel or bridge). + +properties: + reg: +const: 1 + + endpoint: +type: object +additionalProperties: false +properties: + remote-endpoint: true Similar to Jagan's serie, would be great to add bindings for the dual-link LVDS even if not supported by the driver (the driver can fails with a verbose error). I don't want to add any sort of bindings which I cannot validate against real hardware. I would argue that adding the 2x single / dual link LVDS DT property could be added when someone has a need for it and can test it on real hardware, and such a binding should be simple develop and add. And that is better, because we won't end up with some possibly misdesigned untested DT binding which would become part of the ABI and would have to be supported forever. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings
On 4/22/21 10:43 AM, Jagan Teki wrote: [...] diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml new file mode 100644 index ..42d11b46a1eb --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -0,0 +1,134 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: SN65DSI83 and SN65DSI84 DSI to LVDS bridge chip Can it possible to wait for my v4 to have dual-link LVDS supported which is quite discussing points on previous versions? This driver already has the dual-link LVDS support on DSI84 and it is tested on real hardware. So is the single-link LVDS on DSI83, on multiple hardware instances. In addition to that, this driver has proper regmap support and constant time calculation of DSI83/84 clock register settings. Note that I did wait for your v4 for over two weeks already, it just didn't materialize, so I determined it might be a good idea to restart work on this driver instead. Maybe you can test this driver and check what is missing for your usecase? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next-fixes
Hi Dave, Daniel, Here's this week drm-misc-next-fixes PR, for the next merge window Thanks! Maxime drm-misc-next-fixes-2021-04-22: A few fixes for the next merge window, with some build fixes for anx7625 and lt8912b bridges, incorrect error handling for lt8912b and TTM, and one fix for TTM page limit accounting. The following changes since commit 9c0fed84d5750e1eea6c664e073ffa2534a17743: Merge tag 'drm-intel-next-2021-04-01' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2021-04-08 14:02:21 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2021-04-22 for you to fetch changes up to a4394b6d0a273941a75ebe86a86d6416d536ed0f: drm/ttm: Don't count pages in SG BOs against pages_limit (2021-04-21 15:35:20 +0200) A few fixes for the next merge window, with some build fixes for anx7625 and lt8912b bridges, incorrect error handling for lt8912b and TTM, and one fix for TTM page limit accounting. Adrien Grassein (1): drm/bridge: lt8912b: fix incorrect handling of of_* return values Christian König (1): drm/ttm: fix return value check Felix Kuehling (1): drm/ttm: Don't count pages in SG BOs against pages_limit Randy Dunlap (2): drm: bridge: fix ANX7625 use of mipi_dsi_() functions drm: bridge: fix LONTIUM use of mipi_dsi_() functions drivers/gpu/drm/bridge/Kconfig | 3 +++ drivers/gpu/drm/bridge/analogix/Kconfig | 1 + drivers/gpu/drm/bridge/lontium-lt8912b.c | 32 +--- drivers/gpu/drm/ttm/ttm_tt.c | 29 +++-- 4 files changed, 40 insertions(+), 25 deletions(-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PULL] drm-misc-next-fixes
On Thu, Apr 22, 2021 at 12:33 PM Maxime Ripard wrote: > > Hi Dave, Daniel, > > Here's this week drm-misc-next-fixes PR, for the next merge window > Can we also cherry-pick this patch: https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d510c88cfbb294d2b1e2d0b71576e9b79d0e2e83 It should have really gone into drm-misc-next-fixes rather than drm-misc-next, but I misjudged the timing. Thanks, Alex > Thanks! > Maxime > > drm-misc-next-fixes-2021-04-22: > A few fixes for the next merge window, with some build fixes for anx7625 > and lt8912b bridges, incorrect error handling for lt8912b and TTM, and > one fix for TTM page limit accounting. > The following changes since commit 9c0fed84d5750e1eea6c664e073ffa2534a17743: > > Merge tag 'drm-intel-next-2021-04-01' of > git://anongit.freedesktop.org/drm/drm-intel into drm-next (2021-04-08 > 14:02:21 +1000) > > are available in the Git repository at: > > git://anongit.freedesktop.org/drm/drm-misc > tags/drm-misc-next-fixes-2021-04-22 > > for you to fetch changes up to a4394b6d0a273941a75ebe86a86d6416d536ed0f: > > drm/ttm: Don't count pages in SG BOs against pages_limit (2021-04-21 > 15:35:20 +0200) > > > A few fixes for the next merge window, with some build fixes for anx7625 > and lt8912b bridges, incorrect error handling for lt8912b and TTM, and > one fix for TTM page limit accounting. > > > Adrien Grassein (1): > drm/bridge: lt8912b: fix incorrect handling of of_* return values > > Christian König (1): > drm/ttm: fix return value check > > Felix Kuehling (1): > drm/ttm: Don't count pages in SG BOs against pages_limit > > Randy Dunlap (2): > drm: bridge: fix ANX7625 use of mipi_dsi_() functions > drm: bridge: fix LONTIUM use of mipi_dsi_() functions > > drivers/gpu/drm/bridge/Kconfig | 3 +++ > drivers/gpu/drm/bridge/analogix/Kconfig | 1 + > drivers/gpu/drm/bridge/lontium-lt8912b.c | 32 > +--- > drivers/gpu/drm/ttm/ttm_tt.c | 29 +++-- > 4 files changed, 40 insertions(+), 25 deletions(-) > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_lock_all with drm_modeset_lock
On Thu, Apr 22, 2021 at 05:59:51PM +0200, Fabio M. De Francesco wrote: > - drm_modeset_lock_all(drm_dev); > - > drm_for_each_crtc(crtc, drm_dev) { > + drm_modeset_lock(&crtc->mutex, NULL); > if (crtc->state->active) { > ret = -EBUSY; > - break; > } > + drm_modeset_unlock(&crtc->mutex); > + if (ret < 0) > + break; > } > > - drm_modeset_unlock_all(drm_dev); > - I might remove the {} around ret = -EBUSY, but this is good. Reviewed-by: Matthew Wilcox (Oracle) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] drm/msm/dpu: hw_blk: make dpu_hw_blk empty opaque structure
Hi Dmitry, Thank you for the patch! Yet something to improve: [auto build test ERROR on next-20210422] [also build test ERROR on v5.12-rc8] [cannot apply to linus/master v5.12-rc8 v5.12-rc7 v5.12-rc6] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/drm-msm-dpu-simplify-dpu_hw_blk-handling/20210422-211129 base:c457d9676496f5a895509f9c510278beaaffc829 config: arm-defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/c023ff88d40d423b82b71cd504d787049dcd2046 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Dmitry-Baryshkov/drm-msm-dpu-simplify-dpu_hw_blk-handling/20210422-211129 git checkout c023ff88d40d423b82b71cd504d787049dcd2046 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c: In function 'dpu_encoder_phys_vid_setup_timing_engine': >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c:287:48: error: 'struct >> dpu_hw_blk' has no member named 'id' 287 | intf_cfg.merge_3d = phys_enc->hw_pp->merge_3d->id; |^~ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c: In function 'dpu_encoder_phys_vid_enable': drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c:464:72: error: 'struct dpu_hw_blk' has no member named 'id' 464 | ctl->ops.update_pending_flush_merge_3d(ctl, phys_enc->hw_pp->merge_3d->id); | ^~ vim +287 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 239 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 240 static void dpu_encoder_phys_vid_setup_timing_engine( 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 241struct dpu_encoder_phys *phys_enc) 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 242 { 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 243struct drm_display_mode mode; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 244struct intf_timing_params timing_params = { 0 }; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 245const struct dpu_format *fmt = NULL; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 246u32 fmt_fourcc = DRM_FORMAT_RGB888; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 247unsigned long lock_flags; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 248struct dpu_hw_intf_cfg intf_cfg = { 0 }; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 249 5e7d4a8407d37a Drew Davenport2019-12-06 250if (!phys_enc->hw_ctl->ops.setup_intf_cfg) { 30801221a73781 Zheng Bin 2020-01-23 251 DPU_ERROR("invalid encoder %d\n", phys_enc != NULL); 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 252return; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 253} 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 254 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 255mode = phys_enc->cached_mode; b6057cda8f6cac Jeykumar Sankaran 2019-02-13 256if (!phys_enc->hw_intf->ops.setup_timing_gen) { 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 257 DPU_ERROR("timing engine setup is not supported\n"); 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 258return; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 259} 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 260 b6057cda8f6cac Jeykumar Sankaran 2019-02-13 261 DPU_DEBUG_VIDENC(phys_enc, "enabling mode:\n"); 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 262 drm_mode_debug_printmodeline(&mode); 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 263 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 264if (phys_enc->split_role != ENC_ROLE_SOLO) { 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 265mode.hdisplay >>= 1; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 266mode.htotal >>= 1; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 267 mode.hsync_start >>= 1; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 268mode.hsync_end >>= 1; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-2
Re: [PATCH v3 03/20] drm/dp: Move i2c init to drm_dp_aux_init, add __must_check and fini
On Tue, 2021-04-20 at 02:16 +0300, Ville Syrjälä wrote: > > The init vs. register split is intentional. Registering the thing > and allowing userspace access to it before the rest of the driver > is ready isn't particularly great. For a while now we've tried to > move towards an architecture where the driver is fully initialzied > before anything gets exposed to userspace. Yeah-thank you for pointing this out. Thierry - do you think there's an alternate solution we could go with in Tegra to fix the get_device() issue that wouldn't require us trying to expose the i2c adapter early? > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: fix error handling if no BO can be swapped out v4
On 4/22/21 7:57 AM, Christian König wrote: From: Shiwu Zhang In case that all pre-allocated BOs are busy, just continue to populate BOs since likely half of system memory in total is still free. v4 (chk): fix code moved to VMWGFX as well Signed-off-by: Shiwu Zhang Reviewed-by: Christian König Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_device.c| 2 +- drivers/gpu/drm/ttm/ttm_tt.c| 2 ++ drivers/gpu/drm/vmwgfx/ttm_memory.c | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 1f2024164d72..5dc92b056f0a 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -112,7 +112,7 @@ int ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { struct ttm_global *glob = &ttm_glob; struct ttm_device *bdev; - int ret = -EBUSY; + int ret = 0; Looks good to me. Updating the comment above that function to note it returns either tge number of pages swapped out or an error might be a good idea because it's not obvious from the name. Reviewed-by: Zack Rusin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/drm_file.c: Define drm_send_event_helper() as 'static'
drm_send_event_helper() has not prototype, it has internal linkage and therefore it should be defined with storage class 'static'. Signed-off-by: Fabio M. De Francesco --- drivers/gpu/drm/drm_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 7efbccffc2ea..17f38d873972 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -786,7 +786,7 @@ EXPORT_SYMBOL(drm_event_cancel_free); * The timestamp variant of dma_fence_signal is used when the caller * sends a valid timestamp. */ -void drm_send_event_helper(struct drm_device *dev, +static void drm_send_event_helper(struct drm_device *dev, struct drm_pending_event *e, ktime_t timestamp) { assert_spin_locked(&dev->event_lock); -- 2.31.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/1] Document drm_mode_get_plane
This is a follow up of the patchset "Document how userspace should use plane format list and IN_FORMATS": https://lists.freedesktop.org/archives/dri-devel/2021-April/302433.html The first patch of the series ("drm/doc: document drm_mode_get_plane") is still useful, although the other commit of the series was incorrect. So I'm pushing the first commit again. v2: possible_crtcs field is a bitmask, not a pointer. Suggested by Ville Syrjälä Leandro Ribeiro (1): drm/doc: document drm_mode_get_plane include/uapi/drm/drm_mode.h | 16 1 file changed, 16 insertions(+) -- 2.31.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/1] drm/doc: document drm_mode_get_plane
Add a small description and document struct fields of drm_mode_get_plane. Signed-off-by: Leandro Ribeiro --- include/uapi/drm/drm_mode.h | 16 1 file changed, 16 insertions(+) diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index a5e76aa06ad5..3e85de928db9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -312,16 +312,32 @@ struct drm_mode_set_plane { __u32 src_w; }; +/** + * struct drm_mode_get_plane - Get plane metadata. + * + * Userspace can perform a GETPLANE ioctl to retrieve information about a + * plane. + */ struct drm_mode_get_plane { + /** @plane_id: Object ID of the plane. */ __u32 plane_id; + /** @crtc_id: Object ID of the current CRTC. */ __u32 crtc_id; + /** @fb_id: Object ID of the current fb. */ __u32 fb_id; + /** @possible_crtcs: Bitmask of CRTC's compatible with the plane. */ __u32 possible_crtcs; + /** @gamma_size: Size of the legacy gamma table. */ __u32 gamma_size; + /** @count_format_types: Number of formats. */ __u32 count_format_types; + /** +* @format_type_ptr: Pointer to ``__u32`` array of formats that are +* supported by the plane. These formats do not require modifiers. +*/ __u64 format_type_ptr; }; -- 2.31.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_lock_all with drm_modeset_lock
drm_modeset_lock_all() is not needed here, so it is replaced with drm_modeset_lock(). The crtc list around which we are looping never changes, therefore the only lock we need is to protect access to crtc->state. Suggested-by: Daniel Vetter Suggested-by: Matthew Wilcox Signed-off-by: Fabio M. De Francesco Reviewed-by: Matthew Wilcox (Oracle) --- Changes from v1: Removed unnecessary braces around single statement block. drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 671ec1002230..adfeec2b17c0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1439,17 +1439,15 @@ static int amdgpu_pmops_runtime_idle(struct device *dev) if (amdgpu_device_has_dc_support(adev)) { struct drm_crtc *crtc; - drm_modeset_lock_all(drm_dev); - drm_for_each_crtc(crtc, drm_dev) { - if (crtc->state->active) { + drm_modeset_lock(&crtc->mutex, NULL); + if (crtc->state->active) ret = -EBUSY; + drm_modeset_unlock(&crtc->mutex); + if (ret < 0) break; - } } - drm_modeset_unlock_all(drm_dev); - } else { struct drm_connector *list_connector; struct drm_connector_list_iter iter; -- 2.31.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Rewrite CL/CTG L-shaped memory detection
On Thu, Apr 22, 2021 at 11:49:43AM +0200, Daniel Vetter wrote: > On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Currently we try to detect a symmetric memory configurations > > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > > either only set on a very specific subset of machines or it > > just does not exist (it's not mentioned in any public chipset > > datasheets I've found). As it happens my CL/CTG machines never > > set said bit, even if I populate the channels with identical > > sticks. > > > > So let's do the L-shaped memory detection the same way as the > > desktop variants, ie. just look at the DRAM rank boundary > > registers to see if both channels have an identical size. > > > > With this my CL/CTG no longer claim L-shaped memory when I use > > identical sticks. Also tested with non-matching sticks just to > > make sure the L-shaped memory is still properly detected. > > > > And for completeness let's update the debugfs code to dump > > the correct set of registers on each platform. > > > > Cc: Chris Wilson > > Signed-off-by: Ville Syrjälä > > Did you check this with the swapping igt? I have some vague memories of > bug reports where somehow the machine was acting like it's L-shaped memory > despite that banks were populated equally. I've iirc tried all kinds of > tricks to figure it out, all to absolutely no avail. BTW looking at the patches/dumps in eg. https://bugs.freedesktop.org/show_bug.cgi?id=28813 I can't immediately see a single thing that is actually using the correct register offsets for cl/ctg. So I'm a bit sceptical about how well this was researched in the past. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amdkfd: fix uint32 variable compared to less than zero
Am 2021-04-22 um 8:31 a.m. schrieb Colin King: > From: Colin Ian King > > Currently the call to kfd_process_gpuidx_from_gpuid is returning an > int value and this is being assigned to a uint32_t variable gpuidx > and this is being checked for a negative error return which is always > going to be false. Fix this by making gpuidx an int32_t. This makes > gpuidx also type consistent with the use of gpuidx from the callers. > > Addresses-Coverity: ("Unsigned compared against 0") > Fixes: cda0f85bfa5e ("drm/amdkfd: refine migration policy with xnack on") > Signed-off-by: Colin Ian King Applied to amd-staging-drm-next. Thanks, Felix > --- > drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c > b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c > index 0e0b4ffd20ab..bf3c8de85b4a 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c > @@ -1330,7 +1330,7 @@ static void svm_range_unreserve_bos(struct > svm_validate_context *ctx) > */ > static int svm_range_validate_and_map(struct mm_struct *mm, > struct svm_range *prange, > - uint32_t gpuidx, bool intr, bool wait) > + int32_t gpuidx, bool intr, bool wait) > { > struct svm_validate_context ctx; > struct hmm_range *hmm_range; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amdkfd: remove redundant initialization to variable r
Am 2021-04-22 um 8:44 a.m. schrieb Colin King: > From: Colin Ian King > > The variable r is being initialized with a value that is never read > and it is being updated later with a new value. The initialization is > redundant and can be removed. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King Applied to amd-staging-drm-next. Thanks, Felix > --- > drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > index d44a46eb00d6..a66b67083d83 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > @@ -303,7 +303,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, > struct svm_range *prange, > uint64_t vram_addr; > uint64_t offset; > uint64_t i, j; > - int r = -ENOMEM; > + int r; > > pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms, prange->start, >prange->last); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom
Hi, On Mon, Mar 29, 2021 at 9:25 AM Doug Anderson wrote: > > Hi, > > On Thu, Mar 25, 2021 at 5:09 PM Rob Herring wrote: > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > pile of eDP panels. At the moment I'm told that the list might include: > > > - KD KD116N21-30NV-A010 > > > - KD KD116N09-30NH-A016 > > > - Starry 2081116HHD028001-51D > > > - Sharp LQ116M1JW10 > > > > > > It should be noted that while the EDID programmed in the first 3 > > > panels indicates that they should run with exactly the same timing (to > > > keep things simple), the 4th panel not only needs different timing but > > > has a different resolution. > > > > > > As is true in general with eDP panels, we can figure out which panel > > > we have and all the info needed to drive its pixel clock by reading > > > the EDID. However, we can do this only after we've powered the panel > > > on. Powering on the panels requires following the timing diagram in > > > each panel's datasheet which specifies delays between certain > > > actions. This means that, while we can be quite dynamic about handling > > > things we can't just totally skip out on describing the panel like we > > > could do if it was connected to an external-facing DP port. > > > > Is this a 'standard' eDP connector? AFAICT, there does seem to be > > such a thing. > > To answer this one: there's not any "standard" physical plug as far as > I can tell. There's a connector on the board side for the LCD that has > a whole hodgepodge of signals on it. Maybe USB for a camera. Some > power signals. Maybe a PWM for a backlight. Maybe some DMIC signals. > eDP signals which might be anywhere from 1 to 4 lanes. HPD (which is > really a "panel ready" signal for eDP). The size / style of connector > and the exact set of signals (and their ordering) is board specific. > You then get a board-specific cable that splits things out. Some might > go to a camera/MIC sub board. Some go to the panel and hook onto a > panel-specific connector which has pin count and orderings defined by > that panel. :-P > > > > I've said in the past I'd be okay with a edp-connector > > node. If that needs just the "HPD absent delay" property, I think that > > would be okay. It's just a never ending stream of new properties with > > each new panel that I don't want to see. > > Thinking about this we'd need at least one other property right now > which is an enable delay. Specifically at least one panel I've > supported recently lied about HPD for a short period after bootup. > Specifically see commit 667d73d72f31 ("drm: panel: simple: Delay HPD > checking on boe_nv133fhm_n61 for 15 ms"). ...and, of course, the > existing power supply / enable signals that "simple-panel" already > has. > > Also: if we weren't going to add the other delay properties in the > device tree, we'd have to add the code right away that used the EDID > to set other delays. That wouldn't be the end of the world, but it > would be code to write. > > > One last thought to add: I've looked at ~10 panels specs recently. > Though they are all a little different from each other, I will say > that almost every one of them seems to have the exact same timing > diagram in it just with different numbers filled in. To me that backs > up the idea that you can/should do the power sequence with a fairly > standard (parameterized) driver. I can't link the datasheets I have > but searching for "edp panel datasheet" finds me this random > datasheet: > > https://www.data-modul.com/sites/default/files/products/NV156QUM-N72_specification_12039472.pdf > > See "8.0 POWER SEQUENCE" in that document. All the panels have a > nearly identical diagram with different numbers filled in. You can > kinda tell it was copied from some other panel since some numbers > (like T4) aren't even defined. So this thread has been quiet for a while, but the problem still exists. Here's my current plan, but please yell if you disagree: 1. See about adding a generic "eDP connector" node. Having stewed on this for a while I think I'm convinced that even though there's not really a single standard physical connector that is used everywhere that there are at least a set of signals that can be collectively thought about as the "eDP signals". Certainly I have a set of very different panels from very different manufacturers that I can "interchange" and they work fine assuming I have the right cable "adapting" them from the connector on my board to the connector on the panel. While different panels have different timings that they care are enforced, there is a way to express it in a relatively common way as evidenced by the fact that all panel datasheet timing diagrams look similar and the fact that panel-simple handles so many different panels (yes, we periodically add more timing constraints to handle there but mostly that's because the code wasn't able to handle every constraint that
Re: [PATCH v3 03/20] drm/dp: Move i2c init to drm_dp_aux_init, add __must_check and fini
OK - talked with Ville a bit on this and did some of my own research, I actually think that moving i2c to drm_dp_aux_init() is the right decision for the time being. The reasoning behind this being that as shown by my previous work of fixing drivers that call drm_dp_aux_register() too early - it seems like there's already been drivers that have been working just fine with setting up the i2c device before DRM registration. In the future, it'd probably be better if we can split up i2c_add_adapter() into an init and register function - but we'll have to talk with the i2c maintainers to see if this is acceptable w/ them On Thu, 2021-04-22 at 13:18 -0400, Lyude Paul wrote: > On Tue, 2021-04-20 at 02:16 +0300, Ville Syrjälä wrote: > > > > The init vs. register split is intentional. Registering the thing > > and allowing userspace access to it before the rest of the driver > > is ready isn't particularly great. For a while now we've tried to > > move towards an architecture where the driver is fully initialzied > > before anything gets exposed to userspace. > > Yeah-thank you for pointing this out. Thierry - do you think there's an > alternate solution we could go with in Tegra to fix the get_device() issue > that wouldn't require us trying to expose the i2c adapter early? > > > > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 1/4] mfd: rt4831: Adds support for Richtek RT4831
Hi, Lee: ChiYuan Huang 於 2021年4月19日 週一 下午5:55寫道: > > Lee Jones 於 2021年4月19日 週一 下午3:24寫道: > > > > On Mon, 19 Apr 2021, Lee Jones wrote: > > > > > On Mon, 19 Apr 2021, Lee Jones wrote: > > > > > > > On Mon, 19 Apr 2021, ChiYuan Huang wrote: > > > > > > > > > Hi, Linux mfd reviewers: > > > > >It's been three weeks not to get any response from you. > > > > > Is there something wrong about this mfd patch? > > > > > If yes, please feel free to let me know. > > > > > > > > Couple of things: > > > > > > > > First, if you think a patch had fallen through the gaps, which does > > > > happen sometimes, it is generally considered acceptable to submit a > > > > [RESEND] ~2 weeks after the initial submission. FYI: This was such a > > > > patch. It was not on, or had fallen off of my radar for some reason. > > > > > > > > Secondly, we are really late in the release cycle. -rc8 has just been > > > > released. Quite a few maintainers slow down at ~-rc6. Particularly > > > > for new drivers. > > > > > > > > No need to resubmit this driver this time. It is now on my to-review > > > > list and I will tend to it shortly. > > > > > > > > Thanks for your patience. > > > > > > Also you are missing a DT review on patch 4. > > > > ... looks like you forgot to Cc them! > > > Yap, really. I''ll resend patch 4 and cc them. Thx. Should I resend the patch and loop DT reviewers? > > -- > > Lee Jones [李琼斯] > > Senior Technical Lead - Developer Services > > Linaro.org │ Open source software for Arm SoCs > > Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mediatek: clear pending flag when cmdq packet is done.
Hi, Hsin-yi: On Thu, 2021-04-22 at 19:10 +0800, Hsin-Yi Wang wrote: > From: CK Hu > > In cmdq mode, packet may be flushed before it is executed, so > the pending flag should be cleared after cmdq packet is done. > > Signed-off-by: CK Hu > Signed-off-by: Hsin-Yi Wang > --- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 56 +--- > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > 2 files changed, 51 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index 40df2c823187..051bf0eb00d3 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -224,6 +224,45 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct > drm_crtc *crtc, > #if IS_REACHABLE(CONFIG_MTK_CMDQ) > static void ddp_cmdq_cb(struct cmdq_cb_data data) > { > + struct cmdq_pkt *pkt = (struct cmdq_pkt *)data.data; > + struct mtk_drm_crtc *mtk_crtc = (struct mtk_drm_crtc *)pkt->crtc; > + struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state); > + unsigned int i; > + > + if (data.sta == CMDQ_CB_ERROR) I prefer use standard error status instead of proprietary one, so I send a patch [1]. I would like this patch depend on [1]. [1] https://patchwork.kernel.org/project/linux-mediatek/patch/20210314233323.23377-2-chunkuang...@kernel.org/ > + goto destroy_pkt; > + > + if (state->pending_config) { > + state->pending_config = false; > + } > + > + if (mtk_crtc->pending_planes) { > + for (i = 0; i < mtk_crtc->layer_nr; i++) { > + struct drm_plane *plane = &mtk_crtc->planes[i]; > + struct mtk_plane_state *plane_state; > + > + plane_state = to_mtk_plane_state(plane->state); > + > + if (plane_state->pending.config) > + plane_state->pending.config = false; > + } > + mtk_crtc->pending_planes = false; > + } > + > + if (mtk_crtc->pending_async_planes) { > + for (i = 0; i < mtk_crtc->layer_nr; i++) { > + struct drm_plane *plane = &mtk_crtc->planes[i]; > + struct mtk_plane_state *plane_state; > + > + plane_state = to_mtk_plane_state(plane->state); > + > + if (plane_state->pending.async_config) > + plane_state->pending.async_config = false; > + } > + mtk_crtc->pending_async_planes = false; > + } > + > +destroy_pkt: > cmdq_pkt_destroy(data.data); > } > #endif > @@ -377,8 +416,8 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc, > state->pending_height, > state->pending_vrefresh, 0, > cmdq_handle); > - > - state->pending_config = false; > + if (!cmdq_handle) > + state->pending_config = false; > } > > if (mtk_crtc->pending_planes) { > @@ -398,9 +437,11 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc, > mtk_ddp_comp_layer_config(comp, local_layer, > plane_state, > cmdq_handle); > - plane_state->pending.config = false; > + if (!cmdq_handle) > + plane_state->pending.config = false; > } > - mtk_crtc->pending_planes = false; > + if (!cmdq_handle) > + mtk_crtc->pending_planes = false; > } > > if (mtk_crtc->pending_async_planes) { > @@ -420,9 +461,11 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc, > mtk_ddp_comp_layer_config(comp, local_layer, > plane_state, > cmdq_handle); > - plane_state->pending.async_config = false; > + if (!cmdq_handle) > + plane_state->pending.async_config = false; > } > - mtk_crtc->pending_async_planes = false; > + if (!cmdq_handle) > + mtk_crtc->pending_async_planes = false; > } > } > > @@ -475,6 +518,7 @@ static void mtk_drm_crtc_update_config(struct > mtk_drm_crtc *mtk_crtc, > cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false); > mtk_crtc_ddp_config(crtc, cmdq_handle); > cmdq_pkt_finalize(cmdq_handle); > + cmdq_handle->crtc = mtk_crtc; > cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle); > } > #endif > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h > b/include/linux/mailbox/mtk-cmdq-mailbox.
Re: [PATCH v2 04/10] drm/amdgpu: Simplify AQL queue mapping
Regards, Oak On 2021-04-21, 9:31 PM, "amd-gfx on behalf of Felix Kuehling" wrote: Do AQL queue double-mapping with a single attach call. That will make it easier to create per-GPU BOs later, to be shared between the two BO VA mappings on the same GPU. Freeing the attachments is not necessary if map_to_gpu fails. These will be cleaned up when the kdg_mem object is destroyed in amdgpu_amdkfd_gpuvm_free_memory_of_gpu. Signed-off-by: Felix Kuehling --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 103 -- 1 file changed, 48 insertions(+), 55 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 34c9a2d0028e..fbd7e786b54e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -486,70 +486,76 @@ static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem) * 4a. Validate new page tables and directories */ static int kfd_mem_attach(struct amdgpu_device *adev, struct kgd_mem *mem, - struct amdgpu_vm *vm, bool is_aql, - struct kfd_mem_attachment **p_attachment) + struct amdgpu_vm *vm, bool is_aql) { unsigned long bo_size = mem->bo->tbo.base.size; uint64_t va = mem->va; - struct kfd_mem_attachment *attachment; - struct amdgpu_bo *bo; - int ret; + struct kfd_mem_attachment *attachment[2] = {NULL, NULL}; + struct amdgpu_bo *bo[2] = {NULL, NULL}; + int i, ret; if (!va) { pr_err("Invalid VA when adding BO to VM\n"); return -EINVAL; } - if (is_aql) - va += bo_size; - - attachment = kzalloc(sizeof(*attachment), GFP_KERNEL); - if (!attachment) - return -ENOMEM; + for (i = 0; i <= is_aql; i++) { + attachment[i] = kzalloc(sizeof(*attachment[i]), GFP_KERNEL); + if (unlikely(!attachment[i])) { + ret = -ENOMEM; + goto unwind; + } - pr_debug("\t add VA 0x%llx - 0x%llx to vm %p\n", va, - va + bo_size, vm); + pr_debug("\t add VA 0x%llx - 0x%llx to vm %p\n", va, +va + bo_size, vm); - /* FIXME: For now all attachments use the same BO. This is incorrect -* because one BO can only have one DMA mapping for one GPU. We need -* one BO per GPU, e.g. a DMABuf import with dynamic attachment. This -* will be addressed one BO-type at a time in subsequent patches. -*/ - bo = mem->bo; - drm_gem_object_get(&bo->tbo.base); + /* FIXME: For now all attachments use the same BO. This is +* incorrect because one BO can only have one DMA mapping +* for one GPU. We need one BO per GPU, e.g. a DMABuf +* import with dynamic attachment. This will be addressed +* one BO-type at a time in subsequent patches. +*/ + bo[i] = mem->bo; + drm_gem_object_get(&bo[i]->tbo.base); - /* Add BO to VM internal data structures*/ - attachment->bo_va = amdgpu_vm_bo_add(adev, vm, bo); - if (!attachment->bo_va) { - ret = -EINVAL; - pr_err("Failed to add BO object to VM. ret == %d\n", - ret); - goto err_vmadd; - } + /* Add BO to VM internal data structures */ + attachment[i]->bo_va = amdgpu_vm_bo_add(adev, vm, bo[i]); Just for discussion. Are we allowed to add one bo twice to a vm? When I looked at amdgpu_vm_bo_base_init (called by amdgpu_vm_bo_add), line: bo->vm_bo = base; when you add the same bo to vm the second time, bo->vm_bo will be overwritten. I am not sure whether this will cause an issue later. This is not introduced by your code. The original code (calling kfd_mem_attach twice for aql) has the same problem. + if (unlikely(!attachment[i]->bo_va)) { + ret = -ENOMEM; + pr_err("Failed to add BO object to VM. ret == %d\n", + ret); + goto unwind; + } - attachment->va = va; - attachment->pte_flags = get_pte_flags(adev, mem); - attachment->adev = adev; - list_add(&attachment->list, &mem->attachments); + attachment[i]->va = va; + attachment[i]->pte_flags = get_pte_flags(adev, mem); + attachment[i]->adev = adev; + list_add(&attachment[i]->list, &mem->attachments); - if (p_attachment) - *p_attachment = attachment; + va += bo_size; + } /* Allocate validate page tables if needed */ ret = vm_validate_pt_pd_b
Re: [PATCH v5 16/16] of: Add plumbing for restricted DMA pool
On Thu, Apr 22, 2021 at 4:17 PM Claire Chang wrote: > > If a device is not behind an IOMMU, we look up the device node and set > up the restricted DMA when the restricted-dma-pool is presented. > > Signed-off-by: Claire Chang > --- > drivers/of/address.c| 25 + > drivers/of/device.c | 3 +++ > drivers/of/of_private.h | 5 + > 3 files changed, 33 insertions(+) > > diff --git a/drivers/of/address.c b/drivers/of/address.c > index 54f221dde267..fff3adfe4986 100644 > --- a/drivers/of/address.c > +++ b/drivers/of/address.c > @@ -8,6 +8,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -1109,6 +1110,30 @@ bool of_dma_is_coherent(struct device_node *np) > } > EXPORT_SYMBOL_GPL(of_dma_is_coherent); > > +int of_dma_set_restricted_buffer(struct device *dev) > +{ > + struct device_node *node; > + int count, i; > + > + if (!dev->of_node) > + return 0; > + > + count = of_property_count_elems_of_size(dev->of_node, "memory-region", > + sizeof(phandle)); > + for (i = 0; i < count; i++) { > + node = of_parse_phandle(dev->of_node, "memory-region", i); > + /* There might be multiple memory regions, but only one > +* restriced-dma-pool region is allowed. > +*/ > + if (of_device_is_compatible(node, "restricted-dma-pool") && > + of_device_is_available(node)) > + return of_reserved_mem_device_init_by_idx( > + dev, dev->of_node, i); > + } > + > + return 0; > +} > + > /** > * of_mmio_is_nonposted - Check if device uses non-posted MMIO > * @np:device node > diff --git a/drivers/of/device.c b/drivers/of/device.c > index c5a9473a5fb1..d8d865223e51 100644 > --- a/drivers/of/device.c > +++ b/drivers/of/device.c > @@ -165,6 +165,9 @@ int of_dma_configure_id(struct device *dev, struct > device_node *np, > > arch_setup_dma_ops(dev, dma_start, size, iommu, coherent); > > + if (!iommu) > + return of_dma_set_restricted_buffer(dev); > + > return 0; > } > EXPORT_SYMBOL_GPL(of_dma_configure_id); > diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h > index d717efbd637d..e9237f5eff48 100644 > --- a/drivers/of/of_private.h > +++ b/drivers/of/of_private.h > @@ -163,12 +163,17 @@ struct bus_dma_region; > #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_HAS_DMA) > int of_dma_get_range(struct device_node *np, > const struct bus_dma_region **map); > +int of_dma_set_restricted_buffer(struct device *dev); > #else > static inline int of_dma_get_range(struct device_node *np, > const struct bus_dma_region **map) > { > return -ENODEV; > } > +static inline int of_dma_get_restricted_buffer(struct device *dev) This one should be of_dma_set_restricted_buffer. Sorry for the typo. > +{ > + return -ENODEV; > +} > #endif > > #endif /* _LINUX_OF_PRIVATE_H */ > -- > 2.31.1.368.gbe11c130af-goog > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] SPI: meson-spifc add missed calls to remove function
Problem: rmmod meson_gx_mmc - not stable without spi_master_suspend call and we can get stuck when try unload this module rmmod meson_gx_mmc ... [ 421.108614] Deleting MTD partitions on "spi0.0": [ 424.219862] spi_master spi0: Failed to power device: -13 ... lsmod | grep spi spi_meson_spifc16384 -1 Solution: just add spi_master_suspend(master) call Signed-off-by: Artem Lapkin --- drivers/spi/spi-meson-spifc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/spi/spi-meson-spifc.c b/drivers/spi/spi-meson-spifc.c index 8eca6f24c..8a97a6dbf 100644 --- a/drivers/spi/spi-meson-spifc.c +++ b/drivers/spi/spi-meson-spifc.c @@ -359,6 +359,7 @@ static int meson_spifc_remove(struct platform_device *pdev) struct spi_master *master = platform_get_drvdata(pdev); struct meson_spifc *spifc = spi_master_get_devdata(master); + spi_master_suspend(master); pm_runtime_get_sync(&pdev->dev); clk_disable_unprepare(spifc->clk); pm_runtime_disable(&pdev->dev); -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm fixes for 5.12 final
Hi Linus, Just some small i915 and amdgpu fixes this week, Should be all until you open the merge window. Regards, Dave. drm-fixes-2021-04-23: drm fixes for 5.12 final amdgpu: - Fix gpuvm page table update issue - Modifier fixes - Register fix for dimgrey cavefish i915: - GVT's BDW regression fix for cmd parser - Fix modesetting in case of unexpected AUX timeouts The following changes since commit bf05bf16c76bb44ab5156223e1e58e26dfe30a88: Linux 5.12-rc8 (2021-04-18 14:45:32 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-04-23 for you to fetch changes up to aca38735ae624b93c71c055b68d5802b8f356ea5: Merge tag 'drm-intel-fixes-2021-04-22' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2021-04-23 12:18:21 +1000) drm fixes for 5.12 final amdgpu: - Fix gpuvm page table update issue - Modifier fixes - Register fix for dimgrey cavefish i915: - GVT's BDW regression fix for cmd parser - Fix modesetting in case of unexpected AUX timeouts Dave Airlie (2): Merge tag 'amd-drm-fixes-5.12-2021-04-21' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Merge tag 'drm-intel-fixes-2021-04-22' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Imre Deak (1): drm/i915: Fix modesetting in case of unexpected AUX timeouts Jiansong Chen (1): drm/amdgpu: fix GCR_GENERAL_CNTL offset for dimgrey_cavefish Philip Yang (1): drm/amdgpu: reserve fence slot to update page table Qingqing Zhuo (1): drm/amd/display: Update modifier list for gfx10_3 Rodrigo Vivi (1): Merge tag 'gvt-fixes-2021-04-20' of https://github.com/intel/gvt-linux into drm-intel-fixes Simon Ser (1): amd/display: allow non-linear multi-planar formats Zhenyu Wang (1): drm/i915/gvt: Fix BDW command parser regression drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 10 -- drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c| 2 +- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 ++- drivers/gpu/drm/i915/display/intel_dp_link_training.c | 3 ++- drivers/gpu/drm/i915/gvt/cmd_parser.c | 19 +-- 5 files changed, 30 insertions(+), 19 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 03/20] drm/dp: Move i2c init to drm_dp_aux_init, add __must_check and fini
On Thu, 2021-04-22 at 18:33 -0400, Lyude Paul wrote: > OK - talked with Ville a bit on this and did some of my own research, I > actually think that moving i2c to drm_dp_aux_init() is the right decision > for > the time being. The reasoning behind this being that as shown by my previous > work of fixing drivers that call drm_dp_aux_register() too early - it seems > like there's already been drivers that have been working just fine with > setting up the i2c device before DRM registration. > > In the future, it'd probably be better if we can split up i2c_add_adapter() > into an init and register function - but we'll have to talk with the i2c > maintainers to see if this is acceptable w/ them Actually - I think adding the ability to refcount dp aux adapters might be a better solution so I'm going to try that! > > On Thu, 2021-04-22 at 13:18 -0400, Lyude Paul wrote: > > On Tue, 2021-04-20 at 02:16 +0300, Ville Syrjälä wrote: > > > > > > The init vs. register split is intentional. Registering the thing > > > and allowing userspace access to it before the rest of the driver > > > is ready isn't particularly great. For a while now we've tried to > > > move towards an architecture where the driver is fully initialzied > > > before anything gets exposed to userspace. > > > > Yeah-thank you for pointing this out. Thierry - do you think there's an > > alternate solution we could go with in Tegra to fix the get_device() issue > > that wouldn't require us trying to expose the i2c adapter early? > > > > > > > > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vfio/gvt: fix DRM_I915_GVT dependency on VFIO_MDEV
On 2021.04.22 10:58:10 -0300, Jason Gunthorpe wrote: > On Thu, Apr 22, 2021 at 03:35:33PM +0200, Arnd Bergmann wrote: > > From: Arnd Bergmann > > > > The Kconfig dependency is incomplete since DRM_I915_GVT is a 'bool' > > symbol that depends on the 'tristate' VFIO_MDEV. This allows a > > configuration with VFIO_MDEV=m, DRM_I915_GVT=y and DRM_I915=y that > > causes a link failure: > > > > x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function > > `available_instances_show': > > gvt.c:(.text+0x67a): undefined reference to `mtype_get_parent_dev' > > x86_64-linux-ld: gvt.c:(.text+0x6a5): undefined reference to > > `mtype_get_type_group_id' > > x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function > > `description_show': > > gvt.c:(.text+0x76e): undefined reference to `mtype_get_parent_dev' > > x86_64-linux-ld: gvt.c:(.text+0x799): undefined reference to > > `mtype_get_type_group_id' > > > > Clarify the dependency by specifically disallowing the broken > > configuration. If VFIO_MDEV is built-in, it will work, but if > > VFIO_MDEV=m, the i915 driver cannot be built-in here. > > > > Fixes: 07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV") > > Fixes: 9169cff168ff ("vfio/mdev: Correct the function signatures for the > > mdev_type_attributes") > > Signed-off-by: Arnd Bergmann > > --- > > drivers/gpu/drm/i915/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Oh kconfig stuff like this makes my head hurt, thanks for finding it > > I also can't see an alternative to this ugly thing, besides having the > i915 guys properly modularize this code someday > > Reviewed-by: Jason Gunthorpe > I don't really want this mess to propagate further. We should move mdev related stuff to kvmgt module instead, so not pretend any more to possibly use that for other hypervisor.. Sorry that I didn't realize this issue when Jason proposed this. Let me do the left cleanup. Thanks signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/i915: Invoke BXT _DSM to enable MUX on HP Workstation laptops
On HP Fury G7 Workstations, graphics output is re-routed from Intel GFX to discrete GFX after S3. This is not desirable, because userspace will treat connected display as a new one, losing display settings. The expected behavior is to let discrete GFX drives all external displays. The platform in question uses ACPI method \_SB.PCI0.HGME to enable MUX. The method is inside the BXT _DSM, so add the _DSM and call it accordingly. I also tested some MUX-less and iGPU only laptops with the BXT _DSM, no regression was found. v2: - Forward declare struct pci_dev. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3113 References: https://lore.kernel.org/intel-gfx/1460040732-31417-4-git-send-email-animesh.ma...@intel.com/ Signed-off-by: Kai-Heng Feng --- drivers/gpu/drm/i915/display/intel_acpi.c | 17 + drivers/gpu/drm/i915/display/intel_acpi.h | 3 +++ drivers/gpu/drm/i915/i915_drv.c | 5 + 3 files changed, 25 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c index 833d0c1be4f1..c7b57c22dce3 100644 --- a/drivers/gpu/drm/i915/display/intel_acpi.c +++ b/drivers/gpu/drm/i915/display/intel_acpi.c @@ -14,11 +14,16 @@ #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */ #define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */ +#define INTEL_DSM_FN_PLATFORM_BXT_MUX_INFO 0 /* No args */ static const guid_t intel_dsm_guid = GUID_INIT(0x7ed873d3, 0xc2d0, 0x4e4f, 0xa8, 0x54, 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c); +static const guid_t intel_bxt_dsm_guid = + GUID_INIT(0x3e5b41c6, 0xeb1d, 0x4260, + 0x9d, 0x15, 0xc7, 0x1f, 0xba, 0xda, 0xe4, 0x14); + static char *intel_dsm_port_name(u8 id) { switch (id) { @@ -176,6 +181,18 @@ void intel_unregister_dsm_handler(void) { } +void intel_bxt_dsm_detect(struct pci_dev *pdev) +{ + acpi_handle dhandle; + + dhandle = ACPI_HANDLE(&pdev->dev); + if (!dhandle) + return; + + acpi_evaluate_dsm(dhandle, &intel_bxt_dsm_guid, INTEL_DSM_REVISION_ID, + INTEL_DSM_FN_PLATFORM_BXT_MUX_INFO, NULL); +} + /* * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All Devices * Attached to the Display Adapter). diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h b/drivers/gpu/drm/i915/display/intel_acpi.h index e8b068661d22..d2d560d63bb3 100644 --- a/drivers/gpu/drm/i915/display/intel_acpi.h +++ b/drivers/gpu/drm/i915/display/intel_acpi.h @@ -6,15 +6,18 @@ #ifndef __INTEL_ACPI_H__ #define __INTEL_ACPI_H__ +struct pci_dev; struct drm_i915_private; #ifdef CONFIG_ACPI void intel_register_dsm_handler(void); void intel_unregister_dsm_handler(void); +void intel_bxt_dsm_detect(struct pci_dev *pdev); void intel_acpi_device_id_update(struct drm_i915_private *i915); #else static inline void intel_register_dsm_handler(void) { return; } static inline void intel_unregister_dsm_handler(void) { return; } +static inline void intel_bxt_dsm_detect(struct pci_dev *pdev) { return; } static inline void intel_acpi_device_id_update(struct drm_i915_private *i915) { return; } #endif /* CONFIG_ACPI */ diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 785dcf20c77b..57b12068aab4 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -853,6 +853,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto out_cleanup_gem; + intel_bxt_dsm_detect(pdev); + i915_driver_register(i915); enable_rpm_wakeref_asserts(&i915->runtime_pm); @@ -1215,6 +1217,7 @@ int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state) static int i915_drm_resume(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); int ret; disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); @@ -1271,6 +1274,8 @@ static int i915_drm_resume(struct drm_device *dev) intel_gvt_resume(dev_priv); + intel_bxt_dsm_detect(pdev); + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); return 0; -- 2.30.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: remove usage of drm_pci_alloc/free
Remove usage of legacy dma-api abstraction in preparation for removal Signed-off-by: Joseph Kogut --- Checkpatch warns here that r128 is marked obsolete, and asks for no unnecessary modifications. This series aims to address the FIXME in drivers/gpu/drm/drm_pci.c explaining that drm_pci_alloc/free is a needless abstraction of the dma-api, and it should be removed. Unfortunately, doing this requires removing the usage from an obsolete driver as well. If this patch is rejected for modifying an obsolete driver, would it be appropriate to follow up removing the FIXME from drm_pci? drivers/gpu/drm/drm_bufs.c | 19 --- drivers/gpu/drm/drm_dma.c | 8 +++- drivers/gpu/drm/r128/ati_pcigart.c | 22 ++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index e3d77dfefb0a..94bc1f6049c9 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -674,12 +674,17 @@ int drm_legacy_rmmap_ioctl(struct drm_device *dev, void *data, static void drm_cleanup_buf_error(struct drm_device *dev, struct drm_buf_entry *entry) { + drm_dma_handle_t *dmah; int i; if (entry->seg_count) { for (i = 0; i < entry->seg_count; i++) { if (entry->seglist[i]) { - drm_pci_free(dev, entry->seglist[i]); + dmah = entry->seglist[i]; + dma_free_coherent(dev->dev, + dmah->size, + dmah->vaddr, + dmah->busaddr); } } kfree(entry->seglist); @@ -978,10 +983,18 @@ int drm_legacy_addbufs_pci(struct drm_device *dev, page_count = 0; while (entry->buf_count < count) { + dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL); + if (!dmah) + return -ENOMEM; - dmah = drm_pci_alloc(dev, PAGE_SIZE << page_order, 0x1000); + dmah->size = total; + dmah->vaddr = dma_alloc_coherent(dev->dev, +dmah->size, +&dmah->busaddr, +GFP_KERNEL); + if (!dmah->vaddr) { + kfree(dmah); - if (!dmah) { /* Set count correctly so we free the proper amount. */ entry->buf_count = count; entry->seg_count = count; diff --git a/drivers/gpu/drm/drm_dma.c b/drivers/gpu/drm/drm_dma.c index d07ba54ec945..eb6b741a6f99 100644 --- a/drivers/gpu/drm/drm_dma.c +++ b/drivers/gpu/drm/drm_dma.c @@ -81,6 +81,7 @@ int drm_legacy_dma_setup(struct drm_device *dev) void drm_legacy_dma_takedown(struct drm_device *dev) { struct drm_device_dma *dma = dev->dma; + drm_dma_handle_t *dmah; int i, j; if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA) || @@ -100,7 +101,12 @@ void drm_legacy_dma_takedown(struct drm_device *dev) dma->bufs[i].seg_count); for (j = 0; j < dma->bufs[i].seg_count; j++) { if (dma->bufs[i].seglist[j]) { - drm_pci_free(dev, dma->bufs[i].seglist[j]); + dmah = dma->bufs[i].seglist[j]; + dma_free_coherent(dev->dev, + dmah->size, + dmah->vaddr, + dmah->busaddr); + kfree(dmah); } } kfree(dma->bufs[i].seglist); diff --git a/drivers/gpu/drm/r128/ati_pcigart.c b/drivers/gpu/drm/r128/ati_pcigart.c index 1234ec60c0af..fbb0cfd79758 100644 --- a/drivers/gpu/drm/r128/ati_pcigart.c +++ b/drivers/gpu/drm/r128/ati_pcigart.c @@ -45,18 +45,32 @@ static int drm_ati_alloc_pcigart_table(struct drm_device *dev, struct drm_ati_pcigart_info *gart_info) { - gart_info->table_handle = drm_pci_alloc(dev, gart_info->table_size, - PAGE_SIZE); - if (gart_info->table_handle == NULL) + drm_dma_handle_t *dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL); + + if (!dmah) + return -ENOMEM; + + dmah->size = gart_info->table_size; + dmah->vaddr = dma_alloc_coherent(dev->dev, +dmah->size, +&dmah->busaddr, +
[PATCH 2/2] drm: remove legacy drm_pci_alloc/free abstraction
The drm_pci_alloc/free abstraction of the dma-api is no longer required, remove it. Signed-off-by: Joseph Kogut --- drivers/gpu/drm/drm_pci.c | 58 --- include/drm/drm_legacy.h | 4 --- 2 files changed, 62 deletions(-) diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 2294a1580d35..1a1a2d4046e9 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -41,64 +41,6 @@ /* List of devices hanging off drivers with stealth attach. */ static LIST_HEAD(legacy_dev_list); static DEFINE_MUTEX(legacy_dev_list_lock); - -/** - * drm_pci_alloc - Allocate a PCI consistent memory block, for DMA. - * @dev: DRM device - * @size: size of block to allocate - * @align: alignment of block - * - * FIXME: This is a needless abstraction of the Linux dma-api and should be - * removed. - * - * Return: A handle to the allocated memory block on success or NULL on - * failure. - */ -drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align) -{ - drm_dma_handle_t *dmah; - - /* pci_alloc_consistent only guarantees alignment to the smallest -* PAGE_SIZE order which is greater than or equal to the requested size. -* Return NULL here for now to make sure nobody tries for larger alignment -*/ - if (align > size) - return NULL; - - dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL); - if (!dmah) - return NULL; - - dmah->size = size; - dmah->vaddr = dma_alloc_coherent(dev->dev, size, -&dmah->busaddr, -GFP_KERNEL); - - if (dmah->vaddr == NULL) { - kfree(dmah); - return NULL; - } - - return dmah; -} -EXPORT_SYMBOL(drm_pci_alloc); - -/** - * drm_pci_free - Free a PCI consistent memory block - * @dev: DRM device - * @dmah: handle to memory block - * - * FIXME: This is a needless abstraction of the Linux dma-api and should be - * removed. - */ -void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) -{ - dma_free_coherent(dev->dev, dmah->size, dmah->vaddr, - dmah->busaddr); - kfree(dmah); -} - -EXPORT_SYMBOL(drm_pci_free); #endif static int drm_get_pci_domain(struct drm_device *dev) diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 8ed04e9be997..faf64319be76 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -194,10 +194,6 @@ void drm_legacy_idlelock_release(struct drm_lock_data *lock); #ifdef CONFIG_PCI -struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size, -size_t align); -void drm_pci_free(struct drm_device *dev, struct drm_dma_handle *dmah); - int drm_legacy_pci_init(const struct drm_driver *driver, struct pci_driver *pdriver); void drm_legacy_pci_exit(const struct drm_driver *driver, -- 2.31.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] coda: fix reference counting in coda_file_mmap error path
On Thu, Apr 22, 2021 at 02:39:41PM +0200, Christian König wrote: > Am 22.04.21 um 14:27 schrieb Jan Harkes: > > Looks good to me. > > > > I'm also maintaining an out of tree coda module build that people sometimes > > use, which has workarounds for differences between the various kernel > > versions. > > > > Do you have a reference to the corresponding mmap_region change? If it is > > merged already I'll probably be able to find it. Is this mmap_region change > > expected to be backported to any lts kernels? > > That is the following upstream commit in Linus tree: > > commit 1527f926fd04490f648c42f42b45218a04754f87 > Author: Christian König > Date: Fri Oct 9 15:08:55 2020 +0200 > > mm: mmap: fix fput in error path v2 > > But I don't think we should backport that. > > And sorry for the noise. We had so many places which expected different > behavior that I didn't noticed that two occasions in the fs code actually > rely on the current behavior. > > For your out of tree module you could make the code version independent by > setting the vma back to the original file in case of an error. That should > work with both behaviors in mmap_region. Awesome, I'll give that a try, it may very well be a cleaner solution either way. And thank you for following up after your original patch and finding the filesystems that mess around with those mappings. I'm sure it would have taken me a while to figure out why file refcounts would go weird for some people, especially because this only happens in the error path. Jan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] coda: fix reference counting in coda_file_mmap error path
Looks good to me. I'm also maintaining an out of tree coda module build that people sometimes use, which has workarounds for differences between the various kernel versions. Do you have a reference to the corresponding mmap_region change? If it is merged already I'll probably be able to find it. Is this mmap_region change expected to be backported to any lts kernels? Jan On April 21, 2021 9:20:11 AM EDT, "Christian König" wrote: >mmap_region() now calls fput() on the vma->vm_file. > >So we need to drop the extra reference on the coda file instead of the >host file. > >Signed-off-by: Christian König >Fixes: 1527f926fd04 ("mm: mmap: fix fput in error path v2") >CC: sta...@vger.kernel.org # 5.11+ >--- > fs/coda/file.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > >diff --git a/fs/coda/file.c b/fs/coda/file.c >index 128d63df5bfb..ef5ca22bfb3e 100644 >--- a/fs/coda/file.c >+++ b/fs/coda/file.c >@@ -175,10 +175,10 @@ coda_file_mmap(struct file *coda_file, struct >vm_area_struct *vma) > ret = call_mmap(vma->vm_file, vma); > > if (ret) { >- /* if call_mmap fails, our caller will put coda_file so we >- * should drop the reference to the host_file that we got. >+ /* if call_mmap fails, our caller will put host_file so we >+ * should drop the reference to the coda_file that we got. >*/ >- fput(host_file); >+ fput(coda_file); > kfree(cvm_ops); > } else { > /* here we add redirects for the open/close vm_operations */ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/doc/rfc: i915 DG1 uAPI
On Tue, 20 Apr 2021 at 02:45, Matthew Auld wrote: > > Add an entry for the new uAPI needed for DG1. Also add the overall > upstream plan, including some notes for the TTM conversion. > > v2(Daniel): > - include the overall upstreaming plan > - add a note for mmap, there are differences here for TTM vs i915 > - bunch of other suggestions from Daniel > v3: > (Daniel) > - add a note for set/get caching stuff > - add some more docs for existing query and extensions stuff > - add an actual code example for regions query > - bunch of other stuff > (Jason) > - uAPI change(!): > - try a simpler design with the placements extension > - rather than have a generic setparam which can cover multiple > use cases, have each extension be responsible for one thing > only > v4: > (Daniel) > - add some more notes for ttm conversion > - bunch of other stuff > (Jason) > - uAPI change(!): > - drop all the extra rsvd members for the region_query and > region_info, just keep the bare minimum needed for padding Staying out of the ioctl's being over engineering, I hope they have a good future use case. The plan seems like a good plan. Acked-by: Dave Airlie Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel