Re: [PATCH] Revert "drm/amd/powerplay: Enable/Disable NBPSTATE on On/OFF of UVD"
On 10/26/2018 8:34 AM, Alex Deucher wrote: > On Thu, Oct 25, 2018 at 10:52 PM Agrawal, Akshu wrote: >> >> >> >> On 10/26/2018 8:09 AM, Alex Deucher wrote: >>> On Thu, Oct 25, 2018 at 12:27 PM S, Shirish wrote: >>>> >>>> This reverts commit dbd8299c32f6f413f6cfe322fe0308f3cfc577e8. >>>> >>>> Reason for revert: >>>> This patch sends msg PPSMC_MSG_DisableLowMemoryPstate(0x002e) >>>> in wrong of sequence to SMU which is before PPSMC_MSG_UVDPowerON (0x0008). >>>> This leads to SMU failing to service the request as it is >>>> dependent on UVD to be powered ON, since it accesses UVD >>>> registers. >>> >>> Does this patch that is being reverted actually break something or is >>> it ok to leave as a workaround? It supposedly fixed display issues at >>> 4k with video. Reverting it will bring that back won't it? >>> >>> Alex >>> >> Yes Alex, it will break 4k video as there will be underrun. But we are >> working on patches that will Disable memory NBPstate only for 4k videos. >> We can have this patch in and will be posting couple of patches to fix >> 4k videos display issues. > > Can we land them all together? Otherwise, we'll have a regressed > state until the later fixes land. > > Alex > Agreed, will push them as a series. Thanks, Akshu ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] Revert "drm/amd/powerplay: Enable/Disable NBPSTATE on On/OFF of UVD"
On 10/26/2018 8:09 AM, Alex Deucher wrote: > On Thu, Oct 25, 2018 at 12:27 PM S, Shirish wrote: >> >> This reverts commit dbd8299c32f6f413f6cfe322fe0308f3cfc577e8. >> >> Reason for revert: >> This patch sends msg PPSMC_MSG_DisableLowMemoryPstate(0x002e) >> in wrong of sequence to SMU which is before PPSMC_MSG_UVDPowerON (0x0008). >> This leads to SMU failing to service the request as it is >> dependent on UVD to be powered ON, since it accesses UVD >> registers. > > Does this patch that is being reverted actually break something or is > it ok to leave as a workaround? It supposedly fixed display issues at > 4k with video. Reverting it will bring that back won't it? > > Alex > Yes Alex, it will break 4k video as there will be underrun. But we are working on patches that will Disable memory NBPstate only for 4k videos. We can have this patch in and will be posting couple of patches to fix 4k videos display issues. Thanks, Akshu ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu/acp: Fix slab-out-of-bounds in mfd_add_device in acp_hw_init
Was this patch ever picked up? I can't find it in agd5f/linux.
>>>
>>>
>>> It wasn't applied. I don't see 51f7415039d4 ("drm/amd/amdgpu:
>>> creating two I2S instances for stoney/cz") upstream yet either.
>>> Daniel, Vijendar, which ones do you want applied? Can you send me the
>>> patches?
>>>
>>> Alex
>>
>>
>> Hi Alex,
>>
>> "drm/amd/amdgpu: creating two I2S instances for stoney/cz" patch exists in
>> drm-next branch. Please pick the patch .
>
> So just that one? I seem to recall there being later revisions of
> that patch that you reworked after applying the original version.
> Also that patch was originally part of a larger series. Are those
> changes required too?
>
> Alex
>
Hi Alex,
In agd5f/linux, branch "amd-staging-drm-next",
506f7d1 drm/amd/amdgpu: creating two I2S instances for stoney/cz
patch is present.
This patch is the correct version and there aren't any other changes
required with it.
Only Dan's, this mail's patch is currently missing form the tree.
Thanks,
Akshu
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu/acp: Fix slab-out-of-bounds in mfd_add_device in acp_hw_init
On 4/13/2018 9:45 PM, Daniel Kurtz wrote:
Commit 51f7415039d4 ("drm/amd/amdgpu: creating two I2S instances for
stoney/cz") added support for the "BT_I2S" ACP i2s channel. As part of
this change, one additional acp resource was added, but the "num_resource"
count was accidentally incremented by 2.
This incorrect count eventually causes mfd_add_device() to try to access
an invalid memory address (the location of non-existent resource 5.
This fault was detected by running a KASAN enabled kernel, which produced
the following splat at boot:
[6.612987]
==
[6.613509] BUG: KASAN: slab-out-of-bounds in mfd_add_device+0x4bc/0x7a7
[6.613509] Read of size 8 at addr 880107d4dc58 by task swapper/0/1
[6.613509]
[6.613509] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.14.33 #349
[6.613509] Hardware name: Google Grunt/Grunt, BIOS
Google_Grunt.10543.0.2018_04_03_1812 04/02/2018
[6.613509] Call Trace:
[6.613509] dump_stack+0x4d/0x63
[6.613509] print_address_description+0x80/0x2d6
[6.613509] ? mfd_add_device+0x4bc/0x7a7
[6.613509] kasan_report+0x255/0x295
[6.613509] mfd_add_device+0x4bc/0x7a7
[6.613509] ? kasan_kmalloc+0x99/0xa8
[6.613509] ? mfd_add_devices+0x58/0xe4
[6.613509] ? __kmalloc+0x154/0x178
[6.613509] mfd_add_devices+0xa5/0xe4
[6.613509] acp_hw_init+0x92e/0xc4a
[6.613509] amdgpu_device_init+0x1dfb/0x22a2
[6.613509] ? kmalloc_order+0x53/0x5d
[6.613509] ? kmalloc_order_trace+0x23/0xb3
[6.613509] amdgpu_driver_load_kms+0xce/0x267
[6.613509] drm_dev_register+0x169/0x2fb
[6.613509] amdgpu_pci_probe+0x217/0x242
[6.613509] pci_device_probe+0x101/0x18e
[6.613509] driver_probe_device+0x1dd/0x419
[6.613509] ? ___might_sleep+0x80/0x1b6
[6.613509] __driver_attach+0x9f/0xc9
[6.613509] ? driver_probe_device+0x419/0x419
[6.613509] bus_for_each_dev+0xbc/0xe1
[6.613509] bus_add_driver+0x189/0x2c0
[6.613509] driver_register+0x108/0x156
[6.613509] ? ttm_init+0x67/0x67
[6.613509] do_one_initcall+0xb2/0x161
[6.613509] kernel_init_freeable+0x25a/0x308
[6.613509] ? rest_init+0xcc/0xcc
[6.613509] kernel_init+0x11/0x10d
[6.613509] ? rest_init+0xcc/0xcc
[6.613509] ret_from_fork+0x22/0x40
[6.613509]
[6.613509] Allocated by task 1:
[6.613509] save_stack+0x46/0xce
[6.613509] kasan_kmalloc+0x99/0xa8
[6.613509] kmem_cache_alloc_trace+0x11a/0x13e
[6.613509] acp_hw_init+0x210/0xc4a
[6.613509] amdgpu_device_init+0x1dfb/0x22a2
[6.613509] amdgpu_driver_load_kms+0xce/0x267
[6.613509] drm_dev_register+0x169/0x2fb
[6.613509] amdgpu_pci_probe+0x217/0x242
[6.613509] pci_device_probe+0x101/0x18e
[6.613509] driver_probe_device+0x1dd/0x419
[6.613509] __driver_attach+0x9f/0xc9
[6.613509] bus_for_each_dev+0xbc/0xe1
[6.613509] bus_add_driver+0x189/0x2c0
[6.613509] driver_register+0x108/0x156
[6.613509] do_one_initcall+0xb2/0x161
[6.613509] kernel_init_freeable+0x25a/0x308
[6.613509] kernel_init+0x11/0x10d
[6.613509] ret_from_fork+0x22/0x40
[6.613509]
[6.613509] Freed by task 0:
[6.613509] (stack is not available)
[6.613509]
[6.613509] The buggy address belongs to the object at 880107d4db08
[6.613509] which belongs to the cache kmalloc-512 of size 512
[6.613509] The buggy address is located 336 bytes inside of
[6.613509] 512-byte region [880107d4db08, 880107d4dd08)
[6.613509] The buggy address belongs to the page:
[6.613509] page:ea00041f5300 count:1 mapcount:0 mapping:
(null) index:0x0 compound_mapcount: 0
[6.613509] flags: 0x80008100(slab|head)
[6.613509] raw: 80008100
000100120012
[6.613509] raw: ea0004208520 88010b001680 88010b002cc0
[6.613509] page dumped because: kasan: bad access detected
[6.613509]
[6.613509] Memory state around the buggy address:
[6.613509] 880107d4db00: fc 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00
[6.613509] 880107d4db80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00
[6.613509] >880107d4dc00: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc
fc
[6.613509] ^
[6.613509] 880107d4dc80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
fc
[6.613509] 880107d4dd00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
fc
[6.613509]
==
Fixes: 51f7415039d4 ("drm/amd/amdgpu: creating two I2S instances for stoney/cz")
Signed-off-by: Daniel Kurtz
Acked-by: Akshu Agrawal
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
Re: [PATCH v2 2/3] ASoC: rt5645: Wait for 400msec before concluding on value of RT5645_VENDOR_ID2
On 11/8/2017 11:39 PM, Mark Brown wrote: On Wed, Nov 08, 2017 at 12:24:03PM -0500, Alex Deucher wrote: regmap_read(regmap, RT5645_VENDOR_ID2, ); + /* +* Read after 400msec, as it is the interval required between +* read and power On. +*/ + msleep(TIME_TO_POWER_MS); + regmap_read(regmap, RT5645_VENDOR_ID2, ); + This leaves the original read in there so we've both got the early read (which might upset things potentially) and the delayed read. Shouldn't we just be adding a msleep() before the existing read? My bad, I should have removed the addition of register read from the patch. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/3] ASoC: amd: Report accurate hw_ptr during dma
On 11/7/2017 5:07 PM, Mark Brown wrote: On Tue, Nov 07, 2017 at 07:26:03PM +0530, Mukunda,Vijendar wrote: Removing URL links and commit-ready description in v2. This doesn't really answer my question: These two URLs are different, what was being reviewed here? What is Commit-Ready supposed to mean? Same patch is reviewed, once on 4.4 kernel (659699) and then on 4.12 kernel (672267). Commit-ready is to get it merged on tree after receiving a +2. Please don't top post, reply in line with needed context. This allows readers to readily follow the flow of conversation and understand what you are talking about and also helps ensure that everything in the discussion is being addressed. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 3/3] ASoC: rt5645: Wait for 400msec before concluding on value of RT5645_VENDOR_ID2
On 11/6/2017 9:54 PM, Mark Brown wrote:
On Fri, Nov 03, 2017 at 04:35:45PM -0400, Alex Deucher wrote:
Minimum time required between power On of codec and read
of RT5645_VENDOR_ID2 is 400msec. We should wait and attempt
before erroring out.
So the description says we have to wait 400ms before attempting a
read...
Yes, that's true.
BUG=b:66978383
What does this mean?
A bug ID. Removing it in V2.
@@ -3786,6 +3789,15 @@ static int rt5645_i2c_probe(struct i2c_client *i2c,
}
regmap_read(regmap, RT5645_VENDOR_ID2, );
+ /*
+* Read for 400msec, as it is the interval required between
+* read and power On.
+*/
+ while (val != RT5645_DEVICE_ID && val != RT5650_DEVICE_ID && --timeout)
{
+ msleep(1);
+ regmap_read(regmap, RT5645_VENDOR_ID2, );
+ }
+
...but what we actually do is try to read up to 400 times starting well
before that 400ms is up. This directly contradicts what the commit
message said we needed, may take a lot longer if the chip misbehaves on
the I2C bus while it's not ready (which wouldn't be that much of a
surprise), might lead to us reporting before the chip is really stable
(if the read happens to work while the chip isn't yet stable) and could
cause lots of noise on the console if the I2C controller gets upset.
What are we actually waiting for here?
In my understanding if we get RT5645 or RT5650 DEVICE ID from register
RT5645_VENDOR_ID2 then that means the chip is stable.
If we really need 400ms of dead reckoning time (which is a lot for a
modern chip, that feels more like a VMID ramp) then it's going to be
safer to just do that.
Agreed, will just sleep for 400ms before reading the register value.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 6/6 v3] ASoC: AMD: Add machine driver for cz rt5650
On 8/31/2017 5:08 PM, Mark Brown wrote:
> On Fri, Aug 18, 2017 at 02:10:30PM -0400, Alex Deucher wrote:
>
>> +++ b/sound/soc/amd/Kconfig
>> @@ -2,3 +2,10 @@ config SND_SOC_AMD_ACP
>> tristate "AMD Audio Coprocessor support"
>> help
>> This option enables ACP DMA support on AMD platform.
>> +config SND_SOC_AMD_CZ_RT5645_MACH
>> +tristate "AMD CZ support for RT5645"
>
> Missing blank line between the stanzas.
Done. Will push the change in next revision.
>
>> +select SND_SOC_RT5645
>> +select SND_SOC_AMD_ACP
>> +depends on I2C_DESIGNWARE_PLATFORM
>
> No system dependencies of any kind? Looking at this I'd expect at least
> CONFIG_ACPI || COMPILE_TEST. It's also unclear to me how the DesignWare
> device is going to be instantiated here or if that should be a direct
> depenency at all here.
>
Added I2C for system dependency and removed
I2C_DESIGNWARE_PLATFORM as dependency
>> +ret = snd_soc_register_card(card);
>
> devm_snd_soc_register_card() and then you don't need the remove
> function.
>
Done.
>> +static const struct acpi_device_id cz_audio_acpi_match[] = {
>> +{ "AMDI1002", 0 },
>> +{},
>> +};
>> +
>
> Missing MODULE_DEVICE_TABLE().
>
Done.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
