date:20241019

Re: [PATCH] drm: a6xx: avoid excessive stack usage

2024-10-19 Thread Akhil P Oommen

On Fri, Oct 18, 2024 at 03:11:38PM +, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> Clang-19 and above sometimes end up with multiple copies of the large
> a6xx_hfi_msg_bw_table structure on the stack. The problem is that
> a6xx_hfi_send_bw_table() calls a number of device specific functions to
> fill the structure, but these create another copy of the structure on
> the stack which gets copied to the first.
> 
> If the functions get inlined, that busts the warning limit:
> 
> drivers/gpu/drm/msm/adreno/a6xx_hfi.c:631:12: error: stack frame size (1032) 
> exceeds limit (1024) in 'a6xx_hfi_send_bw_table' [-Werror,-Wframe-larger-than]

Why does this warning says that the limit is 1024? 1024 bytes is too small, 
isn't it?

-Akhil.

> 
> Mark all of them as 'noinline_for_stack' ensure we only have one copy
> of the structure per function.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> index cdb3f6e74d3e..5699e0420eb8 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> @@ -259,7 +259,8 @@ static int a6xx_hfi_send_perf_table(struct a6xx_gmu *gmu)
>   NULL, 0);
>  }
>  
> -static void a618_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +/* noinline to avoid having multiple copies of 'msg' on stack */
> +static noinline_for_stack void a618_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>  {
>   /* Send a single "off" entry since the 618 GMU doesn't do bus scaling */
>   msg->bw_level_num = 1;
> @@ -287,7 +288,7 @@ static void a618_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>   msg->cnoc_cmds_data[1][0] =  0x6001;
>  }
>  
> -static void a619_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a619_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>  {
>   msg->bw_level_num = 13;
>  
> @@ -346,7 +347,7 @@ static void a619_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>   msg->cnoc_cmds_data[0][0] = 0x4000;
>  }
>  
> -static void a640_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a640_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>  {
>   /*
>* Send a single "off" entry just to get things running
> @@ -385,7 +386,7 @@ static void a640_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>   msg->cnoc_cmds_data[1][2] =  0x6001;
>  }
>  
> -static void a650_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a650_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>  {
>   /*
>* Send a single "off" entry just to get things running
> @@ -416,7 +417,7 @@ static void a650_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>   msg->cnoc_cmds_data[1][0] =  0x6001;
>  }
>  
> -static void a690_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a690_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>  {
>   /*
>* Send a single "off" entry just to get things running
> @@ -447,7 +448,7 @@ static void a690_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>   msg->cnoc_cmds_data[1][0] =  0x6001;
>  }
>  
> -static void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a660_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>  {
>   /*
>* Send a single "off" entry just to get things running
> @@ -478,7 +479,7 @@ static void a660_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>   msg->cnoc_cmds_data[1][0] =  0x6001;
>  }
>  
> -static void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void adreno_7c3_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>  {
>   /*
>* Send a single "off" entry just to get things running
> @@ -509,7 +510,7 @@ static void adreno_7c3_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>   msg->cnoc_cmds_data[1][0] =  0x6001;
>  }
>  
> -static void a730_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a730_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>  {
>   msg->bw_level_num = 12;
>  
> @@ -565,7 +566,7 @@ static void a730_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>   msg->cnoc_cmds_data[1][0] = 0x6001;
>  }
>  
> -static void a740_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a740_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>  {
>   msg->bw_level_num = 1;
>  
> @@ -590,7 +591,7 @@ static void a740_build_bw_table(struct 
> a6xx_hfi_msg_bw_table *msg)
>   msg->cnoc_cmds_data[1][0] = 0x6001;
>  }
>  
> -static void a6xx_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a

Re: vc4: HDMI Sink doesn't support RGB, something's wrong.

2024-10-19 Thread Stefan Wahren

Hi,

Am 17.10.24 um 17:59 schrieb Maxime Ripard:

On Thu, Oct 17, 2024 at 05:26:46PM GMT, Stefan Wahren wrote:

Am 17.10.24 um 16:27 schrieb Maxime Ripard:

On Wed, Oct 16, 2024 at 07:16:43PM GMT, Dave Stevenson wrote:

Hi Stefan

On Tue, 15 Oct 2024 at 22:13, Stefan Wahren wrote:

Hi Dave,

...

i prepared a branch for you, which contains the latest suspend2idle patches:

https://github.com/lategoodbye/linux-dev/commits/v6.12-pm/

Steps:
1. Flash latest Raspberry Pi OS (32 bit) on SD card
2. Build Kernel from repo above with arm/multi_v7_defconfig
3. Replace Kernel, modules + DTB on SD card with build ones
4. add the following to confix.txt
device_tree=bcm2837-rpi-3-b-plus.dtb
enable_uart=1
5. change/add the following to cmdline.txt
console=ttyS1,115200
no_console_suspend=1
6. connect the following devices to Raspberry Pi 3 B+ :
USB mouse
USB keyboard
HDMI monitor
Debug UART adapter (USB side to PC)
7. Power on board and boot into X11
8. Change to root
9. Enable wakeup for ttyS1

So I remember for next time
echo enabled > /sys/class/tty/ttyS1/power/wakeup

10. Trigger suspend to idle via X11 (echo freeze > /sys/power/state)
11. Wakeup Raspberry Pi via Debug UART

I don't get the error you are seeing, but I also don't get the display resuming.
pm has obviously killed the power to the HDMI block, and it has the
reset values in as can be seen via /sys/kernel/debug/dri/0/hdmi_regs.
Nothing in the driver restores these registers, and I'm not sure if it
is meant to do so.
Run kmstest or similar from this state and the change of mode
reprogrammes the blocks so we get the display back again.

I've also enabled CONFIG_DRM_LOAD_EDID_FIRMWARE so that I can use your
EDID, and get the same results.

Knee-capping the HDMI block on suspend seems an unlikely mechanism to
work reliably. On the more recent Pis there is a need to be quite
careful in disabling the pipeline to avoid getting data stuck in
FIFOs.
I feel I must be missing something here.

I think we're probably missing calls to drm_mode_config_helper_suspend and
drm_mode_config_helper_resume.

Okay, i tried this and it works better (HDMI resumes fast), but it also
triggers a lot of WARN

vc4_plane_reset deviates from the helper there:
https://elixir.bootlin.com/linux/v6.11.3/source/drivers/gpu/drm/drm_atomic_state_helper.c#L326

We should adjust it.

and the "doesn't support RGB ..." warnings are still there.

I pushed my changes to the branch and attached the dmesg output.

I can't help you there, it doesn't make sense to me. The EDID should be correct.

Maybe I should clarify that I provided the EDID from the X11 log during
normal boot (good case). I wasn't aware how to dump the EDID during the
suspend.

I tested with the new branch and these warning doesn't always occurs
during resume. So it seems to be timing related.

AFAIU the EDID is read via I2C DDC and the attached clock in this case
is VPU clock. Correct?

So I added the following to the config.txt

force_turbo=1

After that I wasn't able to reproduce these HDMI Sink warnings.

Is it possible that the I2C data get corrupted by VPU clock changes?

Regards

Maxime

Re: [PATCH RFC 3/3] arm64: dts: qcom: x1e80100: Add ACD levels for GPU

2024-10-19 Thread Akhil P Oommen

On Thu, Oct 17, 2024 at 09:05:50AM +0200, Krzysztof Kozlowski wrote:
> On 17/10/2024 08:12, Akhil P Oommen wrote:
> > On Wed, Oct 16, 2024 at 09:50:04AM +0200, Krzysztof Kozlowski wrote:
> >> On 15/10/2024 21:35, Akhil P Oommen wrote:
> >>> On Mon, Oct 14, 2024 at 09:40:13AM +0200, Krzysztof Kozlowski wrote:
>  On Sat, Oct 12, 2024 at 01:59:30AM +0530, Akhil P Oommen wrote:
> > Update GPU node to include acd level values.
> >
> > Signed-off-by: Akhil P Oommen 
> > ---
> >  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi 
> > b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > index a36076e3c56b..e6c500480eb1 100644
> > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > @@ -3323,60 +3323,69 @@ zap-shader {
> > };
> >  
> > gpu_opp_table: opp-table {
> > -   compatible = "operating-points-v2";
> > +   compatible = 
> > "operating-points-v2-adreno";
> 
>  This nicely breaks all existing users of this DTS. Sorry, no. We are way
>  past initial bringup/development. One year past.
> > 
> > How do I identify when devicetree is considered stable? An arbitrary
> > time period doesn't sound like a good idea. Is there a general consensus
> > on this?
> > 
> > X1E chipset is still considered under development at least till the end of 
> > this
> > year, right?
> 
> Stable could be when people already get their consumer/final product
> with it. I got some weeks ago Lenovo T14s laptop and since yesterday
> working fine with Ubuntu:
> https://discourse.ubuntu.com/t/ubuntu-24-10-concept-snapdragon-x-elite/48800
> 
> All chipsets are under development, even old SM8450, but we avoid
> breaking it while doing that.
> 
I still have questions about the practicality especially in IoT/Auto chipsets,
but I will try to get it clarified when I face them.

I will go ahead and send out the v2 series addressing the suggestions.

-Akhil.

> 
> 
> Best regards,
> Krzysztof
>

Re: [PATCH 05/11] accel/ivpu: Unmap partially mapped BOs in case of errors

2024-10-19 Thread Jeffrey Hugo


On 10/17/2024 8:58 AM, Jacek Lawrynowicz wrote:

From: Karol Wachowski 

Ensure that all buffers that were created only partially through
allocated scatter-gather table are unmapped from MMU600 in case of errors.

Signed-off-by: Karol Wachowski 
Reviewed-by: Jacek Lawrynowicz 
Signed-off-by: Jacek Lawrynowicz 


Reviewed-by: Jeffrey Hugo

64 matches

Mail list logo