date:20200504

Hi David.

On Tue, Apr 28, 2020 at 02:45:21PM +0800, David Lu wrote:
> fix boe_tv105wum_nw0 display shift.
> 
> Signed-off-by: David Lu 
Added fixes: tag and applied to drm-misc-next.

Sam

> ---
>  drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c 
> b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> index f89861c8598a..46fe1805c588 100644
> --- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> +++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> @@ -697,15 +697,15 @@ static const struct panel_desc auo_b101uan08_3_desc = {
>  };
>  
>  static const struct drm_display_mode boe_tv105wum_nw0_default_mode = {
> - .clock = 159260,
> + .clock = 159916,
>   .hdisplay = 1200,
>   .hsync_start = 1200 + 80,
>   .hsync_end = 1200 + 80 + 24,
>   .htotal = 1200 + 80 + 24 + 60,
>   .vdisplay = 1920,
> - .vsync_start = 1920 + 10,
> - .vsync_end = 1920 + 10 + 2,
> - .vtotal = 1920 + 10 + 2 + 14,
> + .vsync_start = 1920 + 20,
> + .vsync_end = 1920 + 20 + 4,
> + .vtotal = 1920 + 20 + 4 + 10,
>   .vrefresh = 60,
>   .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
>  };
> -- 
> 2.17.1
> 
> 
> 
> ___
> 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 V2 00/11] Subject: Remove duplicated kmap code

2020-05-04 Thread Ira Weiny

On Mon, May 04, 2020 at 06:33:57AM +0100, Al Viro wrote:
> On Sun, May 03, 2020 at 10:04:47PM -0700, Ira Weiny wrote:
> 
> > Grepping for 'asm/highmem.h' and investigations don't reveal any issues...  
> > But
> > you do have me worried.  That said 0-day has been crunching on multiple
> > versions of this series without issues such as this (save the mips issue
> > above).
> > 
> > I have to say it would be nice if the relation between linux/highmem.h and
> > asm/highmem.h was more straightforward.
> 
> IIRC, the headache was due to asm/pgtable.h on several architectures and
> asm/cacheflush.h on parisc.
> 
> 
> 
> || IOW, there's one in linux/highmem.h (conditional on 
> !CONFIG_HIGHMEM,
> || !ARCH_HAS_KMAP) and several per-architecture variants, usually declared in
> || their asm/highmem.h.  In three of those (microblaze, parisc and powerpc) 
> these
> || are inlines (parisc one identical to linux/highmem.h, lives in 
> asm/cacheflush.h,
> || powerpc and microblaze ones calling kmap_atomic_prot() which is defined in
> || arch/$ARCH/mm/highmem.c).
> || 
> || parisc case is weird - essentially, they want to call 
> || flush_kernel_dcache_page_addr() at the places where kunmap/kunmap_atomic
> || is done.  And they do so despite not selecting HIGHMEM, with definitions
> || in usual place.  They do have ARCH_HAS_KMAP defined, which prevents
> || getting buggered in linux/highmem.h.  ARCH_HAS_KMAP is parisc-unique,
> || BTW, and checked only in linux/highmem.h.
> || 
> || All genuine arch-specific variants are defined in (or call 
> functions
> || defined in) translation units that are only included CONFIG_HIGHMEM builds.

I agree with this statement.  But IMO additional confusion is caused by the
fact that some arch's condition the declarations on CONFIG_HIGHMEM within
asm/highmem.h (arc, arm, nds32) while others depend on linux/highmem.h (and
elsewhere) to do so (csky, microblaze, mips, powerpc, sparc, x86, xtensa).

Why?

I think (perhaps naive) over time asm/highmem.h needs to be isolated to being
included in linux/highmem.h.  But as you point out below that is not so easy.
I think that this series helps toward that goal.

> || 
> || It would be tempting to consolidate those, e.g. by adding 
> __kmap_atomic()
> || and __kmap_atomic_prot() without that boilerplate, with universal 
> kmap_atomic()
> || and kmap_atomic_prot() in linux/highmem.h.  Potential problem with that 
> would
> || be something that pulls ash/highmem.h (or asm/cacheflush.h in case of 
> parisc)
> || directly and uses kmap_atomic/kmap_atomic_prot.  There's not a lot places
> || pulling asm/highmem.h, and many of them are not even in includes:
> || 
> || arch/arm/include/asm/efi.h:13:#include 
> || arch/arm/mm/dma-mapping.c:31:#include 
> || arch/arm/mm/flush.c:14:#include 
> || arch/arm/mm/mmu.c:27:#include 
> || arch/mips/include/asm/pgtable-32.h:22:#include 
> || arch/mips/mm/cache.c:19:#include 
> || arch/mips/mm/fault.c:28:#include /* For 
> VMALLOC_END */
> || arch/nds32/include/asm/pgtable.h:60:#include 
> || arch/x86/kernel/setup_percpu.c:20:#include 
> || include/linux/highmem.h:35:#include 
> || 
> || Users of asm/cacheflush.h are rather more numerous; however, anything
> || outside of parisc-specific code has to pull linux/highmem.h, or it won't 
> see
> || the definitions of kmap_atomic/kmap_atomic_prot at all.  arch/parisc itself
> || has no callers of those.
> || 
> || Outside of arch/* there is a plenty of callers.  However, the following is
> || true: all instances of kmap_atomic or kmap_atomic_prot outside of arch/*
> || are either inside the linux/highmem.h or are preceded by include of
> || linux/highmem.h on any build that sees them (there is a common include
> || chain that is conditional upon CONFIG_BLOCK, but it's only needed in
> || drivers that are BLOCK-dependent).  It was not fun to verify, to put
> || it mildly...
> || 
> || So for parisc we have no problem - leaving 
> __kmap_atomic()/__kmap_atomic_prot()
> || in asm/cachefile.h and adding universal wrappers in linux/highmem.h will be
> || fine.  For other architectures the things might be trickier.

And the follow up series removes kmap_* from asm/cachefile.h in parisc which
should be cleaner going forward.

> || 
> || * arc: all users in arch/arc/ are within arch/arc/mm/{cache,highmem}.c;
> || both pull linux/highmem.h.  We are fine.

Still fine.

> || 
> || * arm: much, much worse.  We have several files that pull linux/highmem.h:
> || arch/arm/mm/cache-feroceon-l2.c, arch/arm/mm/cache-xsc3l2.c,
> || arch/arm/mm/copypage-*.c, arch/arm/mm/dma-mapping.c, arch/arm/mm/flush.c,
> || arch/arm/mm/highmem.c, arch/arm/probes/uprobes/core.c,
> || arch/arm/include/asm/kvm_mmu.h (kmap_atomic_pfn()).
> || Those are fine, but we also have this:
> || arch/arm/include/asm/pgtable.h:200:#define __pte_map(pmd)   
> (pte_t *)kmap_atomic(pmd_page(*(pmd)))
> || arch/arm/include/asm/pgtable.h:208:#define

Re: [PATCH v5 3/3] drm/bridge: chrontel-ch7033: Add a new driver

Hi Lubomir.

Drivers looks good. I look forward to the day we have moved
connector stuff to the displaydriver - this will simplify this driver
even more.

One Q in the following.

Sam

On Fri, Apr 24, 2020 at 11:35:39PM +0200, Lubomir Rintel wrote:
> This is a driver for video encoder with VGA and DVI/HDMI outputs.
> 
> There is no documentation for the chip -- the operation was guessed from
> what was sniffed on a Dell Wyse 3020 ThinOS terminal, the register names
> come from the ch7035 driver in Mediatek's GPL code dump.
> 
> Only bare minimum is implemented -- no fancy stuff, such as scaling. That
> would only worsen our misery. We don't load the firmware and we don't need
> to even bother enabling the MCU.  There are probably no distributable
> firmware images anyway.
> 
> Tested with a handful of monitors ranging from 1024x768@75 to 1400x1050@60,
> with VGA as well as DVI.
> 
> Signed-off-by: Lubomir Rintel 
Acked-by: Sam Ravnborg 
> 
> ---
> Changes since v4:
> - Removed the registration with the component framework and creation of
>   drm_encoder.
> 
> Changes since v3:
> - Cosmetic changes; drop ch7033_encoder_destroy() and use
>   drm_encoder_cleanup() for drm_encoder_funcs.destroy callback
>   directly.
> 
> Changes since v1:
> - Sort the includes
> - Drop a useless model id read
> - Chain to the bridge-connector instead of dealing with the HPD/EDID
>   readout machinery ourselves
> - Utilize regmap to access the registers
> 
>  drivers/gpu/drm/bridge/Kconfig   |  10 +
>  drivers/gpu/drm/bridge/Makefile  |   1 +
>  drivers/gpu/drm/bridge/chrontel-ch7033.c | 620 +++
>  3 files changed, 631 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/chrontel-ch7033.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index aaed2347ace9..0ebc72d62a5b 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -27,6 +27,16 @@ config DRM_CDNS_DSI
> Support Cadence DPI to DSI bridge. This is an internal
> bridge and is meant to be directly embedded in a SoC.
>  
> +config DRM_CHRONTEL_CH7033
> + tristate "Chrontel CH7033 Video Encoder"
> + depends on OF
> + select DRM_KMS_HELPER
> + help
> +   Enable support for the Chrontel CH7033 VGA/DVI/HDMI Encoder, as
> +   found in the Dell Wyse 3020 thin client.
> +
> +   If in doubt, say "N".
> +
>  config DRM_DISPLAY_CONNECTOR
>   tristate "Display connector support"
>   depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 6fb062b5b0f0..a844315feddb 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
> +obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
>  obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o
>  obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
>  obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += 
> megachips-stdp-ge-b850v3-fw.o
> diff --git a/drivers/gpu/drm/bridge/chrontel-ch7033.c 
> b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> new file mode 100644
> index ..f8675d82974b
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> @@ -0,0 +1,620 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Chrontel CH7033 Video Encoder Driver
> + *
> + * Copyright (C) 2019,2020 Lubomir Rintel
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Page 0, Register 0x07 */
> +enum {
> + DRI_PD  = BIT(3),
> + IO_PD   = BIT(5),
> +};
> +
> +/* Page 0, Register 0x08 */
> +enum {
> + DRI_PDDRI   = GENMASK(7, 4),
> + PDDAC   = GENMASK(3, 1),
> + PANEN   = BIT(0),
> +};
> +
> +/* Page 0, Register 0x09 */
> +enum {
> + DPD = BIT(7),
> + GCKOFF  = BIT(6),
> + TV_BP   = BIT(5),
> + SCLPD   = BIT(4),
> + SDPD= BIT(3),
> + VGA_PD  = BIT(2),
> + HDBKPD  = BIT(1),
> + HDMI_PD = BIT(0),
> +};
> +
> +/* Page 0, Register 0x0a */
> +enum {
> + MEMINIT = BIT(7),
> + MEMIDLE = BIT(6),
> + MEMPD   = BIT(5),
> + STOP= BIT(4),
> + LVDS_PD = BIT(3),
> + HD_DVIB = BIT(2),
> + HDCP_PD = BIT(1),
> + MCU_PD  = BIT(0),
> +};
> +
> +/* Page 0, Register 0x18 */
> +enum {
> + IDF = GENMASK(7, 4),
> + INTEN   = BIT(3),
> + SWAP= GENMASK(2, 0),
> +};
> +
> +enum {
> + BYTE_SWAP_RGB   = 0,
> + BYTE_SWAP_RBG   = 1,
> + BYTE_SWAP_GRB   = 2,
> + BYTE_SWAP_GBR   = 3,
> + BYTE_SWAP_BRG   = 4,
> + BYTE_SWAP_BGR   = 5,
> +};
> +
> +/* Page 0, Register 0x19 */
> +enum {
> + HPO_I   = BIT(5),
> +

Re: [PATCH v4] dt-bindings: display: dw_mipi_dsi.txt: convert to yaml

On Thu, Apr 23, 2020 at 01:00:58PM +0300, Adrian Ratiu wrote:
> This converts the Synopsis MIPI DSI binding documentation to yaml and
> should be quite straightforward. I've added a missing ref clk and also
> added Philippe as maintainer b/c he's the original txt author following
> the algorithm provided in Message-ID 20200420175909.ga5...@ravnborg.org.
> 
> Cc: Philippe CORNU 
> Cc: devicet...@vger.kernel.org
> Suggested-by: Laurent Pinchart 
> Reviewed-by: Rob Herring 
> Signed-off-by: Adrian Ratiu 
Applied to drm-misc-next.

Sam


> ---
> Changes in v4:
>   - Dropped panel patternProperties (Rob)
> 
> Changes in v3:
>   - Added ports property and its children which are required (Laurent)
>   - Sorted required list alphabetically
> 
> Changes in v2:
>   - Removed unnecessary descriptions and maxItems (Rob)
>   - Changed maintainers entry / dropped Mark (Rob)
>   - Added dsi-controller.yaml ref (Rob)
> ---
>  .../bindings/display/bridge/dw_mipi_dsi.txt   | 32 -
>  .../display/bridge/snps,dw-mipi-dsi.yaml  | 68 +++
>  2 files changed, 68 insertions(+), 32 deletions(-)
>  delete mode 100644 
> Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/snps,dw-mipi-dsi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt 
> b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt
> deleted file mode 100644
> index b13adf30b8d3b..0
> --- a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -Synopsys DesignWare MIPI DSI host controller
> -
> -
> -This document defines device tree properties for the Synopsys DesignWare MIPI
> -DSI host controller. It doesn't constitue a device tree binding specification
> -by itself but is meant to be referenced by platform-specific device tree
> -bindings.
> -
> -When referenced from platform device tree bindings the properties defined in
> -this document are defined as follows. The platform device tree bindings are
> -responsible for defining whether each optional property is used or not.
> -
> -- reg: Memory mapped base address and length of the DesignWare MIPI DSI
> -  host controller registers. (mandatory)
> -
> -- clocks: References to all the clocks specified in the clock-names property
> -  as specified in [1]. (mandatory)
> -
> -- clock-names:
> -  - "pclk" is the peripheral clock for either AHB and APB. (mandatory)
> -  - "px_clk" is the pixel clock for the DPI/RGB input. (optional)
> -
> -- resets: References to all the resets specified in the reset-names property
> -  as specified in [2]. (optional)
> -
> -- reset-names: string reset name, must be "apb" if used. (optional)
> -
> -- panel or bridge node: see [3]. (mandatory)
> -
> -[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> -[2] Documentation/devicetree/bindings/reset/reset.txt
> -[3] Documentation/devicetree/bindings/display/mipi-dsi-bus.txt
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/snps,dw-mipi-dsi.yaml 
> b/Documentation/devicetree/bindings/display/bridge/snps,dw-mipi-dsi.yaml
> new file mode 100644
> index 0..012aa8e7cb8cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/snps,dw-mipi-dsi.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/snps,dw-mipi-dsi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare MIPI DSI host controller
> +
> +maintainers:
> +  - Philippe CORNU 
> +
> +description: |
> +  This document defines device tree properties for the Synopsys DesignWare 
> MIPI
> +  DSI host controller. It doesn't constitue a device tree binding 
> specification
> +  by itself but is meant to be referenced by platform-specific device tree
> +  bindings.
> +
> +  When referenced from platform device tree bindings the properties defined 
> in
> +  this document are defined as follows. The platform device tree bindings are
> +  responsible for defining whether each property is required or optional.
> +
> +allOf:
> +  - $ref: ../dsi-controller.yaml#
> +
> +properties:
> +  reg:
> +maxItems: 1
> +
> +  clocks:
> +items:
> +  - description: Module clock
> +  - description: DSI bus clock for either AHB and APB
> +  - description: Pixel clock for the DPI/RGB input
> +minItems: 2
> +
> +  clock-names:
> +items:
> +  - const: ref
> +  - const: pclk
> +  - const: px_clk
> +minItems: 2
> +
> +  resets:
> +maxItems: 1
> +
> +  reset-names:
> +const: apb
> +
> +  ports:
> +type: object
> +
> +properties:
> +  port@0:
> +type: object
> +description: Input node to receive pixel data.
> +  port@1:
> +type: object
> +description: DSI

Re: [PATCH] drm/panel: panel-simple: Set AUO G101EVN010 panel type

Hi Tomi.

On Fri, Apr 17, 2020 at 02:40:43PM +0300, Tomi Valkeinen wrote:
> The AUO G101EVN010 is a 18-bit LVDS panel, not a parallel panel, as
> indicated by the current bus_format.
> 
> Fix the bus_format to MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, and also set the
> connector_type to LVDS.
> 
> Signed-off-by: Tomi Valkeinen 

Updated subject a little and applied.

Sam

> ---
>  drivers/gpu/drm/panel/panel-simple.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index 3ad828eaefe1..bf781e260fe7 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -836,7 +836,8 @@ static const struct panel_desc auo_g101evn010 = {
>   .width = 216,
>   .height = 135,
>   },
> - .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> + .connector_type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode auo_g104sn02_mode = {
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> ___
> 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] docs: dt: fix broken links due to txt->yaml renames

2020-05-04 Thread Joe Perches

On Mon, 2020-05-04 at 19:55 +0200, Uwe Kleine-König wrote:
> Hi Sam,
> 
> On Mon, May 04, 2020 at 07:45:22PM +0200, Sam Ravnborg wrote:
> > On Mon, May 04, 2020 at 11:30:20AM +0200, Mauro Carvalho Chehab wrote:
> > > There are some new broken doc links due to yaml renames
> > > at DT. Developers should really run:
> > > 
> > >   ./scripts/documentation-file-ref-check
> > > 
> > > in order to solve those issues while submitting patches.
> > Would love if some bot could do this for me on any patches that creates
> > .yaml files or so.
> > I know I will forget this and it can be automated.
> > If I get a bot mail that my patch would broke a link I would
> > have it fixed before it hits any tree.
> 
> What about adding a check to check_patch?

There's already a checkpatch warning when a patch renames
a file without a MAINTAINERS update.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 2/2] drm: xlnx: driver for Xilinx DSI TX Subsystem

2020-05-04 Thread Hyun Kwon

Hi GVRao,

Thanks for the patch. Sorry for late reply.

On Mon, 2020-04-20 at 14:20:56 -0700, Venkateshwar Rao Gannavarapu wrote:
> The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
> data from AXI-4 stream interface.
> 
> It supports upto 4 lanes, optional register interface for the DPHY,

I don't see the register interface for dphy support.

> multiple RGB color formats, command mode and video mode.
> This is a MIPI-DSI host driver and provides DSI bus for panels.
> This driver also helps to communicate with its panel using panel
> framework.
> 
> Signed-off-by: Venkateshwar Rao Gannavarapu 
> 
> ---
>  drivers/gpu/drm/xlnx/Kconfig|  11 +
>  drivers/gpu/drm/xlnx/Makefile   |   2 +
>  drivers/gpu/drm/xlnx/xlnx_dsi.c | 755 
> 
>  3 files changed, 768 insertions(+)
>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
> 
> diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
> index aa6cd88..73873cf 100644
> --- a/drivers/gpu/drm/xlnx/Kconfig
> +++ b/drivers/gpu/drm/xlnx/Kconfig
> @@ -11,3 +11,14 @@ config DRM_ZYNQMP_DPSUB
> This is a DRM/KMS driver for ZynqMP DisplayPort controller. Choose
> this option if you have a Xilinx ZynqMP SoC with DisplayPort
> subsystem.
> +
> +config DRM_XLNX_DSI
> +tristate "Xilinx DRM DSI Subsystem Driver"
> +select DRM_MIPI_DSI
> +select DRM_PANEL
> +select DRM_PANEL_SIMPLE
> +help
> +   This enables support for Xilinx MIPI-DSI.

This sentence is not needed with below. Could you please rephrase the whole?

> +   This is a DRM/KMS driver for Xilinx programmable DSI controller.
> +   Choose this option if you have a Xilinx MIPI DSI-TX controller
> +   subsytem.

These seem incorrectly indented.

> diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile
> index 2b844c6..b7ee6ef 100644
> --- a/drivers/gpu/drm/xlnx/Makefile
> +++ b/drivers/gpu/drm/xlnx/Makefile
> @@ -1,2 +1,4 @@
>  zynqmp-dpsub-objs += zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o
>  obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o
> +
> +obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o
> diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c
> new file mode 100644
> index 000..b8cae59
> --- /dev/null
> +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c
> @@ -0,0 +1,755 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx FPGA MIPI DSI Tx Controller driver
> + *
> + * Copyright (C) 2017 - 2019 Xilinx, Inc.
> + *
> + * Authors:
> + * - Saurabh Sengar 
> + * - Venkateshwar Rao Gannavarapu 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +/* DSI Tx IP registers */
> +#define XDSI_CCR 0x00
> +#define XDSI_CCR_COREENB BIT(0)
> +#define XDSI_CCR_SOFTRST BIT(1)
> +#define XDSI_CCR_CRREADY BIT(2)
> +#define XDSI_CCR_CMDMODE BIT(3)
> +#define XDSI_CCR_DFIFORSTBIT(4)
> +#define XDSI_CCR_CMDFIFORST  BIT(5)
> +#define XDSI_PCR 0x04
> +#define XDSI_PCR_VIDEOMODE(x)(((x) & 0x3) << 3)
> +#define XDSI_PCR_VIDEOMODE_MASK  (0x3 << 3)
> +#define XDSI_PCR_VIDEOMODE_SHIFT 3
> +#define XDSI_PCR_BLLPTYPE(x) ((x) << 5)
> +#define XDSI_PCR_BLLPMODE(x) ((x) << 6)
> +#define XDSI_PCR_EOTPENABLE(x)   ((x) << 13)
> +#define XDSI_GIER0x20
> +#define XDSI_ISR 0x24
> +#define XDSI_IER 0x28
> +#define XDSI_STR 0x2C
> +#define XDSI_STR_RDY_SHPKT   BIT(6)
> +#define XDSI_STR_RDY_LNGPKT  BIT(7)
> +#define XDSI_STR_DFIFO_FULL  BIT(8)
> +#define XDSI_STR_DFIFO_EMPTY BIT(9)
> +#define XDSI_STR_WAITFR_DATA BIT(10)
> +#define XDSI_STR_CMD_EXE_PGS BIT(11)
> +#define XDSI_STR_CCMD_PROC   BIT(12)
> +#define XDSI_STR_LPKT_MASK   (0x5 << 7)
> +#define XDSI_CMD 0x30
> +#define XDSI_CMD_QUEUE_PACKET(x) ((x) & GENMASK(23, 0))
> +#define XDSI_DFR 0x34
> +#define XDSI_TIME1   0x50
> +#define XDSI_TIME1_BLLP_BURST(x) ((x) & GENMASK(15, 0))
> +#define XDSI_TIME1_HSA(x)(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_TIME2   0x54
> +#define XDSI_TIME2_VACT(x)   ((x) & GENMASK(15, 0))
> +#define XDSI_TIME2_HACT(x)   (((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_HACT_MULTIPLIER GENMASK(1, 0)
> +#define XDSI_TIME3   0x58
> +#define XDSI_TIME3_HFP(x)((x) & GENMASK(15, 0))
> +#define XDSI_TIME3_HBP(x)(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_TIME4   0x5c
> +#define

[PATCH v2 8/9] drm/i915/pm: Prefer drm_WARN_ON over WARN_ON

struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.

Prefer drm_WARN_ON over WARN_ON.

Conversion is done with below sementic patch:

@@
identifier func, T;
@@
func(...) {
...
struct intel_crtc *T = ...;
+struct drm_i915_private *dev_priv = to_i915(T->base.dev);
<+...
-WARN_ON(
+drm_WARN_ON(_priv->drm,
...)
...+>

}

@@
identifier func, T;
@@
func(struct intel_crtc_state *T,...) {
+struct drm_i915_private *dev_priv = to_i915(T->uapi.crtc->dev);
<+...
-WARN_ON(
+drm_WARN_ON(_priv->drm,
...)
...+>

}

changes since v1:
- Added dev_priv local variable and used it in drm_WARN_ON calls (Jani)

Signed-off-by: Pankaj Bharadiya 
---
 drivers/gpu/drm/i915/intel_pm.c | 63 -
 1 file changed, 38 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bfb180fe8047..7e8da6c5bfb9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1436,6 +1436,7 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state 
*crtc_state)
 static int g4x_compute_intermediate_wm(struct intel_crtc_state *new_crtc_state)
 {
struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
struct g4x_wm_state *intermediate = 
_crtc_state->wm.g4x.intermediate;
const struct g4x_wm_state *optimal = _crtc_state->wm.g4x.optimal;
struct intel_atomic_state *intel_state =
@@ -1464,8 +1465,8 @@ static int g4x_compute_intermediate_wm(struct 
intel_crtc_state *new_crtc_state)
max(optimal->wm.plane[plane_id],
active->wm.plane[plane_id]);
 
-   WARN_ON(intermediate->wm.plane[plane_id] >
-   g4x_plane_fifo_size(plane_id, G4X_WM_LEVEL_NORMAL));
+   drm_WARN_ON(_priv->drm, intermediate->wm.plane[plane_id] >
+   g4x_plane_fifo_size(plane_id, G4X_WM_LEVEL_NORMAL));
}
 
intermediate->sr.plane = max(optimal->sr.plane,
@@ -1482,21 +1483,25 @@ static int g4x_compute_intermediate_wm(struct 
intel_crtc_state *new_crtc_state)
intermediate->hpll.fbc = max(optimal->hpll.fbc,
 active->hpll.fbc);
 
-   WARN_ON((intermediate->sr.plane >
-g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_SR) ||
-intermediate->sr.cursor >
-g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_SR)) &&
-   intermediate->cxsr);
-   WARN_ON((intermediate->sr.plane >
-g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_HPLL) ||
-intermediate->sr.cursor >
-g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_HPLL)) &&
-   intermediate->hpll_en);
-
-   WARN_ON(intermediate->sr.fbc > g4x_fbc_fifo_size(1) &&
-   intermediate->fbc_en && intermediate->cxsr);
-   WARN_ON(intermediate->hpll.fbc > g4x_fbc_fifo_size(2) &&
-   intermediate->fbc_en && intermediate->hpll_en);
+   drm_WARN_ON(_priv->drm,
+   (intermediate->sr.plane >
+g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_SR) ||
+intermediate->sr.cursor >
+g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_SR)) &&
+   intermediate->cxsr);
+   drm_WARN_ON(_priv->drm,
+   (intermediate->sr.plane >
+g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_HPLL) ||
+intermediate->sr.cursor >
+g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_HPLL)) &&
+   intermediate->hpll_en);
+
+   drm_WARN_ON(_priv->drm,
+   intermediate->sr.fbc > g4x_fbc_fifo_size(1) &&
+   intermediate->fbc_en && intermediate->cxsr);
+   drm_WARN_ON(_priv->drm,
+   intermediate->hpll.fbc > g4x_fbc_fifo_size(2) &&
+   intermediate->fbc_en && intermediate->hpll_en);
 
 out:
/*
@@ -1680,6 +1685,7 @@ static bool vlv_need_sprite0_fifo_workaround(unsigned int 
active_planes)
 static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
 {
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
const struct g4x_pipe_wm *raw =
_state->wm.vlv.raw[VLV_WM_LEVEL_PM2];
struct vlv_fifo_state *fifo_state = _state->wm.vlv.fifo_state;
@@ -1748,11 +1754,11 @@ static int vlv_compute_fifo(struct intel_crtc_state 
*crtc_state)
fifo_left -= plane_extra;
}
 
-   WARN_ON(active_planes != 0 && fifo_left != 0);
+   drm_WARN_ON(_priv->drm, active_planes != 0 && fifo_left != 0);
 
/* give it all to the first plane if none are active */
if (active_planes == 0) {
-

[PATCH v2 5/9] drm/i915/gem: Prefer drm_WARN* over WARN*

struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.

Prefer drm_WARN* over WARN* at places where struct drm_device pointer
can be extracted.

Signed-off-by: Pankaj Bharadiya 
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
 drivers/gpu/drm/i915/gem/i915_gem_phys.c   | 3 ++-
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c| 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index cce7df231cb9..cf9f0e078202 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1574,7 +1574,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
err = i915_vma_bind(target->vma,
target->vma->obj->cache_level,
PIN_GLOBAL, NULL);
-   if (WARN_ONCE(err,
+   if (drm_WARN_ONCE(>drm, err,
  "Unexpected failure to bind target VMA!"))
return err;
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c 
b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 7fe9831aa9ba..4c1c7232b024 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -27,7 +27,8 @@ static int i915_gem_object_get_pages_phys(struct 
drm_i915_gem_object *obj)
void *dst;
int i;
 
-   if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
+   if (drm_WARN_ON(obj->base.dev,
+   i915_gem_object_needs_bit17_swizzle(obj)))
return -EINVAL;
 
/*
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 7ffd7afeb7a5..8b0708708671 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -235,7 +235,7 @@ i915_gem_userptr_init__mmu_notifier(struct 
drm_i915_gem_object *obj,
if (flags & I915_USERPTR_UNSYNCHRONIZED)
return capable(CAP_SYS_ADMIN) ? 0 : -EPERM;
 
-   if (WARN_ON(obj->userptr.mm == NULL))
+   if (drm_WARN_ON(obj->base.dev, obj->userptr.mm == NULL))
return -EINVAL;
 
mn = i915_mmu_notifier_find(obj->userptr.mm);
-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/9] drm/i915/display/tc: Prefer drm_WARN_ON over WARN_ON

struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.

Prefer drm_WARN_ON over WARN_ON.

Conversion is done with below sementic patch:

@@
identifier func, T;
@@
func(...) {
...
struct drm_i915_private *T = ...;
<+...
-WARN_ON(
+drm_WARN_ON(>drm,
...)
...+>
}

@@
identifier func, T;
@@
func(struct intel_digital_port *T,...) {
+struct drm_i915_private *i915 = to_i915(T->base.base.dev);
<+...
-WARN_ON(
+drm_WARN_ON(>drm,
...)
...+>

}

changes since v1:
- Add i915 local variable and use it in drm_WARN_ON (Jani)

Signed-off-by: Pankaj Bharadiya 
---
 drivers/gpu/drm/i915/display/intel_tc.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_tc.c 
b/drivers/gpu/drm/i915/display/intel_tc.c
index d3bd5e798fbc..a2ffc991bc0e 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -360,12 +360,12 @@ static void icl_tc_phy_connect(struct intel_digital_port 
*dig_port,
}
 
if (!icl_tc_phy_set_safe_mode(dig_port, false) &&
-   !WARN_ON(dig_port->tc_legacy_port))
+   !drm_WARN_ON(>drm, dig_port->tc_legacy_port))
goto out_set_tbt_alt_mode;
 
max_lanes = intel_tc_port_fia_max_lane_count(dig_port);
if (dig_port->tc_legacy_port) {
-   WARN_ON(max_lanes != 4);
+   drm_WARN_ON(>drm, max_lanes != 4);
dig_port->tc_mode = TC_PORT_LEGACY;
 
return;
@@ -445,18 +445,20 @@ static bool icl_tc_phy_is_connected(struct 
intel_digital_port *dig_port)
 static enum tc_port_mode
 intel_tc_port_get_current_mode(struct intel_digital_port *dig_port)
 {
+   struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
u32 live_status_mask = tc_port_live_status_mask(dig_port);
bool in_safe_mode = icl_tc_phy_is_in_safe_mode(dig_port);
enum tc_port_mode mode;
 
-   if (in_safe_mode || WARN_ON(!icl_tc_phy_status_complete(dig_port)))
+   if (in_safe_mode ||
+   drm_WARN_ON(>drm, !icl_tc_phy_status_complete(dig_port)))
return TC_PORT_TBT_ALT;
 
mode = dig_port->tc_legacy_port ? TC_PORT_LEGACY : TC_PORT_DP_ALT;
if (live_status_mask) {
enum tc_port_mode live_mode = fls(live_status_mask) - 1;
 
-   if (!WARN_ON(live_mode == TC_PORT_TBT_ALT))
+   if (!drm_WARN_ON(>drm, live_mode == TC_PORT_TBT_ALT))
mode = live_mode;
}
 
@@ -505,7 +507,9 @@ static void
 intel_tc_port_link_init_refcount(struct intel_digital_port *dig_port,
 int refcount)
 {
-   WARN_ON(dig_port->tc_link_refcount);
+   struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+
+   drm_WARN_ON(>drm, dig_port->tc_link_refcount);
dig_port->tc_link_refcount = refcount;
 }
 
-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 9/9] drm/i915/runtime_pm: Prefer drm_WARN* over WARN*

struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.

Prefer drm_WARN* over WARN*.

Conversion is done with below semantic patch:

@@
identifier func, T;
@@
func(struct intel_runtime_pm *T,...) {
+ struct drm_i915_private *i915 = container_of(T, struct drm_i915_private, 
runtime_pm);
<+...
(
-WARN(
+drm_WARN(>drm,
...)
|
-WARN_ON(
+drm_WARN_ON(>drm,
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(>drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(>drm,
...)
)
...+>

}

Signed-off-by: Pankaj Bharadiya 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 39 ++---
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ad719c9602af..31ccd0559c55 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -116,6 +116,9 @@ track_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm)
 static void untrack_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm,
 depot_stack_handle_t stack)
 {
+   struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
unsigned long flags, n;
bool found = false;
 
@@ -134,9 +137,9 @@ static void untrack_intel_runtime_pm_wakeref(struct 
intel_runtime_pm *rpm,
}
spin_unlock_irqrestore(>debug.lock, flags);
 
-   if (WARN(!found,
-"Unmatched wakeref (tracking %lu), count %u\n",
-rpm->debug.count, atomic_read(>wakeref_count))) {
+   if (drm_WARN(>drm, !found,
+"Unmatched wakeref (tracking %lu), count %u\n",
+rpm->debug.count, atomic_read(>wakeref_count))) {
char *buf;
 
buf = kmalloc(PAGE_SIZE, GFP_NOWAIT | __GFP_NOWARN);
@@ -355,10 +358,14 @@ intel_runtime_pm_release(struct intel_runtime_pm *rpm, 
int wakelock)
 static intel_wakeref_t __intel_runtime_pm_get(struct intel_runtime_pm *rpm,
  bool wakelock)
 {
+   struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
int ret;
 
ret = pm_runtime_get_sync(rpm->kdev);
-   WARN_ONCE(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret);
+   drm_WARN_ONCE(>drm, ret < 0,
+ "pm_runtime_get_sync() failed: %d\n", ret);
 
intel_runtime_pm_acquire(rpm, wakelock);
 
@@ -539,6 +546,9 @@ void intel_runtime_pm_put(struct intel_runtime_pm *rpm, 
intel_wakeref_t wref)
  */
 void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
 {
+   struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
struct device *kdev = rpm->kdev;
 
/*
@@ -565,7 +575,8 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
 
pm_runtime_dont_use_autosuspend(kdev);
ret = pm_runtime_get_sync(kdev);
-   WARN(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret);
+   drm_WARN(>drm, ret < 0,
+"pm_runtime_get_sync() failed: %d\n", ret);
} else {
pm_runtime_use_autosuspend(kdev);
}
@@ -580,11 +591,14 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
 
 void intel_runtime_pm_disable(struct intel_runtime_pm *rpm)
 {
+   struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
struct device *kdev = rpm->kdev;
 
/* Transfer rpm ownership back to core */
-   WARN(pm_runtime_get_sync(kdev) < 0,
-"Failed to pass rpm ownership back to core\n");
+   drm_WARN(>drm, pm_runtime_get_sync(kdev) < 0,
+"Failed to pass rpm ownership back to core\n");
 
pm_runtime_dont_use_autosuspend(kdev);
 
@@ -594,12 +608,15 @@ void intel_runtime_pm_disable(struct intel_runtime_pm 
*rpm)
 
 void intel_runtime_pm_driver_release(struct intel_runtime_pm *rpm)
 {
+   struct drm_i915_private *i915 = container_of(rpm,
+struct drm_i915_private,
+runtime_pm);
int count = atomic_read(>wakeref_count);
 
-   WARN(count,
-"i915 raw-wakerefs=%d wakelocks=%d on cleanup\n",
-intel_rpm_raw_wakeref_count(count),
-intel_rpm_wakelock_count(count));
+   drm_WARN(>drm, count,
+"i915 raw-wakerefs=%d

[PATCH v2 2/9] drm/i915/display/dp: Prefer drm_WARN* over WARN*

struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.

Prefer drm_WARN* over WARN* at places where struct intel_dp or struct
drm_i915_private pointer is available.

Conversion is done with below sementic patch:

@rule1@
identifier func, T;
@@
func(...) {
...
struct drm_i915_private *T = ...;
<+...
(
-WARN_ON(
+drm_WARN_ON(>drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(>drm,
...)
)
...+>
}

@rule2@
identifier func, T;
@@
func(struct drm_i915_private *T,...) {
<+...
(
-WARN_ON(
+drm_WARN_ON(>drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(>drm,
...)
)
...+>
}

@rule3@
identifier func, T;
@@
func(struct intel_dp *T,...) {
+ struct drm_i915_private *i915 = dp_to_i915(T);
<+...
(
-WARN_ON(
+drm_WARN_ON(>drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(>drm,
...)
)
...+>

}

@rule4@
identifier func, T;
@@
func(...) {
...
struct intel_dp *T = ...;
+ struct drm_i915_private *i915 = dp_to_i915(T);
<+...
(
-WARN_ON(
+drm_WARN_ON(>drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(>drm,
...)
)
...+>

}

Signed-off-by: Pankaj Bharadiya 
---
 drivers/gpu/drm/i915/display/intel_dp.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 6952b0295096..ac44fc242879 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -409,7 +409,10 @@ static int intel_dp_rate_index(const int *rates, int len, 
int rate)
 
 static void intel_dp_set_common_rates(struct intel_dp *intel_dp)
 {
-   WARN_ON(!intel_dp->num_source_rates || !intel_dp->num_sink_rates);
+   struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+   drm_WARN_ON(>drm,
+   !intel_dp->num_source_rates || !intel_dp->num_sink_rates);
 
intel_dp->num_common_rates = intersect_rates(intel_dp->source_rates,
 intel_dp->num_source_rates,
@@ -418,7 +421,7 @@ static void intel_dp_set_common_rates(struct intel_dp 
*intel_dp)
 intel_dp->common_rates);
 
/* Paranoia, there should always be something in common. */
-   if (WARN_ON(intel_dp->num_common_rates == 0)) {
+   if (drm_WARN_ON(>drm, intel_dp->num_common_rates == 0)) {
intel_dp->common_rates[0] = 162000;
intel_dp->num_common_rates = 1;
}
@@ -1554,6 +1557,7 @@ static ssize_t
 intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
+   struct drm_i915_private *i915 = dp_to_i915(intel_dp);
u8 txbuf[20], rxbuf[20];
size_t txsize, rxsize;
int ret;
@@ -1567,10 +1571,10 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct 
drm_dp_aux_msg *msg)
txsize = msg->size ? HEADER_SIZE + msg->size : 
BARE_ADDRESS_SIZE;
rxsize = 2; /* 0 or 1 data bytes */
 
-   if (WARN_ON(txsize > 20))
+   if (drm_WARN_ON(>drm, txsize > 20))
return -E2BIG;
 
-   WARN_ON(!msg->buffer != !msg->size);
+   drm_WARN_ON(>drm, !msg->buffer != !msg->size);
 
if (msg->buffer)
memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
@@ -1595,7 +1599,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct 
drm_dp_aux_msg *msg)
txsize = msg->size ? HEADER_SIZE : BARE_ADDRESS_SIZE;
rxsize = msg->size + 1;
 
-   if (WARN_ON(rxsize > 20))
+   if (drm_WARN_ON(>drm, rxsize > 20))
return -E2BIG;
 
ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
@@ -1870,10 +1874,11 @@ static void intel_dp_print_rates(struct intel_dp 
*intel_dp)
 int
 intel_dp_max_link_rate(struct intel_dp *intel_dp)
 {
+   struct drm_i915_private *i915 = dp_to_i915(intel_dp);
int len;
 
len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->max_link_rate);
-   if (WARN_ON(len <= 0))
+   if (drm_WARN_ON(>drm, len <= 0))
return 162000;
 
return intel_dp->common_rates[len - 1];
@@ -1881,10 +1886,11 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
 
 int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
 {
+   struct drm_i915_private *i915 = dp_to_i915(intel_dp);
int i = intel_dp_rate_index(intel_dp->sink_rates,
intel_dp->num_sink_rates, rate);
 
-   if (WARN_ON(i < 0))
+   if (drm_WARN_ON(>drm, i < 0))
i = 0;
 
return i;
@@ -5580,7 +5586,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
if (!intel_dp->is_mst)
return -EINVAL;
 
-   WARN_ON_ONCE(intel_dp->active_mst_links < 0);
+   drm_WARN_ON_ONCE(>drm, intel_dp->active_mst_links <

[PATCH v2 7/9] drm/i915/pmu: Prefer drm_WARN_ON over WARN_ON

struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.

Prefer drm_WARN_ON over WARN_ON.

Signed-off-by: Pankaj Bharadiya 
---
 drivers/gpu/drm/i915/i915_pmu.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index e991a707bdb7..f6f44ad5e335 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -441,7 +441,11 @@ static u64 count_interrupts(struct drm_i915_private *i915)
 
 static void i915_pmu_event_destroy(struct perf_event *event)
 {
-   WARN_ON(event->parent);
+   struct drm_i915_private *i915 =
+   container_of(event->pmu, typeof(*i915), pmu.base);
+
+   drm_WARN_ON(>drm, event->parent);
+
module_put(THIS_MODULE);
 }
 
@@ -1058,8 +1062,10 @@ static int i915_pmu_register_cpuhp_state(struct i915_pmu 
*pmu)
 
 static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
 {
-   WARN_ON(pmu->cpuhp.slot == CPUHP_INVALID);
-   WARN_ON(cpuhp_state_remove_instance(pmu->cpuhp.slot, >cpuhp.node));
+   struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
+
+   drm_WARN_ON(>drm, pmu->cpuhp.slot == CPUHP_INVALID);
+   drm_WARN_ON(>drm, cpuhp_state_remove_instance(pmu->cpuhp.slot, 
>cpuhp.node));
cpuhp_remove_multi_state(pmu->cpuhp.slot);
pmu->cpuhp.slot = CPUHP_INVALID;
 }
-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over WARN*

struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.

Prefer drm_WARN* over WARN* calls.

changes since v1:
- Added dev_priv local variable and used it in drm_WARN* calls (Jani)

Signed-off-by: Pankaj Bharadiya 
---
 drivers/gpu/drm/i915/display/intel_sdvo.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c 
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index bc6c26818e15..773523dcd107 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -411,6 +411,7 @@ static const char *sdvo_cmd_name(u8 cmd)
 static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
   const void *args, int args_len)
 {
+   struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
const char *cmd_name;
int i, pos = 0;
char buffer[64];
@@ -431,7 +432,7 @@ static void intel_sdvo_debug_write(struct intel_sdvo 
*intel_sdvo, u8 cmd,
else
BUF_PRINT("(%02X)", cmd);
 
-   WARN_ON(pos >= sizeof(buffer) - 1);
+   drm_WARN_ON(_priv->drm, pos >= sizeof(buffer) - 1);
 #undef BUF_PRINT
 
DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, buffer);
@@ -533,6 +534,7 @@ static bool intel_sdvo_write_cmd(struct intel_sdvo 
*intel_sdvo, u8 cmd,
 static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 void *response, int response_len)
 {
+   struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
const char *cmd_status;
u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
u8 status;
@@ -597,7 +599,7 @@ static bool intel_sdvo_read_response(struct intel_sdvo 
*intel_sdvo,
BUF_PRINT(" %02X", ((u8 *)response)[i]);
}
 
-   WARN_ON(pos >= sizeof(buffer) - 1);
+   drm_WARN_ON(_priv->drm, pos >= sizeof(buffer) - 1);
 #undef BUF_PRINT
 
DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
@@ -1081,6 +1083,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct 
intel_sdvo *intel_sdvo,
 struct intel_crtc_state 
*crtc_state,
 struct drm_connector_state 
*conn_state)
 {
+   struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
struct hdmi_avi_infoframe *frame = _state->infoframes.avi.avi;
const struct drm_display_mode *adjusted_mode =
_state->hw.adjusted_mode;
@@ -1106,7 +1109,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct 
intel_sdvo *intel_sdvo,
   HDMI_QUANTIZATION_RANGE_FULL);
 
ret = hdmi_avi_infoframe_check(frame);
-   if (WARN_ON(ret))
+   if (drm_WARN_ON(_priv->drm, ret))
return false;
 
return true;
@@ -1115,6 +1118,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct 
intel_sdvo *intel_sdvo,
 static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 const struct intel_crtc_state 
*crtc_state)
 {
+   struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
u8 sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
const union hdmi_infoframe *frame = _state->infoframes.avi;
ssize_t len;
@@ -1123,11 +1127,12 @@ static bool intel_sdvo_set_avi_infoframe(struct 
intel_sdvo *intel_sdvo,
 intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI)) == 0)
return true;
 
-   if (WARN_ON(frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
+   if (drm_WARN_ON(_priv->drm,
+   frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
return false;
 
len = hdmi_infoframe_pack_only(frame, sdvo_data, sizeof(sdvo_data));
-   if (WARN_ON(len < 0))
+   if (drm_WARN_ON(_priv->drm, len < 0))
return false;
 
return intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_AVI_IF,
@@ -1237,6 +1242,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo 
*intel_sdvo,
 
 static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
 {
+   struct drm_i915_private *dev_priv = 
to_i915(pipe_config->uapi.crtc->dev);
unsigned dotclock = pipe_config->port_clock;
struct dpll *clock = _config->dpll;
 
@@ -1257,7 +1263,8 @@ static void i9xx_adjust_sdvo_tv_clock(struct 
intel_crtc_state *pipe_config)
clock->m1 = 12;
clock->m2 = 8;
} else {
-   WARN(1, "SDVO TV clock out of range: %i\n", dotclock);
+   drm_WARN(_priv->drm, 1,
+"SDVO TV clock out of range: %i\n", dotclock);
}
 
pipe_config->clock_set = true;
@@ -2293,7 +2300,7 @@

[PATCH v2 6/9] drm/i915/i915_drv: Prefer drm_WARN_ON over WARN_ON

struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.

Prefer drm_WARN_ON over WARN_ON.

changes since v1:
- Add parentheses around the dev_priv macro argument (Jani)

Signed-off-by: Pankaj Bharadiya 
---
 drivers/gpu/drm/i915/i915_drv.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6af69555733e..9fdf4bcd06e1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1649,7 +1649,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define HAS_DISPLAY(dev_priv) (INTEL_INFO(dev_priv)->pipe_mask != 0)
 
 /* Only valid when HAS_DISPLAY() is true */
-#define INTEL_DISPLAY_ENABLED(dev_priv) (WARN_ON(!HAS_DISPLAY(dev_priv)), 
!i915_modparams.disable_display)
+#define INTEL_DISPLAY_ENABLED(dev_priv) \
+   (drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)), 
!i915_modparams.disable_display)
 
 static inline bool intel_vtd_active(void)
 {
-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/9] drm/i915/display/display_power: Prefer drm_WARN_ON over WARN_ON

struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.

Prefer drm_WARN_ON over WARN_ON at places where struct i915_power_domains
struct is available.

Conversion is done with below sementic patch:

@@
identifier func, T;
@@
func(struct i915_power_domains *T,...) {
+ struct drm_i915_private *i915 = container_of(T, struct drm_i915_private, 
power_domains);
<+...
-WARN_ON(
+drm_WARN_ON(>drm,
...)
...+>

}

changes since v1:
- Fix commit subject (Jani)

Signed-off-by: Pankaj Bharadiya 
---
 .../drm/i915/display/intel_display_power.c| 35 +--
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c 
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 49998906cc61..69039cea1b5a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1943,22 +1943,29 @@ static u64 __async_put_domains_mask(struct 
i915_power_domains *power_domains)
 static bool
 assert_async_put_domain_masks_disjoint(struct i915_power_domains 
*power_domains)
 {
-   return !WARN_ON(power_domains->async_put_domains[0] &
-   power_domains->async_put_domains[1]);
+   struct drm_i915_private *i915 = container_of(power_domains,
+struct drm_i915_private,
+power_domains);
+   return !drm_WARN_ON(>drm, power_domains->async_put_domains[0] &
+   power_domains->async_put_domains[1]);
 }
 
 static bool
 __async_put_domains_state_ok(struct i915_power_domains *power_domains)
 {
+   struct drm_i915_private *i915 = container_of(power_domains,
+struct drm_i915_private,
+power_domains);
enum intel_display_power_domain domain;
bool err = false;
 
err |= !assert_async_put_domain_masks_disjoint(power_domains);
-   err |= WARN_ON(!!power_domains->async_put_wakeref !=
-  !!__async_put_domains_mask(power_domains));
+   err |= drm_WARN_ON(>drm, !!power_domains->async_put_wakeref !=
+  !!__async_put_domains_mask(power_domains));
 
for_each_power_domain(domain, __async_put_domains_mask(power_domains))
-   err |= WARN_ON(power_domains->domain_use_count[domain] != 1);
+   err |= drm_WARN_ON(>drm,
+  power_domains->domain_use_count[domain] != 
1);
 
return !err;
 }
@@ -2200,11 +2207,14 @@ static void
 queue_async_put_domains_work(struct i915_power_domains *power_domains,
 intel_wakeref_t wakeref)
 {
-   WARN_ON(power_domains->async_put_wakeref);
+   struct drm_i915_private *i915 = container_of(power_domains,
+struct drm_i915_private,
+power_domains);
+   drm_WARN_ON(>drm, power_domains->async_put_wakeref);
power_domains->async_put_wakeref = wakeref;
-   WARN_ON(!queue_delayed_work(system_unbound_wq,
-   _domains->async_put_work,
-   msecs_to_jiffies(100)));
+   drm_WARN_ON(>drm, !queue_delayed_work(system_unbound_wq,
+   
_domains->async_put_work,
+   msecs_to_jiffies(100)));
 }
 
 static void
@@ -4365,6 +4375,9 @@ __set_power_wells(struct i915_power_domains 
*power_domains,
  const struct i915_power_well_desc *power_well_descs,
  int power_well_count)
 {
+   struct drm_i915_private *i915 = container_of(power_domains,
+struct drm_i915_private,
+power_domains);
u64 power_well_ids = 0;
int i;
 
@@ -4384,8 +4397,8 @@ __set_power_wells(struct i915_power_domains 
*power_domains,
if (id == DISP_PW_ID_NONE)
continue;
 
-   WARN_ON(id >= sizeof(power_well_ids) * 8);
-   WARN_ON(power_well_ids & BIT_ULL(id));
+   drm_WARN_ON(>drm, id >= sizeof(power_well_ids) * 8);
+   drm_WARN_ON(>drm, power_well_ids & BIT_ULL(id));
power_well_ids |= BIT_ULL(id);
}
 
-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 0/9] Prefer drm_WARN* over WARN*

Now we have struct drm_device specific drm_WARN* macros which include
device information in the backtrace, so we know what device the
warnings originate from.

This series converts WARN* with drm_WARN* where struct drm_device
pointer can be extracted.

This series is the continuation of:
https://patchwork.freedesktop.org/series/72035/

changes since v1:
- Addressed Jani's review comments
- Rebase patches on latest tip.

Pankaj Bharadiya (9):
  drm/i915/display/display_power: Prefer drm_WARN_ON over WARN_ON
  drm/i915/display/dp: Prefer drm_WARN* over WARN*
  drm/i915/display/sdvo: Prefer drm_WARN* over WARN*
  drm/i915/display/tc: Prefer drm_WARN_ON over WARN_ON
  drm/i915/gem: Prefer drm_WARN* over WARN*
  drm/i915/i915_drv: Prefer drm_WARN_ON over WARN_ON
  drm/i915/pmu: Prefer drm_WARN_ON over WARN_ON
  drm/i915/pm: Prefer drm_WARN_ON over WARN_ON
  drm/i915/runtime_pm: Prefer drm_WARN* over WARN*

 .../drm/i915/display/intel_display_power.c| 35 +++
 drivers/gpu/drm/i915/display/intel_dp.c   | 24 ---
 drivers/gpu/drm/i915/display/intel_sdvo.c | 21 ---
 drivers/gpu/drm/i915/display/intel_tc.c   | 14 +++--
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c|  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_phys.c  |  3 +-
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c   |  2 +-
 drivers/gpu/drm/i915/i915_drv.h   |  3 +-
 drivers/gpu/drm/i915/i915_pmu.c   | 12 +++-
 drivers/gpu/drm/i915/intel_pm.c   | 63 +++
 drivers/gpu/drm/i915/intel_runtime_pm.c   | 39 
 11 files changed, 143 insertions(+), 75 deletions(-)

-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] docs: dt: fix broken links due to txt->yaml renames

2020-05-04 Thread Uwe Kleine-König

Hi Sam,

On Mon, May 04, 2020 at 07:45:22PM +0200, Sam Ravnborg wrote:
> On Mon, May 04, 2020 at 11:30:20AM +0200, Mauro Carvalho Chehab wrote:
> > There are some new broken doc links due to yaml renames
> > at DT. Developers should really run:
> > 
> > ./scripts/documentation-file-ref-check
> > 
> > in order to solve those issues while submitting patches.
> Would love if some bot could do this for me on any patches that creates
> .yaml files or so.
> I know I will forget this and it can be automated.
> If I get a bot mail that my patch would broke a link I would
> have it fixed before it hits any tree.

What about adding a check to check_patch?

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] docs: dt: fix broken links due to txt->yaml renames

Hi Mauro.

On Mon, May 04, 2020 at 11:30:20AM +0200, Mauro Carvalho Chehab wrote:
> There are some new broken doc links due to yaml renames
> at DT. Developers should really run:
> 
>   ./scripts/documentation-file-ref-check
> 
> in order to solve those issues while submitting patches.
Would love if some bot could do this for me on any patches that creates
.yaml files or so.
I know I will forget this and it can be automated.
If I get a bot mail that my patch would broke a link I would
have it fixed before it hits any tree.


> This tool can even fix most of the issues with:
> 
>   ./scripts/documentation-file-ref-check --fix
> 
> Signed-off-by: Mauro Carvalho Chehab 

Patch looks good.
Acked-by: Sam Ravnborg 

> ---
> 
> PS.: This patch is against today's linux-next.
> 
> 
>  .../devicetree/bindings/display/bridge/sii902x.txt  | 2 +-
>  .../devicetree/bindings/display/rockchip/rockchip-drm.yaml  | 2 +-
>  .../devicetree/bindings/net/mediatek-bluetooth.txt  | 2 +-
>  .../devicetree/bindings/sound/audio-graph-card.txt  | 2 +-
>  .../devicetree/bindings/sound/st,sti-asoc-card.txt  | 2 +-
>  Documentation/mips/ingenic-tcu.rst  | 2 +-
>  MAINTAINERS | 6 +++---
>  7 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt 
> b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> index 6e14e087c0d0..0d1db3f9da84 100644
> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> @@ -37,7 +37,7 @@ Optional properties:
>   simple-card or audio-graph-card binding. See their binding
>   documents on how to describe the way the sii902x device is
>   connected to the rest of the audio system:
> - Documentation/devicetree/bindings/sound/simple-card.txt
> + Documentation/devicetree/bindings/sound/simple-card.yaml
>   Documentation/devicetree/bindings/sound/audio-graph-card.txt
>   Note: In case of the audio-graph-card binding the used port
>   index should be 3.
> diff --git 
> a/Documentation/devicetree/bindings/display/rockchip/rockchip-drm.yaml 
> b/Documentation/devicetree/bindings/display/rockchip/rockchip-drm.yaml
> index ec8ae742d4da..7204da5eb4c5 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-drm.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-drm.yaml
> @@ -24,7 +24,7 @@ properties:
>  description: |
>Should contain a list of phandles pointing to display interface port
>of vop devices. vop definitions as defined in
> -  Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt
> +  Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml
>  
>  required:
>- compatible
> diff --git a/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt 
> b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> index 219bcbd0d344..9ef5bacda8c1 100644
> --- a/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> +++ b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> @@ -3,7 +3,7 @@ MediaTek SoC built-in Bluetooth Devices
>  
>  This device is a serial attached device to BTIF device and thus it must be a
>  child node of the serial node with BTIF. The dt-bindings details for BTIF
> -device can be known via Documentation/devicetree/bindings/serial/8250.txt.
> +device can be known via Documentation/devicetree/bindings/serial/8250.yaml.
>  
>  Required properties:
>  
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card.txt 
> b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> index 269682619a70..d5f6919a2d69 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> @@ -5,7 +5,7 @@ It is based on common bindings for device graphs.
>  see ${LINUX}/Documentation/devicetree/bindings/graph.txt
>  
>  Basically, Audio Graph Card property is same as Simple Card.
> -see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.txt
> +see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.yaml
>  
>  Below are same as Simple-Card.
>  
> diff --git a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt 
> b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> index 4d51f3f5ea98..a6ffcdec6f6a 100644
> --- a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> +++ b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> @@ -5,7 +5,7 @@ codec or external codecs.
>  
>  sti sound drivers allows to expose sti SoC audio interface through the
>  generic ASoC simple card. For details about sound card declaration please 
> refer to
> -Documentation/devicetree/bindings/sound/simple-card.txt.
>

Re: [PATCH] drm/amd/display: Fix pageflip event race condition for DCN. (v2)

2020-05-04 Thread Matt Coffin

Hey guys,

This is still an issue for me, and I'm still having to run a patch to
revert this as of 5.7-rc4. To avoid breaking a lot of people's Navi
setups in 5.7, is there any news on this? Has anyone else at the very
least been able to reproduce the problem?

It happens for me in every single program that mesa allows to utilize
variable refresh rates, and reverting it "fixes" the issue.

Cheers, and sorry for the extra email, just making sure this is still on
someone's radar,
Matt

On 4/14/20 5:32 PM, Matt Coffin wrote:
> Hey everyone,
> 
> This patch broke variable refresh rate in games (all that I've tried so
> far... Project CARS 2, DiRT Rally 2.0, Assetto Corsa Competizione) as
> well as a simple freesync tester application.
> 
> FreeSync tester I've been using: https://github.com/Nixola/VRRTest
> 
> I'm not at all familiar with the page flipping code, so it would take me
> a long time to find the *right* way to fix it, but does someone else see
> why it would do that?
> 
> The symptom is that the refresh rate of the display constantly bounces
> between the two ends of the FreeSync range (for me 40 -> 144), and the
> game stutters like a madman.
> 
> Any help on where to start, ideas on how to fix it (other than just
> revert this commit, which I've done in the interim), or alternative
> patches would be appreciated.
> 
> Thanks in advance for the work/help,
> Matt
> 
> On 3/13/20 8:42 AM, Michel Dänzer wrote:
>> On 2020-03-13 1:35 p.m., Kazlauskas, Nicholas wrote:
>>> On 2020-03-12 10:32 a.m., Alex Deucher wrote:
 On Thu, Mar 5, 2020 at 4:21 PM Mario Kleiner
  wrote:
>
> Commit '16f17eda8bad ("drm/amd/display: Send vblank and user
> events at vsartup for DCN")' introduces a new way of pageflip
> completion handling for DCN, and some trouble.
>
> The current implementation introduces a race condition, which
> can cause pageflip completion events to be sent out one vblank
> too early, thereby confusing userspace and causing flicker:
>
> prepare_flip_isr():
>
> 1. Pageflip programming takes the ddev->event_lock.
> 2. Sets acrtc->pflip_status == AMDGPU_FLIP_SUBMITTED
> 3. Releases ddev->event_lock.
>
> --> Deadline for surface address regs double-buffering passes on
>  target pipe.
>
> 4. dc_commit_updates_for_stream() MMIO programs the new pageflip
>     into hw, but too late for current vblank.
>
> => pflip_status == AMDGPU_FLIP_SUBMITTED, but flip won't complete
>     in current vblank due to missing the double-buffering deadline
>     by a tiny bit.
>
> 5. VSTARTUP trigger point in vblank is reached, VSTARTUP irq fires,
>     dm_dcn_crtc_high_irq() gets called.
>
> 6. Detects pflip_status == AMDGPU_FLIP_SUBMITTED and assumes the
>     pageflip has been completed/will complete in this vblank and
>     sends out pageflip completion event to userspace and resets
>     pflip_status = AMDGPU_FLIP_NONE.
>
> => Flip completion event sent out one vblank too early.
>
> This behaviour has been observed during my testing with measurement
> hardware a couple of time.
>
> The commit message says that the extra flip event code was added to
> dm_dcn_crtc_high_irq() to prevent missing to send out pageflip events
> in case the pflip irq doesn't fire, because the "DCH HUBP" component
> is clock gated and doesn't fire pflip irqs in that state. Also that
> this clock gating may happen if no planes are active. According to
> Nicholas, the clock gating can also happen if psr is active, and the
> gating is controlled independently by the hardware, so difficult to
> detect if and when the completion code in above commit is needed.
>
> This patch tries the following solution: It only executes the extra
> pflip
> completion code in dm_dcn_crtc_high_irq() iff the hardware reports
> that there aren't any surface updated pending in the double-buffered
> surface scanout address registers. Otherwise it leaves pflip completion
> to the pflip irq handler, for a more race-free experience.
>
> This would only guard against the order of events mentioned above.
> If Step 5 (VSTARTUP trigger) happens before step 4 then this won't help
> at all, because 1-3 + 5 might happen even without the hw being
> programmed
> at all, ie. no surface update pending because none yet programmed
> into hw.
>
> Therefore this patch also changes locking in amdgpu_dm_commit_planes(),
> so that prepare_flip_isr() and dc_commit_updates_for_stream() are done
> under event_lock protection within the same critical section.
>
> v2: Take Nicholas comments into account, try a different solution.
>
> Lightly tested on Polaris (locking) and Raven (the whole DCN stuff).
> Seems to work without causing obvious new trouble.

 Nick, any comments on this?  Can we get this

Re: [PATCH] docs: dt: fix broken links due to txt->yaml renames

2020-05-04 Thread Joe Perches

On Mon, 2020-05-04 at 11:30 +0200, Mauro Carvalho Chehab wrote:
> There are some new broken doc links due to yaml renames
> at DT. Developers should really run:
> 
>   ./scripts/documentation-file-ref-check
> 
> in order to solve those issues while submitting patches.
> This tool can even fix most of the issues with:
> 
>   ./scripts/documentation-file-ref-check --fix

Thanks Mauro.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915: Don't enable WaIncreaseLatencyIPCEnabled when IPC is disabled

2020-05-04 Thread Ville Syrjälä

On Thu, Apr 30, 2020 at 02:46:54PM -0700, Sultan Alsawaf wrote:
> From: Sultan Alsawaf 
> 
> In commit 5a7d202b1574, a logical AND was erroneously changed to an OR,
> causing WaIncreaseLatencyIPCEnabled to be enabled unconditionally for
> kabylake and coffeelake, even when IPC is disabled. Fix the logic so
> that WaIncreaseLatencyIPCEnabled is only used when IPC is enabled.
> 
> Fixes: 5a7d202b1574 ("drm/i915: Drop WaIncreaseLatencyIPCEnabled/1140 for 
> cnl")
> Cc: sta...@vger.kernel.org # 5.3.x+
> Signed-off-by: Sultan Alsawaf 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8375054ba27d..a52986a9e7a6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4992,7 +4992,7 @@ static void skl_compute_plane_wm(const struct 
> intel_crtc_state *crtc_state,
>* WaIncreaseLatencyIPCEnabled: kbl,cfl
>* Display WA #1141: kbl,cfl
>*/
> - if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) ||
> + if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) &&

Whoops. Thanks for the fix. Pushed.

>   dev_priv->ipc_enabled)
>   latency += 4;
>  
> -- 
> 2.26.2

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/10] drm: Add backlight helper

2020-05-04 Thread Noralf Trønnes



Den 04.05.2020 14.06, skrev Daniel Vetter:
> On Wed, Apr 29, 2020 at 02:48:22PM +0200, Noralf Trønnes wrote:
>> This adds a function that creates a backlight device for a connector.
>> It does not deal with the KMS backlight ABI proposition[1] to add a
>> connector property. It only takes the current best practise to standardise
>> the creation of a backlight device for DRM drivers while we wait for the
>> property.
>>
>> The brightness value is set using a connector state variable and an atomic
>> commit.
>>
>> I have looked through some of the backlight users and this is what I've 
>> found:
>>
>> GNOME [2]
>> -
>>
>> Brightness range: 0-100
>> Scale: Assumes perceptual
>>
>> Avoids setting the sysfs brightness value to zero if max_brightness >= 99.
>> Can connect connector and backlight using the sysfs device.
>>
>> KDE [3]
>> ---
>>
>> Brightness range: 0-100
>> Scale: Assumes perceptual
>>
>> Weston [4]
>> --
>>
>> Brightness range: 0-255
>> Scale: Assumes perceptual
>>
>> Chromium OS [5]
>> ---
>>
>> Brightness range: 0-100
>> Scale: Depends on the sysfs file 'scale' which is a recent addition (2019)
>>
>> xserver [6]
>> ---
>>
>> Brightness range: 0-x (driver specific) (1 is minimum, 0 is OFF)
>> Scale: Assumes perceptual
>>
>> The builtin modesetting driver[7] does not support Backlight, Intel[8] does.
>>
>> [1] 
>> https://lore.kernel.org/dri-devel/4b17ba08-39f3-57dd-5aad-d37d844b0...@linux.intel.com/
>> [2] 
>> https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/blob/master/plugins/power/gsd-backlight.c
>> [3] 
>> https://github.com/KDE/powerdevil/blob/master/daemon/backends/upower/backlighthelper.cpp
>> [4] 
>> https://gitlab.freedesktop.org/wayland/weston/-/blob/master/libweston/backend-drm/drm.c
>> [5] 
>> https://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/master/power_manager/powerd/system/internal_backlight.cc
>> [6] https://github.com/freedesktop/xorg-randrproto/blob/master/randrproto.txt
>> [7] 
>> https://gitlab.freedesktop.org/xorg/xserver/-/blob/master/hw/xfree86/drivers/modesetting/drmmode_display.c
>> [8] 
>> https://gitlab.freedesktop.org/xorg/driver/xf86-video-intel/-/blob/master/src/backlight.c
>>
>> Cc: Hans de Goede 
>> Cc: Jani Nikula 
>> Cc: Martin Peres 
>> Cc: Daniel Thompson 
>> Signed-off-by: Noralf Trønnes 
>> ---
>>  Documentation/gpu/drm-kms-helpers.rst  |   6 +
>>  drivers/gpu/drm/Kconfig|   7 ++
>>  drivers/gpu/drm/Makefile   |   1 +
>>  drivers/gpu/drm/drm_backlight_helper.c | 154 +
>>  include/drm/drm_backlight_helper.h |   9 ++
>>  include/drm/drm_connector.h|  10 ++
>>  6 files changed, 187 insertions(+)
>>  create mode 100644 drivers/gpu/drm/drm_backlight_helper.c
>>  create mode 100644 include/drm/drm_backlight_helper.h
>>
>> diff --git a/Documentation/gpu/drm-kms-helpers.rst 
>> b/Documentation/gpu/drm-kms-helpers.rst
>> index 9668a7fe2408..29a2f4b49fd2 100644
>> --- a/Documentation/gpu/drm-kms-helpers.rst
>> +++ b/Documentation/gpu/drm-kms-helpers.rst
>> @@ -411,3 +411,9 @@ SHMEM GEM Helper Reference
>>  
>>  .. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c
>> :export:
>> +
>> +Backlight Helper Reference
>> +==
>> +
>> +.. kernel-doc:: drivers/gpu/drm/drm_backlight_helper.c
>> +   :export:
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index d0aa6cff2e02..f6e13e18c9ca 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -224,6 +224,13 @@ config DRM_GEM_SHMEM_HELPER
>>  help
>>Choose this if you need the GEM shmem helper functions
>>  
>> +config DRM_BACKLIGHT_HELPER
>> +bool
>> +depends on DRM
>> +select BACKLIGHT_CLASS_DEVICE
>> +help
>> +  Choose this if you need the backlight device helper functions
>> +
>>  config DRM_VM
>>  bool
>>  depends on DRM && MMU
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 6493088a0fdd..0d17662dde0a 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -52,6 +52,7 @@ drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += 
>> drm_fb_helper.o
>>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>>  drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
>>  drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o
>> +drm_kms_helper-$(CONFIG_DRM_BACKLIGHT_HELPER) += drm_backlight_helper.o
>>  
>>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>>  obj-$(CONFIG_DRM_DEBUG_SELFTEST) += selftests/
>> diff --git a/drivers/gpu/drm/drm_backlight_helper.c 
>> b/drivers/gpu/drm/drm_backlight_helper.c
>> new file mode 100644
>> index ..06e6a75d1d0a
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_backlight_helper.c
>> @@ -0,0 +1,154 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>> +/*
>> + * Copyright 2020 Noralf Trønnes
>> + */
>> +
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include

[PATCH v4 1/1] drm/mm: optimize rb_hole_addr rbtree search

2020-05-04 Thread Nirmoy Das

Userspace can severely fragment rb_hole_addr rbtree by manipulating
alignment while allocating buffers. Fragmented rb_hole_addr rbtree
would result in large delays while allocating buffer object for a
userspace application. It takes long time to find suitable hole
because if we fail to find a suitable hole in the first attempt
then we look for neighbouring nodes using rb_prev()/rb_next().
Traversing rbtree using rb_prev()/rb_next() can take really long
time if the tree is fragmented.

This patch improves searches in fragmented rb_hole_addr rbtree by
modifying it to an augmented rbtree which will store an extra field
in drm_mm_node, subtree_max_hole. Each drm_mm_node now stores maximum
hole size for its subtree in drm_mm_node->subtree_max_hole. Using
drm_mm_node->subtree_max_hole, it is possible to eliminate a complete
subtree if that subtree is unable to serve a request hence reducing
number of rb_prev()/rb_next() used.

With this patch applied, 1 million bo allocs on amdgpu took ~8 sec,
compared to 50k bo allocs which took 28 sec without it.

partial test code:
int test_fragmentation(void)
{

int i = 0;
uint32_t  minor_version;
uint32_t  major_version;

struct amdgpu_bo_alloc_request request = {};
amdgpu_bo_handle vram_handle[MAX_ALLOC] = {};
amdgpu_device_handle device_handle;

request.alloc_size = 4096;
request.phys_alignment = 8192;
request.preferred_heap = AMDGPU_GEM_DOMAIN_VRAM;

int fd = open("/dev/dri/card0", O_RDWR | O_CLOEXEC);
amdgpu_device_initialize(fd, _version,  _version,
 _handle);

for (i = 0; i < MAX_ALLOC; i++) {
amdgpu_bo_alloc(device_handle, , _handle[i]);
}

for (i = 0; i < MAX_ALLOC; i++)
amdgpu_bo_free(vram_handle[i]);

return 0;
}

v2:
Use RB_DECLARE_CALLBACKS_MAX to maintain subtree_max_hole
v3:
insert_hole_addr() should be static a function
fix return value of next_hole_high_addr()/next_hole_low_addr()
Reported-by: kbuild test robot 
v4:
Fix commit message.

Signed-off-by: Nirmoy Das 
Reviewed-by: Chris Wilson 
Acked-by: Christian König 
---
 drivers/gpu/drm/drm_mm.c | 133 +--
 include/drm/drm_mm.h |   1 +
 2 files changed, 115 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 8981abe8b7c9..f4ca1ff80af9 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -212,20 +212,6 @@ static void drm_mm_interval_tree_add_node(struct 
drm_mm_node *hole_node,
   _mm_interval_tree_augment);
 }
 
-#define RB_INSERT(root, member, expr) do { \
-   struct rb_node **link = _node, *rb = NULL; \
-   u64 x = expr(node); \
-   while (*link) { \
-   rb = *link; \
-   if (x < expr(rb_entry(rb, struct drm_mm_node, member))) \
-   link = >rb_left; \
-   else \
-   link = >rb_right; \
-   } \
-   rb_link_node(>member, rb, link); \
-   rb_insert_color(>member, ); \
-} while (0)
-
 #define HOLE_SIZE(NODE) ((NODE)->hole_size)
 #define HOLE_ADDR(NODE) (__drm_mm_hole_node_start(NODE))
 
@@ -255,16 +241,42 @@ static void insert_hole_size(struct rb_root_cached *root,
rb_insert_color_cached(>rb_hole_size, root, first);
 }
 
+RB_DECLARE_CALLBACKS_MAX(static, augment_callbacks,
+struct drm_mm_node, rb_hole_addr,
+u64, subtree_max_hole, HOLE_SIZE)
+
+static void insert_hole_addr(struct rb_root *root, struct drm_mm_node *node)
+{
+   struct rb_node **link = >rb_node, *rb_parent = NULL;
+   u64 start = HOLE_ADDR(node), subtree_max_hole = node->subtree_max_hole;
+   struct drm_mm_node *parent;
+
+   while (*link) {
+   rb_parent = *link;
+   parent = rb_entry(rb_parent, struct drm_mm_node, rb_hole_addr);
+   if (parent->subtree_max_hole < subtree_max_hole)
+   parent->subtree_max_hole = subtree_max_hole;
+   if (start < HOLE_ADDR(parent))
+   link = >rb_hole_addr.rb_left;
+   else
+   link = >rb_hole_addr.rb_right;
+   }
+
+   rb_link_node(>rb_hole_addr, rb_parent, link);
+   rb_insert_augmented(>rb_hole_addr, root, _callbacks);
+}
+
 static void add_hole(struct drm_mm_node *node)
 {
struct drm_mm *mm = node->mm;
 
node->hole_size =
__drm_mm_hole_node_end(node) - __drm_mm_hole_node_start(node);
+   node->subtree_max_hole = node->hole_size;
DRM_MM_BUG_ON(!drm_mm_hole_follows(node));
 
insert_hole_size(>holes_size, node);
-   RB_INSERT(mm->holes_addr, rb_hole_addr, HOLE_ADDR);
+   insert_hole_addr(>holes_addr, node);
 
list_add(>hole_stack, >hole_stack);
 }
@@ -275,8 +287,10 @@ static void rm_hole(struct drm_mm_node *node)

Re: [PATCH v3 1/1] drm/mm: optimize rb_hole_addr rbtree search

2020-05-04 Thread Nirmoy



On 5/4/20 3:35 PM, Christian König wrote:

Am 04.05.20 um 12:42 schrieb Nirmoy Das:

Userspace can severely fragment rb_hole_addr rbtree by manipulating
alignment while allocating buffers. Fragmented rb_hole_addr rbtree
would result in large delays while allocating buffer object for a
userspace application. It takes long time to find suitable hole
because if we fail to find a suitable hole in the first attempt
then we look for neighbouring nodes using rb_prev()/rb_next().
Traversing rbtree using rb_prev()/rb_next() can take really long
time if the tree is fragmented.

This patch improves searches in fragmented rb_hole_addr rbtree by
modifying it to an augmented rbtree which will store an extra field
in drm_mm_node, subtree_max_hole. Each drm_mm_node now stores maximum
hole size for its subtree in drm_mm_node->subtree_max_hole. Using
drm_mm_node->subtree_max_hole, it is possible to eliminate complete 
subtree


"eliminate a complete subtree", but I'm not a native speaker either so 
take this with a grain of salt.



No,  you are right.





if that subtree is unable to serve a request hence reducing number of
rb_prev()/rb_next() used.

With this patch applied, 1 million bo allocations on amdgpu took ~8 
sec and


"1 million bo allocs on amdgpu took ~8 sec, compared to 50k bo allocs 
took 28 sec without".


E.g. keep the numbers in the same order for easy compare.



Thanks, this looks easier to understand.





without the patch the test code took 28 sec for only 50k bo allocs.

partial test code:
int test_fragmentation(void)
{

int i = 0;
 uint32_t  minor_version;
 uint32_t  major_version;

 struct amdgpu_bo_alloc_request request = {};
 amdgpu_bo_handle vram_handle[MAX_ALLOC] = {};
 amdgpu_device_handle device_handle;

 request.alloc_size = 4096;
 request.phys_alignment = 8192;
 request.preferred_heap = AMDGPU_GEM_DOMAIN_VRAM;

 int fd = open("/dev/dri/card0", O_RDWR | O_CLOEXEC);
 amdgpu_device_initialize(fd, _version, _version,
 _handle);

 for (i = 0; i < MAX_ALLOC; i++) {
 amdgpu_bo_alloc(device_handle, , 
_handle[i]);

 }

 for (i = 0; i < MAX_ALLOC; i++)
 amdgpu_bo_free(vram_handle[i]);

 return 0;
}

v2:
Use RB_DECLARE_CALLBACKS_MAX to maintain subtree_max_hole
v3:
insert_hole_addr() should be static a function
fix return value of next_hole_high_addr()/next_hole_low_addr()
Reported-by: kbuild test robot 

Signed-off-by: Nirmoy Das 
Reviewed-by: Chris Wilson 
---
  drivers/gpu/drm/drm_mm.c | 133 +--
  include/drm/drm_mm.h |   1 +
  2 files changed, 115 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 8981abe8b7c9..f4ca1ff80af9 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -212,20 +212,6 @@ static void drm_mm_interval_tree_add_node(struct 
drm_mm_node *hole_node,

 _mm_interval_tree_augment);
  }
  -#define RB_INSERT(root, member, expr) do { \
-    struct rb_node **link = _node, *rb = NULL; \
-    u64 x = expr(node); \
-    while (*link) { \
-    rb = *link; \
-    if (x < expr(rb_entry(rb, struct drm_mm_node, member))) \
-    link = >rb_left; \
-    else \
-    link = >rb_right; \
-    } \
-    rb_link_node(>member, rb, link); \
-    rb_insert_color(>member, ); \
-} while (0)
-
  #define HOLE_SIZE(NODE) ((NODE)->hole_size)
  #define HOLE_ADDR(NODE) (__drm_mm_hole_node_start(NODE))
  @@ -255,16 +241,42 @@ static void insert_hole_size(struct 
rb_root_cached *root,

  rb_insert_color_cached(>rb_hole_size, root, first);
  }
  +RB_DECLARE_CALLBACKS_MAX(static, augment_callbacks,
+ struct drm_mm_node, rb_hole_addr,
+ u64, subtree_max_hole, HOLE_SIZE)


Well that is a really nice solution. I've reviewed the implementation 
and couldn't find anything obvious wrong.


But I'm neither an expert on the drm_mm code nor the augmented RB tree 
implementation. So Acked-by: Christian König 
 for this patch.



Thanks I will resend after updating the commit message.




As a follow up I think we should extend this into keeping track of 
subtree_max_hole_align. With HOLE_SIZE_ALIGN(NODE) defined as 
(((NODE)->hole_size) << 6) | ffs(HOLE_ADDR(NODE)).



This should make it more faster .


Regards,

Nirmoy




This way we can also handle the ugly case of 4K allocation which are 
8K aligned.


Regards,
Christian.


+
+static void insert_hole_addr(struct rb_root *root, struct 
drm_mm_node *node)

+{
+    struct rb_node **link = >rb_node, *rb_parent = NULL;
+    u64 start = HOLE_ADDR(node), subtree_max_hole = 
node->subtree_max_hole;

+    struct drm_mm_node *parent;
+
+    while (*link) {
+    rb_parent = *link;
+    parent = rb_entry(rb_parent, struct drm_mm_node, rb_hole_addr);
+    if (parent->subtree_max_hole < subtree_max_hole)
+

[PATCH][next] drm/i915/gem: Fix inconsistent IS_ERR and PTR_ERR

2020-05-04 Thread Gustavo A. R. Silva

Fix inconsistent IS_ERR and PTR_ERR in __reloc_gpu_alloc().

The proper pointer to be passed as argument is ce.

This bug was detected with the help of Coccinelle.

Fixes: 6f576d6277ce ("drm/i915/gem: Try an alternate engine for relocations")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index cce7df231cb9..78fdbfd068d3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1325,7 +1325,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
 
ce = intel_context_create(engine);
if (IS_ERR(ce)) {
-   err = PTR_ERR(rq);
+   err = PTR_ERR(ce);
goto err_unpin;
}
 
-- 
2.26.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ast: Don't check new mode if CRTC is being disabled

On Mon, May 04, 2020 at 02:07:22PM +0200, Thomas Zimmermann wrote:
> Hi Emil
> 
> Am 01.05.20 um 15:20 schrieb Emil Velikov:
> > Hi Thomas,
> > 
> > Couple of fly-by ideas/suggestions.
> > 
> > On Thu, 30 Apr 2020 at 10:13, Thomas Zimmermann  wrote:
> >>
> >> Suspending failed because there's no mode if the CRTC is being
> >> disabled. Early-out in this case. This fixes runtime PM for ast.
> >>
> >> Signed-off-by: Thomas Zimmermann 
> >> ---
> >>  drivers/gpu/drm/ast/ast_mode.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/ast/ast_mode.c 
> >> b/drivers/gpu/drm/ast/ast_mode.c
> >> index 7a9f20a2fd303..089b7d9a0cf3f 100644
> >> --- a/drivers/gpu/drm/ast/ast_mode.c
> >> +++ b/drivers/gpu/drm/ast/ast_mode.c
> >> @@ -801,6 +801,9 @@ static int ast_crtc_helper_atomic_check(struct 
> >> drm_crtc *crtc,
> >> return -EINVAL;
> > Unrelated:
> > This feels quite dirty. If AST1180 does not support atomic modeset
> > simply remove the DRIVER_ATOMIC bit.
> > You can do that at runtime, via drm_device::driver_features in say,
> > ast_detect_chip()?
> 
> The line you commented on dates back to non-atomic modesetting, but I
> don't know what the story behind AST1180 is. It is explicitly disabled
> in the list of PCI IDs, but the driver has plenty of code for it. It
> looks as if the chip can only do pageflipping with a pre-set video mode.
> 
> As it is right now, the AST1180 code could probably be deleted entirely.
> 
> > 
> > The drm_driver::driver_features is immutable, or it ought to be.
> > 
> >> }
> >>
> >> +   if (!state->enable)
> >> +   return 0; /* no checks required if CRTC is being disabled 
> >> */
> >> +
> > I cannot think of a reason why a driver would need to perform
> > crtc_atomic_check, if the crtc is being disabled.
> > Can you spot any? If not, this should be better served in core, which
> > calls this callback.
> > Correct?
> Ast is a bit of a special case, because it tests the incoming mode
> against a list of re-defined modes. With the crtc being disabled, the
> incoming mode is 0 in all fields. Obviously that's not a valid mode, and
> we need that additional test here.
> 
> In the general case, I'd see 'crtc check' as part of the larger atomic
> infrastructure. I can imagine that configurations require the CRTC to be
> enabled before other HW blocks work. So a driver might have a reason to
> run crtc's check even for disabled crtcs (at least to verify that the
> crtc is not disabled). I don't think this can be handled in the core easily.

Jumping out of ->atomic_check callbacks when stuff is all off is fairly
standard pattern. So much standard that I'm kinda wondering whether we
shouldn't just make it the default in atomic helpers - so many drivers
forget to do it and break in funny ways.
-Daniel

> 
> Best regards
> Thomas
> 
> > 
> > HTH
> > -Emil
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




> ___
> 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 v6 0/6] drm/meson: add support for Amlogic Video FBC

2020-05-04 Thread Neil Armstrong

Hi Daniel,

On 21/04/2020 18:15, Neil Armstrong wrote:
> Amlogic uses a proprietary lossless image compression protocol and format
> for their hardware video codec accelerators, either video decoders or
> video input encoders.
> 
> It considerably reduces memory bandwidth while writing and reading
> frames in memory.
> 
> The underlying storage is considered to be 3 components, 8bit or 10-bit
> per component, YCbCr 420, single plane :
> - DRM_FORMAT_YUV420_8BIT
> - DRM_FORMAT_YUV420_10BIT
> 
> This modifier will be notably added to DMA-BUF frames imported from the V4L2
> Amlogic VDEC decoder.
> 
> At least two layout are supported :
> - Basic: composed of a body and a header
> - Scatter: the buffer is filled with a IOMMU scatter table referring
>   to the encoder current memory layout. This mode if more efficient in terms
>   of memory allocation but frames are not dumpable and only valid during until
>   the buffer is freed and back in control of the encoder
> 
> At least two options are supported :
> - Memory saving: when the pixel bpp is 8b, the size of the superblock can
>   be reduced, thus saving memory.

Is the fourcc DRM_FORMAT_MOD_ definition correct now ? It includes the changes 
I proposed on the
v5 thread.

Thanks,

Neil

> 
> This serie adds the missing register, updated the FBC decoder registers
> content to be committed by the crtc code.
> 
> The Amlogic FBC has been tested with compressed content from the Amlogic
> HW VP9 decoder on S905X (GXL), S905D2 (G12A) and S905X3 (SM1) in 8bit
> (Scatter+Mem Saving on G12A/SM1, Mem Saving on GXL) and 10bit
> (Scatter on G12A/SM1, default on GXL).
> 
> It's expected to work as-is on GXM and G12B SoCs.
> 
> Changes since v5 at [5]:
> - merged all fourcc patches in 1
> - fixed fourcc definition to have only a single DRM_MOD_
> - fixed 2 checkpatch issues
> 
> Changes since v4 at [4]:
> - added layout and options mask
> - cosmetic changes in fourcc.h
> - fixed mod check using the masks
> - fixed plane apply using the masks
> 
> Changes since v3 at [3]:
> - added dropped fourcc patch for scatter
> - fixed build of last patch
> 
> Changes since v2 at [2]:
> - Added "BASIC" layout and moved the SCATTER mode as layout, making
>   BASIC and SCATTER layout exclusives
> - Moved the Memory Saving at bit 8 for options fields
> - Split fourcc and overlay patch to introduce basic, mem saving and then
>   scatter in separate patches
> - Added comment about "transferability" of the buffers
> 
> Changes since v1 at [1]:
> - s/VD1_AXI_SEL_AFB/VD1_AXI_SEL_AFBC/ into meson_registers.h
> 
> [1] https://patchwork.freedesktop.org/series/73722/#rev1
> [2] https://patchwork.freedesktop.org/series/73722/#rev2
> [3] https://patchwork.freedesktop.org/series/73722/#rev3
> [4] https://patchwork.freedesktop.org/series/73722/#rev4
> [4] https://patchwork.freedesktop.org/series/73722/#rev5
> 
> Neil Armstrong (6):
>   drm/fourcc: Add modifier definitions for describing Amlogic Video
> Framebuffer Compression
>   drm/meson: add Amlogic Video FBC registers
>   drm/meson: overlay: setup overlay for Amlogic FBC
>   drm/meson: overlay: setup overlay for Amlogic FBC Memory Saving mode
>   drm/meson: overlay: setup overlay for Amlogic FBC Scatter Memory
> layout
>   drm/meson: crtc: handle commit of Amlogic FBC frames
> 
>  drivers/gpu/drm/meson/meson_crtc.c  | 118 +++---
>  drivers/gpu/drm/meson/meson_drv.h   |  16 ++
>  drivers/gpu/drm/meson/meson_overlay.c   | 290 +++-
>  drivers/gpu/drm/meson/meson_registers.h |  22 ++
>  include/uapi/drm/drm_fourcc.h   |  74 ++
>  5 files changed, 482 insertions(+), 38 deletions(-)
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tidss: remove AM65x PG1 YUV erratum code

2020-05-04 Thread Tomi Valkeinen


On 04/05/2020 13:51, Jyri Sarha wrote:

On 29/04/2020 15:10, Tomi Valkeinen wrote:

AM65x PG1 has a HW issue with YUV pixel formats, resulting in wrong
colors on the screen. This issue is fixed in PG2 hardware.

The driver currently has code to hide YUV pixel formats from the
userspace. To support PG2, we would need to add code to detect the SoC
version and hide the YUV formats based on that.

However, as PG1 will be phased out and PG2 will be the main platform, a
much simpler solution is just to drop the code in question. The downside
is that the users will be able to use YUV formats on PG1, getting wrong
colors on the screen. On the other hand, that may also be a plus, as the
same applications will now work on PG1 and PG2, even if the colors are
wrong on PG1.

Signed-off-by: Tomi Valkeinen 


One minor nitpick bellow, but otherwise:

Reviewed-by: Jyri Sarha 


---
  drivers/gpu/drm/tidss/tidss_dispc.c | 11 ++-
  drivers/gpu/drm/tidss/tidss_dispc.h |  5 -
  2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c 
b/drivers/gpu/drm/tidss/tidss_dispc.c
index 29f42768e294..629dd06393f6 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.c
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c
@@ -181,10 +181,6 @@ const struct dispc_features dispc_am65x_feats = {
.vid_name = { "vid", "vidl1" },
.vid_lite = { false, true, },
.vid_order = { 1, 0 },
-
-   .errata = {
-   .i2000 = true,
-   },
  };
  
  static const u16 tidss_j721e_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {

@@ -2674,12 +2670,9 @@ int dispc_init(struct tidss_device *tidss)
return -ENOMEM;
  
  	num_fourccs = 0;

-   for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i) {
-   if (feat->errata.i2000 &&
-   dispc_fourcc_is_yuv(dispc_color_formats[i].fourcc))
-   continue;
+   for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i)
dispc->fourccs[num_fourccs++] = dispc_color_formats[i].fourcc;
-   }
+
dispc->num_fourccs = num_fourccs;
dispc->tidss = tidss;
dispc->dev = dev;
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h 
b/drivers/gpu/drm/tidss/tidss_dispc.h
index a4a68249e44b..1228b166b255 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.h
+++ b/drivers/gpu/drm/tidss/tidss_dispc.h
@@ -46,10 +46,6 @@ struct dispc_features_scaling {
u32 xinc_max;
  };
  
-struct dispc_errata {

-   bool i2000; /* DSS Does Not Support YUV Pixel Data Formats */
-};
-
  enum dispc_vp_bus_type {
DISPC_VP_DPI,   /* DPI output */
DISPC_VP_OLDI,  /* OLDI (LVDS) output */
@@ -84,7 +80,6 @@ struct dispc_features {
bool vid_lite[TIDSS_MAX_PLANES];
u32 vid_order[TIDSS_MAX_PLANES];
  


Shouldn't we remove the above empty line too?


Indeed, I'll do the change. Thanks!

 Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tidss: remove AM65x PG1 YUV erratum code

2020-05-04 Thread Jyri Sarha

On 29/04/2020 15:10, Tomi Valkeinen wrote:
> AM65x PG1 has a HW issue with YUV pixel formats, resulting in wrong
> colors on the screen. This issue is fixed in PG2 hardware.
> 
> The driver currently has code to hide YUV pixel formats from the
> userspace. To support PG2, we would need to add code to detect the SoC
> version and hide the YUV formats based on that.
> 
> However, as PG1 will be phased out and PG2 will be the main platform, a
> much simpler solution is just to drop the code in question. The downside
> is that the users will be able to use YUV formats on PG1, getting wrong
> colors on the screen. On the other hand, that may also be a plus, as the
> same applications will now work on PG1 and PG2, even if the colors are
> wrong on PG1.
> 
> Signed-off-by: Tomi Valkeinen 

One minor nitpick bellow, but otherwise:

Reviewed-by: Jyri Sarha 

> ---
>  drivers/gpu/drm/tidss/tidss_dispc.c | 11 ++-
>  drivers/gpu/drm/tidss/tidss_dispc.h |  5 -
>  2 files changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c 
> b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 29f42768e294..629dd06393f6 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -181,10 +181,6 @@ const struct dispc_features dispc_am65x_feats = {
>   .vid_name = { "vid", "vidl1" },
>   .vid_lite = { false, true, },
>   .vid_order = { 1, 0 },
> -
> - .errata = {
> - .i2000 = true,
> - },
>  };
>  
>  static const u16 tidss_j721e_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {
> @@ -2674,12 +2670,9 @@ int dispc_init(struct tidss_device *tidss)
>   return -ENOMEM;
>  
>   num_fourccs = 0;
> - for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i) {
> - if (feat->errata.i2000 &&
> - dispc_fourcc_is_yuv(dispc_color_formats[i].fourcc))
> - continue;
> + for (i = 0; i < ARRAY_SIZE(dispc_color_formats); ++i)
>   dispc->fourccs[num_fourccs++] = dispc_color_formats[i].fourcc;
> - }
> +
>   dispc->num_fourccs = num_fourccs;
>   dispc->tidss = tidss;
>   dispc->dev = dev;
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h 
> b/drivers/gpu/drm/tidss/tidss_dispc.h
> index a4a68249e44b..1228b166b255 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -46,10 +46,6 @@ struct dispc_features_scaling {
>   u32 xinc_max;
>  };
>  
> -struct dispc_errata {
> - bool i2000; /* DSS Does Not Support YUV Pixel Data Formats */
> -};
> -
>  enum dispc_vp_bus_type {
>   DISPC_VP_DPI,   /* DPI output */
>   DISPC_VP_OLDI,  /* OLDI (LVDS) output */
> @@ -84,7 +80,6 @@ struct dispc_features {
>   bool vid_lite[TIDSS_MAX_PLANES];
>   u32 vid_order[TIDSS_MAX_PLANES];
>  

Shouldn't we remove the above empty line too?

> - struct dispc_errata errata;
>  };
>  
>  extern const struct dispc_features dispc_k2g_feats;
> 


-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/17] drm/mgag200: Move TAGFIFO reset into separate function

Hi Thomas.

> > 
> > 
> >> +  WREG_SEQ(0x01, seq1);
> >> +
> >> +  memctl = RREG32(MGAREG_MEMCTL);
> >> +
> >> +  memctl |= RESET_FLAG;
> >> +  WREG32(MGAREG_MEMCTL, memctl);
> >> +
> >> +  udelay(1000);
> >> +
> >> +  memctl &= ~RESET_FLAG;
> >> +  WREG32(MGAREG_MEMCTL, memctl);
> >> +
> >> +  /* screen on */
> >> +  RREG_SEQ(0x01, seq1);
> >> +  seq1 &= ~0x20;
> >> +  WREG_SEQ(0x01, seq1);
> > Here seq1 is read again, the old code used the old value.
> > I think new code is better.
> 
> You mean 'the old code was better,' right?
Well, if there is no good reason to change it stick with the old code we
know works.

I was not sure what would happen with the register when reset
was performed. So maybe reading back would be better, hence my comment.
But re-using the old value gives full control of the register.
So yeah, old code was better.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/17] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O

Hi Thomas.

> >> @@ -1161,9 +1174,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
> >>}
> >>  
> >>WREG_ECRT(0, ext_vga[0]);
> >> -  /* Enable mga pixel clock */
> >> -  misc = 0x2d;
> >>  
> >> +  misc = RREG8(MGA_MISC_IN);
> >> +  misc |= MGAREG_MISC_IOADSEL |
> >> +  MGAREG_MISC_RAMMAPEN |
> >> +  MGAREG_MISC_HIGH_PG_SEL;
> >>WREG8(MGA_MISC_OUT, misc);
> > 
> > I am left puzzeled why there is several writes to MGA_MISC_OUT.
> > The driver needs to read back and then write again.
> > 
> > Would it be simpler to have one function that only took care of
> > misc register update?
> 
> I'm not quite sure what you mean. MISC contains all kinds of unrelated
> state. In the final atomic code, different state is set in different
> places. It's simple to have a function read-modify-write the content of
> MISC for the bits that it cares about. With multiple functions, that
> leads to some duplicated reads and write, but the code as a whole is simple.
OK, when I looked at the code I initially got the impression that it was
a bit random. But then I also switched between patch and code etc.
The explanation above makes sense.

Sam

> 
> Best regards
> Thomas
> 
> > 
> >>  
> >>mga_crtc_do_set_base(mdev, fb, old_fb);
> >> diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h 
> >> b/drivers/gpu/drm/mgag200/mgag200_reg.h
> >> index c096a9d6bcbc1..89e12c55153cf 100644
> >> --- a/drivers/gpu/drm/mgag200/mgag200_reg.h
> >> +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
> >> @@ -16,10 +16,11 @@
> >>   *MGA1064SG Mystique register file
> >>   */
> >>  
> >> -
> >>  #ifndef _MGA_REG_H_
> >>  #define _MGA_REG_H_
> >>  
> >> +#include 
> >> +
> >>  #define   MGAREG_DWGCTL   0x1c00
> >>  #define   MGAREG_MACCESS  0x1c04
> >>  /* the following is a mystique only register */
> >> @@ -227,6 +228,8 @@
> >>  #define MGAREG_MISC_CLK_SEL_MGA_MSK   (0x3 << 2)
> >>  #define MGAREG_MISC_VIDEO_DIS (0x1 << 4)
> >>  #define MGAREG_MISC_HIGH_PG_SEL   (0x1 << 5)
> >> +#define MGAREG_MISC_HSYNCPOL  BIT(6)
> >> +#define MGAREG_MISC_VSYNCPOL  BIT(7)
> > I like BIT(), but mixing (1 << N) and BIT() does not look nice.
> > Oh, and do not get me started on GENMASK() :-)
> > 
> > Sam
> >>  
> >>  /* MMIO VGA registers */
> >>  #define MGAREG_SEQ_INDEX  0x1fc4
> >> -- 
> >> 2.26.0
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/17] drm/mgag200: Convert to atomic modesetting

Hi John

Am 30.04.20 um 02:11 schrieb John Donnelly:
> On 4/29/20 9:32 AM, Thomas Zimmermann wrote:
>> This patchset converts mgag200 to atomic modesetting. It uses simple
>> KMS helpers and SHMEM.
>>
>> Patches 1 to 4 simplifies the driver before the conversion. For example,
>> the HW cursor is not usable with the way universal planes work. A few
>> data structures can be cleaned up.
>>
>> Patches 5 to 15 untangle the existing modesetting code into smaller
>> functions. Specifically, mode setting and plane updates are being
>> separated from each other.
>>
>> Patch 16 converts mgag200 to simple KMS helpers and enables atomic
>> mode setting.
>>
>> As some HW seems to require a framebuffer offset of 0 within the video
>> memory, it does not work with atomic modesetting. Atomically switching
>> plane framebuffers, requires either source or target buffer to be located
>> at a non-0 offet. To resolve this problem, patch 17 converts mgag200 from
>> VRAM helpers to SHMEM helpers. During plane updates, the content of the
>> SHMEM BO is memcpy'd to VRAM. From my subjective obersation, performance
>> is not nuch different from the original code.
>>
>> The patchset has been tested on MGA G200EH hardware.
>>
>> Thomas Zimmermann (17):
>>    drm/mgag200: Remove HW cursor
>>    drm/mgag200: Remove unused fields from struct mga_device
>>    drm/mgag200: Embed connector instance in struct mga_device
>>    drm/mgag200: Use managed mode-config initialization
>>    drm/mgag200: Clean up mga_set_start_address()
>>    drm/mgag200: Clean up mga_crtc_do_set_base()
>>    drm/mgag200: Move mode-setting code into separate helper function
>>    drm/mgag200: Split MISC register update into PLL selection, SYNC and
>>  I/O
>>    drm/mgag200: Update mode registers after plane registers
>>    drm/mgag200: Set pitch in a separate helper function
>>    drm/mgag200: Set primary plane's format in separate helper function
>>    drm/mgag200: Move TAGFIFO reset into separate function
>>    drm/mgag200: Move hiprilvl setting into separate functions
>>    drm/mgag200: Move register initialization into separate function
>>    drm/mgag200: Remove waiting from DPMS code
>>    drm/mgag200: Convert to simple KMS helper
>>    drm/mgag200: Replace VRAM helpers with SHMEM helpers
>>
>>   drivers/gpu/drm/mgag200/Kconfig  |   4 +-
>>   drivers/gpu/drm/mgag200/Makefile |   2 +-
>>   drivers/gpu/drm/mgag200/mgag200_cursor.c | 319 
>>   drivers/gpu/drm/mgag200/mgag200_drv.c    |  51 +-
>>   drivers/gpu/drm/mgag200/mgag200_drv.h    |  43 +-
>>   drivers/gpu/drm/mgag200/mgag200_main.c   |  28 -
>>   drivers/gpu/drm/mgag200/mgag200_mode.c   | 948 ---
>>   drivers/gpu/drm/mgag200/mgag200_reg.h    |   5 +-
>>   drivers/gpu/drm/mgag200/mgag200_ttm.c    |  35 +-
>>   9 files changed, 563 insertions(+), 872 deletions(-)
>>   delete mode 100644 drivers/gpu/drm/mgag200/mgag200_cursor.c
>>
>> -- 
>> 2.26.0
>>
> 
> 
>  Hi Thomas ,
> 
>  I would like to test this on hardware that uses this device integrated
> into as BMC  ( iLo ) that I have ran into problems before. Can you post
> your staging URL so I can clone it ?

I uploaded the patches at


https://gitlab.freedesktop.org/tzimmermann/linux/-/tree/mgag200-simplekms-20200504

You can clone them with

  git clone g...@gitlab.freedesktop.org:tzimmermann/linux.git

and checkout the mgag200-simplekms-20200405 branch afterwards.

Best regards
Thomas

> 
> 
> ( Thank you for CC'ing me. I removed my email from on dlist recently) .
> 
> 
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/1] drm/mm: optimize rb_hole_addr rbtree search

2020-05-04 Thread Christian König


Am 04.05.20 um 12:42 schrieb Nirmoy Das:

Userspace can severely fragment rb_hole_addr rbtree by manipulating
alignment while allocating buffers. Fragmented rb_hole_addr rbtree
would result in large delays while allocating buffer object for a
userspace application. It takes long time to find suitable hole
because if we fail to find a suitable hole in the first attempt
then we look for neighbouring nodes using rb_prev()/rb_next().
Traversing rbtree using rb_prev()/rb_next() can take really long
time if the tree is fragmented.

This patch improves searches in fragmented rb_hole_addr rbtree by
modifying it to an augmented rbtree which will store an extra field
in drm_mm_node, subtree_max_hole. Each drm_mm_node now stores maximum
hole size for its subtree in drm_mm_node->subtree_max_hole. Using
drm_mm_node->subtree_max_hole, it is possible to eliminate complete subtree


"eliminate a complete subtree", but I'm not a native speaker either so 
take this with a grain of salt.



if that subtree is unable to serve a request hence reducing number of
rb_prev()/rb_next() used.

With this patch applied, 1 million bo allocations on amdgpu took ~8 sec and


"1 million bo allocs on amdgpu took ~8 sec, compared to 50k bo allocs 
took 28 sec without".


E.g. keep the numbers in the same order for easy compare.


without the patch the test code took 28 sec for only 50k bo allocs.

partial test code:
int test_fragmentation(void)
{

int i = 0;
 uint32_t  minor_version;
 uint32_t  major_version;

 struct amdgpu_bo_alloc_request request = {};
 amdgpu_bo_handle vram_handle[MAX_ALLOC] = {};
 amdgpu_device_handle device_handle;

 request.alloc_size = 4096;
 request.phys_alignment = 8192;
 request.preferred_heap = AMDGPU_GEM_DOMAIN_VRAM;

 int fd = open("/dev/dri/card0", O_RDWR | O_CLOEXEC);
 amdgpu_device_initialize(fd, _version,  _version,
 _handle);

 for (i = 0; i < MAX_ALLOC; i++) {
 amdgpu_bo_alloc(device_handle, , _handle[i]);
 }

 for (i = 0; i < MAX_ALLOC; i++)
 amdgpu_bo_free(vram_handle[i]);

 return 0;
}

v2:
Use RB_DECLARE_CALLBACKS_MAX to maintain subtree_max_hole
v3:
insert_hole_addr() should be static a function
fix return value of next_hole_high_addr()/next_hole_low_addr()
Reported-by: kbuild test robot 

Signed-off-by: Nirmoy Das 
Reviewed-by: Chris Wilson 
---
  drivers/gpu/drm/drm_mm.c | 133 +--
  include/drm/drm_mm.h |   1 +
  2 files changed, 115 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 8981abe8b7c9..f4ca1ff80af9 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -212,20 +212,6 @@ static void drm_mm_interval_tree_add_node(struct 
drm_mm_node *hole_node,
   _mm_interval_tree_augment);
  }
  
-#define RB_INSERT(root, member, expr) do { \

-   struct rb_node **link = _node, *rb = NULL; \
-   u64 x = expr(node); \
-   while (*link) { \
-   rb = *link; \
-   if (x < expr(rb_entry(rb, struct drm_mm_node, member))) \
-   link = >rb_left; \
-   else \
-   link = >rb_right; \
-   } \
-   rb_link_node(>member, rb, link); \
-   rb_insert_color(>member, ); \
-} while (0)
-
  #define HOLE_SIZE(NODE) ((NODE)->hole_size)
  #define HOLE_ADDR(NODE) (__drm_mm_hole_node_start(NODE))
  
@@ -255,16 +241,42 @@ static void insert_hole_size(struct rb_root_cached *root,

rb_insert_color_cached(>rb_hole_size, root, first);
  }
  
+RB_DECLARE_CALLBACKS_MAX(static, augment_callbacks,

+struct drm_mm_node, rb_hole_addr,
+u64, subtree_max_hole, HOLE_SIZE)


Well that is a really nice solution. I've reviewed the implementation 
and couldn't find anything obvious wrong.


But I'm neither an expert on the drm_mm code nor the augmented RB tree 
implementation. So Acked-by: Christian König  
for this patch.


As a follow up I think we should extend this into keeping track of 
subtree_max_hole_align. With HOLE_SIZE_ALIGN(NODE) defined as 
(((NODE)->hole_size) << 6) | ffs(HOLE_ADDR(NODE)).


This way we can also handle the ugly case of 4K allocation which are 8K 
aligned.


Regards,
Christian.


+
+static void insert_hole_addr(struct rb_root *root, struct drm_mm_node *node)
+{
+   struct rb_node **link = >rb_node, *rb_parent = NULL;
+   u64 start = HOLE_ADDR(node), subtree_max_hole = node->subtree_max_hole;
+   struct drm_mm_node *parent;
+
+   while (*link) {
+   rb_parent = *link;
+   parent = rb_entry(rb_parent, struct drm_mm_node, rb_hole_addr);
+   if (parent->subtree_max_hole < subtree_max_hole)
+   parent->subtree_max_hole = subtree_max_hole;
+   if (start <

Re: [PATCH v6 2/3] drm: Add support for the LogiCVC display controller

2020-05-04 Thread Emil Velikov

Hi Paul,

Just had a casual quick look for custom KMS properties, since new
drivers made that mistake in the past.
Thanks for not including any o/

I made a couple of trivial suggestions - if you agree, feel free to
keep them as follow-up patches.

On Thu, 30 Apr 2020 at 20:28, Paul Kocialkowski
 wrote:

> +int logicvc_of_property_parse_u32(struct device_node *of_node,
> + const char *name, u32 *target)
> +{
> +   struct logicvc_of_property *property;
> +   const char *string;
> +   u32 value;
> +   int ret;
> +
> +   property = logicvc_of_property_lookup(name);
> +   if (!property)
> +   return -EINVAL;
> +
One could have the logicvc_of_properties[] entries indexed with the
logicvc_of_property_parse_{u32,bool} caller, using that instead of the
name string.

Aside: I suspect the array (as most other arrays in this patch) should
be annotated const, correct?

> +   if (property->range[0] || property->range[1])
> +   if (value < property->range[0] || value > property->range[1])
Combine the two ifs?

-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 09/21] drm: panfrost: fix sg_table nents vs. orig_nents misuse

2020-05-04 Thread Steven Price


On 04/05/2020 13:53, Marek Szyprowski wrote:

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.


I find this commit message a bit confusing, but AFAICT the problem with 
the Panfrost code is really in mmu_map_sg() where we don't have the 
return value from dma_map_sg() and the for_each_sg() loop could (in 
theory) run off the end of the list.


The fix seems correct - store the return where it's meant to be (nents) 
and make sure when unmapping we use the original (orig_nents). So you 
might also consider adding:


Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver")

Even better would be the wrappers you mention in the cover letter! ;)

Reviewed-by: Steven Price 



Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
  drivers/gpu/drm/panfrost/panfrost_gem.c | 3 ++-
  drivers/gpu/drm/panfrost/panfrost_mmu.c | 4 +++-
  2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c 
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 17b654e..22fec7c 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -42,7 +42,8 @@ static void panfrost_gem_free_object(struct drm_gem_object 
*obj)
for (i = 0; i < n_sgt; i++) {
if (bo->sgts[i].sgl) {
dma_unmap_sg(pfdev->dev, bo->sgts[i].sgl,
-bo->sgts[i].nents, 
DMA_BIDIRECTIONAL);
+bo->sgts[i].orig_nents,
+DMA_BIDIRECTIONAL);
sg_free_table(>sgts[i]);
}
}
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c 
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index ed28aeb..2d9b1f9 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -517,7 +517,9 @@ static int panfrost_mmu_map_fault_addr(struct 
panfrost_device *pfdev, int as,
if (ret)
goto err_pages;
  
-	if (!dma_map_sg(pfdev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL)) {

+   sgt->nents = dma_map_sg(pfdev->dev, sgt->sgl, sgt->orig_nents,
+   DMA_BIDIRECTIONAL);
+   if (!sgt->nents) {
ret = -EINVAL;
goto err_map;
}



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Linux GOP initialization is wrong

2020-05-04 Thread Hans de Goede


Hi,

On 4/30/20 3:22 AM, David Santamaría Rogado wrote:

I'm thinking also about the i915 part. Could be something different
that Intel does in the Windows version of their driver? Seem very
strange that Microsoft could patch something because of this
particular hardwares.


On Windows devices often ship with vendor modified drivers,
for the graphics driver Intel even allowed the vendor to
ship its own driver and opt out of generic Intel driver
updates (this changes very recently).

Also chances are that this is all controlled by some registry
setting, so as long as the device ships with the registry
setting telling the driver to deal with this, then on upgrades
of the driver, the driver will still see the old registry setting
and behave accordingly. I've very carefully checked the VBT
(Video Bios Tables) and I see no hints there which the windows
driver could use. So I believe that this is taking care of
with device specific settings done by the device's vendor.

Regards,

Hans





El mar., 28 abr. 2020 a las 10:45, Hans de Goede
() escribió:


Hi David,

On 4/28/20 1:58 AM, David Santamaría Rogado wrote:

This is related to the issues at least on some devices for panel
orientation quirks where added.


Thank you for looking into this.


My tests have been done over a Lenovo ideapad D330.

This devices like the other ones that need panel orientation quirks,
shows the initramfs with wrong stride and x and y swapped. By applying
the panel orientation quirks this gets solved but many parts of the
systems components needs to be patched. Hans has done a great job with
plymouth, mutter... but always appears a new problem derived as for
example vnc desktop sharing with this devices doesn't work and the
output is send messed up.


When I first started adding support for devices which have their
screen mounted 90 degrees rotated my first attempts where aimed
at solving this transparently in the kernel.

Unfortunately this is not possible. On most affected devices
the hardware does not support 90 degrees rotation for the
primary display layer; or if it does this requires the framebuffer
being in a hardware-specific tiled format rather then being a
linear framebuffer. Using these tiled formats requires userspace
to be aware of this, which rules out transparently handling this
in the kernel.

Other layers (cursor layer, video overlay layers) have similar
issues which require userspace to be aware of what is happening,
so unfortunately there is no other way to deal with this then
fixing involved userspace components.

I'm a bit surprised that you sat that vnc desktop sharing does
not work, I guess that also depends on how the desktop sharing
works. If it pokes directly at the framebuffer somehow, then yes
it will be messed up. But if it goes through the display server
then things might work. I guess that it is possible that the
code doing this cannot deal with Xrandr output rotation ...


The strange thing is that bootloaders like GRUB or rEFInd seems to be
able to handle this and they paint themselves right, despite when
booting Windows directly Windows paints itself right and booted with
GRUB or rEFInd the first second also paint itself wrong. Haven't
tested this too much but the interesting thing is in the next
paragraph.


My experience with bootloaders showing themselves the right way
up is mixed. It seems that the firmware is doing some hacks for
this on some devices, at least for the EFI text console.

Funnily enough (for some definition of fun) on at least one of
the devices where the firmware is playing tricks (Asus T100HA IIRC)
the position of the carret for text-editing is off by one, which
is very annoying when editing the kernel commandline and which
clearly shows that things are being emulated in software here.


I decided to get the UEFI GOP video modes and found that the D330 have
these ones:
Mode 0: 1200x1920
Mode 1: 640x480
Mode 2: 800x600
Mode 3: 1024x768
Mode 4: 1920x1200 (this is the default one started by the firmware)
Mode 5: 480x640
Mode 6: 600x800
Mode 7: 768x1024

So I thought that Linux is taking the first mode despite is not the
active one and that's why the display is messed up.


Nope, Linux does not touch the mode at all (nor does grub by default).
Doing a EFI/GOP modeset has the risk of triggering all sort of firmware
bugs. So we stick with what we get. This has interesting side effects
where on some systems you get a different mode when turning on the
machine and letting it boot, vs turning it on, pressing e.g. F12 to get
the boot menu and then boot Linux.


Playing a little I could modify the GOP video mode before booting with
the UEFI Shell by simple using the mode 150 101. This causes GOP video
mode 5 to be switched to video mode 0, the first one. Booting now
makes initramfs messages to be correctly rendered but in the wrong
orientation.


Right, the rendering on the side thing is expected. As said the hardware
cannot do 90 degrees rotation with a linear framebuffer and the GOP

Re: [PATCH 12/17] drm/mgag200: Move TAGFIFO reset into separate function

Hi

Am 03.05.20 um 18:25 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> One nit about a bit name below.
> Acked-by: Sam Ravnborg 
> 
> On Wed, Apr 29, 2020 at 04:32:33PM +0200, Thomas Zimmermann wrote:
>> 5
>>
>> The TAGFIFO state is now reset in mgag200_g200er_reset_tagfifo().
>>
>> Signed-off-by: Thomas Zimmermann 
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  6 
>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 45 +-
>>  2 files changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
>> b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> index 9b957d9fc7e04..b10da90e0f35a 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> @@ -49,6 +49,12 @@
>>  WREG8(ATTR_DATA, v);\
>>  } while (0) \
>>  
>> +#define RREG_SEQ(reg, v)\
>> +do {\
>> +WREG8(MGAREG_SEQ_INDEX, reg);   \
>> +v = RREG8(MGAREG_SEQ_DATA); \
>> +} while (0) \
>> +
>>  #define WREG_SEQ(reg, v)\
>>  do {\
>>  WREG8(MGAREG_SEQ_INDEX, reg);   \
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
>> b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> index 73f7135cbb3d8..6b88c306ff4d7 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> @@ -1091,6 +1091,33 @@ static void mgag200_set_format_regs(struct mga_device 
>> *mdev,
>>  WREG_ECRT(3, crtcext3);
>>  }
>>  
>> +static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev)
>> +{
>> +static uint32_t RESET_FLAG = 0x0020; /* undocumented magic value */
>> +u8 seq1;
>> +u32 memctl;
>> +
>> +/* screen off */
>> +RREG_SEQ(0x01, seq1);
>> +seq1 |= 0x20;
> This looks like this:
> #defineM_SEQ1_SCROFF0x20
> 
> 
>> +WREG_SEQ(0x01, seq1);
>> +
>> +memctl = RREG32(MGAREG_MEMCTL);
>> +
>> +memctl |= RESET_FLAG;
>> +WREG32(MGAREG_MEMCTL, memctl);
>> +
>> +udelay(1000);
>> +
>> +memctl &= ~RESET_FLAG;
>> +WREG32(MGAREG_MEMCTL, memctl);
>> +
>> +/* screen on */
>> +RREG_SEQ(0x01, seq1);
>> +seq1 &= ~0x20;
>> +WREG_SEQ(0x01, seq1);
> Here seq1 is read again, the old code used the old value.
> I think new code is better.

You mean 'the old code was better,' right?

Best regards
Thomas

> 
>> +}
>> +
>>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  struct drm_display_mode *mode,
>>  struct drm_display_mode *adjusted_mode,
>> @@ -1225,22 +1252,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  
>>  mgag200_set_mode_regs(mdev, mode);
>>  
>> -/* reset tagfifo */
>> -if (mdev->type == G200_ER) {
>> -u32 mem_ctl = RREG32(MGAREG_MEMCTL);
>> -u8 seq1;
>> -
>> -/* screen off */
>> -WREG8(MGAREG_SEQ_INDEX, 0x01);
>> -seq1 = RREG8(MGAREG_SEQ_DATA) | 0x20;
>> -WREG8(MGAREG_SEQ_DATA, seq1);
>> -
>> -WREG32(MGAREG_MEMCTL, mem_ctl | 0x0020);
>> -udelay(1000);
>> -WREG32(MGAREG_MEMCTL, mem_ctl & ~0x0020);
>> -
>> -WREG8(MGAREG_SEQ_DATA, seq1 & ~0x20);
>> -}
>> +if (mdev->type == G200_ER)
>> +mgag200_g200er_reset_tagfifo(mdev);
>>  
>>  
>>  if (IS_G200_SE(mdev)) {
>> -- 
>> 2.26.0
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-dev

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/17] drm/mgag200: Set pitch in a separate helper function

Hi

Am 03.05.20 um 17:42 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Wed, Apr 29, 2020 at 04:32:31PM +0200, Thomas Zimmermann wrote:
>> The framebuffer's pitch is now set in mgag200_set_offset().
>>
>> Signed-off-by: Thomas Zimmermann 
> 
> I failed to follow this code.

Is it because of the line-offset calculation? MGAs want offsets in
multiples of pixels, not bytes, and that factor depends on the value of
cpp. I guess putting the calculation into a separate function will help
with readability.

> 
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 41 +-
>>  1 file changed, 27 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
>> b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> index 92dee31f16c42..eb83e471d72fc 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> @@ -1003,6 +1003,32 @@ static void mgag200_set_mode_regs(struct mga_device 
>> *mdev,
>>  mga_crtc_set_plls(mdev, mode->clock);
>>  }
>>  
>> +static void mgag200_set_offset(struct mga_device *mdev,
>> +   const struct drm_framebuffer *fb)
>> +{
>> +unsigned int offset;
>> +uint8_t crtc13, crtcext0;
>> +u8 bppshift;
>> +
>> +bppshift = mdev->bpp_shifts[fb->format->cpp[0] - 1];
> Hmm, use of cpp is deprecated. But is continue to be used all over.

I try to move code without modifying it too much. Replacing cpp with
some newer interface would obfuscate the moving to some extend. It's
always a balancing act.

> 
>> +
>> +offset = fb->pitches[0] / fb->format->cpp[0];
>> +if (fb->format->cpp[0] * 8 == 24)
>> +offset = (offset * 3) >> (4 - bppshift);
>> +else
>> +offset = offset >> (4 - bppshift);
>> +
>> +RREG_ECRT(0, crtcext0);
>> +
>> +crtc13 = offset & 0xff;
>> +
>> +crtcext0 &= ~GENMASK(5, 4);
>> +crtcext0 |= (offset & GENMASK(9, 8)) >> 4;
> Lot's of hardcoded numbers.
> Could the reg file include these so you had more readable defined names?

Sure.

Best regards
Thomas

> 
>> +
>> +WREG_CRT(0x13, crtc13);
>> +WREG_ECRT(0x00, crtcext0);
>> +}
>> +
>>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  struct drm_display_mode *mode,
>>  struct drm_display_mode *adjusted_mode,
>> @@ -1011,7 +1037,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  struct drm_device *dev = crtc->dev;
>>  struct mga_device *mdev = dev->dev_private;
>>  const struct drm_framebuffer *fb = crtc->primary->fb;
>> -int pitch;
>>  int option = 0, option2 = 0;
>>  int i;
>>  unsigned char misc = 0;
>> @@ -1122,12 +1147,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  WREG_SEQ(3, 0);
>>  WREG_SEQ(4, 0xe);
>>  
>> -pitch = fb->pitches[0] / fb->format->cpp[0];
>> -if (fb->format->cpp[0] * 8 == 24)
>> -pitch = (pitch * 3) >> (4 - bppshift);
>> -else
>> -pitch = pitch >> (4 - bppshift);
>> -
>>  WREG_GFX(0, 0);
>>  WREG_GFX(1, 0);
>>  WREG_GFX(2, 0);
>> @@ -1144,20 +1163,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  WREG_CRT(13, 0);
>>  WREG_CRT(14, 0);
>>  WREG_CRT(15, 0);
>> -WREG_CRT(19, pitch & 0xFF);
>> -
>> -ext_vga[0] = 0;
>>  
>>  /* TODO interlace */
>>  
>> -ext_vga[0] |= (pitch & 0x300) >> 4;
>>  if (fb->format->cpp[0] * 8 == 24)
>>  ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80;
>>  else
>>  ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
>>  ext_vga[4] = 0;
>>  
>> -WREG_ECRT(0, ext_vga[0]);
>>  WREG_ECRT(3, ext_vga[3]);
>>  WREG_ECRT(4, ext_vga[4]);
>>  
>> @@ -1171,8 +1185,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  WREG_ECRT(6, 0);
>>  }
>>  
>> -WREG_ECRT(0, ext_vga[0]);
>> -
>>  misc = RREG8(MGA_MISC_IN);
>>  misc |= MGAREG_MISC_IOADSEL |
>>  MGAREG_MISC_RAMMAPEN |
>> @@ -1180,6 +1192,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  WREG8(MGA_MISC_OUT, misc);
>>  
>>  mga_crtc_do_set_base(mdev, fb, old_fb);
>> +mgag200_set_offset(mdev, fb);
>>  
>>  mgag200_set_mode_regs(mdev, mode);
> 
>   Sam
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/21] DRM: fix struct sg_table nents vs. orig_nents misuse

Hi Christoph,

On 04.05.2020 14:52, Christoph Hellwig wrote:
> On Mon, May 04, 2020 at 02:50:17PM +0200, Marek Szyprowski wrote:
>> v2:
>> - dropped most of the changes to drm/i915
>> - added fixes for rcar-du, xen, media and ion
>> - fixed a few issues pointed by kbuild test robot
>> - added wide cc: list for each patch
> Didn't you plan to add dma_map_sgbuf and co helper?

Yes, I have a followup patches for that, but I wanted to fix the 
existing code in the first step. Then I wanted to send a wrappers and 
their application. Do you think I should do everything at once, in one 
patchset?

Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/17] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O

Hi

Am 03.05.20 um 17:34 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Wed, Apr 29, 2020 at 04:32:29PM +0200, Thomas Zimmermann wrote:
>> Set different fields in MISC in their rsp location in the code. This
>> patch also fixes a bug in the original code where the mode's SYNC flags
>> were never written into the MISC register.
>>
>> Signed-off-by: Thomas Zimmermann 
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 37 ++
>>  drivers/gpu/drm/mgag200/mgag200_reg.h  |  5 +++-
>>  2 files changed, 30 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
>> b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> index 749ba6e420ac7..b5bb02e9f05d6 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> @@ -704,6 +704,8 @@ static int mga_g200er_set_plls(struct mga_device *mdev, 
>> long clock)
>>  
>>  static int mga_crtc_set_plls(struct mga_device *mdev, long clock)
>>  {
>> +uint8_t misc;
> 
> General comment.
> uint8_t and friends are for uapi stuff.
> kernel internal prefer u8 and friends.

Ok.

> 
> Would be good to clean this up in the drivire, maybe as a follow-up
> patch after is becomes atomic.
> 
> 
>> +
>>  switch(mdev->type) {
>>  case G200_SE_A:
>>  case G200_SE_B:
>> @@ -724,6 +726,12 @@ static int mga_crtc_set_plls(struct mga_device *mdev, 
>> long clock)
>>  return mga_g200er_set_plls(mdev, clock);
>>  break;
>>  }
>> +
>> +misc = RREG8(MGA_MISC_IN);
>> +misc &= ~GENMASK(3, 2);
> The code would be easier to read if we had a 
> #define MGAREG_MISC_CLK_SEL_MASK  GENMASK(3, 2)
> 
> So the above became:
>   misc &= ~MGAREG_MISC_CLK_SEL_MASK;
> 
> Then it is more clear that the CLK_SEL bits are clared and then
> MGA_MSK is set.

Sure.

> 
>> +misc |= MGAREG_MISC_CLK_SEL_MGA_MSK;
>> +WREG8(MGA_MISC_OUT, misc);
>> +
>>  return 0;
>>  }
>>  
>> @@ -916,7 +924,7 @@ static void mgag200_set_mode_regs(struct mga_device 
>> *mdev,
>>  {
>>  unsigned int hdisplay, hsyncstart, hsyncend, htotal;
>>  unsigned int vdisplay, vsyncstart, vsyncend, vtotal;
>> -uint8_t misc = 0;
>> +uint8_t misc;
>>  uint8_t crtcext1, crtcext2, crtcext5;
>>  
>>  hdisplay = mode->hdisplay / 8 - 1;
>> @@ -933,10 +941,17 @@ static void mgag200_set_mode_regs(struct mga_device 
>> *mdev,
>>  vsyncend = mode->vsync_end - 1;
>>  vtotal = mode->vtotal - 2;
>>  
>> +misc = RREG8(MGA_MISC_IN);
>> +
>>  if (mode->flags & DRM_MODE_FLAG_NHSYNC)
>> -misc |= 0x40;
>> +misc |= MGAREG_MISC_HSYNCPOL;
>> +else
>> +misc &= ~MGAREG_MISC_HSYNCPOL;
>> +
> So the code just assumes DRM_MODE_FLAG_PHSYNC if
> DRM_MODE_FLAG_NHSYNC is not set.
> I think that is fine, but it also indicate how hoorible the
> flags definitions are in mode->flags

If polarity is not negative (i.e., bit set), it is positive (i.e., bit
cleared). What else could you set in MISC?

Having multiple flags in mode->flags that signal the same state is
somewhat problematic. I expect that the consistency of a mode's flags is
validated somewhere within the core.

> 
> 
>>  if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>> -misc |= 0x80;
>> +misc |= MGAREG_MISC_VSYNCPOL;
>> +else
>> +misc &= ~MGAREG_MISC_VSYNCPOL;
> And this code was touched in previous patch, but I gess it is better
> to update it here.
> 
>>  
>>  crtcext1 = (((htotal - 4) & 0x100) >> 8) |
>> ((hdisplay & 0x100) >> 7) |
>> @@ -982,6 +997,10 @@ static void mgag200_set_mode_regs(struct mga_device 
>> *mdev,
>>  WREG_ECRT(0x01, crtcext1);
>>  WREG_ECRT(0x02, crtcext2);
>>  WREG_ECRT(0x05, crtcext5);
>> +
>> +WREG8(MGA_MISC_OUT, misc);
>> +
>> +mga_crtc_set_plls(mdev, mode->clock);
>>  }
>>  
>>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
>> @@ -1140,12 +1159,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
>>  ext_vga[4] = 0;
>>  
>> -/* Set pixel clocks */
>> -misc = 0x2d;
>> -WREG8(MGA_MISC_OUT, misc);
>> -
>> -mga_crtc_set_plls(mdev, mode->clock);
>> -
>>  WREG_ECRT(0, ext_vga[0]);
>>  WREG_ECRT(3, ext_vga[3]);
>>  WREG_ECRT(4, ext_vga[4]);
>> @@ -1161,9 +1174,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>  }
>>  
>>  WREG_ECRT(0, ext_vga[0]);
>> -/* Enable mga pixel clock */
>> -misc = 0x2d;
>>  
>> +misc = RREG8(MGA_MISC_IN);
>> +misc |= MGAREG_MISC_IOADSEL |
>> +MGAREG_MISC_RAMMAPEN |
>> +MGAREG_MISC_HIGH_PG_SEL;
>>  WREG8(MGA_MISC_OUT, misc);
> 
> I am left puzzeled why there is several writes to MGA_MISC_OUT.
> The driver needs to read back and then write again.
> 
> Would it be simpler to have one function that only took care of
> misc register update?

I'm not quite sure what you mean. MISC contains all kinds of unrelated
state.

[PATCH v2 19/21] dmabuf: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/dma-buf/heaps/heap-helpers.c | 7 ---
 drivers/dma-buf/udmabuf.c| 5 +++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/dma-buf/heaps/heap-helpers.c 
b/drivers/dma-buf/heaps/heap-helpers.c
index 9f964ca..b923863 100644
--- a/drivers/dma-buf/heaps/heap-helpers.c
+++ b/drivers/dma-buf/heaps/heap-helpers.c
@@ -144,8 +144,9 @@ struct sg_table *dma_heap_map_dma_buf(struct 
dma_buf_attachment *attachment,
 
table = >table;
 
-   if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
-   direction))
+   table->nents = dma_map_sg(attachment->dev, table->sgl,
+ table->orig_nents, direction);
+   if (!table->nents)
table = ERR_PTR(-ENOMEM);
return table;
 }
@@ -154,7 +155,7 @@ static void dma_heap_unmap_dma_buf(struct 
dma_buf_attachment *attachment,
   struct sg_table *table,
   enum dma_data_direction direction)
 {
-   dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+   dma_unmap_sg(attachment->dev, table->sgl, table->orig_nents, direction);
 }
 
 static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index acb26c6..ea0cf71 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -63,7 +63,8 @@ static struct sg_table *get_sg_table(struct device *dev, 
struct dma_buf *buf,
GFP_KERNEL);
if (ret < 0)
goto err;
-   if (!dma_map_sg(dev, sg->sgl, sg->nents, direction)) {
+   sg->nents = dma_map_sg(dev, sg->sgl, sg->orig_nents, direction);
+   if (!sg->nents) {
ret = -EINVAL;
goto err;
}
@@ -78,7 +79,7 @@ static struct sg_table *get_sg_table(struct device *dev, 
struct dma_buf *buf,
 static void put_sg_table(struct device *dev, struct sg_table *sg,
 enum dma_data_direction direction)
 {
-   dma_unmap_sg(dev, sg->sgl, sg->nents, direction);
+   dma_unmap_sg(dev, sg->sgl, sg->orig_nents, direction);
sg_free_table(sg);
kfree(sg);
 }
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 06/21] drm: i915: fix sg_table nents vs. orig_nents misuse for dmabuf objects

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h.

This driver creatively uses sg_table->orig_nents to store the size of the
allocate scatterlist and ignores the number of the entries returned by
dma_map_sg function. The sg_table->orig_nents is (mis)used to properly
free the (over)allocated scatterlist.

This patch only fixes the sg_table->nents entries in the sg_table objects
exported by the dmabuf related functions, so the other drivers, which
might share buffers with i915 could rely on the nents and orig_nents
values.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c   | 9 +
 drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c | 5 +++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index 7db5a79..98159df 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -48,9 +48,10 @@ static struct sg_table *i915_gem_map_dma_buf(struct 
dma_buf_attachment *attachme
src = sg_next(src);
}
 
-   if (!dma_map_sg_attrs(attachment->dev,
- st->sgl, st->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC)) {
+   st->nents = dma_map_sg_attrs(attachment->dev,
+st->sgl, st->orig_nents, dir,
+DMA_ATTR_SKIP_CPU_SYNC);
+   if (!st->nents) {
ret = -ENOMEM;
goto err_free_sg;
}
@@ -74,7 +75,7 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment 
*attachment,
struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
 
dma_unmap_sg_attrs(attachment->dev,
-  sg->sgl, sg->nents, dir,
+  sg->sgl, sg->orig_nents, dir,
   DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sg);
kfree(sg);
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c 
b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
index debaf7b..5723525 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
@@ -28,7 +28,8 @@ static struct sg_table *mock_map_dma_buf(struct 
dma_buf_attachment *attachment,
sg = sg_next(sg);
}
 
-   if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
+   st->nents = dma_map_sg(attachment->dev, st->sgl, st->orig_nents, dir);
+   if (!st->nents) {
err = -ENOMEM;
goto err_st;
}
@@ -46,7 +47,7 @@ static void mock_unmap_dma_buf(struct dma_buf_attachment 
*attachment,
   struct sg_table *st,
   enum dma_data_direction dir)
 {
-   dma_unmap_sg(attachment->dev, st->sgl, st->nents, dir);
+   dma_unmap_sg(attachment->dev, st->sgl, st->orig_nents, dir);
sg_free_table(st);
kfree(st);
 }
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 05/21] drm: exynos: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index fcee33a..f995b0c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -396,7 +396,7 @@ static void g2d_userptr_put_dma_addr(struct g2d_data *g2d,
 
 out:
dma_unmap_sg(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt->sgl,
-   g2d_userptr->sgt->nents, DMA_BIDIRECTIONAL);
+g2d_userptr->sgt->orig_nents, DMA_BIDIRECTIONAL);
 
pages = frame_vector_pages(g2d_userptr->vec);
if (!IS_ERR(pages)) {
@@ -511,8 +511,9 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct g2d_data 
*g2d,
 
g2d_userptr->sgt = sgt;
 
-   if (!dma_map_sg(to_dma_dev(g2d->drm_dev), sgt->sgl, sgt->nents,
-   DMA_BIDIRECTIONAL)) {
+   sgt->nents = dma_map_sg(to_dma_dev(g2d->drm_dev), sgt->sgl,
+   sgt->orig_nents, DMA_BIDIRECTIONAL);
+   if (!sgt->nents) {
DRM_DEV_ERROR(g2d->dev, "failed to map sgt with dma region.\n");
ret = -ENOMEM;
goto err_sg_free_table;
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 07/21] drm: lima: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/lima/lima_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 5404e0d..3edd2ff 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -70,7 +70,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
 
if (bo->base.sgt) {
dma_unmap_sg(dev, bo->base.sgt->sgl,
-bo->base.sgt->nents, DMA_BIDIRECTIONAL);
+bo->base.sgt->orig_nents, DMA_BIDIRECTIONAL);
sg_free_table(bo->base.sgt);
} else {
bo->base.sgt = kmalloc(sizeof(*bo->base.sgt), GFP_KERNEL);
@@ -80,7 +80,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
}
}
 
-   dma_map_sg(dev, sgt.sgl, sgt.nents, DMA_BIDIRECTIONAL);
+   sgt.nents = dma_map_sg(dev, sgt.sgl, sgt.orig_nents, DMA_BIDIRECTIONAL);
 
*bo->base.sgt = sgt;
 
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 10/21] drm: radeon: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
Reviewed-by: Christian König 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 5d50c9e..f8275c8 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -481,7 +481,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
 {
struct radeon_device *rdev = radeon_get_rdev(ttm->bdev);
struct radeon_ttm_tt *gtt = (void *)ttm;
-   unsigned pinned = 0, nents;
+   unsigned pinned = 0;
int r;
 
int write = !(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
@@ -522,8 +522,9 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
goto release_sg;
 
r = -ENOMEM;
-   nents = dma_map_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
-   if (nents == 0)
+   ttm->sg->nents = dma_map_sg(rdev->dev, ttm->sg->sgl,
+   ttm->sg->orig_nents, direction);
+   if (ttm->sg->nents == 0)
goto release_sg;
 
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
@@ -554,9 +555,9 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt *ttm)
return;
 
/* free the sg table and pages again */
-   dma_unmap_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
+   dma_unmap_sg(rdev->dev, ttm->sg->sgl, ttm->sg->orig_nents, direction);
 
-   for_each_sg_page(ttm->sg->sgl, _iter, ttm->sg->nents, 0) {
+   for_each_sg_page(ttm->sg->sgl, _iter, ttm->sg->orig_nents, 0) {
struct page *page = sg_page_iter_page(_iter);
if (!(gtt->userflags & RADEON_GEM_USERPTR_READONLY))
set_page_dirty(page);
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 20/21] media: pci: fix common ALSA DMA-mapping related code

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/media/pci/cx23885/cx23885-alsa.c | 2 +-
 drivers/media/pci/cx25821/cx25821-alsa.c | 2 +-
 drivers/media/pci/cx88/cx88-alsa.c   | 2 +-
 drivers/media/pci/saa7134/saa7134-alsa.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-alsa.c 
b/drivers/media/pci/cx23885/cx23885-alsa.c
index df44ed7..3f366e4 100644
--- a/drivers/media/pci/cx23885/cx23885-alsa.c
+++ b/drivers/media/pci/cx23885/cx23885-alsa.c
@@ -129,7 +129,7 @@ static int cx23885_alsa_dma_unmap(struct cx23885_audio_dev 
*dev)
if (!buf->sglen)
return 0;
 
-   dma_unmap_sg(>pci->dev, buf->sglist, buf->sglen, 
PCI_DMA_FROMDEVICE);
+   dma_unmap_sg(>pci->dev, buf->sglist, buf->nr_pages, 
PCI_DMA_FROMDEVICE);
buf->sglen = 0;
return 0;
 }
diff --git a/drivers/media/pci/cx25821/cx25821-alsa.c 
b/drivers/media/pci/cx25821/cx25821-alsa.c
index 3016164..c40304d 100644
--- a/drivers/media/pci/cx25821/cx25821-alsa.c
+++ b/drivers/media/pci/cx25821/cx25821-alsa.c
@@ -193,7 +193,7 @@ static int cx25821_alsa_dma_unmap(struct cx25821_audio_dev 
*dev)
if (!buf->sglen)
return 0;
 
-   dma_unmap_sg(>pci->dev, buf->sglist, buf->sglen, 
PCI_DMA_FROMDEVICE);
+   dma_unmap_sg(>pci->dev, buf->sglist, buf->nr_pages, 
PCI_DMA_FROMDEVICE);
buf->sglen = 0;
return 0;
 }
diff --git a/drivers/media/pci/cx88/cx88-alsa.c 
b/drivers/media/pci/cx88/cx88-alsa.c
index 7d7acee..3c6fe6c 100644
--- a/drivers/media/pci/cx88/cx88-alsa.c
+++ b/drivers/media/pci/cx88/cx88-alsa.c
@@ -332,7 +332,7 @@ static int cx88_alsa_dma_unmap(struct cx88_audio_dev *dev)
if (!buf->sglen)
return 0;
 
-   dma_unmap_sg(>pci->dev, buf->sglist, buf->sglen,
+   dma_unmap_sg(>pci->dev, buf->sglist, buf->nr_pages,
 PCI_DMA_FROMDEVICE);
buf->sglen = 0;
return 0;
diff --git a/drivers/media/pci/saa7134/saa7134-alsa.c 
b/drivers/media/pci/saa7134/saa7134-alsa.c
index 544ca57..398c47f 100644
--- a/drivers/media/pci/saa7134/saa7134-alsa.c
+++ b/drivers/media/pci/saa7134/saa7134-alsa.c
@@ -313,7 +313,7 @@ static int saa7134_alsa_dma_unmap(struct saa7134_dev *dev)
if (!dma->sglen)
return 0;
 
-   dma_unmap_sg(>pci->dev, dma->sglist, dma->sglen, 
PCI_DMA_FROMDEVICE);
+   dma_unmap_sg(>pci->dev, dma->sglist, dma->nr_pages, 
PCI_DMA_FROMDEVICE);
dma->sglen = 0;
return 0;
 }
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 21/21] staging: ion: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/staging/android/ion/ion.c | 17 +
 drivers/staging/android/ion/ion_heap.c|  6 +++---
 drivers/staging/android/ion/ion_system_heap.c |  2 +-
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 38b51ea..b14170c 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -147,14 +147,14 @@ static struct sg_table *dup_sg_table(struct sg_table 
*table)
if (!new_table)
return ERR_PTR(-ENOMEM);
 
-   ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
+   ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL);
if (ret) {
kfree(new_table);
return ERR_PTR(-ENOMEM);
}
 
new_sg = new_table->sgl;
-   for_each_sg(table->sgl, sg, table->nents, i) {
+   for_each_sg(table->sgl, sg, table->orig_nents, i) {
memcpy(new_sg, sg, sizeof(*sg));
new_sg->dma_address = 0;
new_sg = sg_next(new_sg);
@@ -227,8 +227,9 @@ static struct sg_table *ion_map_dma_buf(struct 
dma_buf_attachment *attachment,
 
table = a->table;
 
-   if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
-   direction))
+   table->nents = dma_map_sg(attachment->dev, table->sgl,
+ table->orig_nents, direction);
+   if (!table->nents)
return ERR_PTR(-ENOMEM);
 
return table;
@@ -238,7 +239,7 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment 
*attachment,
  struct sg_table *table,
  enum dma_data_direction direction)
 {
-   dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+   dma_unmap_sg(attachment->dev, table->sgl, table->orig_nents, direction);
 }
 
 static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
@@ -297,7 +298,7 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf 
*dmabuf,
 
mutex_lock(>lock);
list_for_each_entry(a, >attachments, list) {
-   dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
+   dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->orig_nents,
direction);
}
 
@@ -320,8 +321,8 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf 
*dmabuf,
 
mutex_lock(>lock);
list_for_each_entry(a, >attachments, list) {
-   dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,
-  direction);
+   dma_sync_sg_for_device(a->dev, a->table->sgl,
+  a->table->orig_nents, direction);
}
mutex_unlock(>lock);
 
diff --git a/drivers/staging/android/ion/ion_heap.c 
b/drivers/staging/android/ion/ion_heap.c
index 0755b11..f2f7ca7 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -38,7 +38,7 @@ void *ion_heap_map_kernel(struct ion_heap *heap,
else
pgprot = pgprot_writecombine(PAGE_KERNEL);
 
-   for_each_sg(table->sgl, sg, table->nents, i) {
+   for_each_sg(table->sgl, sg, table->orig_nents, i) {
int npages_this_entry = PAGE_ALIGN(sg->length) / PAGE_SIZE;
struct page *page = sg_page(sg);
 
@@ -71,7 +71,7 @@ int ion_heap_map_user(struct ion_heap *heap, struct 
ion_buffer *buffer,
int i;
int ret;
 
-   for_each_sg(table->sgl, sg, table->nents, i) {
+   for_each_sg(table->sgl, sg, table->orig_nents, i) {
struct page *page = sg_page(sg);
unsigned long remainder = vma->vm_end - addr;
unsigned long len = sg->length;
@@ -142,7 +142,7 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer)
else
pgprot = pgprot_writecombine(PAGE_KERNEL);
 
-   return ion_heap_sglist_zero(table->sgl, table->nents, pgprot);
+   return ion_heap_sglist_zero(table->sgl, table->orig_nents, pgprot);
 }
 
 int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot)
diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index b83a1d1..34f6e81 100644
---

[PATCH v2 17/21] drm: rcar-du: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/media/platform/vsp1/vsp1_drm.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a4a45d6..b54a30f 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -912,8 +912,9 @@ int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt)
 * skip cache sync. This will need to be revisited when support for
 * non-coherent buffers will be added to the DU driver.
 */
-   return dma_map_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents,
-   DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+   sgt->nents = dma_map_sg_attrs(vsp1->bus_master, sgt->sgl,
+   sgt->orig_nents, DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+   return sgt->nents;
 }
 EXPORT_SYMBOL_GPL(vsp1_du_map_sg);
 
@@ -921,7 +922,7 @@ void vsp1_du_unmap_sg(struct device *dev, struct sg_table 
*sgt)
 {
struct vsp1_device *vsp1 = dev_get_drvdata(dev);
 
-   dma_unmap_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents,
+   dma_unmap_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->orig_nents,
   DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
 }
 EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg);
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 08/21] drm: msm: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/msm/msm_gem.c   | 8 
 drivers/gpu/drm/msm/msm_iommu.c | 3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 5a6a79f..54c3bbb 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -54,10 +54,10 @@ static void sync_for_device(struct msm_gem_object *msm_obj)
 
if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) {
dma_sync_sg_for_device(dev, msm_obj->sgt->sgl,
-   msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+   msm_obj->sgt->orig_nents, DMA_BIDIRECTIONAL);
} else {
dma_map_sg(dev, msm_obj->sgt->sgl,
-   msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+   msm_obj->sgt->orig_nents, DMA_BIDIRECTIONAL);
}
 }
 
@@ -67,10 +67,10 @@ static void sync_for_cpu(struct msm_gem_object *msm_obj)
 
if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) {
dma_sync_sg_for_cpu(dev, msm_obj->sgt->sgl,
-   msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+   msm_obj->sgt->orig_nents, DMA_BIDIRECTIONAL);
} else {
dma_unmap_sg(dev, msm_obj->sgt->sgl,
-   msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
+   msm_obj->sgt->orig_nents, DMA_BIDIRECTIONAL);
}
 }
 
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index ad58cfe..b0ca084 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -43,7 +43,8 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
struct msm_iommu *iommu = to_msm_iommu(mmu);
size_t ret;
 
-   ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot);
+   ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->orig_nents,
+  prot);
WARN_ON(!ret);
 
return (ret == len) ? 0 : -EINVAL;
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 14/21] drm: vmwgfx: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index bf0bc46..a5fd128 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -362,7 +362,7 @@ static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt 
*vmw_tt)
 {
struct device *dev = vmw_tt->dev_priv->dev->dev;
 
-   dma_unmap_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.nents,
+   dma_unmap_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.orig_nents,
DMA_BIDIRECTIONAL);
vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents;
 }
@@ -449,10 +449,10 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
if (unlikely(ret != 0))
goto out_sg_alloc_fail;
 
-   if (vsgt->num_pages > vmw_tt->sgt.nents) {
+   if (vsgt->num_pages > vmw_tt->sgt.orig_nents) {
uint64_t over_alloc =
sgl_size * (vsgt->num_pages -
-   vmw_tt->sgt.nents);
+   vmw_tt->sgt.orig_nents);
 
ttm_mem_global_free(glob, over_alloc);
vmw_tt->sg_alloc_size -= over_alloc;
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 15/21] drm: xen: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c 
b/drivers/gpu/drm/xen/xen_drm_front_gem.c
index f0b85e0..ba4bdc5 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -215,7 +215,7 @@ struct drm_gem_object *
return ERR_PTR(ret);
 
DRM_DEBUG("Imported buffer of size %zu with nents %u\n",
- size, sgt->nents);
+ size, sgt->orig_nents);
 
return _obj->base;
 }
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 18/21] xen: gntdev: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/xen/gntdev-dmabuf.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c
index 75d3bb9..ed749fd 100644
--- a/drivers/xen/gntdev-dmabuf.c
+++ b/drivers/xen/gntdev-dmabuf.c
@@ -248,7 +248,7 @@ static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf,
if (sgt) {
if (gntdev_dmabuf_attach->dir != DMA_NONE)
dma_unmap_sg_attrs(attach->dev, sgt->sgl,
-  sgt->nents,
+  sgt->orig_nents,
   gntdev_dmabuf_attach->dir,
   DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sgt);
@@ -288,8 +288,10 @@ static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf,
sgt = dmabuf_pages_to_sgt(gntdev_dmabuf->pages,
  gntdev_dmabuf->nr_pages);
if (!IS_ERR(sgt)) {
-   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC)) {
+   sgt->nents = dma_map_sg_attrs(attach->dev, sgt->sgl,
+ sgt->orig_nents, dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
+   if (!sgt->nents) {
sg_free_table(sgt);
kfree(sgt);
sgt = ERR_PTR(-ENOMEM);
@@ -625,7 +627,7 @@ static struct gntdev_dmabuf *dmabuf_imp_alloc_storage(int 
count)
 
/* Now convert sgt to array of pages and check for page validity. */
i = 0;
-   for_each_sg_page(sgt->sgl, _iter, sgt->nents, 0) {
+   for_each_sg_page(sgt->sgl, _iter, sgt->orig_nents, 0) {
struct page *page = sg_page_iter_page(_iter);
/*
 * Check if page is valid: this can happen if we are given
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 16/21] drm: host1x: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/host1x/job.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index a10643a..3ea185e 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -166,8 +166,9 @@ static unsigned int pin_job(struct host1x *host, struct 
host1x_job *job)
goto unpin;
}
 
-   err = dma_map_sg(dev, sgt->sgl, sgt->nents, dir);
-   if (!err) {
+   sgt->nents = dma_map_sg(dev, sgt->sgl, sgt->orig_nents,
+   dir);
+   if (!sgt->nents) {
err = -ENOMEM;
goto unpin;
}
@@ -217,7 +218,7 @@ static unsigned int pin_job(struct host1x *host, struct 
host1x_job *job)
}
 
if (!IS_ENABLED(CONFIG_TEGRA_HOST1X_FIREWALL) && host->domain) {
-   for_each_sg(sgt->sgl, sg, sgt->nents, j)
+   for_each_sg(sgt->sgl, sg, sgt->orig_nents, j)
gather_size += sg->length;
gather_size = iova_align(>iova, gather_size);
 
@@ -231,7 +232,7 @@ static unsigned int pin_job(struct host1x *host, struct 
host1x_job *job)
 
err = iommu_map_sg(host->domain,
iova_dma_addr(>iova, alloc),
-   sgt->sgl, sgt->nents, IOMMU_READ);
+   sgt->sgl, sgt->orig_nents, IOMMU_READ);
if (err == 0) {
__free_iova(>iova, alloc);
err = -EINVAL;
@@ -241,9 +242,9 @@ static unsigned int pin_job(struct host1x *host, struct 
host1x_job *job)
job->unpins[job->num_unpins].size = gather_size;
phys_addr = iova_dma_addr(>iova, alloc);
} else if (sgt) {
-   err = dma_map_sg(host->dev, sgt->sgl, sgt->nents,
-DMA_TO_DEVICE);
-   if (!err) {
+   sgt->nents = dma_map_sg(host->dev, sgt->sgl,
+   sgt->orig_nents, DMA_TO_DEVICE);
+   if (!sgt->nents) {
err = -ENOMEM;
goto unpin;
}
@@ -647,7 +648,7 @@ void host1x_job_unpin(struct host1x_job *job)
}
 
if (unpin->dev && sgt)
-   dma_unmap_sg(unpin->dev, sgt->sgl, sgt->nents,
+   dma_unmap_sg(unpin->dev, sgt->sgl, sgt->orig_nents,
 unpin->dir);
 
host1x_bo_unpin(dev, unpin->bo, sgt);
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 11/21] drm: rockchip: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 0d18846..a024c71 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -37,7 +37,7 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object 
*rk_obj)
rk_obj->dma_addr = rk_obj->mm.start;
 
ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
-  rk_obj->sgt->nents, prot);
+  rk_obj->sgt->orig_nents, prot);
if (ret < rk_obj->base.size) {
DRM_ERROR("failed to map buffer: size=%zd request_size=%zd\n",
  ret, rk_obj->base.size);
@@ -98,11 +98,11 @@ static int rockchip_gem_get_pages(struct 
rockchip_gem_object *rk_obj)
 * TODO: Replace this by drm_clflush_sg() once it can be implemented
 * without relying on symbols that are not exported.
 */
-   for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
+   for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->orig_nents, i)
sg_dma_address(s) = sg_phys(s);
 
-   dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
-  DMA_TO_DEVICE);
+   dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl,
+  rk_obj->sgt->orig_nents, DMA_TO_DEVICE);
 
return 0;
 
@@ -351,7 +351,8 @@ void rockchip_gem_free_object(struct drm_gem_object *obj)
rockchip_gem_iommu_unmap(rk_obj);
} else {
dma_unmap_sg(drm->dev, rk_obj->sgt->sgl,
-rk_obj->sgt->nents, DMA_BIDIRECTIONAL);
+rk_obj->sgt->orig_nents,
+DMA_BIDIRECTIONAL);
}
drm_prime_gem_destroy(obj, rk_obj->sgt);
} else {
@@ -493,14 +494,14 @@ static unsigned long 
rockchip_sg_get_contiguous_size(struct sg_table *sgt,
struct sg_table *sg,
struct rockchip_gem_object *rk_obj)
 {
-   int count = dma_map_sg(drm->dev, sg->sgl, sg->nents,
+   int count = dma_map_sg(drm->dev, sg->sgl, sg->orig_nents,
   DMA_BIDIRECTIONAL);
if (!count)
return -EINVAL;
 
if (rockchip_sg_get_contiguous_size(sg, count) < attach->dmabuf->size) {
DRM_ERROR("failed to map sg_table to contiguous linear 
address.\n");
-   dma_unmap_sg(drm->dev, sg->sgl, sg->nents,
+   dma_unmap_sg(drm->dev, sg->sgl, sg->orig_nents,
 DMA_BIDIRECTIONAL);
return -EINVAL;
}
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 04/21] drm: etnaviv: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index dc9ef30..a224a97 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -27,7 +27,8 @@ static void etnaviv_gem_scatter_map(struct etnaviv_gem_object 
*etnaviv_obj)
 * because display controller, GPU, etc. are not coherent.
 */
if (etnaviv_obj->flags & ETNA_BO_CACHE_MASK)
-   dma_map_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+   sgt->nents = dma_map_sg(dev->dev, sgt->sgl, sgt->orig_nents,
+   DMA_BIDIRECTIONAL);
 }
 
 static void etnaviv_gem_scatterlist_unmap(struct etnaviv_gem_object 
*etnaviv_obj)
@@ -51,7 +52,8 @@ static void etnaviv_gem_scatterlist_unmap(struct 
etnaviv_gem_object *etnaviv_obj
 * discard those writes.
 */
if (etnaviv_obj->flags & ETNA_BO_CACHE_MASK)
-   dma_unmap_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+   dma_unmap_sg(dev->dev, sgt->sgl, sgt->orig_nents,
+DMA_BIDIRECTIONAL);
 }
 
 /* called with etnaviv_obj->lock held */
@@ -405,7 +407,7 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
 
if (etnaviv_obj->flags & ETNA_BO_CACHED) {
dma_sync_sg_for_cpu(dev->dev, etnaviv_obj->sgt->sgl,
-   etnaviv_obj->sgt->nents,
+   etnaviv_obj->sgt->orig_nents,
etnaviv_op_to_dma_dir(op));
etnaviv_obj->last_cpu_prep_op = op;
}
@@ -422,7 +424,7 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)
/* fini without a prep is almost certainly a userspace error */
WARN_ON(etnaviv_obj->last_cpu_prep_op == 0);
dma_sync_sg_for_device(dev->dev, etnaviv_obj->sgt->sgl,
-   etnaviv_obj->sgt->nents,
+   etnaviv_obj->sgt->orig_nents,
etnaviv_op_to_dma_dir(etnaviv_obj->last_cpu_prep_op));
etnaviv_obj->last_cpu_prep_op = 0;
}
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 03/21] drm: armada: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/armada/armada_gem.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_gem.c 
b/drivers/gpu/drm/armada/armada_gem.c
index 976685f..749647f 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -407,8 +407,10 @@ int armada_gem_pwrite_ioctl(struct drm_device *dev, void 
*data,
sg_set_page(sg, page, PAGE_SIZE, 0);
}
 
-   if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0) {
-   num = sgt->nents;
+   sgt->nents = dma_map_sg(attach->dev, sgt->sgl, sgt->orig_nents,
+   dir);
+   if (sgt->nents == 0) {
+   num = sgt->orig_nents;
goto release;
}
} else if (dobj->page) {
@@ -418,7 +420,9 @@ int armada_gem_pwrite_ioctl(struct drm_device *dev, void 
*data,
 
sg_set_page(sgt->sgl, dobj->page, dobj->obj.size, 0);
 
-   if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0)
+   sgt->nents = dma_map_sg(attach->dev, sgt->sgl, sgt->orig_nents,
+   dir);
+   if (sgt->nents == 0)
goto free_table;
} else if (dobj->linear) {
/* Single contiguous physical region - no struct page */
@@ -449,11 +453,11 @@ static void armada_gem_prime_unmap_dma_buf(struct 
dma_buf_attachment *attach,
int i;
 
if (!dobj->linear)
-   dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+   dma_unmap_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir);
 
if (dobj->obj.filp) {
struct scatterlist *sg;
-   for_each_sg(sgt->sgl, sg, sgt->nents, i)
+   for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
put_page(sg_page(sg));
}
 
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 13/21] drm: virtio: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/virtio/virtgpu_object.c | 11 ++-
 drivers/gpu/drm/virtio/virtgpu_vq.c |  8 
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c 
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 6ccbd01..12f6bee 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -73,7 +73,8 @@ void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
if (shmem->pages) {
if (shmem->mapped) {
dma_unmap_sg(vgdev->vdev->dev.parent,
-shmem->pages->sgl, shmem->mapped,
+shmem->pages->sgl,
+shmem->pages->orig_nents,
 DMA_TO_DEVICE);
shmem->mapped = 0;
}
@@ -157,13 +158,13 @@ static int virtio_gpu_object_shmem_init(struct 
virtio_gpu_device *vgdev,
}
 
if (use_dma_api) {
-   shmem->mapped = dma_map_sg(vgdev->vdev->dev.parent,
+   shmem->pages->nents = dma_map_sg(vgdev->vdev->dev.parent,
   shmem->pages->sgl,
-  shmem->pages->nents,
+  shmem->pages->orig_nents,
   DMA_TO_DEVICE);
-   *nents = shmem->mapped;
+   *nents = shmem->mapped = shmem->pages->nents;
} else {
-   *nents = shmem->pages->nents;
+   *nents = shmem->pages->orig_nents;
}
 
*ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry),
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c 
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 9e663a5..661325b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -604,8 +604,8 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct 
virtio_gpu_device *vgdev,
 
if (use_dma_api)
dma_sync_sg_for_device(vgdev->vdev->dev.parent,
-  shmem->pages->sgl, shmem->pages->nents,
-  DMA_TO_DEVICE);
+  shmem->pages->sgl,
+  shmem->pages->orig_nents, DMA_TO_DEVICE);
 
cmd_p = virtio_gpu_alloc_cmd(vgdev, , sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
@@ -1020,8 +1020,8 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct 
virtio_gpu_device *vgdev,
 
if (use_dma_api)
dma_sync_sg_for_device(vgdev->vdev->dev.parent,
-  shmem->pages->sgl, shmem->pages->nents,
-  DMA_TO_DEVICE);
+  shmem->pages->sgl,
+  shmem->pages->orig_nents, DMA_TO_DEVICE);
 
cmd_p = virtio_gpu_alloc_cmd(vgdev, , sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 09/21] drm: panfrost: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/panfrost/panfrost_gem.c | 3 ++-
 drivers/gpu/drm/panfrost/panfrost_mmu.c | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c 
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 17b654e..22fec7c 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -42,7 +42,8 @@ static void panfrost_gem_free_object(struct drm_gem_object 
*obj)
for (i = 0; i < n_sgt; i++) {
if (bo->sgts[i].sgl) {
dma_unmap_sg(pfdev->dev, bo->sgts[i].sgl,
-bo->sgts[i].nents, 
DMA_BIDIRECTIONAL);
+bo->sgts[i].orig_nents,
+DMA_BIDIRECTIONAL);
sg_free_table(>sgts[i]);
}
}
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c 
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index ed28aeb..2d9b1f9 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -517,7 +517,9 @@ static int panfrost_mmu_map_fault_addr(struct 
panfrost_device *pfdev, int as,
if (ret)
goto err_pages;
 
-   if (!dma_map_sg(pfdev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL)) {
+   sgt->nents = dma_map_sg(pfdev->dev, sgt->sgl, sgt->orig_nents,
+   DMA_BIDIRECTIONAL);
+   if (!sgt->nents) {
ret = -EINVAL;
goto err_map;
}
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 12/21] drm: tegra: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/tegra/gem.c   | 25 +
 drivers/gpu/drm/tegra/plane.c | 13 +++--
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 6237681..5710ab4 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -98,8 +98,8 @@ static struct sg_table *tegra_bo_pin(struct device *dev, 
struct host1x_bo *bo,
 * the SG table needs to be copied to avoid overwriting any
 * other potential users of the original SG table.
 */
-   err = sg_alloc_table_from_sg(sgt, obj->sgt->sgl, 
obj->sgt->nents,
-GFP_KERNEL);
+   err = sg_alloc_table_from_sg(sgt, obj->sgt->sgl,
+obj->sgt->orig_nents, GFP_KERNEL);
if (err < 0)
goto free;
} else {
@@ -197,7 +197,7 @@ static int tegra_bo_iommu_map(struct tegra_drm *tegra, 
struct tegra_bo *bo)
bo->iova = bo->mm->start;
 
bo->size = iommu_map_sg(tegra->domain, bo->iova, bo->sgt->sgl,
-   bo->sgt->nents, prot);
+   bo->sgt->orig_nents, prot);
if (!bo->size) {
dev_err(tegra->drm->dev, "failed to map buffer\n");
err = -ENOMEM;
@@ -264,7 +264,7 @@ static struct tegra_bo *tegra_bo_alloc_object(struct 
drm_device *drm,
 static void tegra_bo_free(struct drm_device *drm, struct tegra_bo *bo)
 {
if (bo->pages) {
-   dma_unmap_sg(drm->dev, bo->sgt->sgl, bo->sgt->nents,
+   dma_unmap_sg(drm->dev, bo->sgt->sgl, bo->sgt->orig_nents,
 DMA_FROM_DEVICE);
drm_gem_put_pages(>gem, bo->pages, true, true);
sg_free_table(bo->sgt);
@@ -290,9 +290,9 @@ static int tegra_bo_get_pages(struct drm_device *drm, 
struct tegra_bo *bo)
goto put_pages;
}
 
-   err = dma_map_sg(drm->dev, bo->sgt->sgl, bo->sgt->nents,
-DMA_FROM_DEVICE);
-   if (err == 0) {
+   bo->sgt->nents = dma_map_sg(drm->dev, bo->sgt->sgl, bo->sgt->orig_nents,
+   DMA_FROM_DEVICE);
+   if (bo->sgt->nents == 0) {
err = -EFAULT;
goto free_sgt;
}
@@ -571,7 +571,8 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct 
*vma)
goto free;
}
 
-   if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0)
+   sgt->nents = dma_map_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir);
+   if (sgt->nents == 0)
goto free;
 
return sgt;
@@ -590,7 +591,7 @@ static void tegra_gem_prime_unmap_dma_buf(struct 
dma_buf_attachment *attach,
struct tegra_bo *bo = to_tegra_bo(gem);
 
if (bo->pages)
-   dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+   dma_unmap_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir);
 
sg_free_table(sgt);
kfree(sgt);
@@ -609,7 +610,7 @@ static int tegra_gem_prime_begin_cpu_access(struct dma_buf 
*buf,
struct drm_device *drm = gem->dev;
 
if (bo->pages)
-   dma_sync_sg_for_cpu(drm->dev, bo->sgt->sgl, bo->sgt->nents,
+   dma_sync_sg_for_cpu(drm->dev, bo->sgt->sgl, bo->sgt->orig_nents,
DMA_FROM_DEVICE);
 
return 0;
@@ -623,8 +624,8 @@ static int tegra_gem_prime_end_cpu_access(struct dma_buf 
*buf,
struct drm_device *drm = gem->dev;
 
if (bo->pages)
-   dma_sync_sg_for_device(drm->dev, bo->sgt->sgl, bo->sgt->nents,
-  DMA_TO_DEVICE);
+   dma_sync_sg_for_device(drm->dev, bo->sgt->sgl,
+  bo->sgt->orig_nents, DMA_TO_DEVICE);
 
return 0;
 }
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 9ccfb56..3982bf8 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -130,9 +130,10 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct 
tegra_plane_state *state)
}
 
if (sgt) {
-   err = dma_map_sg(dc->dev, sgt->sgl, sgt->nents,
-

[PATCH v2 02/21] drm: amdgpu: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
Reviewed-by: Christian König 
---
For more information, see '[PATCH v2 00/21] DRM: fix struct sg_table nents
vs. orig_nents misuse' thread: https://lkml.org/lkml/2020/5/4/373
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 7 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 43d8ed7..4df813e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -307,8 +307,9 @@ static struct sg_table *amdgpu_dma_buf_map(struct 
dma_buf_attachment *attach,
if (IS_ERR(sgt))
return sgt;
 
-   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC))
+   sgt->nents = dma_map_sg_attrs(attach->dev, sgt->sgl, 
sgt->orig_nents,
+ dir, DMA_ATTR_SKIP_CPU_SYNC);
+   if (!sgt->nents)
goto error_free;
break;
 
@@ -349,7 +350,7 @@ static void amdgpu_dma_buf_unmap(struct dma_buf_attachment 
*attach,
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
 
if (sgt->sgl->page_link) {
-   dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+   dma_unmap_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir);
sg_free_table(sgt);
kfree(sgt);
} else {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index eff1f73..1f8c507 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1043,7 +1043,6 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
struct amdgpu_ttm_tt *gtt = (void *)ttm;
-   unsigned nents;
int r;
 
int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
@@ -1059,8 +1058,9 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
 
/* Map SG to device */
r = -ENOMEM;
-   nents = dma_map_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
-   if (nents == 0)
+   ttm->sg->nents = dma_map_sg(adev->dev, ttm->sg->sgl,
+   ttm->sg->orig_nents, direction);
+   if (ttm->sg->nents == 0)
goto release_sg;
 
/* convert SG to linear array of pages and dma addresses */
@@ -1091,7 +1091,7 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt 
*ttm)
return;
 
/* unmap the pages mapped to the device */
-   dma_unmap_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
+   dma_unmap_sg(adev->dev, ttm->sg->sgl, ttm->sg->orig_nents, direction);
 
sg_free_table(ttm->sg);
 
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 01/21] drm: core: fix sg_table nents vs. orig_nents misuse

The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.

Signed-off-by: Marek Szyprowski 
---
 drivers/gpu/drm/drm_cache.c| 2 +-
 drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ---
 drivers/gpu/drm/drm_prime.c| 9 +
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 03e01b0..63bd497 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -127,7 +127,7 @@ static void drm_cache_flush_clflush(struct page *pages[],
struct sg_page_iter sg_iter;
 
mb(); /*CLFLUSH is ordered only by using memory barriers*/
-   for_each_sg_page(st->sgl, _iter, st->nents, 0)
+   for_each_sg_page(st->sgl, _iter, st->orig_nents, 0)
drm_clflush_page(sg_page_iter_page(_iter));
mb(); /*Make sure that all cache line entry is flushed*/
 
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index df31e57..f47caa7 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -118,7 +118,7 @@ void drm_gem_shmem_free_object(struct drm_gem_object *obj)
} else {
if (shmem->sgt) {
dma_unmap_sg(obj->dev->dev, shmem->sgt->sgl,
-shmem->sgt->nents, DMA_BIDIRECTIONAL);
+shmem->sgt->orig_nents, DMA_BIDIRECTIONAL);
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
}
@@ -396,7 +396,7 @@ void drm_gem_shmem_purge_locked(struct drm_gem_object *obj)
WARN_ON(!drm_gem_shmem_is_purgeable(shmem));
 
dma_unmap_sg(obj->dev->dev, shmem->sgt->sgl,
-shmem->sgt->nents, DMA_BIDIRECTIONAL);
+shmem->sgt->orig_nents, DMA_BIDIRECTIONAL);
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
shmem->sgt = NULL;
@@ -623,7 +623,8 @@ struct sg_table *drm_gem_shmem_get_pages_sgt(struct 
drm_gem_object *obj)
goto err_put_pages;
}
/* Map the pages for use by the h/w. */
-   dma_map_sg(obj->dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+   sgt->nents = dma_map_sg(obj->dev->dev, sgt->sgl, sgt->orig_nents,
+   DMA_BIDIRECTIONAL);
 
shmem->sgt = sgt;
 
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 282774e..f3e2d2a 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -626,8 +626,9 @@ struct sg_table *drm_gem_map_dma_buf(struct 
dma_buf_attachment *attach,
else
sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
 
-   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC)) {
+   sgt->nents = dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->orig_nents,
+ dir, DMA_ATTR_SKIP_CPU_SYNC);
+   if (!sgt->nents) {
sg_free_table(sgt);
kfree(sgt);
sgt = ERR_PTR(-ENOMEM);
@@ -652,7 +653,7 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment 
*attach,
if (!sgt)
return;
 
-   dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
+   dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->orig_nents, dir,
   DMA_ATTR_SKIP_CPU_SYNC);
sg_free_table(sgt);
kfree(sgt);
@@ -975,7 +976,7 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, 
struct page **pages,
 */
page_index = 0;
dma_index = 0;
-   for_each_sg(sgt->sgl, sg, sgt->nents, count) {
+   for_each_sg(sgt->sgl, sg, sgt->orig_nents, count) {
page_len = sg->length;
page = sg_page(sg);
dma_len = sg_dma_len(sg);
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 00/21] DRM: fix struct sg_table nents vs. orig_nents misuse

Dear All,

During the Exynos DRM GEM rework and fixing the issues in the
drm_prime_sg_to_page_addr_arrays() function [1] I've noticed that most
drivers in DRM framework incorrectly use nents and orig_nents entries of
the struct sg_table.

In case of the most DMA-mapping implementations exchanging those two
entries or using nents for all loops on the scatterlist is harmless,
because they both have the same value. There exists however a DMA-mapping
implementations, for which such incorrect usage breaks things. The nents
returned by dma_map_sg() might be lower than the nents passed as its
parameter and this is perfectly fine. DMA framework or IOMMU is allowed
to join consecutive chunks while mapping if such operation is supported
by the underlying HW (bus, bridge, IOMMU, etc). Example of the case
where dma_map_sg() might return 1 'DMA' chunk for the 4 'physical' pages
is described here [2]

The DMA-mapping framework documentation [3] states that dma_map_sg()
returns the numer of the created entries in the DMA address space.
However the subsequent calls to dma_sync_sg_for_{device,cpu} and
dma_unmap_sg must be called with the original number of entries passed to
dma_map_sg. The common pattern in DRM drivers were to assign the
dma_map_sg() return value to sg_table->nents and use that value for
the subsequent calls to dma_sync_sg_* or dma_unmap_sg functions. Also
the code iterated over nents times to access the pages stored in the
processed scatterlist, while it should use orig_nents as the numer of
the page entries.

I've tried to identify all such incorrect usage of sg_table->nents and
this is a result of my research. It looks that the incorrect pattern has
been copied over the many drivers mainly in the DRM subsystem. Too bad in
most cases it even worked correctly if the system used simple,
linear DMA-mapping implementation, for which swapping nents and
orig_nents doesn't make any difference.

The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix
it fully. The driver creatively uses sg_table->orig_nents to store the
size of the allocate scatterlist and ignores the number of the entries
returned by dma_map_sg function. In this patchset I only fixed the
sg_table objects exported by dmabuf related functions. I hope that I
didn't break anything there.

As a follow-up for this patchset I will prepare a common
dma_{map,sync,unmap}_sgtable() wrappers, which will use a proper sg_table
entries and call respective DMA-mapping functions. I hope this will help
to avoid issue like this in the future.

Patches are based on top of Linux next-20200504.

Best regards,
Marek Szyprowski

References:

[1] https://lkml.org/lkml/2020/3/27/555
[2] https://lkml.org/lkml/2020/3/29/65
[3] Documentation/DMA-API-HOWTO.txt

Changelog:

v2:
- dropped most of the changes to drm/i915
- added fixes for rcar-du, xen, media and ion
- fixed a few issues pointed by kbuild test robot
- added wide cc: list for each patch

v1:
https://lore.kernel.org/linux-iommu/c01c9766-9778-fd1f-f36e-2dc7bd376...@arm.com/T/#m879a727e4e46b5479ef8603994b1a006fb2aa337
- initial version

Patch summary:

Marek Szyprowski (21):
drm: core: fix sg_table nents vs. orig_nents misuse
drm: amdgpu: fix sg_table nents vs. orig_nents misuse
drm: armada: fix sg_table nents vs. orig_nents misuse
drm: etnaviv: fix sg_table nents vs. orig_nents misuse
drm: exynos: fix sg_table nents vs. orig_nents misuse
drm: i915: fix sg_table nents vs. orig_nents misuse for dmabuf objects
drm: lima: fix sg_table nents vs. orig_nents misuse
drm: msm: fix sg_table nents vs. orig_nents misuse
drm: panfrost: fix sg_table nents vs. orig_nents misuse
drm: radeon: fix sg_table nents vs. orig_nents misuse
drm: rockchip: fix sg_table nents vs. orig_nents misuse
drm: tegra: fix sg_table nents vs. orig_nents misuse
drm: virtio: fix sg_table nents vs. orig_nents misuse
drm: vmwgfx: fix sg_table nents vs. orig_nents misuse
drm: xen: fix sg_table nents vs. orig_nents misuse
drm: host1x: fix sg_table nents vs. orig_nents misuse
drm: rcar-du: fix sg_table nents vs. orig_nents misuse
xen: gntdev: fix sg_table nents vs. orig_nents misuse
dmabuf: fix sg_table nents vs. orig_nents misuse
media: pci: fix common ALSA DMA-mapping related code
staging: ion: fix sg_table nents vs. orig_nents misuse

Re: [PATCH v2] drm/ast: Don't check new mode if CRTC is being disabled



Am 04.05.20 um 14:12 schrieb Thomas Zimmermann:
> Suspending failed because there's no mode if the CRTC is being
> disabled. Early-out in this case. This fixes runtime PM for ast.
> 
> v2:
>   * add Tested-by/Reviewed-by tags

s/Reviewed-by/Reported-by

Will be fixed in v3.

>   * add Fixes tags and CC (Sam)
>   * improved comment
> 
> Signed-off-by: Thomas Zimmermann 
> Reported-by: Cary Garrett 
> Tested-by: Cary Garrett 
> Fixes: b48e1b6ffd28 ("drm/ast: Add CRTC helpers for atomic modesetting")
> Cc: Thomas Zimmermann 
> Cc: Gerd Hoffmann 
> Cc: Dave Airlie 
> Cc: Daniel Vetter 
> Cc: Sam Ravnborg 
> Cc:  # v5.6+
> ---
>  drivers/gpu/drm/ast/ast_mode.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 7a9f20a2fd303..0cbbb21edb4e1 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -801,6 +801,9 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc 
> *crtc,
>   return -EINVAL;
>   }
>  
> + if (!state->enable)
> + return 0; /* no mode checks if CRTC is being disabled */
> +
>   ast_state = to_ast_crtc_state(state);
>  
>   format = ast_state->format;
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 17/17] drm/mgag200: Replace VRAM helpers with SHMEM helpers

Hi

Am 04.05.20 um 14:29 schrieb Emil Velikov:
> Hi Thomas,
> 
> Just a couple of fly-by comments.
> 
> On Wed, 29 Apr 2020 at 15:33, Thomas Zimmermann  wrote:
> 
>> @@ -1618,21 +1640,13 @@ mgag200_simple_display_pipe_update(struct 
>> drm_simple_display_pipe *pipe,
>> struct mga_device *mdev = to_mga_device(dev);
>> struct drm_plane_state *state = plane->state;
>> struct drm_framebuffer *fb = state->fb;
>> -   struct drm_gem_vram_object *gbo;
>> -   s64 gpu_addr;
>> +   struct drm_rect damage;
>>
>> if (!fb)
>> return;
>>
>> -   gbo = drm_gem_vram_of_gem(fb->obj[0]);
>> -
>> -   gpu_addr = drm_gem_vram_offset(gbo);
>> -   if (drm_WARN_ON_ONCE(dev, gpu_addr < 0))
>> -   return; /* BUG: BO should have been pinned to VRAM. */
>> -
>> -   mgag200_set_format_regs(mdev, fb);
> This function seems to be missing from the handle_damage helper.
> 
> Is that intentional, something which should be squashed with another
> patch or it belongs in the helper?

Thanks for noticing. That line should not have been here at all.
Changing format registers might require an update to the PLL. And that
in turn requires a full modeset in _enable(). The enable function
already sets the format regs; this line can be removed.


> 
>> -   mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
>> -   mgag200_set_offset(mdev, fb);
>> +   if (drm_atomic_helper_damage_merged(old_state, state, ))
>> +   mgag200_handle_damage(mdev, fb, );
>>  }
> 
> 
> 
>>  int mgag200_mm_init(struct mga_device *mdev)
>>  {
> 
>> +   arch_io_reserve_memtype_wc(pci_resource_start(pdev, 0),
>> +  pci_resource_len(pdev, 0));
>>
>> -   arch_io_reserve_memtype_wc(pci_resource_start(dev->pdev, 0),
>> -  pci_resource_len(dev->pdev, 0));
>> +   mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(pdev, 0),
>> +pci_resource_len(pdev, 0));
>>
>> -   mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
>> -pci_resource_len(dev->pdev, 0));
> 
> Some spurious s/dev->pdev/pdev/g changes got in the way. Might as well
> do those separately...
> 
>> +   mdev->vram = ioremap(pci_resource_start(pdev, 0),
>> +pci_resource_len(pdev, 0));
>> +   if (!mdev->vram) {
>> +   ret = -ENOMEM;
>> +   goto err_arch_phys_wc_del;
>> +   }
>>
>> mdev->vram_fb_available = mdev->mc.vram_size;
>>
>> return 0;
>> +
>> +err_arch_phys_wc_del:
>> +   arch_phys_wc_del(mdev->fb_mtrr);
>> +   arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
>> +   pci_resource_len(dev->pdev, 0));
> ... and consistently?

Good points.

Best regards
Thomas

> 
> HTH
> Emil
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/17] drm/mgag200: Remove waiting from DPMS code

Hi

Am 04.05.20 um 14:10 schrieb Daniel Vetter:
> On Wed, Apr 29, 2020 at 04:32:36PM +0200, Thomas Zimmermann wrote:
>> The mgag200 drivers waits for the VSYNC flag to get signalled (i.e.,
>> the page flip happens) before changing DPMS settings. This doesn't work
>> reliably if no mode has been programmed. Therefore remove the waiting
>> code. Synchronization with page flips should be done by DRM's vblank
>> handlers anyway.
>>
>> Signed-off-by: Thomas Zimmermann 
> 
> This looks a bit dangerous, hw might get angry if we drop these waits.
> 
> Generally with atomic you should never have a situation in driver code
> where you expect the display to be on, but it isn't. So this should be
> fixable by making sure we're calling this dpms function at the right spot,
> e.g. for the enable path obviously the display is always going to be off,
> and for the disable path the display is guaranteed to be on. So maybe just
> a bool enable, or split the dpms function into two.

I think this code was taken from the X11 userspace driver, which does
that same waiting.

But it's waiting even for the DPMS_ON case. If the signal generation is
disabled, why does it wait for the vsync flag? After a while the code
times out from a timeout given in HZ/jiffies. I would have expected a
value in usec. All this makes it somewhat dubious and I doubt that it's
actually correct.

If we want to keep the waiting, I agree on splitting the code into an
enable and a disable function.

The driver also busy waited during pageflips (see patch 5). That should
really be done with interrupts.

Best regards
Thomas


> 
> The old legacy helpers where a lot more fast in this regard.
> -Daniel
> 
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 26 --
>>  1 file changed, 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
>> b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> index ee1cbe5decd71..884fc668a6dae 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> @@ -75,30 +75,6 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
>>  }
>>  }
>>  
>> -static inline void mga_wait_vsync(struct mga_device *mdev)
>> -{
>> -unsigned long timeout = jiffies + HZ/10;
>> -unsigned int status = 0;
>> -
>> -do {
>> -status = RREG32(MGAREG_Status);
>> -} while ((status & 0x08) && time_before(jiffies, timeout));
>> -timeout = jiffies + HZ/10;
>> -status = 0;
>> -do {
>> -status = RREG32(MGAREG_Status);
>> -} while (!(status & 0x08) && time_before(jiffies, timeout));
>> -}
>> -
>> -static inline void mga_wait_busy(struct mga_device *mdev)
>> -{
>> -unsigned long timeout = jiffies + HZ;
>> -unsigned int status = 0;
>> -do {
>> -status = RREG8(MGAREG_Status + 2);
>> -} while ((status & 0x01) && time_before(jiffies, timeout));
>> -}
>> -
>>  #define P_ARRAY_SIZE 9
>>  
>>  static int mga_g200se_set_plls(struct mga_device *mdev, long clock)
>> @@ -1435,8 +1411,6 @@ static void mga_crtc_dpms(struct drm_crtc *crtc, int 
>> mode)
>>  #endif
>>  WREG8(MGAREG_SEQ_INDEX, 0x01);
>>  seq1 |= RREG8(MGAREG_SEQ_DATA) & ~0x20;
>> -mga_wait_vsync(mdev);
>> -mga_wait_busy(mdev);
>>  WREG8(MGAREG_SEQ_DATA, seq1);
>>  msleep(20);
>>  WREG8(MGAREG_CRTCEXT_INDEX, 0x01);
>> -- 
>> 2.26.0
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 17/17] drm/mgag200: Replace VRAM helpers with SHMEM helpers

2020-05-04 Thread Emil Velikov

Hi Thomas,

Just a couple of fly-by comments.

On Wed, 29 Apr 2020 at 15:33, Thomas Zimmermann  wrote:

> @@ -1618,21 +1640,13 @@ mgag200_simple_display_pipe_update(struct 
> drm_simple_display_pipe *pipe,
> struct mga_device *mdev = to_mga_device(dev);
> struct drm_plane_state *state = plane->state;
> struct drm_framebuffer *fb = state->fb;
> -   struct drm_gem_vram_object *gbo;
> -   s64 gpu_addr;
> +   struct drm_rect damage;
>
> if (!fb)
> return;
>
> -   gbo = drm_gem_vram_of_gem(fb->obj[0]);
> -
> -   gpu_addr = drm_gem_vram_offset(gbo);
> -   if (drm_WARN_ON_ONCE(dev, gpu_addr < 0))
> -   return; /* BUG: BO should have been pinned to VRAM. */
> -
> -   mgag200_set_format_regs(mdev, fb);
This function seems to be missing from the handle_damage helper.

Is that intentional, something which should be squashed with another
patch or it belongs in the helper?

> -   mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
> -   mgag200_set_offset(mdev, fb);
> +   if (drm_atomic_helper_damage_merged(old_state, state, ))
> +   mgag200_handle_damage(mdev, fb, );
>  }



>  int mgag200_mm_init(struct mga_device *mdev)
>  {

> +   arch_io_reserve_memtype_wc(pci_resource_start(pdev, 0),
> +  pci_resource_len(pdev, 0));
>
> -   arch_io_reserve_memtype_wc(pci_resource_start(dev->pdev, 0),
> -  pci_resource_len(dev->pdev, 0));
> +   mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(pdev, 0),
> +pci_resource_len(pdev, 0));
>
> -   mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
> -pci_resource_len(dev->pdev, 0));

Some spurious s/dev->pdev/pdev/g changes got in the way. Might as well
do those separately...

> +   mdev->vram = ioremap(pci_resource_start(pdev, 0),
> +pci_resource_len(pdev, 0));
> +   if (!mdev->vram) {
> +   ret = -ENOMEM;
> +   goto err_arch_phys_wc_del;
> +   }
>
> mdev->vram_fb_available = mdev->mc.vram_size;
>
> return 0;
> +
> +err_arch_phys_wc_del:
> +   arch_phys_wc_del(mdev->fb_mtrr);
> +   arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
> +   pci_resource_len(dev->pdev, 0));
... and consistently?

HTH
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 207463] [amdgpu] System freeze / corrupted graphics

2020-05-04 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=207463

Rokas Kupstys (rokups+kernel-b...@zoho.com) changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |MOVED

--- Comment #3 from Rokas Kupstys (rokups+kernel-b...@zoho.com) ---
This makes sense. I am pretty sure i never observed this issue with LTS kernel
in the past, and now it is present. I will report it on mesa bugtracker. Thank
you for replying.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)

2020-05-04 Thread Pekka Paalanen

On Mon, 4 May 2020 13:00:02 +0200
Daniel Vetter  wrote:

> On Mon, May 4, 2020 at 11:49 AM Pekka Paalanen  wrote:
> >
> > On Thu, 30 Apr 2020 15:53:23 +0200
> > Daniel Vetter  wrote:
> >  
> > > On Wed, Apr 29, 2020 at 01:07:54PM +0300, Pekka Paalanen wrote:  
> > > > On Tue, 28 Apr 2020 16:51:57 +0200
> > > > Daniel Vetter  wrote:
> > > >  
> > > > > On Fri, Apr 24, 2020 at 11:32:16AM +0300, Pekka Paalanen wrote:  
> > > > > > On Thu, 23 Apr 2020 17:01:49 +0200
> > > > > > Daniel Vetter  wrote:
> > > > > >  
> > > > > > > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen 
> > > > > > >  wrote:  
> > > > > > > >
> > > > > > > > On Tue, 21 Apr 2020 14:15:52 +0200
> > > > > > > > Daniel Vetter  wrote:
> > > > > > > >  
> > > > > >

> > > Include a connector = you have a property for a connector included in your
> > > atomic update. Sounds like you do that, so if you have a real world
> > > link-status failure, then things go a bit boom already.  
> >
> > Are you talking about setting a connector's property, like "CRTC_ID",
> > "Content Protection", "HDCP Content Type"? Weston sets all of those on
> > every update if they exist. Or is it about any property on any
> > connector, not just a specific property on the specific connector, or
> > any property on the specific connector?  
> 
> Any property on the specific connector where link-status has gone "bad".

Ok, now I got it clearly. Thanks.

> > Also CRTC properties "MODE_ID" and "ACTIVE" are set on every update,
> > modeset or not. Weston just trusts that no-op changes in state do not
> > require a modeset  
> 
> Yup that's generally valid assumption, but also that's enough to hit
> the link-status == "bad" forced modeset I think.
> 
> > Is it not the opposite? If a link fails, then Weston will produce a
> > glitch when the kernel automatically re-trains the link, and then
> > everything continues happily?  
> 
> That's the soft link failure, where the kernel can recover on its own.
> There's the harder one where the new link status is degraded, and the
> old mode doesn't fit. Or the kernel just threw a fit and can't make
> stuff work anymore, and waits for the next userspace commit. Iirc for
> MST links we can't easily do this because locking, so gets pushed to a
> full modeset commit. Or something like that.
> 
> > > I guess we need a kernel patch to make sure link-status only gets fixed
> > > when userspace is ok with a modeset.  
> >
> > That would be nice, but would it not also mean the above Weston case
> > ends up with a permanent black screen instead of a temporary glitch?
> >
> > Or is there a hotplug uevent at play here too?  
> 
> Yeah you get a hotplug uevent, so as long as you eventually process
> that and do a full modeset should be ok.

I suppose Weston falls short there. Getting a hotplug event does not
force a modeset on all active CRTCs. Weston does re-read resources and
properties, but it does not trigger anything like re-picking the video
mode if the connection status and EDID remains the same.

I hope we can say Weston never worked with link failures to begin with,
and fix things properly through the link status property in Weston.

> > > > Weston does this, because it is the easy thing to do and debug. You
> > > > don't have to track back thousands of frames to figure out what the
> > > > mode or the connectors are, when something doesn't work like it should.
> > > > Or to figure out why some state wasn't emitted when it was supposed to.
> > > >  
> >
> > ...
> >  
> > > One idea I proposed on irc is that logind would capture the boot-up state,
> > > and you'd do a loginctl reset-screen cmd to reset it if something broke
> > > somewhere. logind already has the code for forced vt switching, that feels
> > > like a reasonable extension of that.  
> >
> > Needing to run a command manually to restore the screen instead of
> > "simply" switching to the graphical login manager to re-gain user
> > control seems a bit difficult.  
> 
> But that requires mind reading skills. Computer can't tell the "pls
> don't wreak stuff, I want smooth switching" from "pls wreak stuff,
> because it's glitching" wish the user has when vt-switching.

No, it doesn't. The graphical login manager already saved its default
state and it will always reinstate its default state. The session
compositor hopefully inherits the same or at least compatible default
state from the login manager, so the session compositor has it too. So
smooth hand-over is always possible unless the session compositor goes
out of its way to set otherwise. If the session compositor screws
something up like a gamma table, the login manager's default state will
just overwrite the bad data anyway. Whether that becomes a smooth
transition or not depends on whether the reset needs a modeset and if
the old FB is understandable.

The session will be permanently broken, as it will re-break when
switching back to it, but at least the login manager remains always
accessible.

> > > Then you could pick

[PATCH v2] drm/ast: Don't check new mode if CRTC is being disabled

Suspending failed because there's no mode if the CRTC is being
disabled. Early-out in this case. This fixes runtime PM for ast.

v2:
* add Tested-by/Reviewed-by tags
* add Fixes tags and CC (Sam)
* improved comment

Signed-off-by: Thomas Zimmermann 
Reported-by: Cary Garrett 
Tested-by: Cary Garrett 
Fixes: b48e1b6ffd28 ("drm/ast: Add CRTC helpers for atomic modesetting")
Cc: Thomas Zimmermann 
Cc: Gerd Hoffmann 
Cc: Dave Airlie 
Cc: Daniel Vetter 
Cc: Sam Ravnborg 
Cc:  # v5.6+
---
 drivers/gpu/drm/ast/ast_mode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 7a9f20a2fd303..0cbbb21edb4e1 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -801,6 +801,9 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc 
*crtc,
return -EINVAL;
}
 
+   if (!state->enable)
+   return 0; /* no mode checks if CRTC is being disabled */
+
ast_state = to_ast_crtc_state(state);
 
format = ast_state->format;
-- 
2.26.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/17] drm/mgag200: Remove waiting from DPMS code

On Wed, Apr 29, 2020 at 04:32:36PM +0200, Thomas Zimmermann wrote:
> The mgag200 drivers waits for the VSYNC flag to get signalled (i.e.,
> the page flip happens) before changing DPMS settings. This doesn't work
> reliably if no mode has been programmed. Therefore remove the waiting
> code. Synchronization with page flips should be done by DRM's vblank
> handlers anyway.
> 
> Signed-off-by: Thomas Zimmermann 

This looks a bit dangerous, hw might get angry if we drop these waits.

Generally with atomic you should never have a situation in driver code
where you expect the display to be on, but it isn't. So this should be
fixable by making sure we're calling this dpms function at the right spot,
e.g. for the enable path obviously the display is always going to be off,
and for the disable path the display is guaranteed to be on. So maybe just
a bool enable, or split the dpms function into two.

The old legacy helpers where a lot more fast in this regard.
-Daniel

> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 26 --
>  1 file changed, 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index ee1cbe5decd71..884fc668a6dae 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -75,30 +75,6 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
>   }
>  }
>  
> -static inline void mga_wait_vsync(struct mga_device *mdev)
> -{
> - unsigned long timeout = jiffies + HZ/10;
> - unsigned int status = 0;
> -
> - do {
> - status = RREG32(MGAREG_Status);
> - } while ((status & 0x08) && time_before(jiffies, timeout));
> - timeout = jiffies + HZ/10;
> - status = 0;
> - do {
> - status = RREG32(MGAREG_Status);
> - } while (!(status & 0x08) && time_before(jiffies, timeout));
> -}
> -
> -static inline void mga_wait_busy(struct mga_device *mdev)
> -{
> - unsigned long timeout = jiffies + HZ;
> - unsigned int status = 0;
> - do {
> - status = RREG8(MGAREG_Status + 2);
> - } while ((status & 0x01) && time_before(jiffies, timeout));
> -}
> -
>  #define P_ARRAY_SIZE 9
>  
>  static int mga_g200se_set_plls(struct mga_device *mdev, long clock)
> @@ -1435,8 +1411,6 @@ static void mga_crtc_dpms(struct drm_crtc *crtc, int 
> mode)
>  #endif
>   WREG8(MGAREG_SEQ_INDEX, 0x01);
>   seq1 |= RREG8(MGAREG_SEQ_DATA) & ~0x20;
> - mga_wait_vsync(mdev);
> - mga_wait_busy(mdev);
>   WREG8(MGAREG_SEQ_DATA, seq1);
>   msleep(20);
>   WREG8(MGAREG_CRTCEXT_INDEX, 0x01);
> -- 
> 2.26.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

Re: [PATCH] drm/ast: Don't check new mode if CRTC is being disabled

Hi Emil

Am 01.05.20 um 15:20 schrieb Emil Velikov:
> Hi Thomas,
> 
> Couple of fly-by ideas/suggestions.
> 
> On Thu, 30 Apr 2020 at 10:13, Thomas Zimmermann  wrote:
>>
>> Suspending failed because there's no mode if the CRTC is being
>> disabled. Early-out in this case. This fixes runtime PM for ast.
>>
>> Signed-off-by: Thomas Zimmermann 
>> ---
>>  drivers/gpu/drm/ast/ast_mode.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index 7a9f20a2fd303..089b7d9a0cf3f 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -801,6 +801,9 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc 
>> *crtc,
>> return -EINVAL;
> Unrelated:
> This feels quite dirty. If AST1180 does not support atomic modeset
> simply remove the DRIVER_ATOMIC bit.
> You can do that at runtime, via drm_device::driver_features in say,
> ast_detect_chip()?

The line you commented on dates back to non-atomic modesetting, but I
don't know what the story behind AST1180 is. It is explicitly disabled
in the list of PCI IDs, but the driver has plenty of code for it. It
looks as if the chip can only do pageflipping with a pre-set video mode.

As it is right now, the AST1180 code could probably be deleted entirely.

> 
> The drm_driver::driver_features is immutable, or it ought to be.
> 
>> }
>>
>> +   if (!state->enable)
>> +   return 0; /* no checks required if CRTC is being disabled */
>> +
> I cannot think of a reason why a driver would need to perform
> crtc_atomic_check, if the crtc is being disabled.
> Can you spot any? If not, this should be better served in core, which
> calls this callback.
> Correct?
Ast is a bit of a special case, because it tests the incoming mode
against a list of re-defined modes. With the crtc being disabled, the
incoming mode is 0 in all fields. Obviously that's not a valid mode, and
we need that additional test here.

In the general case, I'd see 'crtc check' as part of the larger atomic
infrastructure. I can imagine that configurations require the CRTC to be
enabled before other HW blocks work. So a driver might have a reason to
run crtc's check even for disabled crtcs (at least to verify that the
crtc is not disabled). I don't think this can be handled in the core easily.

Best regards
Thomas

> 
> HTH
> -Emil
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/10] drm: Add backlight helper

On Wed, Apr 29, 2020 at 02:48:22PM +0200, Noralf Trønnes wrote:
> This adds a function that creates a backlight device for a connector.
> It does not deal with the KMS backlight ABI proposition[1] to add a
> connector property. It only takes the current best practise to standardise
> the creation of a backlight device for DRM drivers while we wait for the
> property.
> 
> The brightness value is set using a connector state variable and an atomic
> commit.
> 
> I have looked through some of the backlight users and this is what I've found:
> 
> GNOME [2]
> -
> 
> Brightness range: 0-100
> Scale: Assumes perceptual
> 
> Avoids setting the sysfs brightness value to zero if max_brightness >= 99.
> Can connect connector and backlight using the sysfs device.
> 
> KDE [3]
> ---
> 
> Brightness range: 0-100
> Scale: Assumes perceptual
> 
> Weston [4]
> --
> 
> Brightness range: 0-255
> Scale: Assumes perceptual
> 
> Chromium OS [5]
> ---
> 
> Brightness range: 0-100
> Scale: Depends on the sysfs file 'scale' which is a recent addition (2019)
> 
> xserver [6]
> ---
> 
> Brightness range: 0-x (driver specific) (1 is minimum, 0 is OFF)
> Scale: Assumes perceptual
> 
> The builtin modesetting driver[7] does not support Backlight, Intel[8] does.
> 
> [1] 
> https://lore.kernel.org/dri-devel/4b17ba08-39f3-57dd-5aad-d37d844b0...@linux.intel.com/
> [2] 
> https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/blob/master/plugins/power/gsd-backlight.c
> [3] 
> https://github.com/KDE/powerdevil/blob/master/daemon/backends/upower/backlighthelper.cpp
> [4] 
> https://gitlab.freedesktop.org/wayland/weston/-/blob/master/libweston/backend-drm/drm.c
> [5] 
> https://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/master/power_manager/powerd/system/internal_backlight.cc
> [6] https://github.com/freedesktop/xorg-randrproto/blob/master/randrproto.txt
> [7] 
> https://gitlab.freedesktop.org/xorg/xserver/-/blob/master/hw/xfree86/drivers/modesetting/drmmode_display.c
> [8] 
> https://gitlab.freedesktop.org/xorg/driver/xf86-video-intel/-/blob/master/src/backlight.c
> 
> Cc: Hans de Goede 
> Cc: Jani Nikula 
> Cc: Martin Peres 
> Cc: Daniel Thompson 
> Signed-off-by: Noralf Trønnes 
> ---
>  Documentation/gpu/drm-kms-helpers.rst  |   6 +
>  drivers/gpu/drm/Kconfig|   7 ++
>  drivers/gpu/drm/Makefile   |   1 +
>  drivers/gpu/drm/drm_backlight_helper.c | 154 +
>  include/drm/drm_backlight_helper.h |   9 ++
>  include/drm/drm_connector.h|  10 ++
>  6 files changed, 187 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_backlight_helper.c
>  create mode 100644 include/drm/drm_backlight_helper.h
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst 
> b/Documentation/gpu/drm-kms-helpers.rst
> index 9668a7fe2408..29a2f4b49fd2 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -411,3 +411,9 @@ SHMEM GEM Helper Reference
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c
> :export:
> +
> +Backlight Helper Reference
> +==
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_backlight_helper.c
> +   :export:
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index d0aa6cff2e02..f6e13e18c9ca 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -224,6 +224,13 @@ config DRM_GEM_SHMEM_HELPER
>   help
> Choose this if you need the GEM shmem helper functions
>  
> +config DRM_BACKLIGHT_HELPER
> + bool
> + depends on DRM
> + select BACKLIGHT_CLASS_DEVICE
> + help
> +   Choose this if you need the backlight device helper functions
> +
>  config DRM_VM
>   bool
>   depends on DRM && MMU
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6493088a0fdd..0d17662dde0a 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -52,6 +52,7 @@ drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += 
> drm_fb_helper.o
>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>  drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
>  drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o
> +drm_kms_helper-$(CONFIG_DRM_BACKLIGHT_HELPER) += drm_backlight_helper.o
>  
>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>  obj-$(CONFIG_DRM_DEBUG_SELFTEST) += selftests/
> diff --git a/drivers/gpu/drm/drm_backlight_helper.c 
> b/drivers/gpu/drm/drm_backlight_helper.c
> new file mode 100644
> index ..06e6a75d1d0a
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_backlight_helper.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright 2020 Noralf Trønnes
> + */
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static int drm_backlight_update_status(struct backlight_device *bd)
> +{
> + struct drm_connector *connector = bl_get_data(bd);
> + struct

Re: [PATCH] drm: Replace drm_modeset_lock/unlock_all with DRM_MODESET_LOCK_ALL_* helpers

On Fri, May 01, 2020 at 05:49:33PM +0200, Michał Orzeł wrote:
> 
> 
> On 30.04.2020 20:30, Daniel Vetter wrote:
> > On Thu, Apr 30, 2020 at 5:38 PM Sean Paul  wrote:
> >>
> >> On Wed, Apr 29, 2020 at 4:57 AM Jani Nikula  
> >> wrote:
> >>>
> >>> On Tue, 28 Apr 2020, Michal Orzel  wrote:
>  As suggested by the TODO list for the kernel DRM subsystem, replace
>  the deprecated functions that take/drop modeset locks with new helpers.
> 
>  Signed-off-by: Michal Orzel 
>  ---
>   drivers/gpu/drm/drm_mode_object.c | 10 ++
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
>  diff --git a/drivers/gpu/drm/drm_mode_object.c 
>  b/drivers/gpu/drm/drm_mode_object.c
>  index 35c2719..901b078 100644
>  --- a/drivers/gpu/drm/drm_mode_object.c
>  +++ b/drivers/gpu/drm/drm_mode_object.c
>  @@ -402,12 +402,13 @@ int drm_mode_obj_get_properties_ioctl(struct 
>  drm_device *dev, void *data,
>   {
>    struct drm_mode_obj_get_properties *arg = data;
>    struct drm_mode_object *obj;
>  + struct drm_modeset_acquire_ctx ctx;
>    int ret = 0;
> 
>    if (!drm_core_check_feature(dev, DRIVER_MODESET))
>    return -EOPNOTSUPP;
> 
>  - drm_modeset_lock_all(dev);
>  + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
> >>>
> >>> I cry a little every time I look at the DRM_MODESET_LOCK_ALL_BEGIN and
> >>> DRM_MODESET_LOCK_ALL_END macros. :(
> >>>
> >>> Currently only six users... but there are ~60 calls to
> >>> drm_modeset_lock_all{,_ctx} that I presume are to be replaced. I wonder
> >>> if this will come back and haunt us.
> >>>
> >>
> >> What's the alternative? Seems like the options without the macros is
> >> to use incorrect scope or have a bunch of retry/backoff cargo-cult
> >> everywhere (and hope the copy source is done correctly).
> > 
> > Yeah Sean & me had a bunch of bikesheds and this is the least worst
> > option we could come up with. You can't make it a function because of
> > the control flow. You don't want to open code this because it's tricky
> > to get right, if all you want is to just grab all locks. But it is
> > magic hidden behind a macro, which occasionally ends up hurting.
> > -Daniel
> So what are we doing with this problem? Should we replace at once approx. 60 
> calls?

I'm confused by your question - dradual conversion is entirely orthogonal
to what exactly we're converting too. All I added here is that we've
discussed this at length, and the macro is the best thing we've come up
with. I still think it's the best compromise.

Flag-day conversion for over 60 calls doesn't work, no matter what.
-Daniel

> 
> Michal
> > 
> >> Sean
> >>
> >>> BR,
> >>> Jani.
> >>>
> >>>
> 
>    obj = drm_mode_object_find(dev, file_priv, arg->obj_id, 
>  arg->obj_type);
>    if (!obj) {
>  @@ -427,7 +428,7 @@ int drm_mode_obj_get_properties_ioctl(struct 
>  drm_device *dev, void *data,
>   out_unref:
>    drm_mode_object_put(obj);
>   out:
>  - drm_modeset_unlock_all(dev);
>  + DRM_MODESET_LOCK_ALL_END(ctx, ret);
>    return ret;
>   }
> 
>  @@ -449,12 +450,13 @@ static int set_property_legacy(struct 
>  drm_mode_object *obj,
>   {
>    struct drm_device *dev = prop->dev;
>    struct drm_mode_object *ref;
>  + struct drm_modeset_acquire_ctx ctx;
>    int ret = -EINVAL;
> 
>    if (!drm_property_change_valid_get(prop, prop_value, ))
>    return -EINVAL;
> 
>  - drm_modeset_lock_all(dev);
>  + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>    switch (obj->type) {
>    case DRM_MODE_OBJECT_CONNECTOR:
>    ret = drm_connector_set_obj_prop(obj, prop, prop_value);
>  @@ -468,7 +470,7 @@ static int set_property_legacy(struct 
>  drm_mode_object *obj,
>    break;
>    }
>    drm_property_change_valid_put(prop, ref);
>  - drm_modeset_unlock_all(dev);
>  + DRM_MODESET_LOCK_ALL_END(ctx, ret);
> 
>    return ret;
>   }
> >>>
> >>> --
> >>> Jani Nikula, Intel Open Source Graphics Center
> > 
> > 
> > 

-- 
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: enable render nodes wherever buffer sharing is supported

On Thu, Apr 30, 2020 at 12:57:40PM -0700, Eric Anholt wrote:
> On Thu, Apr 30, 2020 at 3:44 AM Daniel Vetter  wrote:
> >
> > On Tue, Apr 28, 2020 at 2:46 PM Peter Collingbourne  wrote:
> > >
> > > Render nodes are not just useful for devices supporting GPU hardware
> > > acceleration. Even on devices that only support dumb frame buffers,
> > > they are useful in situations where composition (using software
> > > rasterization) and KMS are done in different processes with buffer
> > > sharing being used to send frame buffers between them. This is the
> > > situation on Android, where surfaceflinger is the compositor and the
> > > composer HAL uses KMS to display the buffers. Thus it is beneficial
> > > to expose render nodes on all devices that support buffer sharing.
> > >
> > > Because all drivers that currently support render nodes also support
> > > buffer sharing, the DRIVER_RENDER flag is no longer necessary to mark
> > > devices as supporting render nodes, so remove it and just rely on
> > > the presence of a prime_handle_to_fd function pointer to determine
> > > whether buffer sharing is supported.
> >
> > The idea behind render nodes is that you can freely pass these to
> > unpriviledged users, and nothing bad happens. That's why we have gpu
> > reset code in drivers, proper pagetables, and also (in at least the
> > solid drivers) ban code so that repeat offenders from userspace who
> > constantly submit endless batch buffers and funny stuff like that
> > can't DOS the gpu.
> >
> > Ofc in practice there's hw issues and fun stuff like that sometimes,
> > and driver bugs, and all that. But that's the aspiration.
> >
> > Now many of these display-only drivers need contiguous buffers, and
> > there's not endless amounts of that around. So if you allow random
> > clients to allocate buffers, they can easily exhaust that, and not
> > just upset the render side of the gpu, but essentially make it
> > impossible for a compositor to allocate more framebuffers. I don't
> > think that's a good idea.
> >
> > I know there's hw like vc4 which needs contiguous buffers for
> > everything, but that's kinda the places where aspiration falls a bit
> > short.
> >
> > So from that pov I'm a rather worried with handing out render rights
> > to everyone for these display-only buffers. It's not entirely
> > harmless.
> 
> This doesn't feel like a contiguous-mem-specific concern to me.  We
> don't have resource limits on renderer GPU nodes today, you can
> allocate memory there to fill up and DOS the system, and unless
> something changed since last time I was looking, we don't even tell
> the OOM killer about our allocations so they can kill the right app!
> (my compositor always got picked, in my experience)
> 
> And, keep in mind what we're fixing at a system level here: the
> current workaround is you either get your compositor to hand you a GPU
> fd, at which point you can DOS the system, or you open the master node
> yourself and try to drop master before the compositor comes up, then
> DOS the system. "But there are access controls for the compositor or
> the card node!" you say: yes, but there are for render nodes too.  I
> know my distro doesn't default to open access to /dev/dri/render*

Hm indeed, I thought we've limited dumb buffer creation to master only on
the primary device.

But that kinda leaves me wondering, how do you allocate buffers on these
then? Exposing dumb on render nodes gets the Dave "it's not for rendering"
Airlie nack, at least for drivers which do have real userspace sitting on
the same drm driver. And exposing dumb only for these display-only devices
feels somewhat silly and inconsistent too.

So not sure.
-Daniel
-- 
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 v5 2/2] drm: Add support for the LogiCVC display controller

On Thu, Apr 30, 2020 at 09:10:07PM +0200, Paul Kocialkowski wrote:
> Hi Daniel,
> 
> On Fri 03 Apr 20, 13:04, Daniel Vetter wrote:
> > On Fri, Apr 03, 2020 at 11:36:17AM +0200, Paul Kocialkowski wrote:
> > > Introduces a driver for the LogiCVC display controller, a programmable
> > > logic controller optimized for use in Xilinx Zynq-7000 SoCs and other
> > > Xilinx FPGAs. The controller is mostly configured at logic synthesis
> > > time so only a subset of configuration is left for the driver to
> > > handle.
> > > 
> > > The following features are implemented and tested:
> > > - LVDS 4-bit interface;
> > > - RGB565 pixel formats;
> > > - Multiple layers and hardware composition;
> > > - Layer-wide alpha mode;
> > > 
> > > The following features are implemented but untested:
> > > - Other RGB pixel formats;
> > > - Layer framebuffer configuration for version 4;
> > > - Lowest-layer used as background color;
> > > - Per-pixel alpha mode.
> > > 
> > > The following features are not implemented:
> > > - YUV pixel formats;
> > > - DVI, LVDS 3-bit, ITU656 and camera link interfaces;
> > > - External parallel input for layer;
> > > - Color-keying;
> > > - LUT-based alpha modes.
> > > 
> > > Additional implementation-specific notes:
> > > - Panels are only enabled after the first page flip to avoid flashing a
> > >   white screen.
> > > - Depth used in context of the LogiCVC driver only counts color components
> > >   to match the definition of the synthesis parameters.
> > > 
> > > Support is implemented for both version 3 and 4 of the controller.
> > > 
> > > With version 3, framebuffers are stored in a dedicated contiguous
> > > memory area, with a base address hardcoded for each layer. This requires
> > > using a dedicated CMA pool registered at the base address and tweaking a
> > > few offset-related registers to try to use any buffer allocated from
> > > the pool. This is done on a best-effort basis to have the hardware cope
> > > with the DRM framebuffer allocation model and there is no guarantee
> > > that each buffer allocated by GEM CMA can be used for any layer.
> > > In particular, buffers allocated below the base address for a layer are
> > > guaranteed not to be configurable for that layer. See the implementation 
> > > of
> > > logicvc_layer_buffer_find_setup for specifics.
> > > 
> > > Version 4 allows configuring each buffer address directly, which
> > > guarantees that any buffer can be configured.
> > > 
> > > Signed-off-by: Paul Kocialkowski 
> > > Reviewed-by: Maxime Ripard 
> > 
> > Just a few comments on stuff I've spotted since I'm working in this area.
> 
> Thanks for the review (and the devm_drm_dev_alloc series)!
> 
> I'm preparing a new version taking in account your comments. However, I didn't
> find a suitable panel connector wrapper: all I see is a wrapper for the
> bridge + panel use case.

Yup that's the one I mean. drm_bridge is supposed to be the generic "glue
stuff into my output pipeline" thing. Including panels. It's a few more
layers of abstraction, but should still be a bunch less code for you.
-Daniel

> 
> Cheers,
> 
> Paul
> 
> > > ---
> > >  MAINTAINERS |   6 +
> > >  drivers/gpu/drm/Kconfig |   2 +
> > >  drivers/gpu/drm/Makefile|   1 +
> > >  drivers/gpu/drm/logicvc/Kconfig |   9 +
> > >  drivers/gpu/drm/logicvc/Makefile|   4 +
> > >  drivers/gpu/drm/logicvc/logicvc_crtc.c  | 272 +
> > >  drivers/gpu/drm/logicvc/logicvc_crtc.h  |  23 +
> > >  drivers/gpu/drm/logicvc/logicvc_drm.c   | 471 +++
> > >  drivers/gpu/drm/logicvc/logicvc_drm.h   |  60 ++
> > >  drivers/gpu/drm/logicvc/logicvc_interface.c | 240 
> > >  drivers/gpu/drm/logicvc/logicvc_interface.h |  32 ++
> > >  drivers/gpu/drm/logicvc/logicvc_layer.c | 603 
> > >  drivers/gpu/drm/logicvc/logicvc_layer.h |  65 +++
> > >  drivers/gpu/drm/logicvc/logicvc_mode.c  | 104 
> > >  drivers/gpu/drm/logicvc/logicvc_mode.h  |  15 +
> > >  drivers/gpu/drm/logicvc/logicvc_of.c| 205 +++
> > >  drivers/gpu/drm/logicvc/logicvc_of.h|  28 +
> > >  drivers/gpu/drm/logicvc/logicvc_regs.h  |  88 +++
> > >  18 files changed, 2228 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/logicvc/Kconfig
> > >  create mode 100644 drivers/gpu/drm/logicvc/Makefile
> > >  create mode 100644 drivers/gpu/drm/logicvc/logicvc_crtc.c
> > >  create mode 100644 drivers/gpu/drm/logicvc/logicvc_crtc.h
> > >  create mode 100644 drivers/gpu/drm/logicvc/logicvc_drm.c
> > >  create mode 100644 drivers/gpu/drm/logicvc/logicvc_drm.h
> > >  create mode 100644 drivers/gpu/drm/logicvc/logicvc_interface.c
> > >  create mode 100644 drivers/gpu/drm/logicvc/logicvc_interface.h
> > >  create mode 100644 drivers/gpu/drm/logicvc/logicvc_layer.c
> > >  create mode 100644 drivers/gpu/drm/logicvc/logicvc_layer.h
> > >  create mode 100644

Re: [PATCH V2 11/11] drm: Remove drm specific kmap_atomic code

On Mon, May 4, 2020 at 3:09 AM  wrote:
>
> From: Ira Weiny 
>
> kmap_atomic_prot() is now exported by all architectures.  Use this
> function rather than open coding a driver specific kmap_atomic.
>
> Reviewed-by: Christian König 
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: Ira Weiny 

I'm assuming this lands through some other tree or a topic branch or whatever.

Acked-by: Daniel Vetter 

Cheers, Daniel

> ---
>  drivers/gpu/drm/ttm/ttm_bo_util.c| 56 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 16 
>  include/drm/ttm/ttm_bo_api.h |  4 --
>  3 files changed, 12 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 52d2b71f1588..f09b096ba4fd 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -257,54 +257,6 @@ static int ttm_copy_io_page(void *dst, void *src, 
> unsigned long page)
> return 0;
>  }
>
> -#ifdef CONFIG_X86
> -#define __ttm_kmap_atomic_prot(__page, __prot) kmap_atomic_prot(__page, 
> __prot)
> -#define __ttm_kunmap_atomic(__addr) kunmap_atomic(__addr)
> -#else
> -#define __ttm_kmap_atomic_prot(__page, __prot) vmap(&__page, 1, 0,  __prot)
> -#define __ttm_kunmap_atomic(__addr) vunmap(__addr)
> -#endif
> -
> -
> -/**
> - * ttm_kmap_atomic_prot - Efficient kernel map of a single page with
> - * specified page protection.
> - *
> - * @page: The page to map.
> - * @prot: The page protection.
> - *
> - * This function maps a TTM page using the kmap_atomic api if available,
> - * otherwise falls back to vmap. The user must make sure that the
> - * specified page does not have an aliased mapping with a different caching
> - * policy unless the architecture explicitly allows it. Also mapping and
> - * unmapping using this api must be correctly nested. Unmapping should
> - * occur in the reverse order of mapping.
> - */
> -void *ttm_kmap_atomic_prot(struct page *page, pgprot_t prot)
> -{
> -   if (pgprot_val(prot) == pgprot_val(PAGE_KERNEL))
> -   return kmap_atomic(page);
> -   else
> -   return __ttm_kmap_atomic_prot(page, prot);
> -}
> -EXPORT_SYMBOL(ttm_kmap_atomic_prot);
> -
> -/**
> - * ttm_kunmap_atomic_prot - Unmap a page that was mapped using
> - * ttm_kmap_atomic_prot.
> - *
> - * @addr: The virtual address from the map.
> - * @prot: The page protection.
> - */
> -void ttm_kunmap_atomic_prot(void *addr, pgprot_t prot)
> -{
> -   if (pgprot_val(prot) == pgprot_val(PAGE_KERNEL))
> -   kunmap_atomic(addr);
> -   else
> -   __ttm_kunmap_atomic(addr);
> -}
> -EXPORT_SYMBOL(ttm_kunmap_atomic_prot);
> -
>  static int ttm_copy_io_ttm_page(struct ttm_tt *ttm, void *src,
> unsigned long page,
> pgprot_t prot)
> @@ -316,13 +268,13 @@ static int ttm_copy_io_ttm_page(struct ttm_tt *ttm, 
> void *src,
> return -ENOMEM;
>
> src = (void *)((unsigned long)src + (page << PAGE_SHIFT));
> -   dst = ttm_kmap_atomic_prot(d, prot);
> +   dst = kmap_atomic_prot(d, prot);
> if (!dst)
> return -ENOMEM;
>
> memcpy_fromio(dst, src, PAGE_SIZE);
>
> -   ttm_kunmap_atomic_prot(dst, prot);
> +   kunmap_atomic(dst);
>
> return 0;
>  }
> @@ -338,13 +290,13 @@ static int ttm_copy_ttm_io_page(struct ttm_tt *ttm, 
> void *dst,
> return -ENOMEM;
>
> dst = (void *)((unsigned long)dst + (page << PAGE_SHIFT));
> -   src = ttm_kmap_atomic_prot(s, prot);
> +   src = kmap_atomic_prot(s, prot);
> if (!src)
> return -ENOMEM;
>
> memcpy_toio(dst, src, PAGE_SIZE);
>
> -   ttm_kunmap_atomic_prot(src, prot);
> +   kunmap_atomic(src);
>
> return 0;
>  }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> index bb46ca0c458f..94d456a1d1a9 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> @@ -374,12 +374,12 @@ static int vmw_bo_cpu_blit_line(struct 
> vmw_bo_blit_line_data *d,
> copy_size = min_t(u32, copy_size, PAGE_SIZE - 
> src_page_offset);
>
> if (unmap_src) {
> -   ttm_kunmap_atomic_prot(d->src_addr, d->src_prot);
> +   kunmap_atomic(d->src_addr);
> d->src_addr = NULL;
> }
>
> if (unmap_dst) {
> -   ttm_kunmap_atomic_prot(d->dst_addr, d->dst_prot);
> +   kunmap_atomic(d->dst_addr);
> d->dst_addr = NULL;
> }
>
> @@ -388,8 +388,8 @@ static int vmw_bo_cpu_blit_line(struct 
> vmw_bo_blit_line_data *d,
> return -EINVAL;
>
> d->dst_addr =
> -   ttm_kmap_atomic_prot(d->dst_pages[dst_page],
> -

Re: [PATCH] uapi/drm/drm_fourcc.h: Note on platform specificity for format modifiers

On Mon, May 4, 2020 at 11:08 AM Mika Kahola  wrote:
>
> Make an additional note on DRM format modifiers for x and y tiling. These
> format modifiers are defined for BDW+ platforms and therefore definition
> is not valid for older gens. This is due to address swizzling for tiled
> surfaces is no longer used. For newer platforms main memory controller has
> a more effective address swizzling algorithm.
>
> Signed-off-by: Mika Kahola 
> ---
>  include/uapi/drm/drm_fourcc.h | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 8bc0b31597d8..3e56c24cabb6 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -356,7 +356,8 @@ extern "C" {
>   *
>   * This format is highly platforms specific and not useful for cross-driver
>   * sharing. It exists since on a given platform it does uniquely identify the
> - * layout in a simple way for i915-specific userspace.
> + * layout in a simple way for i915-specific userspace.  The format is
> + * ill-defined for platforms older than BDW.

Hm this is a bit confusing, since you're just adding a new sentences
and not really clarifying the paragraph overall. Right above we have
the detailed description of the modifier on gen8+, I'd replace the
entire paragraph here with something like:

"Note that this layout is only accurate on intel gen 8+ or valleyview
chipsets. On earlier platforms the is highly platforms specific and
not useful for cross-driver
sharing. It exists since on a given platform it does uniquely identify
the layout in a simple way for i915-specific userspace, which
facilitated conversion of userspace to modifiers. Additionally the
exact format on some really old platforms is not known."

That way it's much clearer that the specification is well-defined and
where, with the old stuff as exceptions. Your patch kinda makes bdw+
the exception of the exception, and that's hard to understand.

Cheers, Daniel

>   */
>  #define I915_FORMAT_MOD_X_TILEDfourcc_mod_code(INTEL, 1)
>
> @@ -371,7 +372,8 @@ extern "C" {
>   *
>   * This format is highly platforms specific and not useful for cross-driver
>   * sharing. It exists since on a given platform it does uniquely identify the
> - * layout in a simple way for i915-specific userspace.
> + * layout in a simple way for i915-specific userspace. The format is
> + * ill-defined for platforms older than BDW.
>   */
>  #define I915_FORMAT_MOD_Y_TILEDfourcc_mod_code(INTEL, 2)
>
> --
> 2.20.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)