Re: [PATCH v2 01/11] pwm: Make .get_state() callback return an error code

[Neil Armstrong]

On 30/11/2022 16:21, Uwe Kleine-König wrote:

.get_state() might fail in some cases. To make it possible that a driver
signals such a failure change the prototype of .get_state() to return an
error code.

This patch was created using coccinelle and the following semantic patch:

@p1@
identifier getstatefunc;
identifier driver;
@@
  struct pwm_ops driver = {
 ...,
 .get_state = getstatefunc
 ,...
  };

@p2@
identifier p1.getstatefunc;
identifier chip, pwm, state;
@@
-void
+int
  getstatefunc(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state 
*state)
  {
...
-  return;
+  return 0;
...
  }

plus the actual change of the prototype in include/linux/pwm.h (plus some
manual fixing of indentions and empty lines).

So for now all drivers return success unconditionally. They are adapted
in the following patches to make the changes easier reviewable.

Signed-off-by: Uwe Kleine-König 
---
  drivers/gpio/gpio-mvebu.c |  9 ++---
  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 14 --
  drivers/leds/rgb/leds-qcom-lpg.c  | 14 --
  drivers/pwm/pwm-atmel.c   |  6 --
  drivers/pwm/pwm-bcm-iproc.c   |  8 +---
  drivers/pwm/pwm-crc.c | 10 ++
  drivers/pwm/pwm-cros-ec.c |  8 +---
  drivers/pwm/pwm-dwc.c |  6 --
  drivers/pwm/pwm-hibvt.c   |  6 --
  drivers/pwm/pwm-imx-tpm.c |  8 +---
  drivers/pwm/pwm-imx27.c   |  8 +---
  drivers/pwm/pwm-intel-lgm.c   |  6 --
  drivers/pwm/pwm-iqs620a.c |  6 --
  drivers/pwm/pwm-keembay.c |  6 --
  drivers/pwm/pwm-lpss.c|  6 --
  drivers/pwm/pwm-meson.c   |  8 +---
  drivers/pwm/pwm-mtk-disp.c| 12 +++-
  drivers/pwm/pwm-pca9685.c |  8 +---
  drivers/pwm/pwm-raspberrypi-poe.c |  8 +---
  drivers/pwm/pwm-rockchip.c| 12 +++-
  drivers/pwm/pwm-sifive.c  |  6 --
  drivers/pwm/pwm-sl28cpld.c|  8 +---
  drivers/pwm/pwm-sprd.c|  8 +---
  drivers/pwm/pwm-stm32-lp.c|  8 +---
  drivers/pwm/pwm-sun4i.c   | 12 +++-
  drivers/pwm/pwm-sunplus.c |  6 --
  drivers/pwm/pwm-visconti.c|  6 --
  drivers/pwm/pwm-xilinx.c  |  8 +---
  include/linux/pwm.h   |  4 ++--
  29 files changed, 146 insertions(+), 89 deletions(-)






diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 57112f438c6d..16d79ca5d8f5 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -318,8 +318,8 @@ static unsigned int meson_pwm_cnt_to_ns(struct pwm_chip 
*chip,
return cnt * fin_ns * (channel->pre_div + 1);
  }
  
-static void meson_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,

-   struct pwm_state *state)
+static int meson_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+  struct pwm_state *state)
  {
struct meson_pwm *meson = to_meson_pwm(chip);
struct meson_pwm_channel_data *channel_data;
@@ -327,7 +327,7 @@ static void meson_pwm_get_state(struct pwm_chip *chip, 
struct pwm_device *pwm,
u32 value, tmp;
  
  	if (!state)

-   return;
+   return 0;
  
  	channel = &meson->channels[pwm->hwpwm];

channel_data = &meson_pwm_per_channel_data[pwm->hwpwm];
@@ -357,6 +357,8 @@ static void meson_pwm_get_state(struct pwm_chip *chip, 
struct pwm_device *pwm,
state->period = 0;
state->duty_cycle = 0;
}
+
+   return 0;
  }
  
  static const struct pwm_ops meson_pwm_ops = {




For pwm-meson:

Reviewed-by: Neil Armstrong 

[PATCH] gpu: host1x: Remove redundant null checks before kfree

[zys . zljxml]

From: Yushan Zhou 

Fix the following coccicheck warning:
./drivers/gpu/host1x/fence.c:97:2-7: WARNING:
NULL check before some freeing functions is not needed.

Signed-off-by: Yushan Zhou 
---
 drivers/gpu/host1x/fence.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/host1x/fence.c b/drivers/gpu/host1x/fence.c
index ecab72882192..05b36bfc8b74 100644
--- a/drivers/gpu/host1x/fence.c
+++ b/drivers/gpu/host1x/fence.c
@@ -93,8 +93,7 @@ static void host1x_syncpt_fence_release(struct dma_fence *f)
 {
struct host1x_syncpt_fence *sf = to_host1x_fence(f);
 
-   if (sf->waiter)
-   kfree(sf->waiter);
+   kfree(sf->waiter);
 
dma_fence_free(f);
 }
-- 
2.27.0

Re: [PATCH] net: tun: Remove redundant null checks before kfree

[Katrin Jo]

On Wed, Nov 30, 2022 at 9:57 PM Simon Horman  wrote:
>
> + Thierry Reding, linux-tegra, dri-devel
>
> On Tue, Nov 29, 2022 at 04:43:29PM +0800, zys.zlj...@gmail.com wrote:
> > From: Yushan Zhou 
> >
> > Fix the following coccicheck warning:
> > ./drivers/gpu/host1x/fence.c:97:2-7: WARNING:
> > NULL check before some freeing functions is not needed.
> >
> > Signed-off-by: Yushan Zhou 
>
> Hi,
>
> the change in the patch looks good to me.
> However, it does not appear to be a networking patch,
> so I think you have sent it to the wrong place.
>
> With reference to:
>
> $ ./scripts/get_maintainer.pl drivers/gpu/host1x/fence.c
> Thierry Reding  (supporter:DRM DRIVERS FOR NVIDIA 
> TEGRA)
> David Airlie  (maintainer:DRM DRIVERS)
> Daniel Vetter  (maintainer:DRM DRIVERS)
> Sumit Semwal  (maintainer:DMA BUFFER SHARING 
> FRAMEWORK)
> "Christian König"  (maintainer:DMA BUFFER SHARING 
> FRAMEWORK)
> dri-devel@lists.freedesktop.org (open list:DRM DRIVERS FOR NVIDIA TEGRA)
> linux-te...@vger.kernel.org (open list:DRM DRIVERS FOR NVIDIA TEGRA)
> linux-ker...@vger.kernel.org (open list)
> linux-me...@vger.kernel.org (open list:DMA BUFFER SHARING FRAMEWORK)
> linaro-mm-...@lists.linaro.org (moderated list:DMA BUFFER SHARING FRAMEWORK)
>
> And 
> https://lore.kernel.org/dri-devel/39c44dce203112a8dfe279e8e2c4ad164e3cf5e5.1666275461.git.robin.mur...@arm.com/
>
> I would suggest that the patch subject should be:
>
>  [PATCH] gpu: host1x: Remove redundant null check before kfree
>
> And you should send it:
>
>   To: Thierry Reding 
>   Cc: linux-te...@vger.kernel.org, dri-devel@lists.freedesktop.org
>
> > ---
> >  drivers/gpu/host1x/fence.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/host1x/fence.c b/drivers/gpu/host1x/fence.c
> > index ecab72882192..05b36bfc8b74 100644
> > --- a/drivers/gpu/host1x/fence.c
> > +++ b/drivers/gpu/host1x/fence.c
> > @@ -93,8 +93,7 @@ static void host1x_syncpt_fence_release(struct dma_fence 
> > *f)
> >  {
> > struct host1x_syncpt_fence *sf = to_host1x_fence(f);
> >
> > -   if (sf->waiter)
> > -   kfree(sf->waiter);
> > +   kfree(sf->waiter);
> >
> > dma_fence_free(f);
> >  }
> > --
> > 2.27.0
> >

Apologies for the mistake... I'll resend it to the correct place.
Thanks for your reminder, anyway.

Best,
Katrin

Re: Screen corruption using radeon kernel driver

[Mikhail Krylov]

On Wed, Nov 30, 2022 at 11:07:32AM -0500, Alex Deucher wrote:
> On Wed, Nov 30, 2022 at 10:42 AM Robin Murphy  wrote:
> >
> > On 2022-11-30 14:28, Alex Deucher wrote:
> > > On Wed, Nov 30, 2022 at 7:54 AM Robin Murphy  wrote:
> > >>
> > >> On 2022-11-29 17:11, Mikhail Krylov wrote:
> > >>> On Tue, Nov 29, 2022 at 11:05:28AM -0500, Alex Deucher wrote:
> >  On Tue, Nov 29, 2022 at 10:59 AM Mikhail Krylov  
> >  wrote:
> > >
> > > On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote:
> > >> On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov  
> > >> wrote:
> > >>>
> > >>> On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher wrote:
> > >>>
> > >> [excessive quoting removed]
> > >>>
> > > So, is there any progress on this issue? I do understand it's not 
> > > a high
> > > priority one, and today I've checked it on 6.0 kernel, and
> > > unfortunately, it still persists...
> > >
> > > I'm considering writing a patch that will allow user to override
> > > need_dma32/dma_bits setting with a module parameter. I'll have 
> > > some time
> > > after the New Year for that.
> > >
> > > Is it at all possible that such a patch will be merged into 
> > > kernel?
> > >
> >  On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov  
> >  wrote:
> >  Unless someone familiar with HIMEM can figure out what is going 
> >  wrong
> >  we should just revert the patch.
> > 
> >  Alex
> > >>>
> > >>>
> > >>> Okay, I was suggesting that mostly because
> > >>>
> > >>> a) it works for me with dma_bits = 40 (I understand that's what it 
> > >>> is
> > >>> without the original patch applied);
> > >>>
> > >>> b) there's a hint of uncertainity on this line
> > >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359
> > >>> saying that for AGP dma_bits = 32 is the safest option, so 
> > >>> apparently there are
> > >>> setups, unlike mine, where dma_bits = 32 is better than 40.
> > >>>
> > >>> But I'm in no position to argue, just wanted to make myself clear.
> > >>> I'm okay with rebuilding the kernel for my machine until the 
> > >>> original
> > >>> patch is reverted or any other fix is applied.
> > >>
> > >> What GPU do you have and is it AGP?  If it is AGP, does setting
> > >> radeon.agpmode=-1 also fix it?
> > >>
> > >> Alex
> > >
> > > That is ATI Radeon X1950, and, unfortunately, radeon.agpmode=-1 
> > > doesn't
> > > help, it just makes 3D acceleration in games such as OpenArena stop
> > > working.
> > 
> >  Just to confirm, is the board AGP or PCIe?
> > 
> >  Alex
> > >>>
> > >>> It is AGP. That's an old machine.
> > >>
> > >> Can you check whether dma_addressing_limited() is actually returning the
> > >> expected result at the point of radeon_ttm_init()? Disabling highmem is
> > >> presumably just hiding whatever problem exists, by throwing away all
> > >>   >32-bit RAM such that use_dma32 doesn't matter.
> > >
> > > The device in question only supports a 32 bit DMA mask so
> > > dma_addressing_limited() should return true.  Bounce buffers are not
> > > really usable on GPUs because they map so much memory.  If
> > > dma_addressing_limited() returns false, that would explain it.
> >
> > Right, it appears to be the only part of the offending commit that
> > *could* reasonably make any difference, so I'm primarily wondering if
> > dma_get_required_mask() somehow gets confused.
> 
> Mikhail,
> 
> Can you see that dma_addressing_limited() and dma_get_required_mask()
> return in this case?
> 
> Alex
> 
> 
> >
> > Thanks,
> > Robin.

Unfortunately, right now I don't have enough time for kernel
modifications and rebuilds (I will later!), so I did a quick-and-dirty
research with kprobe. 

The problem is that dma_addressing_limited() seems to be inlined and
kprobe fails to intercept it.

But I managed to get the result of dma_get_required_mask(). It returns
0x7fff (!) on the vanilla (with the patch, buggy) kernel:
 
$ sudo kprobe-perf 'r:dma_get_required_mask $retval'
Tracing kprobe dma_get_required_mask. Ctrl-C to end.
modprobe-1244[000] d...   105.582816: dma_get_required_mask: 
(radeon_ttm_init+0x61/0x240 [radeon] <- dma_get_required_mask) arg1=0x7fff

This function does not even get called in the kernel without the patch
that I built myself. I believe that's because ttm_bo_device_init()
doesn't call it without the patch.

Hope that helps at least a bit. If not, I'll be able to do more thorough
research in a couple of weeks, probably.


signature.asc
Description: PGP signature

Re: [PATCH v5 1/2] drm/fourcc: Add Synaptics VideoSmart tiled modifiers

[Randy Li]

Sent from my iPad

> On Nov 30, 2022, at 7:30 PM, Daniel Vetter  wrote:
> 
> CAUTION: Email originated externally, do not click links or open attachments 
> unless you recognize the sender and know the content is safe.
> 
> 
>> On Wed, Nov 30, 2022 at 05:21:48PM +0800, Hsia-Jun Li wrote:
>> From: "Hsia-Jun(Randy) Li" 
>> 
>> Those modifiers only record the parameters would effort pixel
>> layout or memory layout. Whether physical memory page mapping
>> is used is not a part of format.
>> 
>> Signed-off-by: Hsia-Jun(Randy) Li 
>> ---
>> include/uapi/drm/drm_fourcc.h | 76 +++
>> 1 file changed, 76 insertions(+)
>> 
>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> index bc056f2d537d..e0905f573f43 100644
>> --- a/include/uapi/drm/drm_fourcc.h
>> +++ b/include/uapi/drm/drm_fourcc.h
>> @@ -407,6 +407,7 @@ extern "C" {
>> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
>> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
>> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
>> +#define DRM_FORMAT_MOD_VENDOR_SYNAPTICS 0x0b
>> 
>> /* add more to the end as needed */
>> 
>> @@ -1507,6 +1508,81 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 
>> modifier)
>> #define AMD_FMT_MOD_CLEAR(field) \
>>  (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
>> 
>> +/*
>> + * Synaptics VideoSmart modifiers
>> + *
>> + * Tiles could be arranged in Groups of Tiles (GOTs), it is a small tile
>> + * within a tile. GOT size and layout varies based on platform and
>> + * performance concern.
>> + *
>> + * Besides, an 8 length 4 bytes arrary (32 bytes) would be need to store
>> + * some compression parameters for a compression metadata plane.
>> + *
>> + * Further information can be found in
>> + * Documentation/gpu/synaptics.rst
>> + *
>> + *   Macro
>> + * Bits  Param Description
>> + *   - 
>> -
>> + *
>> + *  7:0  f Scan direction description.
>> + *
>> + *   0 = Invalid
>> + *   1 = V4, the scan would always start from vertical for 4 
>> pixel
>> + *   then move back to the start pixel of the next 
>> horizontal
>> + *   direction.
>> + *   2 = Reserved for future use.
>> + *
>> + * 15:8  m The times of pattern repeat in the right angle direction from
>> + * the first scan direction.
>> + *
>> + * 19:16 p The padding bits after the whole scan, could be zero.
>> + *
>> + * 20:20 g GOT packing flag.
>> + *
>> + * 23:21 - Reserved for future use.  Must be zero.
> 
> Can you pls fold all the future use reservations into the top end?
You see we could put more related flag in each of reserved area.
Here is for the group of tiles flag.
Bit 35 to 32 could be used for describing the dimension of the group of tiles.
> Also I
> think it'd be good to at least reserve maybe the top 8 bits or so for a
> synaptics specific format indicator, so that it's easier to extend this in
> the future ...
I think the  bit 56 to 63 are used for storing the vendor id. That is why I 
didn’t include them below. Or you mean the bit 7 to 0?
Do yo
> -Daniel
> 
> 
>> + *
>> + * 27:24 h log2(horizontal) of pixels, in GOTs.
>> + *
>> + * 31:28 v log2(vertical) of pixels, in GOTs.
>> + *
>> + * 35:32 - Reserved for future use.  Must be zero.
>> + *
>> + * 36:36 c Compression flag.
>> + *
>> + * 55:37 - Reserved for future use.  Must be zero.
>> + *
>> + */
>> +
>> +#define DRM_FORMAT_MOD_SYNA_V4_TILED fourcc_mod_code(SYNAPTICS, 1)
>> +
>> +#define DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(f, m, p, g, h, v, c) \
>> + fourcc_mod_code(SYNAPTICS, ((__u64)((f) & 0xff) | \
>> +  ((__u64)((m) & 0xff) << 8) | \
>> +  ((__u64)((p) & 0xf) << 16) | \
>> +  ((__u64)((g) & 0x1) << 20) | \
>> +  ((__u64)((h) & 0xf) << 24) | \
>> +  ((__u64)((v) & 0xf) << 28) | \
>> +  ((__u64)((c) & 0x1) << 36)))
>> +
>> +#define DRM_FORMAT_MOD_SYNA_V4H1 \
>> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 1, 0, 0, 0, 0, 0)
>> +
>> +#define DRM_FORMAT_MOD_SYNA_V4H3P8 \
>> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 3, 8, 0, 0, 0, 0)
>> +
>> +#define DRM_FORMAT_MOD_SYNA_V4H1_64L4_COMPRESSED \
>> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 1, 0, 1, 6, 2, 1)
>> +
>> +#define DRM_FORMAT_MOD_SYNA_V4H3P8_64L4_COMPRESSED \
>> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 3, 8, 1, 6, 2, 1)
>> +
>> +#define DRM_FORMAT_MOD_SYNA_V4H1_128L128_COMPRESSED \
>> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 1, 0, 1, 7, 7, 1)
>> +
>> +#define DRM_FORMAT_MOD_SYNA_V4H3P8_128L128_COMPRESSED \
>> + DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(1, 3, 8, 1, 7, 7, 1)
>> +
>> #if defined(__cplusplus)
>> }
>> #endif
>> --
>> 2.37.3
>> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> https://

Re: [PATCH v2 01/11] pwm: Make .get_state() callback return an error code

[Baolin Wang]

On 11/30/2022 11:21 PM, Uwe Kleine-König wrote:

diff --git a/drivers/pwm/pwm-sprd.c b/drivers/pwm/pwm-sprd.c
index 7004f55bbf11..bda8bc5af976 100644
--- a/drivers/pwm/pwm-sprd.c
+++ b/drivers/pwm/pwm-sprd.c
@@ -65,8 +65,8 @@ static void sprd_pwm_write(struct sprd_pwm_chip *spc, u32 
hwid,
writel_relaxed(val, spc->base + offset);
  }
  
-static void sprd_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,

-  struct pwm_state *state)
+static int sprd_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
  {
struct sprd_pwm_chip *spc =
container_of(chip, struct sprd_pwm_chip, chip);
@@ -83,7 +83,7 @@ static void sprd_pwm_get_state(struct pwm_chip *chip, struct 
pwm_device *pwm,
if (ret) {
dev_err(spc->dev, "failed to enable pwm%u clocks\n",
pwm->hwpwm);
-   return;
+   return 0;
}
  
  	val = sprd_pwm_read(spc, pwm->hwpwm, SPRD_PWM_ENABLE);

@@ -113,6 +113,8 @@ static void sprd_pwm_get_state(struct pwm_chip *chip, 
struct pwm_device *pwm,
/* Disable PWM clocks if the PWM channel is not in enable state. */
if (!state->enabled)
clk_bulk_disable_unprepare(SPRD_PWM_CHN_CLKS_NUM, chn->clks);
+
+   return 0;
  }
  
  static int sprd_pwm_config(struct sprd_pwm_chip *spc, struct pwm_device *pwm,


For sprd pwm parts:
Reviewed-by: Baolin Wang 

Re: [PATCH] drm: bridge: dw_hdmi: fix preference of RGB modes over YUV420

[Neil Armstrong]

Hi,

On Wed, 16 Nov 2022 15:35:23 +0100, Guillaume BRUN wrote:
> Cheap monitors sometimes advertise YUV modes they don't really have
> (HDMI specification mandates YUV support so even monitors without actual
> support will often wrongfully advertise it) which results in YUV matches
> and user forum complaints of a red tint to light colour display areas in
> common desktop environments.
> 
> Moving the default RGB fall-back before YUV selection results in RGB
> mode matching in most cases, reducing complaints.
> 
> [...]

Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git 
(drm-misc-fixes)

[1/1] drm: bridge: dw_hdmi: fix preference of RGB modes over YUV420
  
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d3d6b1bf85aefe0ebc0624574b3bb62f0693914c

-- 
Neil

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Greg KH]

On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 Submission 
Endpoint wrote:
> Hi,
> 
> I have started to look at igt for testing and want to use CRC tests. To
> implement support for this I need to move away from the simple kms
> helper.
> 
> When looking around for examples I came across Thomas' nice shadow
> helper and thought, yes this is perfect for drm/gud. So I'll switch to
> that before I move away from the simple kms helper.
> 
> The async framebuffer flushing code path now uses a shadow buffer and
> doesn't touch the framebuffer when it shouldn't. I have also taken the
> opportunity to inline the synchronous flush code path and make this the
> default flushing stategy.
> 
> Noralf.
> 
> Cc: Maxime Ripard 
> Cc: Thomas Zimmermann 
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Noralf Trønnes 
> 
> ---
> Changes in v2:
> - Drop patch (Thomas):
>   drm/gem: shadow_fb_access: Prepare imported buffers for CPU access
> - Use src as variable name for iosys_map (Thomas)
> - Prepare imported buffer for CPU access in the driver (Thomas)
> - New patch: make sync flushing the default (Thomas)
> - Link to v1: 
> https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa33...@tronnes.org



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

Re: [PATCH] gpu: host1x: Remove redundant null checks before kfree

[Mikko Perttunen]

On 12/1/22 03:55, zys.zlj...@gmail.com wrote:

From: Yushan Zhou 

Fix the following coccicheck warning:
./drivers/gpu/host1x/fence.c:97:2-7: WARNING:
NULL check before some freeing functions is not needed.

Signed-off-by: Yushan Zhou 
---
  drivers/gpu/host1x/fence.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/host1x/fence.c b/drivers/gpu/host1x/fence.c
index ecab72882192..05b36bfc8b74 100644
--- a/drivers/gpu/host1x/fence.c
+++ b/drivers/gpu/host1x/fence.c
@@ -93,8 +93,7 @@ static void host1x_syncpt_fence_release(struct dma_fence *f)
  {
struct host1x_syncpt_fence *sf = to_host1x_fence(f);
  
-	if (sf->waiter)

-   kfree(sf->waiter);
+   kfree(sf->waiter);
  
  	dma_fence_free(f);

  }


I disagree with this coccinelle rule; I think it obfuscates from the 
reader the fact that the pointer could be NULL.


Mikko

Re: [PATCH RESEND2 v4 2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()

[Neil Armstrong]

On 30/11/2022 10:23, Matti Vaittinen wrote:

Simplify using the devm_regulator_get_enable_optional(). Also drop the
now unused struct member 'hdmi_supply'.

Signed-off-by: Matti Vaittinen 
Martin Blumenstingl 


Missing Acked-by, I'll add it while applying.

Neil



---
v4 resend 2:
Respinning unchanged code with the commit title changed as wa suggested
by Robert Foss and commit message changed as was suggested by Martin.

I am doing a clean-up for my local git and encountered this one.
Respinning as it seems this one fell through the cracks.
---
  drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
  1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
-   struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi 
*meson_dw_hdmi)
  
  }
  
-static void meson_disable_regulator(void *data)

-{
-   regulator_disable(data);
-}
-
  static void meson_disable_clk(void *data)
  {
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct 
device *master,
meson_dw_hdmi->data = match;
dw_plat_data = &meson_dw_hdmi->dw_plat_data;
  
-	meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");

-   if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
-   if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
-   return -EPROBE_DEFER;
-   meson_dw_hdmi->hdmi_supply = NULL;
-   } else {
-   ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
-   if (ret)
-   return ret;
-   ret = devm_add_action_or_reset(dev, meson_disable_regulator,
-  meson_dw_hdmi->hdmi_supply);
-   if (ret)
-   return ret;
-   }
+   ret = devm_regulator_get_enable_optional(dev, "hdmi");
+   if (ret != -ENODEV)
+   return ret;
  
  	meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,

"hdmitx_apb");

Re: [PATCH] drm/etnaviv: print MMU exception cause

[Philipp Zabel]

On Mi, 2022-11-30 at 19:53 +0100, Lucas Stach wrote:
From: Christian Gmeiner 

The MMU tells us the fault status. While the raw register value is
already printed, it's a bit more user friendly to translate the
fault reasons into human readable format.

Signed-off-by: Christian Gmeiner 
Signed-off-by: Lucas Stach 
---
I've rewritten parts of the patch to properly cover multiple
MMUs and squashed the reason into the existing message. Christian,
please tell me if you are fine with having your name attached to
this patch.
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..f79203b774d9 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1426,6 +1426,15 @@ static void sync_point_worker(struct work_struct *work)
 

 static void dump_mmu_fault(struct etnaviv_gpu *gpu)
 {
+   static const char *fault_reasons[] = {
+   "slave not present",
+   "page not present",
+   "write violation",
+   "out of bounds",
+   "read security violation",
+   "write security violation",
+   };
+
    u32 status_reg, status;
    int i;
 

@@ -1438,18 +1447,25 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu)
    dev_err_ratelimited(gpu->dev, "MMU fault status 0x%08x\n", status);
 

    for (i = 0; i < 4; i++) {
+   const char *reason = "unknown";
    u32 address_reg;
+   u32 mmu_status;
 

-   if (!(status & (VIVS_MMUv2_STATUS_EXCEPTION0__MASK << (i * 4
+   mmu_status = (status >> (i * 4)) & 
VIVS_MMUv2_STATUS_EXCEPTION0__MASK;

VIVS_MMUv2_STATUS_EXCEPTION0__MASK is 0x3 ...

+   if (!mmu_status)
    continue;
 

+   if ((mmu_status - 1) < ARRAY_SIZE(fault_reasons))
+   reason = fault_reasons[mmu_status - 1];

... so (mmu_status - 1) can be 2 at most. This leaves me wondering how
"out of bounds" and the "security violation" errors can be reached. I
think this requires the exception bitfield masks to be extended to 0x7.

regards
Philipp

Re: [PATCH RESEND2 v4 0/2] Use devm helpers for regulator get and enable

[Neil Armstrong]

Hi,

On Wed, 30 Nov 2022 11:21:51 +0200, Matti Vaittinen wrote:
> Simplify couple of drivers by using the new devm_regulator_*get_enable*()
> 
> These patches were previously part of the series:
> https://lore.kernel.org/lkml/cover.1660934107.git.mazziesacco...@gmail.com/
> "Devm helpers for regulator get and enable". I did keep the patch series
> versioning even though I changed the series name (subject of this mail)
> to "Use devm helpers for regulator get and enable". Name was changed
> because the devm helpers are already in 6.1-rc1.
> 
> [...]

Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git 
(drm-misc-next)

[1/2] drm/bridge: sii902x: Use devm_regulator_bulk_get_enable()
  
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ff1eae1201a46f997126297d2d3440baa2d1b9a9
[2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
  
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=429e8706366166494314a46c82a9a9929aedbb8c

-- 
Neil

Re: [PATCH RESEND2 v4 2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()

[Matti Vaittinen]

On 12/1/22 10:38, Neil Armstrong wrote:

On 30/11/2022 10:23, Matti Vaittinen wrote:

Simplify using the devm_regulator_get_enable_optional(). Also drop the
now unused struct member 'hdmi_supply'.

Signed-off-by: Matti Vaittinen 
Martin Blumenstingl 


Missing Acked-by, I'll add it while applying.


Oh, well spotted. I should've been more careful.. Sorry and thanks for 
sorting it out!


--Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

[PATCH v3 0/7] drm/vc4: dpi: Various improvements

[Maxime Ripard]

Hi,

Those patches have been in the downstream RaspberryPi tree for a while and help
to support more DPI displays.

Let me know what you think,
Maxime

To: Emma Anholt 
To: Maxime Ripard 
To: David Airlie 
To: Daniel Vetter 
To: Eric Anholt 
To: Rob Herring 
Cc: linux-ker...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: Chris Morgan 
Cc: Joerg Quinten 
Cc: Laurent Pinchart 
Cc: Dave Stevenson 
Signed-off-by: Maxime Ripard 

---
Changes in v3:
- Rebased on drm-misc-next-2022-11-24
- Fixed the order of the new defines and documentation
- Link to v2: 
https://lore.kernel.org/r/20221013-rpi-dpi-improvements-v2-0-7691903fb...@cerno.tech

Changes in v2:
- Documentation for the media bus formats
- Reword the commit log of patch 5
- Link to v1: 
https://lore.kernel.org/r/20221013-rpi-dpi-improvements-v1-0-8a7a96949...@cerno.tech

---
Chris Morgan (2):
  media: uapi: add MEDIA_BUS_FMT_RGB565_1X24_CPADHI
  drm/vc4: dpi: Support RGB565 format

Dave Stevenson (2):
  drm/vc4: dpi: Change the default DPI format to being 18bpp, not 24.
  drm/vc4: dpi: Fix format mapping for RGB565

Joerg Quinten (3):
  media: uapi: add MEDIA_BUS_FMT_BGR666_1X18
  media: uapi: add MEDIA_BUS_FMT_BGR666_1X24_CPADHI
  drm/vc4: dpi: Support BGR666 formats

 .../userspace-api/media/v4l/subdev-formats.rst | 111 +
 drivers/gpu/drm/vc4/vc4_dpi.c  |  16 ++-
 include/uapi/linux/media-bus-format.h  |   5 +-
 3 files changed, 128 insertions(+), 4 deletions(-)
---
base-commit: 6fb6c979ca628583d4d0c59a0f8ff977e581ecc0
change-id: 20221013-rpi-dpi-improvements-c3d755531c39

Best regards,
-- 
Maxime Ripard 

[PATCH v3 1/7] media: uapi: add MEDIA_BUS_FMT_RGB565_1X24_CPADHI

[Maxime Ripard]

From: Chris Morgan 

Add the MEDIA_BUS_FMT_RGB565_1X24_CPADHI format used by the Geekworm
MZP280 panel for the Raspberry Pi.

Signed-off-by: Chris Morgan 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Maxime Ripard 
---
 .../userspace-api/media/v4l/subdev-formats.rst | 37 ++
 include/uapi/linux/media-bus-format.h  |  3 +-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst 
b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index d21d532eee15..aa549c42e798 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -1023,6 +1023,43 @@ The following tables list existing packed RGB formats.
   - b\ :sub:`2`
   - b\ :sub:`1`
   - b\ :sub:`0`
+* .. _MEDIA-BUS-FMT-RGB565-1X24_CPADHI:
+
+  - MEDIA_BUS_FMT_RGB565_1X24_CPADHI
+  - 0x1022
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  - 0
+  - 0
+  - 0
+  - r\ :sub:`4`
+  - r\ :sub:`3`
+  - r\ :sub:`2`
+  - r\ :sub:`1`
+  - r\ :sub:`0`
+  - 0
+  - 0
+  - g\ :sub:`5`
+  - g\ :sub:`4`
+  - g\ :sub:`3`
+  - g\ :sub:`2`
+  - g\ :sub:`1`
+  - g\ :sub:`0`
+  - 0
+  - 0
+  - 0
+  - b\ :sub:`4`
+  - b\ :sub:`3`
+  - b\ :sub:`2`
+  - b\ :sub:`1`
+  - b\ :sub:`0`
 * .. _MEDIA-BUS-FMT-BGR888-1X24:
 
   - MEDIA_BUS_FMT_BGR888_1X24
diff --git a/include/uapi/linux/media-bus-format.h 
b/include/uapi/linux/media-bus-format.h
index ec3323dbb927..8e159e6b4d21 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -34,7 +34,7 @@
 
 #define MEDIA_BUS_FMT_FIXED0x0001
 
-/* RGB - next is   0x1022 */
+/* RGB - next is   0x1023 */
 #define MEDIA_BUS_FMT_RGB444_1X12  0x1016
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE  0x1001
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE  0x1002
@@ -48,6 +48,7 @@
 #define MEDIA_BUS_FMT_RGB666_1X18  0x1009
 #define MEDIA_BUS_FMT_RBG888_1X24  0x100e
 #define MEDIA_BUS_FMT_RGB666_1X24_CPADHI   0x1015
+#define MEDIA_BUS_FMT_RGB565_1X24_CPADHI   0x1022
 #define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG0x1010
 #define MEDIA_BUS_FMT_BGR888_1X24  0x1013
 #define MEDIA_BUS_FMT_BGR888_3X8   0x101b

-- 
b4 0.10.1

[PATCH v3 2/7] media: uapi: add MEDIA_BUS_FMT_BGR666_1X18

[Maxime Ripard]

From: Joerg Quinten 

Add the BGR666 format MEDIA_BUS_FMT_BGR666_1X18 supported by the
RaspberryPi.

Signed-off-by: Joerg Quinten 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Maxime Ripard 
---
 .../userspace-api/media/v4l/subdev-formats.rst | 37 ++
 include/uapi/linux/media-bus-format.h  |  3 +-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst 
b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index aa549c42e798..6605c056cc7c 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -949,6 +949,43 @@ The following tables list existing packed RGB formats.
   - b\ :sub:`2`
   - b\ :sub:`1`
   - b\ :sub:`0`
+* .. _MEDIA-BUS-FMT-BGR666-1X18:
+
+  - MEDIA_BUS_FMT_BGR666_1X18
+  - 0x1023
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  - b\ :sub:`5`
+  - b\ :sub:`4`
+  - b\ :sub:`3`
+  - b\ :sub:`2`
+  - b\ :sub:`1`
+  - b\ :sub:`0`
+  - g\ :sub:`5`
+  - g\ :sub:`4`
+  - g\ :sub:`3`
+  - g\ :sub:`2`
+  - g\ :sub:`1`
+  - g\ :sub:`0`
+  - r\ :sub:`5`
+  - r\ :sub:`4`
+  - r\ :sub:`3`
+  - r\ :sub:`2`
+  - r\ :sub:`1`
+  - r\ :sub:`0`
 * .. _MEDIA-BUS-FMT-RBG888-1X24:
 
   - MEDIA_BUS_FMT_RBG888_1X24
diff --git a/include/uapi/linux/media-bus-format.h 
b/include/uapi/linux/media-bus-format.h
index 8e159e6b4d21..6ce56a984112 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -34,7 +34,7 @@
 
 #define MEDIA_BUS_FMT_FIXED0x0001
 
-/* RGB - next is   0x1023 */
+/* RGB - next is   0x1024 */
 #define MEDIA_BUS_FMT_RGB444_1X12  0x1016
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE  0x1001
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE  0x1002
@@ -46,6 +46,7 @@
 #define MEDIA_BUS_FMT_RGB565_2X8_BE0x1007
 #define MEDIA_BUS_FMT_RGB565_2X8_LE0x1008
 #define MEDIA_BUS_FMT_RGB666_1X18  0x1009
+#define MEDIA_BUS_FMT_BGR666_1X18  0x1023
 #define MEDIA_BUS_FMT_RBG888_1X24  0x100e
 #define MEDIA_BUS_FMT_RGB666_1X24_CPADHI   0x1015
 #define MEDIA_BUS_FMT_RGB565_1X24_CPADHI   0x1022

-- 
b4 0.10.1

[PATCH v3 3/7] media: uapi: add MEDIA_BUS_FMT_BGR666_1X24_CPADHI

[Maxime Ripard]

From: Joerg Quinten 

Add the BGR666 format MEDIA_BUS_FMT_BGR666_1X24_CPADHI supported by the
RaspberryPi.

Signed-off-by: Joerg Quinten 
Signed-off-by: Maxime Ripard 
---
 .../userspace-api/media/v4l/subdev-formats.rst | 37 ++
 include/uapi/linux/media-bus-format.h  |  3 +-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst 
b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index 6605c056cc7c..5f2ce6eada71 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -1060,6 +1060,43 @@ The following tables list existing packed RGB formats.
   - b\ :sub:`2`
   - b\ :sub:`1`
   - b\ :sub:`0`
+* .. _MEDIA-BUS-FMT-BGR666-1X24_CPADHI:
+
+  - MEDIA_BUS_FMT_BGR666_1X24_CPADHI
+  - 0x1024
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+  - 0
+  - 0
+  - b\ :sub:`5`
+  - b\ :sub:`4`
+  - b\ :sub:`3`
+  - b\ :sub:`2`
+  - b\ :sub:`1`
+  - b\ :sub:`0`
+  - 0
+  - 0
+  - g\ :sub:`5`
+  - g\ :sub:`4`
+  - g\ :sub:`3`
+  - g\ :sub:`2`
+  - g\ :sub:`1`
+  - g\ :sub:`0`
+  - 0
+  - 0
+  - r\ :sub:`5`
+  - r\ :sub:`4`
+  - r\ :sub:`3`
+  - r\ :sub:`2`
+  - r\ :sub:`1`
+  - r\ :sub:`0`
 * .. _MEDIA-BUS-FMT-RGB565-1X24_CPADHI:
 
   - MEDIA_BUS_FMT_RGB565_1X24_CPADHI
diff --git a/include/uapi/linux/media-bus-format.h 
b/include/uapi/linux/media-bus-format.h
index 6ce56a984112..f3b0b8091a2c 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -34,7 +34,7 @@
 
 #define MEDIA_BUS_FMT_FIXED0x0001
 
-/* RGB - next is   0x1024 */
+/* RGB - next is   0x1025 */
 #define MEDIA_BUS_FMT_RGB444_1X12  0x1016
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE  0x1001
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE  0x1002
@@ -49,6 +49,7 @@
 #define MEDIA_BUS_FMT_BGR666_1X18  0x1023
 #define MEDIA_BUS_FMT_RBG888_1X24  0x100e
 #define MEDIA_BUS_FMT_RGB666_1X24_CPADHI   0x1015
+#define MEDIA_BUS_FMT_BGR666_1X24_CPADHI   0x1024
 #define MEDIA_BUS_FMT_RGB565_1X24_CPADHI   0x1022
 #define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG0x1010
 #define MEDIA_BUS_FMT_BGR888_1X24  0x1013

-- 
b4 0.10.1

[PATCH v3 4/7] drm/vc4: dpi: Support RGB565 format

[Maxime Ripard]

From: Chris Morgan 

The RGB565 format with padding over 24 bits
(MEDIA_BUS_FMT_RGB565_1X24_CPADHI) is supported by the vc4 DPI
controller. This is what the Geekworm MZP280 DPI display uses, so let's
add support for it in the DPI controller driver.

Reviewed-by: Dave Stevenson 
Signed-off-by: Chris Morgan 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_dpi.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 1f8f44b7b5a5..7da3dd1db50e 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -182,6 +182,10 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
*encoder)
dpi_c |= 
VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_3,
   DPI_FORMAT);
break;
+   case MEDIA_BUS_FMT_RGB565_1X24_CPADHI:
+   dpi_c |= 
VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_2,
+  DPI_FORMAT);
+   break;
default:
DRM_ERROR("Unknown media bus format %d\n",
  bus_format);

-- 
b4 0.10.1

[PATCH v3 5/7] drm/vc4: dpi: Support BGR666 formats

[Maxime Ripard]

From: Joerg Quinten 

The VC4 DPI output can support multiple BGR666 variants, but they were
never added to the driver. Let's add the the support for those formats.

Signed-off-by: Joerg Quinten 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_dpi.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 7da3dd1db50e..ecbe4cd87036 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -170,10 +170,16 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
*encoder)
dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR,
   DPI_ORDER);
break;
+   case MEDIA_BUS_FMT_BGR666_1X24_CPADHI:
+   dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR, 
DPI_ORDER);
+   fallthrough;
case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
dpi_c |= 
VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_2,
   DPI_FORMAT);
break;
+   case MEDIA_BUS_FMT_BGR666_1X18:
+   dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR, 
DPI_ORDER);
+   fallthrough;
case MEDIA_BUS_FMT_RGB666_1X18:
dpi_c |= 
VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_1,
   DPI_FORMAT);

-- 
b4 0.10.1

[PATCH v3 6/7] drm/vc4: dpi: Change the default DPI format to being 18bpp, not 24.

[Maxime Ripard]

From: Dave Stevenson 

DPI hasn't really been used up until now, so the default has
been meaningless.
In theory we should be able to pass the desired format for the
adjacent bridge chip through, but framework seems to be missing
for that.

As the main device to use DPI is the VGA666 or Adafruit Kippah,
both of which use RGB666, change the default to being RGB666 instead
of RGB888.

Signed-off-by: Dave Stevenson 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_dpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index ecbe4cd87036..fdae02760b6d 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -150,8 +150,8 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
*encoder)
}
drm_connector_list_iter_end(&conn_iter);
 
-   /* Default to 24bit if no connector or format found. */
-   dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, DPI_FORMAT);
+   /* Default to 18bit if no connector or format found. */
+   dpi_c |= VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_1, DPI_FORMAT);
 
if (connector) {
if (connector->display_info.num_bus_formats) {

-- 
b4 0.10.1

[PATCH v3 7/7] drm/vc4: dpi: Fix format mapping for RGB565

[Maxime Ripard]

From: Dave Stevenson 

The mapping is incorrect for RGB565_1X16 as it should be
DPI_FORMAT_18BIT_666_RGB_1 instead of DPI_FORMAT_18BIT_666_RGB_3.

Fixes: 08302c35b59d ("drm/vc4: Add DPI driver")
Signed-off-by: Dave Stevenson 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_dpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index fdae02760b6d..a7bebfa5d5b0 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -185,7 +185,7 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
*encoder)
   DPI_FORMAT);
break;
case MEDIA_BUS_FMT_RGB565_1X16:
-   dpi_c |= 
VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_3,
+   dpi_c |= 
VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_1,
   DPI_FORMAT);
break;
case MEDIA_BUS_FMT_RGB565_1X24_CPADHI:

-- 
b4 0.10.1

[PULL] drm-intel-fixes

[Tvrtko Ursulin]

Hi Dave, Daniel,

A few more small fixes for the release candidate week.

Fixes for confused return values when waiting on request retirement, which
can explode in the GuC backend, fix for reading on DRAM info data and a
fix to filter out impossible display pipes from the bigjoiner code.

Regards,

Tvrtko

drm-intel-fixes-2022-12-01:
- Fix dram info readout (Radhakrishna Sripada)
- Remove non-existent pipes from bigjoiner pipe mask (Ville Syrjälä)
- Fix negative value passed as remaining time (Janusz Krzysztofik)
- Never return 0 if not all requests retired (Janusz Krzysztofik)
The following changes since commit b7b275e60bcd5f89771e865a8239325f86d9927d:

  Linux 6.1-rc7 (2022-11-27 13:31:48 -0800)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2022-12-01

for you to fetch changes up to 12b8b046e4c9de40fa59b6f067d6826f4e688f68:

  drm/i915: Never return 0 if not all requests retired (2022-11-29 08:49:15 
+)


- Fix dram info readout (Radhakrishna Sripada)
- Remove non-existent pipes from bigjoiner pipe mask (Ville Syrjälä)
- Fix negative value passed as remaining time (Janusz Krzysztofik)
- Never return 0 if not all requests retired (Janusz Krzysztofik)


Janusz Krzysztofik (2):
  drm/i915: Fix negative value passed as remaining time
  drm/i915: Never return 0 if not all requests retired

Radhakrishna Sripada (1):
  drm/i915/mtl: Fix dram info readout

Ville Syrjälä (1):
  drm/i915: Remove non-existent pipes from bigjoiner pipe mask

 drivers/gpu/drm/i915/display/intel_display.c | 10 +++---
 drivers/gpu/drm/i915/gt/intel_gt.c   |  9 +++--
 drivers/gpu/drm/i915/gt/intel_gt_requests.c  |  2 +-
 drivers/gpu/drm/i915/intel_dram.c|  3 +--
 4 files changed, 16 insertions(+), 8 deletions(-)

Re: [PATCH v2 4/6] drm/gud: Prepare buffer for CPU access in gud_flush_work()

[Thomas Zimmermann]

Am 30.11.22 um 20:26 schrieb Noralf Trønnes via B4 Submission Endpoint:

From: Noralf Trønnes 

In preparation for moving to the shadow plane helper prepare the
framebuffer for CPU access as early as possible.

v2:
- Use src as variable name for iosys_map (Thomas)

Reviewed-by: Javier Martinez Canillas 
Signed-off-by: Noralf Trønnes 


Reviewed-by: Thomas Zimmermann 


---
  drivers/gpu/drm/gud/gud_pipe.c | 67 +-
  1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index d2af9947494f..98fe8efda4a9 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -15,6 +15,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -152,32 +153,21 @@ static size_t gud_xrgb_to_color(u8 *dst, const struct 
drm_format_info *forma
  }
  
  static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,

+ const struct iosys_map *src, bool cached_reads,
  const struct drm_format_info *format, struct drm_rect 
*rect,
  struct gud_set_buffer_req *req)
  {
-   struct dma_buf_attachment *import_attach = fb->obj[0]->import_attach;
u8 compression = gdrm->compression;
-   struct iosys_map map[DRM_FORMAT_MAX_PLANES] = { };
-   struct iosys_map map_data[DRM_FORMAT_MAX_PLANES] = { };
struct iosys_map dst;
void *vaddr, *buf;
size_t pitch, len;
-   int ret = 0;
  
  	pitch = drm_format_info_min_pitch(format, 0, drm_rect_width(rect));

len = pitch * drm_rect_height(rect);
if (len > gdrm->bulk_len)
return -E2BIG;
  
-	ret = drm_gem_fb_vmap(fb, map, map_data);

-   if (ret)
-   return ret;
-
-   vaddr = map_data[0].vaddr;
-
-   ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
-   if (ret)
-   goto vunmap;
+   vaddr = src[0].vaddr;
  retry:
if (compression)
buf = gdrm->compress_buf;
@@ -192,29 +182,27 @@ static int gud_prep_flush(struct gud_device *gdrm, struct 
drm_framebuffer *fb,
if (format != fb->format) {
if (format->format == GUD_DRM_FORMAT_R1) {
len = gud_xrgb_to_r124(buf, format, vaddr, fb, 
rect);
-   if (!len) {
-   ret = -ENOMEM;
-   goto end_cpu_access;
-   }
+   if (!len)
+   return -ENOMEM;
} else if (format->format == DRM_FORMAT_R8) {
-   drm_fb_xrgb_to_gray8(&dst, NULL, map_data, fb, 
rect);
+   drm_fb_xrgb_to_gray8(&dst, NULL, src, fb, rect);
} else if (format->format == DRM_FORMAT_RGB332) {
-   drm_fb_xrgb_to_rgb332(&dst, NULL, map_data, fb, 
rect);
+   drm_fb_xrgb_to_rgb332(&dst, NULL, src, fb, rect);
} else if (format->format == DRM_FORMAT_RGB565) {
-   drm_fb_xrgb_to_rgb565(&dst, NULL, map_data, fb, 
rect,
+   drm_fb_xrgb_to_rgb565(&dst, NULL, src, fb, rect,
  gud_is_big_endian());
} else if (format->format == DRM_FORMAT_RGB888) {
-   drm_fb_xrgb_to_rgb888(&dst, NULL, map_data, fb, 
rect);
+   drm_fb_xrgb_to_rgb888(&dst, NULL, src, fb, rect);
} else {
len = gud_xrgb_to_color(buf, format, vaddr, fb, 
rect);
}
} else if (gud_is_big_endian() && format->cpp[0] > 1) {
-   drm_fb_swab(&dst, NULL, map_data, fb, rect, !import_attach);
-   } else if (compression && !import_attach && pitch == fb->pitches[0]) {
+   drm_fb_swab(&dst, NULL, src, fb, rect, cached_reads);
+   } else if (compression && cached_reads && pitch == fb->pitches[0]) {
/* can compress directly from the framebuffer */
buf = vaddr + rect->y1 * pitch;
} else {
-   drm_fb_memcpy(&dst, NULL, map_data, fb, rect);
+   drm_fb_memcpy(&dst, NULL, src, fb, rect);
}
  
  	memset(req, 0, sizeof(*req));

@@ -237,12 +225,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct 
drm_framebuffer *fb,
req->compressed_length = cpu_to_le32(complen);
}
  
-end_cpu_access:

-   drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-vunmap:
-   drm_gem_fb_vunmap(fb, map);
-
-   return ret;
+   return 0;
  }
  
  struct gud_usb_bulk_context {

@@ -285,6 +268,7 @@ static int gud_usb_bulk(struct gud_device *gdrm, size_t len)
  }
  
  static int gud_flush_rect(struct gud_device *gdrm, struct drm_framebuffer *fb,

+ const struct iosys_map *src, b

Re: [PATCH v2 5/6] drm/gud: Use the shadow plane helper

[Thomas Zimmermann]

Am 30.11.22 um 20:26 schrieb Noralf Trønnes via B4 Submission Endpoint:

From: Noralf Trønnes 

Use the shadow plane helper to take care of mapping the framebuffer for
CPU access. The synchronous flushing is now done inline without the use of
a worker. The async path now uses a shadow buffer to hold framebuffer
changes and it doesn't read the framebuffer behind userspace's back
anymore.

v2:
- Use src as variable name for iosys_map (Thomas)
- Prepare imported buffer for CPU access in the driver (Thomas)

Signed-off-by: Noralf Trønnes 


Reviewed-by: Thomas Zimmermann 


---
  drivers/gpu/drm/gud/gud_drv.c  |  1 +
  drivers/gpu/drm/gud/gud_internal.h |  1 +
  drivers/gpu/drm/gud/gud_pipe.c | 81 ++
  3 files changed, 57 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index d57dab104358..5aac7cda0505 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -365,6 +365,7 @@ static void gud_debugfs_init(struct drm_minor *minor)
  static const struct drm_simple_display_pipe_funcs gud_pipe_funcs = {
.check  = gud_pipe_check,
.update = gud_pipe_update,
+   DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS
  };
  
  static const struct drm_mode_config_funcs gud_mode_config_funcs = {

diff --git a/drivers/gpu/drm/gud/gud_internal.h 
b/drivers/gpu/drm/gud/gud_internal.h
index e351a1f1420d..0d148a6f27aa 100644
--- a/drivers/gpu/drm/gud/gud_internal.h
+++ b/drivers/gpu/drm/gud/gud_internal.h
@@ -43,6 +43,7 @@ struct gud_device {
struct drm_framebuffer *fb;
struct drm_rect damage;
bool prev_flush_failed;
+   void *shadow_buf;
  };
  
  static inline struct gud_device *to_gud_device(struct drm_device *drm)

diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index 98fe8efda4a9..92189474a7ed 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -358,10 +358,10 @@ static void gud_flush_damage(struct gud_device *gdrm, 
struct drm_framebuffer *fb
  void gud_flush_work(struct work_struct *work)
  {
struct gud_device *gdrm = container_of(work, struct gud_device, work);
-   struct iosys_map gem_map = { }, fb_map = { };
+   struct iosys_map shadow_map;
struct drm_framebuffer *fb;
struct drm_rect damage;
-   int idx, ret;
+   int idx;
  
  	if (!drm_dev_enter(&gdrm->drm, &idx))

return;
@@ -369,6 +369,7 @@ void gud_flush_work(struct work_struct *work)
mutex_lock(&gdrm->damage_lock);
fb = gdrm->fb;
gdrm->fb = NULL;
+   iosys_map_set_vaddr(&shadow_map, gdrm->shadow_buf);
damage = gdrm->damage;
gud_clear_damage(gdrm);
mutex_unlock(&gdrm->damage_lock);
@@ -376,33 +377,33 @@ void gud_flush_work(struct work_struct *work)
if (!fb)
goto out;
  
-	ret = drm_gem_fb_vmap(fb, &gem_map, &fb_map);

-   if (ret)
-   goto fb_put;
+   gud_flush_damage(gdrm, fb, &shadow_map, true, &damage);
  
-	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);

-   if (ret)
-   goto vunmap;
-
-   /* Imported buffers are assumed to be WriteCombined with uncached reads 
*/
-   gud_flush_damage(gdrm, fb, &fb_map, !fb->obj[0]->import_attach, 
&damage);
-
-   drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-vunmap:
-   drm_gem_fb_vunmap(fb, &gem_map);
-fb_put:
drm_framebuffer_put(fb);
  out:
drm_dev_exit(idx);
  }
  
-static void gud_fb_queue_damage(struct gud_device *gdrm, struct drm_framebuffer *fb,

-   struct drm_rect *damage)
+static int gud_fb_queue_damage(struct gud_device *gdrm, struct drm_framebuffer 
*fb,
+  const struct iosys_map *src, struct drm_rect 
*damage)
  {
struct drm_framebuffer *old_fb = NULL;
+   struct iosys_map shadow_map;
  
  	mutex_lock(&gdrm->damage_lock);
  
+	if (!gdrm->shadow_buf) {

+   gdrm->shadow_buf = vzalloc(fb->pitches[0] * fb->height);
+   if (!gdrm->shadow_buf) {
+   mutex_unlock(&gdrm->damage_lock);
+   return -ENOMEM;
+   }
+   }
+
+   iosys_map_set_vaddr(&shadow_map, gdrm->shadow_buf);
+   iosys_map_incr(&shadow_map, drm_fb_clip_offset(fb->pitches[0], 
fb->format, damage));
+   drm_fb_memcpy(&shadow_map, fb->pitches, src, fb, damage);
+
if (fb != gdrm->fb) {
old_fb = gdrm->fb;
drm_framebuffer_get(fb);
@@ -420,6 +421,26 @@ static void gud_fb_queue_damage(struct gud_device *gdrm, 
struct drm_framebuffer
  
  	if (old_fb)

drm_framebuffer_put(old_fb);
+
+   return 0;
+}
+
+static void gud_fb_handle_damage(struct gud_device *gdrm, struct 
drm_framebuffer *fb,
+const struct iosys_map *src, struct drm_rect 
*damage)
+{
+   int ret;
+
+   if

Re: [PATCH v2 6/6] drm/gud: Enable synchronous flushing by default

[Thomas Zimmermann]

Am 30.11.22 um 20:26 schrieb Noralf Trønnes via B4 Submission Endpoint:

From: Noralf Trønnes 

gud has a module parameter that controls whether framebuffer flushing
happens synchronously during the commit or asynchronously in a worker.

GNOME before version 3.38 handled all displays in the same rendering loop.
This lead to gud slowing down the refresh rate for a faster monitor. This
has now been fixed so lets change the default.

The plan is to remove async flushing in the future. The code is now
structured in a way that makes it easy to do this.

Link: https://blogs.gnome.org/shell-dev/2020/07/02/splitting-up-the-frame-clock/
Suggested-by: Thomas Zimmermann 
Signed-off-by: Noralf Trønnes 


Reviewed-by: Thomas Zimmermann 


---
  drivers/gpu/drm/gud/gud_pipe.c | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index 92189474a7ed..62c43d3632d4 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -25,17 +25,13 @@
  #include "gud_internal.h"
  
  /*

- * Some userspace rendering loops runs all displays in the same loop.
+ * Some userspace rendering loops run all displays in the same loop.
   * This means that a fast display will have to wait for a slow one.
- * For this reason gud does flushing asynchronous by default.
- * The down side is that in e.g. a single display setup userspace thinks
- * the display is insanely fast since the driver reports back immediately
- * that the flush/pageflip is done. This wastes CPU and power.
- * Such users might want to set this module parameter to false.
+ * Such users might want to enable this module parameter.
   */
-static bool gud_async_flush = true;
+static bool gud_async_flush;
  module_param_named(async_flush, gud_async_flush, bool, 0644);
-MODULE_PARM_DESC(async_flush, "Enable asynchronous flushing [default=true]");
+MODULE_PARM_DESC(async_flush, "Enable asynchronous flushing [default=0]");
  
  /*

   * FIXME: The driver is probably broken on Big Endian machines.



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


OpenPGP_signature
Description: OpenPGP digital signature

Re: [Intel-gfx] [PATCH v4 3/5] drm/i915: Introduce guard pages to i915_vma

[Tvrtko Ursulin]

On 30/11/2022 23:58, Andi Shyti wrote:

From: Chris Wilson 

Introduce the concept of padding the i915_vma with guard pages before
and after. The major consequence is that all ordinary uses of i915_vma
must use i915_vma_offset/i915_vma_size and not i915_vma.node.start/size
directly, as the drm_mm_node will include the guard pages that surround
our object.

The biggest connundrum is how exactly to mix requesting a fixed address
with guard pages, particularly through the existing uABI. The user does
not know about guard pages, so such must be transparent to the user, and
so the execobj.offset must be that of the object itself excluding the
guard. So a PIN_OFFSET_FIXED must then be exclusive of the guard pages.
The caveat is that some placements will be impossible with guard pages,
as wrap arounds need to be avoided, and the vma itself will require a
larger node. We must not report EINVAL but ENOSPC as these are unavailable
locations within the GTT rather than conflicting user requirements.

In the next patch, we start using guard pages for scanout objects. While
these are limited to GGTT vma, on a few platforms these vma (or at least
an alias of the vma) is shared with userspace, so we may leak the
existence of such guards if we are not careful to ensure that the
execobj.offset is transparent and excludes the guards. (On such platforms
like ivb, without full-ppgtt, userspace has to use relocations so the
presence of more untouchable regions within its GTT such be of no further
issue.)

Signed-off-by: Chris Wilson 
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Tvrtko Ursulin 
Signed-off-by: Andi Shyti 
---
  drivers/gpu/drm/i915/gt/intel_ggtt.c | 14 ++---
  drivers/gpu/drm/i915/i915_gem_gtt.h  |  3 +-
  drivers/gpu/drm/i915/i915_vma.c  | 40 +++-
  drivers/gpu/drm/i915/i915_vma.h  |  5 +--
  drivers/gpu/drm/i915/i915_vma_resource.c |  4 +--
  drivers/gpu/drm/i915/i915_vma_resource.h |  7 -
  drivers/gpu/drm/i915/i915_vma_types.h|  1 +
  7 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 7644738b9cdbe..784d4a8c43ba9 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -296,8 +296,11 @@ static void gen8_ggtt_insert_entries(struct 
i915_address_space *vm,
 */
  
  	gte = (gen8_pte_t __iomem *)ggtt->gsm;

-   gte += vma_res->start / I915_GTT_PAGE_SIZE;
-   end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
+   gte += (vma_res->start - vma_res->guard) / I915_GTT_PAGE_SIZE;
+   end = gte + vma_res->guard / I915_GTT_PAGE_SIZE;
+   while (gte < end)
+   gen8_set_pte(gte++, vm->scratch[0]->encode);
+   end += (vma_res->node_size + vma_res->guard) / I915_GTT_PAGE_SIZE;
  
  	for_each_sgt_daddr(addr, iter, vma_res->bi.pages)

gen8_set_pte(gte++, pte_encode | addr);
@@ -347,9 +350,12 @@ static void gen6_ggtt_insert_entries(struct 
i915_address_space *vm,
dma_addr_t addr;
  
  	gte = (gen6_pte_t __iomem *)ggtt->gsm;

-   gte += vma_res->start / I915_GTT_PAGE_SIZE;
-   end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
+   gte += (vma_res->start - vma_res->guard) / I915_GTT_PAGE_SIZE;
  
+	end = gte + vma_res->guard / I915_GTT_PAGE_SIZE;

+   while (gte < end)
+   iowrite32(vm->scratch[0]->encode, gte++);
+   end += (vma_res->node_size + vma_res->guard) / I915_GTT_PAGE_SIZE;
for_each_sgt_daddr(addr, iter, vma_res->bi.pages)
iowrite32(vm->pte_encode(addr, level, flags), gte++);
GEM_BUG_ON(gte > end);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8c2f57eb5ddaa..2434197830523 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -44,7 +44,8 @@ int i915_gem_gtt_insert(struct i915_address_space *vm,
  #define PIN_HIGH  BIT_ULL(5)
  #define PIN_OFFSET_BIAS   BIT_ULL(6)
  #define PIN_OFFSET_FIXED  BIT_ULL(7)
-#define PIN_VALIDATE   BIT_ULL(8) /* validate placement only, no need 
to call unpin() */
+#define PIN_OFFSET_GUARD   BIT_ULL(8)
+#define PIN_VALIDATE   BIT_ULL(9) /* validate placement only, no need 
to call unpin() */
  
  #define PIN_GLOBAL		BIT_ULL(10) /* I915_VMA_GLOBAL_BIND */

  #define PIN_USER  BIT_ULL(11) /* I915_VMA_LOCAL_BIND */
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index fefee5fef38d3..3877847179710 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -419,7 +419,7 @@ i915_vma_resource_init_from_vma(struct i915_vma_resource 
*vma_res,
   obj->mm.rsgt, i915_gem_object_is_readonly(obj),
   i915_gem_object_is_lmem(obj), obj->mm.region,
   vma->ops, vma->private, __i915_vma_offset(vma),
-

Re: [Intel-gfx] [PATCH v4 4/5] drm/i915: Refine VT-d scanout workaround

[Tvrtko Ursulin]

On 30/11/2022 23:58, Andi Shyti wrote:

From: Chris Wilson 

VT-d may cause overfetch of the scanout PTE, both before and after the
vma (depending on the scanout orientation). bspec recommends that we
provide a tile-row in either directions, and suggests using 168 PTE,
warning that the accesses will wrap around the ends of the GGTT.
Currently, we fill the entire GGTT with scratch pages when using VT-d to
always ensure there are valid entries around every vma, including
scanout. However, writing every PTE is slow as on recent devices we
perform 8MiB of uncached writes, incurring an extra 100ms during resume.

If instead we focus on only putting guard pages around scanout, we can
avoid touching the whole GGTT. To avoid having to introduce extra nodes
around each scanout vma, we adjust the scanout drm_mm_node to be smaller
than the allocated space, and fixup the extra PTE during dma binding.

Signed-off-by: Chris Wilson 
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Tvrtko Ursulin 
Signed-off-by: Andi Shyti 
---
  drivers/gpu/drm/i915/gem/i915_gem_domain.c | 13 +++
  drivers/gpu/drm/i915/gt/intel_ggtt.c   | 25 +-
  2 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c 
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 850776a783ac7..9969e687ad857 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -17,6 +17,8 @@
  #include "i915_gem_object.h"
  #include "i915_vma.h"
  
+#define VTD_GUARD (168u * I915_GTT_PAGE_SIZE) /* 168 or tile-row PTE padding */

+
  static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj)
  {
struct drm_i915_private *i915 = to_i915(obj->base.dev);
@@ -424,6 +426,17 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
if (ret)
return ERR_PTR(ret);
  
+	/* VT-d may overfetch before/after the vma, so pad with scratch */

+   if (intel_scanout_needs_vtd_wa(i915)) {
+   unsigned int guard = VTD_GUARD;
+
+   if (i915_gem_object_is_tiled(obj))
+   guard = max(guard,
+   i915_gem_object_get_tile_row_size(obj));
+
+   flags |= PIN_OFFSET_GUARD | guard;
+   }
+
/*
 * As the user may map the buffer once pinned in the display plane
 * (e.g. libkms for the bootup splash), we have to ensure that we
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 784d4a8c43ba9..fa96d925c2596 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -376,27 +376,6 @@ static void nop_clear_range(struct i915_address_space *vm,
  {
  }
  
-static void gen8_ggtt_clear_range(struct i915_address_space *vm,

- u64 start, u64 length)
-{
-   struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
-   unsigned int first_entry = start / I915_GTT_PAGE_SIZE;
-   unsigned int num_entries = length / I915_GTT_PAGE_SIZE;
-   const gen8_pte_t scratch_pte = vm->scratch[0]->encode;
-   gen8_pte_t __iomem *gtt_base =
-   (gen8_pte_t __iomem *)ggtt->gsm + first_entry;
-   const int max_entries = ggtt_total_entries(ggtt) - first_entry;
-   int i;
-
-   if (WARN(num_entries > max_entries,
-"First entry = %d; Num entries = %d (max=%d)\n",
-first_entry, num_entries, max_entries))
-   num_entries = max_entries;
-
-   for (i = 0; i < num_entries; i++)
-   gen8_set_pte(>t_base[i], scratch_pte);
-}
-
  static void bxt_vtd_ggtt_wa(struct i915_address_space *vm)
  {
/*
@@ -968,8 +947,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
ggtt->vm.cleanup = gen6_gmch_remove;
ggtt->vm.insert_page = gen8_ggtt_insert_page;
ggtt->vm.clear_range = nop_clear_range;
-   if (intel_scanout_needs_vtd_wa(i915))
-   ggtt->vm.clear_range = gen8_ggtt_clear_range;
  
  	ggtt->vm.insert_entries = gen8_ggtt_insert_entries;
  
@@ -1128,7 +1105,7 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)

ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
  
  	ggtt->vm.clear_range = nop_clear_range;

-   if (!HAS_FULL_PPGTT(i915) || intel_scanout_needs_vtd_wa(i915))
+   if (!HAS_FULL_PPGTT(i915))
ggtt->vm.clear_range = gen6_ggtt_clear_range;
ggtt->vm.insert_page = gen6_ggtt_insert_page;
ggtt->vm.insert_entries = gen6_ggtt_insert_entries;


Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko

Re: [Intel-gfx] [PATCH v4 5/5] Revert "drm/i915: Improve on suspend / resume time with VT-d enabled"

[Tvrtko Ursulin]

On 30/11/2022 23:58, Andi Shyti wrote:

This reverts commit 2ef6efa79fecd5e3457b324155d35524d95f2b6b.

Checking the presence if the IRST (Intel Rapid Start Technology)
through the ACPI to decide whether to rebuild or not the GGTT
puts us at the mercy of the boot firmware and we need to
unnecessarily rely on third parties.

Because now we avoid adding scratch pages to the entire GGTT we
don't need this hack anymore.

Signed-off-by: Andi Shyti 
Cc: Thomas Hellström 
Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
---
  drivers/gpu/drm/i915/gt/intel_ggtt.c | 69 ++--
  drivers/gpu/drm/i915/gt/intel_gtt.h  | 24 --
  drivers/gpu/drm/i915/i915_driver.c   | 16 ---
  3 files changed, 13 insertions(+), 96 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index fa96d925c2596..5896ac44010b0 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -27,13 +27,6 @@
  #include "intel_gtt.h"
  #include "gen8_ppgtt.h"
  
-static inline bool suspend_retains_ptes(struct i915_address_space *vm)

-{
-   return GRAPHICS_VER(vm->i915) >= 8 &&
-   !HAS_LMEM(vm->i915) &&
-   vm->is_ggtt;
-}
-
  static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
   unsigned long color,
   u64 *start,
@@ -105,23 +98,6 @@ int i915_ggtt_init_hw(struct drm_i915_private *i915)
return 0;
  }
  
-/*

- * Return the value of the last GGTT pte cast to an u64, if
- * the system is supposed to retain ptes across resume. 0 otherwise.
- */
-static u64 read_last_pte(struct i915_address_space *vm)
-{
-   struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
-   gen8_pte_t __iomem *ptep;
-
-   if (!suspend_retains_ptes(vm))
-   return 0;
-
-   GEM_BUG_ON(GRAPHICS_VER(vm->i915) < 8);
-   ptep = (typeof(ptep))ggtt->gsm + (ggtt_total_entries(ggtt) - 1);
-   return readq(ptep);
-}
-
  /**
   * i915_ggtt_suspend_vm - Suspend the memory mappings for a GGTT or DPT VM
   * @vm: The VM to suspend the mappings for
@@ -185,10 +161,7 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
i915_gem_object_unlock(obj);
}
  
-	if (!suspend_retains_ptes(vm))

-   vm->clear_range(vm, 0, vm->total);
-   else
-   i915_vm_to_ggtt(vm)->probed_pte = read_last_pte(vm);
+   vm->clear_range(vm, 0, vm->total);
  
  	vm->skip_pte_rewrite = save_skip_rewrite;
  
@@ -545,8 +518,6 @@ static int init_ggtt(struct i915_ggtt *ggtt)

struct drm_mm_node *entry;
int ret;
  
-	ggtt->pte_lost = true;

-
/*
 * GuC requires all resources that we're sharing with it to be placed in
 * non-WOPCM memory. If GuC is not present or not in use we still need a
@@ -1263,20 +1234,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
  {
struct i915_vma *vma;
bool write_domain_objs = false;
-   bool retained_ptes;
  
  	drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt);
  
-	/*

-* First fill our portion of the GTT with scratch pages if
-* they were not retained across suspend.
-*/
-   retained_ptes = suspend_retains_ptes(vm) &&
-   !i915_vm_to_ggtt(vm)->pte_lost &&
-   !GEM_WARN_ON(i915_vm_to_ggtt(vm)->probed_pte != 
read_last_pte(vm));
-
-   if (!retained_ptes)
-   vm->clear_range(vm, 0, vm->total);
+   /* First fill our portion of the GTT with scratch pages */
+   vm->clear_range(vm, 0, vm->total);
  
  	/* clflush objects bound into the GGTT and rebind them. */

list_for_each_entry(vma, &vm->bound_list, vm_link) {
@@ -1285,16 +1247,16 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
atomic_read(&vma->flags) & I915_VMA_BIND_MASK;
  
  		GEM_BUG_ON(!was_bound);

-   if (!retained_ptes) {
-   /*
-* Clear the bound flags of the vma resource to allow
-* ptes to be repopulated.
-*/
-   vma->resource->bound_flags = 0;
-   vma->ops->bind_vma(vm, NULL, vma->resource,
-  obj ? obj->cache_level : 0,
-  was_bound);
-   }
+
+   /*
+* Clear the bound flags of the vma resource to allow
+* ptes to be repopulated.
+*/
+   vma->resource->bound_flags = 0;
+   vma->ops->bind_vma(vm, NULL, vma->resource,
+  obj ? obj->cache_level : 0,
+  was_bound);
+
if (obj) { /* only used during resume => exclusive access */
write_domain_objs |= fetch_and_zero(&obj->write_domain);
obj->read_domains 

[PATCH] drm/tests: probe_helper: Fix unitialized variable

[Maxime Ripard]

The len variable is used while uninitialized. Initialize it.

Fixes: 1e4a91db109f ("drm/probe-helper: Provide a TV get_modes helper")
Reported-by: kernel test robot 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/tests/drm_probe_helper_test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tests/drm_probe_helper_test.c 
b/drivers/gpu/drm/tests/drm_probe_helper_test.c
index 7e938258c742..211131405500 100644
--- a/drivers/gpu/drm/tests/drm_probe_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_probe_helper_test.c
@@ -115,6 +115,7 @@ drm_test_connector_helper_tv_get_modes_check(struct kunit 
*test)
ret = drm_connector_helper_tv_get_modes(connector);
KUNIT_EXPECT_EQ(test, ret, params->num_expected_modes);
 
+   len = 0;
list_for_each_entry(mode, &connector->probed_modes, head)
len++;
KUNIT_EXPECT_EQ(test, len, params->num_expected_modes);
-- 
2.38.1

Re: [PATCH] drm/etnaviv: print MMU exception cause

[Lucas Stach]

Hi Philipp,

Am Donnerstag, dem 01.12.2022 um 09:40 +0100 schrieb Philipp Zabel:
> On Mi, 2022-11-30 at 19:53 +0100, Lucas Stach wrote:
> From: Christian Gmeiner 
> 
> The MMU tells us the fault status. While the raw register value is
> already printed, it's a bit more user friendly to translate the
> fault reasons into human readable format.
> 
> Signed-off-by: Christian Gmeiner 
> Signed-off-by: Lucas Stach 
> ---
> I've rewritten parts of the patch to properly cover multiple
> MMUs and squashed the reason into the existing message. Christian,
> please tell me if you are fine with having your name attached to
> this patch.
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 22 +++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 37018bc55810..f79203b774d9 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1426,6 +1426,15 @@ static void sync_point_worker(struct work_struct *work)
>  
> 
>  static void dump_mmu_fault(struct etnaviv_gpu *gpu)
>  {
> + static const char *fault_reasons[] = {
> + "slave not present",
> + "page not present",
> + "write violation",
> + "out of bounds",
> + "read security violation",
> + "write security violation",
> + };
> +
>   u32 status_reg, status;
>   int i;
>  
> 
> @@ -1438,18 +1447,25 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu)
>   dev_err_ratelimited(gpu->dev, "MMU fault status 0x%08x\n", status);
>  
> 
>   for (i = 0; i < 4; i++) {
> + const char *reason = "unknown";
>   u32 address_reg;
> + u32 mmu_status;
>  
> 
> - if (!(status & (VIVS_MMUv2_STATUS_EXCEPTION0__MASK << (i * 4
> + mmu_status = (status >> (i * 4)) & 
> VIVS_MMUv2_STATUS_EXCEPTION0__MASK;
> 
> VIVS_MMUv2_STATUS_EXCEPTION0__MASK is 0x3 ...
> 
> + if (!mmu_status)
>   continue;
>  
> 
> + if ((mmu_status - 1) < ARRAY_SIZE(fault_reasons))
> + reason = fault_reasons[mmu_status - 1];
> 
Your mail quoting seems to be broken, again.

> ... so (mmu_status - 1) can be 2 at most. This leaves me wondering how
> "out of bounds" and the "security violation" errors can be reached. I
> think this requires the exception bitfield masks to be extended to 0x7.

Good catch! That's a inconsistency in rnndb, where we claim to be able
to stuff the full exception enum into 2 bits. Will fix!

Regards,
Lucas

Re: [PATCH v2 4/7] arm64: dts: renesas: r8a779g0: Add display related nodes

[Geert Uytterhoeven]

On Wed, Nov 23, 2022 at 8:00 AM Tomi Valkeinen
 wrote:
> Add DT nodes for components needed to get the DSI output working:
> - FCPv
> - VSPd
> - DU
> - DSI
>
> Signed-off-by: Tomi Valkeinen 
> Reviewed-by: Kieran Bingham 
> Reviewed-by: Laurent Pinchart 

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in renesas-devel for v6.3.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH v4 0/7] Support for the NPU in Vim3

[Tomeu Vizoso]

Hi,

This series adds support for the Verisilicon VIPNano-QI NPU in the A311D
as in the VIM3 board.

The IP is very closely based on previous Vivante GPUs, so the etnaviv
kernel driver works basically unchanged.

The userspace part of the driver is being reviewed at:

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18986

v2: Move reference to RESET_NNA to npu node (Neil)
v3: Fix indentation mistake (Neil)
v4: Add warning when etnaviv probes on a NPU

Regards,

Tomeu

Tomeu Vizoso (7):
  dt-bindings: reset: meson-g12a: Add missing NNA reset
  dt-bindings: power: Add G12A NNA power domain
  soc: amlogic: meson-pwrc: Add NNA power domain for A311D
  arm64: dts: Add DT node for the VIPNano-QI on the A311D
  drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055
  drm/etnaviv: Add nn_core_count to chip feature struct
  drm/etnaviv: Warn when probing on NPUs

 .../boot/dts/amlogic/meson-g12-common.dtsi| 11 ++
 .../amlogic/meson-g12b-a311d-khadas-vim3.dts  |  4 +++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c |  4 +++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h |  3 ++
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c| 36 +++
 drivers/soc/amlogic/meson-ee-pwrc.c   | 17 +
 include/dt-bindings/power/meson-g12a-power.h  |  1 +
 .../reset/amlogic,meson-g12a-reset.h  |  4 ++-
 8 files changed, 79 insertions(+), 1 deletion(-)

-- 
2.38.1

[PATCH v4 5/7] drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055

[Tomeu Vizoso]

This is a compute-only module marketed towards AI and vision
acceleration. This particular version can be found on the Amlogic A311D
SoC.

The feature bits are taken from the Khadas downstream kernel driver
6.4.4.3.310723AAA.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 31 ++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c 
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index f2fc645c7956..3f6fd9a3c088 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -130,6 +130,37 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.minor_features10 = 0x90044250,
.minor_features11 = 0x0024,
},
+   {
+   .model = 0x8000,
+   .revision = 0x7120,
+   .product_id = 0x45080009,
+   .customer_id = 0x88,
+   .eco_id = 0,
+   .stream_count = 8,
+   .register_max = 64,
+   .thread_count = 256,
+   .shader_core_count = 1,
+   .vertex_cache_size = 16,
+   .vertex_output_buffer_size = 1024,
+   .pixel_pipes = 1,
+   .instruction_count = 512,
+   .num_constants = 320,
+   .buffer_size = 0,
+   .varyings_count = 16,
+   .features = 0xe0287cac,
+   .minor_features0 = 0xc1799eff,
+   .minor_features1 = 0xfefbfadb,
+   .minor_features2 = 0xeb9d6fbf,
+   .minor_features3 = 0xedfffced,
+   .minor_features4 = 0xd30dafc7,
+   .minor_features5 = 0x7b5ac333,
+   .minor_features6 = 0xfc8ee200,
+   .minor_features7 = 0x03fffa6f,
+   .minor_features8 = 0x00fe0ef0,
+   .minor_features9 = 0x0088003c,
+   .minor_features10 = 0x108048c0,
+   .minor_features11 = 0x0010,
+   },
 };
 
 bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
-- 
2.38.1

[PATCH v4 7/7] drm/etnaviv: Warn when probing on NPUs

[Tomeu Vizoso]

Userspace is still not making full use of the hardware, so we don't know
yet if changes to the UAPI won't be needed. Warn about it.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..3cbc82bbf8d4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -765,6 +765,10 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
goto fail;
}
 
+   if (gpu->identity.nn_core_count > 0)
+   dev_warn(gpu->dev, "etnaviv has been instantiated on a NPU, "
+   "for which the UAPI is still 
experimental\n");
+
/* Exclude VG cores with FE2.0 */
if (gpu->identity.features & chipFeatures_PIPE_VG &&
gpu->identity.features & chipFeatures_FE20) {
-- 
2.38.1

[PATCH v4 6/7] drm/etnaviv: Add nn_core_count to chip feature struct

[Tomeu Vizoso]

We will use these for differentiating between GPUs and NPUs, as the
downstream driver does.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h  | 3 +++
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 5 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 85eddd492774..c8f3ad2031ce 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -50,6 +50,9 @@ struct etnaviv_chip_identity {
/* Number of shader cores. */
u32 shader_core_count;
 
+   /* Number of Neural Network cores. */
+   u32 nn_core_count;
+
/* Size of the vertex cache. */
u32 vertex_cache_size;
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c 
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index 3f6fd9a3c088..9fc5223299e4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -16,6 +16,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 128,
.shader_core_count = 1,
+   .nn_core_count = 0,
.vertex_cache_size = 8,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -47,6 +48,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 512,
.shader_core_count = 2,
+   .nn_core_count = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -78,6 +80,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 512,
.shader_core_count = 2,
+   .nn_core_count = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -109,6 +112,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 1024,
.shader_core_count = 4,
+   .nn_core_count = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 2,
@@ -140,6 +144,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 256,
.shader_core_count = 1,
+   .nn_core_count = 8,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
-- 
2.38.1

Re: [PATCH v3 5/5] drm/i915/mtl: Hold forcewake and MCR lock over PPAT setup

[Balasubramani Vivekanandan]

On 30.11.2022 07:58, Matt Roper wrote:
> PPAT setup involves a series of multicast writes.  This can be optimized
> slightly be acquiring forcewake and the steering lock just once for the
> entire sequence.
> 
> v2:
>  - We should use FW_REG_WRITE instead of FW_REG_READ.  (Bala)
> 
> Suggested-by: Balasubramani Vivekanandan 
> 
> Signed-off-by: Matt Roper 

Reviewed-by: Balasubramani Vivekanandan 

Regards,
Bala

> ---
>  drivers/gpu/drm/i915/gt/intel_gtt.c | 27 +++
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
> b/drivers/gpu/drm/i915/gt/intel_gtt.c
> index 2ba3983984b9..e37164a60d37 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> @@ -482,14 +482,25 @@ static void tgl_setup_private_ppat(struct intel_uncore 
> *uncore)
>  
>  static void xehp_setup_private_ppat(struct intel_gt *gt)
>  {
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(0), GEN8_PPAT_WB);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(1), GEN8_PPAT_WC);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(2), GEN8_PPAT_WT);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(3), GEN8_PPAT_UC);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(4), GEN8_PPAT_WB);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(5), GEN8_PPAT_WB);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(6), GEN8_PPAT_WB);
> - intel_gt_mcr_multicast_write(gt, XEHP_PAT_INDEX(7), GEN8_PPAT_WB);
> + enum forcewake_domains fw;
> + unsigned long flags;
> +
> + fw = intel_uncore_forcewake_for_reg(gt->uncore, 
> _MMIO(XEHP_PAT_INDEX(0).reg),
> + FW_REG_WRITE);
> + intel_uncore_forcewake_get(gt->uncore, fw);
> +
> + intel_gt_mcr_lock(gt, &flags);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(0), GEN8_PPAT_WB);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(1), GEN8_PPAT_WC);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(2), GEN8_PPAT_WT);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(3), GEN8_PPAT_UC);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(4), GEN8_PPAT_WB);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(5), GEN8_PPAT_WB);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(6), GEN8_PPAT_WB);
> + intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(7), GEN8_PPAT_WB);
> + intel_gt_mcr_unlock(gt, flags);
> +
> + intel_uncore_forcewake_put(gt->uncore, fw);
>  }
>  
>  static void icl_setup_private_ppat(struct intel_uncore *uncore)
> -- 
> 2.38.1
> 

Re: [PATCH v2 3/7] clk: renesas: r8a779g0: Add display related clocks

[Geert Uytterhoeven]

Hi Tomi,

On Thu, Dec 1, 2022 at 10:06 AM Tomi Valkeinen
 wrote:
> On 30/11/2022 21:18, Geert Uytterhoeven wrote:
> > On Wed, Nov 23, 2022 at 8:00 AM Tomi Valkeinen
> >  wrote:
> >> Add clocks related to display which are needed to get the DSI output
> >> working.
> >>
> >> Extracted from Renesas BSP tree.
> >>
> >> Signed-off-by: Tomi Valkeinen 
> >> Reviewed-by: Kieran Bingham 
> >> Reviewed-by: Laurent Pinchart 

> > Not that all of this matters a lot: all of these parents are always-on,
> > and I think "dis0" is the only clock where we care about the actual
> > clock rate?
>
> No, of the clocks added above, in the drivers we only care about the
> dsiref rate. That's used for the DSI PLL, and that PLL is used as the
> DU's pclk.

IC. As the DU node has only a single clocks property, I thought that
clock is used to derive the pixel clock from.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v5 1/2] drm/fourcc: Add Synaptics VideoSmart tiled modifiers

[Daniel Vetter]

On Thu, Dec 01, 2022 at 12:49:16AM +0800, Randy Li wrote:
> 
> 
> Sent from my iPad
> 
> > On Nov 30, 2022, at 7:30 PM, Daniel Vetter  wrote:
> > 
> > CAUTION: Email originated externally, do not click links or open 
> > attachments unless you recognize the sender and know the content is safe.
> > 
> > 
> >> On Wed, Nov 30, 2022 at 05:21:48PM +0800, Hsia-Jun Li wrote:
> >> From: "Hsia-Jun(Randy) Li" 
> >> 
> >> Those modifiers only record the parameters would effort pixel
> >> layout or memory layout. Whether physical memory page mapping
> >> is used is not a part of format.
> >> 
> >> Signed-off-by: Hsia-Jun(Randy) Li 
> >> ---
> >> include/uapi/drm/drm_fourcc.h | 76 +++
> >> 1 file changed, 76 insertions(+)
> >> 
> >> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> >> index bc056f2d537d..e0905f573f43 100644
> >> --- a/include/uapi/drm/drm_fourcc.h
> >> +++ b/include/uapi/drm/drm_fourcc.h
> >> @@ -407,6 +407,7 @@ extern "C" {
> >> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
> >> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
> >> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
> >> +#define DRM_FORMAT_MOD_VENDOR_SYNAPTICS 0x0b
> >> 
> >> /* add more to the end as needed */
> >> 
> >> @@ -1507,6 +1508,81 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 
> >> modifier)
> >> #define AMD_FMT_MOD_CLEAR(field) \
> >>  (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
> >> 
> >> +/*
> >> + * Synaptics VideoSmart modifiers
> >> + *
> >> + * Tiles could be arranged in Groups of Tiles (GOTs), it is a small tile
> >> + * within a tile. GOT size and layout varies based on platform and
> >> + * performance concern.
> >> + *
> >> + * Besides, an 8 length 4 bytes arrary (32 bytes) would be need to store
> >> + * some compression parameters for a compression metadata plane.
> >> + *
> >> + * Further information can be found in
> >> + * Documentation/gpu/synaptics.rst
> >> + *
> >> + *   Macro
> >> + * Bits  Param Description
> >> + *   - 
> >> -
> >> + *
> >> + *  7:0  f Scan direction description.
> >> + *
> >> + *   0 = Invalid
> >> + *   1 = V4, the scan would always start from vertical for 4 
> >> pixel
> >> + *   then move back to the start pixel of the next 
> >> horizontal
> >> + *   direction.
> >> + *   2 = Reserved for future use.
> >> + *
> >> + * 15:8  m The times of pattern repeat in the right angle direction 
> >> from
> >> + * the first scan direction.
> >> + *
> >> + * 19:16 p The padding bits after the whole scan, could be zero.
> >> + *
> >> + * 20:20 g GOT packing flag.
> >> + *
> >> + * 23:21 - Reserved for future use.  Must be zero.
> > 
> > Can you pls fold all the future use reservations into the top end?
> You see we could put more related flag in each of reserved area.
> Here is for the group of tiles flag.
> Bit 35 to 32 could be used for describing the dimension of the group of tiles.

Oh also on the dimension thing, this is the tile size and has nothing to
do with the overall buffer size, right? Because the overall buffer size is
meant to be carried in separate metadata (like the drm_framebuffer
structure or ADDFB2 ioctl data). drm fourcc/modifier assume that height,
width, offset and stride are specified per plane already (unless the
auxiary plane has a fixed layout and is not tracked as a separate plane
for this format).

> > Also I
> > think it'd be good to at least reserve maybe the top 8 bits or so for a
> > synaptics specific format indicator, so that it's easier to extend this in
> > the future ...
> I think the  bit 56 to 63 are used for storing the vendor id. That is why I 
> didn’t include them below. Or you mean the bit 7 to 0?
> Do yo

Yeah there's 8 bit vendor id, but you could reserve another 8 bit at the
top (so 48:55 or something like that) to enumerate within the synaptics
space. Just to future proof the schema, because experience says that hw
engineers absolutely do love to change this stuff eventually.
-Daniel

> > -Daniel
> > 
> > 
> >> + *
> >> + * 27:24 h log2(horizontal) of pixels, in GOTs.
> >> + *
> >> + * 31:28 v log2(vertical) of pixels, in GOTs.
> >> + *
> >> + * 35:32 - Reserved for future use.  Must be zero.
> >> + *
> >> + * 36:36 c Compression flag.
> >> + *
> >> + * 55:37 - Reserved for future use.  Must be zero.
> >> + *
> >> + */
> >> +
> >> +#define DRM_FORMAT_MOD_SYNA_V4_TILED fourcc_mod_code(SYNAPTICS, 1)
> >> +
> >> +#define DRM_FORMAT_MOD_SYNA_MTR_LINEAR_2D(f, m, p, g, h, v, c) \
> >> + fourcc_mod_code(SYNAPTICS, ((__u64)((f) & 0xff) | \
> >> +  ((__u64)((m) & 0xff) << 8) | \
> >> +  ((__u64)((p) & 0xf) << 16) | \
> >> +  ((__u64)((g) & 0x1) << 20) | \
> >> +  (

Re: [PATCH v2 3/7] clk: renesas: r8a779g0: Add display related clocks

[Geert Uytterhoeven]

Hi Tomi,

On Thu, Dec 1, 2022 at 10:26 AM Tomi Valkeinen
 wrote:
> On 30/11/2022 21:18, Geert Uytterhoeven wrote:
> > On Wed, Nov 23, 2022 at 8:00 AM Tomi Valkeinen
> >  wrote:
> >> Add clocks related to display which are needed to get the DSI output
> >> working.
> >>
> >> Extracted from Renesas BSP tree.
> >>
> >> Signed-off-by: Tomi Valkeinen 
> >> Reviewed-by: Kieran Bingham 
> >> Reviewed-by: Laurent Pinchart 

> >> --- a/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> >> +++ b/drivers/clk/renesas/r8a779g0-cpg-mssr.c

> >> +   DEF_MOD("dis0", 411,R8A779G0_CLK_S0D3),
> >
> > I doubt this parent clock is correct.
> > Based on Table 8.1.4e ("Lists of CPG clocks generated from PLL5"),
> > this should be one of the VIOBUS clocks.
> > VIOBUSD2 has the same rate as S0D3, so I'd use that one.
> >
> >> +   DEF_MOD("dsitxlink0",   415,R8A779G0_CLK_DSIREF),
> >> +   DEF_MOD("dsitxlink1",   416,R8A779G0_CLK_DSIREF),
>
> Now that you started questioning about the clocks, I started to wonder
> about the DSI clocks. They don't quite make sense to me, but here also I
> just assumed it's "fine" as I copied it and it works.
>
> The VIOBUS & VIOBUSD2 are marked to as going to the DSI. But we don't
> actually mark any of the DSI clocks as coming from those sources.
>
> DSIREF is quite clear, it's the source for DSI PLL.
>
> DSIEXT goes to the DSI PHY and is also marked to be used for LP-TX.
>
> In the DT we have now:
>
> clocks = <&cpg CPG_MOD 415>,
>  <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>,
>  <&cpg CPG_CORE R8A779G0_CLK_DSIREF>;
> clock-names = "fck", "dsi", "pll";
>
> The "dsi" clock name is a bit vague, but maybe it's "not fclk, not pll,
> but still needed for dsi"? =)
>
> Is it ok to refer to DSIEXT & DSIREF like that, or should they be in the

Sounds fine to me.

> r8a779g0_mod_clks list? Or is that list for fclks only?

That list is only for clocks which have a bit in an MSTPCR (module
stop control register, Section 9.2.3).  These are typically controlled
through the Clock Domain and Runtime PM (but not for the DU, as there
is always only a single node in DT, even when the DU has multiple module
clocks on R-Car Gen2/3).

Actually our abstraction may be a bit off: sometimes that bit may gate
multiple clocks leading to the module, but as that was never documented
well, we settled on a single functional clock only, which is the most
common case.

> So the fclk in the dts is mod clock 415 (416 for the second dsi), which
> is dsitxlink0 or dsitxlink1. Well, those names don't quite make sense if
> it's a fclk.
>
> I would rename those clocks to "dsi0" and "dsi1", and source them from
> R8A779G0_CLK_VIOBUSD2, similarly to the other video clocks.
>
> Does the above make sense?

Please keep the names, as that's how they are called in Section 9.2.3.5
("Module Stop Control Register 4 (MSTPCR4)").

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v4 5/7] drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055

[Lucas Stach]

Hi Tomeu,

the changes itself look good to me now, I was just very confused about
the ordering of the patches.

I would have expected them to be in this order:
1. Add NN cores to chip identities struct (set to 0 for all existing
entries in HWDB)
2. Add UAPI warning
3. Add HWDB entry for VIPNano-QI.7120.0055 (having NN cores set to
correct value, so you don't touch the entry twice in the same series)

Regards,
Lucas

Am Donnerstag, dem 01.12.2022 um 10:21 +0100 schrieb Tomeu Vizoso:
> This is a compute-only module marketed towards AI and vision
> acceleration. This particular version can be found on the Amlogic A311D
> SoC.
> 
> The feature bits are taken from the Khadas downstream kernel driver
> 6.4.4.3.310723AAA.
> 
> Signed-off-by: Tomeu Vizoso 
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 31 ++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> index f2fc645c7956..3f6fd9a3c088 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> @@ -130,6 +130,37 @@ static const struct etnaviv_chip_identity 
> etnaviv_chip_identities[] = {
>   .minor_features10 = 0x90044250,
>   .minor_features11 = 0x0024,
>   },
> + {
> + .model = 0x8000,
> + .revision = 0x7120,
> + .product_id = 0x45080009,
> + .customer_id = 0x88,
> + .eco_id = 0,
> + .stream_count = 8,
> + .register_max = 64,
> + .thread_count = 256,
> + .shader_core_count = 1,
> + .vertex_cache_size = 16,
> + .vertex_output_buffer_size = 1024,
> + .pixel_pipes = 1,
> + .instruction_count = 512,
> + .num_constants = 320,
> + .buffer_size = 0,
> + .varyings_count = 16,
> + .features = 0xe0287cac,
> + .minor_features0 = 0xc1799eff,
> + .minor_features1 = 0xfefbfadb,
> + .minor_features2 = 0xeb9d6fbf,
> + .minor_features3 = 0xedfffced,
> + .minor_features4 = 0xd30dafc7,
> + .minor_features5 = 0x7b5ac333,
> + .minor_features6 = 0xfc8ee200,
> + .minor_features7 = 0x03fffa6f,
> + .minor_features8 = 0x00fe0ef0,
> + .minor_features9 = 0x0088003c,
> + .minor_features10 = 0x108048c0,
> + .minor_features11 = 0x0010,
> + },
>  };
>  
>  bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)

Re: [PATCH 1/3] drm/doc: Fix title underline length

[Javier Martinez Canillas]

On 11/28/22 09:19, Maxime Ripard wrote:
> The underline length for the new Analog TV properties section doesn't
> match the title length, resulting in a warning.
> 
> Fixes: 7d63cd8526f1 ("drm/connector: Add TV standard property")
> Reported-by: kernel test robot 
> Signed-off-by: Maxime Ripard 
> ---

Ah, I wasn't aware that this would lead to a kernel-doc warning.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/3] drm/modes: Use strscpy() to copy command-line mode name

[Javier Martinez Canillas]

On 11/28/22 09:19, Maxime Ripard wrote:
> The mode name in struct drm_cmdline_mode can hold 32 characters at most,
> which can easily get overrun. Switch to strscpy() to prevent such a
> thing.
> 
> Reported-by: coverity-bot 
> Addresses-Coverity-ID: 1527354 ("Security best practices violations")
> Fixes: a7ab155397dd ("drm/modes: Switch to named mode descriptors")
> Signed-off-by: Maxime Ripard 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v3 3/7] media: uapi: add MEDIA_BUS_FMT_BGR666_1X24_CPADHI

[Laurent Pinchart]

Hi Maxime,

Thank you for the patch.

On Thu, Dec 01, 2022 at 09:42:48AM +0100, Maxime Ripard wrote:
> From: Joerg Quinten 
> 
> Add the BGR666 format MEDIA_BUS_FMT_BGR666_1X24_CPADHI supported by the
> RaspberryPi.
> 
> Signed-off-by: Joerg Quinten 
> Signed-off-by: Maxime Ripard 

Reviewed-by: Laurent Pinchart 

> ---
>  .../userspace-api/media/v4l/subdev-formats.rst | 37 
> ++
>  include/uapi/linux/media-bus-format.h  |  3 +-
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst 
> b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> index 6605c056cc7c..5f2ce6eada71 100644
> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> @@ -1060,6 +1060,43 @@ The following tables list existing packed RGB formats.
>- b\ :sub:`2`
>- b\ :sub:`1`
>- b\ :sub:`0`
> +* .. _MEDIA-BUS-FMT-BGR666-1X24_CPADHI:
> +
> +  - MEDIA_BUS_FMT_BGR666_1X24_CPADHI
> +  - 0x1024
> +  -
> +  -
> +  -
> +  -
> +  -
> +  -
> +  -
> +  -
> +  -
> +  - 0
> +  - 0
> +  - b\ :sub:`5`
> +  - b\ :sub:`4`
> +  - b\ :sub:`3`
> +  - b\ :sub:`2`
> +  - b\ :sub:`1`
> +  - b\ :sub:`0`
> +  - 0
> +  - 0
> +  - g\ :sub:`5`
> +  - g\ :sub:`4`
> +  - g\ :sub:`3`
> +  - g\ :sub:`2`
> +  - g\ :sub:`1`
> +  - g\ :sub:`0`
> +  - 0
> +  - 0
> +  - r\ :sub:`5`
> +  - r\ :sub:`4`
> +  - r\ :sub:`3`
> +  - r\ :sub:`2`
> +  - r\ :sub:`1`
> +  - r\ :sub:`0`
>  * .. _MEDIA-BUS-FMT-RGB565-1X24_CPADHI:
>  
>- MEDIA_BUS_FMT_RGB565_1X24_CPADHI
> diff --git a/include/uapi/linux/media-bus-format.h 
> b/include/uapi/linux/media-bus-format.h
> index 6ce56a984112..f3b0b8091a2c 100644
> --- a/include/uapi/linux/media-bus-format.h
> +++ b/include/uapi/linux/media-bus-format.h
> @@ -34,7 +34,7 @@
>  
>  #define MEDIA_BUS_FMT_FIXED  0x0001
>  
> -/* RGB - next is 0x1024 */
> +/* RGB - next is 0x1025 */
>  #define MEDIA_BUS_FMT_RGB444_1X120x1016
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE0x1001
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE0x1002
> @@ -49,6 +49,7 @@
>  #define MEDIA_BUS_FMT_BGR666_1X180x1023
>  #define MEDIA_BUS_FMT_RBG888_1X240x100e
>  #define MEDIA_BUS_FMT_RGB666_1X24_CPADHI 0x1015
> +#define MEDIA_BUS_FMT_BGR666_1X24_CPADHI 0x1024
>  #define MEDIA_BUS_FMT_RGB565_1X24_CPADHI 0x1022
>  #define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG  0x1010
>  #define MEDIA_BUS_FMT_BGR888_1X240x1013
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm/tests: probe_helper: Fix unitialized variable

[Javier Martinez Canillas]

On 12/1/22 10:07, Maxime Ripard wrote:
> The len variable is used while uninitialized. Initialize it.
> 
> Fixes: 1e4a91db109f ("drm/probe-helper: Provide a TV get_modes helper")
> Reported-by: kernel test robot 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/tests/drm_probe_helper_test.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/tests/drm_probe_helper_test.c 
> b/drivers/gpu/drm/tests/drm_probe_helper_test.c
> index 7e938258c742..211131405500 100644
> --- a/drivers/gpu/drm/tests/drm_probe_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_probe_helper_test.c
> @@ -115,6 +115,7 @@ drm_test_connector_helper_tv_get_modes_check(struct kunit 
> *test)
>   ret = drm_connector_helper_tv_get_modes(connector);
>   KUNIT_EXPECT_EQ(test, ret, params->num_expected_modes);
>  
> + len = 0;

I would probably do `size_t len = 0;` instead but don't have a strong opinion.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Noralf Trønnes]

Den 01.12.2022 06.55, skrev Greg KH:
> On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 Submission 
> Endpoint wrote:
>> Hi,
>>
>> I have started to look at igt for testing and want to use CRC tests. To
>> implement support for this I need to move away from the simple kms
>> helper.
>>
>> When looking around for examples I came across Thomas' nice shadow
>> helper and thought, yes this is perfect for drm/gud. So I'll switch to
>> that before I move away from the simple kms helper.
>>
>> The async framebuffer flushing code path now uses a shadow buffer and
>> doesn't touch the framebuffer when it shouldn't. I have also taken the
>> opportunity to inline the synchronous flush code path and make this the
>> default flushing stategy.
>>
>> Noralf.
>>
>> Cc: Maxime Ripard 
>> Cc: Thomas Zimmermann 
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: Noralf Trønnes 
>>
>> ---
>> Changes in v2:
>> - Drop patch (Thomas):
>>   drm/gem: shadow_fb_access: Prepare imported buffers for CPU access
>> - Use src as variable name for iosys_map (Thomas)
>> - Prepare imported buffer for CPU access in the driver (Thomas)
>> - New patch: make sync flushing the default (Thomas)
>> - Link to v1: 
>> https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa33...@tronnes.org
> 
> 
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> 

Care to elaborate?
Is it because stable got the whole patchset and not just the one fix
patch that cc'ed stable?

This patchset was sent using the b4 tool and I can't control this
aspect. Everyone mentioned in the patches gets the whole set.

Noralf.

Re: [PATCH v2 5/7] arm64: dts: renesas: white-hawk-cpu: Add DP output support

[Geert Uytterhoeven]

Hi Tomi,

On Wed, Nov 23, 2022 at 8:00 AM Tomi Valkeinen
 wrote:
> Add DT nodes needed for the mini DP connector. The DP is driven by
> sn65dsi86, which in turn gets the pixel data from the SoC via DSI.
>
> Signed-off-by: Tomi Valkeinen 
> Reviewed-by: Kieran Bingham 
> Reviewed-by: Laurent Pinchart 

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in renesas-devel for v6.3, with the mini-dp-con node
moved up.

> --- a/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
> @@ -97,6 +97,15 @@ memory@6 {
> reg = <0x6 0x 0x1 0x>;
> };
>
> +   reg_1p2v: regulator-1p2v {
> +   compatible = "regulator-fixed";
> +   regulator-name = "fixed-1.2V";
> +   regulator-min-microvolt = <120>;
> +   regulator-max-microvolt = <120>;
> +   regulator-boot-on;
> +   regulator-always-on;
> +   };
> +
> reg_1p8v: regulator-1p8v {
> compatible = "regulator-fixed";
> regulator-name = "fixed-1.8V";
> @@ -114,6 +123,24 @@ reg_3p3v: regulator-3p3v {
> regulator-boot-on;
> regulator-always-on;
> };
> +
> +   mini-dp-con {
> +   compatible = "dp-connector";
> +   label = "CN5";
> +   type = "mini";
> +
> +   port {
> +   mini_dp_con_in: endpoint {
> +   remote-endpoint = <&sn65dsi86_out>;
> +   };
> +   };
> +   };

Moving up while applying to preserve sort order...

> +
> +   sn65dsi86_refclk: clk-x6 {
> +   compatible = "fixed-clock";
> +   #clock-cells = <0>;
> +   clock-frequency = <3840>;
> +   };
>  };
>
>  &avb0 {

> @@ -172,6 +216,51 @@ eeprom@50 {
> };
>  };
>
> +&i2c1 {
> +   pinctrl-0 = <&i2c1_pins>;
> +   pinctrl-names = "default";
> +
> +   status = "okay";
> +   clock-frequency = <40>;
> +
> +   bridge@2c {

Ideally, this needs pinctrl for the intc_ex irq0 pin.
Unfortunately[1] is still in limbo as the naming of the alternate pins
is inconsistent.

> +   compatible = "ti,sn65dsi86";
> +   reg = <0x2c>;
> +
> +   clocks = <&sn65dsi86_refclk>;
> +   clock-names = "refclk";
> +
> +   interrupt-parent = <&intc_ex>;
> +   interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +
> +   enable-gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
> +
> +   vccio-supply = <®_1p8v>;
> +   vpll-supply = <®_1p8v>;
> +   vcca-supply = <®_1p2v>;
> +   vcc-supply = <®_1p2v>;
> +
> +   ports {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   port@0 {
> +   reg = <0>;
> +   sn65dsi86_in: endpoint {
> +   remote-endpoint = <&dsi0_out>;
> +   };
> +   };
> +
> +   port@1 {
> +   reg = <1>;
> +   sn65dsi86_out: endpoint {
> +   remote-endpoint = <&mini_dp_con_in>;
> +   };
> +   };
> +   };
> +   };
> +};
> +
>  &mmc0 {
> pinctrl-0 = <&mmc_pins>;
> pinctrl-1 = <&mmc_pins>;

[1] "[PATCH/RFC] pinctrl: renesas: r8a779g0: Add INTC-EX pins, groups,
and function"

https://lore.kernel.org/all/28fe05d41bea5a03ea6c8434f5a4fb6c80b48867.1664368425.git.geert+rene...@glider.be

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] drm/fourcc: Document open source user waiver

[Daniel Vetter]

On Wed, Nov 23, 2022 at 08:24:37PM +0100, Daniel Vetter wrote:
> It's a bit a FAQ, and we really can't claim to be the authoritative
> source for allocating these numbers used in many standard extensions
> if we tell closed source or vendor stacks in general to go away.
> 
> Iirc this was already clarified in some vulkan discussions, but I
> can't find that anywhere anymore. At least not in a public link.
> 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Thomas Zimmermann 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Alex Deucher 
> Cc: Daniel Stone 
> Cc: Bas Nieuwenhuizen 
> Cc: Jason Ekstrand 
> Cc: Neil Trevett 
> Signed-off-by: Daniel Vetter 

>From irc:

 danvet: ack from me

> ---
>  include/uapi/drm/drm_fourcc.h | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index bc056f2d537d..de703c6be969 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -88,6 +88,18 @@ extern "C" {
>   *
>   * The authoritative list of format modifier codes is found in
>   * `include/uapi/drm/drm_fourcc.h`
> + *
> + * Open Source User Waiver
> + * ---
> + *
> + * Because this is the authoritative source for pixel formats and modifiers
> + * referenced by GL, Vulkan extensions and other standards and hence used 
> both
> + * by open source and closed source driver stacks, the usual requirement for 
> an
> + * upstream in-kernel or open source userspace user does not apply.
> + *
> + * To ensure, as much as feasible, compatibility across stacks and avoid
> + * confusion with incompatible enumerations stakeholders for all relevant 
> driver
> + * stacks should approve additions.
>   */
>  
>  #define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
> -- 
> 2.37.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v7 20/20] drm/i915/vm_bind: Async vm_unbind support

[Matthew Auld]

On 29/11/2022 23:26, Niranjana Vishwanathapura wrote:

On Wed, Nov 23, 2022 at 11:42:58AM +, Matthew Auld wrote:

On 16/11/2022 00:37, Niranjana Vishwanathapura wrote:
On Tue, Nov 15, 2022 at 03:15:03PM -0800, Niranjana Vishwanathapura 
wrote:
On Tue, Nov 15, 2022 at 08:33:47AM -0800, Niranjana Vishwanathapura 
wrote:

On Tue, Nov 15, 2022 at 04:20:54PM +, Matthew Auld wrote:

On 15/11/2022 16:15, Niranjana Vishwanathapura wrote:

On Tue, Nov 15, 2022 at 11:05:21AM +, Matthew Auld wrote:

On 13/11/2022 07:57, Niranjana Vishwanathapura wrote:

Asynchronously unbind the vma upon vm_unbind call.
Fall back to synchronous unbind if backend doesn't support
async unbind or if async unbind fails.

No need for vm_unbind out fence support as i915 will internally
handle all sequencing and user need not try to sequence any
operation with the unbind completion.

v2: use i915_vma_destroy_async in vm_unbind ioctl

Signed-off-by: Niranjana Vishwanathapura 



This only does it for non-partial vma, right? Or was that 
changed somewhere?




No, it applies to any vma (partial or non-partial).
It was so from the beginning.


Doesn't __i915_vma_unbind_async() return an error when mm.pages != 
vma->pages? IIRC this was discussed before. Just trying to think 
about the consequences of this change.


I am not seeing any such restriction. Let me probe and check if there
is any such restriction anywhere in the call chain.


I checked and I am not seeing any restriction anywher in the call 
chain.




Note that just like binding case, unbinding is also synchronous unless
there is a pending activity, in which case, it will be asynchronous.


In __i915_vma_unbind_async() there is:

if (i915_vma_is_pinned(vma) ||
   &vma->obj->mm.rsgt->table != vma->resource->bi.pages)
   return ERR_PTR(-EAGAIN);

AFAICT we then also don't get any pipelined moves with such an object, 
if there is such a binding present.


I had to remove this check as otherwise for VM_BIND (persistent) mappings,
it will alwasy be true and we will always endup with -EAGAIN.
Persistent mappings, as they support partial binding, can't have an sg
table which is just a pointer to object's sg table.


Ok, maybe make that a seperate patch, since it seems to change the 
behaviour for more than just persisent mappings, AFAICT.




Niranjana





Niranjana


Niranjana



Niranjana





Niranjana


Reviewed-by: Matthew Auld 


---
 .../drm/i915/gem/i915_gem_vm_bind_object.c    |  2 +-
 drivers/gpu/drm/i915/i915_vma.c   | 51 
+--

 drivers/gpu/drm/i915/i915_vma.h   |  1 +
 include/uapi/drm/i915_drm.h   |  3 +-
 4 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c 
b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c

index d87d1210365b..36651b447966 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
@@ -210,7 +210,7 @@ static int i915_gem_vm_unbind_vma(struct 
i915_address_space *vm,

  */
 obj = vma->obj;
 i915_gem_object_lock(obj, NULL);
-    i915_vma_destroy(vma);
+    i915_vma_destroy_async(vma);
 i915_gem_object_unlock(obj);
 i915_gem_object_put(obj);
diff --git a/drivers/gpu/drm/i915/i915_vma.c 
b/drivers/gpu/drm/i915/i915_vma.c

index 7cf77c67d755..483d25f2425c 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -42,6 +42,8 @@
 #include "i915_vma.h"
 #include "i915_vma_resource.h"
+static struct dma_fence *__i915_vma_unbind_async(struct 
i915_vma *vma);

+
 static inline void assert_vma_held_evict(const struct i915_vma 
*vma)

 {
 /*
@@ -1713,7 +1715,7 @@ void i915_vma_reopen(struct i915_vma *vma)
 spin_unlock_irq(>->closed_lock);
 }
-static void force_unbind(struct i915_vma *vma)
+static void force_unbind(struct i915_vma *vma, bool async)
 {
 if (!drm_mm_node_allocated(&vma->node))
 return;
@@ -1727,7 +1729,21 @@ static void force_unbind(struct i915_vma 
*vma)

 i915_vma_set_purged(vma);
 atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
-    WARN_ON(__i915_vma_unbind(vma));
+    if (async) {
+    struct dma_fence *fence;
+
+    fence = __i915_vma_unbind_async(vma);
+    if (IS_ERR_OR_NULL(fence)) {
+    async = false;
+    } else {
+    dma_resv_add_fence(vma->obj->base.resv, fence,
+   DMA_RESV_USAGE_READ);
+    dma_fence_put(fence);
+    }
+    }
+
+    if (!async)
+    WARN_ON(__i915_vma_unbind(vma));
 GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
 }
@@ -1787,7 +1803,7 @@ void i915_vma_destroy_locked(struct 
i915_vma *vma)

 {
 lockdep_assert_held(&vma->vm->mutex);
-    force_unbind(vma);
+    force_unbind(vma, false);
 list_del_init(&vma->vm_link);
 release_references(vma, vma->vm->gt, false);
 }
@@ -1798,7 +1814,34 @@ void i915_vma_destroy(struct i915_vma *vma)
 bool vm_ddestroy;
 mutex_lock(&v

Re: [PATCH 1/3] drm/doc: Fix title underline length

[Maxime Ripard]

On Mon, 28 Nov 2022 09:19:36 +0100, Maxime Ripard wrote:
> The underline length for the new Analog TV properties section doesn't
> match the title length, resulting in a warning.
> 
> 

Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime

Re: [PATCH] drm/tests: probe_helper: Fix unitialized variable

[Maxime Ripard]

On Thu, 1 Dec 2022 10:07:36 +0100, Maxime Ripard wrote:
> The len variable is used while uninitialized. Initialize it.
> 
> 

Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime

Re: [PATCH v3 0/7] drm/vc4: dpi: Various improvements

[Maxime Ripard]

On Thu, 01 Dec 2022 09:42:45 +0100, Maxime Ripard wrote:
> Those patches have been in the downstream RaspberryPi tree for a while and 
> help
> to support more DPI displays.
> 
> Let me know what you think,
> Maxime
> 
> 
> [...]

Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime

Re: [Intel-gfx] [PATCH 2/2] drm/i915/guc: Look for a guilty context when an engine reset fails

[Tvrtko Ursulin]

On 30/11/2022 21:04, John Harrison wrote:

On 11/30/2022 00:30, Tvrtko Ursulin wrote:

On 29/11/2022 21:12, john.c.harri...@intel.com wrote:

From: John Harrison 

Engine resets are supposed to never happen. But in the case when one


Engine resets or engine reset failures? Hopefully the latter.

Oops. Yes, that was meant to say "engine resets are never supposed to 
fail."



does (due to unknwon reasons that normally come down to a missing

unknwon -> unknown


w/a), it is useful to get as much information out of the system as
possible. Given that the GuC effectively dies on such a situation, it
is not possible to get a guilty context notification back. So do a
manual search instead. Given that GuC is dead, this is safe because
GuC won't be changing the engine state asynchronously.

Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 ++-
  1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c

index 0a42f1807f52c..c82730804a1c4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -4751,11 +4751,24 @@ static void reset_fail_worker_func(struct 
work_struct *w)

  guc->submission_state.reset_fail_mask = 0;
  spin_unlock_irqrestore(&guc->submission_state.lock, flags);
  -    if (likely(reset_fail_mask))
+    if (likely(reset_fail_mask)) {
+    struct intel_engine_cs *engine;
+    enum intel_engine_id id;
+
+    /*
+ * GuC is toast at this point - it dead loops after sending 
the failed
+ * reset notification. So need to manually determine the 
guilty context.
+ * Note that it should be safe/reliable to do this here 
because the GuC

+ * is toast and will not be scheduling behind the KMD's back.
+ */
+    for_each_engine_masked(engine, gt, reset_fail_mask, id)
+    intel_guc_find_hung_context(engine);
+
  intel_gt_handle_error(gt, reset_fail_mask,
    I915_ERROR_CAPTURE,
    "GuC failed to reset engine mask=0x%x\n",
    reset_fail_mask);


If GuC is defined by ABI contract to be dead, should the flow be 
attempting to do a full GPU reset here, or maybe it happens somewhere 
else as a consequence anyway? (In which case is the engine reset here 
even needed?)
This is a full GT reset. i915 is not allowed to perform an engine reset 
when using GuC submission. Those can only be done by GuC. So any forced 
reset by i915 will be escalated to full GT internally.


Okay, I saw passing in of the engine mask and drew the wrong conclusion.

Regards,

Tvrtko

Re: [PATCH] drm/fourcc: Document open source user waiver

[Daniel Vetter]

On Thu, 1 Dec 2022 at 11:07, Daniel Vetter  wrote:
>
> On Wed, Nov 23, 2022 at 08:24:37PM +0100, Daniel Vetter wrote:
> > It's a bit a FAQ, and we really can't claim to be the authoritative
> > source for allocating these numbers used in many standard extensions
> > if we tell closed source or vendor stacks in general to go away.
> >
> > Iirc this was already clarified in some vulkan discussions, but I
> > can't find that anywhere anymore. At least not in a public link.
> >
> > Cc: Maarten Lankhorst 
> > Cc: Maxime Ripard 
> > Cc: Thomas Zimmermann 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: Alex Deucher 
> > Cc: Daniel Stone 
> > Cc: Bas Nieuwenhuizen 
> > Cc: Jason Ekstrand 
> > Cc: Neil Trevett 
> > Signed-off-by: Daniel Vetter 
>
> From irc:
>
>  danvet: ack from me

Also from irc:

 danvet: Acked

-Daniel

> > ---
> >  include/uapi/drm/drm_fourcc.h | 12 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index bc056f2d537d..de703c6be969 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -88,6 +88,18 @@ extern "C" {
> >   *
> >   * The authoritative list of format modifier codes is found in
> >   * `include/uapi/drm/drm_fourcc.h`
> > + *
> > + * Open Source User Waiver
> > + * ---
> > + *
> > + * Because this is the authoritative source for pixel formats and modifiers
> > + * referenced by GL, Vulkan extensions and other standards and hence used 
> > both
> > + * by open source and closed source driver stacks, the usual requirement 
> > for an
> > + * upstream in-kernel or open source userspace user does not apply.
> > + *
> > + * To ensure, as much as feasible, compatibility across stacks and avoid
> > + * confusion with incompatible enumerations stakeholders for all relevant 
> > driver
> > + * stacks should approve additions.
> >   */
> >
> >  #define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
> > --
> > 2.37.2
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 01/11] pwm: Make .get_state() callback return an error code

[Andre Przywara]

On Wed, 30 Nov 2022 16:21:38 +0100
Uwe Kleine-König  wrote:

Hi,

> .get_state() might fail in some cases. To make it possible that a driver
> signals such a failure change the prototype of .get_state() to return an
> error code.
> 
> This patch was created using coccinelle and the following semantic patch:
> 
> @p1@
> identifier getstatefunc;
> identifier driver;
> @@
>  struct pwm_ops driver = {
> ...,
> .get_state = getstatefunc
> ,...
>  };
> 
> @p2@
> identifier p1.getstatefunc;
> identifier chip, pwm, state;
> @@
> -void
> +int
>  getstatefunc(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state 
> *state)
>  {
>...
> -  return;
> +  return 0;
>...
>  }
> 
> plus the actual change of the prototype in include/linux/pwm.h (plus some
> manual fixing of indentions and empty lines).
> 
> So for now all drivers return success unconditionally. They are adapted
> in the following patches to make the changes easier reviewable.
> 
> Signed-off-by: Uwe Kleine-König 
> ---
>  drivers/gpio/gpio-mvebu.c |  9 ++---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 14 --
>  drivers/leds/rgb/leds-qcom-lpg.c  | 14 --
>  drivers/pwm/pwm-atmel.c   |  6 --
>  drivers/pwm/pwm-bcm-iproc.c   |  8 +---
>  drivers/pwm/pwm-crc.c | 10 ++
>  drivers/pwm/pwm-cros-ec.c |  8 +---
>  drivers/pwm/pwm-dwc.c |  6 --
>  drivers/pwm/pwm-hibvt.c   |  6 --
>  drivers/pwm/pwm-imx-tpm.c |  8 +---
>  drivers/pwm/pwm-imx27.c   |  8 +---
>  drivers/pwm/pwm-intel-lgm.c   |  6 --
>  drivers/pwm/pwm-iqs620a.c |  6 --
>  drivers/pwm/pwm-keembay.c |  6 --
>  drivers/pwm/pwm-lpss.c|  6 --
>  drivers/pwm/pwm-meson.c   |  8 +---
>  drivers/pwm/pwm-mtk-disp.c| 12 +++-
>  drivers/pwm/pwm-pca9685.c |  8 +---
>  drivers/pwm/pwm-raspberrypi-poe.c |  8 +---
>  drivers/pwm/pwm-rockchip.c| 12 +++-
>  drivers/pwm/pwm-sifive.c  |  6 --
>  drivers/pwm/pwm-sl28cpld.c|  8 +---
>  drivers/pwm/pwm-sprd.c|  8 +---
>  drivers/pwm/pwm-stm32-lp.c|  8 +---
>  drivers/pwm/pwm-sun4i.c   | 12 +++-
>  drivers/pwm/pwm-sunplus.c |  6 --
>  drivers/pwm/pwm-visconti.c|  6 --
>  drivers/pwm/pwm-xilinx.c  |  8 +---
>  include/linux/pwm.h   |  4 ++--
>  29 files changed, 146 insertions(+), 89 deletions(-)
> 

[ ... ]
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index c8445b0a3339..37d75e252d4e 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -108,9 +108,9 @@ static inline void sun4i_pwm_writel(struct sun4i_pwm_chip 
> *chip,
>   writel(val, chip->base + offset);
>  }
>  
> -static void sun4i_pwm_get_state(struct pwm_chip *chip,
> - struct pwm_device *pwm,
> - struct pwm_state *state)
> +static int sun4i_pwm_get_state(struct pwm_chip *chip,
> +struct pwm_device *pwm,
> +struct pwm_state *state)
>  {
>   struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
>   u64 clk_rate, tmp;
> @@ -132,7 +132,7 @@ static void sun4i_pwm_get_state(struct pwm_chip *chip,
>   state->duty_cycle = DIV_ROUND_UP_ULL(state->period, 2);
>   state->polarity = PWM_POLARITY_NORMAL;
>   state->enabled = true;
> - return;
> + return 0;
>   }
>  
>   if ((PWM_REG_PRESCAL(val, pwm->hwpwm) == PWM_PRESCAL_MASK) &&
> @@ -142,7 +142,7 @@ static void sun4i_pwm_get_state(struct pwm_chip *chip,
>   prescaler = prescaler_table[PWM_REG_PRESCAL(val, pwm->hwpwm)];
>  
>   if (prescaler == 0)
> - return;
> + return 0;
>  
>   if (val & BIT_CH(PWM_ACT_STATE, pwm->hwpwm))
>   state->polarity = PWM_POLARITY_NORMAL;
> @@ -162,6 +162,8 @@ static void sun4i_pwm_get_state(struct pwm_chip *chip,
>  
>   tmp = (u64)prescaler * NSEC_PER_SEC * PWM_REG_PRD(val);
>   state->period = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
> +
> + return 0;
>  }
>  
>  static int sun4i_pwm_calculate(struct sun4i_pwm_chip *sun4i_pwm,

For sunxi:

Reviewed-by: Andre Przywara 

Just one comment: I don't see a sunxi specific patch later in the series,
though it seems we have at least one error error exit (see prescaler == 0
above). Plus potentially another exit if clk_get_rate() (at the very
beginning) fails.
Shall I send a patch for that?

Cheers,
Andre.

Re: [PATCH v2 01/17] drm/tests: helpers: Move the helper header to include/drm

[Maxime Ripard]

Hi Javier,

On Wed, Nov 30, 2022 at 09:00:03AM +0100, Javier Martinez Canillas wrote:
> On 11/28/22 15:53, Maxime Ripard wrote:
> > We'll need to use those helpers from drivers too, so let's move it to a
> > more visible location.
> > 
> > Signed-off-by: Maxime Ripard 
> > ---
> >  drivers/gpu/drm/tests/drm_client_modeset_test.c| 3 +--
> >  drivers/gpu/drm/tests/drm_kunit_helpers.c  | 3 +--
> >  drivers/gpu/drm/tests/drm_modes_test.c | 3 +--
> >  drivers/gpu/drm/tests/drm_probe_helper_test.c  | 3 +--
> >  {drivers/gpu/drm/tests => include/drm}/drm_kunit_helpers.h | 0
> >  5 files changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c 
> > b/drivers/gpu/drm/tests/drm_client_modeset_test.c
> > index 52929536a158..ed2f62e92fea 100644
> > --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
> > +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
> > @@ -8,12 +8,11 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> 
> I wonder if now that this header was moved outside of the tests directory,
> if we should add stub functions in the header file that are just defined
> but do nothing if CONFIG_DRM_KUNIT_TEST isn't enabled. So that including
> it in drivers will be a no-op.
> 
> Or do you plan to conditionally include this header file in drivers? So
> that is only included when CONFIG_DRM_KUNIT_TEST is enabled?

I'm not entirely sure. I'd expect only the tests to include it, and thus
would depend on DRM_KUNIT_TEST already. But we can always add the stubs
if it's ever included in a different context.

> Another thing that wondered is if we want a different namespace for this
> header, i.e: , to make it clear that is
> not part of the DRM API but just for testing helpers.

If there's a single header, I don't think we need to create the
directory. This is also something we can consolidate later on if needed.

> But these are open questions really, and they can be done as follow-up:
> 
> Reviewed-by: Javier Martinez Canillas 

Thanks :)
Maxime


signature.asc
Description: PGP signature

[PATCH v5 0/7] Support for the NPU in Vim3

[Tomeu Vizoso]

Hi,

This series adds support for the Verisilicon VIPNano-QI NPU in the A311D
as in the VIM3 board.

The IP is very closely based on previous Vivante GPUs, so the etnaviv
kernel driver works basically unchanged.

The userspace part of the driver is being reviewed at:

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18986

v2: Move reference to RESET_NNA to npu node (Neil)
v3: Fix indentation mistake (Neil)
v4: Add warning when etnaviv probes on a NPU (Lucas)
v5: Reorder HWDB commit to be the last (Lucas)

Regards,

Tomeu

Tomeu Vizoso (7):
  dt-bindings: reset: meson-g12a: Add missing NNA reset
  dt-bindings: power: Add G12A NNA power domain
  soc: amlogic: meson-pwrc: Add NNA power domain for A311D
  arm64: dts: Add DT node for the VIPNano-QI on the A311D
  drm/etnaviv: Add nn_core_count to chip feature struct
  drm/etnaviv: Warn when probing on NPUs
  drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055

 .../boot/dts/amlogic/meson-g12-common.dtsi| 11 ++
 .../amlogic/meson-g12b-a311d-khadas-vim3.dts  |  4 +++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c |  4 +++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h |  3 ++
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c| 35 +++
 drivers/soc/amlogic/meson-ee-pwrc.c   | 17 +
 include/dt-bindings/power/meson-g12a-power.h  |  1 +
 .../reset/amlogic,meson-g12a-reset.h  |  4 ++-
 8 files changed, 78 insertions(+), 1 deletion(-)

-- 
2.38.1

[PATCH v5 5/7] drm/etnaviv: Add nn_core_count to chip feature struct

[Tomeu Vizoso]

We will use these for differentiating between GPUs and NPUs, as the
downstream driver does.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h  | 3 +++
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 4 
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 85eddd492774..c8f3ad2031ce 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -50,6 +50,9 @@ struct etnaviv_chip_identity {
/* Number of shader cores. */
u32 shader_core_count;
 
+   /* Number of Neural Network cores. */
+   u32 nn_core_count;
+
/* Size of the vertex cache. */
u32 vertex_cache_size;
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c 
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index f2fc645c7956..44df273a5aae 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -16,6 +16,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 128,
.shader_core_count = 1,
+   .nn_core_count = 0,
.vertex_cache_size = 8,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -47,6 +48,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 512,
.shader_core_count = 2,
+   .nn_core_count = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -78,6 +80,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 512,
.shader_core_count = 2,
+   .nn_core_count = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -109,6 +112,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 1024,
.shader_core_count = 4,
+   .nn_core_count = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 2,
-- 
2.38.1

[PATCH v5 6/7] drm/etnaviv: Warn when probing on NPUs

[Tomeu Vizoso]

Userspace is still not making full use of the hardware, so we don't know
yet if changes to the UAPI won't be needed. Warn about it.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..3cbc82bbf8d4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -765,6 +765,10 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
goto fail;
}
 
+   if (gpu->identity.nn_core_count > 0)
+   dev_warn(gpu->dev, "etnaviv has been instantiated on a NPU, "
+   "for which the UAPI is still 
experimental\n");
+
/* Exclude VG cores with FE2.0 */
if (gpu->identity.features & chipFeatures_PIPE_VG &&
gpu->identity.features & chipFeatures_FE20) {
-- 
2.38.1

[PATCH v5 7/7] drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055

[Tomeu Vizoso]

This is a compute-only module marketed towards AI and vision
acceleration. This particular version can be found on the Amlogic A311D
SoC.

The feature bits are taken from the Khadas downstream kernel driver
6.4.4.3.310723AAA.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 31 ++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c 
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index 44df273a5aae..66b8ad6c7d26 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -134,6 +134,37 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.minor_features10 = 0x90044250,
.minor_features11 = 0x0024,
},
+   {
+   .model = 0x8000,
+   .revision = 0x7120,
+   .product_id = 0x45080009,
+   .customer_id = 0x88,
+   .eco_id = 0,
+   .stream_count = 8,
+   .register_max = 64,
+   .thread_count = 256,
+   .shader_core_count = 1,
+   .vertex_cache_size = 16,
+   .vertex_output_buffer_size = 1024,
+   .pixel_pipes = 1,
+   .instruction_count = 512,
+   .num_constants = 320,
+   .buffer_size = 0,
+   .varyings_count = 16,
+   .features = 0xe0287cac,
+   .minor_features0 = 0xc1799eff,
+   .minor_features1 = 0xfefbfadb,
+   .minor_features2 = 0xeb9d6fbf,
+   .minor_features3 = 0xedfffced,
+   .minor_features4 = 0xd30dafc7,
+   .minor_features5 = 0x7b5ac333,
+   .minor_features6 = 0xfc8ee200,
+   .minor_features7 = 0x03fffa6f,
+   .minor_features8 = 0x00fe0ef0,
+   .minor_features9 = 0x0088003c,
+   .minor_features10 = 0x108048c0,
+   .minor_features11 = 0x0010,
+   },
 };
 
 bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
-- 
2.38.1

Re: [PATCH v2 01/12] dt-bindings: display: msm: Rename mdss node name in example

[Krzysztof Kozlowski]

On 30/11/2022 21:09, Adam Skladowski wrote:
> Follow other YAMLs and replace mdss name into display-subystem.
> 
> Signed-off-by: Adam Skladowski 


Acked-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PATCH v2 11/12] arm64: dts: qcom: sm6115: Add WCN node.

[Krzysztof Kozlowski]

On 30/11/2022 21:09, Adam Skladowski wrote:
> Add WCN node to allow using wifi module.
> 

A nit: Drop full stop from commit subject.

Best regards,
Krzysztof

Re: [PATCH v2 01/17] drm/tests: helpers: Move the helper header to include/drm

[Javier Martinez Canillas]

Hello Maxime,

On 12/1/22 11:27, Maxime Ripard wrote:

[...]

>>
>> I wonder if now that this header was moved outside of the tests directory,
>> if we should add stub functions in the header file that are just defined
>> but do nothing if CONFIG_DRM_KUNIT_TEST isn't enabled. So that including
>> it in drivers will be a no-op.
>>
>> Or do you plan to conditionally include this header file in drivers? So
>> that is only included when CONFIG_DRM_KUNIT_TEST is enabled?
> 
> I'm not entirely sure. I'd expect only the tests to include it, and thus
> would depend on DRM_KUNIT_TEST already. But we can always add the stubs
> if it's ever included in a different context.
> 
>> Another thing that wondered is if we want a different namespace for this
>> header, i.e: , to make it clear that is
>> not part of the DRM API but just for testing helpers.
> 
> If there's a single header, I don't think we need to create the
> directory. This is also something we can consolidate later on if needed.
>

Agree on both. It's better to land as is and then figure out if needs
to be changed once other drivers add more tests.
 
>> But these are open questions really, and they can be done as follow-up:
>>
>> Reviewed-by: Javier Martinez Canillas 
> 
> Thanks :)

You are welcome!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 01/11] pwm: Make .get_state() callback return an error code

[Dave Stevenson]

On Wed, 30 Nov 2022 at 15:23, Uwe Kleine-König
 wrote:
>
> .get_state() might fail in some cases. To make it possible that a driver
> signals such a failure change the prototype of .get_state() to return an
> error code.
>
> This patch was created using coccinelle and the following semantic patch:
>
> @p1@
> identifier getstatefunc;
> identifier driver;
> @@
>  struct pwm_ops driver = {
> ...,
> .get_state = getstatefunc
> ,...
>  };
>
> @p2@
> identifier p1.getstatefunc;
> identifier chip, pwm, state;
> @@
> -void
> +int
>  getstatefunc(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state 
> *state)
>  {
>...
> -  return;
> +  return 0;
>...
>  }
>
> plus the actual change of the prototype in include/linux/pwm.h (plus some
> manual fixing of indentions and empty lines).
>
> So for now all drivers return success unconditionally. They are adapted
> in the following patches to make the changes easier reviewable.
>
> Signed-off-by: Uwe Kleine-König 
> ---
>  drivers/gpio/gpio-mvebu.c |  9 ++---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 14 --
>  drivers/leds/rgb/leds-qcom-lpg.c  | 14 --
>  drivers/pwm/pwm-atmel.c   |  6 --
>  drivers/pwm/pwm-bcm-iproc.c   |  8 +---
>  drivers/pwm/pwm-crc.c | 10 ++
>  drivers/pwm/pwm-cros-ec.c |  8 +---
>  drivers/pwm/pwm-dwc.c |  6 --
>  drivers/pwm/pwm-hibvt.c   |  6 --
>  drivers/pwm/pwm-imx-tpm.c |  8 +---
>  drivers/pwm/pwm-imx27.c   |  8 +---
>  drivers/pwm/pwm-intel-lgm.c   |  6 --
>  drivers/pwm/pwm-iqs620a.c |  6 --
>  drivers/pwm/pwm-keembay.c |  6 --
>  drivers/pwm/pwm-lpss.c|  6 --
>  drivers/pwm/pwm-meson.c   |  8 +---
>  drivers/pwm/pwm-mtk-disp.c| 12 +++-
>  drivers/pwm/pwm-pca9685.c |  8 +---
>  drivers/pwm/pwm-raspberrypi-poe.c |  8 +---
>  drivers/pwm/pwm-rockchip.c| 12 +++-
>  drivers/pwm/pwm-sifive.c  |  6 --
>  drivers/pwm/pwm-sl28cpld.c|  8 +---
>  drivers/pwm/pwm-sprd.c|  8 +---
>  drivers/pwm/pwm-stm32-lp.c|  8 +---
>  drivers/pwm/pwm-sun4i.c   | 12 +++-
>  drivers/pwm/pwm-sunplus.c |  6 --
>  drivers/pwm/pwm-visconti.c|  6 --
>  drivers/pwm/pwm-xilinx.c  |  8 +---
>  include/linux/pwm.h   |  4 ++--
>  29 files changed, 146 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index 1bb317b8dcce..91a4232ee58c 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -657,9 +657,10 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct 
> pwm_device *pwm)
> spin_unlock_irqrestore(&mvpwm->lock, flags);
>  }
>
> -static void mvebu_pwm_get_state(struct pwm_chip *chip,
> -   struct pwm_device *pwm,
> -   struct pwm_state *state) {
> +static int mvebu_pwm_get_state(struct pwm_chip *chip,
> +  struct pwm_device *pwm,
> +  struct pwm_state *state)
> +{
>
> struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
> @@ -693,6 +694,8 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
> state->enabled = false;
>
> spin_unlock_irqrestore(&mvpwm->lock, flags);
> +
> +   return 0;
>  }
>
>  static int mvebu_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 3c3561942eb6..6826d2423ae9 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -1500,8 +1500,8 @@ static int ti_sn_pwm_apply(struct pwm_chip *chip, 
> struct pwm_device *pwm,
> return ret;
>  }
>
> -static void ti_sn_pwm_get_state(struct pwm_chip *chip, struct pwm_device 
> *pwm,
> -   struct pwm_state *state)
> +static int ti_sn_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> +  struct pwm_state *state)
>  {
> struct ti_sn65dsi86 *pdata = pwm_chip_to_ti_sn_bridge(chip);
> unsigned int pwm_en_inv;
> @@ -1512,19 +1512,19 @@ static void ti_sn_pwm_get_state(struct pwm_chip 
> *chip, struct pwm_device *pwm,
>
> ret = regmap_read(pdata->regmap, SN_PWM_EN_INV_REG, &pwm_en_inv);
> if (ret)
> -   return;
> +   return 0;
>
> ret = ti_sn65dsi86_read_u16(pdata, SN_BACKLIGHT_SCALE_REG, &scale);
> if (ret)
> -   return;
> +   return 0;
>
> ret = ti_sn65dsi86_read_u16(pdata, SN_BACKLIGHT_REG, &backli

Re: [Intel-gfx] [PATCH v4 3/5] drm/i915: Introduce guard pages to i915_vma

[Andi Shyti]

Hi Tvrtko,

[...]

> > @@ -768,8 +773,17 @@ i915_vma_insert(struct i915_vma *vma, struct 
> > i915_gem_ww_ctx *ww,
> > GEM_BUG_ON(!IS_ALIGNED(alignment, I915_GTT_MIN_ALIGNMENT));
> > GEM_BUG_ON(!is_power_of_2(alignment));
> > +   guard = vma->guard; /* retain guard across rebinds */
> > +   if (flags & PIN_OFFSET_GUARD) {
> > +   GEM_BUG_ON(overflows_type(flags & PIN_OFFSET_MASK, u32));
> > +   guard = max_t(u32, guard, flags & PIN_OFFSET_MASK);
> > +   }
> > +   roundup(guard, BIT(vma->vm->scratch_order + PAGE_SHIFT));
> 
> roundup = ?

ehehe... yes, please ignore, that's some copy/paste error during
the rebase...

> Lets have a comment here as well.
> 
> /*
>  * Be efficient with PTE use by using the native size for the guard.
>  */
> 
> Would that be accurate?

and I also forgot the update of my previous comment... yours is
quite accurate.

> > +
> > start = flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0;
> > GEM_BUG_ON(!IS_ALIGNED(start, I915_GTT_PAGE_SIZE));
> > +   /* We need to be sure we do not ecceed the va area */
> > +   GEM_BUG_ON(2 * guard > end);
> 
> "exceed" but haven't we said this is not needed?

I wrote it in the cover letter. I had an offline chat with Chris
and he was keen to have this check not only for overflow
protection but also for a documentation purpose so that the
reader knows better about the size and usage of the guard.

Does it make sense?

Thanks a lot!

Andi

Re: [PATCH v8 22/22] drm/i915/vm_bind: Support capture of persistent mappings

[Matthew Auld]

On 29/11/2022 07:26, Niranjana Vishwanathapura wrote:

Support dump capture of persistent mappings upon user request.

Signed-off-by: Brian Welty 
Signed-off-by: Niranjana Vishwanathapura 
---
  .../drm/i915/gem/i915_gem_vm_bind_object.c| 11 +++
  drivers/gpu/drm/i915/gt/intel_gtt.c   |  3 +++
  drivers/gpu/drm/i915/gt/intel_gtt.h   |  5 +
  drivers/gpu/drm/i915/i915_gpu_error.c | 19 +++
  drivers/gpu/drm/i915/i915_vma.c   |  1 +
  drivers/gpu/drm/i915/i915_vma_types.h |  2 ++
  include/uapi/drm/i915_drm.h   |  3 ++-
  7 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c 
b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
index 78e7c0642c5f..50969613daf6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
@@ -88,6 +88,11 @@ static void i915_gem_vm_bind_remove(struct i915_vma *vma, 
bool release_obj)
  {
lockdep_assert_held(&vma->vm->vm_bind_lock);
  
+	spin_lock(&vma->vm->vm_capture_lock);

+   if (!list_empty(&vma->vm_capture_link))
+   list_del_init(&vma->vm_capture_link);
+   spin_unlock(&vma->vm->vm_capture_lock);
+
spin_lock(&vma->vm->vm_rebind_lock);
if (!list_empty(&vma->vm_rebind_link))
list_del_init(&vma->vm_rebind_link);
@@ -357,6 +362,12 @@ static int i915_gem_vm_bind_obj(struct i915_address_space 
*vm,
continue;
}
  
+		if (va->flags & I915_GEM_VM_BIND_CAPTURE) {

+   spin_lock(&vm->vm_capture_lock);
+   list_add_tail(&vma->vm_capture_link, 
&vm->vm_capture_list);
+   spin_unlock(&vm->vm_capture_lock);
+   }
+
list_add_tail(&vma->vm_bind_link, &vm->vm_bound_list);
i915_vm_bind_it_insert(vma, &vm->va);
if (!obj->priv_root)
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index ebf6830574a0..bdabe13fc30e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -297,6 +297,9 @@ void i915_address_space_init(struct i915_address_space *vm, 
int subclass)
spin_lock_init(&vm->vm_rebind_lock);
spin_lock_init(&vm->userptr_invalidated_lock);
INIT_LIST_HEAD(&vm->userptr_invalidated_list);
+
+   INIT_LIST_HEAD(&vm->vm_capture_list);
+   spin_lock_init(&vm->vm_capture_lock);
  }
  
  void *__px_vaddr(struct drm_i915_gem_object *p)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h 
b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 87e5b6568a00..8e4ddd073348 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -281,6 +281,11 @@ struct i915_address_space {
/** @root_obj: root object for dma-resv sharing by private objects */
struct drm_i915_gem_object *root_obj;
  
+	/* @vm_capture_list: list of vm captures */

+   struct list_head vm_capture_list;
+   /* @vm_capture_lock: protects vm_capture_list */
+   spinlock_t vm_capture_lock;
+
/* Global GTT */
bool is_ggtt:1;
  
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c

index 9d5d5a397b64..3b2b12a739f7 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1460,6 +1460,22 @@ capture_vma(struct intel_engine_capture_vma *next,
return next;
  }
  
+static struct intel_engine_capture_vma *

+capture_user_vm(struct intel_engine_capture_vma *capture,
+   struct i915_address_space *vm, gfp_t gfp)
+{
+   struct i915_vma *vma;
+
+   spin_lock(&vm->vm_capture_lock);


Does it make sense to move this into the eb3 submission stage, like we 
do for eb2? IIRC the gfp flags here are quite limiting due to 
potentially being in a fence critical section. If we can use 
rq->capture_list for eb3, we shouldn't need to change much here?


Also there is the existing CONFIG_DRM_I915_CAPTURE_ERROR. Should we take 
that into account?



+   /* vma->resource must be valid here as persistent vmas are bound */
+   list_for_each_entry(vma, &vm->vm_capture_list, vm_capture_link)
+   capture = capture_vma_snapshot(capture, vma->resource,
+  gfp, "user");
+   spin_unlock(&vm->vm_capture_lock);
+
+   return capture;
+}
+
  static struct intel_engine_capture_vma *
  capture_user(struct intel_engine_capture_vma *capture,
 const struct i915_request *rq,
@@ -1471,6 +1487,9 @@ capture_user(struct intel_engine_capture_vma *capture,
capture = capture_vma_snapshot(capture, c->vma_res, gfp,
   "user");
  
+	capture = capture_user_vm(capture, rq->context->vm,

+ GFP_NOWAIT | __GFP_NOWARN);


An

Re: [Intel-gfx] [PATCH v4 3/5] drm/i915: Introduce guard pages to i915_vma

[Tvrtko Ursulin]

On 01/12/2022 10:45, Andi Shyti wrote:

Hi Tvrtko,

[...]


@@ -768,8 +773,17 @@ i915_vma_insert(struct i915_vma *vma, struct 
i915_gem_ww_ctx *ww,
GEM_BUG_ON(!IS_ALIGNED(alignment, I915_GTT_MIN_ALIGNMENT));
GEM_BUG_ON(!is_power_of_2(alignment));
+   guard = vma->guard; /* retain guard across rebinds */
+   if (flags & PIN_OFFSET_GUARD) {
+   GEM_BUG_ON(overflows_type(flags & PIN_OFFSET_MASK, u32));
+   guard = max_t(u32, guard, flags & PIN_OFFSET_MASK);
+   }
+   roundup(guard, BIT(vma->vm->scratch_order + PAGE_SHIFT));


roundup = ?


ehehe... yes, please ignore, that's some copy/paste error during
the rebase...


Lets have a comment here as well.

/*
  * Be efficient with PTE use by using the native size for the guard.
  */

Would that be accurate?


and I also forgot the update of my previous comment... yours is
quite accurate.


+
start = flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0;
GEM_BUG_ON(!IS_ALIGNED(start, I915_GTT_PAGE_SIZE));
+   /* We need to be sure we do not ecceed the va area */
+   GEM_BUG_ON(2 * guard > end);


"exceed" but haven't we said this is not needed?


I wrote it in the cover letter. I had an offline chat with Chris
and he was keen to have this check not only for overflow
protection but also for a documentation purpose so that the
reader knows better about the size and usage of the guard.

Does it make sense?


Not to me really, I have no idea how could anyone ever end up with guard 
size of half+ of GGTT. And the total 2 * guard + size is checked and 
rejected already. So I have no idea what it is supposed to be 
documenting. GEM_BUG_ON suggests really bad things would happen if that 
was passed in, like something would be incorrectly calculated. If that 
is not the case and things would just safely fail then it is just 
confusing to have it.


Regards,

Tvrtko

Re: [RFC 3/3] drm: Update file owner during use

[Tvrtko Ursulin]

On 30/11/2022 14:18, Daniel Vetter wrote:

On Wed, Nov 30, 2022 at 01:34:07PM +, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

With the typical model where the display server opends the file descriptor
and then hands it over to the client we were showing stale data in
debugfs.

Fix it by updating the drm_file->pid on ioctl access from a different
process.

The field is also made RCU protected to allow for lockless readers. Update
side is protected with dev->filelist_mutex.

Signed-off-by: Tvrtko Ursulin 
Cc: "Christian König" 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  6 +++--
  drivers/gpu/drm/drm_auth.c  |  3 ++-
  drivers/gpu/drm/drm_debugfs.c   | 10 
  drivers/gpu/drm/drm_file.c  | 32 -
  drivers/gpu/drm/drm_ioctl.c |  3 +++
  drivers/gpu/drm/nouveau/nouveau_drm.c   |  5 +++-
  drivers/gpu/drm/vmwgfx/vmwgfx_gem.c |  6 +++--
  include/drm/drm_file.h  | 13 --
  8 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 30e24da1f398..385deb044058 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -959,6 +959,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, 
void *unused)
list_for_each_entry(file, &dev->filelist, lhead) {
struct task_struct *task;
struct drm_gem_object *gobj;
+   struct pid *pid;
int id;
  
  		/*

@@ -968,8 +969,9 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, 
void *unused)
 * Therefore, we need to protect this ->comm access using RCU.
 */
rcu_read_lock();
-   task = pid_task(file->pid, PIDTYPE_TGID);
-   seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
+   pid = rcu_dereference(file->pid);
+   task = pid_task(pid, PIDTYPE_TGID);
+   seq_printf(m, "pid %8d command %s:\n", pid_nr(pid),
   task ? task->comm : "");
rcu_read_unlock();
  
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c

index cf92a9ae8034..2ed2585ded37 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -235,7 +235,8 @@ static int drm_new_set_master(struct drm_device *dev, 
struct drm_file *fpriv)
  static int
  drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv)
  {
-   if (file_priv->pid == task_pid(current) && file_priv->was_master)
+   if (file_priv->was_master &&
+   rcu_access_pointer(file_priv->pid) == task_pid(current))


This scares me, and also makes me wonder whether we really want to
conflate the original owner with the rendering owner. And also, whether we
really want to keep updating that, because for some of the "bind an fd to
a pid" use-cases like svm we really do not want to ever again allow a
change.

So sligthly different idea:
- we have a separate render pid/drm_file owner frome the open() owner that
   we track in drm_auth.c
- that one is set the first time a driver specific ioctl is called (which
   for the "pass me the fd" dri3 mode should never be the compositor)
- we start out with nothing and only set it once, which further simplifies
   the model (still need the mutex for concurrent first ioctl ofc)


Simpler solution sounds plausible and mostly works for me. Certainly is 
attractive to simplify things. And as the disclaimer I put in the cover 
letter - I wasn't really sure at all if passing a master fd is a thing 
or not. Happy to implement your version if that will be the decision.


The only downside I can think of right now with having two owners is if 
someone is "naughty" and actually uses the fd for rendering from two 
sides. That wouldn't conceptually work for what I am doing in the cgroup 
controller, where I need to attribute GPU usage to a process, which is a 
lookup from struct pid -> list of drm_files -> etc. So in the two owners 
scheme I would just need to ignore the "open owner" and rely that 
"render ownder" truly is the only one doing the rendering. Or maybe I'd 
need to add support for multiple owners as well.. would be a bit 
annoying probably.


Hm now that I think about more.. the one shot nature of this scheme 
would have another downside. One could just send the fd back to itself 
via a throway forked helper, which only does one ioctl before sending it 
back, and then the "render owner" is forever lost. The proposal as I had 
it would be immune to this problem at least.



Eventually we could then use that to enforce static binding to a pid,
which is what we want for svm style models, i.e. if the pid changes, the
fd does an -EACCESS or similar.

Thoughts?


This use case I am not familiar with at all so can't comment. Only 
intuitively I would ask - why is it something that needs to b

Re: [PATCH v2 00/11] pwm: Allow .get_state to fail

[Conor Dooley]

Hey Uwe!

On Wed, Nov 30, 2022 at 04:21:37PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> I forgot about this series and was remembered when I talked to Conor
> Dooley about how .get_state() should behave in an error case.

In the context of "my" driver, get_state() the proposal was to fail with
-ETIMEDOUT rather than block a caller, potentially, for seconds or
report a potentially "random" state.

Specifically, values writen to the registers that control the PWM duty
cycle are not visible to the cpu until the changes have propagated to
the waveform at the start of a new period.
The timeout would occur if the bit that signifies that the "shadow
registers" contain a value which has not yet propagated. This bit is
per PWM "controller" and not per PWM channel.

Returning from apply() without waiting, possibly for seconds, for the
writes to become visible could cause get_state() to see anything between
the new and old states, inclusive!

If anyone cares at all, the discussion is here:
https://lore.kernel.org/linux-pwm/20221110093512.333881-1-conor.doo...@microchip.com/T/#m800eeabad29067940a5684e54106fd0bb7261944

> In v1 Thierry had the concern:
> 
> | That raises the question about what to do in these cases. If we return
> | an error, that could potentially throw off consumers. So perhaps the
> | closest would be to return a disabled PWM?
> | Or perhaps it'd be up to the
> | consumer to provide some fallback configuration for invalidly configured
> | or unconfigured PWMs.
> 
> .get_state() is only called in pwm_device_request on a pwm_state that a
> consumer might see. Before my series a consumer might have seen a
> partial modified pwm_state (because .get_state() might have modified
> .period, then stumbled and returned silently). The last patch ensures
> that this partial modification isn't given out to the consumer. Instead
> they now see the same as if .get_state wasn't implemented at all.

Getting the same thing as if get_state() did not exist seems
preferable to me in this context than "lying" and pretending that a PWM
is disabled or potentially inconsistent reports from get_state() that I
mentioned above.

TL;DR, I quite like the ability to return an error and not mislead the
caller.

Thanks for sending a v2 of this so quickly :)
Conor.

Re: [PATCH v28 05/11] soc: mediatek: refine code to use mtk_mmsys_update_bits API

[Chen-Yu Tsai]

On Mon, Nov 7, 2022 at 3:23 PM Nancy.Lin  wrote:
>
> Simplify code for update  mmsys reg.
>
> Signed-off-by: Nancy.Lin 
> Reviewed-by: AngeloGioacchino Del Regno 
> 
> Reviewed-by: CK Hu 
> Tested-by: AngeloGioacchino Del Regno 
> 
> Tested-by: Bo-Chen Chen 
> Reviewed-by: Nícolas F. R. A. Prado 
> ---
>  drivers/soc/mediatek/mtk-mmsys.c | 45 
>  1 file changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c 
> b/drivers/soc/mediatek/mtk-mmsys.c
> index 9a327eb5d9d7..73c8bd27e6ae 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c

[...]

> @@ -124,27 +129,14 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
>  {
> struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
> const struct mtk_mmsys_routes *routes = mmsys->data->routes;
> -   u32 reg;
> int i;
>
> for (i = 0; i < mmsys->data->num_routes; i++)
> -   if (cur == routes[i].from_comp && next == routes[i].to_comp) {
> -   reg = readl_relaxed(mmsys->regs + routes[i].addr);
> -   reg &= ~routes[i].mask;
> -   writel_relaxed(reg, mmsys->regs + routes[i].addr);
> -   }
> +   if (cur == routes[i].from_comp && next == routes[i].to_comp)
> +   mtk_mmsys_update_bits(mmsys, routes[i].addr, 
> routes[i].mask, 0);
>  }
>  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);
>
> -static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32 offset, u32 
> mask, u32 val)
> -{
> -   u32 tmp;
> -
> -   tmp = readl_relaxed(mmsys->regs + offset);
> -   tmp = (tmp & ~mask) | val;
> -   writel_relaxed(tmp, mmsys->regs + offset);
> -}
> -
>  void mtk_mmsys_ddp_dpi_fmt_config(struct device *dev, u32 val)
>  {
> if (val)

This hunk now doesn't apply due to

soc: mediatek: Add all settings to mtk_mmsys_ddp_dpi_fmt_config func

touching mtk_mmsys_ddp_dpi_fmt_config() as well. It's trivial to resolve
though.

ChenYu

Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Add GuC CT specific debug print wrappers

[Michal Wajdeczko]

On 01.12.2022 01:41, John Harrison wrote:
> On 11/23/2022 12:45, Michal Wajdeczko wrote:
>> On 23.11.2022 02:25, John Harrison wrote:
>>> On 11/22/2022 09:54, Michal Wajdeczko wrote:
 On 18.11.2022 02:58, john.c.harri...@intel.com wrote:
> From: John Harrison 
>
> Re-work the existing GuC CT printers and extend as required to match
> the new wrapping scheme.
>
> Signed-off-by: John Harrison 
> ---
>    drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 222
> +++---
>    1 file changed, 113 insertions(+), 109 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> index 2b22065e87bf9..9d404fb377637 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> @@ -18,31 +18,49 @@ static inline struct intel_guc *ct_to_guc(struct
> intel_guc_ct *ct)
>    return container_of(ct, struct intel_guc, ct);
>    }
>    -static inline struct intel_gt *ct_to_gt(struct intel_guc_ct *ct)
> -{
> -    return guc_to_gt(ct_to_guc(ct));
> -}
> -
>    static inline struct drm_i915_private *ct_to_i915(struct
> intel_guc_ct *ct)
>    {
> -    return ct_to_gt(ct)->i915;
> -}
> +    struct intel_guc *guc = ct_to_guc(ct);
> +    struct intel_gt *gt = guc_to_gt(guc);
>    -static inline struct drm_device *ct_to_drm(struct intel_guc_ct
> *ct)
> -{
> -    return &ct_to_i915(ct)->drm;
> +    return gt->i915;
>    }
>    -#define CT_ERROR(_ct, _fmt, ...) \
> -    drm_err(ct_to_drm(_ct), "CT: " _fmt, ##__VA_ARGS__)
> +#define ct_err(_ct, _fmt, ...) \
> +    guc_err(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
> +
> +#define ct_warn(_ct, _fmt, ...) \
> +    guc_warn(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
> +
> +#define ct_notice(_ct, _fmt, ...) \
> +    guc_notice(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
> +
> +#define ct_info(_ct, _fmt, ...) \
> +    guc_info(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
> +
>    #ifdef CONFIG_DRM_I915_DEBUG_GUC
> -#define CT_DEBUG(_ct, _fmt, ...) \
> -    drm_dbg(ct_to_drm(_ct), "CT: " _fmt, ##__VA_ARGS__)
> +#define ct_dbg(_ct, _fmt, ...) \
> +    guc_dbg(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
>    #else
> -#define CT_DEBUG(...)    do { } while (0)
> +#define ct_dbg(...)    do { } while (0)
>    #endif
> -#define CT_PROBE_ERROR(_ct, _fmt, ...) \
> -    i915_probe_error(ct_to_i915(ct), "CT: " _fmt, ##__VA_ARGS__)
> +
> +#define ct_probe_error(_ct, _fmt, ...) \
> +    do { \
> +    if (i915_error_injected()) \
> +    ct_dbg(_ct, _fmt, ##__VA_ARGS__); \
> +    else \
> +    ct_err(_ct, _fmt, ##__VA_ARGS__); \
> +    } while (0)
 guc_probe_error ?

> +
> +#define ct_WARN_ON(_ct, _condition) \
> +    ct_WARN(_ct, _condition, "%s", "ct_WARN_ON("
> __stringify(_condition) ")")
> +
> +#define ct_WARN(_ct, _condition, _fmt, ...) \
> +    guc_WARN(ct_to_guc(_ct), _condition, "CT " _fmt, ##__VA_ARGS__)
> +
> +#define ct_WARN_ONCE(_ct, _condition, _fmt, ...) \
> +    guc_WARN_ONCE(ct_to_guc(_ct), _condition, "CT " _fmt,
> ##__VA_ARGS__)
>      /**
>     * DOC: CTB Blob
> @@ -170,7 +188,7 @@ static int ct_control_enable(struct intel_guc_ct
> *ct, bool enable)
>    err = guc_action_control_ctb(ct_to_guc(ct), enable ?
>     GUC_CTB_CONTROL_ENABLE :
> GUC_CTB_CONTROL_DISABLE);
>    if (unlikely(err))
> -    CT_PROBE_ERROR(ct, "Failed to control/%s CTB (%pe)\n",
> +    ct_probe_error(ct, "Failed to control/%s CTB (%pe)\n",
>   str_enable_disable(enable), ERR_PTR(err));
 btw, shouldn't we change all messages to start with lowercase ?

 was:
  "CT0: Failed to control/%s CTB (%pe)"
 is:
  "GT0: GuC CT Failed to control/%s CTB (%pe)"

 unless we keep colon (as suggested by Tvrtko) as then:

  "GT0: GuC CT: Failed to control/%s CTB (%pe)"
>>> Blanket added the colon makes it messy when a string actually wants to
>>> start with the prefix. The rule I've been using is lower case word when
>>> the prefix was part of the string, upper case word when the prefix is
>> Hmm, I'm not sure that we should attempt to have such a flexible rule as
>> we shouldn't rely too much on actual format of the prefix as it could be
>> changed any time.  All we should know about final log message is that it
>> _will_ properly identify the "GT" or "GuC" that this log is related to.
>>
>> So I would suggest to be just consistent and probably always start with
>> upper case, as that seems to be mostly used in kernel error logs, and
>> just make sure that any prefix will honor that (by including colon, or
>> braces),

Re: [PATCH v5 7/7] drm: rcar-du: dsi: Add r8A779g0 support

[Kieran Bingham]

Quoting Tomi Valkeinen (2022-12-01 09:56:31)
> Add DSI support for r8a779g0. The main differences to r8a779a0 are in
> the PLL and PHTW setups.
> 
> Signed-off-by: Tomi Valkeinen 
> Reviewed-by: Laurent Pinchart 

Now that the differences I saw about the PHTW values are understood, I'm
happy.

I like the MHZ() macro for readability too.

Reviewed-by: Kieran Bingham 


> ---
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c  | 497 ++-
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h |   6 +-
>  2 files changed, 375 insertions(+), 128 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c 
> b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index a7f2b7f66a17..e10e4d4b89a2 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -28,6 +29,31 @@
>  #include "rcar_mipi_dsi.h"
>  #include "rcar_mipi_dsi_regs.h"
>  
> +#define MHZ(v) ((u32)((v) * 100U))
> +
> +enum rcar_mipi_dsi_hw_model {
> +   RCAR_DSI_V3U,
> +   RCAR_DSI_V4H,
> +};
> +
> +struct rcar_mipi_dsi_device_info {
> +   enum rcar_mipi_dsi_hw_model model;
> +
> +   const struct dsi_clk_config *clk_cfg;
> +
> +   u8 clockset2_m_offset;
> +
> +   u8 n_min;
> +   u8 n_max;
> +   u8 n_mul;
> +   unsigned long fpfd_min;
> +   unsigned long fpfd_max;
> +   u16 m_min;
> +   u16 m_max;
> +   unsigned long fout_min;
> +   unsigned long fout_max;
> +};
> +
>  struct rcar_mipi_dsi {
> struct device *dev;
> const struct rcar_mipi_dsi_device_info *info;
> @@ -50,6 +76,17 @@ struct rcar_mipi_dsi {
> unsigned int lanes;
>  };
>  
> +struct dsi_setup_info {
> +   unsigned long hsfreq;
> +   u16 hsfreqrange;
> +
> +   unsigned long fout;
> +   u16 m;
> +   u16 n;
> +   u16 vclk_divider;
> +   const struct dsi_clk_config *clkset;
> +};
> +
>  static inline struct rcar_mipi_dsi *
>  bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
>  {
> @@ -62,65 +99,78 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
> return container_of(host, struct rcar_mipi_dsi, host);
>  }
>  
> -static const u32 phtw[] = {
> -   0x01020114, 0x01600115, /* General testing */
> -   0x01030116, 0x0102011d, /* General testing */
> -   0x011101a4, 0x018601a4, /* 1Gbps testing */
> -   0x014201a0, 0x010001a3, /* 1Gbps testing */
> -   0x0101011f, /* 1Gbps testing */
> -};
> -
> -static const u32 phtw2[] = {
> -   0x010c0130, 0x010c0140, /* General testing */
> -   0x010c0150, 0x010c0180, /* General testing */
> -   0x010c0190,
> -   0x010a0160, 0x010a0170,
> -   0x01800164, 0x01800174, /* 1Gbps testing */
> -};
> -
>  static const u32 hsfreqrange_table[][2] = {
> -   { 8000U,   0x00 }, { 9000U,   0x10 }, { 1U,  0x20 },
> -   { 11000U,  0x30 }, { 12000U,  0x01 }, { 13000U,  0x11 },
> -   { 14000U,  0x21 }, { 15000U,  0x31 }, { 16000U,  0x02 },
> -   { 17000U,  0x12 }, { 18000U,  0x22 }, { 19000U,  0x32 },
> -   { 20500U,  0x03 }, { 22000U,  0x13 }, { 23500U,  0x23 },
> -   { 25000U,  0x33 }, { 27500U,  0x04 }, { 3U,  0x14 },
> -   { 32500U,  0x25 }, { 35000U,  0x35 }, { 4U,  0x05 },
> -   { 45000U,  0x16 }, { 5U,  0x26 }, { 55000U,  0x37 },
> -   { 6U,  0x07 }, { 65000U,  0x18 }, { 7U,  0x28 },
> -   { 75000U,  0x39 }, { 8U,  0x09 }, { 85000U,  0x19 },
> -   { 9U,  0x29 }, { 95000U,  0x3a }, { 10U, 0x0a },
> -   { 105000U, 0x1a }, { 11U, 0x2a }, { 115000U, 0x3b },
> -   { 12U, 0x0b }, { 125000U, 0x1b }, { 13U, 0x2b },
> -   { 135000U, 0x3c }, { 14U, 0x0c }, { 145000U, 0x1c },
> -   { 15U, 0x2c }, { 155000U, 0x3d }, { 16U, 0x0d },
> -   { 165000U, 0x1d }, { 17U, 0x2e }, { 175000U, 0x3e },
> -   { 18U, 0x0e }, { 185000U, 0x1e }, { 19U, 0x2f },
> -   { 195000U, 0x3f }, { 20U, 0x0f }, { 205000U, 0x40 },
> -   { 21U, 0x41 }, { 215000U, 0x42 }, { 22U, 0x43 },
> -   { 225000U, 0x44 }, { 23U, 0x45 }, { 235000U, 0x46 },
> -   { 24U, 0x47 }, { 245000U, 0x48 }, { 25U, 0x49 },
> +   {   MHZ(80), 0x00 }, {   MHZ(90), 0x10 }, {  MHZ(100), 0x20 },
> +   {  MHZ(110), 0x30 }, {  MHZ(120), 0x01 }, {  MHZ(130), 0x11 },
> +   {  MHZ(140), 0x21 }, {  MHZ(150), 0x31 }, {  MHZ(160), 0x02 },
> +   {  MHZ(170), 0x12 }, {  MHZ(180), 0x22 }, {  MHZ(190), 0x32 },
> +   {  MHZ(205), 0x03 }, {  MHZ(220), 0x13 }, {  MHZ(235), 0x23 },
> +   {  MHZ(250), 0x33 }, {  MHZ(275), 0x04 }, {  MHZ(300), 0x14 },
> +   {  MHZ(325), 0x25 }, 

Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Add GuC CT specific debug print wrappers

[Tvrtko Ursulin]

On 01/12/2022 11:56, Michal Wajdeczko wrote:

On 01.12.2022 01:41, John Harrison wrote:

On 11/23/2022 12:45, Michal Wajdeczko wrote:

On 23.11.2022 02:25, John Harrison wrote:

On 11/22/2022 09:54, Michal Wajdeczko wrote:

On 18.11.2022 02:58, john.c.harri...@intel.com wrote:

From: John Harrison 

Re-work the existing GuC CT printers and extend as required to match
the new wrapping scheme.

Signed-off-by: John Harrison 
---
    drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 222
+++---
    1 file changed, 113 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index 2b22065e87bf9..9d404fb377637 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -18,31 +18,49 @@ static inline struct intel_guc *ct_to_guc(struct
intel_guc_ct *ct)
    return container_of(ct, struct intel_guc, ct);
    }
    -static inline struct intel_gt *ct_to_gt(struct intel_guc_ct *ct)
-{
-    return guc_to_gt(ct_to_guc(ct));
-}
-
    static inline struct drm_i915_private *ct_to_i915(struct
intel_guc_ct *ct)
    {
-    return ct_to_gt(ct)->i915;
-}
+    struct intel_guc *guc = ct_to_guc(ct);
+    struct intel_gt *gt = guc_to_gt(guc);
    -static inline struct drm_device *ct_to_drm(struct intel_guc_ct
*ct)
-{
-    return &ct_to_i915(ct)->drm;
+    return gt->i915;
    }
    -#define CT_ERROR(_ct, _fmt, ...) \
-    drm_err(ct_to_drm(_ct), "CT: " _fmt, ##__VA_ARGS__)
+#define ct_err(_ct, _fmt, ...) \
+    guc_err(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
+
+#define ct_warn(_ct, _fmt, ...) \
+    guc_warn(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
+
+#define ct_notice(_ct, _fmt, ...) \
+    guc_notice(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
+
+#define ct_info(_ct, _fmt, ...) \
+    guc_info(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
+
    #ifdef CONFIG_DRM_I915_DEBUG_GUC
-#define CT_DEBUG(_ct, _fmt, ...) \
-    drm_dbg(ct_to_drm(_ct), "CT: " _fmt, ##__VA_ARGS__)
+#define ct_dbg(_ct, _fmt, ...) \
+    guc_dbg(ct_to_guc(_ct), "CT " _fmt, ##__VA_ARGS__)
    #else
-#define CT_DEBUG(...)    do { } while (0)
+#define ct_dbg(...)    do { } while (0)
    #endif
-#define CT_PROBE_ERROR(_ct, _fmt, ...) \
-    i915_probe_error(ct_to_i915(ct), "CT: " _fmt, ##__VA_ARGS__)
+
+#define ct_probe_error(_ct, _fmt, ...) \
+    do { \
+    if (i915_error_injected()) \
+    ct_dbg(_ct, _fmt, ##__VA_ARGS__); \
+    else \
+    ct_err(_ct, _fmt, ##__VA_ARGS__); \
+    } while (0)

guc_probe_error ?


+
+#define ct_WARN_ON(_ct, _condition) \
+    ct_WARN(_ct, _condition, "%s", "ct_WARN_ON("
__stringify(_condition) ")")
+
+#define ct_WARN(_ct, _condition, _fmt, ...) \
+    guc_WARN(ct_to_guc(_ct), _condition, "CT " _fmt, ##__VA_ARGS__)
+
+#define ct_WARN_ONCE(_ct, _condition, _fmt, ...) \
+    guc_WARN_ONCE(ct_to_guc(_ct), _condition, "CT " _fmt,
##__VA_ARGS__)
      /**
     * DOC: CTB Blob
@@ -170,7 +188,7 @@ static int ct_control_enable(struct intel_guc_ct
*ct, bool enable)
    err = guc_action_control_ctb(ct_to_guc(ct), enable ?
     GUC_CTB_CONTROL_ENABLE :
GUC_CTB_CONTROL_DISABLE);
    if (unlikely(err))
-    CT_PROBE_ERROR(ct, "Failed to control/%s CTB (%pe)\n",
+    ct_probe_error(ct, "Failed to control/%s CTB (%pe)\n",
   str_enable_disable(enable), ERR_PTR(err));

btw, shouldn't we change all messages to start with lowercase ?

was:
  "CT0: Failed to control/%s CTB (%pe)"
is:
  "GT0: GuC CT Failed to control/%s CTB (%pe)"

unless we keep colon (as suggested by Tvrtko) as then:

  "GT0: GuC CT: Failed to control/%s CTB (%pe)"

Blanket added the colon makes it messy when a string actually wants to
start with the prefix. The rule I've been using is lower case word when
the prefix was part of the string, upper case word when the prefix is

Hmm, I'm not sure that we should attempt to have such a flexible rule as
we shouldn't rely too much on actual format of the prefix as it could be
changed any time.  All we should know about final log message is that it
_will_ properly identify the "GT" or "GuC" that this log is related to.

So I would suggest to be just consistent and probably always start with
upper case, as that seems to be mostly used in kernel error logs, and
just make sure that any prefix will honor that (by including colon, or
braces), so this will always work like:

"[drm] *ERROR* GT0: Failed to foo (-EIO)"
"[drm] *ERROR* GT0: GUC: Failed to foo (-EIO)"
"[drm] *ERROR* GT0: GUC: CT: Failed to foo (-EIO)"

or

"[drm] *ERROR* GT0: Failed to foo (-EIO)"
"[drm] *ERROR* GT0: [GUC] Failed to foo (-EIO)"
"[drm] *ERROR* GT0: [GUC] CT: Failed to foo (-EIO)"

and even for:

"[drm] *ERROR* GT(root) Failed to foo (-EIO)"
"[drm] *ERROR* GuC(media) Failed to foo (-EIO)"
"[drm] *ERROR* GT0 [GuC:CT] Failed to foo (-EIO)"

All of which are hideous/complex/verbose/inconsistent. 'GT0: GUC: CT:'?
Really? Or 'GT0: [GUC] CT:'? Why

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Greg KH]

On Thu, Dec 01, 2022 at 11:00:44AM +0100, Noralf Trønnes wrote:
> 
> 
> Den 01.12.2022 06.55, skrev Greg KH:
> > On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 Submission 
> > Endpoint wrote:
> >> Hi,
> >>
> >> I have started to look at igt for testing and want to use CRC tests. To
> >> implement support for this I need to move away from the simple kms
> >> helper.
> >>
> >> When looking around for examples I came across Thomas' nice shadow
> >> helper and thought, yes this is perfect for drm/gud. So I'll switch to
> >> that before I move away from the simple kms helper.
> >>
> >> The async framebuffer flushing code path now uses a shadow buffer and
> >> doesn't touch the framebuffer when it shouldn't. I have also taken the
> >> opportunity to inline the synchronous flush code path and make this the
> >> default flushing stategy.
> >>
> >> Noralf.
> >>
> >> Cc: Maxime Ripard 
> >> Cc: Thomas Zimmermann 
> >> Cc: dri-devel@lists.freedesktop.org
> >> Signed-off-by: Noralf Trønnes 
> >>
> >> ---
> >> Changes in v2:
> >> - Drop patch (Thomas):
> >>   drm/gem: shadow_fb_access: Prepare imported buffers for CPU access
> >> - Use src as variable name for iosys_map (Thomas)
> >> - Prepare imported buffer for CPU access in the driver (Thomas)
> >> - New patch: make sync flushing the default (Thomas)
> >> - Link to v1: 
> >> https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa33...@tronnes.org
> > 
> > 
> > 
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree.  Please read:
> > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > for how to do this properly.
> > 
> > 
> 
> Care to elaborate?
> Is it because stable got the whole patchset and not just the one fix
> patch that cc'ed stable?

That is what triggered this, yes.

> This patchset was sent using the b4 tool and I can't control this
> aspect. Everyone mentioned in the patches gets the whole set.

Fair enough, but watch out, bots will report this as being a problem as
they can't always read through all patches in a series to notice this...

thanks,

greg k-h

[PATCH 2/3] drm/i915: export all mock selftest functions

[Mauro Carvalho Chehab]

In order to prepare for a new KUnit module that will run
selftests, export all mock selftest functions to I915_SELFTEST
namespace.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab 
---

To avoid mailbombing on a large number of people, only mailing lists were C/C 
on the cover.
See [PATCH 0/3] at: 
https://lore.kernel.org/all/cover.1669897668.git.mche...@kernel.org/

 drivers/gpu/drm/i915/gem/selftests/huge_pages.c  | 1 +
 drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 1 +
 drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c | 1 +
 drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c   | 1 +
 drivers/gpu/drm/i915/gt/selftest_engine_cs.c | 1 +
 drivers/gpu/drm/i915/gt/selftest_ring.c  | 1 +
 drivers/gpu/drm/i915/gt/selftest_timeline.c  | 1 +
 drivers/gpu/drm/i915/gt/st_shmem_utils.c | 1 +
 drivers/gpu/drm/i915/selftests/i915_gem_evict.c  | 1 +
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c| 1 +
 drivers/gpu/drm/i915/selftests/i915_request.c| 1 +
 drivers/gpu/drm/i915/selftests/i915_selftest.c   | 1 +
 drivers/gpu/drm/i915/selftests/i915_sw_fence.c   | 1 +
 drivers/gpu/drm/i915/selftests/i915_syncmap.c| 1 +
 drivers/gpu/drm/i915/selftests/i915_vma.c| 1 +
 drivers/gpu/drm/i915/selftests/intel_memory_region.c | 1 +
 drivers/gpu/drm/i915/selftests/intel_uncore.c| 1 +
 drivers/gpu/drm/i915/selftests/scatterlist.c | 1 +
 18 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c 
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index beaf27e09e8a..954d37552681 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1944,6 +1944,7 @@ int i915_gem_huge_page_mock_selftests(void)
mock_destroy_device(dev_priv);
return err;
 }
+EXPORT_SYMBOL_NS_GPL(i915_gem_huge_page_mock_selftests, I915_SELFTEST);
 
 int i915_gem_huge_page_live_selftests(struct drm_i915_private *i915)
 {
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index e57f9390076c..2f6422eb9801 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -543,6 +543,7 @@ int i915_gem_dmabuf_mock_selftests(void)
mock_destroy_device(i915);
return err;
 }
+EXPORT_SYMBOL_NS_GPL(i915_gem_dmabuf_mock_selftests, I915_SELFTEST);
 
 int i915_gem_dmabuf_live_selftests(struct drm_i915_private *i915)
 {
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
index bdf5bb40ccf1..4c50be935462 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
@@ -88,6 +88,7 @@ int i915_gem_object_mock_selftests(void)
mock_destroy_device(i915);
return err;
 }
+EXPORT_SYMBOL_NS_GPL(i915_gem_object_mock_selftests, I915_SELFTEST);
 
 int i915_gem_object_live_selftests(struct drm_i915_private *i915)
 {
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
index d43d8dae0f69..03cd27066153 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
@@ -85,3 +85,4 @@ int i915_gem_phys_mock_selftests(void)
mock_destroy_device(i915);
return err;
 }
+EXPORT_SYMBOL_NS_GPL(i915_gem_phys_mock_selftests, I915_SELFTEST);
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c 
b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
index 881b64f3e7b9..e3e4918b3f9e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
@@ -437,3 +437,4 @@ int intel_engine_cs_mock_selftests(void)
 
return i915_subtests(tests, NULL);
 }
+EXPORT_SYMBOL_NS_GPL(intel_engine_cs_mock_selftests, I915_SELFTEST);
diff --git a/drivers/gpu/drm/i915/gt/selftest_ring.c 
b/drivers/gpu/drm/i915/gt/selftest_ring.c
index 2a8c534dc125..6590c9c504b9 100644
--- a/drivers/gpu/drm/i915/gt/selftest_ring.c
+++ b/drivers/gpu/drm/i915/gt/selftest_ring.c
@@ -108,3 +108,4 @@ int intel_ring_mock_selftests(void)
 
return i915_subtests(tests, NULL);
 }
+EXPORT_SYMBOL_NS_GPL(intel_ring_mock_selftests, I915_SELFTEST);
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c 
b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index 522d0190509c..fcf044c9feea 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -450,6 +450,7 @@ int intel_timeline_mock_selftests(void)
 
return i915_subtests(tests, NULL);
 }
+EXPORT_SYMBOL_NS_GPL(intel_timeline_mock_selftests, I915_SELFTEST);
 
 static int emit_ggtt_store_dw(struct i915_request *rq, u32 addr, u32 value)
 {
diff --git a/drivers/gpu/drm/i915/gt/st_shmem_utils.c 
b/drivers/gpu/drm/i915/gt/st_shmem_utils.c
ind

[PATCH 1/3] drm/i915: place selftest preparation on a separate function

[Mauro Carvalho Chehab]

The selftest preparation logic should also be used by KUnit. So,
place it on a separate function and export it.

Signed-off-by: Mauro Carvalho Chehab 
---

To avoid mailbombing on a large number of people, only mailing lists were C/C 
on the cover.
See [PATCH 0/3] at: 
https://lore.kernel.org/all/cover.1669897668.git.mche...@kernel.org/

 drivers/gpu/drm/i915/i915_selftest.h  |  2 ++
 .../gpu/drm/i915/selftests/i915_selftest.c| 22 ---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_selftest.h 
b/drivers/gpu/drm/i915/i915_selftest.h
index bdf3e22c0a34..cd0065033ed9 100644
--- a/drivers/gpu/drm/i915/i915_selftest.h
+++ b/drivers/gpu/drm/i915/i915_selftest.h
@@ -44,6 +44,7 @@ struct i915_selftest {
 
 extern struct i915_selftest i915_selftest;
 
+void i915_prepare_selftests(const char *name);
 int i915_mock_selftests(void);
 int i915_live_selftests(struct pci_dev *pdev);
 int i915_perf_selftests(struct pci_dev *pdev);
@@ -113,6 +114,7 @@ int __i915_subtests(const char *caller,
 
 #else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
 
+static inline void i915_prepare_selftests(const char *) {};
 static inline int i915_mock_selftests(void) { return 0; }
 static inline int i915_live_selftests(struct pci_dev *pdev) { return 0; }
 static inline int i915_perf_selftests(struct pci_dev *pdev) { return 0; }
diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c 
b/drivers/gpu/drm/i915/selftests/i915_selftest.c
index 39da0fb0d6d2..bc85dac4eb15 100644
--- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
+++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
@@ -127,13 +127,8 @@ static void set_default_test_all(struct selftest *st, 
unsigned int count)
st[i].enabled = true;
 }
 
-static int __run_selftests(const char *name,
-  struct selftest *st,
-  unsigned int count,
-  void *data)
+void i915_prepare_selftests(const char *name)
 {
-   int err = 0;
-
while (!i915_selftest.random_seed)
i915_selftest.random_seed = get_random_u32();
 
@@ -142,10 +137,21 @@ static int __run_selftests(const char *name,
msecs_to_jiffies_timeout(i915_selftest.timeout_ms) :
MAX_SCHEDULE_TIMEOUT;
 
-   set_default_test_all(st, count);
-
pr_info(DRIVER_NAME ": Performing %s selftests with st_random_seed=0x%x 
st_timeout=%u\n",
name, i915_selftest.random_seed, i915_selftest.timeout_ms);
+}
+EXPORT_SYMBOL_NS_GPL(i915_prepare_selftests, I915_SELFTEST);
+
+static int __run_selftests(const char *name,
+  struct selftest *st,
+  unsigned int count,
+  void *data)
+{
+   int err = 0;
+
+   i915_prepare_selftests(name);
+
+   set_default_test_all(st, count);
 
/* Tests are listed in order in i915_*_selftests.h */
for (; count--; st++) {
-- 
2.38.1

[PATCH 3/3] drm/i915: allow running mock selftests via Kunit

[Mauro Carvalho Chehab]

The mock selftests don't require any hardware to run. They can
easily run via kunit with qemu or bare metal.

Add support for it.

With this change, mock selftest can now run in qemu with:

$ ./tools/testing/kunit/kunit.py run --arch=x86_64 \
--kunitconfig=drivers/gpu/drm/i915/selftests
[16:50:24] Configuring KUnit Kernel ...
[16:50:24] Building KUnit Kernel ...
Populating config with:
$ make ARCH=x86_64 O=.kunit olddefconfig
Building with:
$ make ARCH=x86_64 O=.kunit --jobs=8
[16:50:32] Starting KUnit Kernel (1/1)...
[16:50:32] 
Running tests with:
$ qemu-system-x86_64 -nodefaults -m 1024 -kernel 
.kunit/arch/x86/boot/bzImage -append 'kunit.enable=1 console=ttyS0 
kunit_shutdown=reboot' -no-reboot -nographic -serial stdio
[16:50:33]  i915 mock selftests (18 subtests) =
[16:50:33] [PASSED] mock_sanitycheck
[16:50:33] [PASSED] mock_shmem
[16:50:37] [PASSED] mock_fence
[16:50:38] [PASSED] mock_scatterlist
[16:50:39] [PASSED] mock_syncmap
[16:50:39] [PASSED] mock_uncore
[16:50:39] [PASSED] mock_ring
[16:50:39] [PASSED] mock_engine
[16:50:43] [PASSED] mock_timelines
[16:50:45] [PASSED] mock_requests
[16:50:45] [PASSED] mock_objects
[16:50:45] [PASSED] mock_phys
[16:50:45] [PASSED] mock_dmabuf
[16:50:50] [PASSED] mock_vma
[16:50:51] [PASSED] mock_evict
[16:50:53] [PASSED] mock_gtt
[16:50:54] [PASSED] mock_hugepages
[16:50:55] [PASSED] mock_memory_region
[16:50:55] === [PASSED] i915 mock selftests ===
[16:50:55] 
[16:50:55] Testing complete. Ran 18 tests: passed: 18
[16:50:55] Elapsed time: 31.530s total, 0.003s configuring, 8.512s 
building, 22.974s running

It is also possible to run the tests on a bare metal machine,
and even collect code coverage data with:

$ sudo lcov -z && sudo modprobe test-i915 && sudo rmmod test-i915 &&
  sudo IGT_KERNEL_TREE=~/linux 
~/freedesktop-igt/scripts/code_cov_capture mock_selftest
Auto-detecting gcov kernel support.
Found upstream gcov kernel support at /sys/kernel/debug/gcov
Resetting kernel execution counters
Done.
[627.14] Code coverage wrote to mock_selftest.info

The KTAP and Kernel logs will be similar to:

[  596.382685] # Subtest: i915 mock selftests
[  596.382688] 1..18
[  596.387744] i915: i915_mock_sanitycheck() - ok!
[  596.395423] ok 1 - mock_sanitycheck
[  596.395495] i915: Running shmem_utils_mock_selftests/igt_shmem_basic
[  596.406650] ok 2 - mock_shmem
[  596.406737] i915: Running i915_sw_fence_mock_selftests/test_self
[  596.416868] i915: Running i915_sw_fence_mock_selftests/test_dag
[  596.423220] i915: Running i915_sw_fence_mock_selftests/test_AB
[  596.429585] i915: Running i915_sw_fence_mock_selftests/test_ABC
[  596.435921] i915: Running i915_sw_fence_mock_selftests/test_AB_C
[  596.442293] i915: Running i915_sw_fence_mock_selftests/test_C_AB
[  596.448671] i915: Running i915_sw_fence_mock_selftests/test_chain
[  596.485336] i915: Running i915_sw_fence_mock_selftests/test_ipc
[  596.492984] i915: Running i915_sw_fence_mock_selftests/test_timer
[  602.484395] i915: Running i915_sw_fence_mock_selftests/test_dma_fence
[  603.876315] Asynchronous wait on fence mock:mock:0 timed out 
(hint:fence_notify [i915])
[  603.886148] ok 3 - mock_fence
[  603.886377] i915: Running scatterlist_mock_selftests/igt_sg_alloc
[  604.398052] sg_alloc_table timed out
[  604.401979] i915: Running scatterlist_mock_selftests/igt_sg_trim
[  604.909003] i915_sg_trim timed out
[  604.912850] ok 4 - mock_scatterlist
[  604.912987] i915: Running 
i915_syncmap_mock_selftests/igt_syncmap_init
[  604.924092] i915: Running i915_syncmap_mock_selftests/igt_syncmap_one
[  605.430961] i915: Running 
i915_syncmap_mock_selftests/igt_syncmap_join_above
[  605.438458] i915: Running 
i915_syncmap_mock_selftests/igt_syncmap_join_below
[  605.446670] i915: Running 
i915_syncmap_mock_selftests/igt_syncmap_neighbours
[  606.736462] i915: Running 
i915_syncmap_mock_selftests/igt_syncmap_compact
[  606.744342] i915: Running 
i915_syncmap_mock_selftests/igt_syncmap_random
[  607.266569] ok 5 - mock_syncmap
[  607.266771] ok 6 - mock_uncore
[  607.271144] i915: Running 
intel_ring_mock_selftests/igt_ring_direction
[  607.281872] ok 7 - mock_ring
[  607.282142] i915: Running 
intel_engine

[PATCH 0/3] Add KUnit support for i915 mock selftests

[Mauro Carvalho Chehab]

That's an updated version of my previous KUnit RFC series:
https://patchwork.freedesktop.org/series/110481/

While the RFC series added support for live and perf, let's start with
mock, as running tests in bare metal is not the current focus of KUnit.
So, basically patch 1 was changed to export just mock functions,
and the bare metal patches got removed from this version.

As before, running KUnit on i915 driver requires the --arch parameter:

./tools/testing/kunit/kunit.py run --arch=x86_64 
--kunitconfig=drivers/gpu/drm/i915/selftests/  --jobs=`nproc --all`
[13:18:40] Configuring KUnit Kernel ...
[13:18:40] Building KUnit Kernel ...
Populating config with:
$ make ARCH=x86_64 O=.kunit olddefconfig
Building with:
$ make ARCH=x86_64 O=.kunit --jobs=8
[13:23:20] Starting KUnit Kernel (1/1)...
[13:23:20] 
Running tests with:
$ qemu-system-x86_64 -nodefaults -m 1024 -kernel .kunit/arch/x86/boot/bzImage 
-append 'kunit.enable=1 console=ttyS0 kunit_shutdown=reboot' -no-reboot 
-nographic -serial stdio
[13:23:21]  i915 mock selftests (18 subtests) =
[13:23:21] [PASSED] mock_sanitycheck
[13:23:21] [PASSED] mock_shmem
[13:23:24] [PASSED] mock_fence
[13:23:25] [PASSED] mock_scatterlist
[13:23:27] [PASSED] mock_syncmap
[13:23:27] [PASSED] mock_uncore
[13:23:27] [PASSED] mock_ring
[13:23:27] [PASSED] mock_engine
[13:23:31] [PASSED] mock_timelines
[13:23:32] [PASSED] mock_requests
[13:23:32] [PASSED] mock_objects
[13:23:32] [PASSED] mock_phys
[13:23:32] [PASSED] mock_dmabuf
[13:23:38] [PASSED] mock_vma
[13:23:38] [PASSED] mock_evict
[13:23:41] [PASSED] mock_gtt
[13:23:42] [PASSED] mock_hugepages
[13:23:42] [PASSED] mock_memory_region
[13:23:42] === [PASSED] i915 mock selftests ===
[13:23:42] 
[13:23:42] Testing complete. Ran 18 tests: passed: 18
[13:23:42] Elapsed time: 302.766s total, 0.003s configuring, 280.393s building, 
22.341s running

Mauro Carvalho Chehab (3):
  drm/i915: place selftest preparation on a separate function
  drm/i915: export all mock selftest functions
  drm/i915: allow running mock selftests via Kunit

 drivers/gpu/drm/i915/Kconfig  | 15 +++
 drivers/gpu/drm/i915/Makefile |  5 +
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  1 +
 .../drm/i915/gem/selftests/i915_gem_dmabuf.c  |  1 +
 .../drm/i915/gem/selftests/i915_gem_object.c  |  1 +
 .../drm/i915/gem/selftests/i915_gem_phys.c|  1 +
 drivers/gpu/drm/i915/gt/selftest_engine_cs.c  |  1 +
 drivers/gpu/drm/i915/gt/selftest_ring.c   |  1 +
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |  1 +
 drivers/gpu/drm/i915/gt/st_shmem_utils.c  |  1 +
 drivers/gpu/drm/i915/i915_selftest.h  |  2 +
 drivers/gpu/drm/i915/selftests/.kunitconfig   | 12 +++
 .../gpu/drm/i915/selftests/i915_gem_evict.c   |  1 +
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  1 +
 drivers/gpu/drm/i915/selftests/i915_kunit.c   | 95 +++
 drivers/gpu/drm/i915/selftests/i915_request.c |  1 +
 .../gpu/drm/i915/selftests/i915_selftest.c| 23 +++--
 .../gpu/drm/i915/selftests/i915_sw_fence.c|  1 +
 drivers/gpu/drm/i915/selftests/i915_syncmap.c |  1 +
 drivers/gpu/drm/i915/selftests/i915_vma.c |  1 +
 .../drm/i915/selftests/intel_memory_region.c  |  1 +
 drivers/gpu/drm/i915/selftests/intel_uncore.c |  1 +
 drivers/gpu/drm/i915/selftests/scatterlist.c  |  1 +
 23 files changed, 161 insertions(+), 8 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/selftests/.kunitconfig
 create mode 100644 drivers/gpu/drm/i915/selftests/i915_kunit.c

-- 
2.38.1

Re: [PATCH v2 15/17] drm/vc4: tests: Introduce a mocking infrastructure

[Maxime Ripard]

Hi Javier,

On Wed, Nov 30, 2022 at 10:59:37AM +0100, Javier Martinez Canillas wrote:
> On 11/28/22 15:53, Maxime Ripard wrote:
> > In order to test the current atomic_check hooks we need to have a DRM
> > device that has roughly the same capabilities and layout that the actual
> > hardware. We'll also need a bunch of functions to create arbitrary
> > atomic states.
> > 
> > Let's create some helpers to create a device that behaves like the real
> > one, and some helpers to maintain the atomic state we want to check.
> > 
> > Signed-off-by: Maxime Ripard 
> > ---
> 
> [...]
> 
> > +
> > +config DRM_VC4_KUNIT_TEST
> > +   bool "KUnit tests for VC4" if !KUNIT_ALL_TESTS
> > +   depends on DRM_VC4 && KUNIT
> 
> shouldn't this depend on DRM_KUNIT_TEST instead ?
> 
> [...]

You're right, but the rework suggested by Maíra will add a select to the
helpers Kconfig symbol there so we should be safe.

> > +static struct vc4_dev *__mock_device(struct kunit *test, bool is_vc5)
> > +{
> > +   struct drm_device *drm;
> > +   const struct drm_driver *drv = is_vc5 ? &vc5_drm_driver : 
> > &vc4_drm_driver;
> > +   const struct vc4_mock_desc *desc = is_vc5 ? &vc5_mock : &vc4_mock;
> > +   struct vc4_dev *vc4;
> 
> Since it could be vc4 or vc5, maybe can be renamed to just struct vc_dev *vc ?

vc4_dev is the main structure in the driver for the DRM device, so we
can't rename it easily.

Generally speaking the driver was (and still is) called vc4 after the IP
name in the original RaspberryPi SoC.

There's been a new generation since, but we supported it through the vc4
driver. Even if it's a bit ambiguous, vc4 refers to both the driver name
and is used extensively in the infrastructure, but also refers to the
initial generation we supported. vc5 is only the new generation.

I'm not sure removing the number would be less confusing.

> > +struct vc4_dummy_plane *vc4_dummy_plane(struct kunit *test,
> > +   struct drm_device *drm,
> > +   enum drm_plane_type type)
> > +{
> > +   struct vc4_dummy_plane *dummy_plane;
> > +   struct drm_plane *plane;
> > +
> > +   dummy_plane = drmm_universal_plane_alloc(drm,
> > +struct vc4_dummy_plane, 
> > plane.base,
> > +0,
> > +&vc4_dummy_plane_funcs,
> > +vc4_dummy_plane_formats,
> > +
> > ARRAY_SIZE(vc4_dummy_plane_formats),
> > +NULL,
> > +DRM_PLANE_TYPE_PRIMARY,
> > +NULL);
> > +   KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy_plane);
> > +
> > +   plane = &dummy_plane->plane.base;
> > +   drm_plane_helper_add(plane, &vc4_dummy_plane_helper_funcs);
> > +
> > +   return dummy_plane;
> > +}
> 
> I guess many of these helpers could grow to be generic, like this one since
> most drivers support the DRM_FORMAT_XRGB format for their primary plane.

Yeah, that's what I'd expect at some point as well :)

> > +extern const struct vc4_pv_data bcm2835_pv0_data;
> > +extern const struct vc4_pv_data bcm2835_pv1_data;
> > +extern const struct vc4_pv_data bcm2835_pv2_data;
> > +extern const struct vc4_pv_data bcm2711_pv0_data;
> > +extern const struct vc4_pv_data bcm2711_pv1_data;
> > +extern const struct vc4_pv_data bcm2711_pv2_data;
> > +extern const struct vc4_pv_data bcm2711_pv3_data;
> > +extern const struct vc4_pv_data bcm2711_pv4_data;
> > +
> 
> Maybe the driver could expose a helper function to get the pixelvalve data
> and avoid having to expose all of these variables? For example you could
> define an enum vc4_pixelvalve type and have something like the following:
> 
> const struct vc4_pv_data *vc4_crtc_get_pixelvalve_data(enum vc4_pixelvalve 
> pv);
> 
> All these are small nits though, the patch looks great to me and I think is
> awesome to have this level of testing with KUnit. Hope other drivers follow
> your lead.

I'm not sure. It adds an interface for something we don't really need,
so I'm not sure if it's really beneficial.

David pointed at that patch though, which seems more promising:
https://lore.kernel.org/linux-kselftest/20221102175959.2921063-1-rm...@google.com/

Maxime


signature.asc
Description: PGP signature

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Noralf Trønnes]

Den 01.12.2022 13.12, skrev Greg KH:
> On Thu, Dec 01, 2022 at 11:00:44AM +0100, Noralf Trønnes wrote:
>>
>>
>> Den 01.12.2022 06.55, skrev Greg KH:
>>> On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 Submission 
>>> Endpoint wrote:
 Hi,

 I have started to look at igt for testing and want to use CRC tests. To
 implement support for this I need to move away from the simple kms
 helper.

 When looking around for examples I came across Thomas' nice shadow
 helper and thought, yes this is perfect for drm/gud. So I'll switch to
 that before I move away from the simple kms helper.

 The async framebuffer flushing code path now uses a shadow buffer and
 doesn't touch the framebuffer when it shouldn't. I have also taken the
 opportunity to inline the synchronous flush code path and make this the
 default flushing stategy.

 Noralf.

 Cc: Maxime Ripard 
 Cc: Thomas Zimmermann 
 Cc: dri-devel@lists.freedesktop.org
 Signed-off-by: Noralf Trønnes 

 ---
 Changes in v2:
 - Drop patch (Thomas):
   drm/gem: shadow_fb_access: Prepare imported buffers for CPU access
 - Use src as variable name for iosys_map (Thomas)
 - Prepare imported buffer for CPU access in the driver (Thomas)
 - New patch: make sync flushing the default (Thomas)
 - Link to v1: 
 https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa33...@tronnes.org
>>>
>>> 
>>>
>>> This is not the correct way to submit patches for inclusion in the
>>> stable kernel tree.  Please read:
>>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>> for how to do this properly.
>>>
>>> 
>>
>> Care to elaborate?
>> Is it because stable got the whole patchset and not just the one fix
>> patch that cc'ed stable?
> 
> That is what triggered this, yes.
> 
>> This patchset was sent using the b4 tool and I can't control this
>> aspect. Everyone mentioned in the patches gets the whole set.
> 
> Fair enough, but watch out, bots will report this as being a problem as
> they can't always read through all patches in a series to notice this...
> 

Konstantin,

Can you add a rule in b4 to exclude sta...@vger.kernel.org
(sta...@kernel.org as well?) from getting the whole patchset?

Noralf.

Re: [PATCH v2 01/11] pwm: Make .get_state() callback return an error code

[Uwe Kleine-König]

Hello Andre,

On Thu, Dec 01, 2022 at 10:22:52AM +, Andre Przywara wrote:
> Just one comment: I don't see a sunxi specific patch later in the series,
> though it seems we have at least one error error exit (see prescaler == 0
> above). Plus potentially another exit if clk_get_rate() (at the very
> beginning) fails.
> Shall I send a patch for that?

That would we very welcome. I mentioned that shortly in the cover
letter, I wasn't entirely sure how to handle that prescaler = 0 case.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |


signature.asc
Description: PGP signature

Re: [PATCH v2 00/11] pwm: Allow .get_state to fail

[Uwe Kleine-König]

Hello Conor,

On Thu, Dec 01, 2022 at 11:11:51AM +, Conor Dooley wrote:
> TL;DR, I quite like the ability to return an error and not mislead the
> caller.

Is this an Ack?

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |


signature.asc
Description: PGP signature

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Greg KH]

On Thu, Dec 01, 2022 at 02:14:42PM +0100, Noralf Trønnes wrote:
> 
> 
> Den 01.12.2022 13.12, skrev Greg KH:
> > On Thu, Dec 01, 2022 at 11:00:44AM +0100, Noralf Trønnes wrote:
> >>
> >>
> >> Den 01.12.2022 06.55, skrev Greg KH:
> >>> On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 
> >>> Submission Endpoint wrote:
>  Hi,
> 
>  I have started to look at igt for testing and want to use CRC tests. To
>  implement support for this I need to move away from the simple kms
>  helper.
> 
>  When looking around for examples I came across Thomas' nice shadow
>  helper and thought, yes this is perfect for drm/gud. So I'll switch to
>  that before I move away from the simple kms helper.
> 
>  The async framebuffer flushing code path now uses a shadow buffer and
>  doesn't touch the framebuffer when it shouldn't. I have also taken the
>  opportunity to inline the synchronous flush code path and make this the
>  default flushing stategy.
> 
>  Noralf.
> 
>  Cc: Maxime Ripard 
>  Cc: Thomas Zimmermann 
>  Cc: dri-devel@lists.freedesktop.org
>  Signed-off-by: Noralf Trønnes 
> 
>  ---
>  Changes in v2:
>  - Drop patch (Thomas):
>    drm/gem: shadow_fb_access: Prepare imported buffers for CPU access
>  - Use src as variable name for iosys_map (Thomas)
>  - Prepare imported buffer for CPU access in the driver (Thomas)
>  - New patch: make sync flushing the default (Thomas)
>  - Link to v1: 
>  https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa33...@tronnes.org
> >>>
> >>> 
> >>>
> >>> This is not the correct way to submit patches for inclusion in the
> >>> stable kernel tree.  Please read:
> >>> 
> >>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> >>> for how to do this properly.
> >>>
> >>> 
> >>
> >> Care to elaborate?
> >> Is it because stable got the whole patchset and not just the one fix
> >> patch that cc'ed stable?
> > 
> > That is what triggered this, yes.
> > 
> >> This patchset was sent using the b4 tool and I can't control this
> >> aspect. Everyone mentioned in the patches gets the whole set.
> > 
> > Fair enough, but watch out, bots will report this as being a problem as
> > they can't always read through all patches in a series to notice this...
> > 
> 
> Konstantin,
> 
> Can you add a rule in b4 to exclude sta...@vger.kernel.org
> (sta...@kernel.org as well?) from getting the whole patchset?

sta...@kernel.org is a pipe to /dev/null so that's not needed to be
messed with.

As for this needing special casing in b4, it's rare that you send out a
patch series and only want 1 or 2 of them in stable, right?

thanks,

greg k-h

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Javier Martinez Canillas]

Hello Greg,

On 12/1/22 14:21, Greg KH wrote:

[...]

 This patchset was sent using the b4 tool and I can't control this
 aspect. Everyone mentioned in the patches gets the whole set.
>>>
>>> Fair enough, but watch out, bots will report this as being a problem as
>>> they can't always read through all patches in a series to notice this...
>>>
>>
>> Konstantin,
>>
>> Can you add a rule in b4 to exclude sta...@vger.kernel.org
>> (sta...@kernel.org as well?) from getting the whole patchset?
> 
> sta...@kernel.org is a pipe to /dev/null so that's not needed to be
> messed with.
> 
> As for this needing special casing in b4, it's rare that you send out a
> patch series and only want 1 or 2 of them in stable, right?
>

Not really, it's very common for a patch-series to contain fixes (that could
go to stable if applicable) and change that are not suitable for stable. The
problem as Noralf mentioned is that the b4 tool doesn't seem to allow Cc'ing
individual patches to different recipients, and everyone get the whole set.

So either b4 needs to have this support, exclude sta...@vger.kernel.org when
sending a set or sta...@vger.kernel.org ignore patches without a Fixes: tag.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 00/11] pwm: Allow .get_state to fail

[Conor Dooley]

On Thu, Dec 01, 2022 at 02:19:07PM +0100, Uwe Kleine-König wrote:
> Hello Conor,
> 
> On Thu, Dec 01, 2022 at 11:11:51AM +, Conor Dooley wrote:
> > TL;DR, I quite like the ability to return an error and not mislead the
> > caller.
> 
> Is this an Ack?

It is if you want it to be! I didn't really feel qualified to do so
which is why I gave some context etc.

I did check out the callsites for the non-void returning op, and it
looked good to me, so sure, why not:

Acked-by: Conor Dooley 

Thanks,
Conor.

Re: Screen corruption using radeon kernel driver

[Robin Murphy]

On 2022-11-30 19:59, Mikhail Krylov wrote:

On Wed, Nov 30, 2022 at 11:07:32AM -0500, Alex Deucher wrote:

On Wed, Nov 30, 2022 at 10:42 AM Robin Murphy  wrote:


On 2022-11-30 14:28, Alex Deucher wrote:

On Wed, Nov 30, 2022 at 7:54 AM Robin Murphy  wrote:


On 2022-11-29 17:11, Mikhail Krylov wrote:

On Tue, Nov 29, 2022 at 11:05:28AM -0500, Alex Deucher wrote:

On Tue, Nov 29, 2022 at 10:59 AM Mikhail Krylov  wrote:


On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote:

On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov  wrote:


On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher wrote:


[excessive quoting removed]



So, is there any progress on this issue? I do understand it's not a high
priority one, and today I've checked it on 6.0 kernel, and
unfortunately, it still persists...

I'm considering writing a patch that will allow user to override
need_dma32/dma_bits setting with a module parameter. I'll have some time
after the New Year for that.

Is it at all possible that such a patch will be merged into kernel?


On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov  wrote:
Unless someone familiar with HIMEM can figure out what is going wrong
we should just revert the patch.

Alex



Okay, I was suggesting that mostly because

a) it works for me with dma_bits = 40 (I understand that's what it is
without the original patch applied);

b) there's a hint of uncertainity on this line
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359
saying that for AGP dma_bits = 32 is the safest option, so apparently there are
setups, unlike mine, where dma_bits = 32 is better than 40.

But I'm in no position to argue, just wanted to make myself clear.
I'm okay with rebuilding the kernel for my machine until the original
patch is reverted or any other fix is applied.


What GPU do you have and is it AGP?  If it is AGP, does setting
radeon.agpmode=-1 also fix it?

Alex


That is ATI Radeon X1950, and, unfortunately, radeon.agpmode=-1 doesn't
help, it just makes 3D acceleration in games such as OpenArena stop
working.


Just to confirm, is the board AGP or PCIe?

Alex


It is AGP. That's an old machine.


Can you check whether dma_addressing_limited() is actually returning the
expected result at the point of radeon_ttm_init()? Disabling highmem is
presumably just hiding whatever problem exists, by throwing away all
   >32-bit RAM such that use_dma32 doesn't matter.


The device in question only supports a 32 bit DMA mask so
dma_addressing_limited() should return true.  Bounce buffers are not
really usable on GPUs because they map so much memory.  If
dma_addressing_limited() returns false, that would explain it.


Right, it appears to be the only part of the offending commit that
*could* reasonably make any difference, so I'm primarily wondering if
dma_get_required_mask() somehow gets confused.


Mikhail,

Can you see that dma_addressing_limited() and dma_get_required_mask()
return in this case?

Alex




Thanks,
Robin.


Unfortunately, right now I don't have enough time for kernel
modifications and rebuilds (I will later!), so I did a quick-and-dirty
research with kprobe.

The problem is that dma_addressing_limited() seems to be inlined and
kprobe fails to intercept it.

But I managed to get the result of dma_get_required_mask(). It returns
0x7fff (!) on the vanilla (with the patch, buggy) kernel:
  
$ sudo kprobe-perf 'r:dma_get_required_mask $retval'

Tracing kprobe dma_get_required_mask. Ctrl-C to end.
 modprobe-1244[000] d...   105.582816: dma_get_required_mask: 
(radeon_ttm_init+0x61/0x240 [radeon] <- dma_get_required_mask) arg1=0x7fff

This function does not even get called in the kernel without the patch
that I built myself. I believe that's because ttm_bo_device_init()
doesn't call it without the patch.

Hope that helps at least a bit. If not, I'll be able to do more thorough
research in a couple of weeks, probably.


Hmm, just to clarify, what's your actual RAM layout? I've been assuming
that the issue must be caused by unexpected DMA address truncation, but
double-checking the older threads it seems that might not be the case.
I just did a quick sanity-check of both HIGHMEM4G and HIGHMEM64G configs
in a VM with either 2GB or 4GB of RAM assigned, and the
dma_direct_get_required_mask() calculation seemed to return the
appropriate result for all combinations.

Otherwise, the only significant difference of use_dma32 seems to be to
switch TTM's allocation flags from GFP_HIGHUSER to GFP_DMA32. Could it
just be that the highmem support somewhere between TTM and radeon has
bitrotted, and it hasn't been noticed until this change because everyone
still using a 32-bit system with highmem also happens not to be using a
newer 40-bit-capable GPU? Or perhaps it never worked for AGP at all, in
which case an explicit special case might be clearer?

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
ind

Re: Screen corruption using radeon kernel driver

[Alex Deucher]

On Thu, Dec 1, 2022 at 9:01 AM Robin Murphy  wrote:
>
> On 2022-11-30 19:59, Mikhail Krylov wrote:
> > On Wed, Nov 30, 2022 at 11:07:32AM -0500, Alex Deucher wrote:
> >> On Wed, Nov 30, 2022 at 10:42 AM Robin Murphy  wrote:
> >>>
> >>> On 2022-11-30 14:28, Alex Deucher wrote:
>  On Wed, Nov 30, 2022 at 7:54 AM Robin Murphy  
>  wrote:
> >
> > On 2022-11-29 17:11, Mikhail Krylov wrote:
> >> On Tue, Nov 29, 2022 at 11:05:28AM -0500, Alex Deucher wrote:
> >>> On Tue, Nov 29, 2022 at 10:59 AM Mikhail Krylov  
> >>> wrote:
> 
>  On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote:
> > On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov  
> > wrote:
> >>
> >> On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher wrote:
> >>
> > [excessive quoting removed]
> >>
>  So, is there any progress on this issue? I do understand it's 
>  not a high
>  priority one, and today I've checked it on 6.0 kernel, and
>  unfortunately, it still persists...
> 
>  I'm considering writing a patch that will allow user to override
>  need_dma32/dma_bits setting with a module parameter. I'll have 
>  some time
>  after the New Year for that.
> 
>  Is it at all possible that such a patch will be merged into 
>  kernel?
> 
> >>> On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov 
> >>>  wrote:
> >>> Unless someone familiar with HIMEM can figure out what is going 
> >>> wrong
> >>> we should just revert the patch.
> >>>
> >>> Alex
> >>
> >>
> >> Okay, I was suggesting that mostly because
> >>
> >> a) it works for me with dma_bits = 40 (I understand that's what it 
> >> is
> >> without the original patch applied);
> >>
> >> b) there's a hint of uncertainity on this line
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359
> >> saying that for AGP dma_bits = 32 is the safest option, so 
> >> apparently there are
> >> setups, unlike mine, where dma_bits = 32 is better than 40.
> >>
> >> But I'm in no position to argue, just wanted to make myself clear.
> >> I'm okay with rebuilding the kernel for my machine until the 
> >> original
> >> patch is reverted or any other fix is applied.
> >
> > What GPU do you have and is it AGP?  If it is AGP, does setting
> > radeon.agpmode=-1 also fix it?
> >
> > Alex
> 
>  That is ATI Radeon X1950, and, unfortunately, radeon.agpmode=-1 
>  doesn't
>  help, it just makes 3D acceleration in games such as OpenArena stop
>  working.
> >>>
> >>> Just to confirm, is the board AGP or PCIe?
> >>>
> >>> Alex
> >>
> >> It is AGP. That's an old machine.
> >
> > Can you check whether dma_addressing_limited() is actually returning the
> > expected result at the point of radeon_ttm_init()? Disabling highmem is
> > presumably just hiding whatever problem exists, by throwing away all
> >>32-bit RAM such that use_dma32 doesn't matter.
> 
>  The device in question only supports a 32 bit DMA mask so
>  dma_addressing_limited() should return true.  Bounce buffers are not
>  really usable on GPUs because they map so much memory.  If
>  dma_addressing_limited() returns false, that would explain it.
> >>>
> >>> Right, it appears to be the only part of the offending commit that
> >>> *could* reasonably make any difference, so I'm primarily wondering if
> >>> dma_get_required_mask() somehow gets confused.
> >>
> >> Mikhail,
> >>
> >> Can you see that dma_addressing_limited() and dma_get_required_mask()
> >> return in this case?
> >>
> >> Alex
> >>
> >>
> >>>
> >>> Thanks,
> >>> Robin.
> >
> > Unfortunately, right now I don't have enough time for kernel
> > modifications and rebuilds (I will later!), so I did a quick-and-dirty
> > research with kprobe.
> >
> > The problem is that dma_addressing_limited() seems to be inlined and
> > kprobe fails to intercept it.
> >
> > But I managed to get the result of dma_get_required_mask(). It returns
> > 0x7fff (!) on the vanilla (with the patch, buggy) kernel:
> >
> > $ sudo kprobe-perf 'r:dma_get_required_mask $retval'
> > Tracing kprobe dma_get_required_mask. Ctrl-C to end.
> >  modprobe-1244[000] d...   105.582816: dma_get_required_mask: 
> > (radeon_ttm_init+0x61/0x240 [radeon] <- dma_get_required_mask) 
> > arg1=0x7fff
> >
> > This function does not even get called in the kernel without the patch
> > that I built myself. I believe that's because ttm_bo_device_init()
> > doesn't call it without the patch.
>

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Konstantin Ryabitsev]

On Thu, Dec 01, 2022 at 02:34:41PM +0100, Javier Martinez Canillas wrote:
> >> Konstantin,
> >>
> >> Can you add a rule in b4 to exclude sta...@vger.kernel.org
> >> (sta...@kernel.org as well?) from getting the whole patchset?
> > 
> > sta...@kernel.org is a pipe to /dev/null so that's not needed to be
> > messed with.
> > 
> > As for this needing special casing in b4, it's rare that you send out a
> > patch series and only want 1 or 2 of them in stable, right?
> >
> 
> Not really, it's very common for a patch-series to contain fixes (that could
> go to stable if applicable) and change that are not suitable for stable. The
> problem as Noralf mentioned is that the b4 tool doesn't seem to allow Cc'ing
> individual patches to different recipients, and everyone get the whole set.
> 
> So either b4 needs to have this support, exclude sta...@vger.kernel.org when
> sending a set or sta...@vger.kernel.org ignore patches without a Fixes: tag.

I think what I can do is a special logic for Cc: trailers:

- Any Cc: trailers we find in the cover letter receive the entire series
- Any Cc: trailers in individual patches only receive these individual patches

Thank you for being patient -- we'll get this right, I promise.

-K

Re: [PATCH v2 01/11] pwm: Make .get_state() callback return an error code

[Andre Przywara]

On Thu, 1 Dec 2022 14:16:04 +0100
Uwe Kleine-König  wrote:

Hi Uwe,

> Hello Andre,
> 
> On Thu, Dec 01, 2022 at 10:22:52AM +, Andre Przywara wrote:
> > Just one comment: I don't see a sunxi specific patch later in the series,
> > though it seems we have at least one error error exit (see prescaler == 0
> > above). Plus potentially another exit if clk_get_rate() (at the very
> > beginning) fails.
> > Shall I send a patch for that?  
> 
> That would we very welcome. I mentioned that shortly in the cover
> letter, I wasn't entirely sure how to handle that prescaler = 0 case.

Ah right, sorry, I missed that.
So the Allwinner manual somehow marks those prescaler encodings as reserved
or invalid (it's just a "/" in there), and we never set those values in the
driver (there is an explicit check). So it could only be a leftover from
firmware/bootloader, or someone poking at this register behind our back.
I am tempted to just return some -EINVAL. As the current code stands, we
don't manipulate any state flags before that check, so it doesn't
really matter, but would be best practise, at least.

Cheers,
Andre

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Javier Martinez Canillas]

On 12/1/22 15:16, Konstantin Ryabitsev wrote:
> On Thu, Dec 01, 2022 at 02:34:41PM +0100, Javier Martinez Canillas wrote:
 Konstantin,

 Can you add a rule in b4 to exclude sta...@vger.kernel.org
 (sta...@kernel.org as well?) from getting the whole patchset?
>>>
>>> sta...@kernel.org is a pipe to /dev/null so that's not needed to be
>>> messed with.
>>>
>>> As for this needing special casing in b4, it's rare that you send out a
>>> patch series and only want 1 or 2 of them in stable, right?
>>>
>>
>> Not really, it's very common for a patch-series to contain fixes (that could
>> go to stable if applicable) and change that are not suitable for stable. The
>> problem as Noralf mentioned is that the b4 tool doesn't seem to allow Cc'ing
>> individual patches to different recipients, and everyone get the whole set.
>>
>> So either b4 needs to have this support, exclude sta...@vger.kernel.org when
>> sending a set or sta...@vger.kernel.org ignore patches without a Fixes: tag.
> 
> I think what I can do is a special logic for Cc: trailers:
> 
> - Any Cc: trailers we find in the cover letter receive the entire series
> - Any Cc: trailers in individual patches only receive these individual patches
>

I think that make sense. It's similar to how for example patman works.
 
> Thank you for being patient -- we'll get this right, I promise.
> 

On the contrary, thanks a lot for working on this tool and for being
that responsive to the feedback.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Vlastimil Babka]

On 12/1/22 15:16, Konstantin Ryabitsev wrote:

On Thu, Dec 01, 2022 at 02:34:41PM +0100, Javier Martinez Canillas wrote:

Konstantin,

Can you add a rule in b4 to exclude sta...@vger.kernel.org
(sta...@kernel.org as well?) from getting the whole patchset?


sta...@kernel.org is a pipe to /dev/null so that's not needed to be
messed with.

As for this needing special casing in b4, it's rare that you send out a
patch series and only want 1 or 2 of them in stable, right?



Not really, it's very common for a patch-series to contain fixes (that could
go to stable if applicable) and change that are not suitable for stable. The
problem as Noralf mentioned is that the b4 tool doesn't seem to allow Cc'ing
individual patches to different recipients, and everyone get the whole set.

So either b4 needs to have this support, exclude sta...@vger.kernel.org when
sending a set or sta...@vger.kernel.org ignore patches without a Fixes: tag.


I think what I can do is a special logic for Cc: trailers:

- Any Cc: trailers we find in the cover letter receive the entire series
- Any Cc: trailers in individual patches only receive these individual patches


I usually do that with git send-email and a custom --cc-cmd script, but 
additionally it sends the cover letter also to everyone that's on any 
individual patch's Cc, so everyone gets at least the cover letter + 
their specific patch(es).


But that extra logic could be optional.


Thank you for being patient -- we'll get this right, I promise.

-K

Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper

[Mark Brown]

On Thu, Dec 01, 2022 at 03:27:32PM +0100, Vlastimil Babka wrote:

> I usually do that with git send-email and a custom --cc-cmd script, but
> additionally it sends the cover letter also to everyone that's on any
> individual patch's Cc, so everyone gets at least the cover letter + their
> specific patch(es).

> But that extra logic could be optional.

Yeah, without the cover letter if you've just got an individual patch it
can be unclear what's going on with dependencies and so on for getting
the patches merged.


signature.asc
Description: PGP signature

[PATCH v5 3/5] drm/i915: Introduce guard pages to i915_vma

[Andi Shyti]

From: Chris Wilson 

Introduce the concept of padding the i915_vma with guard pages before
and after. The major consequence is that all ordinary uses of i915_vma
must use i915_vma_offset/i915_vma_size and not i915_vma.node.start/size
directly, as the drm_mm_node will include the guard pages that surround
our object.

The biggest connundrum is how exactly to mix requesting a fixed address
with guard pages, particularly through the existing uABI. The user does
not know about guard pages, so such must be transparent to the user, and
so the execobj.offset must be that of the object itself excluding the
guard. So a PIN_OFFSET_FIXED must then be exclusive of the guard pages.
The caveat is that some placements will be impossible with guard pages,
as wrap arounds need to be avoided, and the vma itself will require a
larger node. We must not report EINVAL but ENOSPC as these are unavailable
locations within the GTT rather than conflicting user requirements.

In the next patch, we start using guard pages for scanout objects. While
these are limited to GGTT vma, on a few platforms these vma (or at least
an alias of the vma) is shared with userspace, so we may leak the
existence of such guards if we are not careful to ensure that the
execobj.offset is transparent and excludes the guards. (On such platforms
like ivb, without full-ppgtt, userspace has to use relocations so the
presence of more untouchable regions within its GTT such be of no further
issue.)

Signed-off-by: Chris Wilson 
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Tvrtko Ursulin 
Signed-off-by: Andi Shyti 
---
Hi,

this the v5 changelog, the overall changelog is in the v4 cover letter:

v4 -> v5:
 - remove again the GEM_BUG_ON()
 - fix an oversight where the rounding was called without assigning the
   value to the guard.

Thanks Tvrtko for the reviews.

Thanks,
Andi

 drivers/gpu/drm/i915/gt/intel_ggtt.c | 14 +---
 drivers/gpu/drm/i915/i915_gem_gtt.h  |  3 +-
 drivers/gpu/drm/i915/i915_vma.c  | 41 
 drivers/gpu/drm/i915/i915_vma.h  |  5 +--
 drivers/gpu/drm/i915/i915_vma_resource.c |  4 +--
 drivers/gpu/drm/i915/i915_vma_resource.h |  7 +++-
 drivers/gpu/drm/i915/i915_vma_types.h|  1 +
 7 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 7644738b9cdbe..784d4a8c43ba9 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -296,8 +296,11 @@ static void gen8_ggtt_insert_entries(struct 
i915_address_space *vm,
 */
 
gte = (gen8_pte_t __iomem *)ggtt->gsm;
-   gte += vma_res->start / I915_GTT_PAGE_SIZE;
-   end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
+   gte += (vma_res->start - vma_res->guard) / I915_GTT_PAGE_SIZE;
+   end = gte + vma_res->guard / I915_GTT_PAGE_SIZE;
+   while (gte < end)
+   gen8_set_pte(gte++, vm->scratch[0]->encode);
+   end += (vma_res->node_size + vma_res->guard) / I915_GTT_PAGE_SIZE;
 
for_each_sgt_daddr(addr, iter, vma_res->bi.pages)
gen8_set_pte(gte++, pte_encode | addr);
@@ -347,9 +350,12 @@ static void gen6_ggtt_insert_entries(struct 
i915_address_space *vm,
dma_addr_t addr;
 
gte = (gen6_pte_t __iomem *)ggtt->gsm;
-   gte += vma_res->start / I915_GTT_PAGE_SIZE;
-   end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
+   gte += (vma_res->start - vma_res->guard) / I915_GTT_PAGE_SIZE;
 
+   end = gte + vma_res->guard / I915_GTT_PAGE_SIZE;
+   while (gte < end)
+   iowrite32(vm->scratch[0]->encode, gte++);
+   end += (vma_res->node_size + vma_res->guard) / I915_GTT_PAGE_SIZE;
for_each_sgt_daddr(addr, iter, vma_res->bi.pages)
iowrite32(vm->pte_encode(addr, level, flags), gte++);
GEM_BUG_ON(gte > end);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8c2f57eb5ddaa..2434197830523 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -44,7 +44,8 @@ int i915_gem_gtt_insert(struct i915_address_space *vm,
 #define PIN_HIGH   BIT_ULL(5)
 #define PIN_OFFSET_BIASBIT_ULL(6)
 #define PIN_OFFSET_FIXED   BIT_ULL(7)
-#define PIN_VALIDATE   BIT_ULL(8) /* validate placement only, no need 
to call unpin() */
+#define PIN_OFFSET_GUARD   BIT_ULL(8)
+#define PIN_VALIDATE   BIT_ULL(9) /* validate placement only, no need 
to call unpin() */
 
 #define PIN_GLOBAL BIT_ULL(10) /* I915_VMA_GLOBAL_BIND */
 #define PIN_USER   BIT_ULL(11) /* I915_VMA_LOCAL_BIND */
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index fefee5fef38d3..cda90c7818af3 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -419,7 +419,7 @@ i915_vma_resource_init_from_vma(struct i915_vma_resource 
*vma_res,
 

Re: [Intel-gfx] [PATCH v5 3/5] drm/i915: Introduce guard pages to i915_vma

[Tvrtko Ursulin]

On 01/12/2022 14:44, Andi Shyti wrote:

From: Chris Wilson 

Introduce the concept of padding the i915_vma with guard pages before
and after. The major consequence is that all ordinary uses of i915_vma
must use i915_vma_offset/i915_vma_size and not i915_vma.node.start/size
directly, as the drm_mm_node will include the guard pages that surround
our object.

The biggest connundrum is how exactly to mix requesting a fixed address
with guard pages, particularly through the existing uABI. The user does
not know about guard pages, so such must be transparent to the user, and
so the execobj.offset must be that of the object itself excluding the
guard. So a PIN_OFFSET_FIXED must then be exclusive of the guard pages.
The caveat is that some placements will be impossible with guard pages,
as wrap arounds need to be avoided, and the vma itself will require a
larger node. We must not report EINVAL but ENOSPC as these are unavailable
locations within the GTT rather than conflicting user requirements.

In the next patch, we start using guard pages for scanout objects. While
these are limited to GGTT vma, on a few platforms these vma (or at least
an alias of the vma) is shared with userspace, so we may leak the
existence of such guards if we are not careful to ensure that the
execobj.offset is transparent and excludes the guards. (On such platforms
like ivb, without full-ppgtt, userspace has to use relocations so the
presence of more untouchable regions within its GTT such be of no further
issue.)

Signed-off-by: Chris Wilson 
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Tvrtko Ursulin 
Signed-off-by: Andi Shyti 
---
Hi,

this the v5 changelog, the overall changelog is in the v4 cover letter:

v4 -> v5:
  - remove again the GEM_BUG_ON()
  - fix an oversight where the rounding was called without assigning the
value to the guard.

Thanks Tvrtko for the reviews.


I think that was the last open I had so LGTM now. Thanks for the respins!

Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko


Thanks,
Andi

  drivers/gpu/drm/i915/gt/intel_ggtt.c | 14 +---
  drivers/gpu/drm/i915/i915_gem_gtt.h  |  3 +-
  drivers/gpu/drm/i915/i915_vma.c  | 41 
  drivers/gpu/drm/i915/i915_vma.h  |  5 +--
  drivers/gpu/drm/i915/i915_vma_resource.c |  4 +--
  drivers/gpu/drm/i915/i915_vma_resource.h |  7 +++-
  drivers/gpu/drm/i915/i915_vma_types.h|  1 +
  7 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 7644738b9cdbe..784d4a8c43ba9 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -296,8 +296,11 @@ static void gen8_ggtt_insert_entries(struct 
i915_address_space *vm,
 */
  
  	gte = (gen8_pte_t __iomem *)ggtt->gsm;

-   gte += vma_res->start / I915_GTT_PAGE_SIZE;
-   end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
+   gte += (vma_res->start - vma_res->guard) / I915_GTT_PAGE_SIZE;
+   end = gte + vma_res->guard / I915_GTT_PAGE_SIZE;
+   while (gte < end)
+   gen8_set_pte(gte++, vm->scratch[0]->encode);
+   end += (vma_res->node_size + vma_res->guard) / I915_GTT_PAGE_SIZE;
  
  	for_each_sgt_daddr(addr, iter, vma_res->bi.pages)

gen8_set_pte(gte++, pte_encode | addr);
@@ -347,9 +350,12 @@ static void gen6_ggtt_insert_entries(struct 
i915_address_space *vm,
dma_addr_t addr;
  
  	gte = (gen6_pte_t __iomem *)ggtt->gsm;

-   gte += vma_res->start / I915_GTT_PAGE_SIZE;
-   end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
+   gte += (vma_res->start - vma_res->guard) / I915_GTT_PAGE_SIZE;
  
+	end = gte + vma_res->guard / I915_GTT_PAGE_SIZE;

+   while (gte < end)
+   iowrite32(vm->scratch[0]->encode, gte++);
+   end += (vma_res->node_size + vma_res->guard) / I915_GTT_PAGE_SIZE;
for_each_sgt_daddr(addr, iter, vma_res->bi.pages)
iowrite32(vm->pte_encode(addr, level, flags), gte++);
GEM_BUG_ON(gte > end);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8c2f57eb5ddaa..2434197830523 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -44,7 +44,8 @@ int i915_gem_gtt_insert(struct i915_address_space *vm,
  #define PIN_HIGH  BIT_ULL(5)
  #define PIN_OFFSET_BIAS   BIT_ULL(6)
  #define PIN_OFFSET_FIXED  BIT_ULL(7)
-#define PIN_VALIDATE   BIT_ULL(8) /* validate placement only, no need 
to call unpin() */
+#define PIN_OFFSET_GUARD   BIT_ULL(8)
+#define PIN_VALIDATE   BIT_ULL(9) /* validate placement only, no need 
to call unpin() */
  
  #define PIN_GLOBAL		BIT_ULL(10) /* I915_VMA_GLOBAL_BIND */

  #define PIN_USER  BIT_ULL(11) /* I915_VMA_LOCAL_BIND */
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index fefee5fef38d3..cda90c7818af3 100644
--- a/drive

Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX

[俞家鑫]

Re: [PATCH v7 20/20] drm/i915/vm_bind: Async vm_unbind support

[Niranjana Vishwanathapura]

On Thu, Dec 01, 2022 at 10:10:14AM +, Matthew Auld wrote:

On 29/11/2022 23:26, Niranjana Vishwanathapura wrote:

On Wed, Nov 23, 2022 at 11:42:58AM +, Matthew Auld wrote:

On 16/11/2022 00:37, Niranjana Vishwanathapura wrote:
On Tue, Nov 15, 2022 at 03:15:03PM -0800, Niranjana 
Vishwanathapura wrote:
On Tue, Nov 15, 2022 at 08:33:47AM -0800, Niranjana 
Vishwanathapura wrote:

On Tue, Nov 15, 2022 at 04:20:54PM +, Matthew Auld wrote:

On 15/11/2022 16:15, Niranjana Vishwanathapura wrote:

On Tue, Nov 15, 2022 at 11:05:21AM +, Matthew Auld wrote:

On 13/11/2022 07:57, Niranjana Vishwanathapura wrote:

Asynchronously unbind the vma upon vm_unbind call.
Fall back to synchronous unbind if backend doesn't support
async unbind or if async unbind fails.

No need for vm_unbind out fence support as i915 will internally
handle all sequencing and user need not try to sequence any
operation with the unbind completion.

v2: use i915_vma_destroy_async in vm_unbind ioctl

Signed-off-by: Niranjana Vishwanathapura 



This only does it for non-partial vma, right? Or was 
that changed somewhere?




No, it applies to any vma (partial or non-partial).
It was so from the beginning.


Doesn't __i915_vma_unbind_async() return an error when 
mm.pages != vma->pages? IIRC this was discussed before. 
Just trying to think about the consequences of this 
change.


I am not seeing any such restriction. Let me probe and check if there
is any such restriction anywhere in the call chain.


I checked and I am not seeing any restriction anywher in the 
call chain.




Note that just like binding case, unbinding is also synchronous unless
there is a pending activity, in which case, it will be asynchronous.


In __i915_vma_unbind_async() there is:

if (i915_vma_is_pinned(vma) ||
   &vma->obj->mm.rsgt->table != vma->resource->bi.pages)
   return ERR_PTR(-EAGAIN);

AFAICT we then also don't get any pipelined moves with such an 
object, if there is such a binding present.


I had to remove this check as otherwise for VM_BIND (persistent) mappings,
it will alwasy be true and we will always endup with -EAGAIN.
Persistent mappings, as they support partial binding, can't have an sg
table which is just a pointer to object's sg table.


Ok, maybe make that a seperate patch, since it seems to change the 
behaviour for more than just persisent mappings, AFAICT.




Ok, will do.

Niranjana



Niranjana





Niranjana


Niranjana



Niranjana





Niranjana


Reviewed-by: Matthew Auld 


---
 .../drm/i915/gem/i915_gem_vm_bind_object.c    |  2 +-
 drivers/gpu/drm/i915/i915_vma.c   | 51 
+--

 drivers/gpu/drm/i915/i915_vma.h   |  1 +
 include/uapi/drm/i915_drm.h   |  3 +-
 4 files changed, 51 insertions(+), 6 deletions(-)

diff --git 
a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c 
b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c

index d87d1210365b..36651b447966 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
@@ -210,7 +210,7 @@ static int 
i915_gem_vm_unbind_vma(struct i915_address_space 
*vm,

  */
 obj = vma->obj;
 i915_gem_object_lock(obj, NULL);
-    i915_vma_destroy(vma);
+    i915_vma_destroy_async(vma);
 i915_gem_object_unlock(obj);
 i915_gem_object_put(obj);
diff --git a/drivers/gpu/drm/i915/i915_vma.c 
b/drivers/gpu/drm/i915/i915_vma.c

index 7cf77c67d755..483d25f2425c 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -42,6 +42,8 @@
 #include "i915_vma.h"
 #include "i915_vma_resource.h"
+static struct dma_fence 
*__i915_vma_unbind_async(struct i915_vma *vma);

+
 static inline void assert_vma_held_evict(const 
struct i915_vma *vma)

 {
 /*
@@ -1713,7 +1715,7 @@ void i915_vma_reopen(struct i915_vma *vma)
 spin_unlock_irq(>->closed_lock);
 }
-static void force_unbind(struct i915_vma *vma)
+static void force_unbind(struct i915_vma *vma, bool async)
 {
 if (!drm_mm_node_allocated(&vma->node))
 return;
@@ -1727,7 +1729,21 @@ static void 
force_unbind(struct i915_vma *vma)

 i915_vma_set_purged(vma);
 atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
-    WARN_ON(__i915_vma_unbind(vma));
+    if (async) {
+    struct dma_fence *fence;
+
+    fence = __i915_vma_unbind_async(vma);
+    if (IS_ERR_OR_NULL(fence)) {
+    async = false;
+    } else {
+    dma_resv_add_fence(vma->obj->base.resv, fence,
+   DMA_RESV_USAGE_READ);
+    dma_fence_put(fence);
+    }
+    }
+
+    if (!async)
+    WARN_ON(__i915_vma_unbind(vma));
 GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
 }
@@ -1787,7 +1803,7 @@ void 
i915_vma_destroy_locked(struct i915_vma *vma)

 {
 lockdep_assert_held(&vma->vm->mutex);
-    force_unbind(vma);
+    force_unbind(vma, false);
 list_del_init(&vma->vm_link);
 release_references(vma, vma->vm->gt, false);
 }
@@ -1798,7 +181

[PATCH v3 01/20] drm/tests: helpers: Move the helper header to include/drm

[Maxime Ripard]

We'll need to use those helpers from drivers too, so let's move it to a
more visible location.

Reviewed-by: Javier Martinez Canillas 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/tests/drm_client_modeset_test.c| 3 +--
 drivers/gpu/drm/tests/drm_kunit_helpers.c  | 3 +--
 drivers/gpu/drm/tests/drm_modes_test.c | 3 +--
 drivers/gpu/drm/tests/drm_probe_helper_test.c  | 3 +--
 {drivers/gpu/drm/tests => include/drm}/drm_kunit_helpers.h | 0
 5 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c 
b/drivers/gpu/drm/tests/drm_client_modeset_test.c
index 52929536a158..ed2f62e92fea 100644
--- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
+++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
@@ -8,12 +8,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
-#include "drm_kunit_helpers.h"
-
 struct drm_client_modeset_test_priv {
struct drm_device *drm;
struct drm_connector connector;
diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c 
b/drivers/gpu/drm/tests/drm_kunit_helpers.c
index 8c738384a992..6600a4db3158 100644
--- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
+++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
@@ -1,14 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include 
+#include 
 #include 
 
 #include 
 
 #include 
 
-#include "drm_kunit_helpers.h"
-
 struct kunit_dev {
struct drm_device base;
 };
diff --git a/drivers/gpu/drm/tests/drm_modes_test.c 
b/drivers/gpu/drm/tests/drm_modes_test.c
index 9358a885c58b..3953e478c4d0 100644
--- a/drivers/gpu/drm/tests/drm_modes_test.c
+++ b/drivers/gpu/drm/tests/drm_modes_test.c
@@ -4,14 +4,13 @@
  */
 
 #include 
+#include 
 #include 
 
 #include 
 
 #include 
 
-#include "drm_kunit_helpers.h"
-
 struct drm_test_modes_priv {
struct drm_device *drm;
 };
diff --git a/drivers/gpu/drm/tests/drm_probe_helper_test.c 
b/drivers/gpu/drm/tests/drm_probe_helper_test.c
index 7e938258c742..1f3941c150ae 100644
--- a/drivers/gpu/drm/tests/drm_probe_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_probe_helper_test.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -14,8 +15,6 @@
 
 #include 
 
-#include "drm_kunit_helpers.h"
-
 struct drm_probe_helper_test_priv {
struct drm_device *drm;
struct drm_connector connector;
diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.h 
b/include/drm/drm_kunit_helpers.h
similarity index 100%
rename from drivers/gpu/drm/tests/drm_kunit_helpers.h
rename to include/drm/drm_kunit_helpers.h

-- 
b4 0.10.1

[PATCH v3 00/20] drm: Introduce Kunit Tests to VC4

[Maxime Ripard]

Hi,

This series introduce Kunit tests to the vc4 KMS driver, but unlike what we
have been doing so far in KMS, it actually tests the atomic modesetting code.

In order to do so, I've had to improve a fair bit on the Kunit helpers already
found in the tree in order to register a full blown and somewhat functional KMS
driver.

It's of course relying on a mock so that we can test it anywhere. The mocking
approach created a number of issues, the main one being that we need to create
a decent mock in the first place, see patch 22. The basic idea is that I
created some structures to provide a decent approximation of the actual
hardware, and that would support both major architectures supported by vc4.

This is of course meant to evolve over time and support more tests, but I've
focused on testing the HVS FIFO assignment code which is fairly tricky (and the
tests have actually revealed one more bug with our current implementation). I
used to have a userspace implementation of those tests, where I would copy and
paste the kernel code and run the tests on a regular basis. It's was obviously
fairly suboptimal, so it seemed like the perfect testbed for that series.

It can be run using:
./tools/testing/kunit/kunit.py run \
--kunitconfig=drivers/gpu/drm/vc4/tests/.kunitconfig \
--cross_compile aarch64-linux-gnu- --arch arm64

Let me know what you think,
Maxime

To: David Airlie 
To: Daniel Vetter 
To: Maarten Lankhorst 
To: Maxime Ripard 
To: Thomas Zimmermann 
Cc: Dave Stevenson 
Cc: Javier Martinez Canillas 
Cc: Greg Kroah-Hartman 
Cc: Maíra Canal 
Cc: Brendan Higgins 
Cc: David Gow 
Cc: linux-kselft...@vger.kernel.org
Cc: kunit-...@googlegroups.com
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: Maxime Ripard 

---
Changes in v3:
- Add a Kconfig option for the KUnit helpers
- Switch to EXPORT_SYMBOL_GPL for the helpers
- Add some documentation on how to run the tests
- Add some documentation for __vc4_crtc_init
- Fix KUnit casing
- Link to v2: 
https://lore.kernel.org/r/20221123-rpi-kunit-tests-v2-0-efe5ed518...@cerno.tech

Changes in v2:
- Added some documentation for public functions
- Removed the fake device probe/remove workqueue 
- Made sure the tests could be compiled as modules
- Moved the vc4 tests in the vc4 module
- Applied some of the preliminary patches
- Rebased on top of current drm-misc-next branch
- Fixed checkpatch issues
- Introduced BCM2835 (Pi0-3) tests for muxing
- Introduced tests to cover past bugs we had
- Link to v1: 
https://lore.kernel.org/r/20221123-rpi-kunit-tests-v1-0-051a0bb60...@cerno.tech

---
Maxime Ripard (20):
  drm/tests: helpers: Move the helper header to include/drm
  drm/tests: Introduce a config option for the KUnit helpers
  drm/tests: helpers: Document drm_kunit_device_init()
  drm/tests: helpers: Switch to EXPORT_SYMBOL_GPL
  drm/tests: helpers: Rename the device init helper
  drm/tests: helpers: Remove the name parameter
  drm/tests: helpers: Create the device in another function
  drm/tests: helpers: Switch to a platform_device
  drm/tests: helpers: Make sure the device is bound
  drm/tests: helpers: Allow for a custom device struct to be allocated
  drm/tests: helpers: Allow to pass a custom drm_driver
  drm/tests: Add a test for DRM managed actions
  drm/vc4: Move HVS state to main header
  drm/vc4: crtc: Introduce a lower-level crtc init helper
  drm/vc4: crtc: Make encoder lookup helper public
  drm/vc4: hvs: Provide a function to initialize the HVS structure
  drm/vc4: tests: Introduce a mocking infrastructure
  drm/vc4: tests: Fail the current test if we access a register
  drm/vc4: tests: Add unit test suite for the PV muxing
  Documentation: gpu: vc4: Add KUnit Tests Section

 Documentation/gpu/vc4.rst   |   16 +
 drivers/gpu/drm/Kconfig |7 +
 drivers/gpu/drm/Makefile|2 +-
 drivers/gpu/drm/tests/Makefile  |5 +-
 drivers/gpu/drm/tests/drm_client_modeset_test.c |   19 +-
 drivers/gpu/drm/tests/drm_kunit_helpers.c   |  106 ++-
 drivers/gpu/drm/tests/drm_kunit_helpers.h   |   11 -
 drivers/gpu/drm/tests/drm_managed_test.c|   71 ++
 drivers/gpu/drm/tests/drm_modes_test.c  |   19 +-
 drivers/gpu/drm/tests/drm_probe_helper_test.c   |   20 +-
 drivers/gpu/drm/vc4/Kconfig |   16 +
 drivers/gpu/drm/vc4/Makefile|7 +
 drivers/gpu/drm/vc4/tests/.kunitconfig  |   13 +
 drivers/gpu/drm/vc4/tests/vc4_mock.c|  200 +
 drivers/gpu/drm/vc4/tests/vc4_mock.h|   63 ++
 drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c   |   41 +
 drivers/gpu/drm/vc4/tests/vc4_mock_output.c |  138 +++
 drivers/gpu/drm/vc4/tests/vc4_mock_plane.c  |   47 +
 drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c  | 1

[PATCH v3 03/20] drm/tests: helpers: Document drm_kunit_device_init()

[Maxime Ripard]

Commit 44a3928324e9 ("drm/tests: Add Kunit Helpers") introduced the
drm_kunit_device_init() function but didn't document it properly. Add
that documentation.

Reviewed-by: Maíra Canal 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/tests/drm_kunit_helpers.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c 
b/drivers/gpu/drm/tests/drm_kunit_helpers.c
index 6600a4db3158..9ed3cfc2ac03 100644
--- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
+++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
@@ -35,6 +35,23 @@ static void dev_free(struct kunit_resource *res)
root_device_unregister(dev);
 }
 
+/**
+ * drm_kunit_device_init - Allocates a mock DRM device for KUnit tests
+ * @test: The test context object
+ * @features: Mocked DRM device driver features
+ * @name: Name of the struct &device to allocate
+ *
+ * This function allocates a new struct &device, creates a struct
+ * &drm_driver and will create a struct &drm_device using both.
+ *
+ * The device and driver are tied to the @test context and will get
+ * cleaned at the end of the test. The drm_device is allocated through
+ * devm_drm_dev_alloc() and will thus be freed through a device-managed
+ * resource.
+ *
+ * Returns:
+ * A pointer to the new drm_device, or an ERR_PTR() otherwise.
+ */
 struct drm_device *drm_kunit_device_init(struct kunit *test, u32 features, 
char *name)
 {
struct kunit_dev *kdev;

-- 
b4 0.10.1

[PATCH v3 02/20] drm/tests: Introduce a config option for the KUnit helpers

[Maxime Ripard]

Driver-specific tests will need access to the helpers without pulling
every DRM framework test. Let's create an intermediate Kconfig options
for the helpers.

Suggested-by: Maíra Canal 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/Kconfig| 7 +++
 drivers/gpu/drm/Makefile   | 2 +-
 drivers/gpu/drm/tests/Makefile | 4 +++-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 315cbdf61979..9f019cd053e1 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -63,6 +63,12 @@ config DRM_USE_DYNAMIC_DEBUG
  bytes per callsite, the .data costs can be substantial, and
  are therefore configurable.
 
+config DRM_KUNIT_TEST_HELPERS
+   tristate
+   depends on DRM && KUNIT
+   help
+ KUnit Helpers for KMS drivers.
+
 config DRM_KUNIT_TEST
tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS
depends on DRM && KUNIT
@@ -73,6 +79,7 @@ config DRM_KUNIT_TEST
select DRM_KMS_HELPER
select DRM_BUDDY
select DRM_EXPORT_FOR_TESTS if m
+   select DRM_KUNIT_TEST_HELPERS
default KUNIT_ALL_TESTS
help
  This builds unit tests for DRM. This option is not useful for
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index f92cd7892711..8d61fbdfdfac 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -125,7 +125,7 @@ obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
 # Drivers and the rest
 #
 
-obj-$(CONFIG_DRM_KUNIT_TEST) += tests/
+obj-y  += tests/
 
 obj-$(CONFIG_DRM_MIPI_DBI) += drm_mipi_dbi.o
 obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
index 94fe546d937d..ef14bd481139 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -1,5 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
+obj-$(CONFIG_DRM_KUNIT_TEST_HELPERS) += \
+   drm_kunit_helpers.o
+
 obj-$(CONFIG_DRM_KUNIT_TEST) += \
drm_buddy_test.o \
drm_cmdline_parser_test.o \
@@ -9,7 +12,6 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
drm_format_helper_test.o \
drm_format_test.o \
drm_framebuffer_test.o \
-   drm_kunit_helpers.o \
drm_mm_test.o \
drm_modes_test.o \
drm_plane_helper_test.o \

-- 
b4 0.10.1

[PATCH v3 04/20] drm/tests: helpers: Switch to EXPORT_SYMBOL_GPL

[Maxime Ripard]

drm_kunit_device_init() among other things will allocate a device and
wrap around root_device_register. This function is exported with
EXPORT_SYMBOL_GPL, so we can't really change the license.

Fixes: 199557fab925 ("drm/tests: helpers: Add missing export")
Suggested-by: Javier Martinez Canillas 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c 
b/drivers/gpu/drm/tests/drm_kunit_helpers.c
index 9ed3cfc2ac03..4fe131141718 100644
--- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
+++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
@@ -82,7 +82,7 @@ struct drm_device *drm_kunit_device_init(struct kunit *test, 
u32 features, char
 
return drm;
 }
-EXPORT_SYMBOL(drm_kunit_device_init);
+EXPORT_SYMBOL_GPL(drm_kunit_device_init);
 
 MODULE_AUTHOR("Maxime Ripard ");
 MODULE_LICENSE("GPL");

-- 
b4 0.10.1