[PATCH] drm/amdgpu/display: Remove unnecessary conversion to bool

[Jiapeng Chong]

Fix the following coccicheck warnings:

./drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:956:52-57: WARNING:
conversion to bool not needed here.

./drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:8311:16-21: WARNING:
conversion to bool not needed here.

Reported-by: Abaci Robot 
Signed-off-by: Jiapeng Chong 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 3e1fd1e..0e6b7415 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -953,7 +953,7 @@ static void event_mall_stutter(struct work_struct *work)
 
 
dc_allow_idle_optimizations(
-   dm->dc, dm->active_vblank_irq_count == 0 ? true : false);
+   dm->dc, dm->active_vblank_irq_count == 0);
 
DRM_DEBUG_DRIVER("Allow idle optimizations (MALL): %d\n", 
dm->active_vblank_irq_count == 0);
 
@@ -8307,8 +8307,7 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
hdcp_update_display(
adev->dm.hdcp_workqueue, 
aconnector->dc_link->link_index, aconnector,
new_con_state->hdcp_content_type,
-   new_con_state->content_protection == 
DRM_MODE_CONTENT_PROTECTION_DESIRED ? true
-   
 : false);
+   new_con_state->content_protection == 
DRM_MODE_CONTENT_PROTECTION_DESIRED);
}
 #endif
 
-- 
1.8.3.1

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

Re: [PATCH] drm/stm: ltdc: Use simple encoder

[Thomas Zimmermann]

Hi,

shall I merge this patch?

Am 02.03.21 um 18:57 schrieb Jagan Teki:

STM ltdc driver uses an empty implementation for its encoder.
Replace the code with the generic simple encoder.

Signed-off-by: Jagan Teki 
---
  drivers/gpu/drm/stm/ltdc.c | 12 ++--
  1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 7812094f93d6..aeeb43524ca0 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -31,6 +31,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  
  #include 

@@ -1020,14 +1021,6 @@ static int ltdc_crtc_init(struct drm_device *ddev, 
struct drm_crtc *crtc)
return ret;
  }
  
-/*

- * DRM_ENCODER
- */
-
-static const struct drm_encoder_funcs ltdc_encoder_funcs = {
-   .destroy = drm_encoder_cleanup,
-};
-
  static void ltdc_encoder_disable(struct drm_encoder *encoder)
  {
struct drm_device *ddev = encoder->dev;
@@ -1088,8 +1081,7 @@ static int ltdc_encoder_init(struct drm_device *ddev, 
struct drm_bridge *bridge)
encoder->possible_crtcs = CRTC_MASK;
encoder->possible_clones = 0;/* No cloning support */
  
-	drm_encoder_init(ddev, encoder, 

Re: [PATCH] drm/mcde/panel: Inverse misunderstood flag

[Linus Walleij]

On Thu, Mar 4, 2021 at 2:57 AM Nicolas Boichat  wrote:
> On Thu, Mar 4, 2021 at 8:41 AM Linus Walleij  wrote:
> >
> > A recent patch renaming MIPI_DSI_MODE_EOT_PACKET to
> > MIPI_DSI_MODE_NO_EOT_PACKET brought to light the
> > misunderstanding in the current MCDE driver and all
> > its associated panel drivers that MIPI_DSI_MODE_EOT_PACKET
> > would mean "use EOT packet" when in fact it means the
> > reverse.
> >
> > Fix it up by implementing the flag right in the MCDE
> > DSI driver and remove the flag from panels that actually
> > want the EOT packet.
> >
> > Suggested-by: Nicolas Boichat 
> > Signed-off-by: Linus Walleij 
>
> Reviewed-by: Nicolas Boichat 
>
> I wonder if it's worth adding the fixes, should be:
>
> Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
> Fixes: 899f24ed8d3a ("drm/panel: Add driver for Novatek NT35510-based panels")
> Fixes: ac1d6d74884e ("drm/panel: Add driver for Samsung S6D16D0 panel")
> Fixes: 435e06c06cb2 ("drm/panel: s6e63m0: Add DSI transport")
> Fixes: 8152c2bfd780 ("drm/panel: Add driver for Sony ACX424AKP panel")
>
> But then you'd almost need to separate the patches in multiple bits
> (these patches landed in very different releases).
>
> I'll leave that up to the maintainers to decide: this would only
> matter if anybody tried to use these panels on LTS releases with a
> non-MCDE driver (or MCDE with other panels).

It's not even a regression until someone else starts to use these panel
drivers (currently noone does), so I'd say let's just apply this and then
rebase and apply your cleanup on top.

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

Re: [PATCH v13 1/2] drm/tegra: dc: Support memory bandwidth management

[Michał Mirosław]

On Tue, Mar 02, 2021 at 03:44:44PM +0300, Dmitry Osipenko wrote:
> Display controller (DC) performs isochronous memory transfers, and thus,
> has a requirement for a minimum memory bandwidth that shall be fulfilled,
> otherwise framebuffer data can't be fetched fast enough and this results
> in a DC's data-FIFO underflow that follows by a visual corruption.
> 
> The Memory Controller drivers provide facility for memory bandwidth
> management via interconnect API. Let's wire up the interconnect API
> support to the DC driver in order to fix the distorted display output
> on T30 Ouya, T124 TK1 and other Tegra devices.

I did a read on the code. I have put some thoughts and nits inbetween the
diff, but here are more general questions about the patch:

Is there a reason why the bandwidth is allocated per plane instead of just
using one peak and average for the whole configuration? Or is there a need
to expose all the planes as interconnect channels and allocate their
bandwidth individually?

>From algorithmic part I see that the plane overlaps are calculated twice
(A vs B and later B vs A). The cursor plane is ignored, but nevertheless
its overlap mask is calculated before being thrown away. The bandwidths
are also calculated twice: once for pre-commit and then again for
post-commit.  Is setting bandwidth for an interconnect expensive when
re-setting a value that was already set? The code seems to avoid this
case, but maybe unnecessarily?

[...cut the big and interesting part...]

[...]
> @@ -65,7 +75,9 @@ struct tegra_dc_soc_info {
>   unsigned int num_overlay_formats;
>   const u64 *modifiers;
>   bool has_win_a_without_filters;
> + bool has_win_b_vfilter_mem_client;
>   bool has_win_c_without_vert_filter;
> + unsigned int plane_tiled_memory_bandwidth_x2;

This looks more like bool in the code using it.

[...]
> --- a/drivers/gpu/drm/tegra/plane.c
> +++ b/drivers/gpu/drm/tegra/plane.c
[...]
> +static int tegra_plane_check_memory_bandwidth(struct drm_plane_state *state)

The function's body looks more like 'update' or 'recalculate' rather
than 'check' the memory bandwidth.

> + struct tegra_plane_state *tegra_state = to_tegra_plane_state(state);
> + unsigned int i, bpp, bpp_plane, dst_w, src_w, src_h, mul;
> + const struct tegra_dc_soc_info *soc;
> + const struct drm_format_info *fmt;
> + struct drm_crtc_state *crtc_state;
> + u32 avg_bandwidth, peak_bandwidth;
> +
> + if (!state->visible)
> + return 0;
> +
> + crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
> + if (!crtc_state)
> + return -EINVAL;
> +
> + src_w = drm_rect_width(&state->src) >> 16;
> + src_h = drm_rect_height(&state->src) >> 16;
> + dst_w = drm_rect_width(&state->dst);
> +
> + fmt = state->fb->format;
> + soc = to_tegra_dc(state->crtc)->soc;
> +
> + /*
> +  * Note that real memory bandwidth vary depending on format and
> +  * memory layout, we are not taking that into account because small
> +  * estimation error isn't important since bandwidth is rounded up
> +  * anyway.
> +  */
> + for (i = 0, bpp = 0; i < fmt->num_planes; i++) {
> + bpp_plane = fmt->cpp[i] * 8;

Nit: you could declare the bpp_plane here.

> + /*
> +  * Sub-sampling is relevant for chroma planes only and vertical
> +  * readouts are not cached, hence only horizontal sub-sampling
> +  * matters.
> +  */
> + if (i > 0)
> + bpp_plane /= fmt->hsub;
> +
> + bpp += bpp_plane;
> + }
> +
> + /*
> +  * Horizontal downscale takes extra bandwidth which roughly depends
> +  * on the scaled width.
> +  */
> + if (src_w > dst_w)
> + mul = (src_w - dst_w) * bpp / 2048 + 1;
> + else
> + mul = 1;

Does it really need more bandwidth to scale down? Does it read the same
data multiple times just to throw it away?

> + /* average bandwidth in bytes/s */
> + avg_bandwidth  = src_w * src_h * bpp / 8 * mul;
> + avg_bandwidth *= drm_mode_vrefresh(&crtc_state->mode);
> +
> + /* mode.clock in kHz, peak bandwidth in kbit/s */
> + peak_bandwidth = crtc_state->mode.clock * bpp * mul;

I would guess that (src_w * bpp / 8) needs rounding up unless the plane
is continuous. Or you could just add the max rounding error here and
get a safe overestimated value. Maybe this would be better done when
summing per-plane widths.

> + /* ICC bandwidth in kbyte/s */
> + peak_bandwidth = kbps_to_icc(peak_bandwidth);
> + avg_bandwidth  = Bps_to_icc(avg_bandwidth);

This could be merged with the assignments after the following 'if' block.

> + /*
> +  * Tegra30/114 Memory Controller can't interleave DC memory requests
> +  * and DC uses 16-bytes atom for the tiled windows, while DDR3 uses 32
> +  * bytes atom. Hence there is x2 memory overfetch for tiled f

Re: [PATCH] drm/atomic: Add the crtc to affected crtc only if uapi.enable = true

[Pekka Paalanen]

On Wed, 3 Mar 2021 12:44:33 -0800
"Navare, Manasi"  wrote:

> On Wed, Mar 03, 2021 at 10:47:44AM +0200, Pekka Paalanen wrote:
> > On Tue,  2 Mar 2021 12:41:32 -0800
> > Manasi Navare  wrote:
> >   
> > > In case of a modeset where a mode gets split across mutiple CRTCs
> > > in the driver specific implementation (bigjoiner in i915) we wrongly count
> > > the affected CRTCs based on the drm_crtc_mask and indicate the stolen 
> > > CRTC as
> > > an affected CRTC in atomic_check_only().
> > > This triggers a warning since affected CRTCs doent match requested CRTC.
> > > 
> > > To fix this in such bigjoiner configurations, we should only
> > > increment affected crtcs if that CRTC is enabled in UAPI not
> > > if it is just used internally in the driver to split the mode.  
> > 
> > Hi,
> > 
> > I think that makes sense to me. Stealing CRTCs that are not currently
> > used by the userspace (display server) should be ok, as long as that
> > is completely invisible to userspace: meaning that it does not cause
> > userspace to unexpectedly e.g. receive or miss per-crtc atomic
> > completion events.  
> 
> Yes since we are only doing atomic_check_only() here, the stolen

But the real not-test-only commit will follow if this test-only commit
succeeds, and keeping the guarantees for the real commit are important.

> crtc is completely invisible to the userspace and hence that is 
> indicated by uapi.enable which is not true for this stolen
> crtc. However if allow modeset flag set, then it will do a full
> modeset and indicate the uapi.enable for this stolen crtc as well
> since that cannot be used for other modeset requested by userspace.
> 
> > 
> > Can that also be asserted somehow, or does this already do that?  
> 
> Not clear what you want the assertion for? Could you elaborate

As assertion that when the real atomic commit happens and then
completion events are fired, they match exactly the affected crtcs mask.

I understand this may be off-topic for this particular patch, but since
we are discussing the topic, such checks would be really nice. I'm
curious if such checks already exist.


Thanks,
pq

> 
> Manasi
> 
> > 
> > 
> > Thanks,
> > pq
> >   
> > > Cc: Ville Syrjälä 
> > > Cc: Simon Ser 
> > > Cc: Pekka Paalanen 
> > > Cc: Daniel Stone 
> > > Cc: Daniel Vetter 
> > > Cc: dri-devel@lists.freedesktop.org
> > > Signed-off-by: Manasi Navare 
> > > ---
> > >  drivers/gpu/drm/drm_atomic.c | 6 --
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 5b4547e0f775..d7acd6bbd97e 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -1358,8 +1358,10 @@ int drm_atomic_check_only(struct drm_atomic_state 
> > > *state)
> > >   }
> > >   }
> > >  
> > > - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
> > > - affected_crtc |= drm_crtc_mask(crtc);
> > > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> > > + if (new_crtc_state->enable)
> > > + affected_crtc |= drm_crtc_mask(crtc);
> > > + }
> > >  
> > >   /*
> > >* For commits that allow modesets drivers can add other CRTCs to the  
> >   
> 
> 



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

Re: [PATCH v3 1/2] dt-bindings: display: bridge: Add bindings for Chipone ICN6211

[Jagan Teki]

Hi Laurent,

On Thu, Mar 4, 2021 at 4:29 AM Laurent Pinchart
 wrote:
>
> Hi Jagan,
>
> On Wed, Mar 03, 2021 at 08:08:35PM +0530, Jagan Teki wrote:
> > On Wed, Feb 24, 2021 at 6:44 PM Laurent Pinchart wrote:
> > > On Wed, Feb 24, 2021 at 06:07:43PM +0530, Jagan Teki wrote:
> > > > On Mon, Feb 15, 2021 at 5:48 PM Laurent Pinchart wrote:
> > > > > On Sun, Feb 14, 2021 at 11:22:10PM +0530, Jagan Teki wrote:
> > > > > > ICN6211 is MIPI-DSI to RGB Convertor bridge from Chipone.
> > > > > >
> > > > > > It has a flexible configuration of MIPI DSI signal input and
> > > > > > produce RGB565, RGB666, RGB888 output format.
> > > > > >
> > > > > > Add dt-bingings for it.
> > > > > >
> > > > > > Signed-off-by: Jagan Teki 
> > > > > > ---
> > > > > > Changes for v3:
> > > > > > - updated to new dt-bindings style
> > > > > >
> > > > > >  .../display/bridge/chipone,icn6211.yaml   | 90 
> > > > > > +++
> > > > > >  1 file changed, 90 insertions(+)
> > > > > >  create mode 100644 
> > > > > > Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml
> > > > > >
> > > > > > diff --git 
> > > > > > a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml
> > > > > >  
> > > > > > b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml
> > > > > > new file mode 100644
> > > > > > index ..13764f13fe46
> > > > > > --- /dev/null
> > > > > > +++ 
> > > > > > b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml
> > > > > > @@ -0,0 +1,90 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: 
> > > > > > http://devicetree.org/schemas/display/bridge/chipone,icn6211.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: Chipone ICN6211 MIPI-DSI to RGB Converter bridge
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Jagan Teki 
> > > > > > +
> > > > > > +description: |
> > > > > > +  ICN6211 is MIPI-DSI to RGB Convertor bridge from chipone.
> > > > > > +
> > > > > > +  It has a flexible configuration of MIPI DSI signal input and
> > > > > > +  produce RGB565, RGB666, RGB888 output format.
> > > > >
> > > > > How does one select between the output formats ? Should the output
> > > > > connection option be described in the device tree ?
> > > >
> > > > I think that is a good option to select output formats via dts. what
> > > > if it makes it a generic property like data-lanes? since it is common
> > > > across many other bridges.
> > >
> > > Describing the output connection in the device tree sounds like a good
> > > idea indeed. The bus-width property could be used for this, maybe along
> > > the lines of
> > > https://lore.kernel.org/dri-devel/20201013020619.gg3...@pendragon.ideasonboard.com/.
> >
> > I have seen an issue by passing bus-width where the same bus-with 24
> > can use by RGB888 and RGB666 according to
> > mipi_dsi_pixel_format_to_bpp. Having a default RGB888 format now and
> > update it when it supports properly, can be a good Idea I thought of.
> > Let me know if you have any comments?
>
> I'm fine with hardcoding a default for now. If a given bus wiring (which
> is described in DT by bus-width) can transport different formats, that's
> something that should be configured dynamically, either by querying what
> format a sink (such as a panel) requires, or if both the source and the
> sink can support different formats, possibly by involving userspace in
> the selection.

Not sure how we can get userspace involvement in DRM here, but if
source and sink can have different formats then having precedence to
the source would be a good option as it handles low-level
configuration for formats.

Look like it is worth trying-feature. Maybe we can come up with some
RFC and have a discussion.

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

[PATCH v4 2/2] drm: bridge: Add Chipone ICN6211 MIPI-DSI to RGB bridge

[Jagan Teki]

ICN6211 is MIPI-DSI to RGB Converter bridge from Chipone.

It has a flexible configuration of MIPI DSI signal input and
produce RGB565, RGB666, RGB888 output format.

Add bridge driver for it.

Signed-off-by: Jagan Teki 
---
Changes for v4:
- added regulators
- replace reset with EN
- fixed warnings pointed by Robert
Changes for v3:
- updated the driver to inline with new drm bridge style

 MAINTAINERS  |   1 +
 drivers/gpu/drm/bridge/Kconfig   |  13 +
 drivers/gpu/drm/bridge/Makefile  |   1 +
 drivers/gpu/drm/bridge/chipone-icn6211.c | 293 +++
 4 files changed, 308 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/chipone-icn6211.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 065cbdc889d3..9c59402e51bc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5533,6 +5533,7 @@ DRM DRIVER FOR CHIPONE ICN6211 MIPI-DSI to RGB CONVERTER 
BRIDGE
 M: Jagan Teki 
 S: Maintained
 F: Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml
+F: drivers/gpu/drm/bridge/chipone-icn6211.c
 
 DRM DRIVER FOR FARADAY TVE200 TV ENCODER
 M: Linus Walleij 
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index e4110d6ca7b3..330ee70ed746 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -27,6 +27,19 @@ config DRM_CDNS_DSI
  Support Cadence DPI to DSI bridge. This is an internal
  bridge and is meant to be directly embedded in a SoC.
 
+config DRM_CHIPONE_ICN6211
+   tristate "Chipone ICN6211 MIPI-DSI/RGB Converter bridge"
+   depends on OF
+   select DRM_MIPI_DSI
+   select DRM_PANEL_BRIDGE
+   help
+ ICN6211 is MIPI-DSI/RGB Converter bridge from chipone.
+
+ It has a flexible configuration of MIPI DSI signal input
+ and produce RGB565, RGB666, RGB888 output format.
+
+ If in doubt, say "N".
+
 config DRM_CHRONTEL_CH7033
tristate "Chrontel CH7033 Video Encoder"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 86e7acc76f8d..3eb84b638988 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
+obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o
 obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
 obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o
 obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c 
b/drivers/gpu/drm/bridge/chipone-icn6211.c
new file mode 100644
index ..a6151db95586
--- /dev/null
+++ b/drivers/gpu/drm/bridge/chipone-icn6211.c
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Amarula Solutions(India)
+ * Author: Jagan Teki 
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define HACTIVE_LI 0x20
+#define VACTIVE_LI 0x21
+#define VACTIVE_HACTIVE_HI 0x22
+#define HFP_LI 0x23
+#define HSYNC_LI   0x24
+#define HBP_LI 0x25
+#define HFP_HSW_HBP_HI 0x26
+#define VFP0x27
+#define VSYNC  0x28
+#define VBP0x29
+
+struct chipone {
+   struct device *dev;
+   struct drm_bridge bridge;
+   struct drm_bridge *panel_bridge;
+   struct gpio_desc *enable_gpio;
+   struct regulator *vdd1;
+   struct regulator *vdd2;
+   struct regulator *vdd3;
+};
+
+static inline struct chipone *bridge_to_chipone(struct drm_bridge *bridge)
+{
+   return container_of(bridge, struct chipone, bridge);
+}
+
+static struct drm_display_mode *bridge_to_mode(struct drm_bridge *bridge)
+{
+   return &bridge->encoder->crtc->state->adjusted_mode;
+}
+
+static inline int chipone_dsi_write(struct chipone *icn,  const void *seq,
+   size_t len)
+{
+   struct mipi_dsi_device *dsi = to_mipi_dsi_device(icn->dev);
+
+   return mipi_dsi_generic_write(dsi, seq, len);
+}
+
+#define ICN6211_DSI(icn, seq...)   \
+   {   \
+   const u8 d[] = { seq }; \
+   chipone_dsi_write(icn, d, ARRAY_SIZE(d));   \
+   }
+
+static void chipone_enable(struct drm_bridge *bridge)
+{
+   struct chipone *icn = bridge_to_chipone(bridge);
+   struct drm_display_mode *mode = bridge_to_mode(bridge);
+
+   ICN6211_DSI(icn, 0x7a, 0xc1);
+
+   ICN6211_DSI(icn, HACTIVE_LI, mode->hdisplay & 0xff);
+
+   ICN6211_DSI(icn, VACTIVE_LI, mode->vdisplay & 0xff);
+
+   /**
+* lsb nibble: 2nd nibble of hdisplay
+* msb nibble: 2nd nibble of vdisplay
+*/
+   ICN6211_DSI(icn, VACTIVE_HACTIVE_HI,
+   ((mode->hdisplay >

[PATCH v4 1/2] dt-bindings: display: bridge: Add Chipone ICN6211 bindings

[Jagan Teki]

ICN6211 is MIPI-DSI to RGB Converter bridge from Chipone.

It has a flexible configuration of MIPI DSI signal input and
produces RGB565, RGB666, RGB888 output format.

Add dt-bingings for it.

Signed-off-by: Jagan Teki 
---
Changes for v4:
- fixed Laurent comments
- added regulators
- replace reset with EN
- fixed warnings pointed by Robert
Changes for v3:
- updated to new dt-bindings style

 .../display/bridge/chipone,icn6211.yaml   | 99 +++
 MAINTAINERS   |  5 +
 2 files changed, 104 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml

diff --git 
a/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml 
b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml
new file mode 100644
index ..62c3bd4cb28d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/chipone,icn6211.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Chipone ICN6211 MIPI-DSI to RGB Converter bridge
+
+maintainers:
+  - Jagan Teki 
+
+description: |
+  ICN6211 is MIPI-DSI to RGB Converter bridge from chipone.
+
+  It has a flexible configuration of MIPI DSI signal input and
+  produce RGB565, RGB666, RGB888 output format.
+
+properties:
+  compatible:
+enum:
+  - chipone,icn6211
+
+  reg:
+maxItems: 1
+description: virtual channel number of a DSI peripheral
+
+  enable-gpios:
+description: Bridge EN pin, chip is reset when EN is low.
+
+  vdd1-supply:
+description: A 1.8V/2.5V/3.3V supply that power the MIPI RX.
+
+  vdd2-supply:
+description: A 1.8V/2.5V/3.3V supply that power the PLL.
+
+  vdd3-supply:
+description: A 1.8V/2.5V/3.3V supply that power the RGB output.
+
+  ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+  port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+  Video port for MIPI DSI input
+
+  port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+  Video port for MIPI DPI output (panel or connector).
+
+required:
+  - port@0
+  - port@1
+
+required:
+  - compatible
+  - reg
+  - enable-gpios
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+
+dsi {
+  #address-cells = <1>;
+  #size-cells = <0>;
+
+  bridge@0 {
+compatible = "chipone,icn6211";
+reg = <0>;
+enable-gpios = <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5 */
+
+ports {
+  #address-cells = <1>;
+  #size-cells = <0>;
+
+  port@0 {
+reg = <0>;
+
+bridge_in_dsi: endpoint {
+  remote-endpoint = <&dsi_out_bridge>;
+};
+  };
+
+  port@1 {
+reg = <1>;
+
+bridge_out_panel: endpoint {
+  remote-endpoint = <&panel_out_bridge>;
+};
+  };
+};
+  };
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 9d241b832aae..065cbdc889d3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5529,6 +5529,11 @@ S:   Maintained
 F: Documentation/devicetree/bindings/display/panel/boe,himax8279d.yaml
 F: drivers/gpu/drm/panel/panel-boe-himax8279d.c
 
+DRM DRIVER FOR CHIPONE ICN6211 MIPI-DSI to RGB CONVERTER BRIDGE
+M: Jagan Teki 
+S: Maintained
+F: Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml
+
 DRM DRIVER FOR FARADAY TVE200 TV ENCODER
 M: Linus Walleij 
 S: Maintained
-- 
2.25.1

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

[v1] drm/msm/disp/dpu1: fix warning reported by kernel bot in dpu driver

[Kalyan Thota]

Fix a warning, pointing to an early deference of a variable before
check. This bug was introduced in the following commit.

commit 4259ff7ae509
("drm/msm/dpu: add support for pcc color block in dpu driver")

Reported-by: kernel test robot 
Reported-by: Dan Carpenter 
Signed-off-by: Kalyan Thota 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c
index a7a2453..0f9974c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c
@@ -26,10 +26,16 @@ static void dpu_setup_dspp_pcc(struct dpu_hw_dspp *ctx,
struct dpu_hw_pcc_cfg *cfg)
 {
 
-   u32 base = ctx->cap->sblk->pcc.base;
+   u32 base;
 
-   if (!ctx || !base) {
-   DRM_ERROR("invalid ctx %pK pcc base 0x%x\n", ctx, base);
+   if (!ctx) {
+   DRM_ERROR("invalid dspp ctx %pK\n", ctx);
+   return;
+   }
+
+   base = ctx->cap->sblk->pcc.base;
+   if (!base) {
+   DRM_ERROR("invalid pcc base 0x%x\n", base);
return;
}
 
-- 
2.7.4

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

[PATCH] qxl: Fix uninitialised struct field head.surface_id

[Colin King]

From: Colin Ian King 

The surface_id struct field in head is not being initialized and
static analysis warns that this is being passed through to
dev->monitors_config->heads[i] on an assignment. Clear up this
warning by initializing it to zero.

Addresses-Coverity: ("Uninitialized scalar variable")
Fixes: a6d3c4d79822 ("qxl: hook monitors_config updates into crtc, not 
encoder.")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/qxl/qxl_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c 
b/drivers/gpu/drm/qxl/qxl_display.c
index 012bce0cdb65..10738e04c09b 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -328,6 +328,7 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc 
*crtc,
 
head.id = i;
head.flags = 0;
+   head.surface_id = 0;
oldcount = qdev->monitors_config->count;
if (crtc->state->active) {
struct drm_display_mode *mode = &crtc->mode;
-- 
2.30.0

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

Re: [PATCH] drm/stm: ltdc: Use simple encoder

[yannick Fertre]

Hi Thomas,
I wait a few days before merging it.
Thank you for your help.

Best regards

Yannick


On 3/4/21 9:21 AM, Thomas Zimmermann wrote:

Hi,

shall I merge this patch?

Am 02.03.21 um 18:57 schrieb Jagan Teki:

STM ltdc driver uses an empty implementation for its encoder.
Replace the code with the generic simple encoder.

Signed-off-by: Jagan Teki 
---
  drivers/gpu/drm/stm/ltdc.c | 12 ++--
  1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 7812094f93d6..aeeb43524ca0 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -31,6 +31,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
@@ -1020,14 +1021,6 @@ static int ltdc_crtc_init(struct drm_device 
*ddev, struct drm_crtc *crtc)

  return ret;
  }
-/*
- * DRM_ENCODER
- */
-
-static const struct drm_encoder_funcs ltdc_encoder_funcs = {
-    .destroy = drm_encoder_cleanup,
-};
-
  static void ltdc_encoder_disable(struct drm_encoder *encoder)
  {
  struct drm_device *ddev = encoder->dev;
@@ -1088,8 +1081,7 @@ static int ltdc_encoder_init(struct drm_device 
*ddev, struct drm_bridge *bridge)

  encoder->possible_crtcs = CRTC_MASK;
  encoder->possible_clones = 0;    /* No cloning support */
-    drm_encoder_init(ddev, encoder, 

Query regarding DRM mastership sharing between multiple process

[Hardik Panchal]

Hello Sir/Madam,

I am trying to render some stuff using DRM with Qt GUI application and
decoded stream from Intel H/w decoder.

I have two applications one is for GUI content and another one is for
decoded video streams. While doing this I am facing an issue that only
singal process acquires DRM mastership while the other one is getting
error.

While wondering how to get the privilege to render stuff I came
across GET_MAGIC and AUTH_MAGIC.
Please refer to this text from the MAN page of DRM.

All DRM devices provide authentication mechanisms. Only a DRM-Master is
> allowed to perform mode-setting or modify core state and only one user can
> be DRM-Master at a time. See drmSetMaster
> (3) for
> information on how to become DRM-Master and what the limitations are. Other
> DRM users can be authenticated to the DRM-Master via drmAuthMagic
> (3) so
> they can perform buffer allocations and rendering.
>

As per this the client which is authenticated using magic code should be
able to allocate buffer and rendering.
But while doing this I am not able to use drmModeSetPlane() for rendering
stuff on display from an authenticated client application. It is giving me
Permission Denied.

As per my understanding if the client is authenticated by using
GET/AUTH_MAGIC it should be able to set a plane and render stuff on the
display.

Is my understanding correct? Can we use this method to simultaneously
render from two applications?

Thank you in advance.

Having Addiction of Self-Dependence

*Hardik Panchal*
SDT - Satatya Devices
Contact No. : 9924472937
Email : hardik23...@gmail.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 211997] sys-kernel/gentoo-sources-5.11.2 crash upon disconnect HDMI monitor AMDGPU

[bugzilla-daemon]

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

Stefan de Konink (ste...@konink.de) changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |PATCH_ALREADY_AVAILABLE

--- Comment #3 from Stefan de Konink (ste...@konink.de) ---
Resolved by
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=efc8278eecfd5e6fa36c5d41e71d038f534fe107

-- 
You may reply to this email to add a comment.

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

[Bug 211997] sys-kernel/gentoo-sources-5.11.2 crash upon disconnect HDMI monitor AMDGPU

[bugzilla-daemon]

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

--- Comment #4 from Paul Menzel (pmenzel+bugzilla.kernel@molgen.mpg.de) ---
Thank you for updating the status. Can you please mark it as a duplicate of bug
#211649 [1].

[1]: https://bugzilla.kernel.org/show_bug.cgi?id=211649

-- 
You may reply to this email to add a comment.

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

[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu

[bugzilla-daemon]

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

Paul Menzel (pmenzel+bugzilla.kernel@molgen.mpg.de) changed:

   What|Removed |Added

 CC||pmenzel+bugzilla.kernel.org
   ||@molgen.mpg.de

--- Comment #16 from Paul Menzel (pmenzel+bugzilla.kernel@molgen.mpg.de) ---
Can this bug be closed then?

-- 
You may reply to this email to add a comment.

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

Re: drm/ttm: ttm_bo_release called without lock

[Gerd Hoffmann]

On Thu, Mar 04, 2021 at 08:42:55AM +0100, Thomas Zimmermann wrote:
> (cc'ing Gerd)
> 
> This might be related to the recent clean-up patches for the BO handling in
> qxl.

Yes, it is.  Fixed in drm-misc-next, cherry-picked into drm-misc-fixes,
hopefully lands in -rc2.

take care,
  Gerd

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

Re: [PATCH] devfreq: Register devfreq as a cooling device

[Lukasz Luba]

Hi Daniel,

On 3/4/21 12:50 PM, Daniel Lezcano wrote:

Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.

There are no so many and actually it makes more sense to register the
devfreq device when adding it.

Consequently, every devfreq becomes a cooling device like cpufreq is.

Having a devfreq being registered as a cooling device can not mitigate
a thermal zone if it is not bound to this one. Thus, the current
configurations are not impacted by this change.


There are also different type of devices, which register into devfreq
framework like NoC buses, UFS/eMMC, jpeg and video accelerators, ISP,
etc.
In some platforms there are plenty of those devices and they all would
occupy memory due to private freq_table in devfreq_cooling, function:
devfreq_cooling_gen_tables().

IIRC in OdroidXU4 there are ~20 devfreq devs for NoC buses.

It's true that they will not affect thermal zones, but unnecessarily,
they all will show up in the /sys/class/thermal/ as
thermal-devfreq-X.

IMO the devfreq shouldn't be tight with devfreq cooling thermal.

CpuFreq is different because it handles only CPUs. Here we have
many different devices.

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

[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu

[bugzilla-daemon]

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

--- Comment #17 from Alex Deucher (alexdeuc...@gmail.com) ---
(In reply to Paul Menzel from comment #16)
> Can this bug be closed then?

Only the person that opens the bug can close it, but yes, it can be closed.

-- 
You may reply to this email to add a comment.

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

Re: [PATCH] drm/ttm: soften TTM warnings

[Gerd Hoffmann]

On Wed, Mar 03, 2021 at 04:57:57PM +0100, Christian König wrote:
> QXL indeed unrefs pinned BOs and the warnings are spamming peoples log files.
> 
> Make sure we warn only once until the QXL driver is fixed.

> - dma_resv_assert_held(bo->base.resv);
> + if (!bo->deleted)
> + dma_resv_assert_held(bo->base.resv);

Hmm?  I'm not aware of qxl having problems with this one.
Did I miss something?

> - if (WARN_ON(bo->pin_count)) {
> + if (WARN_ON_ONCE(bo->pin_count)) {

Well, as temporary thing this is rather pointless, qxl fix for this one
is already queued in drm-misc-fixes so this would only land after the
qxl fixes ...

But I think using WARN_ON_ONCE() is a good idea in general, especially
in a code path like this where a single bug can easily cause a flood of
stack traces.

Acked-by: Gerd Hoffmann 

take care,
  Gerd

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

[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu

[bugzilla-daemon]

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

youling...@gmail.com changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |CODE_FIX

-- 
You may reply to this email to add a comment.

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

Re: [PATCH] devfreq: Register devfreq as a cooling device

[Chanwoo Choi]

Hi Daniel,

As Lukasz's comment, actually some devfreq devices like memory bus
might not affect the thermal critically. In the mainline,
there are four types devfreq as following:
1. GPU
2. UFS Storage
3. DMC (Memory Controller)
4. Memory bus like AMBA AXI

I think that you can specify this devfreq device will be used
for cooling device by editing the devfreq_dev_profile structure.

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index bf3047896e41..77966a17d03f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -935,6 +935,13 @@ struct devfreq *devfreq_add_device(struct device *dev,

mutex_unlock(&devfreq_list_lock);

+   if (devfreq->profile->is_cooling_device) {
+   devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);
+   if (IS_ERR(devfreq->cdev))
+   dev_info(dev,
+   "Failed to register devfreq cooling 
device\n");

+   }
+
return devfreq;

 err_init:
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 26ea0850be9b..26dc69f1047b 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -103,6 +103,7 @@ struct devfreq_dev_profile {
unsigned long initial_freq;
unsigned int polling_ms;
enum devfreq_timer timer;
+   bool is_cooling_device;

int (*target)(struct device *dev, unsigned long *freq, u32 flags);
int (*get_dev_status)(struct device *dev,


Thanks
Chanwoo Choi

On 21. 3. 4. 오후 9:50, Daniel Lezcano wrote:

Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.

There are no so many and actually it makes more sense to register the
devfreq device when adding it.

Consequently, every devfreq becomes a cooling device like cpufreq is.

Having a devfreq being registered as a cooling device can not mitigate
a thermal zone if it is not bound to this one. Thus, the current
configurations are not impacted by this change.

Signed-off-by: Daniel Lezcano 
---
  drivers/devfreq/devfreq.c   |  8 
  drivers/gpu/drm/lima/lima_devfreq.c | 13 -
  drivers/gpu/drm/lima/lima_devfreq.h |  2 --
  drivers/gpu/drm/msm/msm_gpu.c   | 11 ---
  drivers/gpu/drm/msm/msm_gpu.h   |  2 --
  drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 -
  include/linux/devfreq.h |  3 +++
  7 files changed, 11 insertions(+), 41 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6d63f02d293..19149b31b000 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -11,6 +11,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -26,6 +27,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include "governor.h"
  
@@ -935,6 +937,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
  
  	mutex_unlock(&devfreq_list_lock);
  
+	devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);

+   if (IS_ERR(devfreq->cdev))
+   dev_info(dev, "Failed to register devfreq cooling device\n");
+
return devfreq;
  
  err_init:

@@ -960,6 +966,8 @@ int devfreq_remove_device(struct devfreq *devfreq)
if (!devfreq)
return -EINVAL;
  
+	thermal_cooling_device_unregister(devfreq->cdev);

+
if (devfreq->governor) {
devfreq->governor->event_handler(devfreq,
 DEVFREQ_GOV_STOP, NULL);
diff --git a/drivers/gpu/drm/lima/lima_devfreq.c 
b/drivers/gpu/drm/lima/lima_devfreq.c
index 5686ad4aaf7c..a696eff1642c 100644
--- a/drivers/gpu/drm/lima/lima_devfreq.c
+++ b/drivers/gpu/drm/lima/lima_devfreq.c
@@ -7,7 +7,6 @@
   */
  #include 
  #include 
-#include 
  #include 
  #include 
  #include 
@@ -90,11 +89,6 @@ void lima_devfreq_fini(struct lima_device *ldev)
  {
struct lima_devfreq *devfreq = &ldev->devfreq;
  
-	if (devfreq->cooling) {

-   devfreq_cooling_unregister(devfreq->cooling);
-   devfreq->cooling = NULL;
-   }
-
if (devfreq->devfreq) {
devm_devfreq_remove_device(ldev->dev, devfreq->devfreq);
devfreq->devfreq = NULL;
@@ -110,7 +104,6 @@ void lima_devfreq_fini(struct lima_device *ldev)
  
  int lima_devfreq_init(struct lima_device *ldev)

  {
-   struct thermal_cooling_device *cooling;
struct device *dev = ldev->dev;
struct opp_table *opp_table;
struct devfreq *devfreq;
@@ -173,12 +166,6 @@ int lima_devfreq_init(struct lima_device *ldev)
  
  	ldevfreq->devfreq = devfreq;
  
-	cooling = of_devfreq_cooling_register(dev->of_node, devfreq);

-   if (IS_ERR(cooling))
-   dev_info(dev, "Failed to register cooling device\n");
-   else
-   ldevfreq->cooling = cooling;
-
return 0;
  
  err_fini:

diff --git a/driver

WARNING: CPU: 5 PID: 69 at drivers/gpu/drm/ttm/ttm_bo.c:139 ttm_bo_move_to_lru_tail+0x376/0x500 [ttm]

[Jeff Layton]

I was doing some testing in a KVM with a kernel based on this commit:

f69d02e37a85 (origin/master, dhowells/master, master) Merge tag 
'misc-5.12-2021-03-02' of git://git.kernel.dk/linux-block

...with some ceph+fscache patches on top, and am getting a ton of warnings
popping that look like this:

[  147.601271] [ cut here ]
[  147.606720] WARNING: CPU: 5 PID: 69 at drivers/gpu/drm/ttm/ttm_bo.c:139 
ttm_bo_move_to_lru_tail+0x376/0x500 [ttm]
[  147.613224] Modules linked in: nft_ct(E) nft_chain_nat(E) nf_nat(E) 
nf_conntrack(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) ip_set(E) rfkill(E) 
nf_tables(E) nfnetlink(E) cachefiles(E) fscache(E) netfs(E) sunrpc(E) 
intel_rapl_msr(E) intel_rapl_common(E) iTCO_wdt(E) intel_pmc_bxt(E) 
iTCO_vendor_support(E) joydev(E) virtio_balloon(E) pcspkr(E) i2c_i801(E) 
lpc_ich(E) i2c_smbus(E) fuse(E) zram(E) xfs(E) crct10dif_pclmul(E) 
crc32_pclmul(E) crc32c_intel(E) virtio_blk(E) virtio_console(E) 
ghash_clmulni_intel(E) serio_raw(E) virtio_net(E) net_failover(E) failover(E) 
qxl(E) drm_ttm_helper(E) ttm(E) drm_kms_helper(E) cec(E) qemu_fw_cfg(E) drm(E) 
[last unloaded: ip_tables]
[  147.651585] CPU: 5 PID: 69 Comm: kworker/5:1 Tainted: GW   E 
5.12.0-rc1+ #62
[  147.657667] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
1.14.0-1.fc33 04/01/2014
[  147.663860] Workqueue: events qxl_gc_work [qxl]
[  147.669451] RIP: 0010:ttm_bo_move_to_lru_tail+0x376/0x500 [ttm]
[  147.675241] Code: ff 48 8d bd 38 01 00 00 e8 b7 cd 00 c7 48 8b 85 38 01 00 
00 be ff ff ff ff 48 8d 78 70 e8 42 8a fa c7 85 c0 0f 85 d4 fc ff ff <0f> 0b e9 
cd fc ff ff 48 8b 7c 24 20 e8 49 cc 00 c7 44 8b a5 f8 02
[  147.687933] RSP: 0018:888100e4fb60 EFLAGS: 00010246
[  147.693596] RAX:  RBX: 88812666a270 RCX: 0001
[  147.699529] RDX: 0002 RSI: 88812666a1b0 RDI: 888100e3c418
[  147.705425] RBP: 88812666a000 R08:  R09: 8b7e7b4f
[  147.711335] R10: fbfff16fcf69 R11: 0001 R12: 88812bd49000
[  147.717216] R13: 88812bd4bc00 R14: 88812196ce90 R15: c0561920
[  147.723104] FS:  () GS:88841dc0() 
knlGS:
[  147.729160] CS:  0010 DS:  ES:  CR0: 80050033
[  147.734859] CR2: 55d11e6120b0 CR3: 6ba2c000 CR4: 003506e0
[  147.740764] Call Trace:
[  147.745942]  ttm_bo_release+0x78f/0x850 [ttm]
[  147.751393]  qxl_bo_unref+0x51/0x70 [qxl]
[  147.756761]  qxl_release_free_list+0xa1/0x160 [qxl]
[  147.762287]  ? qxl_get_timeline_name+0x10/0x10 [qxl]
[  147.767856]  qxl_release_free+0xbb/0x140 [qxl]
[  147.773300]  qxl_garbage_collect+0x18a/0x280 [qxl]
[  147.778788]  ? qxl_queue_garbage_collect+0xd0/0xd0 [qxl]
[  147.784375]  ? lockdep_hardirqs_on_prepare+0x178/0x220
[  147.789931]  process_one_work+0x525/0x9b0
[  147.795314]  ? pwq_dec_nr_in_flight+0x110/0x110
[  147.800744]  ? lock_acquired+0x2fe/0x560
[  147.806137]  worker_thread+0x2ea/0x6e0
[  147.811511]  ? __kthread_parkme+0xc0/0xe0
[  147.816916]  ? process_one_work+0x9b0/0x9b0
[  147.822326]  kthread+0x1fb/0x220
[  147.827570]  ? __kthread_bind_mask+0x70/0x70
[  147.832964]  ret_from_fork+0x22/0x30
[  147.838296] irq event stamp: 194523
[  147.843565] hardirqs last  enabled at (194533): [] 
console_unlock+0x631/0x740
[  147.849649] hardirqs last disabled at (194542): [] 
console_unlock+0x57e/0x740
[  147.855777] softirqs last  enabled at (193620): [] 
__irq_exit_rcu+0x13e/0x190
[  147.861908] softirqs last disabled at (193615): [] 
__irq_exit_rcu+0x13e/0x190
[  147.868011] ---[ end trace 595bd7e9298cfe26 ]---

Is this a known bug? Is there a patch that fixes it? I'd be happy to
test it since it's making testing in VMs difficult right now.

Thanks,
-- 
Jeff Layton 

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

Re: [PATCH 14/17] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE

[Robin Murphy]

On 2021-03-01 08:42, Christoph Hellwig wrote:

Use explicit methods for setting and querying the information instead.


Now that everyone's using iommu-dma, is there any point in bouncing this 
through the drivers at all? Seems like it would make more sense for the 
x86 drivers to reflect their private options back to iommu_dma_strict 
(and allow Intel's caching mode to override it as well), then have 
iommu_dma_init_domain just test !iommu_dma_strict && 
domain->ops->flush_iotlb_all.


Robin.


Also remove the now unused iommu_domain_get_attr functionality.

Signed-off-by: Christoph Hellwig 
---
  drivers/iommu/amd/iommu.c   | 23 ++---
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 47 ++---
  drivers/iommu/arm/arm-smmu/arm-smmu.c   | 56 +
  drivers/iommu/dma-iommu.c   |  8 ++-
  drivers/iommu/intel/iommu.c | 27 ++
  drivers/iommu/iommu.c   | 19 +++
  include/linux/iommu.h   | 17 ++-
  7 files changed, 51 insertions(+), 146 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a69a8b573e40d0..37a8e51db17656 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1771,24 +1771,11 @@ static struct iommu_group 
*amd_iommu_device_group(struct device *dev)
return acpihid_device_group(dev);
  }
  
-static int amd_iommu_domain_get_attr(struct iommu_domain *domain,

-   enum iommu_attr attr, void *data)
+static bool amd_iommu_dma_use_flush_queue(struct iommu_domain *domain)
  {
-   switch (domain->type) {
-   case IOMMU_DOMAIN_UNMANAGED:
-   return -ENODEV;
-   case IOMMU_DOMAIN_DMA:
-   switch (attr) {
-   case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
-   *(int *)data = !amd_iommu_unmap_flush;
-   return 0;
-   default:
-   return -ENODEV;
-   }
-   break;
-   default:
-   return -EINVAL;
-   }
+   if (domain->type != IOMMU_DOMAIN_DMA)
+   return false;
+   return !amd_iommu_unmap_flush;
  }
  
  /*

@@ -2257,7 +2244,7 @@ const struct iommu_ops amd_iommu_ops = {
.release_device = amd_iommu_release_device,
.probe_finalize = amd_iommu_probe_finalize,
.device_group = amd_iommu_device_group,
-   .domain_get_attr = amd_iommu_domain_get_attr,
+   .dma_use_flush_queue = amd_iommu_dma_use_flush_queue,
.get_resv_regions = amd_iommu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
.is_attach_deferred = amd_iommu_is_attach_deferred,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 8594b4a8304375..bf96172e8c1f71 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2449,33 +2449,21 @@ static struct iommu_group *arm_smmu_device_group(struct 
device *dev)
return group;
  }
  
-static int arm_smmu_domain_get_attr(struct iommu_domain *domain,

-   enum iommu_attr attr, void *data)
+static bool arm_smmu_dma_use_flush_queue(struct iommu_domain *domain)
  {
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
  
-	switch (domain->type) {

-   case IOMMU_DOMAIN_UNMANAGED:
-   switch (attr) {
-   case DOMAIN_ATTR_NESTING:
-   *(int *)data = (smmu_domain->stage == 
ARM_SMMU_DOMAIN_NESTED);
-   return 0;
-   default:
-   return -ENODEV;
-   }
-   break;
-   case IOMMU_DOMAIN_DMA:
-   switch (attr) {
-   case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
-   *(int *)data = smmu_domain->non_strict;
-   return 0;
-   default:
-   return -ENODEV;
-   }
-   break;
-   default:
-   return -EINVAL;
-   }
+   if (domain->type != IOMMU_DOMAIN_DMA)
+   return false;
+   return smmu_domain->non_strict;
+}
+
+
+static void arm_smmu_dma_enable_flush_queue(struct iommu_domain *domain)
+{
+   if (domain->type != IOMMU_DOMAIN_DMA)
+   return;
+   to_smmu_domain(domain)->non_strict = true;
  }
  
  static int arm_smmu_domain_set_attr(struct iommu_domain *domain,

@@ -2505,13 +2493,7 @@ static int arm_smmu_domain_set_attr(struct iommu_domain 
*domain,
}
break;
case IOMMU_DOMAIN_DMA:
-   switch(attr) {
-   case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
-   smmu_domain->non_strict = *(int *)data;
-   break;
-   default:
-   ret = -EN

Re: [PATCH 16/17] iommu: remove DOMAIN_ATTR_IO_PGTABLE_CFG

[Robin Murphy]

On 2021-03-01 08:42, Christoph Hellwig wrote:

Signed-off-by: Christoph Hellwig 


Moreso than the previous patch, where the feature is at least relatively 
generic (note that there's a bunch of in-flight development around 
DOMAIN_ATTR_NESTING), I'm really not convinced that it's beneficial to 
bloat the generic iommu_ops structure with private driver-specific 
interfaces. The attribute interface is a great compromise for these 
kinds of things, and you can easily add type-checked wrappers around it 
for external callers (maybe even make the actual attributes internal 
between the IOMMU core and drivers) if that's your concern.


Robin.


---
  drivers/gpu/drm/msm/adreno/adreno_gpu.c |  2 +-
  drivers/iommu/arm/arm-smmu/arm-smmu.c   | 40 +++--
  drivers/iommu/iommu.c   |  9 ++
  include/linux/iommu.h   |  9 +-
  4 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 0f184c3dd9d9ec..78d98ab2ee3a68 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -191,7 +191,7 @@ void adreno_set_llc_attributes(struct iommu_domain *iommu)
struct io_pgtable_domain_attr pgtbl_cfg;
  
  	pgtbl_cfg.quirks = IO_PGTABLE_QUIRK_ARM_OUTER_WBWA;

-   iommu_domain_set_attr(iommu, DOMAIN_ATTR_IO_PGTABLE_CFG, &pgtbl_cfg);
+   iommu_domain_set_pgtable_attr(iommu, &pgtbl_cfg);
  }
  
  struct msm_gem_address_space *

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 2e17d990d04481..2858999c86dfd1 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1515,40 +1515,22 @@ static int arm_smmu_domain_enable_nesting(struct 
iommu_domain *domain)
return ret;
  }
  
-static int arm_smmu_domain_set_attr(struct iommu_domain *domain,

-   enum iommu_attr attr, void *data)
+static int arm_smmu_domain_set_pgtable_attr(struct iommu_domain *domain,
+   struct io_pgtable_domain_attr *pgtbl_cfg)
  {
-   int ret = 0;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   int ret = -EPERM;
  
-	mutex_lock(&smmu_domain->init_mutex);

-
-   switch(domain->type) {
-   case IOMMU_DOMAIN_UNMANAGED:
-   switch (attr) {
-   case DOMAIN_ATTR_IO_PGTABLE_CFG: {
-   struct io_pgtable_domain_attr *pgtbl_cfg = data;
-
-   if (smmu_domain->smmu) {
-   ret = -EPERM;
-   goto out_unlock;
-   }
+   if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+   return -EINVAL;
  
-			smmu_domain->pgtbl_cfg = *pgtbl_cfg;

-   break;
-   }
-   default:
-   ret = -ENODEV;
-   }
-   break;
-   case IOMMU_DOMAIN_DMA:
-   ret = -ENODEV;
-   break;
-   default:
-   ret = -EINVAL;
+   mutex_lock(&smmu_domain->init_mutex);
+   if (!smmu_domain->smmu) {
+   smmu_domain->pgtbl_cfg = *pgtbl_cfg;
+   ret = 0;
}
-out_unlock:
mutex_unlock(&smmu_domain->init_mutex);
+
return ret;
  }
  
@@ -1609,7 +1591,7 @@ static struct iommu_ops arm_smmu_ops = {

.device_group   = arm_smmu_device_group,
.dma_use_flush_queue= arm_smmu_dma_use_flush_queue,
.dma_enable_flush_queue = arm_smmu_dma_enable_flush_queue,
-   .domain_set_attr= arm_smmu_domain_set_attr,
+   .domain_set_pgtable_attr = arm_smmu_domain_set_pgtable_attr,
.domain_enable_nesting  = arm_smmu_domain_enable_nesting,
.of_xlate   = arm_smmu_of_xlate,
.get_resv_regions   = arm_smmu_get_resv_regions,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2e9e058501a953..8490aefd4b41f8 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2693,6 +2693,15 @@ int iommu_domain_enable_nesting(struct iommu_domain 
*domain)
  }
  EXPORT_SYMBOL_GPL(iommu_domain_enable_nesting);
  
+int iommu_domain_set_pgtable_attr(struct iommu_domain *domain,

+   struct io_pgtable_domain_attr *pgtbl_cfg)
+{
+   if (!domain->ops->domain_set_pgtable_attr)
+   return -EINVAL;
+   return domain->ops->domain_set_pgtable_attr(domain, pgtbl_cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_domain_set_pgtable_attr);
+
  void iommu_get_resv_regions(struct device *dev, struct list_head *list)
  {
const struct iommu_ops *ops = dev->bus->iommu_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index aed88aa3bd3edf..39d3ed4d2700ac 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -40,6 +40,7 @@ struct iommu_domain;
  struct notifier_block;
  struct iommu_sva;
  struct iommu_fault_event;
+stru

Re: [PATCH] pinctrl/sunxi: adding input-debounce-ns property

[Maxime Ripard]

On Fri, Feb 26, 2021 at 01:53:00PM +0100, Marjan Pascolo wrote:
> Hi Maxime,
> 
> Il 17/02/2021 12:03, Maxime Ripard ha scritto:
> > Hi,
> > 
> > On Wed, Feb 10, 2021 at 05:22:37PM +0100, Marjan Pascolo wrote:
> > > On Allwinner SoC interrupt debounce can be controlled by two oscillator
> > > (32KHz and 24MHz) and a prescale divider.
> > > Oscillator and prescale divider are set through
> > > device tree property "input-debounce" which have 1uS accuracy.
> > > For acheive nS precision a new device tree poperty is made
> > > named "input-debounce-ns".
> > > "input-debounce-ns" is checked only if "input-debounce"
> > > property is not defined.
> > > 
> > > Suggested-by: Maxime Ripard 
> > > Signed-off-by: Marjan Pascolo 
> > > ---
> > > ---
> > >   .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  |  9 +++
> > >   drivers/pinctrl/sunxi/pinctrl-sunxi.c | 25 ---
> > >   2 files changed, 30 insertions(+), 4 deletions(-)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > index 5240487dfe50..346776de3a44 100644
> > > ---
> > > a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > +++
> > > b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > @@ -93,6 +93,15 @@ properties:
> > >   minItems: 1
> > >   maxItems: 5
> > > 
> > > +  input-debounce-ns:
> > > +    description:
> > > +  Debouncing periods in nanoseconds, one period per interrupt
> > > +  bank found in the controller.
> > > +  Only checked if input-debounce is not present
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +    minItems: 1
> > > +    maxItems: 5
> > > +
> > This should be a separate patch, with the DT maintainers in Cc.
> > 
> > You should enforce that the properties are mutually exclusive through
> > the schema too
> 
> I'm sorry, I've ignored documentaion about /Documentation.
> 
> I see that some additional YAML operator (like oneOf) are used.
> 
> oneOf should fit the schema, but I can't understand if oneOf's options must
> be a literal value, or if could also be a node.
> 
> Otherwise I'll use if ..then..else.

dependencies is what you're looking for, not oneOf or if

Maxime


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

[PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

[Oak Zeng]

If tbo.mem.bus.caching is cached, buffer is intended to be mapped
as cached from CPU. Map it with ioremap_cache.

This wasn't necessary before as device memory was never mapped
as cached from CPU side. It becomes necessary for aldebaran as
device memory is mapped cached from CPU.

Signed-off-by: Oak Zeng 
Reviewed-by: Christian Konig 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 031e581..7429464 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -91,6 +91,10 @@ static int ttm_resource_ioremap(struct ttm_device *bdev,
 
if (mem->bus.caching == ttm_write_combined)
addr = ioremap_wc(mem->bus.offset, bus_size);
+#ifdef CONFIG_X86
+   else if (mem->bus.caching == ttm_cached)
+   addr = ioremap_cache(mem->bus.offset, bus_size);
+#endif
else
addr = ioremap(mem->bus.offset, bus_size);
if (!addr) {
@@ -372,6 +376,11 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
if (mem->bus.caching == ttm_write_combined)
map->virtual = ioremap_wc(bo->mem.bus.offset + offset,
  size);
+#ifdef CONFIG_X86
+   else if (mem->bus.caching == ttm_cached)
+   map->virtual = ioremap_cache(bo->mem.bus.offset + 
offset,
+ size);
+#endif
else
map->virtual = ioremap(bo->mem.bus.offset + offset,
   size);
@@ -490,6 +499,11 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo, struct 
dma_buf_map *map)
else if (mem->bus.caching == ttm_write_combined)
vaddr_iomem = ioremap_wc(mem->bus.offset,
 bo->base.size);
+   else if (mem->bus.caching == ttm_cached)
+#ifdef CONFIG_X86
+   vaddr_iomem = ioremap_cache(mem->bus.offset,
+ bo->base.size);
+#endif
else
vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);
 
-- 
2.7.4

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

Re: [Intel-gfx] [WARNING] v5.12-rc1 in intel_pipe_disable tracepoint

[Ville Syrjälä]

On Mon, Mar 01, 2021 at 07:20:59PM +0200, Ville Syrjälä wrote:
> On Mon, Mar 01, 2021 at 11:59:46AM -0500, Steven Rostedt wrote:
> > 
> > On my test box with latest v5.12-rc1, running with all trace events
> > enabled, I consistently trigger this warning.
> > 
> > It appears to get triggered by the trace_intel_pipe_disable() code.
> > 
> > -- Steve
> > 
> >  [ cut here ]
> >  i915 :00:02.0: drm_WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev))
> >  WARNING: CPU: 7 PID: 1258 at drivers/gpu/drm/drm_vblank.c:728 
> > drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x319/0x330 [drm]
> >  Modules linked in: ebtable_filter ebtables bridge stp llc vsock vmw_vmci 
> > ipt_REJECT nf_reject_ipv4 iptable_filter ip6t_REJECT nf_reject_ipv6 
> > xt_state xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 
> > ip6table_filter ip6_tables snd_hda_codec_hdmi snd_h
> > ek snd_hda_codec_generic ledtrig_audio snd_hda_intel snd_intel_dspcfg 
> > snd_hda_codec joydev snd_hwdep intel_rapl_msr snd_hda_core hp_wmi i915 
> > iTCO_wdt snd_seq intel_rapl_common iTCO_vendor_support wmi_bmof 
> > sparse_keymap snd_seq_device rfkill snd_pcm x86_pkg_t
> > d_timer i2c_algo_bit drm_kms_helper mei_me intel_powerclamp snd mei 
> > soundcore i2c_i801 drm coretemp lpc_ich e1000e kvm_intel i2c_smbus kvm 
> > irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel serio_raw 
> > ghash_clmulni_intel video tpm_infineon wmi ip_tables
> >  CPU: 7 PID: 1258 Comm: Xorg Tainted: GW 5.12.0-rc1-test+ 
> > #12
> >  Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 
> > v03.03 07/14/2016
> >  RIP: 0010:drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x319/0x330 
> > [drm]
> >  Code: 4c 8b 6f 50 4d 85 ed 75 03 4c 8b 2f e8 60 92 45 c2 48 c7 c1 28 a5 3c 
> > c0 4c 89 ea 48 c7 c7 15 5a 3c c0 48 89 c6 e8 1f e7 7b c2 <0f> 0b e9 e2 fe 
> > ff ff e8 fb 6c 81 c2 66 66 2e 0f 1f 84 00 00 00 00
> >  RSP: 0018:b77580ea7920 EFLAGS: 00010082
> >  RAX:  RBX: 8afe500c RCX: 
> >  RDX: 0004 RSI: 833c86b8 RDI: 0001
> >  RBP: b77580ea7990 R08: 00700c782173 R09: 
> >  R10: 0001 R11: 0001 R12: 
> >  R13: 8afe41c7eff0 R14: c05e0410 R15: 8afe456a2bf8
> >  FS:  7f8f91869f00() GS:8afe5aa0() 
> > knlGS:
> >  CS:  0010 DS:  ES:  CR0: 80050033
> >  CR2: 7f9523a6cad0 CR3: 01b78002 CR4: 001706e0
> >  Call Trace:
> >   drm_get_last_vbltimestamp+0xaa/0xc0 [drm]
> >   drm_update_vblank_count+0x90/0x2d0 [drm]
> >   drm_crtc_accurate_vblank_count+0x3e/0xc0 [drm]
> >   intel_crtc_get_vblank_counter+0x43/0x50 [i915]
> >   trace_event_raw_event_intel_pipe_disable+0x87/0x110 [i915]
> >   intel_disable_pipe+0x1a8/0x210 [i915]
> 
> Hmm. Yeah we do vblank_off() before pipe_disable() which wants
> to still grab the frame counter in the tracepoint. I think we
> could reorder those two without causing any problems. Either
> that or we put the tracepoint before vblank_off().
> 
> >   ilk_crtc_disable+0x85/0x390 [i915]
> 
> But this part is confusing me. intel_crtc_get_vblank_counter() is
> only supposed to do drm_crtc_accurate_vblank_count() fallback when
> the hardware lacks a working frame counter, and that should only
> be the case for ancient gen2 or semi-ancient i965gm TV output,
> ilk_crtc_disable() is not the function we should be calling in
> either of those cases.

OK, figured this out. It can happen on any platform due to
never initializing .max_vblank_count for pipes that were
disabled at boot. Also spotted some other issues in this
code that needs fixing. I'll mail out some patches...

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

Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

[Bhardwaj, Rajneesh]

I was wondering if a managed version of such API exists but looks like 
none. We only have devm_ioremap_wc but that is valid only for 
PAGE_CACHE_MODE_WC whereas ioremap_cache uses _WB. One more small 
comment below.



Acked-by: Rajneesh Bhardwaj 

On 3/4/2021 11:04 AM, Oak Zeng wrote:

If tbo.mem.bus.caching is cached, buffer is intended to be mapped
as cached from CPU. Map it with ioremap_cache.

This wasn't necessary before as device memory was never mapped
as cached from CPU side. It becomes necessary for aldebaran as
device memory is mapped cached from CPU.

Signed-off-by: Oak Zeng 
Reviewed-by: Christian Konig 
---
  drivers/gpu/drm/ttm/ttm_bo_util.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 031e581..7429464 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -91,6 +91,10 @@ static int ttm_resource_ioremap(struct ttm_device *bdev,
  
  		if (mem->bus.caching == ttm_write_combined)

addr = ioremap_wc(mem->bus.offset, bus_size);
+#ifdef CONFIG_X86



Please use #if defined (CONFIG_X86)


+   else if (mem->bus.caching == ttm_cached)
+   addr = ioremap_cache(mem->bus.offset, bus_size);
+#endif
else
addr = ioremap(mem->bus.offset, bus_size);
if (!addr) {
@@ -372,6 +376,11 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
if (mem->bus.caching == ttm_write_combined)
map->virtual = ioremap_wc(bo->mem.bus.offset + offset,
  size);
+#ifdef CONFIG_X86
+   else if (mem->bus.caching == ttm_cached)
+   map->virtual = ioremap_cache(bo->mem.bus.offset + 
offset,
+ size);
+#endif
else
map->virtual = ioremap(bo->mem.bus.offset + offset,
   size);
@@ -490,6 +499,11 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo, struct 
dma_buf_map *map)
else if (mem->bus.caching == ttm_write_combined)
vaddr_iomem = ioremap_wc(mem->bus.offset,
 bo->base.size);
+   else if (mem->bus.caching == ttm_cached)
+#ifdef CONFIG_X86
+   vaddr_iomem = ioremap_cache(mem->bus.offset,
+ bo->base.size);
+#endif
else
vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);
  

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

Re: [PATCH] devfreq: Register devfreq as a cooling device

[Lukasz Luba]

On 3/4/21 4:53 PM, Daniel Lezcano wrote:


Hi Lukasz,

thanks for commenting this patch,

On 04/03/2021 14:47, Lukasz Luba wrote:

Hi Daniel,

On 3/4/21 12:50 PM, Daniel Lezcano wrote:

Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.

There are no so many and actually it makes more sense to register the
devfreq device when adding it.

Consequently, every devfreq becomes a cooling device like cpufreq is.

Having a devfreq being registered as a cooling device can not mitigate
a thermal zone if it is not bound to this one. Thus, the current
configurations are not impacted by this change.


There are also different type of devices, which register into devfreq
framework like NoC buses, UFS/eMMC, jpeg and video accelerators, ISP,
etc.
In some platforms there are plenty of those devices and they all would
occupy memory due to private freq_table in devfreq_cooling, function:
devfreq_cooling_gen_tables().

IIRC in OdroidXU4 there are ~20 devfreq devs for NoC buses.


I'm curious, do you have a pointer to such kernels having all those
devfreq ?


Sure, it's mainline code, you can build it with exynos_defconfig.
You can check the DT files to find them arch/arm/boot/dts/exynos*.
(this particular OdroidXU4 is Exynos5422, but it grabs some generic dt
files).

Here is the mainline kernel content of /sys/class/devfreq/
--
sys/class/devfreq /
10c2.memory-controller  soc:bus-g2d  soc:bus-mfc
1180.gpusoc:bus-g2d-acp  soc:bus-mscl
soc:bus-disp1   soc:bus-gen  soc:bus-noc
soc:bus-disp1-fimd  soc:bus-gscl-scaler  soc:bus-peri
soc:bus-fsys-apbsoc:bus-jpeg soc:bus-wcore
soc:bus-fsys2   soc:bus-jpeg-apb
--

IIRC some Odroid kernel maintained by Hardkernel had more devices
in this dir.


Here is how these bus devices print themselves during boot:
--
[8.674840] exynos-bus: new bus device registered: soc:bus-wcore ( 
88700 KHz ~ 532000 KHz)
[8.686412] exynos-bus: new bus device registered: soc:bus-noc ( 
66600 KHz ~ 111000 KHz)
[8.696080] exynos-bus: new bus device registered: soc:bus-fsys-apb 
(111000 KHz ~ 222000 KHz)
[8.706590] exynos-bus: new bus device registered: soc:bus-fsys2 ( 
75000 KHz ~ 20 KHz)
[8.717661] exynos-bus: new bus device registered: soc:bus-mfc ( 
83250 KHz ~ 333000 KHz)
[8.728139] exynos-bus: new bus device registered: soc:bus-gen ( 
88700 KHz ~ 266000 KHz)
[8.737551] exynos-bus: new bus device registered: soc:bus-peri ( 
66600 KHz ~  66600 KHz)
[8.748625] exynos-bus: new bus device registered: soc:bus-g2d ( 
83250 KHz ~ 333000 KHz)
[8.759427] exynos-bus: new bus device registered: soc:bus-g2d-acp ( 
66500 KHz ~ 266000 KHz)
[8.770366] exynos-bus: new bus device registered: soc:bus-jpeg ( 
75000 KHz ~ 30 KHz)
[8.781135] exynos-bus: new bus device registered: soc:bus-jpeg-apb ( 
83250 KHz ~ 166500 KHz)
[8.791366] exynos-bus: new bus device registered: soc:bus-disp1-fimd 
(12 KHz ~ 20 KHz)
[8.802418] exynos-bus: new bus device registered: soc:bus-disp1 
(12 KHz ~ 30 KHz)
[8.813050] exynos-bus: new bus device registered: 
soc:bus-gscl-scaler (15 KHz ~ 30 KHz)
[8.825308] exynos-bus: new bus device registered: soc:bus-mscl ( 
84000 KHz ~ 666000 KHz)


--





It's true that they will not affect thermal zones, but unnecessarily,
they all will show up in the /sys/class/thermal/ as
thermal-devfreq-X


IMO the devfreq shouldn't be tight with devfreq cooling thermal.


The energy model is tied with a cooling device initialization.

So if we want to do power limitation, the energy model must be
initialized with the device, thus the cooling device also.

That is the reason why I'm ending up with this change. Chanwoo
suggestion makes sense and that will allow to move the initialization to
devfreq which is more sane but it does not solve the initial problem
with the energy model.


Make sense, the 'is_cooling_device' does the job.

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

Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

[Christian König]

Am 04.03.21 um 18:01 schrieb Bhardwaj, Rajneesh:
I was wondering if a managed version of such API exists but looks like 
none. We only have devm_ioremap_wc but that is valid only for 
PAGE_CACHE_MODE_WC whereas ioremap_cache uses _WB. One more small 
comment below.



Acked-by: Rajneesh Bhardwaj 

On 3/4/2021 11:04 AM, Oak Zeng wrote:

If tbo.mem.bus.caching is cached, buffer is intended to be mapped
as cached from CPU. Map it with ioremap_cache.

This wasn't necessary before as device memory was never mapped
as cached from CPU side. It becomes necessary for aldebaran as
device memory is mapped cached from CPU.

Signed-off-by: Oak Zeng 
Reviewed-by: Christian Konig 
---
  drivers/gpu/drm/ttm/ttm_bo_util.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c

index 031e581..7429464 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -91,6 +91,10 @@ static int ttm_resource_ioremap(struct ttm_device 
*bdev,

    if (mem->bus.caching == ttm_write_combined)
  addr = ioremap_wc(mem->bus.offset, bus_size);
+#ifdef CONFIG_X86



Please use #if defined (CONFIG_X86)


Actually #ifdef is usually preferred.

Christian.




+    else if (mem->bus.caching == ttm_cached)
+    addr = ioremap_cache(mem->bus.offset, bus_size);
+#endif
  else
  addr = ioremap(mem->bus.offset, bus_size);
  if (!addr) {
@@ -372,6 +376,11 @@ static int ttm_bo_ioremap(struct 
ttm_buffer_object *bo,

  if (mem->bus.caching == ttm_write_combined)
  map->virtual = ioremap_wc(bo->mem.bus.offset + offset,
    size);
+#ifdef CONFIG_X86
+    else if (mem->bus.caching == ttm_cached)
+    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
+  size);
+#endif
  else
  map->virtual = ioremap(bo->mem.bus.offset + offset,
 size);
@@ -490,6 +499,11 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo, 
struct dma_buf_map *map)

  else if (mem->bus.caching == ttm_write_combined)
  vaddr_iomem = ioremap_wc(mem->bus.offset,
   bo->base.size);
+    else if (mem->bus.caching == ttm_cached)
+#ifdef CONFIG_X86
+    vaddr_iomem = ioremap_cache(mem->bus.offset,
+  bo->base.size);
+#endif
  else
  vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);

___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

[Bug 211649] "drm/amd/display: reuse current context instead of recreating one" cause hdmi hotplug blackscreen on amdgpu

[bugzilla-daemon]

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

Stefan de Konink (ste...@konink.de) changed:

   What|Removed |Added

 CC||ste...@konink.de

--- Comment #18 from Stefan de Konink (ste...@konink.de) ---
*** Bug 211997 has been marked as a duplicate of this bug. ***

-- 
You may reply to this email to add a comment.

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

[Bug 211997] sys-kernel/gentoo-sources-5.11.2 crash upon disconnect HDMI monitor AMDGPU

[bugzilla-daemon]

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

Stefan de Konink (ste...@konink.de) changed:

   What|Removed |Added

 Resolution|PATCH_ALREADY_AVAILABLE |DUPLICATE

--- Comment #5 from Stefan de Konink (ste...@konink.de) ---


*** This bug has been marked as a duplicate of bug 211649 ***

-- 
You may reply to this email to add a comment.

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

Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

[Bhardwaj, Rajneesh]

On 3/4/2021 12:31 PM, Christian König wrote:

[CAUTION: External Email]

Am 04.03.21 um 18:01 schrieb Bhardwaj, Rajneesh:

I was wondering if a managed version of such API exists but looks like
none. We only have devm_ioremap_wc but that is valid only for
PAGE_CACHE_MODE_WC whereas ioremap_cache uses _WB. One more small
comment below.


Acked-by: Rajneesh Bhardwaj 

On 3/4/2021 11:04 AM, Oak Zeng wrote:

If tbo.mem.bus.caching is cached, buffer is intended to be mapped
as cached from CPU. Map it with ioremap_cache.

This wasn't necessary before as device memory was never mapped
as cached from CPU side. It becomes necessary for aldebaran as
device memory is mapped cached from CPU.

Signed-off-by: Oak Zeng 
Reviewed-by: Christian Konig 
---
  drivers/gpu/drm/ttm/ttm_bo_util.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 031e581..7429464 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -91,6 +91,10 @@ static int ttm_resource_ioremap(struct ttm_device
*bdev,
    if (mem->bus.caching == ttm_write_combined)
  addr = ioremap_wc(mem->bus.offset, bus_size);
+#ifdef CONFIG_X86



Please use #if defined (CONFIG_X86)


Actually #ifdef is usually preferred.


oops, i was referring to IS_ENABLED (CONFIG) and not if defined.




Christian.




+    else if (mem->bus.caching == ttm_cached)
+    addr = ioremap_cache(mem->bus.offset, bus_size);
+#endif
  else
  addr = ioremap(mem->bus.offset, bus_size);
  if (!addr) {
@@ -372,6 +376,11 @@ static int ttm_bo_ioremap(struct
ttm_buffer_object *bo,
  if (mem->bus.caching == ttm_write_combined)
  map->virtual = ioremap_wc(bo->mem.bus.offset + offset,
    size);
+#ifdef CONFIG_X86
+    else if (mem->bus.caching == ttm_cached)
+    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
+  size);
+#endif
  else
  map->virtual = ioremap(bo->mem.bus.offset + offset,
 size);
@@ -490,6 +499,11 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo,
struct dma_buf_map *map)
  else if (mem->bus.caching == ttm_write_combined)
  vaddr_iomem = ioremap_wc(mem->bus.offset,
   bo->base.size);
+    else if (mem->bus.caching == ttm_cached)
+#ifdef CONFIG_X86
+    vaddr_iomem = ioremap_cache(mem->bus.offset,
+  bo->base.size);
+#endif
  else
  vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);

___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Crajneesh.bhardwaj%40amd.com%7Cc4386544ea10487d3a0c08d8df3363a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637504759264793970%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=nM2UtQQdActyapfZSrhfx%2BoJ%2BdszV4Yp62LTehsUWwY%3D&reserved=0 




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

Re: [patch 1/7] drm/ttm: Replace kmap_atomic() usage

[Christian König]

Am 03.03.21 um 14:20 schrieb Thomas Gleixner:

From: Thomas Gleixner 

There is no reason to disable pagefaults and preemption as a side effect of
kmap_atomic_prot().

Use kmap_local_page_prot() instead and document the reasoning for the
mapping usage with the given pgprot.

Remove the NULL pointer check for the map. These functions return a valid
address for valid pages and the return was bogus anyway as it would have
left preemption and pagefaults disabled.

Signed-off-by: Thomas Gleixner 
Cc: Christian Koenig 
Cc: Huang Rui 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
---
  drivers/gpu/drm/ttm/ttm_bo_util.c |   20 
  1 file changed, 12 insertions(+), 8 deletions(-)

--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -181,13 +181,15 @@ static int ttm_copy_io_ttm_page(struct t
return -ENOMEM;
  
  	src = (void *)((unsigned long)src + (page << PAGE_SHIFT));

-   dst = kmap_atomic_prot(d, prot);
-   if (!dst)
-   return -ENOMEM;
+   /*
+* Ensure that a highmem page is mapped with the correct
+* pgprot. For non highmem the mapping is already there.
+*/


I find the comment a bit misleading. Maybe write:

/*
 * Locally map highmem pages with the correct pgprot.
 * Normal memory should already have the correct pgprot in the linear 
mapping.

 */

Apart from that looks good to me.

Regards,
Christian.


+   dst = kmap_local_page_prot(d, prot);
  
  	memcpy_fromio(dst, src, PAGE_SIZE);
  
-	kunmap_atomic(dst);

+   kunmap_local(dst);
  
  	return 0;

  }
@@ -203,13 +205,15 @@ static int ttm_copy_ttm_io_page(struct t
return -ENOMEM;
  
  	dst = (void *)((unsigned long)dst + (page << PAGE_SHIFT));

-   src = kmap_atomic_prot(s, prot);
-   if (!src)
-   return -ENOMEM;
+   /*
+* Ensure that a highmem page is mapped with the correct
+* pgprot. For non highmem the mapping is already there.
+*/
+   src = kmap_local_page_prot(s, prot);
  
  	memcpy_toio(dst, src, PAGE_SIZE);
  
-	kunmap_atomic(src);

+   kunmap_local(src);
  
  	return 0;

  }




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

Re: [PATCH 17/35] drm/amdkfd: register HMM device private zone

[Felix Kuehling]

Am 2021-03-01 um 3:46 a.m. schrieb Thomas Hellström (Intel):
>
> On 3/1/21 9:32 AM, Daniel Vetter wrote:
>> On Wed, Jan 06, 2021 at 10:01:09PM -0500, Felix Kuehling wrote:
>>> From: Philip Yang 
>>>
>>> Register vram memory as MEMORY_DEVICE_PRIVATE type resource, to
>>> allocate vram backing pages for page migration.
>>>
>>> Signed-off-by: Philip Yang 
>>> Signed-off-by: Felix Kuehling 
>> So maybe I'm getting this all wrong, but I think that the current ttm
>> fault code relies on devmap pte entries (especially for hugepte entries)
>> to stop get_user_pages. But this only works if the pte happens to not
>> point at a range with devmap pages.
>
> I don't think that's in TTM yet, but the proposed fix, yes (see email
> I just sent in another thread),
> but only for huge ptes.
>
>>
>> This patch here changes that, and so probably breaks this devmap pte
>> hack
>> ttm is using?
>>
>> If I'm not wrong here then I think we need to first fix up the ttm
>> code to
>> not use the devmap hack anymore, before a ttm based driver can
>> register a
>> dev_pagemap. Also adding Thomas since that just came up in another
>> discussion.
>
> It doesn't break the ttm devmap hack per se, but it indeed allows gup
> to the range registered, but here's where my lack of understanding why
> we can't allow gup-ing TTM ptes if there indeed is a backing
> struct-page? Because registering MEMORY_DEVICE_PRIVATE implies that,
> right?

I wasn't aware that TTM used devmap at all. If it does, what type of
memory does it use?

MEMORY_DEVICE_PRIVATE is like swapped out memory. It cannot be mapped in
the CPU page table. GUP would cause a page fault to swap it back into
system memory. We are looking into use MEMORY_DEVICE_GENERIC for a
future coherent memory architecture, where device memory can be
coherently accessed by the CPU and GPU.

As I understand it, our DEVICE_PRIVATE registration is not tied to an
actual physical address. Thus your devmap registration and our devmap
registration could probably coexist without any conflict. You'll just
have the overhead of two sets of struct pages for the same memory.

Regards,
  Felix


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

Re: [PATCH] drm/ttm: soften TTM warnings

[Christian König]

Am 04.03.21 um 15:05 schrieb Gerd Hoffmann:

On Wed, Mar 03, 2021 at 04:57:57PM +0100, Christian König wrote:

QXL indeed unrefs pinned BOs and the warnings are spamming peoples log files.

Make sure we warn only once until the QXL driver is fixed.
-   dma_resv_assert_held(bo->base.resv);
+   if (!bo->deleted)
+   dma_resv_assert_held(bo->base.resv);

Hmm?  I'm not aware of qxl having problems with this one.
Did I miss something?


See the mail from Peter, but asserts were triggered when the pin_count 
was non zero and destruction.





-   if (WARN_ON(bo->pin_count)) {
+   if (WARN_ON_ONCE(bo->pin_count)) {

Well, as temporary thing this is rather pointless, qxl fix for this one
is already queued in drm-misc-fixes so this would only land after the
qxl fixes ...

But I think using WARN_ON_ONCE() is a good idea in general, especially
in a code path like this where a single bug can easily cause a flood of
stack traces.


Well that flood of stack traces can also be helpful, cause it makes 
people report such kind of issues immediately.


Anyway I'm going to keep that WARN_ON_ONCE for a cycle or two and if I 
don't hear any more complains I'm going to completely remove this 
"feature" and just always warn when we see a non zero pin_count on 
destruction.


Christian.



Acked-by: Gerd Hoffmann 

take care,
   Gerd



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

Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

[Christian König]

Am 04.03.21 um 18:40 schrieb Bhardwaj, Rajneesh:


On 3/4/2021 12:31 PM, Christian König wrote:

[CAUTION: External Email]

Am 04.03.21 um 18:01 schrieb Bhardwaj, Rajneesh:

I was wondering if a managed version of such API exists but looks like
none. We only have devm_ioremap_wc but that is valid only for
PAGE_CACHE_MODE_WC whereas ioremap_cache uses _WB. One more small
comment below.


Acked-by: Rajneesh Bhardwaj 

On 3/4/2021 11:04 AM, Oak Zeng wrote:

If tbo.mem.bus.caching is cached, buffer is intended to be mapped
as cached from CPU. Map it with ioremap_cache.

This wasn't necessary before as device memory was never mapped
as cached from CPU side. It becomes necessary for aldebaran as
device memory is mapped cached from CPU.

Signed-off-by: Oak Zeng 
Reviewed-by: Christian Konig 
---
  drivers/gpu/drm/ttm/ttm_bo_util.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 031e581..7429464 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -91,6 +91,10 @@ static int ttm_resource_ioremap(struct ttm_device
*bdev,
    if (mem->bus.caching == ttm_write_combined)
  addr = ioremap_wc(mem->bus.offset, bus_size);
+#ifdef CONFIG_X86



Please use #if defined (CONFIG_X86)


Actually #ifdef is usually preferred.


oops, i was referring to IS_ENABLED (CONFIG) and not if defined.


Well, that is indeed a good idea for most config options.

But in this case #ifdef alone should work as well.

Christian.






Christian.




+    else if (mem->bus.caching == ttm_cached)
+    addr = ioremap_cache(mem->bus.offset, bus_size);
+#endif
  else
  addr = ioremap(mem->bus.offset, bus_size);
  if (!addr) {
@@ -372,6 +376,11 @@ static int ttm_bo_ioremap(struct
ttm_buffer_object *bo,
  if (mem->bus.caching == ttm_write_combined)
  map->virtual = ioremap_wc(bo->mem.bus.offset + offset,
    size);
+#ifdef CONFIG_X86
+    else if (mem->bus.caching == ttm_cached)
+    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
+  size);
+#endif
  else
  map->virtual = ioremap(bo->mem.bus.offset + offset,
 size);
@@ -490,6 +499,11 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo,
struct dma_buf_map *map)
  else if (mem->bus.caching == ttm_write_combined)
  vaddr_iomem = ioremap_wc(mem->bus.offset,
   bo->base.size);
+    else if (mem->bus.caching == ttm_cached)
+#ifdef CONFIG_X86
+    vaddr_iomem = ioremap_cache(mem->bus.offset,
+  bo->base.size);
+#endif
  else
  vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);

___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Crajneesh.bhardwaj%40amd.com%7Cc4386544ea10487d3a0c08d8df3363a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637504759264793970%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=nM2UtQQdActyapfZSrhfx%2BoJ%2BdszV4Yp62LTehsUWwY%3D&reserved=0 





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

Re: [patch 2/7] drm/vmgfx: Replace kmap_atomic()

[Zack Rusin]

> On Mar 3, 2021, at 08:20, Thomas Gleixner  wrote:
> 
> From: Thomas Gleixner 
> 
> There is no reason to disable pagefaults and preemption as a side effect of
> kmap_atomic_prot().
> 
> Use kmap_local_page_prot() instead and document the reasoning for the
> mapping usage with the given pgprot.
> 
> Remove the NULL pointer check for the map. These functions return a valid
> address for valid pages and the return was bogus anyway as it would have
> left preemption and pagefaults disabled.
> 
> Signed-off-by: Thomas Gleixner 
> Cc: VMware Graphics 
> Cc: Roland Scheidegger 
> Cc: Zack Rusin 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_blit.c |   30 --
> 1 file changed, 12 insertions(+), 18 deletions(-)
> 
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> @@ -375,12 +375,12 @@ static int vmw_bo_cpu_blit_line(struct v
>   copy_size = min_t(u32, copy_size, PAGE_SIZE - src_page_offset);
> 
>   if (unmap_src) {
> - kunmap_atomic(d->src_addr);
> + kunmap_local(d->src_addr);
>   d->src_addr = NULL;
>   }
> 
>   if (unmap_dst) {
> - kunmap_atomic(d->dst_addr);
> + kunmap_local(d->dst_addr);
>   d->dst_addr = NULL;
>   }
> 
> @@ -388,12 +388,8 @@ static int vmw_bo_cpu_blit_line(struct v
>   if (WARN_ON_ONCE(dst_page >= d->dst_num_pages))
>   return -EINVAL;
> 
> - d->dst_addr =
> - kmap_atomic_prot(d->dst_pages[dst_page],
> -  d->dst_prot);
> - if (!d->dst_addr)
> - return -ENOMEM;
> -
> + d->dst_addr = 
> kmap_local_page_prot(d->dst_pages[dst_page],
> +d->dst_prot);
>   d->mapped_dst = dst_page;
>   }
> 
> @@ -401,12 +397,8 @@ static int vmw_bo_cpu_blit_line(struct v
>   if (WARN_ON_ONCE(src_page >= d->src_num_pages))
>   return -EINVAL;
> 
> - d->src_addr =
> - kmap_atomic_prot(d->src_pages[src_page],
> -  d->src_prot);
> - if (!d->src_addr)
> - return -ENOMEM;
> -
> + d->src_addr = 
> kmap_local_page_prot(d->src_pages[src_page],
> +d->src_prot);
>   d->mapped_src = src_page;
>   }
>   diff->do_cpy(diff, d->dst_addr + dst_page_offset,
> @@ -436,8 +428,10 @@ static int vmw_bo_cpu_blit_line(struct v
>  *
>  * Performs a CPU blit from one buffer object to another avoiding a full
>  * bo vmap which may exhaust- or fragment vmalloc space.
> - * On supported architectures (x86), we're using kmap_atomic which avoids
> - * cross-processor TLB- and cache flushes and may, on non-HIGHMEM systems
> + *
> + * On supported architectures (x86), we're using kmap_local_prot() which
> + * avoids cross-processor TLB- and cache flushes. kmap_local_prot() will
> + * either map a highmem page with the proper pgprot on HIGHMEM=y systems or
>  * reference already set-up mappings.
>  *
>  * Neither of the buffer objects may be placed in PCI memory
> @@ -500,9 +494,9 @@ int vmw_bo_cpu_blit(struct ttm_buffer_ob
>   }
> out:
>   if (d.src_addr)
> - kunmap_atomic(d.src_addr);
> + kunmap_local(d.src_addr);
>   if (d.dst_addr)
> - kunmap_atomic(d.dst_addr);
> + kunmap_local(d.dst_addr);
> 
>   return ret;
> }


Looks good. Thanks.

Reviewed-by: Zack Rusin 

z

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

Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

[kernel test robot]

Hi Oak,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip linus/master v5.12-rc1 next-20210304]
[cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next 
drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210305-000626
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/0day-ci/linux/commit/e89ba86e56d95eb097cacfac83b667a92acbf56b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210305-000626
git checkout e89ba86e56d95eb097cacfac83b667a92acbf56b
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_vmap':
>> drivers/gpu/drm/ttm/ttm_bo_util.c:508:3: error: expected expression before 
>> 'else'
 508 |   else
 |   ^~~~


vim +/else +508 drivers/gpu/drm/ttm/ttm_bo_util.c

ba4e7d973dd09b Thomas Hellstrom  2009-06-10  485  
43676605f890b2 Thomas Zimmermann 2020-11-03  486  int ttm_bo_vmap(struct 
ttm_buffer_object *bo, struct dma_buf_map *map)
43676605f890b2 Thomas Zimmermann 2020-11-03  487  {
43676605f890b2 Thomas Zimmermann 2020-11-03  488struct ttm_resource 
*mem = &bo->mem;
43676605f890b2 Thomas Zimmermann 2020-11-03  489int ret;
43676605f890b2 Thomas Zimmermann 2020-11-03  490  
43676605f890b2 Thomas Zimmermann 2020-11-03  491ret = 
ttm_mem_io_reserve(bo->bdev, mem);
43676605f890b2 Thomas Zimmermann 2020-11-03  492if (ret)
43676605f890b2 Thomas Zimmermann 2020-11-03  493return ret;
43676605f890b2 Thomas Zimmermann 2020-11-03  494  
43676605f890b2 Thomas Zimmermann 2020-11-03  495if (mem->bus.is_iomem) {
43676605f890b2 Thomas Zimmermann 2020-11-03  496void __iomem 
*vaddr_iomem;
43676605f890b2 Thomas Zimmermann 2020-11-03  497  
43676605f890b2 Thomas Zimmermann 2020-11-03  498if 
(mem->bus.addr)
43676605f890b2 Thomas Zimmermann 2020-11-03  499
vaddr_iomem = (void __iomem *)mem->bus.addr;
43676605f890b2 Thomas Zimmermann 2020-11-03  500else if 
(mem->bus.caching == ttm_write_combined)
e11bfb99d6ece2 Christian König   2020-12-09  501
vaddr_iomem = ioremap_wc(mem->bus.offset,
e11bfb99d6ece2 Christian König   2020-12-09  502
 bo->base.size);
e89ba86e56d95e Oak Zeng  2021-03-04  503else if 
(mem->bus.caching == ttm_cached)
e89ba86e56d95e Oak Zeng  2021-03-04  504  #ifdef CONFIG_X86
e89ba86e56d95e Oak Zeng  2021-03-04  505
vaddr_iomem = ioremap_cache(mem->bus.offset,
e89ba86e56d95e Oak Zeng  2021-03-04  506
  bo->base.size);
e89ba86e56d95e Oak Zeng  2021-03-04  507  #endif
43676605f890b2 Thomas Zimmermann 2020-11-03 @508else
e11bfb99d6ece2 Christian König   2020-12-09  509
vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);
43676605f890b2 Thomas Zimmermann 2020-11-03  510  
43676605f890b2 Thomas Zimmermann 2020-11-03  511if 
(!vaddr_iomem)
43676605f890b2 Thomas Zimmermann 2020-11-03  512return 
-ENOMEM;
43676605f890b2 Thomas Zimmermann 2020-11-03  513  
43676605f890b2 Thomas Zimmermann 2020-11-03  514
dma_buf_map_set_vaddr_iomem(map, vaddr_iomem);
43676605f890b2 Thomas Zimmermann 2020-11-03  515  
43676605f890b2 Thomas Zimmermann 2020-11-03  516} else {
43676605f890b2 Thomas Zimmermann 2020-11-03  517struct 
ttm_operation_ctx ctx = {
43676605f890b2 Thomas Zimmermann 2020-11-03  518
.interruptible = false,
43676605f890b2 Thomas Zimmermann 2020-11-03  519
.no_wait_gpu = false
43676605f890b2 Thomas Zimmermann 2020-11-03  520};
43676605f890b2 Thomas Zimmermann 2020-11-03  521struct ttm_tt 
*ttm = bo->ttm;

Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

[kernel test robot]

Hi Oak,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip linus/master v5.12-rc1 next-20210304]
[cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next 
drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210305-000626
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: arm64-randconfig-r021-20210304 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 
eec7f8f7b1226be422a76542cb403d02538f453a)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# 
https://github.com/0day-ci/linux/commit/e89ba86e56d95eb097cacfac83b667a92acbf56b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210305-000626
git checkout e89ba86e56d95eb097cacfac83b667a92acbf56b
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/ttm/ttm_bo_util.c:508:3: error: expected expression
   else
   ^
   1 error generated.


vim +508 drivers/gpu/drm/ttm/ttm_bo_util.c

ba4e7d973dd09b66 Thomas Hellstrom  2009-06-10  485  
43676605f890b218 Thomas Zimmermann 2020-11-03  486  int ttm_bo_vmap(struct 
ttm_buffer_object *bo, struct dma_buf_map *map)
43676605f890b218 Thomas Zimmermann 2020-11-03  487  {
43676605f890b218 Thomas Zimmermann 2020-11-03  488  struct ttm_resource 
*mem = &bo->mem;
43676605f890b218 Thomas Zimmermann 2020-11-03  489  int ret;
43676605f890b218 Thomas Zimmermann 2020-11-03  490  
43676605f890b218 Thomas Zimmermann 2020-11-03  491  ret = 
ttm_mem_io_reserve(bo->bdev, mem);
43676605f890b218 Thomas Zimmermann 2020-11-03  492  if (ret)
43676605f890b218 Thomas Zimmermann 2020-11-03  493  return ret;
43676605f890b218 Thomas Zimmermann 2020-11-03  494  
43676605f890b218 Thomas Zimmermann 2020-11-03  495  if (mem->bus.is_iomem) {
43676605f890b218 Thomas Zimmermann 2020-11-03  496  void __iomem 
*vaddr_iomem;
43676605f890b218 Thomas Zimmermann 2020-11-03  497  
43676605f890b218 Thomas Zimmermann 2020-11-03  498  if 
(mem->bus.addr)
43676605f890b218 Thomas Zimmermann 2020-11-03  499  
vaddr_iomem = (void __iomem *)mem->bus.addr;
43676605f890b218 Thomas Zimmermann 2020-11-03  500  else if 
(mem->bus.caching == ttm_write_combined)
e11bfb99d6ece23b Christian König   2020-12-09  501  
vaddr_iomem = ioremap_wc(mem->bus.offset,
e11bfb99d6ece23b Christian König   2020-12-09  502  
 bo->base.size);
e89ba86e56d95eb0 Oak Zeng  2021-03-04  503  else if 
(mem->bus.caching == ttm_cached)
e89ba86e56d95eb0 Oak Zeng  2021-03-04  504  #ifdef CONFIG_X86
e89ba86e56d95eb0 Oak Zeng  2021-03-04  505  
vaddr_iomem = ioremap_cache(mem->bus.offset,
e89ba86e56d95eb0 Oak Zeng  2021-03-04  506  
  bo->base.size);
e89ba86e56d95eb0 Oak Zeng  2021-03-04  507  #endif
43676605f890b218 Thomas Zimmermann 2020-11-03 @508  else
e11bfb99d6ece23b Christian König   2020-12-09  509  
vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);
43676605f890b218 Thomas Zimmermann 2020-11-03  510  
43676605f890b218 Thomas Zimmermann 2020-11-03  511  if 
(!vaddr_iomem)
43676605f890b218 Thomas Zimmermann 2020-11-03  512  return 
-ENOMEM;
43676605f890b218 Thomas Zimmermann 2020-11-03  513  
43676605f890b218 Thomas Zimmermann 2020-11-03  514  
dma_buf_map_set_vaddr_iomem(map, vaddr_iomem);
43676605f890b218 Thomas Zimmermann 2020-11-03  515  
43676605f890b218 Thomas Zimmermann 2020-11-03  516  } else {
43676605f890b218 Thomas Zimmermann 2020-11-03  517  struct 
ttm_operation_ctx ctx = {
43676605f890b218 Thomas Zimmermann 2020-11-03  518  
.interruptible = false,
43676605f890b218 Thomas Zimmermann 2020-11-03  519  
.no_wait_gpu = false
43676605f890b218 Thomas Z

[PATCH] drm/amdgpu: Verify bo size can fit framebuffer size on init.

[Mark Yacoub]

From: Mark Yacoub 

To initialize the framebuffer, use drm_gem_fb_init_with_funcs which
verifies that the BO size can fit the FB size by calculating the minimum
expected size of each plane.

The bug was caught using igt-gpu-tools test: kms_addfb_basic.too-high
and kms_addfb_basic.bo-too-small

Tested on ChromeOS Zork by turning on the display and running a YT
video.

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: Sean Paul 
Signed-off-by: Mark Yacoub 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 66 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h|  8 +++
 3 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 48cb33e5b3826..554038e5bbf6a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -870,18 +870,59 @@ static int amdgpu_display_get_fb_info(const struct 
amdgpu_framebuffer *amdgpu_fb
return r;
 }
 
-int amdgpu_display_framebuffer_init(struct drm_device *dev,
-   struct amdgpu_framebuffer *rfb,
-   const struct drm_mode_fb_cmd2 *mode_cmd,
-   struct drm_gem_object *obj)
+int amdgpu_display_gem_fb_init(struct drm_device *dev,
+  struct amdgpu_framebuffer *rfb,
+  const struct drm_mode_fb_cmd2 *mode_cmd,
+  struct drm_gem_object *obj)
 {
-   int ret, i;
+   int ret;
rfb->base.obj[0] = obj;
drm_helper_mode_fill_fb_struct(dev, &rfb->base, mode_cmd);
ret = drm_framebuffer_init(dev, &rfb->base, &amdgpu_fb_funcs);
if (ret)
-   goto fail;
+   goto err;
+
+   ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj);
+   if (ret)
+   goto err;
+
+   return 0;
+err:
+   drm_err(dev, "Failed to init gem fb: %d\n", ret);
+   rfb->base.obj[0] = NULL;
+   return ret;
+}
+
+int amdgpu_display_gem_fb_verify_and_init(
+   struct drm_device *dev, struct amdgpu_framebuffer *rfb,
+   struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
+   struct drm_gem_object *obj)
+{
+   int ret;
+   rfb->base.obj[0] = obj;
+   // Verify that bo size can fit the fb size.
+   ret = drm_gem_fb_init_with_funcs(dev, &rfb->base, file_priv, mode_cmd,
+&amdgpu_fb_funcs);
+   if (ret)
+   goto err;
 
+   ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj);
+   if (ret)
+   goto err;
+
+   return 0;
+err:
+   drm_err(dev, "Failed to verify and init gem fb: %d\n", ret);
+   rfb->base.obj[0] = NULL;
+   return ret;
+}
+
+int amdgpu_display_framebuffer_init(struct drm_device *dev,
+   struct amdgpu_framebuffer *rfb,
+   const struct drm_mode_fb_cmd2 *mode_cmd,
+   struct drm_gem_object *obj)
+{
+   int ret, i;
/*
 * This needs to happen before modifier conversion as that might change
 * the number of planes.
@@ -891,13 +932,13 @@ int amdgpu_display_framebuffer_init(struct drm_device 
*dev,
drm_dbg_kms(dev, "Plane 0 and %d have different BOs: %u 
vs. %u\n",
i, mode_cmd->handles[0], 
mode_cmd->handles[i]);
ret = -EINVAL;
-   goto fail;
+   return ret;
}
}
 
ret = amdgpu_display_get_fb_info(rfb, &rfb->tiling_flags, 
&rfb->tmz_surface);
if (ret)
-   goto fail;
+   return ret;
 
if (dev->mode_config.allow_fb_modifiers &&
!(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
@@ -905,7 +946,7 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
if (ret) {
drm_dbg_kms(dev, "Failed to convert tiling flags 0x%llX 
to a modifier",
rfb->tiling_flags);
-   goto fail;
+   return ret;
}
}
 
@@ -915,10 +956,6 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
}
 
return 0;
-
-fail:
-   rfb->base.obj[0] = NULL;
-   return ret;
 }
 
 struct drm_framebuffer *
@@ -953,7 +990,8 @@ amdgpu_display_user_framebuffer_create(struct drm_device 
*dev,
return ERR_PTR(-ENOMEM);
}
 
-   ret = amdgpu_display_framebuffer_init(dev, amdgpu_fb, mode_cmd, obj);
+   ret = amdgpu_display_gem_fb_verify_and_init(dev, amdgpu_fb, file_priv,
+   mode_cmd, obj);
if (ret) {
kfree(amdgpu_fb);
drm_gem_object_pu

[PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

[Oak Zeng]

If tbo.mem.bus.caching is cached, buffer is intended to be mapped
as cached from CPU. Map it with ioremap_cache.

This wasn't necessary before as device memory was never mapped
as cached from CPU side. It becomes necessary for aldebaran as
device memory is mapped cached from CPU.

Signed-off-by: Oak Zeng 
Reviewed-by: Christian Konig 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 031e581..296bb20 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -91,6 +91,10 @@ static int ttm_resource_ioremap(struct ttm_device *bdev,
 
if (mem->bus.caching == ttm_write_combined)
addr = ioremap_wc(mem->bus.offset, bus_size);
+#ifdef CONFIG_X86
+   else if (mem->bus.caching == ttm_cached)
+   addr = ioremap_cache(mem->bus.offset, bus_size);
+#endif
else
addr = ioremap(mem->bus.offset, bus_size);
if (!addr) {
@@ -372,6 +376,11 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
if (mem->bus.caching == ttm_write_combined)
map->virtual = ioremap_wc(bo->mem.bus.offset + offset,
  size);
+#ifdef CONFIG_X86
+   else if (mem->bus.caching == ttm_cached)
+   map->virtual = ioremap_cache(bo->mem.bus.offset + 
offset,
+ size);
+#endif
else
map->virtual = ioremap(bo->mem.bus.offset + offset,
   size);
@@ -490,6 +499,11 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo, struct 
dma_buf_map *map)
else if (mem->bus.caching == ttm_write_combined)
vaddr_iomem = ioremap_wc(mem->bus.offset,
 bo->base.size);
+#ifdef CONFIG_X86
+   else if (mem->bus.caching == ttm_cached)
+   vaddr_iomem = ioremap_cache(mem->bus.offset,
+ bo->base.size);
+#endif
else
vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);
 
-- 
2.7.4

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

Re: [PATCH v13 1/2] drm/tegra: dc: Support memory bandwidth management

[Dmitry Osipenko]

04.03.2021 02:08, Michał Mirosław пишет:
> On Tue, Mar 02, 2021 at 03:44:44PM +0300, Dmitry Osipenko wrote:
>> Display controller (DC) performs isochronous memory transfers, and thus,
>> has a requirement for a minimum memory bandwidth that shall be fulfilled,
>> otherwise framebuffer data can't be fetched fast enough and this results
>> in a DC's data-FIFO underflow that follows by a visual corruption.
>>
>> The Memory Controller drivers provide facility for memory bandwidth
>> management via interconnect API. Let's wire up the interconnect API
>> support to the DC driver in order to fix the distorted display output
>> on T30 Ouya, T124 TK1 and other Tegra devices.
> 
> I did a read on the code. I have put some thoughts and nits inbetween the
> diff, but here are more general questions about the patch:

Hello, Michał! Thank you very much for taking a look at the patch!

> Is there a reason why the bandwidth is allocated per plane instead of just
> using one peak and average for the whole configuration? Or is there a need
> to expose all the planes as interconnect channels and allocate their
> bandwidth individually?

Each display plane has individual memory client on Tegra, memory ICC
paths are specified per memory client. This is how memory ICCs are
defined in the DT binding and that's what memory drivers are expecting
to deal with. It is also nice to see in ICC debugfs how much memory
bandwidth is consumed by each individual memory client.

> From algorithmic part I see that the plane overlaps are calculated twice
> (A vs B and later B vs A). The cursor plane is ignored, but nevertheless
> its overlap mask is calculated before being thrown away.

The algorithm assumes that we have a fixed number of planes to care
about and it's not executed in a hot code path, hence it's more optimal
to have a simpler and smaller code rather than try to optimize it
without gaining any benefits, IMO.

> The bandwidths
> are also calculated twice: once for pre-commit and then again for
> post-commit.  Is setting bandwidth for an interconnect expensive when
> re-setting a value that was already set? The code seems to avoid this
> case, but maybe unnecessarily?

The CCF discards dummy rate-changes in the end of ICC code path.
Besides, we're not performing it in a hot code paths, hence performance
isn't a concern in this patch.

The tegra_crtc_update_memory_bandwidth() relies on being called before
and after the atomic commit. For example CRTC's "active" state is
updated only after the completion of commit-tail phase.

Earlier versions of this patch had checks that tried to avoid setting
bandwidth in both 'begin' and 'end' phases of a commit, but then I found
that it was buggy in regards to DPMS handling and it was much more
optimal to remove the unnecessary "optimizations" since code was blowing
up in complexity when I tried to fix it.

The tegra_crtc_update_memory_bandwidth() still checks whether old BW =
new BW, hence in practice actual ICC changes are only performed when
plane is turned on/off.

> [...cut the big and interesting part...]
> 
> [...]
>> @@ -65,7 +75,9 @@ struct tegra_dc_soc_info {
>>  unsigned int num_overlay_formats;
>>  const u64 *modifiers;
>>  bool has_win_a_without_filters;
>> +bool has_win_b_vfilter_mem_client;
>>  bool has_win_c_without_vert_filter;
>> +unsigned int plane_tiled_memory_bandwidth_x2;
> 
> This looks more like bool in the code using it.

Good catch, thank you!

> [...]
>> --- a/drivers/gpu/drm/tegra/plane.c
>> +++ b/drivers/gpu/drm/tegra/plane.c
> [...]
>> +static int tegra_plane_check_memory_bandwidth(struct drm_plane_state *state)
> 
> The function's body looks more like 'update' or 'recalculate' rather
> than 'check' the memory bandwidth.

The 'check' in the name is intended to show that function belongs to
atomic-state checking.

But tegra_plane_calculate_memory_bandwidth_state() also is a good
variant. I'll consider the renaming, thanks!

>> +struct tegra_plane_state *tegra_state = to_tegra_plane_state(state);
>> +unsigned int i, bpp, bpp_plane, dst_w, src_w, src_h, mul;
>> +const struct tegra_dc_soc_info *soc;
>> +const struct drm_format_info *fmt;
>> +struct drm_crtc_state *crtc_state;
>> +u32 avg_bandwidth, peak_bandwidth;
>> +
>> +if (!state->visible)
>> +return 0;
>> +
>> +crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
>> +if (!crtc_state)
>> +return -EINVAL;
>> +
>> +src_w = drm_rect_width(&state->src) >> 16;
>> +src_h = drm_rect_height(&state->src) >> 16;
>> +dst_w = drm_rect_width(&state->dst);
>> +
>> +fmt = state->fb->format;
>> +soc = to_tegra_dc(state->crtc)->soc;
>> +
>> +/*
>> + * Note that real memory bandwidth vary depending on format and
>> + * memory layout, we are not taking that into account because small
>> + * estimation error isn't important since bandwidth is rounded up
>> + * anyway.
>> + */
>> +for (i 

[PATCH] drm/uapi: document kernel capabilities

[Simon Ser]

Document all of the DRM_CAP_* defines.

Signed-off-by: Simon Ser 
Cc: Daniel Vetter 
Cc: Pekka Paalanen 
---
 include/uapi/drm/drm.h | 100 +++--
 1 file changed, 96 insertions(+), 4 deletions(-)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 0827037c5484..4ef07f505725 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -625,30 +625,122 @@ struct drm_gem_open {
__u64 size;
 };
 
+/**
+ * DRM_CAP_DUMB_BUFFER
+ *
+ * If set to 1, the driver supports creating dumb buffers via the
+ * &DRM_IOCTL_MODE_CREATE_DUMB ioctl.
+ */
 #define DRM_CAP_DUMB_BUFFER0x1
+/**
+ * DRM_CAP_VBLANK_HIGH_CRTC
+ *
+ * If set to 1, the kernel supports specifying a CRTC index in the high bits of
+ * &drm_wait_vblank_request.type.
+ */
 #define DRM_CAP_VBLANK_HIGH_CRTC   0x2
+/**
+ * DRM_CAP_DUMB_PREFERRED_DEPTH
+ *
+ * The preferred depth (in bits) for dumb buffers.
+ */
 #define DRM_CAP_DUMB_PREFERRED_DEPTH   0x3
+/**
+ * DRM_CAP_DUMB_PREFER_SHADOW
+ *
+ * If set to 1, the driver prefers userspace to render to a shadow buffer
+ * instead of directly rendering to a dumb buffer.
+ */
 #define DRM_CAP_DUMB_PREFER_SHADOW 0x4
+/**
+ * DRM_CAP_PRIME
+ *
+ * Bitfield of supported PRIME sharing capabilities. See &DRM_PRIME_CAP_IMPORT
+ * and &DRM_PRIME_CAP_EXPORT.
+ */
 #define DRM_CAP_PRIME  0x5
+/**
+ * DRM_PRIME_CAP_IMPORT
+ *
+ * If this bit is set in &DRM_CAP_PRIME, the driver supports importing PRIME
+ * buffers.
+ */
 #define  DRM_PRIME_CAP_IMPORT  0x1
+/**
+ * DRM_PRIME_CAP_EXPORT
+ *
+ * If this bit is set in &DRM_CAP_PRIME, the driver supports exporting PRIME
+ * buffers.
+ */
 #define  DRM_PRIME_CAP_EXPORT  0x2
+/**
+ * DRM_CAP_TIMESTAMP_MONOTONIC
+ *
+ * If set to 0, the kernel will report timestamps with the realtime clock in
+ * struct drm_event_vblank. If set to 1, the kernel will report timestamps with
+ * the monotonic clock.
+ */
 #define DRM_CAP_TIMESTAMP_MONOTONIC0x6
+/**
+ * DRM_CAP_ASYNC_PAGE_FLIP
+ *
+ * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC.
+ */
 #define DRM_CAP_ASYNC_PAGE_FLIP0x7
-/*
- * The CURSOR_WIDTH and CURSOR_HEIGHT capabilities return a valid widthxheight
- * combination for the hardware cursor. The intention is that a hardware
- * agnostic userspace can query a cursor plane size to use.
+/**
+ * DRM_CAP_CURSOR_WIDTH
+ *
+ * The ``CURSOR_WIDTH`` and ``CURSOR_HEIGHT`` capabilities return a valid
+ * width x height combination for the hardware cursor. The intention is that a
+ * hardware agnostic userspace can query a cursor plane size to use.
  *
  * Note that the cross-driver contract is to merely return a valid size;
  * drivers are free to attach another meaning on top, eg. i915 returns the
  * maximum plane size.
  */
 #define DRM_CAP_CURSOR_WIDTH   0x8
+/**
+ * DRM_CAP_CURSOR_HEIGHT
+ *
+ * See &DRM_CAP_CURSOR_WIDTH.
+ */
 #define DRM_CAP_CURSOR_HEIGHT  0x9
+/**
+ * DRM_CAP_ADDFB2_MODIFIERS
+ *
+ * If set to 1, the driver supports supplying modifiers in the
+ * &DRM_IOCTL_MODE_ADDFB2 ioctl.
+ */
 #define DRM_CAP_ADDFB2_MODIFIERS   0x10
+/**
+ * DRM_CAP_PAGE_FLIP_TARGET
+ *
+ * If set to 1, the driver supports the &DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE and
+ * &DRM_MODE_PAGE_FLIP_TARGET_RELATIVE flags in
+ * &drm_mode_crtc_page_flip_target.flags for the &DRM_IOCTL_MODE_PAGE_FLIP
+ * ioctl.
+ */
 #define DRM_CAP_PAGE_FLIP_TARGET   0x11
+/**
+ * DRM_CAP_CRTC_IN_VBLANK_EVENT
+ *
+ * If set to 1, the kernel supports reporting the CRTC ID in
+ * &drm_event_vblank.crtc_id.
+ */
 #define DRM_CAP_CRTC_IN_VBLANK_EVENT   0x12
+/**
+ * DRM_CAP_SYNCOBJ
+ *
+ * If set to 1, the driver supports sync objects. See
+ * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects".
+ */
 #define DRM_CAP_SYNCOBJ0x13
+/**
+ * DRM_CAP_SYNCOBJ_TIMELINE
+ *
+ * If set to 1, the driver supports timeline operations on sync objects. See
+ * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects".
+ */
 #define DRM_CAP_SYNCOBJ_TIMELINE   0x14
 
 /* DRM_IOCTL_GET_CAP ioctl argument type */
-- 
2.30.1

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

Re: [Freedreno] [PATCH 16/17] iommu: remove DOMAIN_ATTR_IO_PGTABLE_CFG

[Rob Clark]

On Thu, Mar 4, 2021 at 7:48 AM Robin Murphy  wrote:
>
> On 2021-03-01 08:42, Christoph Hellwig wrote:
> > Signed-off-by: Christoph Hellwig 
>
> Moreso than the previous patch, where the feature is at least relatively
> generic (note that there's a bunch of in-flight development around
> DOMAIN_ATTR_NESTING), I'm really not convinced that it's beneficial to
> bloat the generic iommu_ops structure with private driver-specific
> interfaces. The attribute interface is a great compromise for these
> kinds of things, and you can easily add type-checked wrappers around it
> for external callers (maybe even make the actual attributes internal
> between the IOMMU core and drivers) if that's your concern.

I suppose if this is *just* for the GPU we could move it into adreno_smmu_priv..

But one thing I'm not sure about is whether
IO_PGTABLE_QUIRK_ARM_OUTER_WBWA is something that other devices
*should* be using as well, but just haven't gotten around to yet.

BR,
-R

> Robin.
>
> > ---
> >   drivers/gpu/drm/msm/adreno/adreno_gpu.c |  2 +-
> >   drivers/iommu/arm/arm-smmu/arm-smmu.c   | 40 +++--
> >   drivers/iommu/iommu.c   |  9 ++
> >   include/linux/iommu.h   |  9 +-
> >   4 files changed, 29 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index 0f184c3dd9d9ec..78d98ab2ee3a68 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -191,7 +191,7 @@ void adreno_set_llc_attributes(struct iommu_domain 
> > *iommu)
> >   struct io_pgtable_domain_attr pgtbl_cfg;
> >
> >   pgtbl_cfg.quirks = IO_PGTABLE_QUIRK_ARM_OUTER_WBWA;
> > - iommu_domain_set_attr(iommu, DOMAIN_ATTR_IO_PGTABLE_CFG, &pgtbl_cfg);
> > + iommu_domain_set_pgtable_attr(iommu, &pgtbl_cfg);
> >   }
> >
> >   struct msm_gem_address_space *
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index 2e17d990d04481..2858999c86dfd1 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -1515,40 +1515,22 @@ static int arm_smmu_domain_enable_nesting(struct 
> > iommu_domain *domain)
> >   return ret;
> >   }
> >
> > -static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
> > - enum iommu_attr attr, void *data)
> > +static int arm_smmu_domain_set_pgtable_attr(struct iommu_domain *domain,
> > + struct io_pgtable_domain_attr *pgtbl_cfg)
> >   {
> > - int ret = 0;
> >   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> > + int ret = -EPERM;
> >
> > - mutex_lock(&smmu_domain->init_mutex);
> > -
> > - switch(domain->type) {
> > - case IOMMU_DOMAIN_UNMANAGED:
> > - switch (attr) {
> > - case DOMAIN_ATTR_IO_PGTABLE_CFG: {
> > - struct io_pgtable_domain_attr *pgtbl_cfg = data;
> > -
> > - if (smmu_domain->smmu) {
> > - ret = -EPERM;
> > - goto out_unlock;
> > - }
> > + if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> > + return -EINVAL;
> >
> > - smmu_domain->pgtbl_cfg = *pgtbl_cfg;
> > - break;
> > - }
> > - default:
> > - ret = -ENODEV;
> > - }
> > - break;
> > - case IOMMU_DOMAIN_DMA:
> > - ret = -ENODEV;
> > - break;
> > - default:
> > - ret = -EINVAL;
> > + mutex_lock(&smmu_domain->init_mutex);
> > + if (!smmu_domain->smmu) {
> > + smmu_domain->pgtbl_cfg = *pgtbl_cfg;
> > + ret = 0;
> >   }
> > -out_unlock:
> >   mutex_unlock(&smmu_domain->init_mutex);
> > +
> >   return ret;
> >   }
> >
> > @@ -1609,7 +1591,7 @@ static struct iommu_ops arm_smmu_ops = {
> >   .device_group   = arm_smmu_device_group,
> >   .dma_use_flush_queue= arm_smmu_dma_use_flush_queue,
> >   .dma_enable_flush_queue = arm_smmu_dma_enable_flush_queue,
> > - .domain_set_attr= arm_smmu_domain_set_attr,
> > + .domain_set_pgtable_attr = arm_smmu_domain_set_pgtable_attr,
> >   .domain_enable_nesting  = arm_smmu_domain_enable_nesting,
> >   .of_xlate   = arm_smmu_of_xlate,
> >   .get_resv_regions   = arm_smmu_get_resv_regions,
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 2e9e058501a953..8490aefd4b41f8 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -2693,6 +2693,15 @@ int iommu_domain_enable_nesting(struct iommu_domain 
> > *domain)
> >   }
> >   EXPORT_SYMBOL_GPL(iommu_domain_enable_nesting);
> >
> > +int iommu_domain_set_pgtable_attr(struct iommu_domain *domain,
> > + struct io_pgtable_domain_attr *pgtbl_

[PATCH v8 0/5] Generic page pool & deferred freeing for system dmabuf heap

[John Stultz]

Apologies for letting so much time pass since the last revision!

The point of this series is trying to add both deferred-freeing
logic as well as a page pool to the DMA-BUF system heap to improve
allocation performance.

This is desired, as the combination of deferred freeing along
with the page pool allows us to offload page-zeroing out of
the allocation hot path. This was done originally with ION
and this patch series allows the DMA-BUF system heap to match
ION's system heap allocation performance in a simple
microbenchmark [1] (ION re-added to the kernel for comparision,
running on an x86 vm image):

./dmabuf-heap-bench -i 0 1 system
Testing dmabuf system vs ion heaptype 0 (flags: 0x1)
-
dmabuf heap: alloc 4096 bytes 5000 times in 88092722 ns  17618 ns/call
ion heap:alloc 4096 bytes 5000 times in 103043547 ns 20608 ns/call
dmabuf heap: alloc 1048576 bytes 5000 times in 252416639 ns  50483 ns/call
ion heap:alloc 1048576 bytes 5000 times in 358190744 ns  71638 ns/call
dmabuf heap: alloc 8388608 bytes 5000 times in 2854351310 ns 570870 ns/call
ion heap:alloc 8388608 bytes 5000 times in 3676328905 ns 735265 ns/call
dmabuf heap: alloc 33554432 bytes 5000 times in 13208119197 ns   2641623 ns/call
ion heap:alloc 33554432 bytes 5000 times in 15306975287 ns   3061395 ns/call


Daniel didn't like earlier attempts to re-use the network
page-pool code to achieve this, and suggested the ttm_pool be
used instead, so this series pulls the page pool functionality
out of the ttm_pool logic and creates a generic page pool
that can be shared.

New in v7 (never submitted):
* Reworked how I integrated the page pool with the ttm logic
  to use container of to avoid allocating structures per page. 

New in v8:
* Due to the dual license requirement from Christian König
  I completely threw away the earlier shared page pool
  implementation (which had evolved from ion code), and
  rewrote it using just the ttm_pool logic. My apologies
  for any previously reviewed issues that I've reintroduced
  in doing so.

Input would be greatly appreciated. Testing as well, as I don't
have any development hardware that utilizes the ttm pool.

thanks
-john

[1] 
https://android.googlesource.com/platform/system/memory/libdmabufheap/+/refs/heads/master/tests/dmabuf_heap_bench.c

Cc: Daniel Vetter 
Cc: Christian Koenig 
Cc: Sumit Semwal 
Cc: Liam Mark 
Cc: Chris Goldsworthy 
Cc: Laura Abbott 
Cc: Brian Starkey 
Cc: Hridya Valsaraju 
Cc: Suren Baghdasaryan 
Cc: Sandeep Patil 
Cc: Daniel Mentz 
Cc: Ørjan Eide 
Cc: Robin Murphy 
Cc: Ezequiel Garcia 
Cc: Simon Ser 
Cc: James Jones 
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org

John Stultz (5):
  drm: Add a sharable drm page-pool implementation
  drm: ttm_pool: Rework ttm_pool to use drm_page_pool
  dma-buf: heaps: Add deferred-free-helper library code
  dma-buf: system_heap: Add drm pagepool support to system heap
  dma-buf: system_heap: Add deferred freeing to the system heap

 drivers/dma-buf/heaps/Kconfig|   5 +
 drivers/dma-buf/heaps/Makefile   |   1 +
 drivers/dma-buf/heaps/deferred-free-helper.c | 138 
 drivers/dma-buf/heaps/deferred-free-helper.h |  55 +
 drivers/dma-buf/heaps/system_heap.c  |  47 +++-
 drivers/gpu/drm/Kconfig  |   5 +
 drivers/gpu/drm/Makefile |   2 +
 drivers/gpu/drm/page_pool.c  | 214 +++
 drivers/gpu/drm/ttm/ttm_pool.c   | 156 +++---
 include/drm/page_pool.h  |  65 ++
 include/drm/ttm/ttm_pool.h   |   6 +-
 11 files changed, 557 insertions(+), 137 deletions(-)
 create mode 100644 drivers/dma-buf/heaps/deferred-free-helper.c
 create mode 100644 drivers/dma-buf/heaps/deferred-free-helper.h
 create mode 100644 drivers/gpu/drm/page_pool.c
 create mode 100644 include/drm/page_pool.h

-- 
2.25.1

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

[PATCH v8 1/5] drm: Add a sharable drm page-pool implementation

[John Stultz]

This adds a shrinker controlled page pool, extracted
out of the ttm_pool logic, and abstracted out a bit
so it can be used by other non-ttm drivers.

Cc: Daniel Vetter 
Cc: Christian Koenig 
Cc: Sumit Semwal 
Cc: Liam Mark 
Cc: Chris Goldsworthy 
Cc: Laura Abbott 
Cc: Brian Starkey 
Cc: Hridya Valsaraju 
Cc: Suren Baghdasaryan 
Cc: Sandeep Patil 
Cc: Daniel Mentz 
Cc: Ørjan Eide 
Cc: Robin Murphy 
Cc: Ezequiel Garcia 
Cc: Simon Ser 
Cc: James Jones 
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz 
---
v8:
* Completely rewritten from scratch, using only the
  ttm_pool logic so it can be dual licensed.
---
 drivers/gpu/drm/Kconfig |   4 +
 drivers/gpu/drm/Makefile|   2 +
 drivers/gpu/drm/page_pool.c | 214 
 include/drm/page_pool.h |  65 +++
 4 files changed, 285 insertions(+)
 create mode 100644 drivers/gpu/drm/page_pool.c
 create mode 100644 include/drm/page_pool.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index e392a90ca687..7cbcecb8f7df 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -177,6 +177,10 @@ config DRM_DP_CEC
  Note: not all adapters support this feature, and even for those
  that do support this they often do not hook up the CEC pin.
 
+config DRM_PAGE_POOL
+   bool
+   depends on DRM
+
 config DRM_TTM
tristate
depends on DRM && MMU
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 926adef289db..2dc7b2fe3fe5 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -39,6 +39,8 @@ obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
 drm_ttm_helper-y := drm_gem_ttm_helper.o
 obj-$(CONFIG_DRM_TTM_HELPER) += drm_ttm_helper.o
 
+drm-$(CONFIG_DRM_PAGE_POOL) += page_pool.o
+
 drm_kms_helper-y := drm_bridge_connector.o drm_crtc_helper.o drm_dp_helper.o \
drm_dsc.o drm_probe_helper.o \
drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
diff --git a/drivers/gpu/drm/page_pool.c b/drivers/gpu/drm/page_pool.c
new file mode 100644
index ..a60b954cfe0f
--- /dev/null
+++ b/drivers/gpu/drm/page_pool.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Sharable page pool implementation
+ *
+ * Extracted from drivers/gpu/drm/ttm/ttm_pool.c
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ * Copyright 2021 Linaro Ltd.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors: Christian König, John Stultz
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+static unsigned long page_pool_size;
+
+MODULE_PARM_DESC(page_pool_size, "Number of pages in the WC/UC/DMA pool");
+module_param(page_pool_size, ulong, 0644);
+
+static atomic_long_t allocated_pages;
+
+static struct mutex shrinker_lock;
+static struct list_head shrinker_list;
+static struct shrinker mm_shrinker;
+
+void drm_page_pool_set_max(unsigned long max)
+{
+   if (!page_pool_size)
+   page_pool_size = max;
+}
+
+unsigned long drm_page_pool_get_max(void)
+{
+   return page_pool_size;
+}
+
+unsigned long drm_page_pool_get_total(void)
+{
+   return atomic_long_read(&allocated_pages);
+}
+
+unsigned long drm_page_pool_get_size(struct drm_page_pool *pool)
+{
+   unsigned long size;
+
+   spin_lock(&pool->lock);
+   size = pool->page_count;
+   spin_unlock(&pool->lock);
+   return size;
+}
+
+/* Give pages into a specific pool */
+void drm_page_pool_add(struct drm_page_pool *pool, struct page *p)
+{
+   unsigned int i, num_pages = 1 << pool->order;
+
+   for (i = 0; i < num_pages; ++i) {
+   if (PageHighMem(p))
+   clear_highpage(p + i);
+   else
+   clear_page(page_address(p + i));
+   }
+
+   spin_lock(&pool->lock);
+   list_add(&p->lru, &pool->pages);
+   pool->page_count += 1 << pool->order;
+   

[PATCH v8 2/5] drm: ttm_pool: Rework ttm_pool to use drm_page_pool

[John Stultz]

This patch reworks the ttm_pool logic to utilize the recently
added drm_page_pool code.

This adds drm_page_pool structures to the ttm_pool_type
structures, and then removes all the ttm_pool_type shrinker
logic (as its handled in the drm_page_pool shrinker).

NOTE: There is one mismatch in the interfaces I'm not totally
happy with. The ttm_pool tracks all of its pooled pages across
a number of different pools, and tries to keep this size under
the specified page_pool_size value. With the drm_page_pool,
there may other users, however there is still one global
shrinker list of pools. So we can't easily reduce the ttm
pool under the ttm specified size without potentially doing
a lot of shrinking to other non-ttm pools. So either we can:
  1) Try to split it so each user of drm_page_pools manages its
 own pool shrinking.
  2) Push the max value into the drm_page_pool, and have it
 manage shrinking to fit under that global max. Then share
 those size/max values out so the ttm_pool debug output
 can have more context.

I've taken the second path in this patch set, but wanted to call
it out so folks could look closely.

Thoughts would be greatly appreciated here!

Cc: Daniel Vetter 
Cc: Christian Koenig 
Cc: Sumit Semwal 
Cc: Liam Mark 
Cc: Chris Goldsworthy 
Cc: Laura Abbott 
Cc: Brian Starkey 
Cc: Hridya Valsaraju 
Cc: Suren Baghdasaryan 
Cc: Sandeep Patil 
Cc: Daniel Mentz 
Cc: Ørjan Eide 
Cc: Robin Murphy 
Cc: Ezequiel Garcia 
Cc: Simon Ser 
Cc: James Jones 
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz 
---
v7:
* Major refactoring to use drm_page_pools inside the
  ttm_pool_type structure. This allows us to use container_of to
  get the needed context to free a page. This also means less
  code is changed overall.
v8:
* Reworked to use the new cleanly rewritten drm_page_pool logic
---
 drivers/gpu/drm/Kconfig|   1 +
 drivers/gpu/drm/ttm/ttm_pool.c | 156 ++---
 include/drm/ttm/ttm_pool.h |   6 +-
 3 files changed, 31 insertions(+), 132 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 7cbcecb8f7df..a6cbdb63f6c7 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -184,6 +184,7 @@ config DRM_PAGE_POOL
 config DRM_TTM
tristate
depends on DRM && MMU
+   select DRM_PAGE_POOL
help
  GPU memory management subsystem for devices with multiple
  GPU memory types. Will be enabled automatically if a device driver
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 6e27cb1bf48b..f74ea801d7ab 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -39,6 +39,7 @@
 #include 
 #endif
 
+#include 
 #include 
 #include 
 #include 
@@ -68,8 +69,6 @@ static struct ttm_pool_type 
global_dma32_write_combined[MAX_ORDER];
 static struct ttm_pool_type global_dma32_uncached[MAX_ORDER];
 
 static struct mutex shrinker_lock;
-static struct list_head shrinker_list;
-static struct shrinker mm_shrinker;
 
 /* Allocate pages of size 1 << order with the given gfp_flags */
 static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags,
@@ -125,8 +124,9 @@ static struct page *ttm_pool_alloc_page(struct ttm_pool 
*pool, gfp_t gfp_flags,
 }
 
 /* Reset the caching and pages of size 1 << order */
-static void ttm_pool_free_page(struct ttm_pool *pool, enum ttm_caching caching,
-  unsigned int order, struct page *p)
+static unsigned long ttm_pool_free_page(struct ttm_pool *pool,
+   enum ttm_caching caching,
+   unsigned int order, struct page *p)
 {
unsigned long attr = DMA_ATTR_FORCE_CONTIGUOUS;
struct ttm_pool_dma *dma;
@@ -142,7 +142,7 @@ static void ttm_pool_free_page(struct ttm_pool *pool, enum 
ttm_caching caching,
 
if (!pool || !pool->use_dma_alloc) {
__free_pages(p, order);
-   return;
+   return 1UL << order;
}
 
if (order)
@@ -153,6 +153,16 @@ static void ttm_pool_free_page(struct ttm_pool *pool, enum 
ttm_caching caching,
dma_free_attrs(pool->dev, (1UL << order) * PAGE_SIZE, vaddr, dma->addr,
   attr);
kfree(dma);
+   return 1UL << order;
+}
+
+static unsigned long ttm_subpool_free_page(struct drm_page_pool *subpool,
+  struct page *p)
+{
+   struct ttm_pool_type *pt;
+
+   pt = container_of(subpool, struct ttm_pool_type, subpool);
+   return ttm_pool_free_page(pt->pool, pt->caching, pt->order, p);
 }
 
 /* Apply a new caching to an array of pages */
@@ -216,40 +226,6 @@ static void ttm_pool_unmap(struct ttm_pool *pool, 
dma_addr_t dma_addr,
   DMA_BIDIRECTIONAL);
 }
 
-/* Give pages into a specific pool_type */
-static void ttm_pool_type_give(struct ttm_pool_type *pt, struct pag

[PATCH v8 3/5] dma-buf: heaps: Add deferred-free-helper library code

[John Stultz]

This patch provides infrastructure for deferring buffer frees.

This is a feature ION provided which when used with some form
of a page pool, provides a nice performance boost in an
allocation microbenchmark. The reason it helps is it allows the
page-zeroing to be done out of the normal allocation/free path,
and pushed off to a kthread.

As not all heaps will find this useful, its implemented as
a optional helper library that heaps can utilize.

Cc: Daniel Vetter 
Cc: Christian Koenig 
Cc: Sumit Semwal 
Cc: Liam Mark 
Cc: Chris Goldsworthy 
Cc: Laura Abbott 
Cc: Brian Starkey 
Cc: Hridya Valsaraju 
Cc: Suren Baghdasaryan 
Cc: Sandeep Patil 
Cc: Daniel Mentz 
Cc: Ørjan Eide 
Cc: Robin Murphy 
Cc: Ezequiel Garcia 
Cc: Simon Ser 
Cc: James Jones 
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz 
---
v2:
* Fix sleep in atomic issue from using a mutex, by switching
  to a spinlock as Reported-by: kernel test robot 
* Cleanup API to use a reason enum for clarity and add some documentation
  comments as suggested by Suren Baghdasaryan.
v3:
* Minor tweaks so it can be built as a module
* A few small fixups suggested by Daniel Mentz
v4:
* Tweak from Daniel Mentz to make sure the shrinker
  count/freed values are tracked in pages not bytes
v5:
* Fix up page count tracking as suggested by Suren Baghdasaryan
v7:
* Rework accounting to use nr_pages rather then size, as suggested
  by Suren Baghdasaryan
---
 drivers/dma-buf/heaps/Kconfig|   3 +
 drivers/dma-buf/heaps/Makefile   |   1 +
 drivers/dma-buf/heaps/deferred-free-helper.c | 138 +++
 drivers/dma-buf/heaps/deferred-free-helper.h |  55 
 4 files changed, 197 insertions(+)
 create mode 100644 drivers/dma-buf/heaps/deferred-free-helper.c
 create mode 100644 drivers/dma-buf/heaps/deferred-free-helper.h

diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index a5eef06c4226..f7aef8bc7119 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -1,3 +1,6 @@
+config DMABUF_HEAPS_DEFERRED_FREE
+   tristate
+
 config DMABUF_HEAPS_SYSTEM
bool "DMA-BUF System Heap"
depends on DMABUF_HEAPS
diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
index 974467791032..4e7839875615 100644
--- a/drivers/dma-buf/heaps/Makefile
+++ b/drivers/dma-buf/heaps/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_DMABUF_HEAPS_DEFERRED_FREE) += deferred-free-helper.o
 obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)  += system_heap.o
 obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
diff --git a/drivers/dma-buf/heaps/deferred-free-helper.c 
b/drivers/dma-buf/heaps/deferred-free-helper.c
new file mode 100644
index ..e19c8b68dfeb
--- /dev/null
+++ b/drivers/dma-buf/heaps/deferred-free-helper.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Deferred dmabuf freeing helper
+ *
+ * Copyright (C) 2020 Linaro, Ltd.
+ *
+ * Based on the ION page pool code
+ * Copyright (C) 2011 Google, Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "deferred-free-helper.h"
+
+static LIST_HEAD(free_list);
+static size_t list_nr_pages;
+wait_queue_head_t freelist_waitqueue;
+struct task_struct *freelist_task;
+static DEFINE_SPINLOCK(free_list_lock);
+
+void deferred_free(struct deferred_freelist_item *item,
+  void (*free)(struct deferred_freelist_item*,
+   enum df_reason),
+  size_t nr_pages)
+{
+   unsigned long flags;
+
+   INIT_LIST_HEAD(&item->list);
+   item->nr_pages = nr_pages;
+   item->free = free;
+
+   spin_lock_irqsave(&free_list_lock, flags);
+   list_add(&item->list, &free_list);
+   list_nr_pages += nr_pages;
+   spin_unlock_irqrestore(&free_list_lock, flags);
+   wake_up(&freelist_waitqueue);
+}
+EXPORT_SYMBOL_GPL(deferred_free);
+
+static size_t free_one_item(enum df_reason reason)
+{
+   unsigned long flags;
+   size_t nr_pages;
+   struct deferred_freelist_item *item;
+
+   spin_lock_irqsave(&free_list_lock, flags);
+   if (list_empty(&free_list)) {
+   spin_unlock_irqrestore(&free_list_lock, flags);
+   return 0;
+   }
+   item = list_first_entry(&free_list, struct deferred_freelist_item, 
list);
+   list_del(&item->list);
+   nr_pages = item->nr_pages;
+   list_nr_pages -= nr_pages;
+   spin_unlock_irqrestore(&free_list_lock, flags);
+
+   item->free(item, reason);
+   return nr_pages;
+}
+
+static unsigned long get_freelist_nr_pages(void)
+{
+   unsigned long nr_pages;
+   unsigned long flags;
+
+   spin_lock_irqsave(&free_list_lock, flags);
+   nr_pages = list_nr_pages;
+   spin_unlock_irqrestore(&free_list_lock, flags);
+   return nr_pages;
+}
+
+static unsigned long freelist_shrink_count(struct shrinker *shrinker,
+  

[PATCH v8 4/5] dma-buf: system_heap: Add drm pagepool support to system heap

[John Stultz]

Utilize the drm pagepool code to speed up allocation
performance.

This is similar to the ION pagepool usage, but tries to
utilize generic code instead of a custom implementation.

Cc: Daniel Vetter 
Cc: Christian Koenig 
Cc: Sumit Semwal 
Cc: Liam Mark 
Cc: Chris Goldsworthy 
Cc: Laura Abbott 
Cc: Brian Starkey 
Cc: Hridya Valsaraju 
Cc: Suren Baghdasaryan 
Cc: Sandeep Patil 
Cc: Daniel Mentz 
Cc: Ørjan Eide 
Cc: Robin Murphy 
Cc: Ezequiel Garcia 
Cc: Simon Ser 
Cc: James Jones 
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz 
---
v2:
* Fix build issue caused by selecting PAGE_POOL w/o NET
  as Reported-by: kernel test robot 
v3:
* Simplify the page zeroing logic a bit by using kmap_atomic
  instead of vmap as suggested by Daniel Mentz
v5:
* Shift away from networking page pool completely to
  dmabuf page pool implementation
v6:
* Switch again to using the drm_page_pool code shared w/
  ttm_pool
v7:
* Slight rework for drm_page_pool changes
v8:
* Rework to use the rewritten drm_page_pool logic
* Drop explicit buffer zeroing, as the drm_page_pool handles that
---
 drivers/dma-buf/heaps/Kconfig   |  1 +
 drivers/dma-buf/heaps/system_heap.c | 27 ---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index f7aef8bc7119..7e28934e0def 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -4,6 +4,7 @@ config DMABUF_HEAPS_DEFERRED_FREE
 config DMABUF_HEAPS_SYSTEM
bool "DMA-BUF System Heap"
depends on DMABUF_HEAPS
+   select DRM_PAGE_POOL
help
  Choose this option to enable the system dmabuf heap. The system heap
  is backed by pages from the buddy allocator. If in doubt, say Y.
diff --git a/drivers/dma-buf/heaps/system_heap.c 
b/drivers/dma-buf/heaps/system_heap.c
index 29e49ac17251..006271881d85 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -21,6 +21,8 @@
 #include 
 #include 
 
+#include 
+
 static struct dma_heap *sys_heap;
 
 struct system_heap_buffer {
@@ -53,6 +55,7 @@ static gfp_t order_flags[] = {HIGH_ORDER_GFP, LOW_ORDER_GFP, 
LOW_ORDER_GFP};
  */
 static const unsigned int orders[] = {8, 4, 0};
 #define NUM_ORDERS ARRAY_SIZE(orders)
+struct drm_page_pool pools[NUM_ORDERS];
 
 static struct sg_table *dup_sg_table(struct sg_table *table)
 {
@@ -281,18 +284,28 @@ static void system_heap_vunmap(struct dma_buf *dmabuf, 
struct dma_buf_map *map)
dma_buf_map_clear(map);
 }
 
+static unsigned long system_heap_free_pages(struct drm_page_pool *pool, struct 
page *p)
+{
+   __free_pages(p, pool->order);
+   return 1 << pool->order;
+}
+
 static void system_heap_dma_buf_release(struct dma_buf *dmabuf)
 {
struct system_heap_buffer *buffer = dmabuf->priv;
struct sg_table *table;
struct scatterlist *sg;
-   int i;
+   int i, j;
 
table = &buffer->sg_table;
for_each_sg(table->sgl, sg, table->nents, i) {
struct page *page = sg_page(sg);
 
-   __free_pages(page, compound_order(page));
+   for (j = 0; j < NUM_ORDERS; j++) {
+   if (compound_order(page) == orders[j])
+   break;
+   }
+   drm_page_pool_add(&pools[j], page);
}
sg_free_table(table);
kfree(buffer);
@@ -323,7 +336,9 @@ static struct page *alloc_largest_available(unsigned long 
size,
if (max_order < orders[i])
continue;
 
-   page = alloc_pages(order_flags[i], orders[i]);
+   page = drm_page_pool_remove(&pools[i]);
+   if (!page)
+   page = alloc_pages(order_flags[i], orders[i]);
if (!page)
continue;
return page;
@@ -423,6 +438,12 @@ static const struct dma_heap_ops system_heap_ops = {
 static int system_heap_create(void)
 {
struct dma_heap_export_info exp_info;
+   int i;
+
+   for (i = 0; i < NUM_ORDERS; i++) {
+   drm_page_pool_init(&pools[i], orders[i],
+  system_heap_free_pages);
+   }
 
exp_info.name = "system";
exp_info.ops = &system_heap_ops;
-- 
2.25.1

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

[PATCH v8 5/5] dma-buf: system_heap: Add deferred freeing to the system heap

[John Stultz]

Utilize the deferred free helper library in the system heap.

This provides a nice performance bump and puts the
system heap performance on par with ION.

Cc: Daniel Vetter 
Cc: Christian Koenig 
Cc: Sumit Semwal 
Cc: Liam Mark 
Cc: Chris Goldsworthy 
Cc: Laura Abbott 
Cc: Brian Starkey 
Cc: Hridya Valsaraju 
Cc: Suren Baghdasaryan 
Cc: Sandeep Patil 
Cc: Daniel Mentz 
Cc: Ørjan Eide 
Cc: Robin Murphy 
Cc: Ezequiel Garcia 
Cc: Simon Ser 
Cc: James Jones 
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz 
---
v2:
* Rework deferred-free api to use reason enum as suggested by
  Suren Baghdasaryan
* Rework for deferred-free api change to use nr_pages rather
  than size as suggsted by Suren Baghdasaryan
v8:
* Reworked to drop buffer zeroing logic, as the drm_page_pool now
  handles that.
---
 drivers/dma-buf/heaps/Kconfig   |  1 +
 drivers/dma-buf/heaps/system_heap.c | 28 ++--
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index 7e28934e0def..10632ccfb4a5 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -5,6 +5,7 @@ config DMABUF_HEAPS_SYSTEM
bool "DMA-BUF System Heap"
depends on DMABUF_HEAPS
select DRM_PAGE_POOL
+   select DMABUF_HEAPS_DEFERRED_FREE
help
  Choose this option to enable the system dmabuf heap. The system heap
  is backed by pages from the buddy allocator. If in doubt, say Y.
diff --git a/drivers/dma-buf/heaps/system_heap.c 
b/drivers/dma-buf/heaps/system_heap.c
index 006271881d85..c753c82fd9f1 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -22,6 +22,7 @@
 #include 
 
 #include 
+#include "deferred-free-helper.h"
 
 static struct dma_heap *sys_heap;
 
@@ -33,6 +34,7 @@ struct system_heap_buffer {
struct sg_table sg_table;
int vmap_cnt;
void *vaddr;
+   struct deferred_freelist_item deferred_free;
 };
 
 struct dma_heap_attachment {
@@ -290,27 +292,41 @@ static unsigned long system_heap_free_pages(struct 
drm_page_pool *pool, struct p
return 1 << pool->order;
 }
 
-static void system_heap_dma_buf_release(struct dma_buf *dmabuf)
+static void system_heap_buf_free(struct deferred_freelist_item *item,
+enum df_reason reason)
 {
-   struct system_heap_buffer *buffer = dmabuf->priv;
+   struct system_heap_buffer *buffer;
struct sg_table *table;
struct scatterlist *sg;
int i, j;
 
+   buffer = container_of(item, struct system_heap_buffer, deferred_free);
table = &buffer->sg_table;
for_each_sg(table->sgl, sg, table->nents, i) {
struct page *page = sg_page(sg);
 
-   for (j = 0; j < NUM_ORDERS; j++) {
-   if (compound_order(page) == orders[j])
-   break;
+   if (reason == DF_UNDER_PRESSURE) {
+   __free_pages(page, compound_order(page));
+   } else {
+   for (j = 0; j < NUM_ORDERS; j++) {
+   if (compound_order(page) == orders[j])
+   break;
+   }
+   drm_page_pool_add(&pools[j], page);
}
-   drm_page_pool_add(&pools[j], page);
}
sg_free_table(table);
kfree(buffer);
 }
 
+static void system_heap_dma_buf_release(struct dma_buf *dmabuf)
+{
+   struct system_heap_buffer *buffer = dmabuf->priv;
+   int npages = PAGE_ALIGN(buffer->len) / PAGE_SIZE;
+
+   deferred_free(&buffer->deferred_free, system_heap_buf_free, npages);
+}
+
 static const struct dma_buf_ops system_heap_buf_ops = {
.attach = system_heap_attach,
.detach = system_heap_detach,
-- 
2.25.1

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

[PATCH 1/3] drm/bridge: ti-sn65dsi86: Simplify refclk handling

[Douglas Anderson]

The clock framework makes it simple to deal with an optional clock.
You can call clk_get_optional() and if the clock isn't specified it'll
just return NULL without complaint. It's valid to pass NULL to
enable/disable/prepare/unprepare. Let's make use of this to simplify
things a tiny bit.

NOTE: this makes things look a tad bit asymmetric now since we check
for NULL before clk_prepare_enable() but not for
clk_disable_unprepare(). This seemed OK to me. We already have to
check for NULL in the enable case anyway so why not avoid the extra
call?

Signed-off-by: Douglas Anderson 
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index f27306c51e4d..942019842ff4 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -1261,14 +1261,9 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
return ret;
}
 
-   pdata->refclk = devm_clk_get(pdata->dev, "refclk");
-   if (IS_ERR(pdata->refclk)) {
-   ret = PTR_ERR(pdata->refclk);
-   if (ret == -EPROBE_DEFER)
-   return ret;
-   DRM_DEBUG_KMS("refclk not found\n");
-   pdata->refclk = NULL;
-   }
+   pdata->refclk = devm_clk_get_optional(pdata->dev, "refclk");
+   if (IS_ERR(pdata->refclk))
+   return PTR_ERR(pdata->refclk);
 
ret = ti_sn_bridge_parse_dsi_host(pdata);
if (ret)
-- 
2.30.1.766.gb4fecdf3b7-goog

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

[PATCH 3/3] drm/bridge: ti-sn65dsi86: Properly get the EDID, but only if refclk

[Douglas Anderson]

In commit 58074b08c04a ("drm/bridge: ti-sn65dsi86: Read EDID blob over
DDC") we attempted to make the ti-sn65dsi86 bridge properly read the
EDID from the panel. That commit kinda worked but it had some serious
problems.

The problems all stem from the fact that userspace wants to be able to
read the EDID before it explicitly enables the panel. For eDP panels,
though, we don't actually power the panel up until the pre-enable
stage and the pre-enable call happens right before the enable call
with no way to interject in-between. For eDP panels, you can't read
the EDID until you power the panel. The result was that
ti_sn_bridge_connector_get_modes() was always failing to read the EDID
(falling back to what drm_panel_get_modes() returned) until _after_
the EDID was needed.

To make it concrete, on my system I saw this happen:
1. We'd attach the bridge.
2. Userspace would ask for the EDID (several times). We'd try but fail
   to read the EDID over and over again and fall back to the hardcoded
   modes.
3. Userspace would decide on a mode based only on the hardcoded modes.
4. Userspace would ask to turn the panel on.
5. Userspace would (eventually) check the modes again (in Chrome OS
   this happens on the handoff from the boot splash screen to the
   browser). Now we'd read them properly and, if they were different,
   userspace would request to change the mode.

The fact that userspace would always end up using the hardcoded modes
at first significantly decreases the benefit of the EDID
reading. Also: if the modes were even a tiny bit different we'd end up
doing a wasteful modeset and at boot.

As a side note: at least early EDID read failures were relatively
fast. Though the old ti_sn_bridge_connector_get_modes() did call
pm_runtime_get_sync() it didn't program the important "HPD_DISABLE"
bit. That meant that all the AUX transfers failed pretty quickly.

In any case, enough about the problem. How are we fixing it? Obviously
we need to power the panel on _before_ reading the EDID, but how? It
turns out that there's really no problem with just doing all the work
of our pre_enable() function right at attach time and reading the EDID
right away. We'll do that. It's not as easy as it sounds, though,
because:

1. Powering the panel up and down is a pretty expensive operation. Not
   only do we need to wait for the HPD line which seems to take up to
   200 ms on most panels, but also most panels say that once you power
   them off you need to wait at least 500 ms before powering them on
   again. We really don't want to incur 700 ms of time here.

2. If we happen not to have a fixed "refclk" we've got a
   chicken-and-egg problem. We seem to need the clock setup to read
   the EDID. Without a fixed "refclk", though, the bridge runs with
   the MIPI pixel clock which means you've got to use a hardcoded mode
   for the MIPI pixel clock.

We'll solve problem #1 above by leaving the panel powered on for a
little while after we read the EDID. If enough time passes and nobody
turns the panel on then we'll undo our work. NOTE: there are no
functional problems if someone turns the panel on after a long delay,
it just might take a little longer to turn on.

We'll solve problem #2 by simply _always_ using a hardcoded mode (not
reading the EDID) if a "refclk" wasn't provided. While it might be
possible to fudge something together to support this, it's my belief
that nobody is using this mode in real life since it's really
inflexible. I saw it used for some really early prototype hardware
that was thrown in the e-waste bin years ago when we realized how
inflexible it was. In any case, if someone is using this they're in no
worse shape than they were before the (fairly recent) commit
58074b08c04a ("drm/bridge: ti-sn65dsi86: Read EDID blob over DDC").

NOTE: while this patch feels a bit hackish, I'm not sure there's much
we can do better without a more fundamental DRM API change. After
looking at it a bunch, it also doesn't feel as hacky to me as I first
thought. The things that pre-enable does are well defined and well
understood and there should be no problems with doing them early nor
with doing them before userspace requests anything.

Fixes: 58074b08c04a ("drm/bridge: ti-sn65dsi86: Read EDID blob over DDC")
Signed-off-by: Douglas Anderson 
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 98 ---
 1 file changed, 88 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 491c9c4f32d1..af3fb4657af6 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -130,6 +131,12 @@
  * @ln_assign:Value to program to the LN_ASSIGN register.
  * @ln_polrs: Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
  *
+ * @pre_enabled_early: If true we did an early pre_enable at attach.
+ * @pre_enable_timeout_work: Delayed wo

[PATCH 2/3] drm/bridge: ti-sn65dsi86: Move code in prep for EDID read fix

[Douglas Anderson]

This patch is _only_ code motion to prepare for the patch
("drm/bridge: ti-sn65dsi86: Properly get the EDID, but only if
refclk") and make it easier to understand.

Signed-off-by: Douglas Anderson 
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 196 +-
 1 file changed, 98 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 942019842ff4..491c9c4f32d1 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -345,6 +345,104 @@ static int ti_sn_bridge_parse_regulators(struct 
ti_sn_bridge *pdata)
   pdata->supplies);
 }
 
+static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn_bridge *pdata)
+{
+   u32 bit_rate_khz, clk_freq_khz;
+   struct drm_display_mode *mode =
+   &pdata->bridge.encoder->crtc->state->adjusted_mode;
+
+   bit_rate_khz = mode->clock *
+   mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
+   clk_freq_khz = bit_rate_khz / (pdata->dsi->lanes * 2);
+
+   return clk_freq_khz;
+}
+
+/* clk frequencies supported by bridge in Hz in case derived from REFCLK pin */
+static const u32 ti_sn_bridge_refclk_lut[] = {
+   1200,
+   1920,
+   2600,
+   2700,
+   3840,
+};
+
+/* clk frequencies supported by bridge in Hz in case derived from DACP/N pin */
+static const u32 ti_sn_bridge_dsiclk_lut[] = {
+   46800,
+   38400,
+   41600,
+   48600,
+   46080,
+};
+
+static void ti_sn_bridge_set_refclk_freq(struct ti_sn_bridge *pdata)
+{
+   int i;
+   u32 refclk_rate;
+   const u32 *refclk_lut;
+   size_t refclk_lut_size;
+
+   if (pdata->refclk) {
+   refclk_rate = clk_get_rate(pdata->refclk);
+   refclk_lut = ti_sn_bridge_refclk_lut;
+   refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
+   clk_prepare_enable(pdata->refclk);
+   } else {
+   refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
+   refclk_lut = ti_sn_bridge_dsiclk_lut;
+   refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_dsiclk_lut);
+   }
+
+   /* for i equals to refclk_lut_size means default frequency */
+   for (i = 0; i < refclk_lut_size; i++)
+   if (refclk_lut[i] == refclk_rate)
+   break;
+
+   regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG, REFCLK_FREQ_MASK,
+  REFCLK_FREQ(i));
+}
+
+static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
+{
+   struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
+
+   clk_disable_unprepare(pdata->refclk);
+
+   pm_runtime_put_sync(pdata->dev);
+}
+
+static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
+{
+   struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
+
+   pm_runtime_get_sync(pdata->dev);
+
+   /* configure bridge ref_clk */
+   ti_sn_bridge_set_refclk_freq(pdata);
+
+   /*
+* HPD on this bridge chip is a bit useless.  This is an eDP bridge
+* so the HPD is an internal signal that's only there to signal that
+* the panel is done powering up.  ...but the bridge chip debounces
+* this signal by between 100 ms and 400 ms (depending on process,
+* voltage, and temperate--I measured it at about 200 ms).  One
+* particular panel asserted HPD 84 ms after it was powered on meaning
+* that we saw HPD 284 ms after power on.  ...but the same panel said
+* that instead of looking at HPD you could just hardcode a delay of
+* 200 ms.  We'll assume that the panel driver will have the hardcoded
+* delay in its prepare and always disable HPD.
+*
+* If HPD somehow makes sense on some future panel we'll have to
+* change this to be conditional on someone specifying that HPD should
+* be used.
+*/
+   regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
+  HPD_DISABLE);
+
+   drm_panel_prepare(pdata->panel);
+}
+
 static int ti_sn_bridge_attach(struct drm_bridge *bridge,
   enum drm_bridge_attach_flags flags)
 {
@@ -443,64 +541,6 @@ static void ti_sn_bridge_disable(struct drm_bridge *bridge)
drm_panel_unprepare(pdata->panel);
 }
 
-static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn_bridge *pdata)
-{
-   u32 bit_rate_khz, clk_freq_khz;
-   struct drm_display_mode *mode =
-   &pdata->bridge.encoder->crtc->state->adjusted_mode;
-
-   bit_rate_khz = mode->clock *
-   mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
-   clk_freq_khz = bit_rate_khz / (pdata->dsi->lanes * 2);
-
-   return clk_freq_khz;
-}
-
-/* clk frequencies supported by bridge in Hz in case derived from REFCLK pin */
-static const u32 ti_sn_bridge_refclk

Re: [PATCH v3 4/8] mm/rmap: Split migration into its own function

[Alistair Popple]

On Wednesday, 3 March 2021 9:08:15 AM AEDT Zi Yan wrote:
> On 26 Feb 2021, at 2:18, Alistair Popple wrote:

> > diff --git a/include/linux/rmap.h b/include/linux/rmap.h
> > index 7f1ee411bd7b..77fa17de51d7 100644
> > --- a/include/linux/rmap.h
> > +++ b/include/linux/rmap.h
> > @@ -86,8 +86,6 @@ struct anon_vma_chain {
> >  };
> >
> >  enum ttu_flags {
> > -   TTU_MIGRATION   = 0x1,  /* migration mode */
> > -
> > TTU_SPLIT_HUGE_PMD  = 0x4,  /* split huge PMD if any */
> 
> It implies freeze in try_to_migrate() and no freeze in try_to_unmap(). I 
think
> we need some comments here, above try_to_migrate(), and above try_to_unmap()
> to clarify the implication.

Sure. This confused me for a bit and I was initially tempted to leave 
TTU_SPLIT_FREEZE as a separate mode flag but looking at what freeze actually 
does it made sense to remove it because try_to_migrate() is for installing 
migration entries (which is what freeze does) and try_to_unmap() just unmaps. 
So I'll add some comments to that effect.
 
> > TTU_IGNORE_MLOCK= 0x8,  /* ignore mlock */
> > TTU_IGNORE_HWPOISON = 0x20, /* corrupted page is recoverable */
> > @@ -96,7 +94,6 @@ enum ttu_flags {
> >  * do a final flush if necessary */
> > TTU_RMAP_LOCKED = 0x80, /* do not grab rmap lock:
> >  * caller holds it */
> > -   TTU_SPLIT_FREEZE= 0x100,/* freeze pte under 
> > splitting thp */
> >  };
> >
> >  #ifdef CONFIG_MMU
> > @@ -193,6 +190,7 @@ static inline void page_dup_rmap(struct page *page, 
bool compound)
> >  int page_referenced(struct page *, int is_locked,
> > struct mem_cgroup *memcg, unsigned long *vm_flags);
> >
> > +bool try_to_migrate(struct page *page, enum ttu_flags flags);
> >  bool try_to_unmap(struct page *, enum ttu_flags flags);
> >
> >  /* Avoid racy checks */
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index d00b93dc2d9e..357052a4567b 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -2351,16 +2351,16 @@ void vma_adjust_trans_huge(struct vm_area_struct 
*vma,
> >
> >  static void unmap_page(struct page *page)
> >  {
> > -   enum ttu_flags ttu_flags = TTU_IGNORE_MLOCK |
> > -   TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD;
> > +   enum ttu_flags ttu_flags = TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD;
> > bool unmap_success;
> >
> > VM_BUG_ON_PAGE(!PageHead(page), page);
> >
> > if (PageAnon(page))
> > -   ttu_flags |= TTU_SPLIT_FREEZE;
> > -
> > -   unmap_success = try_to_unmap(page, ttu_flags);
> > +   unmap_success = try_to_migrate(page, ttu_flags);
> > +   else
> > +   unmap_success = try_to_unmap(page, ttu_flags |
> > +   TTU_IGNORE_MLOCK);
> 
> I think we need a comment here about why anonymous pages need 
try_to_migrate()
> and others need try_to_unmap().

Historically this comes from baa355fd3314 ("thp: file pages support for 
split_huge_page()") which says:

"We don't setup migration entries. Just unmap pages. It helps handling cases 
when i_size is in the middle of the page: no need handle unmap pages beyond 
i_size manually."

But I'll add a comment here, thanks.

 - Alistair

> Thanks.
> 
> —
> Best Regards,
> Yan Zi




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

Re: [PATCH 1/1] drm/ttm: Ignore signaled move fences

[Felix Kuehling]

Am 2021-03-01 um 10:09 a.m. schrieb Christian König:
> Am 27.02.21 um 04:45 schrieb Felix Kuehling:
>> Move fences that have already signaled should not prevent memory
>> allocations with no_wait_gpu.
>>
>> Signed-off-by: Felix Kuehling 
>
> Reviewed-by: Christian König 

I work on this on Alex's rebased amd-staging-drm-next. Should this go
into any other branches?

Thanks,
  Felix


>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 3a10bebb75d6..de1ec838cf8b 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -730,8 +730,9 @@ static int ttm_bo_add_move_fence(struct
>> ttm_buffer_object *bo,
>>   return 0;
>>     if (no_wait_gpu) {
>> +    ret = dma_fence_is_signaled(fence) ? 0 : -EBUSY;
>>   dma_fence_put(fence);
>> -    return -EBUSY;
>> +    return ret;
>>   }
>>     dma_resv_add_shared_fence(bo->base.resv, fence);
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/msm/dp: Fix warnings reported by kbot in DP driver

[Abhinav Kumar]

Fix the warnings reported by kbot across MSM DP driver.

Reported-by: kernel test robot 
Reported-by: Dan Carpenter 
Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/dp/dp_debug.c | 33 +
 drivers/gpu/drm/msm/dp/dp_hpd.c   |  4 ++--
 drivers/gpu/drm/msm/dp/dp_power.c |  2 +-
 3 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c 
b/drivers/gpu/drm/msm/dp/dp_debug.c
index 84670bc..2f6247e 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -226,7 +226,7 @@ static int dp_test_data_show(struct seq_file *m, void *data)
debug->link->test_video.test_h_width);
seq_printf(m, "vdisplay: %d\n",
debug->link->test_video.test_v_height);
-   seq_printf(m, "bpc: %u\n",
+   seq_printf(m, "bpc: %u\n",
dp_link_bit_depth_to_bpc(bpc));
} else
seq_puts(m, "0");
@@ -368,44 +368,21 @@ static int dp_debug_init(struct dp_debug *dp_debug, 
struct drm_minor *minor)
int rc = 0;
struct dp_debug_private *debug = container_of(dp_debug,
struct dp_debug_private, dp_debug);
-   struct dentry *file;
-   struct dentry *test_active;
-   struct dentry *test_data, *test_type;
 
-   file = debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
+   debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
debug, &dp_debug_fops);
-   if (IS_ERR_OR_NULL(file)) {
-   rc = PTR_ERR(file);
-   DRM_ERROR("[%s] debugfs create file failed, rc=%d\n",
- DEBUG_NAME, rc);
-   }
 
-   test_active = debugfs_create_file("msm_dp_test_active", 0444,
+   debugfs_create_file("msm_dp_test_active", 0444,
minor->debugfs_root,
debug, &test_active_fops);
-   if (IS_ERR_OR_NULL(test_active)) {
-   rc = PTR_ERR(test_active);
-   DRM_ERROR("[%s] debugfs test_active failed, rc=%d\n",
- DEBUG_NAME, rc);
-   }
 
-   test_data = debugfs_create_file("msm_dp_test_data", 0444,
+   debugfs_create_file("msm_dp_test_data", 0444,
minor->debugfs_root,
debug, &dp_test_data_fops);
-   if (IS_ERR_OR_NULL(test_data)) {
-   rc = PTR_ERR(test_data);
-   DRM_ERROR("[%s] debugfs test_data failed, rc=%d\n",
- DEBUG_NAME, rc);
-   }
 
-   test_type = debugfs_create_file("msm_dp_test_type", 0444,
+   debugfs_create_file("msm_dp_test_type", 0444,
minor->debugfs_root,
debug, &dp_test_type_fops);
-   if (IS_ERR_OR_NULL(test_type)) {
-   rc = PTR_ERR(test_type);
-   DRM_ERROR("[%s] debugfs test_type failed, rc=%d\n",
- DEBUG_NAME, rc);
-   }
 
debug->root = minor->debugfs_root;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_hpd.c b/drivers/gpu/drm/msm/dp/dp_hpd.c
index 5b8fe3202..e1c90fa 100644
--- a/drivers/gpu/drm/msm/dp/dp_hpd.c
+++ b/drivers/gpu/drm/msm/dp/dp_hpd.c
@@ -34,8 +34,8 @@ int dp_hpd_connect(struct dp_usbpd *dp_usbpd, bool hpd)
 
dp_usbpd->hpd_high = hpd;
 
-   if (!hpd_priv->dp_cb && !hpd_priv->dp_cb->configure
-   && !hpd_priv->dp_cb->disconnect) {
+   if (!hpd_priv->dp_cb || !hpd_priv->dp_cb->configure
+   || !hpd_priv->dp_cb->disconnect) {
pr_err("hpd dp_cb not initialized\n");
return -EINVAL;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
b/drivers/gpu/drm/msm/dp/dp_power.c
index 9c4ea00..3961ba4 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -269,7 +269,7 @@ int dp_power_clk_enable(struct dp_power *dp_power,
DRM_ERROR("failed to '%s' clks for: %s. err=%d\n",
enable ? "enable" : "disable",
dp_parser_pm_name(pm_type), rc);
-   return rc;
+   return rc;
}
 
if (pm_type == DP_CORE_PM)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[PATCH] drm/nouveau/kms/nve4-nv108: Limit cursors to 128x128

[Lyude Paul]

While Kepler does technically support 256x256 cursors, it turns out that
Kepler actually has some additional requirements for scanout surfaces that
we're not enforcing correctly, which aren't present on Maxwell and later.
Cursor surfaces must always use small pages (4K), and overlay surfaces must
always use large pages (128K).

Fixing this correctly though will take a bit more work: as we'll need to
add some code in prepare_fb() to move cursor FBs in large pages to small
pages, and vice-versa for overlay FBs. So until we have the time to do
that, just limit cursor surfaces to 128x128 - a size small enough to always
default to small pages.

This means small ovlys are still broken on Kepler, but it is extremely
unlikely anyone cares about those anyway :).

Signed-off-by: Lyude Paul 
Fixes: d3b2f0f7921c ("drm/nouveau/kms/nv50-: Report max cursor size to 
userspace")
Cc:  # v5.11+
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 196612addfd6..1c9c0cdf85db 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2693,9 +2693,20 @@ nv50_display_create(struct drm_device *dev)
else
nouveau_display(dev)->format_modifiers = disp50xx_modifiers;
 
-   if (disp->disp->object.oclass >= GK104_DISP) {
+   /* FIXME: 256x256 cursors are supported on Kepler, however unlike 
Maxwell and later
+* generations Kepler requires that we use small pages (4K) for cursor 
scanout surfaces. The
+* proper fix for this is to teach nouveau to migrate fbs being used 
for the cursor plane to
+* small page allocations in prepare_fb(). When this is implemented, we 
should also force
+* large pages (128K) for ovly fbs in order to fix Kepler ovlys.
+* But until then, just limit cursors to 128x128 - which is small 
enough to avoid ever using
+* large pages.
+*/
+   if (disp->disp->object.oclass >= GM107_DISP) {
dev->mode_config.cursor_width = 256;
dev->mode_config.cursor_height = 256;
+   } else if (disp->disp->object.oclass >= GK104_DISP) {
+   dev->mode_config.cursor_width = 128;
+   dev->mode_config.cursor_height = 128;
} else {
dev->mode_config.cursor_width = 64;
dev->mode_config.cursor_height = 64;
-- 
2.29.2

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

[git pull] drm fixes for 5.12-rc2

[Dave Airlie]

Hi Linus,

More may show up but this is what I have at this staged. These are
based on the commit in your tree where the swapfile issue is fixed,
and neither of the merged trees are in the bad area.

Otherwise just a single nouveau regression fix, and a bunch of amdgpu fixes.

Dave.

drm-fixes-2021-03-05:
drm fixes for 5.12-rc2

amdgpu:
- S0ix fix
- Handle new NV12 SKU
- Misc power fixes
- Display uninitialized value fix
- PCIE debugfs register access fix

nouveau:
- regression fix for gk104
The following changes since commit f69d02e37a85645aa90d18cacfff36dba370f797:

  Merge tag 'misc-5.12-2021-03-02' of git://git.kernel.dk/linux-block
(2021-03-02 18:18:17 -0800)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-03-05

for you to fetch changes up to a1f1054124936c717a64e47862e3d0d820f67a87:

  Merge tag 'amd-drm-fixes-5.12-2021-03-03' of
https://gitlab.freedesktop.org/agd5f/linux into drm-fixes (2021-03-05
11:13:22 +1000)


drm fixes for 5.12-rc2

amdgpu:
- S0ix fix
- Handle new NV12 SKU
- Misc power fixes
- Display uninitialized value fix
- PCIE debugfs register access fix

nouveau:
- regression fix for gk104


Alex Deucher (4):
  drm/amdgpu: Only check for S0ix if AMD_PMC is configured
  drm/amdgpu/pm: make unsupported power profile messages debug
  drm/amdgpu/swsmu/vangogh: Only use RLCPowerNotify msg for disable
  drm/amdgpu: enable BACO runpm by default on sienna cichlid and
navy flounder

Asher.Song (1):
  drm/amdgpu:disable VCN for Navi12 SKU

Ben Skeggs (1):
  drm/nouveau/fifo/gk104-gp1xx: fix creation of sw class

Colin Ian King (1):
  drm/amd/display: fix the return of the uninitialized value in ret

Dave Airlie (2):
  Merge branch '00.00-inst' of git://github.com/skeggsb/linux into drm-fixes
  Merge tag 'amd-drm-fixes-5.12-2021-03-03' of
https://gitlab.freedesktop.org/agd5f/linux into drm-fixes

Evan Quan (1):
  drm/amd/pm: correct Arcturus mmTHM_BACO_CNTL register address

Kevin Wang (1):
  drm/amdgpu: fix parameter error of RREG32_PCIE() in amdgpu_regs_pcie

 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c  |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |  2 --
 drivers/gpu/drm/amd/amdgpu/nv.c   |  6 --
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  2 +-
 drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c |  2 +-
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c| 15 ---
 drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  |  6 +++---
 drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c   |  2 +-
 drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c  |  3 +++
 10 files changed, 29 insertions(+), 16 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Is LLVM 13 (git) really ready for testing/development? libclc didn't compile

[Dieter Nützel]

Hello Marek,

can't compile anything, here.
Poor Intel Nehalem X3470.

Trying LLVM 12-rc2 later.

Greetings,
Dieter

llvm-project/libclc> cd build && cmake ../
-- The CXX compiler identification is GNU 10.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
LLVM version: 13.0.0git
LLVM system libs:
LLVM libs: -lLLVM-13git
LLVM libdir: /usr/local/lib
LLVM bindir: /usr/local/bin
LLVM ld flags: -L/usr/local/lib
LLVM cxx flags: 
-I/usr/local/include;-std=c++14;;;-fno-exceptions;-D_GNU_SOURCE;-D__STDC_CONSTANT_MACROS;-D__STDC_FORMAT_MACROS;-D__STDC_LIMIT_MACROS;-fno-rtti;-fno-exceptions


clang: /usr/local/bin/clang
llvm-as: /usr/local/bin/llvm-as
llvm-link: /usr/local/bin/llvm-link
opt: /usr/local/bin/opt
llvm-spirv: /usr/local/bin/llvm-spirv

-- Check for working CLC compiler: /usr/local/bin/clang
-- Check for working CLC compiler: /usr/local/bin/clang -- works
-- Check for working LLAsm compiler: /usr/local/bin/llvm-as
-- Check for working LLAsm compiler: /usr/local/bin/llvm-as -- broken
CMake Error at cmake/CMakeTestLLAsmCompiler.cmake:40 (message):
  The LLAsm compiler "/usr/local/bin/llvm-as" is not able to compile a 
simple

  test program.

  It fails with the following output:

   Change Dir: /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp



  Run Build Command(s):/usr/bin/gmake cmTC_87af9/fast && /usr/bin/gmake 
-f

  CMakeFiles/cmTC_87af9.dir/build.make CMakeFiles/cmTC_87af9.dir/build

  gmake[1]: Verzeichnis
  „/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird betreten

  Building LLAsm object CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc

  /usr/local/bin/clang -E -P -x cl
  
/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp/testLLAsmCompiler.ll 
-o

  CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp

  /usr/local/bin/llvm-as -o 
CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc

  CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp

  /usr/local/bin/llvm-as:
  CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp:1:1: error: 
expected

  top-level entity

  typedef unsigned char uchar;

  ^

  gmake[1]: *** [CMakeFiles/cmTC_87af9.dir/build.make:86:
  CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc] Fehler 1

  gmake[1]: Verzeichnis
  „/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird verlassen

  gmake: *** [Makefile:140: cmTC_87af9/fast] Fehler 2







  CMake will not be able to correctly generate this project.
Call Stack (most recent call first):
  CMakeLists.txt:127 (enable_language)


-- Configuring incomplete, errors occurred!
See also "/opt/llvm-project/libclc/build/CMakeFiles/CMakeOutput.log".
See also "/opt/llvm-project/libclc/build/CMakeFiles/CMakeError.log".


CMakeError.log
Determining if the LLAsm compiler works failed with the following 
output:

Change Dir: /opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/gmake cmTC_87af9/fast && /usr/bin/gmake  
-f CMakeFiles/cmTC_87af9.dir/build.make CMakeFiles/cmTC_87af9.dir/build
gmake[1]: Verzeichnis 
„/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird betreten

Building LLAsm object CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc
/usr/local/bin/clang -E -P -x cl 
/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp/testLLAsmCompiler.ll 
-o CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp
/usr/local/bin/llvm-as -o CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc 
CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp
/usr/local/bin/llvm-as: 
CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc.temp:1:1: error: expected 
top-level entity

typedef unsigned char uchar;
^
gmake[1]: *** [CMakeFiles/cmTC_87af9.dir/build.make:86: 
CMakeFiles/cmTC_87af9.dir/testLLAsmCompiler.bc] Fehler 1
gmake[1]: Verzeichnis 
„/opt/llvm-project/libclc/build/CMakeFiles/CMakeTmp“ wird verlassen

gmake: *** [Makefile:140: cmTC_87af9/fast] Fehler 2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm fixes for 5.12-rc2

[pr-tracker-bot]

The pull request you sent on Fri, 5 Mar 2021 12:50:16 +1000:

> git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-03-05

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/280d542f6ffac0e6d65dc267f92191d509b13b64

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH] gpu: drm: swsmu: fix error return code of smu_v11_0_set_allowed_mask()

[Quan, Evan]

[AMD Public Use]

Thanks. Reviewed-by: Evan Quan 

-Original Message-
From: Jia-Ju Bai  
Sent: Friday, March 5, 2021 11:54 AM
To: Deucher, Alexander ; Koenig, Christian 
; airl...@linux.ie; dan...@ffwll.ch; Quan, Evan 
; Zhang, Hawking ; Wang, Kevin(Yang) 
; Gao, Likun 
Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org; Jia-Ju Bai 
Subject: [PATCH] gpu: drm: swsmu: fix error return code of 
smu_v11_0_set_allowed_mask()

When bitmap_empty() or feature->feature_num triggers an error, no error return 
code of smu_v11_0_set_allowed_mask() is assigned.
To fix this bug, ret is assigned with -EINVAL as error return code.

Reported-by: TOTE Robot 
Signed-off-by: Jia-Ju Bai 
---
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 90585461a56e..82731a932308 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -747,8 +747,10 @@ int smu_v11_0_set_allowed_mask(struct smu_context *smu)
int ret = 0;
uint32_t feature_mask[2];
 
-   if (bitmap_empty(feature->allowed, SMU_FEATURE_MAX) || 
feature->feature_num < 64)
+   if (bitmap_empty(feature->allowed, SMU_FEATURE_MAX) || 
feature->feature_num < 64) {
+   ret = -EINVAL;
goto failed;
+   }
 
bitmap_copy((unsigned long *)feature_mask, feature->allowed, 64);
 
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/amd/display: Remove unnecessary conversion to bool

[Jiapeng Chong]

Fix the following coccicheck warnings:

./drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:8257:16-21: WARNING:
conversion to bool not needed here.

Reported-by: Abaci Robot 
Signed-off-by: Jiapeng Chong 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 3e1fd1e..10de6c2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8307,8 +8307,7 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
hdcp_update_display(
adev->dm.hdcp_workqueue, 
aconnector->dc_link->link_index, aconnector,
new_con_state->hdcp_content_type,
-   new_con_state->content_protection == 
DRM_MODE_CONTENT_PROTECTION_DESIRED ? true
-   
 : false);
+   new_con_state->content_protection == 
DRM_MODE_CONTENT_PROTECTION_DESIRED);
}
 #endif
 
-- 
1.8.3.1

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

Re: [PATCH] drm/msm/dp: Fix warnings reported by kbot in DP driver

[Stephen Boyd]

Maybe subject could be "Ignore debugfs failures, fix indentation, fix
logical error"?

Quoting Abhinav Kumar (2021-03-04 17:31:52)
> Fix the warnings reported by kbot across MSM DP driver.

Which warnings? Can you include them? Or at least list the three things
that are being fixed in this patch?

> 
> Reported-by: kernel test robot 
> Reported-by: Dan Carpenter 
> Signed-off-by: Abhinav Kumar 
> ---
>  drivers/gpu/drm/msm/dp/dp_debug.c | 33 +
>  drivers/gpu/drm/msm/dp/dp_hpd.c   |  4 ++--
>  drivers/gpu/drm/msm/dp/dp_power.c |  2 +-
>  3 files changed, 8 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c 
> b/drivers/gpu/drm/msm/dp/dp_debug.c
> index 84670bc..2f6247e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_debug.c
> +++ b/drivers/gpu/drm/msm/dp/dp_debug.c
> @@ -226,7 +226,7 @@ static int dp_test_data_show(struct seq_file *m, void 
> *data)
> debug->link->test_video.test_h_width);
> seq_printf(m, "vdisplay: %d\n",
> 
> debug->link->test_video.test_v_height);
> -   seq_printf(m, "bpc: %u\n",
> +   seq_printf(m, "bpc: %u\n",
> dp_link_bit_depth_to_bpc(bpc));
> } else
> seq_puts(m, "0");

Indentation.

> @@ -368,44 +368,21 @@ static int dp_debug_init(struct dp_debug *dp_debug, 
> struct drm_minor *minor)
> int rc = 0;
> struct dp_debug_private *debug = container_of(dp_debug,
> struct dp_debug_private, dp_debug);
> -   struct dentry *file;
> -   struct dentry *test_active;
> -   struct dentry *test_data, *test_type;
>  
> -   file = debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
> +   debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
> debug, &dp_debug_fops);
> -   if (IS_ERR_OR_NULL(file)) {
> -   rc = PTR_ERR(file);
> -   DRM_ERROR("[%s] debugfs create file failed, rc=%d\n",
> - DEBUG_NAME, rc);
> -   }
>  
> -   test_active = debugfs_create_file("msm_dp_test_active", 0444,
> +   debugfs_create_file("msm_dp_test_active", 0444,
> minor->debugfs_root,
> debug, &test_active_fops);
> -   if (IS_ERR_OR_NULL(test_active)) {
> -   rc = PTR_ERR(test_active);
> -   DRM_ERROR("[%s] debugfs test_active failed, rc=%d\n",
> - DEBUG_NAME, rc);
> -   }
>  
> -   test_data = debugfs_create_file("msm_dp_test_data", 0444,
> +   debugfs_create_file("msm_dp_test_data", 0444,
> minor->debugfs_root,
> debug, &dp_test_data_fops);
> -   if (IS_ERR_OR_NULL(test_data)) {
> -   rc = PTR_ERR(test_data);
> -   DRM_ERROR("[%s] debugfs test_data failed, rc=%d\n",
> - DEBUG_NAME, rc);
> -   }
>  
> -   test_type = debugfs_create_file("msm_dp_test_type", 0444,
> +   debugfs_create_file("msm_dp_test_type", 0444,
> minor->debugfs_root,
> debug, &dp_test_type_fops);
> -   if (IS_ERR_OR_NULL(test_type)) {
> -   rc = PTR_ERR(test_type);
> -   DRM_ERROR("[%s] debugfs test_type failed, rc=%d\n",
> - DEBUG_NAME, rc);
> -   }

Debugfs failures.

>  
> debug->root = minor->debugfs_root;
>  
> diff --git a/drivers/gpu/drm/msm/dp/dp_hpd.c b/drivers/gpu/drm/msm/dp/dp_hpd.c
> index 5b8fe3202..e1c90fa 100644
> --- a/drivers/gpu/drm/msm/dp/dp_hpd.c
> +++ b/drivers/gpu/drm/msm/dp/dp_hpd.c
> @@ -34,8 +34,8 @@ int dp_hpd_connect(struct dp_usbpd *dp_usbpd, bool hpd)
>  
> dp_usbpd->hpd_high = hpd;
>  
> -   if (!hpd_priv->dp_cb && !hpd_priv->dp_cb->configure
> -   && !hpd_priv->dp_cb->disconnect) {
> +   if (!hpd_priv->dp_cb || !hpd_priv->dp_cb->configure
> +   || !hpd_priv->dp_cb->disconnect) {

Logical error.

> pr_err("hpd dp_cb not initialized\n");
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
> b/drivers/gpu/drm/msm/dp/dp_power.c
> index 9c4ea00..3961ba4 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.c
> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
> @@ -269,7 +269,7 @@ int dp_power_clk_enable(struct dp_power *dp_power,
> DRM_ERROR("failed to '%s' clks for: %s. err=%d\n",
> enable ? "enable" : "disable",
> dp_parser_pm_name(pm_type), rc);
> -   return rc;
> +   return rc;

Indentation.

> }
>  
> if (pm_type == DP_CORE_PM)
___
dri-devel mailing l

Re: [PATCH v8 2/5] drm: ttm_pool: Rework ttm_pool to use drm_page_pool

[kernel test robot]

Hi John,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc1]
[cannot apply to linux/master drm-intel/for-linux-next drm-tip/drm-tip 
next-20210305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/John-Stultz/Generic-page-pool-deferred-freeing-for-system-dmabuf-heap/20210305-072137
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
3cb60ee6323968b694208c4cbd56a7176396e931
config: parisc-randconfig-r024-20210305 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/0day-ci/linux/commit/701300f424bbe73234f3dc3b62581a5e6ddef78a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
John-Stultz/Generic-page-pool-deferred-freeing-for-system-dmabuf-heap/20210305-072137
git checkout 701300f424bbe73234f3dc3b62581a5e6ddef78a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross 
ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   hppa-linux-ld: drivers/gpu/drm/page_pool.o: in function 
`drm_page_pool_shrinker_setup':
>> (.text.drm_page_pool_shrinker_setup+0x0): multiple definition of 
>> `init_module'; drivers/gpu/drm/drm_drv.o:(.init.text+0x0): first defined here
   hppa-linux-ld: drivers/gpu/drm/page_pool.o: in function 
`drm_page_pool_shrinker_teardown':
>> (.text.drm_page_pool_shrinker_teardown+0x0): multiple definition of 
>> `cleanup_module'; drivers/gpu/drm/drm_drv.o:(.text.drm_core_exit+0x0): first 
>> defined here

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/msm/dp: Fix warnings reported by kbot in DP driver

[Dan Carpenter]

On Thu, Mar 04, 2021 at 10:55:58PM -0800, Stephen Boyd wrote:
> > @@ -368,44 +368,21 @@ static int dp_debug_init(struct dp_debug *dp_debug, 
> > struct drm_minor *minor)
> > int rc = 0;
> > struct dp_debug_private *debug = container_of(dp_debug,
> > struct dp_debug_private, dp_debug);
> > -   struct dentry *file;
> > -   struct dentry *test_active;
> > -   struct dentry *test_data, *test_type;
> >  
> > -   file = debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
> > +   debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
> > debug, &dp_debug_fops);
> > -   if (IS_ERR_OR_NULL(file)) {
> > -   rc = PTR_ERR(file);
> > -   DRM_ERROR("[%s] debugfs create file failed, rc=%d\n",
> > - DEBUG_NAME, rc);
> > -   }
> >  
> > -   test_active = debugfs_create_file("msm_dp_test_active", 0444,
> > +   debugfs_create_file("msm_dp_test_active", 0444,
> > minor->debugfs_root,
> > debug, &test_active_fops);
> > -   if (IS_ERR_OR_NULL(test_active)) {
> > -   rc = PTR_ERR(test_active);
> > -   DRM_ERROR("[%s] debugfs test_active failed, rc=%d\n",
> > - DEBUG_NAME, rc);
> > -   }
> >  
> > -   test_data = debugfs_create_file("msm_dp_test_data", 0444,
> > +   debugfs_create_file("msm_dp_test_data", 0444,
> > minor->debugfs_root,
> > debug, &dp_test_data_fops);
> > -   if (IS_ERR_OR_NULL(test_data)) {
> > -   rc = PTR_ERR(test_data);
> > -   DRM_ERROR("[%s] debugfs test_data failed, rc=%d\n",
> > - DEBUG_NAME, rc);
> > -   }
> >  
> > -   test_type = debugfs_create_file("msm_dp_test_type", 0444,
> > +   debugfs_create_file("msm_dp_test_type", 0444,
> > minor->debugfs_root,
> > debug, &dp_test_type_fops);
> > -   if (IS_ERR_OR_NULL(test_type)) {
> > -   rc = PTR_ERR(test_type);
> > -   DRM_ERROR("[%s] debugfs test_type failed, rc=%d\n",
> > - DEBUG_NAME, rc);
> > -   }
> 
> Debugfs failures.

[ Update.  I misunderstood what you were saying, and initially thought
  you were critiquing the patch instead of the commit message.  The
  patch looks okay.  Probably a lot of maintainers would prefer it
  broken multiple chunks with one patch per class of warning.  But I
  already wrote this email and I love the sound of my own voice so I'm
  sending it.  - dan ]

The Smatch warning for this was that the error handling was slightly
off because debugfs_create_file() doesn't return NULL these days.  But
really these functions are not supposed to be error checked in the
normal case.

If you do a `git grep -w debugfs_create_file` there are 1472 callers
and only 192 check.  This is partly because Greg went through and did a
mass delete of error handling.

The way that debugfs works is if you fail to create a directory then
the debugfs_create_file will check if the root is an error pointer.  So
passing it "handles" errors itself.

The one time where I've seen that checking for errors is essential is
if they driver dereferences the "test_data" dentry itself.  That's
pretty uncommon.

[ So probably the commit message for this chunk should be:

  Delete unnecessary debugfs error handling

  Debugfs functions are not supposed to be checked in the normal case
  so delete this code.  Also it silences a Smatch warning that we're
  checking for NULL when these functions only return error pointers.  ]

regards,
dan carpenter

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

Re: [PATCH 1/1] drm/ttm: Ignore signaled move fences

[Christian König]

Am 05.03.21 um 02:21 schrieb Felix Kuehling:

Am 2021-03-01 um 10:09 a.m. schrieb Christian König:

Am 27.02.21 um 04:45 schrieb Felix Kuehling:

Move fences that have already signaled should not prevent memory
allocations with no_wait_gpu.

Signed-off-by: Felix Kuehling 

Reviewed-by: Christian König 

I work on this on Alex's rebased amd-staging-drm-next. Should this go
into any other branches?


I have a branch with stuff for 5.13 which I want to push to 
drm-misc-next as soon as 5.12-rc1 is out.


Going to add this one here to that collection as well unless you say 
that this is really a bug fix and we need it earlier.


Regards,
Christian.



Thanks,
   Felix



---
   drivers/gpu/drm/ttm/ttm_bo.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3a10bebb75d6..de1ec838cf8b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -730,8 +730,9 @@ static int ttm_bo_add_move_fence(struct
ttm_buffer_object *bo,
   return 0;
     if (no_wait_gpu) {
+    ret = dma_fence_is_signaled(fence) ? 0 : -EBUSY;
   dma_fence_put(fence);
-    return -EBUSY;
+    return ret;
   }
     dma_resv_add_shared_fence(bo->base.resv, fence);


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