[PATCH v3 0/8] drm/simpledrm: Support system memory framebuffers

[PATCH v3 0/8] drm/simpledrm: Support system memory framebuffers

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Hi,

this series of patches adds support for framebuffers residing in system
memory to the simple-framebuffer DRM driver. To do this, the DT bindings
are extended do accept the memory-region property in addition to the reg
property for specifying the framebuffer memory. This is done because the
framebuffer memory will typically also need to be marked as reserved so
that the operating system will not reuse it and the memory-region
property is the standard property to reference reserved memory regions.

A new compatible string is documented to annotate the framebuffer memory
regions and the simpledrm driver has code added to bind such annotated
regions to the simple-framebuffer device.

The second half of the series then adds support for the XB24 and AB24
formats and ties it all together to provide a simple-framebuffer on
Jetson Xavier NX. It should be noted, though, that the Jetson Xavier NX
device tree nodes are placeholders only and it is expected that firmware
or a bootloader will fill these in at runtime, due to the variable
nature of the values that they contain.

This example also uses (but doesn't depend on) the iommu-addresses
property that has been proposed and which will hopefully be merged soon.

Version 2 of these patches can be found here:

	https://lore.kernel.org/all/20221007124946.406808-1-thierry.reding@gmail.com/

Changes in v3:
- add new formats into conv_from_xrgb8888[] array to make it work after
  commit 6fdaed8c7988 ("drm/format-helper: Only advertise supported
  formats for conversion")
- extract iosys_map fix into a separate patch
- fix bogus increments in struct iosys_map usage
- simplify memory code

Changes in v2:
- DT fields are now cleared so that they can be filled in at runtime
- add XB24 support and treat AB24 the same (alpha bits are unused)
- consistently use struct iosys_map
- fix issues with DT bindings

I've tested these with a simple UEFI implementation that will fill in
the placeholder values and set the simple-framebuffer's status property
to "okay".

Thierry

Thierry Reding (8):
  dt-bindings: display: simple-framebuffer: Support system memory
    framebuffers
  dt-bindings: display: simple-framebuffer: Document 32-bit BGR format
  dt-bindings: reserved-memory: Support framebuffer reserved memory
  drm/simpledrm: Use struct iosys_map consistently
  drm/simpledrm: Add support for system memory framebuffers
  drm/format-helper: Support the XB24 format
  drm/simpledrm: Support the XB24/AB24 format
  arm64: tegra: Add simple framebuffer on Jetson Xavier NX

 .../bindings/display/simple-framebuffer.yaml  |   7 ++
 .../bindings/reserved-memory/framebuffer.yaml |  52 ++++++++
 .../nvidia/tegra194-p3509-0000+p3668-0001.dts |  43 +++++++
 arch/arm64/boot/dts/nvidia/tegra194.dtsi      |   2 +-
 drivers/gpu/drm/drm_format_helper.c           |  39 ++++++
 drivers/gpu/drm/tiny/simpledrm.c              | 114 +++++++++++++-----
 include/linux/platform_data/simplefb.h        |   1 +
 7 files changed, 227 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml

-- 
2.38.1

[PATCH v3 0/8] drm/simpledrm: Support system memory framebuffers

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Hi,

this series of patches adds support for framebuffers residing in system
memory to the simple-framebuffer DRM driver. To do this, the DT bindings
are extended do accept the memory-region property in addition to the reg
property for specifying the framebuffer memory. This is done because the
framebuffer memory will typically also need to be marked as reserved so
that the operating system will not reuse it and the memory-region
property is the standard property to reference reserved memory regions.

A new compatible string is documented to annotate the framebuffer memory
regions and the simpledrm driver has code added to bind such annotated
regions to the simple-framebuffer device.

The second half of the series then adds support for the XB24 and AB24
formats and ties it all together to provide a simple-framebuffer on
Jetson Xavier NX. It should be noted, though, that the Jetson Xavier NX
device tree nodes are placeholders only and it is expected that firmware
or a bootloader will fill these in at runtime, due to the variable
nature of the values that they contain.

This example also uses (but doesn't depend on) the iommu-addresses
property that has been proposed and which will hopefully be merged soon.

Version 2 of these patches can be found here:

	https://lore.kernel.org/all/20221007124946.406808-1-thierry.reding@gmail.com/

Changes in v3:
- add new formats into conv_from_xrgb8888[] array to make it work after
  commit 6fdaed8c7988 ("drm/format-helper: Only advertise supported
  formats for conversion")
- extract iosys_map fix into a separate patch
- fix bogus increments in struct iosys_map usage
- simplify memory code

Changes in v2:
- DT fields are now cleared so that they can be filled in at runtime
- add XB24 support and treat AB24 the same (alpha bits are unused)
- consistently use struct iosys_map
- fix issues with DT bindings

I've tested these with a simple UEFI implementation that will fill in
the placeholder values and set the simple-framebuffer's status property
to "okay".

Thierry

Thierry Reding (8):
  dt-bindings: display: simple-framebuffer: Support system memory
    framebuffers
  dt-bindings: display: simple-framebuffer: Document 32-bit BGR format
  dt-bindings: reserved-memory: Support framebuffer reserved memory
  drm/simpledrm: Use struct iosys_map consistently
  drm/simpledrm: Add support for system memory framebuffers
  drm/format-helper: Support the XB24 format
  drm/simpledrm: Support the XB24/AB24 format
  arm64: tegra: Add simple framebuffer on Jetson Xavier NX

 .../bindings/display/simple-framebuffer.yaml  |   7 ++
 .../bindings/reserved-memory/framebuffer.yaml |  52 ++++++++
 .../nvidia/tegra194-p3509-0000+p3668-0001.dts |  43 +++++++
 arch/arm64/boot/dts/nvidia/tegra194.dtsi      |   2 +-
 drivers/gpu/drm/drm_format_helper.c           |  39 ++++++
 drivers/gpu/drm/tiny/simpledrm.c              | 114 +++++++++++++-----
 include/linux/platform_data/simplefb.h        |   1 +
 7 files changed, 227 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml

-- 
2.38.1

[PATCH v3 1/8] dt-bindings: display: simple-framebuffer: Support system memory framebuffers

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

In order to support framebuffers residing in system memory, allow the
memory-region property to override the framebuffer memory specification
in the "reg" property.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 .../devicetree/bindings/display/simple-framebuffer.yaml      | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
index dd64f70b5014..3e9857eb002e 100644
--- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
+++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
@@ -63,6 +63,11 @@ properties:
   reg:
     description: Location and size of the framebuffer memory
 
+  memory-region:
+    maxItems: 1
+    description: Phandle to a node describing the memory to be used for the
+      framebuffer. If present, overrides the "reg" property (if one exists).
+
   clocks:
     description: List of clocks used by the framebuffer.
 
-- 
2.38.1

[PATCH v3 1/8] dt-bindings: display: simple-framebuffer: Support system memory framebuffers

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

In order to support framebuffers residing in system memory, allow the
memory-region property to override the framebuffer memory specification
in the "reg" property.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 .../devicetree/bindings/display/simple-framebuffer.yaml      | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
index dd64f70b5014..3e9857eb002e 100644
--- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
+++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
@@ -63,6 +63,11 @@ properties:
   reg:
     description: Location and size of the framebuffer memory
 
+  memory-region:
+    maxItems: 1
+    description: Phandle to a node describing the memory to be used for the
+      framebuffer. If present, overrides the "reg" property (if one exists).
+
   clocks:
     description: List of clocks used by the framebuffer.
 
-- 
2.38.1

[PATCH v3 2/8] dt-bindings: display: simple-framebuffer: Document 32-bit BGR format

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This is a variant of the 32-bit RGB format where the red and blue
components are swapped.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 .../devicetree/bindings/display/simple-framebuffer.yaml         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
index 3e9857eb002e..3c9f29e428a4 100644
--- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
+++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
@@ -99,6 +99,7 @@ properties:
         * `x1r5g5b5` - 16-bit pixels, d[14:10]=r, d[9:5]=g, d[4:0]=b
         * `x2r10g10b10` - 32-bit pixels, d[29:20]=r, d[19:10]=g, d[9:0]=b
         * `x8r8g8b8` - 32-bit pixels, d[23:16]=r, d[15:8]=g, d[7:0]=b
+        * `x8b8g8r8` - 32-bit pixels, d[23:16]=b, d[15:8]=g, d[7:0]=r
     enum:
       - a1r5g5b5
       - a2r10g10b10
@@ -110,6 +111,7 @@ properties:
       - x1r5g5b5
       - x2r10g10b10
       - x8r8g8b8
+      - x8b8g8r8
 
   display:
     $ref: /schemas/types.yaml#/definitions/phandle
-- 
2.38.1

[PATCH v3 2/8] dt-bindings: display: simple-framebuffer: Document 32-bit BGR format

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This is a variant of the 32-bit RGB format where the red and blue
components are swapped.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 .../devicetree/bindings/display/simple-framebuffer.yaml         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
index 3e9857eb002e..3c9f29e428a4 100644
--- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
+++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
@@ -99,6 +99,7 @@ properties:
         * `x1r5g5b5` - 16-bit pixels, d[14:10]=r, d[9:5]=g, d[4:0]=b
         * `x2r10g10b10` - 32-bit pixels, d[29:20]=r, d[19:10]=g, d[9:0]=b
         * `x8r8g8b8` - 32-bit pixels, d[23:16]=r, d[15:8]=g, d[7:0]=b
+        * `x8b8g8r8` - 32-bit pixels, d[23:16]=b, d[15:8]=g, d[7:0]=r
     enum:
       - a1r5g5b5
       - a2r10g10b10
@@ -110,6 +111,7 @@ properties:
       - x1r5g5b5
       - x2r10g10b10
       - x8r8g8b8
+      - x8b8g8r8
 
   display:
     $ref: /schemas/types.yaml#/definitions/phandle
-- 
2.38.1

[PATCH v3 3/8] dt-bindings: reserved-memory: Support framebuffer reserved memory

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Document the "framebuffer" compatible string for reserved memory nodes
to annotate reserved memory regions used for framebuffer carveouts.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- use four spaces for indentation in example (as recommended elsewhere)
- add explicit root node
- drop unneeded quotes

 .../bindings/reserved-memory/framebuffer.yaml | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml

diff --git a/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml b/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
new file mode 100644
index 000000000000..05b6648b3458
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/framebuffer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /reserved-memory framebuffer node bindings
+
+maintainers:
+  - devicetree-spec@vger.kernel.org
+
+allOf:
+  - $ref: reserved-memory.yaml
+
+properties:
+  compatible:
+    const: framebuffer
+    description: >
+      This indicates a region of memory meant to be used as a framebuffer for
+      a set of display devices. It can be used by an operating system to keep
+      the framebuffer from being overwritten and use it as the backing memory
+      for a display device (such as simple-framebuffer).
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    / {
+        compatible = "foo";
+        model = "foo";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        chosen {
+            framebuffer {
+                compatible = "simple-framebuffer";
+                memory-region = <&fb>;
+            };
+        };
+
+        reserved-memory {
+            #address-cells = <1>;
+            #size-cells = <1>;
+            ranges;
+
+            fb: framebuffer@80000000 {
+                compatible = "framebuffer";
+                reg = <0x80000000 0x007e9000>;
+            };
+        };
+    };
+...
-- 
2.38.1

[PATCH v3 3/8] dt-bindings: reserved-memory: Support framebuffer reserved memory

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Document the "framebuffer" compatible string for reserved memory nodes
to annotate reserved memory regions used for framebuffer carveouts.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- use four spaces for indentation in example (as recommended elsewhere)
- add explicit root node
- drop unneeded quotes

 .../bindings/reserved-memory/framebuffer.yaml | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml

diff --git a/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml b/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
new file mode 100644
index 000000000000..05b6648b3458
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/framebuffer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /reserved-memory framebuffer node bindings
+
+maintainers:
+  - devicetree-spec@vger.kernel.org
+
+allOf:
+  - $ref: reserved-memory.yaml
+
+properties:
+  compatible:
+    const: framebuffer
+    description: >
+      This indicates a region of memory meant to be used as a framebuffer for
+      a set of display devices. It can be used by an operating system to keep
+      the framebuffer from being overwritten and use it as the backing memory
+      for a display device (such as simple-framebuffer).
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    / {
+        compatible = "foo";
+        model = "foo";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        chosen {
+            framebuffer {
+                compatible = "simple-framebuffer";
+                memory-region = <&fb>;
+            };
+        };
+
+        reserved-memory {
+            #address-cells = <1>;
+            #size-cells = <1>;
+            ranges;
+
+            fb: framebuffer@80000000 {
+                compatible = "framebuffer";
+                reg = <0x80000000 0x007e9000>;
+            };
+        };
+    };
+...
-- 
2.38.1

[PATCH v3 4/8] drm/simpledrm: Use struct iosys_map consistently

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The majority of the driver already uses struct iosys_map to encapsulate
accesses to I/O remapped vs. system memory. Accesses via the screen base
pointer still use __iomem annotations, which can lead to inconsistencies
and conflicts with subsequent patches.

Convert the screen base to a struct iosys_map as well for consistency
and to avoid these issues.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- properly reinitialize struct iosys_map to avoid bogus increments

 drivers/gpu/drm/tiny/simpledrm.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 162eb44dcba8..3673a42e4bf4 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -208,7 +208,7 @@ struct simpledrm_device {
 	unsigned int pitch;
 
 	/* memory management */
-	void __iomem *screen_base;
+	struct iosys_map screen_base;
 
 	/* modesetting */
 	uint32_t formats[8];
@@ -492,15 +492,15 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
 
 	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
 	drm_atomic_for_each_plane_damage(&iter, &damage) {
-		struct iosys_map dst = IOSYS_MAP_INIT_VADDR(sdev->screen_base);
 		struct drm_rect dst_clip = plane_state->dst;
+		struct iosys_map screen = sdev->screen_base;
 
 		if (!drm_rect_intersect(&dst_clip, &damage))
 			continue;
 
-		iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
-		drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, shadow_plane_state->data, fb,
-			    &damage);
+		iosys_map_incr(&screen, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
+		drm_fb_blit(&screen, &sdev->pitch, sdev->format->format, shadow_plane_state->data,
+			    fb, &damage);
 	}
 
 	drm_dev_exit(idx);
@@ -519,7 +519,7 @@ static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plan
 		return;
 
 	/* Clear screen to black if disabled */
-	memset_io(sdev->screen_base, 0, sdev->pitch * sdev->mode.vdisplay);
+	iosys_map_memset(&sdev->screen_base, 0, 0, sdev->pitch * sdev->mode.vdisplay);
 
 	drm_dev_exit(idx);
 }
@@ -722,7 +722,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
 	if (!screen_base)
 		return ERR_PTR(-ENOMEM);
-	sdev->screen_base = screen_base;
+
+	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
 
 	/*
 	 * Modesetting
-- 
2.38.1

[PATCH v3 4/8] drm/simpledrm: Use struct iosys_map consistently

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The majority of the driver already uses struct iosys_map to encapsulate
accesses to I/O remapped vs. system memory. Accesses via the screen base
pointer still use __iomem annotations, which can lead to inconsistencies
and conflicts with subsequent patches.

Convert the screen base to a struct iosys_map as well for consistency
and to avoid these issues.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- properly reinitialize struct iosys_map to avoid bogus increments

 drivers/gpu/drm/tiny/simpledrm.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 162eb44dcba8..3673a42e4bf4 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -208,7 +208,7 @@ struct simpledrm_device {
 	unsigned int pitch;
 
 	/* memory management */
-	void __iomem *screen_base;
+	struct iosys_map screen_base;
 
 	/* modesetting */
 	uint32_t formats[8];
@@ -492,15 +492,15 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
 
 	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
 	drm_atomic_for_each_plane_damage(&iter, &damage) {
-		struct iosys_map dst = IOSYS_MAP_INIT_VADDR(sdev->screen_base);
 		struct drm_rect dst_clip = plane_state->dst;
+		struct iosys_map screen = sdev->screen_base;
 
 		if (!drm_rect_intersect(&dst_clip, &damage))
 			continue;
 
-		iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
-		drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, shadow_plane_state->data, fb,
-			    &damage);
+		iosys_map_incr(&screen, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
+		drm_fb_blit(&screen, &sdev->pitch, sdev->format->format, shadow_plane_state->data,
+			    fb, &damage);
 	}
 
 	drm_dev_exit(idx);
@@ -519,7 +519,7 @@ static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plan
 		return;
 
 	/* Clear screen to black if disabled */
-	memset_io(sdev->screen_base, 0, sdev->pitch * sdev->mode.vdisplay);
+	iosys_map_memset(&sdev->screen_base, 0, 0, sdev->pitch * sdev->mode.vdisplay);
 
 	drm_dev_exit(idx);
 }
@@ -722,7 +722,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
 	if (!screen_base)
 		return ERR_PTR(-ENOMEM);
-	sdev->screen_base = screen_base;
+
+	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
 
 	/*
 	 * Modesetting
-- 
2.38.1

[PATCH v3 5/8] drm/simpledrm: Add support for system memory framebuffers

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Simple framebuffers can be set up in system memory, which cannot be
requested and/or I/O remapped using the I/O resource helpers. Add a
separate code path that obtains system memory framebuffers from the
reserved memory region referenced in the memory-region property.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- simplify memory code and move back to simpledrm_device_create()
- extract screen_base iosys_map fix into separate patch

Changes in v2:
- make screen base a struct iosys_map to avoid sparse warnings

 drivers/gpu/drm/tiny/simpledrm.c | 99 ++++++++++++++++++++++++--------
 1 file changed, 75 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 3673a42e4bf4..7f39bc58da52 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -3,6 +3,7 @@
 #include <linux/clk.h>
 #include <linux/of_clk.h>
 #include <linux/minmax.h>
+#include <linux/of_address.h>
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
@@ -184,6 +185,31 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
 	return simplefb_get_validated_format(dev, format);
 }
 
+static struct resource *
+simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
+{
+	struct device_node *np;
+	struct resource *res;
+	int err;
+
+	np = of_parse_phandle(of_node, "memory-region", 0);
+	if (!np)
+		return NULL;
+
+	res = devm_kzalloc(dev->dev, sizeof(*res), GFP_KERNEL);
+	if (!res)
+		return ERR_PTR(-ENOMEM);
+
+	err = of_address_to_resource(np, 0, res);
+	if (err)
+		return ERR_PTR(err);
+
+	if (of_get_property(of_node, "reg", NULL))
+		drm_warn(dev, "preferring \"memory-region\" over \"reg\" property\n");
+
+	return res;
+}
+
 /*
  * Simple Framebuffer device
  */
@@ -623,8 +649,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	struct drm_device *dev;
 	int width, height, stride;
 	const struct drm_format_info *format;
-	struct resource *res, *mem;
-	void __iomem *screen_base;
+	struct resource *res, *mem = NULL;
 	struct drm_plane *primary_plane;
 	struct drm_crtc *crtc;
 	struct drm_encoder *encoder;
@@ -676,6 +701,9 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 		format = simplefb_get_format_of(dev, of_node);
 		if (IS_ERR(format))
 			return ERR_CAST(format);
+		mem = simplefb_get_memory_of(dev, of_node);
+		if (IS_ERR(mem))
+			return ERR_CAST(mem);
 	} else {
 		drm_err(dev, "no simplefb configuration found\n");
 		return ERR_PTR(-ENODEV);
@@ -698,32 +726,55 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	 * Memory management
 	 */
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return ERR_PTR(-EINVAL);
+	if (mem) {
+		void *screen_base;
 
-	ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
-	if (ret) {
-		drm_err(dev, "could not acquire memory range %pr: error %d\n", res, ret);
-		return ERR_PTR(ret);
-	}
+		ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem));
+		if (ret) {
+			drm_err(dev, "could not acquire memory range %pr: %d\n", mem, ret);
+			return ERR_PTR(ret);
+		}
 
-	mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), drv->name);
-	if (!mem) {
-		/*
-		 * We cannot make this fatal. Sometimes this comes from magic
-		 * spaces our resource handlers simply don't know about. Use
-		 * the I/O-memory resource as-is and try to map that instead.
-		 */
-		drm_warn(dev, "could not acquire memory region %pr\n", res);
-		mem = res;
-	}
+		drm_info(dev, "using system memory framebuffer at %pr\n", mem);
 
-	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
-	if (!screen_base)
-		return ERR_PTR(-ENOMEM);
+		screen_base = devm_memremap(dev->dev, mem->start, resource_size(mem), MEMREMAP_WB);
+		if (!screen_base)
+			return ERR_PTR(-ENOMEM);
+
+		iosys_map_set_vaddr(&sdev->screen_base, screen_base);
+	} else {
+		void __iomem *screen_base;
+
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		if (!res)
+			return ERR_PTR(-EINVAL);
 
-	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
+		ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
+		if (ret) {
+			drm_err(dev, "could not acquire memory range %pr: %d\n", &res, ret);
+			return ERR_PTR(ret);
+		}
+
+		drm_info(dev, "using I/O memory framebuffer at %pr\n", res);
+
+		mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
+					      drv->name);
+		if (!mem) {
+			/*
+			 * We cannot make this fatal. Sometimes this comes from magic
+			 * spaces our resource handlers simply don't know about. Use
+			 * the I/O-memory resource as-is and try to map that instead.
+			 */
+			drm_warn(dev, "could not acquire memory region %pr\n", res);
+			mem = res;
+		}
+
+		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
+		if (!screen_base)
+			return ERR_PTR(-ENOMEM);
+
+		iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
+	}
 
 	/*
 	 * Modesetting
-- 
2.38.1

[PATCH v3 5/8] drm/simpledrm: Add support for system memory framebuffers

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Simple framebuffers can be set up in system memory, which cannot be
requested and/or I/O remapped using the I/O resource helpers. Add a
separate code path that obtains system memory framebuffers from the
reserved memory region referenced in the memory-region property.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- simplify memory code and move back to simpledrm_device_create()
- extract screen_base iosys_map fix into separate patch

Changes in v2:
- make screen base a struct iosys_map to avoid sparse warnings

 drivers/gpu/drm/tiny/simpledrm.c | 99 ++++++++++++++++++++++++--------
 1 file changed, 75 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 3673a42e4bf4..7f39bc58da52 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -3,6 +3,7 @@
 #include <linux/clk.h>
 #include <linux/of_clk.h>
 #include <linux/minmax.h>
+#include <linux/of_address.h>
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
@@ -184,6 +185,31 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
 	return simplefb_get_validated_format(dev, format);
 }
 
+static struct resource *
+simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
+{
+	struct device_node *np;
+	struct resource *res;
+	int err;
+
+	np = of_parse_phandle(of_node, "memory-region", 0);
+	if (!np)
+		return NULL;
+
+	res = devm_kzalloc(dev->dev, sizeof(*res), GFP_KERNEL);
+	if (!res)
+		return ERR_PTR(-ENOMEM);
+
+	err = of_address_to_resource(np, 0, res);
+	if (err)
+		return ERR_PTR(err);
+
+	if (of_get_property(of_node, "reg", NULL))
+		drm_warn(dev, "preferring \"memory-region\" over \"reg\" property\n");
+
+	return res;
+}
+
 /*
  * Simple Framebuffer device
  */
@@ -623,8 +649,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	struct drm_device *dev;
 	int width, height, stride;
 	const struct drm_format_info *format;
-	struct resource *res, *mem;
-	void __iomem *screen_base;
+	struct resource *res, *mem = NULL;
 	struct drm_plane *primary_plane;
 	struct drm_crtc *crtc;
 	struct drm_encoder *encoder;
@@ -676,6 +701,9 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 		format = simplefb_get_format_of(dev, of_node);
 		if (IS_ERR(format))
 			return ERR_CAST(format);
+		mem = simplefb_get_memory_of(dev, of_node);
+		if (IS_ERR(mem))
+			return ERR_CAST(mem);
 	} else {
 		drm_err(dev, "no simplefb configuration found\n");
 		return ERR_PTR(-ENODEV);
@@ -698,32 +726,55 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	 * Memory management
 	 */
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return ERR_PTR(-EINVAL);
+	if (mem) {
+		void *screen_base;
 
-	ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
-	if (ret) {
-		drm_err(dev, "could not acquire memory range %pr: error %d\n", res, ret);
-		return ERR_PTR(ret);
-	}
+		ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem));
+		if (ret) {
+			drm_err(dev, "could not acquire memory range %pr: %d\n", mem, ret);
+			return ERR_PTR(ret);
+		}
 
-	mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), drv->name);
-	if (!mem) {
-		/*
-		 * We cannot make this fatal. Sometimes this comes from magic
-		 * spaces our resource handlers simply don't know about. Use
-		 * the I/O-memory resource as-is and try to map that instead.
-		 */
-		drm_warn(dev, "could not acquire memory region %pr\n", res);
-		mem = res;
-	}
+		drm_info(dev, "using system memory framebuffer at %pr\n", mem);
 
-	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
-	if (!screen_base)
-		return ERR_PTR(-ENOMEM);
+		screen_base = devm_memremap(dev->dev, mem->start, resource_size(mem), MEMREMAP_WB);
+		if (!screen_base)
+			return ERR_PTR(-ENOMEM);
+
+		iosys_map_set_vaddr(&sdev->screen_base, screen_base);
+	} else {
+		void __iomem *screen_base;
+
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		if (!res)
+			return ERR_PTR(-EINVAL);
 
-	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
+		ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
+		if (ret) {
+			drm_err(dev, "could not acquire memory range %pr: %d\n", &res, ret);
+			return ERR_PTR(ret);
+		}
+
+		drm_info(dev, "using I/O memory framebuffer at %pr\n", res);
+
+		mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
+					      drv->name);
+		if (!mem) {
+			/*
+			 * We cannot make this fatal. Sometimes this comes from magic
+			 * spaces our resource handlers simply don't know about. Use
+			 * the I/O-memory resource as-is and try to map that instead.
+			 */
+			drm_warn(dev, "could not acquire memory region %pr\n", res);
+			mem = res;
+		}
+
+		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
+		if (!screen_base)
+			return ERR_PTR(-ENOMEM);
+
+		iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
+	}
 
 	/*
 	 * Modesetting
-- 
2.38.1

[PATCH v3 6/8] drm/format-helper: Support the XB24 format

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add a conversion helper for the XB24 format to use in drm_fb_blit().

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- rebase onto latest drm-next

Changes in v2:
- support XB24 format instead of AB24

 drivers/gpu/drm/drm_format_helper.c | 39 +++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 74ff33c2ddaa..c8764cc61e87 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -503,6 +503,36 @@ static void drm_fb_rgb888_to_xrgb8888(struct iosys_map *dst, const unsigned int
 		    drm_fb_rgb888_to_xrgb8888_line);
 }
 
+static void drm_fb_xrgb8888_to_xbgr8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	__le32 *dbuf32 = dbuf;
+	const __le32 *sbuf32 = sbuf;
+	unsigned int x;
+	u32 pix;
+
+	for (x = 0; x < pixels; x++) {
+		pix = le32_to_cpu(sbuf32[x]);
+		pix = ((pix & 0x00ff0000) >> 16) <<  0 |
+		      ((pix & 0x0000ff00) >>  8) <<  8 |
+		      ((pix & 0x000000ff) >>  0) << 16 |
+		      0xff << 24;
+		*dbuf32++ = cpu_to_le32(pix);
+	}
+}
+
+static void drm_fb_xrgb8888_to_xbgr8888(struct iosys_map *dst, const unsigned int *dst_pitch,
+					const struct iosys_map *src,
+					const struct drm_framebuffer *fb,
+					const struct drm_rect *clip)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		4,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+		    drm_fb_xrgb8888_to_xbgr8888_line);
+}
+
 static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	__le32 *dbuf32 = dbuf;
@@ -646,6 +676,8 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 		fb_format = DRM_FORMAT_XRGB8888;
 	if (dst_format == DRM_FORMAT_ARGB8888)
 		dst_format = DRM_FORMAT_XRGB8888;
+	if (dst_format == DRM_FORMAT_ABGR8888)
+		dst_format = DRM_FORMAT_XBGR8888;
 	if (fb_format == DRM_FORMAT_ARGB2101010)
 		fb_format = DRM_FORMAT_XRGB2101010;
 	if (dst_format == DRM_FORMAT_ARGB2101010)
@@ -678,6 +710,11 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 			drm_fb_rgb565_to_xrgb8888(dst, dst_pitch, src, fb, clip);
 			return 0;
 		}
+	} else if (dst_format == DRM_FORMAT_XBGR8888) {
+		if (fb_format == DRM_FORMAT_XRGB8888) {
+			drm_fb_xrgb8888_to_xbgr8888(dst, dst_pitch, src, fb, clip);
+			return 0;
+		}
 	} else if (dst_format == DRM_FORMAT_XRGB2101010) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
 			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip);
@@ -820,6 +857,8 @@ static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, uint32_t
 static const uint32_t conv_from_xrgb8888[] = {
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_XRGB2101010,
 	DRM_FORMAT_ARGB2101010,
 	DRM_FORMAT_RGB565,
-- 
2.38.1

[PATCH v3 6/8] drm/format-helper: Support the XB24 format

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add a conversion helper for the XB24 format to use in drm_fb_blit().

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- rebase onto latest drm-next

Changes in v2:
- support XB24 format instead of AB24

 drivers/gpu/drm/drm_format_helper.c | 39 +++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 74ff33c2ddaa..c8764cc61e87 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -503,6 +503,36 @@ static void drm_fb_rgb888_to_xrgb8888(struct iosys_map *dst, const unsigned int
 		    drm_fb_rgb888_to_xrgb8888_line);
 }
 
+static void drm_fb_xrgb8888_to_xbgr8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	__le32 *dbuf32 = dbuf;
+	const __le32 *sbuf32 = sbuf;
+	unsigned int x;
+	u32 pix;
+
+	for (x = 0; x < pixels; x++) {
+		pix = le32_to_cpu(sbuf32[x]);
+		pix = ((pix & 0x00ff0000) >> 16) <<  0 |
+		      ((pix & 0x0000ff00) >>  8) <<  8 |
+		      ((pix & 0x000000ff) >>  0) << 16 |
+		      0xff << 24;
+		*dbuf32++ = cpu_to_le32(pix);
+	}
+}
+
+static void drm_fb_xrgb8888_to_xbgr8888(struct iosys_map *dst, const unsigned int *dst_pitch,
+					const struct iosys_map *src,
+					const struct drm_framebuffer *fb,
+					const struct drm_rect *clip)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		4,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+		    drm_fb_xrgb8888_to_xbgr8888_line);
+}
+
 static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	__le32 *dbuf32 = dbuf;
@@ -646,6 +676,8 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 		fb_format = DRM_FORMAT_XRGB8888;
 	if (dst_format == DRM_FORMAT_ARGB8888)
 		dst_format = DRM_FORMAT_XRGB8888;
+	if (dst_format == DRM_FORMAT_ABGR8888)
+		dst_format = DRM_FORMAT_XBGR8888;
 	if (fb_format == DRM_FORMAT_ARGB2101010)
 		fb_format = DRM_FORMAT_XRGB2101010;
 	if (dst_format == DRM_FORMAT_ARGB2101010)
@@ -678,6 +710,11 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 			drm_fb_rgb565_to_xrgb8888(dst, dst_pitch, src, fb, clip);
 			return 0;
 		}
+	} else if (dst_format == DRM_FORMAT_XBGR8888) {
+		if (fb_format == DRM_FORMAT_XRGB8888) {
+			drm_fb_xrgb8888_to_xbgr8888(dst, dst_pitch, src, fb, clip);
+			return 0;
+		}
 	} else if (dst_format == DRM_FORMAT_XRGB2101010) {
 		if (fb_format == DRM_FORMAT_XRGB8888) {
 			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip);
@@ -820,6 +857,8 @@ static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, uint32_t
 static const uint32_t conv_from_xrgb8888[] = {
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_XRGB2101010,
 	DRM_FORMAT_ARGB2101010,
 	DRM_FORMAT_RGB565,
-- 
2.38.1

[PATCH v3 7/8] drm/simpledrm: Support the XB24/AB24 format

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add XB24 and AB24 to the list of supported formats. The format helpers
support conversion to these formats and they are documented in the
simple-framebuffer device tree bindings.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- treat AB24 as XB24 and support both at the same time

 drivers/gpu/drm/tiny/simpledrm.c       | 2 ++
 include/linux/platform_data/simplefb.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 7f39bc58da52..ba1c2057fc65 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -483,6 +483,8 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
 static const uint32_t simpledrm_primary_plane_formats[] = {
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_RGB565,
 	//DRM_FORMAT_XRGB1555,
 	//DRM_FORMAT_ARGB1555,
diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
index 27ea99af6e1d..4f94d52ac99f 100644
--- a/include/linux/platform_data/simplefb.h
+++ b/include/linux/platform_data/simplefb.h
@@ -22,6 +22,7 @@
 	{ "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, \
 	{ "x8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_XRGB8888 }, \
 	{ "a8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {24, 8}, DRM_FORMAT_ARGB8888 }, \
+	{ "x8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {0, 0}, DRM_FORMAT_XBGR8888 }, \
 	{ "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {24, 8}, DRM_FORMAT_ABGR8888 }, \
 	{ "x2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {0, 0}, DRM_FORMAT_XRGB2101010 }, \
 	{ "a2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {30, 2}, DRM_FORMAT_ARGB2101010 }, \
-- 
2.38.1

[PATCH v3 7/8] drm/simpledrm: Support the XB24/AB24 format

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add XB24 and AB24 to the list of supported formats. The format helpers
support conversion to these formats and they are documented in the
simple-framebuffer device tree bindings.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- treat AB24 as XB24 and support both at the same time

 drivers/gpu/drm/tiny/simpledrm.c       | 2 ++
 include/linux/platform_data/simplefb.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 7f39bc58da52..ba1c2057fc65 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -483,6 +483,8 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
 static const uint32_t simpledrm_primary_plane_formats[] = {
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_RGB565,
 	//DRM_FORMAT_XRGB1555,
 	//DRM_FORMAT_ARGB1555,
diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
index 27ea99af6e1d..4f94d52ac99f 100644
--- a/include/linux/platform_data/simplefb.h
+++ b/include/linux/platform_data/simplefb.h
@@ -22,6 +22,7 @@
 	{ "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, \
 	{ "x8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_XRGB8888 }, \
 	{ "a8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {24, 8}, DRM_FORMAT_ARGB8888 }, \
+	{ "x8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {0, 0}, DRM_FORMAT_XBGR8888 }, \
 	{ "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {24, 8}, DRM_FORMAT_ABGR8888 }, \
 	{ "x2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {0, 0}, DRM_FORMAT_XRGB2101010 }, \
 	{ "a2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {30, 2}, DRM_FORMAT_ARGB2101010 }, \
-- 
2.38.1

[PATCH v3 8/8] arm64: tegra: Add simple framebuffer on Jetson Xavier NX

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add the framebuffer carveout reserved memory node as well as a simple-
framebuffer node that is used to bind to the framebuffer that the
bootloader has set up.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- clear out dynamic fields and leave it up to firmware to fill them in
- mark simple-framebuffer node as disabled by default

 .../nvidia/tegra194-p3509-0000+p3668-0001.dts | 43 +++++++++++++++++++
 arch/arm64/boot/dts/nvidia/tegra194.dtsi      |  2 +-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts b/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
index 238fd98e8e45..85b4aaa2ad4e 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
@@ -7,4 +7,47 @@
 / {
 	model = "NVIDIA Jetson Xavier NX Developer Kit (eMMC)";
 	compatible = "nvidia,p3509-0000+p3668-0001", "nvidia,tegra194";
+
+	chosen {
+		framebuffer {
+			compatible = "simple-framebuffer";
+			status = "disabled";
+			memory-region = <&fb>;
+			power-domains = <&bpmp TEGRA194_POWER_DOMAIN_DISP>;
+			clocks = <&bpmp TEGRA194_CLK_SOR1_REF>,
+				 <&bpmp TEGRA194_CLK_SOR1_OUT>,
+				 <&bpmp TEGRA194_CLK_SOR1_PAD_CLKOUT>,
+				 <&bpmp TEGRA194_CLK_PLLD2>,
+				 <&bpmp TEGRA194_CLK_PLLDP>,
+				 <&bpmp TEGRA194_CLK_NVDISPLAY_DISP>,
+				 <&bpmp TEGRA194_CLK_NVDISPLAYHUB>,
+				 <&bpmp TEGRA194_CLK_NVDISPLAY_P0>;
+			width = <0>;
+			height = <0>;
+			stride = <0>;
+			format = "x8b8g8r8";
+		};
+	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		fb: framebuffer@0,0 {
+			compatible = "framebuffer";
+			reg = <0x0 0x0 0x0 0x0>;
+			iommu-addresses = <&dc0 0x0 0x0 0x0 0x0>;
+		};
+	};
+
+	bus@0 {
+		host1x@13e00000 {
+			display-hub@15200000 {
+				display@15200000 {
+					memory-region = <&fb>;
+				};
+			};
+		};
+	};
 };
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index d0dbfafbc930..ec318b9e700c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -1972,7 +1972,7 @@ display-hub@15200000 {
 
 				ranges = <0x15200000 0x15200000 0x40000>;
 
-				display@15200000 {
+				dc0: display@15200000 {
 					compatible = "nvidia,tegra194-dc";
 					reg = <0x15200000 0x10000>;
 					interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.38.1

[PATCH v3 8/8] arm64: tegra: Add simple framebuffer on Jetson Xavier NX

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add the framebuffer carveout reserved memory node as well as a simple-
framebuffer node that is used to bind to the framebuffer that the
bootloader has set up.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- clear out dynamic fields and leave it up to firmware to fill them in
- mark simple-framebuffer node as disabled by default

 .../nvidia/tegra194-p3509-0000+p3668-0001.dts | 43 +++++++++++++++++++
 arch/arm64/boot/dts/nvidia/tegra194.dtsi      |  2 +-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts b/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
index 238fd98e8e45..85b4aaa2ad4e 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
@@ -7,4 +7,47 @@
 / {
 	model = "NVIDIA Jetson Xavier NX Developer Kit (eMMC)";
 	compatible = "nvidia,p3509-0000+p3668-0001", "nvidia,tegra194";
+
+	chosen {
+		framebuffer {
+			compatible = "simple-framebuffer";
+			status = "disabled";
+			memory-region = <&fb>;
+			power-domains = <&bpmp TEGRA194_POWER_DOMAIN_DISP>;
+			clocks = <&bpmp TEGRA194_CLK_SOR1_REF>,
+				 <&bpmp TEGRA194_CLK_SOR1_OUT>,
+				 <&bpmp TEGRA194_CLK_SOR1_PAD_CLKOUT>,
+				 <&bpmp TEGRA194_CLK_PLLD2>,
+				 <&bpmp TEGRA194_CLK_PLLDP>,
+				 <&bpmp TEGRA194_CLK_NVDISPLAY_DISP>,
+				 <&bpmp TEGRA194_CLK_NVDISPLAYHUB>,
+				 <&bpmp TEGRA194_CLK_NVDISPLAY_P0>;
+			width = <0>;
+			height = <0>;
+			stride = <0>;
+			format = "x8b8g8r8";
+		};
+	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		fb: framebuffer@0,0 {
+			compatible = "framebuffer";
+			reg = <0x0 0x0 0x0 0x0>;
+			iommu-addresses = <&dc0 0x0 0x0 0x0 0x0>;
+		};
+	};
+
+	bus@0 {
+		host1x@13e00000 {
+			display-hub@15200000 {
+				display@15200000 {
+					memory-region = <&fb>;
+				};
+			};
+		};
+	};
 };
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index d0dbfafbc930..ec318b9e700c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -1972,7 +1972,7 @@ display-hub@15200000 {
 
 				ranges = <0x15200000 0x15200000 0x40000>;
 
-				display@15200000 {
+				dc0: display@15200000 {
 					compatible = "nvidia,tegra194-dc";
 					reg = <0x15200000 0x10000>;
 					interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.38.1

Re: [PATCH v3 4/8] drm/simpledrm: Use struct iosys_map consistently

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3293 bytes --]

Hi

Am 17.11.22 um 19:40 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
> 
> The majority of the driver already uses struct iosys_map to encapsulate
> accesses to I/O remapped vs. system memory. Accesses via the screen base
> pointer still use __iomem annotations, which can lead to inconsistencies
> and conflicts with subsequent patches.
> 
> Convert the screen base to a struct iosys_map as well for consistency
> and to avoid these issues.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v3:
> - properly reinitialize struct iosys_map to avoid bogus increments
> 
>   drivers/gpu/drm/tiny/simpledrm.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 162eb44dcba8..3673a42e4bf4 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -208,7 +208,7 @@ struct simpledrm_device {
>   	unsigned int pitch;
>   
>   	/* memory management */
> -	void __iomem *screen_base;
> +	struct iosys_map screen_base;
>   
>   	/* modesetting */
>   	uint32_t formats[8];
> @@ -492,15 +492,15 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
>   
>   	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
>   	drm_atomic_for_each_plane_damage(&iter, &damage) {
> -		struct iosys_map dst = IOSYS_MAP_INIT_VADDR(sdev->screen_base);

We use dst throughout the code for such destination buffers. Please keep 
the name. With this fixed, you can add

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

to the patch.

Best regards
Thomas

>   		struct drm_rect dst_clip = plane_state->dst;
> +		struct iosys_map screen = sdev->screen_base;
>   
>   		if (!drm_rect_intersect(&dst_clip, &damage))
>   			continue;
>   
> -		iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
> -		drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, shadow_plane_state->data, fb,
> -			    &damage);
> +		iosys_map_incr(&screen, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
> +		drm_fb_blit(&screen, &sdev->pitch, sdev->format->format, shadow_plane_state->data,
> +			    fb, &damage);
>   	}
>   
>   	drm_dev_exit(idx);
> @@ -519,7 +519,7 @@ static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plan
>   		return;
>   
>   	/* Clear screen to black if disabled */
> -	memset_io(sdev->screen_base, 0, sdev->pitch * sdev->mode.vdisplay);
> +	iosys_map_memset(&sdev->screen_base, 0, 0, sdev->pitch * sdev->mode.vdisplay);
>   
>   	drm_dev_exit(idx);
>   }
> @@ -722,7 +722,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
>   	if (!screen_base)
>   		return ERR_PTR(-ENOMEM);
> -	sdev->screen_base = screen_base;
> +
> +	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
>   
>   	/*
>   	 * Modesetting

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 5/8] drm/simpledrm: Add support for system memory framebuffers

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 6160 bytes --]

Hi

Am 17.11.22 um 19:40 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
> 
> Simple framebuffers can be set up in system memory, which cannot be
> requested and/or I/O remapped using the I/O resource helpers. Add a
> separate code path that obtains system memory framebuffers from the
> reserved memory region referenced in the memory-region property.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v3:
> - simplify memory code and move back to simpledrm_device_create()
> - extract screen_base iosys_map fix into separate patch
> 
> Changes in v2:
> - make screen base a struct iosys_map to avoid sparse warnings
> 
>   drivers/gpu/drm/tiny/simpledrm.c | 99 ++++++++++++++++++++++++--------
>   1 file changed, 75 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 3673a42e4bf4..7f39bc58da52 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -3,6 +3,7 @@
>   #include <linux/clk.h>
>   #include <linux/of_clk.h>
>   #include <linux/minmax.h>
> +#include <linux/of_address.h>
>   #include <linux/platform_data/simplefb.h>
>   #include <linux/platform_device.h>
>   #include <linux/regulator/consumer.h>
> @@ -184,6 +185,31 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
>   	return simplefb_get_validated_format(dev, format);
>   }
>   
> +static struct resource *
> +simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
> +{
> +	struct device_node *np;
> +	struct resource *res;
> +	int err;
> +
> +	np = of_parse_phandle(of_node, "memory-region", 0);
> +	if (!np)
> +		return NULL;
> +
> +	res = devm_kzalloc(dev->dev, sizeof(*res), GFP_KERNEL);
> +	if (!res)
> +		return ERR_PTR(-ENOMEM);
> +
> +	err = of_address_to_resource(np, 0, res);
> +	if (err)
> +		return ERR_PTR(err);
> +
> +	if (of_get_property(of_node, "reg", NULL))
> +		drm_warn(dev, "preferring \"memory-region\" over \"reg\" property\n");
> +
> +	return res;
> +}
> +
>   /*
>    * Simple Framebuffer device
>    */
> @@ -623,8 +649,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   	struct drm_device *dev;
>   	int width, height, stride;
>   	const struct drm_format_info *format;
> -	struct resource *res, *mem;
> -	void __iomem *screen_base;
> +	struct resource *res, *mem = NULL;
>   	struct drm_plane *primary_plane;
>   	struct drm_crtc *crtc;
>   	struct drm_encoder *encoder;
> @@ -676,6 +701,9 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   		format = simplefb_get_format_of(dev, of_node);
>   		if (IS_ERR(format))
>   			return ERR_CAST(format);
> +		mem = simplefb_get_memory_of(dev, of_node);
> +		if (IS_ERR(mem))
> +			return ERR_CAST(mem);
>   	} else {
>   		drm_err(dev, "no simplefb configuration found\n");
>   		return ERR_PTR(-ENODEV);
> @@ -698,32 +726,55 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   	 * Memory management
>   	 */
>   
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res)
> -		return ERR_PTR(-EINVAL);
> +	if (mem) {
> +		void *screen_base;
>   
> -	ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
> -	if (ret) {
> -		drm_err(dev, "could not acquire memory range %pr: error %d\n", res, ret);
> -		return ERR_PTR(ret);
> -	}
> +		ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem));
> +		if (ret) {
> +			drm_err(dev, "could not acquire memory range %pr: %d\n", mem, ret);
> +			return ERR_PTR(ret);
> +		}
>   
> -	mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), drv->name);
> -	if (!mem) {
> -		/*
> -		 * We cannot make this fatal. Sometimes this comes from magic
> -		 * spaces our resource handlers simply don't know about. Use
> -		 * the I/O-memory resource as-is and try to map that instead.
> -		 */
> -		drm_warn(dev, "could not acquire memory region %pr\n", res);
> -		mem = res;
> -	}
> +		drm_info(dev, "using system memory framebuffer at %pr\n", mem);

drm_dbg() please.

>   
> -	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> -	if (!screen_base)
> -		return ERR_PTR(-ENOMEM);
> +		screen_base = devm_memremap(dev->dev, mem->start, resource_size(mem), MEMREMAP_WB);
> +		if (!screen_base)
> +			return ERR_PTR(-ENOMEM);
> +
> +		iosys_map_set_vaddr(&sdev->screen_base, screen_base);
> +	} else {
> +		void __iomem *screen_base;
> +
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +		if (!res)
> +			return ERR_PTR(-EINVAL);
>   
> -	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
> +		ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
> +		if (ret) {
> +			drm_err(dev, "could not acquire memory range %pr: %d\n", &res, ret);
> +			return ERR_PTR(ret);
> +		}
> +
> +		drm_info(dev, "using I/O memory framebuffer at %pr\n", res);

drm_dbg()

With the small fixes, please add

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regards
Thomas

> +
> +		mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
> +					      drv->name);
> +		if (!mem) {
> +			/*
> +			 * We cannot make this fatal. Sometimes this comes from magic
> +			 * spaces our resource handlers simply don't know about. Use
> +			 * the I/O-memory resource as-is and try to map that instead.
> +			 */
> +			drm_warn(dev, "could not acquire memory region %pr\n", res);
> +			mem = res;
> +		}
> +
> +		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> +		if (!screen_base)
> +			return ERR_PTR(-ENOMEM);
> +
> +		iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
> +	}
>   
>   	/*
>   	 * Modesetting

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 5/8] drm/simpledrm: Add support for system memory framebuffers

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 6531 bytes --]

Hi

Am 17.11.22 um 19:40 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
> 
> Simple framebuffers can be set up in system memory, which cannot be
> requested and/or I/O remapped using the I/O resource helpers. Add a
> separate code path that obtains system memory framebuffers from the
> reserved memory region referenced in the memory-region property.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v3:
> - simplify memory code and move back to simpledrm_device_create()
> - extract screen_base iosys_map fix into separate patch
> 
> Changes in v2:
> - make screen base a struct iosys_map to avoid sparse warnings
> 
>   drivers/gpu/drm/tiny/simpledrm.c | 99 ++++++++++++++++++++++++--------
>   1 file changed, 75 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 3673a42e4bf4..7f39bc58da52 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -3,6 +3,7 @@
>   #include <linux/clk.h>
>   #include <linux/of_clk.h>
>   #include <linux/minmax.h>
> +#include <linux/of_address.h>
>   #include <linux/platform_data/simplefb.h>
>   #include <linux/platform_device.h>
>   #include <linux/regulator/consumer.h>
> @@ -184,6 +185,31 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
>   	return simplefb_get_validated_format(dev, format);
>   }
>   
> +static struct resource *
> +simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
> +{
> +	struct device_node *np;
> +	struct resource *res;
> +	int err;
> +
> +	np = of_parse_phandle(of_node, "memory-region", 0);
> +	if (!np)
> +		return NULL;
> +
> +	res = devm_kzalloc(dev->dev, sizeof(*res), GFP_KERNEL);
> +	if (!res)
> +		return ERR_PTR(-ENOMEM);
> +
> +	err = of_address_to_resource(np, 0, res);
> +	if (err)
> +		return ERR_PTR(err);
> +
> +	if (of_get_property(of_node, "reg", NULL))
> +		drm_warn(dev, "preferring \"memory-region\" over \"reg\" property\n");

The reg property is converted to a device resource when we create the 
device at [1].

I have another question, which I was discussing with Javier recently. Is 
it possible to handle memory-region there automatically? If, for 
exmaple, it would create a resource with IORESOURCE_CACHEABLE, simpledrm 
would handle it as memory region. Without the CACHEABLE flag, it would 
be a regular resource as before.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v6.0.9/source/drivers/of/platform.c#L586

> +
> +	return res;
> +}
> +
>   /*
>    * Simple Framebuffer device
>    */
> @@ -623,8 +649,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   	struct drm_device *dev;
>   	int width, height, stride;
>   	const struct drm_format_info *format;
> -	struct resource *res, *mem;
> -	void __iomem *screen_base;
> +	struct resource *res, *mem = NULL;
>   	struct drm_plane *primary_plane;
>   	struct drm_crtc *crtc;
>   	struct drm_encoder *encoder;
> @@ -676,6 +701,9 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   		format = simplefb_get_format_of(dev, of_node);
>   		if (IS_ERR(format))
>   			return ERR_CAST(format);
> +		mem = simplefb_get_memory_of(dev, of_node);
> +		if (IS_ERR(mem))
> +			return ERR_CAST(mem);
>   	} else {
>   		drm_err(dev, "no simplefb configuration found\n");
>   		return ERR_PTR(-ENODEV);
> @@ -698,32 +726,55 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   	 * Memory management
>   	 */
>   
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res)
> -		return ERR_PTR(-EINVAL);
> +	if (mem) {
> +		void *screen_base;
>   
> -	ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
> -	if (ret) {
> -		drm_err(dev, "could not acquire memory range %pr: error %d\n", res, ret);
> -		return ERR_PTR(ret);
> -	}
> +		ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem));
> +		if (ret) {
> +			drm_err(dev, "could not acquire memory range %pr: %d\n", mem, ret);
> +			return ERR_PTR(ret);
> +		}
>   
> -	mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), drv->name);
> -	if (!mem) {
> -		/*
> -		 * We cannot make this fatal. Sometimes this comes from magic
> -		 * spaces our resource handlers simply don't know about. Use
> -		 * the I/O-memory resource as-is and try to map that instead.
> -		 */
> -		drm_warn(dev, "could not acquire memory region %pr\n", res);
> -		mem = res;
> -	}
> +		drm_info(dev, "using system memory framebuffer at %pr\n", mem);
>   
> -	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> -	if (!screen_base)
> -		return ERR_PTR(-ENOMEM);
> +		screen_base = devm_memremap(dev->dev, mem->start, resource_size(mem), MEMREMAP_WB);
> +		if (!screen_base)
> +			return ERR_PTR(-ENOMEM);
> +
> +		iosys_map_set_vaddr(&sdev->screen_base, screen_base);
> +	} else {
> +		void __iomem *screen_base;
> +
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +		if (!res)
> +			return ERR_PTR(-EINVAL);
>   
> -	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
> +		ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
> +		if (ret) {
> +			drm_err(dev, "could not acquire memory range %pr: %d\n", &res, ret);
> +			return ERR_PTR(ret);
> +		}
> +
> +		drm_info(dev, "using I/O memory framebuffer at %pr\n", res);
> +
> +		mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
> +					      drv->name);
> +		if (!mem) {
> +			/*
> +			 * We cannot make this fatal. Sometimes this comes from magic
> +			 * spaces our resource handlers simply don't know about. Use
> +			 * the I/O-memory resource as-is and try to map that instead.
> +			 */
> +			drm_warn(dev, "could not acquire memory region %pr\n", res);
> +			mem = res;
> +		}
> +
> +		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> +		if (!screen_base)
> +			return ERR_PTR(-ENOMEM);
> +
> +		iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
> +	}
>   
>   	/*
>   	 * Modesetting

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 6/8] drm/format-helper: Support the XB24 format

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3533 bytes --]



Am 17.11.22 um 19:40 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
> 
> Add a conversion helper for the XB24 format to use in drm_fb_blit().
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
> Changes in v3:
> - rebase onto latest drm-next
> 
> Changes in v2:
> - support XB24 format instead of AB24
> 
>   drivers/gpu/drm/drm_format_helper.c | 39 +++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 74ff33c2ddaa..c8764cc61e87 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -503,6 +503,36 @@ static void drm_fb_rgb888_to_xrgb8888(struct iosys_map *dst, const unsigned int
>   		    drm_fb_rgb888_to_xrgb8888_line);
>   }
>   
> +static void drm_fb_xrgb8888_to_xbgr8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
> +{
> +	__le32 *dbuf32 = dbuf;
> +	const __le32 *sbuf32 = sbuf;
> +	unsigned int x;
> +	u32 pix;
> +
> +	for (x = 0; x < pixels; x++) {
> +		pix = le32_to_cpu(sbuf32[x]);
> +		pix = ((pix & 0x00ff0000) >> 16) <<  0 |
> +		      ((pix & 0x0000ff00) >>  8) <<  8 |
> +		      ((pix & 0x000000ff) >>  0) << 16 |
> +		      0xff << 24;
> +		*dbuf32++ = cpu_to_le32(pix);
> +	}
> +}
> +
> +static void drm_fb_xrgb8888_to_xbgr8888(struct iosys_map *dst, const unsigned int *dst_pitch,
> +					const struct iosys_map *src,
> +					const struct drm_framebuffer *fb,
> +					const struct drm_rect *clip)
> +{
> +	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> +		4,
> +	};
> +
> +	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
> +		    drm_fb_xrgb8888_to_xbgr8888_line);
> +}
> +
>   static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, unsigned int pixels)
>   {
>   	__le32 *dbuf32 = dbuf;
> @@ -646,6 +676,8 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
>   		fb_format = DRM_FORMAT_XRGB8888;
>   	if (dst_format == DRM_FORMAT_ARGB8888)
>   		dst_format = DRM_FORMAT_XRGB8888;
> +	if (dst_format == DRM_FORMAT_ABGR8888)
> +		dst_format = DRM_FORMAT_XBGR8888;
>   	if (fb_format == DRM_FORMAT_ARGB2101010)
>   		fb_format = DRM_FORMAT_XRGB2101010;
>   	if (dst_format == DRM_FORMAT_ARGB2101010)
> @@ -678,6 +710,11 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
>   			drm_fb_rgb565_to_xrgb8888(dst, dst_pitch, src, fb, clip);
>   			return 0;
>   		}
> +	} else if (dst_format == DRM_FORMAT_XBGR8888) {
> +		if (fb_format == DRM_FORMAT_XRGB8888) {
> +			drm_fb_xrgb8888_to_xbgr8888(dst, dst_pitch, src, fb, clip);
> +			return 0;
> +		}
>   	} else if (dst_format == DRM_FORMAT_XRGB2101010) {
>   		if (fb_format == DRM_FORMAT_XRGB8888) {
>   			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip);
> @@ -820,6 +857,8 @@ static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, uint32_t
>   static const uint32_t conv_from_xrgb8888[] = {
>   	DRM_FORMAT_XRGB8888,
>   	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XBGR8888,
>   	DRM_FORMAT_XRGB2101010,
>   	DRM_FORMAT_ARGB2101010,
>   	DRM_FORMAT_RGB565,

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 7/8] drm/simpledrm: Support the XB24/AB24 format

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2571 bytes --]

Hi

Am 17.11.22 um 19:40 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
> 
> Add XB24 and AB24 to the list of supported formats. The format helpers
> support conversion to these formats and they are documented in the
> simple-framebuffer device tree bindings.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - treat AB24 as XB24 and support both at the same time
> 
>   drivers/gpu/drm/tiny/simpledrm.c       | 2 ++
>   include/linux/platform_data/simplefb.h | 1 +
>   2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 7f39bc58da52..ba1c2057fc65 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -483,6 +483,8 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
>   static const uint32_t simpledrm_primary_plane_formats[] = {
>   	DRM_FORMAT_XRGB8888,
>   	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ABGR8888,

Does the hardware *really* support AB42 on its primary plane?

We recently had a discussion about the exported formats and the 
consensus is that we only want the hardware's native formats plus 
XRGB888. That's not implemented yet in simpledrm, but this format list 
will soon see a larger cleanup.

So I think ARGB8888 likely shouldn't be on the list here.

Best regards
Thomas

>   	DRM_FORMAT_RGB565,
>   	//DRM_FORMAT_XRGB1555,
>   	//DRM_FORMAT_ARGB1555,
> diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
> index 27ea99af6e1d..4f94d52ac99f 100644
> --- a/include/linux/platform_data/simplefb.h
> +++ b/include/linux/platform_data/simplefb.h
> @@ -22,6 +22,7 @@
>   	{ "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, \
>   	{ "x8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_XRGB8888 }, \
>   	{ "a8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {24, 8}, DRM_FORMAT_ARGB8888 }, \
> +	{ "x8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {0, 0}, DRM_FORMAT_XBGR8888 }, \
>   	{ "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {24, 8}, DRM_FORMAT_ABGR8888 }, \
>   	{ "x2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {0, 0}, DRM_FORMAT_XRGB2101010 }, \
>   	{ "a2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {30, 2}, DRM_FORMAT_ARGB2101010 }, \

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 5/8] drm/simpledrm: Add support for system memory framebuffers

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 8004 bytes --]

On Fri, Nov 18, 2022 at 03:21:14PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 17.11.22 um 19:40 schrieb Thierry Reding:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Simple framebuffers can be set up in system memory, which cannot be
> > requested and/or I/O remapped using the I/O resource helpers. Add a
> > separate code path that obtains system memory framebuffers from the
> > reserved memory region referenced in the memory-region property.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Changes in v3:
> > - simplify memory code and move back to simpledrm_device_create()
> > - extract screen_base iosys_map fix into separate patch
> > 
> > Changes in v2:
> > - make screen base a struct iosys_map to avoid sparse warnings
> > 
> >   drivers/gpu/drm/tiny/simpledrm.c | 99 ++++++++++++++++++++++++--------
> >   1 file changed, 75 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> > index 3673a42e4bf4..7f39bc58da52 100644
> > --- a/drivers/gpu/drm/tiny/simpledrm.c
> > +++ b/drivers/gpu/drm/tiny/simpledrm.c
> > @@ -3,6 +3,7 @@
> >   #include <linux/clk.h>
> >   #include <linux/of_clk.h>
> >   #include <linux/minmax.h>
> > +#include <linux/of_address.h>
> >   #include <linux/platform_data/simplefb.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/regulator/consumer.h>
> > @@ -184,6 +185,31 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
> >   	return simplefb_get_validated_format(dev, format);
> >   }
> > +static struct resource *
> > +simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
> > +{
> > +	struct device_node *np;
> > +	struct resource *res;
> > +	int err;
> > +
> > +	np = of_parse_phandle(of_node, "memory-region", 0);
> > +	if (!np)
> > +		return NULL;
> > +
> > +	res = devm_kzalloc(dev->dev, sizeof(*res), GFP_KERNEL);
> > +	if (!res)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	err = of_address_to_resource(np, 0, res);
> > +	if (err)
> > +		return ERR_PTR(err);
> > +
> > +	if (of_get_property(of_node, "reg", NULL))
> > +		drm_warn(dev, "preferring \"memory-region\" over \"reg\" property\n");
> 
> The reg property is converted to a device resource when we create the device
> at [1].
> 
> I have another question, which I was discussing with Javier recently. Is it
> possible to handle memory-region there automatically? If, for exmaple, it
> would create a resource with IORESOURCE_CACHEABLE, simpledrm would handle it
> as memory region. Without the CACHEABLE flag, it would be a regular resource
> as before.

memory-region properties are not typically converted into a standard
resource automatically. One reason may be that they can have additional
properties associated with them and so something like a CACHEABLE type
may not apply.

It's also standard to convert "reg" properties into struct resource and
that's what many drivers will expect. I don't know if all drivers will
gracefully handle being passed a struct resource that was created in
this way from a memory-region property. If at all I think this would
need to be special-cased for simple-framebuffer, in which case I'm not
convinced that putting the special case into the core OF code is any
better than putting it into the simpledrm driver.

Also, even if we did so, what would it really change? We may be able to
avoid the explicit DT lookup, but the bulk of the memory-region code is
actually mapping it, etc. That part we won't be able to automatically
handle, I think.

Ultimately this is up to Rob, not sure if he'll want to extend the
simple-framebuffer node creation code any further.

Thierry

> 
> Best regards
> Thomas
> 
> [1]
> https://elixir.bootlin.com/linux/v6.0.9/source/drivers/of/platform.c#L586
> 
> > +
> > +	return res;
> > +}
> > +
> >   /*
> >    * Simple Framebuffer device
> >    */
> > @@ -623,8 +649,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
> >   	struct drm_device *dev;
> >   	int width, height, stride;
> >   	const struct drm_format_info *format;
> > -	struct resource *res, *mem;
> > -	void __iomem *screen_base;
> > +	struct resource *res, *mem = NULL;
> >   	struct drm_plane *primary_plane;
> >   	struct drm_crtc *crtc;
> >   	struct drm_encoder *encoder;
> > @@ -676,6 +701,9 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
> >   		format = simplefb_get_format_of(dev, of_node);
> >   		if (IS_ERR(format))
> >   			return ERR_CAST(format);
> > +		mem = simplefb_get_memory_of(dev, of_node);
> > +		if (IS_ERR(mem))
> > +			return ERR_CAST(mem);
> >   	} else {
> >   		drm_err(dev, "no simplefb configuration found\n");
> >   		return ERR_PTR(-ENODEV);
> > @@ -698,32 +726,55 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
> >   	 * Memory management
> >   	 */
> > -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	if (!res)
> > -		return ERR_PTR(-EINVAL);
> > +	if (mem) {
> > +		void *screen_base;
> > -	ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
> > -	if (ret) {
> > -		drm_err(dev, "could not acquire memory range %pr: error %d\n", res, ret);
> > -		return ERR_PTR(ret);
> > -	}
> > +		ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem));
> > +		if (ret) {
> > +			drm_err(dev, "could not acquire memory range %pr: %d\n", mem, ret);
> > +			return ERR_PTR(ret);
> > +		}
> > -	mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), drv->name);
> > -	if (!mem) {
> > -		/*
> > -		 * We cannot make this fatal. Sometimes this comes from magic
> > -		 * spaces our resource handlers simply don't know about. Use
> > -		 * the I/O-memory resource as-is and try to map that instead.
> > -		 */
> > -		drm_warn(dev, "could not acquire memory region %pr\n", res);
> > -		mem = res;
> > -	}
> > +		drm_info(dev, "using system memory framebuffer at %pr\n", mem);
> > -	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> > -	if (!screen_base)
> > -		return ERR_PTR(-ENOMEM);
> > +		screen_base = devm_memremap(dev->dev, mem->start, resource_size(mem), MEMREMAP_WB);
> > +		if (!screen_base)
> > +			return ERR_PTR(-ENOMEM);
> > +
> > +		iosys_map_set_vaddr(&sdev->screen_base, screen_base);
> > +	} else {
> > +		void __iomem *screen_base;
> > +
> > +		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +		if (!res)
> > +			return ERR_PTR(-EINVAL);
> > -	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
> > +		ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
> > +		if (ret) {
> > +			drm_err(dev, "could not acquire memory range %pr: %d\n", &res, ret);
> > +			return ERR_PTR(ret);
> > +		}
> > +
> > +		drm_info(dev, "using I/O memory framebuffer at %pr\n", res);
> > +
> > +		mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
> > +					      drv->name);
> > +		if (!mem) {
> > +			/*
> > +			 * We cannot make this fatal. Sometimes this comes from magic
> > +			 * spaces our resource handlers simply don't know about. Use
> > +			 * the I/O-memory resource as-is and try to map that instead.
> > +			 */
> > +			drm_warn(dev, "could not acquire memory region %pr\n", res);
> > +			mem = res;
> > +		}
> > +
> > +		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> > +		if (!screen_base)
> > +			return ERR_PTR(-ENOMEM);
> > +
> > +		iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
> > +	}
> >   	/*
> >   	 * Modesetting
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 5/8] drm/simpledrm: Add support for system memory framebuffers

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 8004 bytes --]

On Fri, Nov 18, 2022 at 03:21:14PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 17.11.22 um 19:40 schrieb Thierry Reding:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Simple framebuffers can be set up in system memory, which cannot be
> > requested and/or I/O remapped using the I/O resource helpers. Add a
> > separate code path that obtains system memory framebuffers from the
> > reserved memory region referenced in the memory-region property.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Changes in v3:
> > - simplify memory code and move back to simpledrm_device_create()
> > - extract screen_base iosys_map fix into separate patch
> > 
> > Changes in v2:
> > - make screen base a struct iosys_map to avoid sparse warnings
> > 
> >   drivers/gpu/drm/tiny/simpledrm.c | 99 ++++++++++++++++++++++++--------
> >   1 file changed, 75 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> > index 3673a42e4bf4..7f39bc58da52 100644
> > --- a/drivers/gpu/drm/tiny/simpledrm.c
> > +++ b/drivers/gpu/drm/tiny/simpledrm.c
> > @@ -3,6 +3,7 @@
> >   #include <linux/clk.h>
> >   #include <linux/of_clk.h>
> >   #include <linux/minmax.h>
> > +#include <linux/of_address.h>
> >   #include <linux/platform_data/simplefb.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/regulator/consumer.h>
> > @@ -184,6 +185,31 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
> >   	return simplefb_get_validated_format(dev, format);
> >   }
> > +static struct resource *
> > +simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
> > +{
> > +	struct device_node *np;
> > +	struct resource *res;
> > +	int err;
> > +
> > +	np = of_parse_phandle(of_node, "memory-region", 0);
> > +	if (!np)
> > +		return NULL;
> > +
> > +	res = devm_kzalloc(dev->dev, sizeof(*res), GFP_KERNEL);
> > +	if (!res)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	err = of_address_to_resource(np, 0, res);
> > +	if (err)
> > +		return ERR_PTR(err);
> > +
> > +	if (of_get_property(of_node, "reg", NULL))
> > +		drm_warn(dev, "preferring \"memory-region\" over \"reg\" property\n");
> 
> The reg property is converted to a device resource when we create the device
> at [1].
> 
> I have another question, which I was discussing with Javier recently. Is it
> possible to handle memory-region there automatically? If, for exmaple, it
> would create a resource with IORESOURCE_CACHEABLE, simpledrm would handle it
> as memory region. Without the CACHEABLE flag, it would be a regular resource
> as before.

memory-region properties are not typically converted into a standard
resource automatically. One reason may be that they can have additional
properties associated with them and so something like a CACHEABLE type
may not apply.

It's also standard to convert "reg" properties into struct resource and
that's what many drivers will expect. I don't know if all drivers will
gracefully handle being passed a struct resource that was created in
this way from a memory-region property. If at all I think this would
need to be special-cased for simple-framebuffer, in which case I'm not
convinced that putting the special case into the core OF code is any
better than putting it into the simpledrm driver.

Also, even if we did so, what would it really change? We may be able to
avoid the explicit DT lookup, but the bulk of the memory-region code is
actually mapping it, etc. That part we won't be able to automatically
handle, I think.

Ultimately this is up to Rob, not sure if he'll want to extend the
simple-framebuffer node creation code any further.

Thierry

> 
> Best regards
> Thomas
> 
> [1]
> https://elixir.bootlin.com/linux/v6.0.9/source/drivers/of/platform.c#L586
> 
> > +
> > +	return res;
> > +}
> > +
> >   /*
> >    * Simple Framebuffer device
> >    */
> > @@ -623,8 +649,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
> >   	struct drm_device *dev;
> >   	int width, height, stride;
> >   	const struct drm_format_info *format;
> > -	struct resource *res, *mem;
> > -	void __iomem *screen_base;
> > +	struct resource *res, *mem = NULL;
> >   	struct drm_plane *primary_plane;
> >   	struct drm_crtc *crtc;
> >   	struct drm_encoder *encoder;
> > @@ -676,6 +701,9 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
> >   		format = simplefb_get_format_of(dev, of_node);
> >   		if (IS_ERR(format))
> >   			return ERR_CAST(format);
> > +		mem = simplefb_get_memory_of(dev, of_node);
> > +		if (IS_ERR(mem))
> > +			return ERR_CAST(mem);
> >   	} else {
> >   		drm_err(dev, "no simplefb configuration found\n");
> >   		return ERR_PTR(-ENODEV);
> > @@ -698,32 +726,55 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
> >   	 * Memory management
> >   	 */
> > -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	if (!res)
> > -		return ERR_PTR(-EINVAL);
> > +	if (mem) {
> > +		void *screen_base;
> > -	ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
> > -	if (ret) {
> > -		drm_err(dev, "could not acquire memory range %pr: error %d\n", res, ret);
> > -		return ERR_PTR(ret);
> > -	}
> > +		ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem));
> > +		if (ret) {
> > +			drm_err(dev, "could not acquire memory range %pr: %d\n", mem, ret);
> > +			return ERR_PTR(ret);
> > +		}
> > -	mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), drv->name);
> > -	if (!mem) {
> > -		/*
> > -		 * We cannot make this fatal. Sometimes this comes from magic
> > -		 * spaces our resource handlers simply don't know about. Use
> > -		 * the I/O-memory resource as-is and try to map that instead.
> > -		 */
> > -		drm_warn(dev, "could not acquire memory region %pr\n", res);
> > -		mem = res;
> > -	}
> > +		drm_info(dev, "using system memory framebuffer at %pr\n", mem);
> > -	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> > -	if (!screen_base)
> > -		return ERR_PTR(-ENOMEM);
> > +		screen_base = devm_memremap(dev->dev, mem->start, resource_size(mem), MEMREMAP_WB);
> > +		if (!screen_base)
> > +			return ERR_PTR(-ENOMEM);
> > +
> > +		iosys_map_set_vaddr(&sdev->screen_base, screen_base);
> > +	} else {
> > +		void __iomem *screen_base;
> > +
> > +		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +		if (!res)
> > +			return ERR_PTR(-EINVAL);
> > -	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
> > +		ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
> > +		if (ret) {
> > +			drm_err(dev, "could not acquire memory range %pr: %d\n", &res, ret);
> > +			return ERR_PTR(ret);
> > +		}
> > +
> > +		drm_info(dev, "using I/O memory framebuffer at %pr\n", res);
> > +
> > +		mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
> > +					      drv->name);
> > +		if (!mem) {
> > +			/*
> > +			 * We cannot make this fatal. Sometimes this comes from magic
> > +			 * spaces our resource handlers simply don't know about. Use
> > +			 * the I/O-memory resource as-is and try to map that instead.
> > +			 */
> > +			drm_warn(dev, "could not acquire memory region %pr\n", res);
> > +			mem = res;
> > +		}
> > +
> > +		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> > +		if (!screen_base)
> > +			return ERR_PTR(-ENOMEM);
> > +
> > +		iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
> > +	}
> >   	/*
> >   	 * Modesetting
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 7/8] drm/simpledrm: Support the XB24/AB24 format

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3021 bytes --]

On Fri, Nov 18, 2022 at 04:08:23PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 17.11.22 um 19:40 schrieb Thierry Reding:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Add XB24 and AB24 to the list of supported formats. The format helpers
> > support conversion to these formats and they are documented in the
> > simple-framebuffer device tree bindings.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Changes in v2:
> > - treat AB24 as XB24 and support both at the same time
> > 
> >   drivers/gpu/drm/tiny/simpledrm.c       | 2 ++
> >   include/linux/platform_data/simplefb.h | 1 +
> >   2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> > index 7f39bc58da52..ba1c2057fc65 100644
> > --- a/drivers/gpu/drm/tiny/simpledrm.c
> > +++ b/drivers/gpu/drm/tiny/simpledrm.c
> > @@ -483,6 +483,8 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
> >   static const uint32_t simpledrm_primary_plane_formats[] = {
> >   	DRM_FORMAT_XRGB8888,
> >   	DRM_FORMAT_ARGB8888,
> > +	DRM_FORMAT_XBGR8888,
> > +	DRM_FORMAT_ABGR8888,
> 
> Does the hardware *really* support AB42 on its primary plane?

Yes, Tegra display hardware supports this format on the primary plane.

> We recently had a discussion about the exported formats and the consensus is
> that we only want the hardware's native formats plus XRGB888. That's not
> implemented yet in simpledrm, but this format list will soon see a larger
> cleanup.
> 
> So I think ARGB8888 likely shouldn't be on the list here.

This is for consistency with the list below. If a device tree claims
that the framebuffer is ABGR8888 using the "a8b8g8r8" string, then
shouldn't we support it?

Thierry

> 
> Best regards
> Thomas
> 
> >   	DRM_FORMAT_RGB565,
> >   	//DRM_FORMAT_XRGB1555,
> >   	//DRM_FORMAT_ARGB1555,
> > diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
> > index 27ea99af6e1d..4f94d52ac99f 100644
> > --- a/include/linux/platform_data/simplefb.h
> > +++ b/include/linux/platform_data/simplefb.h
> > @@ -22,6 +22,7 @@
> >   	{ "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, \
> >   	{ "x8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_XRGB8888 }, \
> >   	{ "a8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {24, 8}, DRM_FORMAT_ARGB8888 }, \
> > +	{ "x8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {0, 0}, DRM_FORMAT_XBGR8888 }, \
> >   	{ "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {24, 8}, DRM_FORMAT_ABGR8888 }, \
> >   	{ "x2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {0, 0}, DRM_FORMAT_XRGB2101010 }, \
> >   	{ "a2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {30, 2}, DRM_FORMAT_ARGB2101010 }, \
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 7/8] drm/simpledrm: Support the XB24/AB24 format

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3021 bytes --]

On Fri, Nov 18, 2022 at 04:08:23PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 17.11.22 um 19:40 schrieb Thierry Reding:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Add XB24 and AB24 to the list of supported formats. The format helpers
> > support conversion to these formats and they are documented in the
> > simple-framebuffer device tree bindings.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Changes in v2:
> > - treat AB24 as XB24 and support both at the same time
> > 
> >   drivers/gpu/drm/tiny/simpledrm.c       | 2 ++
> >   include/linux/platform_data/simplefb.h | 1 +
> >   2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> > index 7f39bc58da52..ba1c2057fc65 100644
> > --- a/drivers/gpu/drm/tiny/simpledrm.c
> > +++ b/drivers/gpu/drm/tiny/simpledrm.c
> > @@ -483,6 +483,8 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
> >   static const uint32_t simpledrm_primary_plane_formats[] = {
> >   	DRM_FORMAT_XRGB8888,
> >   	DRM_FORMAT_ARGB8888,
> > +	DRM_FORMAT_XBGR8888,
> > +	DRM_FORMAT_ABGR8888,
> 
> Does the hardware *really* support AB42 on its primary plane?

Yes, Tegra display hardware supports this format on the primary plane.

> We recently had a discussion about the exported formats and the consensus is
> that we only want the hardware's native formats plus XRGB888. That's not
> implemented yet in simpledrm, but this format list will soon see a larger
> cleanup.
> 
> So I think ARGB8888 likely shouldn't be on the list here.

This is for consistency with the list below. If a device tree claims
that the framebuffer is ABGR8888 using the "a8b8g8r8" string, then
shouldn't we support it?

Thierry

> 
> Best regards
> Thomas
> 
> >   	DRM_FORMAT_RGB565,
> >   	//DRM_FORMAT_XRGB1555,
> >   	//DRM_FORMAT_ARGB1555,
> > diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
> > index 27ea99af6e1d..4f94d52ac99f 100644
> > --- a/include/linux/platform_data/simplefb.h
> > +++ b/include/linux/platform_data/simplefb.h
> > @@ -22,6 +22,7 @@
> >   	{ "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, \
> >   	{ "x8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_XRGB8888 }, \
> >   	{ "a8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {24, 8}, DRM_FORMAT_ARGB8888 }, \
> > +	{ "x8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {0, 0}, DRM_FORMAT_XBGR8888 }, \
> >   	{ "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {24, 8}, DRM_FORMAT_ABGR8888 }, \
> >   	{ "x2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {0, 0}, DRM_FORMAT_XRGB2101010 }, \
> >   	{ "a2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {30, 2}, DRM_FORMAT_ARGB2101010 }, \
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 5/8] drm/simpledrm: Add support for system memory framebuffers

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 8776 bytes --]

Hi

Am 18.11.22 um 16:38 schrieb Thierry Reding:
> On Fri, Nov 18, 2022 at 03:21:14PM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 17.11.22 um 19:40 schrieb Thierry Reding:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Simple framebuffers can be set up in system memory, which cannot be
>>> requested and/or I/O remapped using the I/O resource helpers. Add a
>>> separate code path that obtains system memory framebuffers from the
>>> reserved memory region referenced in the memory-region property.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>> Changes in v3:
>>> - simplify memory code and move back to simpledrm_device_create()
>>> - extract screen_base iosys_map fix into separate patch
>>>
>>> Changes in v2:
>>> - make screen base a struct iosys_map to avoid sparse warnings
>>>
>>>    drivers/gpu/drm/tiny/simpledrm.c | 99 ++++++++++++++++++++++++--------
>>>    1 file changed, 75 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
>>> index 3673a42e4bf4..7f39bc58da52 100644
>>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>>> @@ -3,6 +3,7 @@
>>>    #include <linux/clk.h>
>>>    #include <linux/of_clk.h>
>>>    #include <linux/minmax.h>
>>> +#include <linux/of_address.h>
>>>    #include <linux/platform_data/simplefb.h>
>>>    #include <linux/platform_device.h>
>>>    #include <linux/regulator/consumer.h>
>>> @@ -184,6 +185,31 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
>>>    	return simplefb_get_validated_format(dev, format);
>>>    }
>>> +static struct resource *
>>> +simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
>>> +{
>>> +	struct device_node *np;
>>> +	struct resource *res;
>>> +	int err;
>>> +
>>> +	np = of_parse_phandle(of_node, "memory-region", 0);
>>> +	if (!np)
>>> +		return NULL;
>>> +
>>> +	res = devm_kzalloc(dev->dev, sizeof(*res), GFP_KERNEL);
>>> +	if (!res)
>>> +		return ERR_PTR(-ENOMEM);
>>> +
>>> +	err = of_address_to_resource(np, 0, res);
>>> +	if (err)
>>> +		return ERR_PTR(err);
>>> +
>>> +	if (of_get_property(of_node, "reg", NULL))
>>> +		drm_warn(dev, "preferring \"memory-region\" over \"reg\" property\n");
>>
>> The reg property is converted to a device resource when we create the device
>> at [1].
>>
>> I have another question, which I was discussing with Javier recently. Is it
>> possible to handle memory-region there automatically? If, for exmaple, it
>> would create a resource with IORESOURCE_CACHEABLE, simpledrm would handle it
>> as memory region. Without the CACHEABLE flag, it would be a regular resource
>> as before.
> 
> memory-region properties are not typically converted into a standard
> resource automatically. One reason may be that they can have additional
> properties associated with them and so something like a CACHEABLE type
> may not apply.
> 
> It's also standard to convert "reg" properties into struct resource and
> that's what many drivers will expect. I don't know if all drivers will
> gracefully handle being passed a struct resource that was created in
> this way from a memory-region property. If at all I think this would
> need to be special-cased for simple-framebuffer, in which case I'm not
> convinced that putting the special case into the core OF code is any
> better than putting it into the simpledrm driver.
> 
> Also, even if we did so, what would it really change? We may be able to
> avoid the explicit DT lookup, but the bulk of the memory-region code is
> actually mapping it, etc. That part we won't be able to automatically
> handle, I think.
> 
> Ultimately this is up to Rob, not sure if he'll want to extend the
> simple-framebuffer node creation code any further.

Thanks for explaining. It was simply something we wondered about.

The simpledrm device driver hands over device ownership to the 
hardware's native driver during the boot process. To make this work in 
all cases, the OF code needs to be involved. So at some point, we'll 
need to move some of the memory-region code into the OF code. But how 
exactly this has to be done can be discussed later.

Best regards
Thomas

> 
> Thierry
> 
>>
>> Best regards
>> Thomas
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.0.9/source/drivers/of/platform.c#L586
>>
>>> +
>>> +	return res;
>>> +}
>>> +
>>>    /*
>>>     * Simple Framebuffer device
>>>     */
>>> @@ -623,8 +649,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>>    	struct drm_device *dev;
>>>    	int width, height, stride;
>>>    	const struct drm_format_info *format;
>>> -	struct resource *res, *mem;
>>> -	void __iomem *screen_base;
>>> +	struct resource *res, *mem = NULL;
>>>    	struct drm_plane *primary_plane;
>>>    	struct drm_crtc *crtc;
>>>    	struct drm_encoder *encoder;
>>> @@ -676,6 +701,9 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>>    		format = simplefb_get_format_of(dev, of_node);
>>>    		if (IS_ERR(format))
>>>    			return ERR_CAST(format);
>>> +		mem = simplefb_get_memory_of(dev, of_node);
>>> +		if (IS_ERR(mem))
>>> +			return ERR_CAST(mem);
>>>    	} else {
>>>    		drm_err(dev, "no simplefb configuration found\n");
>>>    		return ERR_PTR(-ENODEV);
>>> @@ -698,32 +726,55 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>>    	 * Memory management
>>>    	 */
>>> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> -	if (!res)
>>> -		return ERR_PTR(-EINVAL);
>>> +	if (mem) {
>>> +		void *screen_base;
>>> -	ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
>>> -	if (ret) {
>>> -		drm_err(dev, "could not acquire memory range %pr: error %d\n", res, ret);
>>> -		return ERR_PTR(ret);
>>> -	}
>>> +		ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem));
>>> +		if (ret) {
>>> +			drm_err(dev, "could not acquire memory range %pr: %d\n", mem, ret);
>>> +			return ERR_PTR(ret);
>>> +		}
>>> -	mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), drv->name);
>>> -	if (!mem) {
>>> -		/*
>>> -		 * We cannot make this fatal. Sometimes this comes from magic
>>> -		 * spaces our resource handlers simply don't know about. Use
>>> -		 * the I/O-memory resource as-is and try to map that instead.
>>> -		 */
>>> -		drm_warn(dev, "could not acquire memory region %pr\n", res);
>>> -		mem = res;
>>> -	}
>>> +		drm_info(dev, "using system memory framebuffer at %pr\n", mem);
>>> -	screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
>>> -	if (!screen_base)
>>> -		return ERR_PTR(-ENOMEM);
>>> +		screen_base = devm_memremap(dev->dev, mem->start, resource_size(mem), MEMREMAP_WB);
>>> +		if (!screen_base)
>>> +			return ERR_PTR(-ENOMEM);
>>> +
>>> +		iosys_map_set_vaddr(&sdev->screen_base, screen_base);
>>> +	} else {
>>> +		void __iomem *screen_base;
>>> +
>>> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +		if (!res)
>>> +			return ERR_PTR(-EINVAL);
>>> -	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
>>> +		ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
>>> +		if (ret) {
>>> +			drm_err(dev, "could not acquire memory range %pr: %d\n", &res, ret);
>>> +			return ERR_PTR(ret);
>>> +		}
>>> +
>>> +		drm_info(dev, "using I/O memory framebuffer at %pr\n", res);
>>> +
>>> +		mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
>>> +					      drv->name);
>>> +		if (!mem) {
>>> +			/*
>>> +			 * We cannot make this fatal. Sometimes this comes from magic
>>> +			 * spaces our resource handlers simply don't know about. Use
>>> +			 * the I/O-memory resource as-is and try to map that instead.
>>> +			 */
>>> +			drm_warn(dev, "could not acquire memory region %pr\n", res);
>>> +			mem = res;
>>> +		}
>>> +
>>> +		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
>>> +		if (!screen_base)
>>> +			return ERR_PTR(-ENOMEM);
>>> +
>>> +		iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
>>> +	}
>>>    	/*
>>>    	 * Modesetting
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
> 
> 
> 

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 7/8] drm/simpledrm: Support the XB24/AB24 format

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 4399 bytes --]

Hi

Am 18.11.22 um 16:44 schrieb Thierry Reding:
> On Fri, Nov 18, 2022 at 04:08:23PM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 17.11.22 um 19:40 schrieb Thierry Reding:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Add XB24 and AB24 to the list of supported formats. The format helpers
>>> support conversion to these formats and they are documented in the
>>> simple-framebuffer device tree bindings.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>> Changes in v2:
>>> - treat AB24 as XB24 and support both at the same time
>>>
>>>    drivers/gpu/drm/tiny/simpledrm.c       | 2 ++
>>>    include/linux/platform_data/simplefb.h | 1 +
>>>    2 files changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
>>> index 7f39bc58da52..ba1c2057fc65 100644
>>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>>> @@ -483,6 +483,8 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
>>>    static const uint32_t simpledrm_primary_plane_formats[] = {
>>>    	DRM_FORMAT_XRGB8888,
>>>    	DRM_FORMAT_ARGB8888,
>>> +	DRM_FORMAT_XBGR8888,
>>> +	DRM_FORMAT_ABGR8888,
>>
>> Does the hardware *really* support AB42 on its primary plane?
> 
> Yes, Tegra display hardware supports this format on the primary plane.
> 
>> We recently had a discussion about the exported formats and the consensus is
>> that we only want the hardware's native formats plus XRGB888. That's not
>> implemented yet in simpledrm, but this format list will soon see a larger
>> cleanup.
>>
>> So I think ARGB8888 likely shouldn't be on the list here.
> 
> This is for consistency with the list below. If a device tree claims
> that the framebuffer is ABGR8888 using the "a8b8g8r8" string, then
> shouldn't we support it?

The situation is complicated. Several DTs claim that their framebuffers 
support Alpha+RGB when they actually mean X+RGB. But for compatibility, 
we cannot change this now AFAIU. So we're stuck with X+RGB framebuffers 
that claim that they have an alpha channel. OTOH, other hardware might 
actually support the announced alpha channel. Trying to render into an 
alpha channel would therefore produce undefined output.

The consensus is that we only want to announce XRGB8888 plus the native 
format to userspace. But if the native format has an alpha channel, we'd 
announce the non-alpha format instead. Our format-conversion helpers 
would then fill the alpha channel automatically with 0xff during the 
pageflip.

(This hasn't yet been fully implemented because we first need to fix a 
few things in fbdev emulation to make it work.)

Therefore ABGR8888 shouldn't be on the list. Note that a native DRM 
driver for your display hardware would be free to export ABGR8888. We 
only have this rule for the hardware-agnostic drivers.

Best regards
Thomas

> 
> Thierry
> 
>>
>> Best regards
>> Thomas
>>
>>>    	DRM_FORMAT_RGB565,
>>>    	//DRM_FORMAT_XRGB1555,
>>>    	//DRM_FORMAT_ARGB1555,
>>> diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
>>> index 27ea99af6e1d..4f94d52ac99f 100644
>>> --- a/include/linux/platform_data/simplefb.h
>>> +++ b/include/linux/platform_data/simplefb.h
>>> @@ -22,6 +22,7 @@
>>>    	{ "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, \
>>>    	{ "x8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_XRGB8888 }, \
>>>    	{ "a8r8g8b8", 32, {16, 8}, {8, 8}, {0, 8}, {24, 8}, DRM_FORMAT_ARGB8888 }, \
>>> +	{ "x8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {0, 0}, DRM_FORMAT_XBGR8888 }, \
>>>    	{ "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {24, 8}, DRM_FORMAT_ABGR8888 }, \
>>>    	{ "x2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {0, 0}, DRM_FORMAT_XRGB2101010 }, \
>>>    	{ "a2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {30, 2}, DRM_FORMAT_ARGB2101010 }, \
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
> 
> 
> 

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 5/8] drm/simpledrm: Add support for system memory framebuffers

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 6089 bytes --]

On Fri, Nov 18, 2022 at 04:54:31PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 18.11.22 um 16:38 schrieb Thierry Reding:
> > On Fri, Nov 18, 2022 at 03:21:14PM +0100, Thomas Zimmermann wrote:
> > > Hi
> > > 
> > > Am 17.11.22 um 19:40 schrieb Thierry Reding:
> > > > From: Thierry Reding <treding@nvidia.com>
> > > > 
> > > > Simple framebuffers can be set up in system memory, which cannot be
> > > > requested and/or I/O remapped using the I/O resource helpers. Add a
> > > > separate code path that obtains system memory framebuffers from the
> > > > reserved memory region referenced in the memory-region property.
> > > > 
> > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > ---
> > > > Changes in v3:
> > > > - simplify memory code and move back to simpledrm_device_create()
> > > > - extract screen_base iosys_map fix into separate patch
> > > > 
> > > > Changes in v2:
> > > > - make screen base a struct iosys_map to avoid sparse warnings
> > > > 
> > > >    drivers/gpu/drm/tiny/simpledrm.c | 99 ++++++++++++++++++++++++--------
> > > >    1 file changed, 75 insertions(+), 24 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> > > > index 3673a42e4bf4..7f39bc58da52 100644
> > > > --- a/drivers/gpu/drm/tiny/simpledrm.c
> > > > +++ b/drivers/gpu/drm/tiny/simpledrm.c
> > > > @@ -3,6 +3,7 @@
> > > >    #include <linux/clk.h>
> > > >    #include <linux/of_clk.h>
> > > >    #include <linux/minmax.h>
> > > > +#include <linux/of_address.h>
> > > >    #include <linux/platform_data/simplefb.h>
> > > >    #include <linux/platform_device.h>
> > > >    #include <linux/regulator/consumer.h>
> > > > @@ -184,6 +185,31 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
> > > >    	return simplefb_get_validated_format(dev, format);
> > > >    }
> > > > +static struct resource *
> > > > +simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
> > > > +{
> > > > +	struct device_node *np;
> > > > +	struct resource *res;
> > > > +	int err;
> > > > +
> > > > +	np = of_parse_phandle(of_node, "memory-region", 0);
> > > > +	if (!np)
> > > > +		return NULL;
> > > > +
> > > > +	res = devm_kzalloc(dev->dev, sizeof(*res), GFP_KERNEL);
> > > > +	if (!res)
> > > > +		return ERR_PTR(-ENOMEM);
> > > > +
> > > > +	err = of_address_to_resource(np, 0, res);
> > > > +	if (err)
> > > > +		return ERR_PTR(err);
> > > > +
> > > > +	if (of_get_property(of_node, "reg", NULL))
> > > > +		drm_warn(dev, "preferring \"memory-region\" over \"reg\" property\n");
> > > 
> > > The reg property is converted to a device resource when we create the device
> > > at [1].
> > > 
> > > I have another question, which I was discussing with Javier recently. Is it
> > > possible to handle memory-region there automatically? If, for exmaple, it
> > > would create a resource with IORESOURCE_CACHEABLE, simpledrm would handle it
> > > as memory region. Without the CACHEABLE flag, it would be a regular resource
> > > as before.
> > 
> > memory-region properties are not typically converted into a standard
> > resource automatically. One reason may be that they can have additional
> > properties associated with them and so something like a CACHEABLE type
> > may not apply.
> > 
> > It's also standard to convert "reg" properties into struct resource and
> > that's what many drivers will expect. I don't know if all drivers will
> > gracefully handle being passed a struct resource that was created in
> > this way from a memory-region property. If at all I think this would
> > need to be special-cased for simple-framebuffer, in which case I'm not
> > convinced that putting the special case into the core OF code is any
> > better than putting it into the simpledrm driver.
> > 
> > Also, even if we did so, what would it really change? We may be able to
> > avoid the explicit DT lookup, but the bulk of the memory-region code is
> > actually mapping it, etc. That part we won't be able to automatically
> > handle, I think.
> > 
> > Ultimately this is up to Rob, not sure if he'll want to extend the
> > simple-framebuffer node creation code any further.
> 
> Thanks for explaining. It was simply something we wondered about.
> 
> The simpledrm device driver hands over device ownership to the hardware's
> native driver during the boot process. To make this work in all cases, the
> OF code needs to be involved. So at some point, we'll need to move some of
> the memory-region code into the OF code. But how exactly this has to be done
> can be discussed later.

Currently the simpledrm driver will be removed when the native driver is
loaded (on Tegra at least) and then the Tegra DRM driver will set itself
up from scratch and anything that simpledrm had set up will be discarded
after that.

The tentative plan for Tegra is to eventually take over the memory from
simpledrm (basically via the same memory-region mechanism) and copy it
into a native buffer and also adopt the display configuration so that
the transition can happen seamlessly. I'm not sure to what degree the OF
core needs to get involved. Once we have the reserved-memory region, we
don't really need OF anymore.

One thing I'm not sure about yet is what to do with the reserved-memory
region. Ideally I think we would want to return the memory to the buddy
allocator so that it can be reused. I'm not sure if that's possible or
how to do it, since most of the low-level memblock code that is
responsible for cleaning things up has already run at this point. We'd
probably need to manually return the buffer somehow (perhaps with
something like generic_online_page()). Alternatively we may also hang
onto the memory and reuse it in the native driver, though that could be
a bit messy since it can't be transparently handled like any other
buffer.

I could find a few drivers that use memory-region, but none of them seem
to return that region to the buddy allocator.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 5/8] drm/simpledrm: Add support for system memory framebuffers

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 6089 bytes --]

On Fri, Nov 18, 2022 at 04:54:31PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 18.11.22 um 16:38 schrieb Thierry Reding:
> > On Fri, Nov 18, 2022 at 03:21:14PM +0100, Thomas Zimmermann wrote:
> > > Hi
> > > 
> > > Am 17.11.22 um 19:40 schrieb Thierry Reding:
> > > > From: Thierry Reding <treding@nvidia.com>
> > > > 
> > > > Simple framebuffers can be set up in system memory, which cannot be
> > > > requested and/or I/O remapped using the I/O resource helpers. Add a
> > > > separate code path that obtains system memory framebuffers from the
> > > > reserved memory region referenced in the memory-region property.
> > > > 
> > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > ---
> > > > Changes in v3:
> > > > - simplify memory code and move back to simpledrm_device_create()
> > > > - extract screen_base iosys_map fix into separate patch
> > > > 
> > > > Changes in v2:
> > > > - make screen base a struct iosys_map to avoid sparse warnings
> > > > 
> > > >    drivers/gpu/drm/tiny/simpledrm.c | 99 ++++++++++++++++++++++++--------
> > > >    1 file changed, 75 insertions(+), 24 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> > > > index 3673a42e4bf4..7f39bc58da52 100644
> > > > --- a/drivers/gpu/drm/tiny/simpledrm.c
> > > > +++ b/drivers/gpu/drm/tiny/simpledrm.c
> > > > @@ -3,6 +3,7 @@
> > > >    #include <linux/clk.h>
> > > >    #include <linux/of_clk.h>
> > > >    #include <linux/minmax.h>
> > > > +#include <linux/of_address.h>
> > > >    #include <linux/platform_data/simplefb.h>
> > > >    #include <linux/platform_device.h>
> > > >    #include <linux/regulator/consumer.h>
> > > > @@ -184,6 +185,31 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
> > > >    	return simplefb_get_validated_format(dev, format);
> > > >    }
> > > > +static struct resource *
> > > > +simplefb_get_memory_of(struct drm_device *dev, struct device_node *of_node)
> > > > +{
> > > > +	struct device_node *np;
> > > > +	struct resource *res;
> > > > +	int err;
> > > > +
> > > > +	np = of_parse_phandle(of_node, "memory-region", 0);
> > > > +	if (!np)
> > > > +		return NULL;
> > > > +
> > > > +	res = devm_kzalloc(dev->dev, sizeof(*res), GFP_KERNEL);
> > > > +	if (!res)
> > > > +		return ERR_PTR(-ENOMEM);
> > > > +
> > > > +	err = of_address_to_resource(np, 0, res);
> > > > +	if (err)
> > > > +		return ERR_PTR(err);
> > > > +
> > > > +	if (of_get_property(of_node, "reg", NULL))
> > > > +		drm_warn(dev, "preferring \"memory-region\" over \"reg\" property\n");
> > > 
> > > The reg property is converted to a device resource when we create the device
> > > at [1].
> > > 
> > > I have another question, which I was discussing with Javier recently. Is it
> > > possible to handle memory-region there automatically? If, for exmaple, it
> > > would create a resource with IORESOURCE_CACHEABLE, simpledrm would handle it
> > > as memory region. Without the CACHEABLE flag, it would be a regular resource
> > > as before.
> > 
> > memory-region properties are not typically converted into a standard
> > resource automatically. One reason may be that they can have additional
> > properties associated with them and so something like a CACHEABLE type
> > may not apply.
> > 
> > It's also standard to convert "reg" properties into struct resource and
> > that's what many drivers will expect. I don't know if all drivers will
> > gracefully handle being passed a struct resource that was created in
> > this way from a memory-region property. If at all I think this would
> > need to be special-cased for simple-framebuffer, in which case I'm not
> > convinced that putting the special case into the core OF code is any
> > better than putting it into the simpledrm driver.
> > 
> > Also, even if we did so, what would it really change? We may be able to
> > avoid the explicit DT lookup, but the bulk of the memory-region code is
> > actually mapping it, etc. That part we won't be able to automatically
> > handle, I think.
> > 
> > Ultimately this is up to Rob, not sure if he'll want to extend the
> > simple-framebuffer node creation code any further.
> 
> Thanks for explaining. It was simply something we wondered about.
> 
> The simpledrm device driver hands over device ownership to the hardware's
> native driver during the boot process. To make this work in all cases, the
> OF code needs to be involved. So at some point, we'll need to move some of
> the memory-region code into the OF code. But how exactly this has to be done
> can be discussed later.

Currently the simpledrm driver will be removed when the native driver is
loaded (on Tegra at least) and then the Tegra DRM driver will set itself
up from scratch and anything that simpledrm had set up will be discarded
after that.

The tentative plan for Tegra is to eventually take over the memory from
simpledrm (basically via the same memory-region mechanism) and copy it
into a native buffer and also adopt the display configuration so that
the transition can happen seamlessly. I'm not sure to what degree the OF
core needs to get involved. Once we have the reserved-memory region, we
don't really need OF anymore.

One thing I'm not sure about yet is what to do with the reserved-memory
region. Ideally I think we would want to return the memory to the buddy
allocator so that it can be reused. I'm not sure if that's possible or
how to do it, since most of the low-level memblock code that is
responsible for cleaning things up has already run at this point. We'd
probably need to manually return the buffer somehow (perhaps with
something like generic_online_page()). Alternatively we may also hang
onto the memory and reuse it in the native driver, though that could be
a bit messy since it can't be transparently handled like any other
buffer.

I could find a few drivers that use memory-region, but none of them seem
to return that region to the buddy allocator.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 7/8] drm/simpledrm: Support the XB24/AB24 format

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3734 bytes --]

On Fri, Nov 18, 2022 at 05:10:38PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 18.11.22 um 16:44 schrieb Thierry Reding:
> > On Fri, Nov 18, 2022 at 04:08:23PM +0100, Thomas Zimmermann wrote:
> > > Hi
> > > 
> > > Am 17.11.22 um 19:40 schrieb Thierry Reding:
> > > > From: Thierry Reding <treding@nvidia.com>
> > > > 
> > > > Add XB24 and AB24 to the list of supported formats. The format helpers
> > > > support conversion to these formats and they are documented in the
> > > > simple-framebuffer device tree bindings.
> > > > 
> > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > ---
> > > > Changes in v2:
> > > > - treat AB24 as XB24 and support both at the same time
> > > > 
> > > >    drivers/gpu/drm/tiny/simpledrm.c       | 2 ++
> > > >    include/linux/platform_data/simplefb.h | 1 +
> > > >    2 files changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> > > > index 7f39bc58da52..ba1c2057fc65 100644
> > > > --- a/drivers/gpu/drm/tiny/simpledrm.c
> > > > +++ b/drivers/gpu/drm/tiny/simpledrm.c
> > > > @@ -483,6 +483,8 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
> > > >    static const uint32_t simpledrm_primary_plane_formats[] = {
> > > >    	DRM_FORMAT_XRGB8888,
> > > >    	DRM_FORMAT_ARGB8888,
> > > > +	DRM_FORMAT_XBGR8888,
> > > > +	DRM_FORMAT_ABGR8888,
> > > 
> > > Does the hardware *really* support AB42 on its primary plane?
> > 
> > Yes, Tegra display hardware supports this format on the primary plane.
> > 
> > > We recently had a discussion about the exported formats and the consensus is
> > > that we only want the hardware's native formats plus XRGB888. That's not
> > > implemented yet in simpledrm, but this format list will soon see a larger
> > > cleanup.
> > > 
> > > So I think ARGB8888 likely shouldn't be on the list here.
> > 
> > This is for consistency with the list below. If a device tree claims
> > that the framebuffer is ABGR8888 using the "a8b8g8r8" string, then
> > shouldn't we support it?
> 
> The situation is complicated. Several DTs claim that their framebuffers
> support Alpha+RGB when they actually mean X+RGB. But for compatibility, we
> cannot change this now AFAIU. So we're stuck with X+RGB framebuffers that
> claim that they have an alpha channel. OTOH, other hardware might actually
> support the announced alpha channel. Trying to render into an alpha channel
> would therefore produce undefined output.

As long as we output 0xff into the alpha channel we should be able to
support those modes as well, shouldn't we? This would effectively be the
same as XRGB variants, except that the native mode could still be
reflected. It probably doesn't make sense to have an alpha channel for
these use-cases (what would we blend with), but I don't see how it would
hurt.

> The consensus is that we only want to announce XRGB8888 plus the native
> format to userspace. But if the native format has an alpha channel, we'd
> announce the non-alpha format instead. Our format-conversion helpers would
> then fill the alpha channel automatically with 0xff during the pageflip.
> 
> (This hasn't yet been fully implemented because we first need to fix a few
> things in fbdev emulation to make it work.)
> 
> Therefore ABGR8888 shouldn't be on the list. Note that a native DRM driver
> for your display hardware would be free to export ABGR8888. We only have
> this rule for the hardware-agnostic drivers.

I don't feel strongly about it, so I can also drop that format. Do we
also want to drop ARGB8888 and ARGB2101010 while we're at it? In a
separate patch of course.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 8/8] arm64: tegra: Add simple framebuffer on Jetson Xavier NX

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3132 bytes --]

Hi

Am 17.11.22 um 19:40 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
> 
> Add the framebuffer carveout reserved memory node as well as a simple-
> framebuffer node that is used to bind to the framebuffer that the
> bootloader has set up.

I don't know about the current status of this patchset, but feel free to 
send whatever update you have.

Best regards
Thomas

> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - clear out dynamic fields and leave it up to firmware to fill them in
> - mark simple-framebuffer node as disabled by default
> 
>   .../nvidia/tegra194-p3509-0000+p3668-0001.dts | 43 +++++++++++++++++++
>   arch/arm64/boot/dts/nvidia/tegra194.dtsi      |  2 +-
>   2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts b/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
> index 238fd98e8e45..85b4aaa2ad4e 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
> @@ -7,4 +7,47 @@
>   / {
>   	model = "NVIDIA Jetson Xavier NX Developer Kit (eMMC)";
>   	compatible = "nvidia,p3509-0000+p3668-0001", "nvidia,tegra194";
> +
> +	chosen {
> +		framebuffer {
> +			compatible = "simple-framebuffer";
> +			status = "disabled";
> +			memory-region = <&fb>;
> +			power-domains = <&bpmp TEGRA194_POWER_DOMAIN_DISP>;
> +			clocks = <&bpmp TEGRA194_CLK_SOR1_REF>,
> +				 <&bpmp TEGRA194_CLK_SOR1_OUT>,
> +				 <&bpmp TEGRA194_CLK_SOR1_PAD_CLKOUT>,
> +				 <&bpmp TEGRA194_CLK_PLLD2>,
> +				 <&bpmp TEGRA194_CLK_PLLDP>,
> +				 <&bpmp TEGRA194_CLK_NVDISPLAY_DISP>,
> +				 <&bpmp TEGRA194_CLK_NVDISPLAYHUB>,
> +				 <&bpmp TEGRA194_CLK_NVDISPLAY_P0>;
> +			width = <0>;
> +			height = <0>;
> +			stride = <0>;
> +			format = "x8b8g8r8";
> +		};
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		fb: framebuffer@0,0 {
> +			compatible = "framebuffer";
> +			reg = <0x0 0x0 0x0 0x0>;
> +			iommu-addresses = <&dc0 0x0 0x0 0x0 0x0>;
> +		};
> +	};
> +
> +	bus@0 {
> +		host1x@13e00000 {
> +			display-hub@15200000 {
> +				display@15200000 {
> +					memory-region = <&fb>;
> +				};
> +			};
> +		};
> +	};
>   };
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index d0dbfafbc930..ec318b9e700c 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -1972,7 +1972,7 @@ display-hub@15200000 {
>   
>   				ranges = <0x15200000 0x15200000 0x40000>;
>   
> -				display@15200000 {
> +				dc0: display@15200000 {
>   					compatible = "nvidia,tegra194-dc";
>   					reg = <0x15200000 0x10000>;
>   					interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 8/8] arm64: tegra: Add simple framebuffer on Jetson Xavier NX

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3132 bytes --]

Hi

Am 17.11.22 um 19:40 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
> 
> Add the framebuffer carveout reserved memory node as well as a simple-
> framebuffer node that is used to bind to the framebuffer that the
> bootloader has set up.

I don't know about the current status of this patchset, but feel free to 
send whatever update you have.

Best regards
Thomas

> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - clear out dynamic fields and leave it up to firmware to fill them in
> - mark simple-framebuffer node as disabled by default
> 
>   .../nvidia/tegra194-p3509-0000+p3668-0001.dts | 43 +++++++++++++++++++
>   arch/arm64/boot/dts/nvidia/tegra194.dtsi      |  2 +-
>   2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts b/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
> index 238fd98e8e45..85b4aaa2ad4e 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dts
> @@ -7,4 +7,47 @@
>   / {
>   	model = "NVIDIA Jetson Xavier NX Developer Kit (eMMC)";
>   	compatible = "nvidia,p3509-0000+p3668-0001", "nvidia,tegra194";
> +
> +	chosen {
> +		framebuffer {
> +			compatible = "simple-framebuffer";
> +			status = "disabled";
> +			memory-region = <&fb>;
> +			power-domains = <&bpmp TEGRA194_POWER_DOMAIN_DISP>;
> +			clocks = <&bpmp TEGRA194_CLK_SOR1_REF>,
> +				 <&bpmp TEGRA194_CLK_SOR1_OUT>,
> +				 <&bpmp TEGRA194_CLK_SOR1_PAD_CLKOUT>,
> +				 <&bpmp TEGRA194_CLK_PLLD2>,
> +				 <&bpmp TEGRA194_CLK_PLLDP>,
> +				 <&bpmp TEGRA194_CLK_NVDISPLAY_DISP>,
> +				 <&bpmp TEGRA194_CLK_NVDISPLAYHUB>,
> +				 <&bpmp TEGRA194_CLK_NVDISPLAY_P0>;
> +			width = <0>;
> +			height = <0>;
> +			stride = <0>;
> +			format = "x8b8g8r8";
> +		};
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		fb: framebuffer@0,0 {
> +			compatible = "framebuffer";
> +			reg = <0x0 0x0 0x0 0x0>;
> +			iommu-addresses = <&dc0 0x0 0x0 0x0 0x0>;
> +		};
> +	};
> +
> +	bus@0 {
> +		host1x@13e00000 {
> +			display-hub@15200000 {
> +				display@15200000 {
> +					memory-region = <&fb>;
> +				};
> +			};
> +		};
> +	};
>   };
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index d0dbfafbc930..ec318b9e700c 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -1972,7 +1972,7 @@ display-hub@15200000 {
>   
>   				ranges = <0x15200000 0x15200000 0x40000>;
>   
> -				display@15200000 {
> +				dc0: display@15200000 {
>   					compatible = "nvidia,tegra194-dc";
>   					reg = <0x15200000 0x10000>;
>   					interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 8/8] arm64: tegra: Add simple framebuffer on Jetson Xavier NX

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Tue, Jan 17, 2023 at 03:55:21PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 17.11.22 um 19:40 schrieb Thierry Reding:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Add the framebuffer carveout reserved memory node as well as a simple-
> > framebuffer node that is used to bind to the framebuffer that the
> > bootloader has set up.
> 
> I don't know about the current status of this patchset, but feel free to
> send whatever update you have.

Sorry, got side-tracked a few times during the last few weeks. I've had
to rework some parts of this for the recent changes to the format
helpers, but nothing major. I'll send out the updated version shortly
once I've tested all cases.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 8/8] arm64: tegra: Add simple framebuffer on Jetson Xavier NX

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Tue, Jan 17, 2023 at 03:55:21PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 17.11.22 um 19:40 schrieb Thierry Reding:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Add the framebuffer carveout reserved memory node as well as a simple-
> > framebuffer node that is used to bind to the framebuffer that the
> > bootloader has set up.
> 
> I don't know about the current status of this patchset, but feel free to
> send whatever update you have.

Sorry, got side-tracked a few times during the last few weeks. I've had
to rework some parts of this for the recent changes to the format
helpers, but nothing major. I'll send out the updated version shortly
once I've tested all cases.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]