* Re: [PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399
From: Daniel Lezcano @ 2022-01-05 11:25 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: rjw, lukasz.luba, heiko, arnd, linux-kernel, linux-pm,
Geert Uytterhoeven, moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support
In-Reply-To: <CAPDyKFqWUJTKte3dM=7xG6EtKR8i9neCCNYFs7Jf1J34TezUEQ@mail.gmail.com>
On 31/12/2021 14:57, Ulf Hansson wrote:
[ ... ]
>> +static struct dtpm_node __initdata rk3399_hierarchy[] = {
>> + [0]{ .name = "rk3399" },
>> + [1]{ .name = "package",
>> + .parent = &rk3399_hierarchy[0] },
>> + [2]{ .name = "/cpus/cpu@0",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [3]{ .name = "/cpus/cpu@1",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [4]{ .name = "/cpus/cpu@2",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [5]{ .name = "/cpus/cpu@3",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [6]{ .name = "/cpus/cpu@100",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [7]{ .name = "/cpus/cpu@101",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [8]{ .name = "rockchip,rk3399-mali",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [9]{ },
>> +};
>
> I will not object to this, as in the end this seems like what we need
> to do, unless we can describe things through generic DT bindings for
> DTPM. Right?
Yes, as asked by Rob, we should try to describe in the kernel first.
> Although, if the above is correct, I need to stress that I am kind of
> worried that this doesn't really scale. We would need to copy lots of
> information from the DTS files into platform specific c-files, to be
> able to describe the DTPM hierarchy.
TBH I don't think it is a lot and it is a __initdata. At least we should
begin with something and see later how to consolidate if it is needed, no?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 0/9] Multigenerational LRU Framework
From: SeongJae Park @ 2022-01-05 11:25 UTC (permalink / raw)
To: Yu Zhao
Cc: SeongJae Park, Andrew Morton, Linus Torvalds, Andi Kleen,
Catalin Marinas, Dave Hansen, Hillf Danton, Jens Axboe,
Jesse Barnes, Johannes Weiner, Jonathan Corbet, Matthew Wilcox,
Mel Gorman, Michael Larabel, Michal Hocko, Rik van Riel,
Vlastimil Babka, Will Deacon, Ying Huang, linux-arm-kernel,
linux-doc, linux-kernel, linux-mm, page-reclaim, x86
In-Reply-To: <YdV4k1+zEbtzmUkK@google.com>
On Wed, 5 Jan 2022 03:53:07 -0700 Yu Zhao <yuzhao@google.com> wrote:
> On Wed, Jan 05, 2022 at 08:55:34AM +0000, SeongJae Park wrote:
> > Hi Yu,
> >
> > On Tue, 4 Jan 2022 13:22:19 -0700 Yu Zhao <yuzhao@google.com> wrote:
[...]
> > I think similar works are already available out of the box with the latest
> > mainline tree, though it might be suboptimal in some cases.
>
> Ok, I will sound harsh because I hate it when people challenge facts
> while having no idea what they are talking about.
>
> Our jobs are help the leadership make best decisions by providing them
> with facts, not feeding them crap.
I was using the word "similar", to represent this is only for a rough concept
level similarity, rather than detailed facts. But, seems it was not enough,
sorry. Anyway, I will not talk more and thus disturb you having the important
discussion with leaders here, as you are asking.
>
> Don't get me wrong -- you are welcome to start another thread and have
> a casual discussion with me. But this thread is not for that; it's for
> the leadership and stakeholder to make a decision. Check who are in
> "To" and "Cc" and what my request is.
Haha. Ok, good luck for you.
Thanks,
SJ
[...]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v13 3/5] arm64: perf: Add userspace counter access disable switch
From: Will Deacon @ 2022-01-05 11:25 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rob Herring, Mark Rutland, Peter Zijlstra, Vince Weaver,
Jonathan Corbet, Catalin Marinas, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Thomas Gleixner, Borislav Petkov,
the arch/x86 maintainers, H. Peter Anvin,
Linux Kernel Mailing List, Linux ARM, linux-perf-users,
Linux-Renesas
In-Reply-To: <20220104135658.GC1827@willie-the-truck>
On Tue, Jan 04, 2022 at 01:56:59PM +0000, Will Deacon wrote:
> On Tue, Dec 28, 2021 at 12:07:02PM +0100, Geert Uytterhoeven wrote:
> > On Wed, Dec 8, 2021 at 9:19 PM Rob Herring <robh@kernel.org> wrote:
> > > Like x86, some users may want to disable userspace PMU counter
> > > altogether. Add a sysctl 'perf_user_access' file to control userspace
> > > counter access. The default is '0' which is disabled. Writing '1'
> > > enables access.
> > >
> > > Note that x86 supports globally enabling user access by writing '2' to
> > > /sys/bus/event_source/devices/cpu/rdpmc. As there's not existing
> > > userspace support to worry about, this shouldn't be necessary for Arm.
> > > It could be added later if the need arises.
> >
> > Thanks for your patch, which is now commit e2012600810c9ded ("arm64:
> > perf: Add userspace counter access disable switch") in arm64/for-next/core.
> >
> > This is causing two issues on Renesas Salvator-XS with R-Car H3.
> > One during kernel boot:
> >
> > hw perfevents: enabled with armv8_cortex_a53 PMU driver, 7
> > counters available
> > +sysctl duplicate entry: /kernel//perf_user_access
> > +CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> > 5.16.0-rc3-arm64-renesas-00003-ge2012600810c #1420
> > +Hardware name: Renesas Salvator-X 2nd version board based on r8a77951 (DT)
> > +Call trace:
> > + dump_backtrace+0x0/0x190
> > + show_stack+0x14/0x20
> > + dump_stack_lvl+0x88/0xb0
> > + dump_stack+0x14/0x2c
> > + __register_sysctl_table+0x384/0x818
> > + register_sysctl+0x20/0x28
> > + armv8_pmu_init.constprop.0+0x118/0x150
> > + armv8_a57_pmu_init+0x1c/0x28
> > + arm_pmu_device_probe+0x1b4/0x558
> > + armv8_pmu_device_probe+0x18/0x20
> > + platform_probe+0x64/0xd0
> > + really_probe+0xb4/0x2f8
> > + __driver_probe_device+0x74/0xd8
> > + driver_probe_device+0x3c/0xe0
> > + __driver_attach+0x80/0x110
> > + bus_for_each_dev+0x6c/0xc0
> > + driver_attach+0x20/0x28
> > + bus_add_driver+0x138/0x1e0
> > + driver_register+0x60/0x110
> > + __platform_driver_register+0x24/0x30
> > + armv8_pmu_driver_init+0x18/0x20
> > + do_one_initcall+0x15c/0x31c
> > + kernel_init_freeable+0x2f0/0x354
> > + kernel_init+0x20/0x120
> > + ret_from_fork+0x10/0x20
> > hw perfevents: enabled with armv8_cortex_a57 PMU driver, 7
> > counters available
> >
> > Presumably the same entry is added twice, once for the A53 PMU,
> > and a second time for the A57 PMU?
>
> Looks like it, and perhaps that's also what is confusing systemd?
> Rob -- how come you didn't see this during your testing?
>
> Anywho, please can you try the untested diff below?
I just remembered I have a big/little SoC on my desk after borrowing a
NanoPi (RK3399) from Marc Z, so I took this diff for a spin there and
both the kernel and systemd seem happy.
Will
^ permalink raw reply
* Re: [PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399
From: Daniel Lezcano @ 2022-01-05 11:25 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: rjw, lukasz.luba, heiko, arnd, linux-kernel, linux-pm,
Geert Uytterhoeven, moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support
In-Reply-To: <CAPDyKFqWUJTKte3dM=7xG6EtKR8i9neCCNYFs7Jf1J34TezUEQ@mail.gmail.com>
On 31/12/2021 14:57, Ulf Hansson wrote:
[ ... ]
>> +static struct dtpm_node __initdata rk3399_hierarchy[] = {
>> + [0]{ .name = "rk3399" },
>> + [1]{ .name = "package",
>> + .parent = &rk3399_hierarchy[0] },
>> + [2]{ .name = "/cpus/cpu@0",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [3]{ .name = "/cpus/cpu@1",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [4]{ .name = "/cpus/cpu@2",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [5]{ .name = "/cpus/cpu@3",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [6]{ .name = "/cpus/cpu@100",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [7]{ .name = "/cpus/cpu@101",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [8]{ .name = "rockchip,rk3399-mali",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [9]{ },
>> +};
>
> I will not object to this, as in the end this seems like what we need
> to do, unless we can describe things through generic DT bindings for
> DTPM. Right?
Yes, as asked by Rob, we should try to describe in the kernel first.
> Although, if the above is correct, I need to stress that I am kind of
> worried that this doesn't really scale. We would need to copy lots of
> information from the DTS files into platform specific c-files, to be
> able to describe the DTPM hierarchy.
TBH I don't think it is a lot and it is a __initdata. At least we should
begin with something and see later how to consolidate if it is needed, no?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399
From: Daniel Lezcano @ 2022-01-05 11:25 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring
Cc: rjw, lukasz.luba, heiko, arnd, linux-kernel, linux-pm,
Geert Uytterhoeven, moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support
In-Reply-To: <CAPDyKFqWUJTKte3dM=7xG6EtKR8i9neCCNYFs7Jf1J34TezUEQ@mail.gmail.com>
On 31/12/2021 14:57, Ulf Hansson wrote:
[ ... ]
>> +static struct dtpm_node __initdata rk3399_hierarchy[] = {
>> + [0]{ .name = "rk3399" },
>> + [1]{ .name = "package",
>> + .parent = &rk3399_hierarchy[0] },
>> + [2]{ .name = "/cpus/cpu@0",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [3]{ .name = "/cpus/cpu@1",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [4]{ .name = "/cpus/cpu@2",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [5]{ .name = "/cpus/cpu@3",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [6]{ .name = "/cpus/cpu@100",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [7]{ .name = "/cpus/cpu@101",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [8]{ .name = "rockchip,rk3399-mali",
>> + .type = DTPM_NODE_DT,
>> + .parent = &rk3399_hierarchy[1] },
>> + [9]{ },
>> +};
>
> I will not object to this, as in the end this seems like what we need
> to do, unless we can describe things through generic DT bindings for
> DTPM. Right?
Yes, as asked by Rob, we should try to describe in the kernel first.
> Although, if the above is correct, I need to stress that I am kind of
> worried that this doesn't really scale. We would need to copy lots of
> information from the DTS files into platform specific c-files, to be
> able to describe the DTPM hierarchy.
TBH I don't think it is a lot and it is a __initdata. At least we should
begin with something and see later how to consolidate if it is needed, no?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v6 0/9] Multigenerational LRU Framework
From: SeongJae Park @ 2022-01-05 11:25 UTC (permalink / raw)
To: Yu Zhao
Cc: SeongJae Park, Andrew Morton, Linus Torvalds, Andi Kleen,
Catalin Marinas, Dave Hansen, Hillf Danton, Jens Axboe,
Jesse Barnes, Johannes Weiner, Jonathan Corbet, Matthew Wilcox,
Mel Gorman, Michael Larabel, Michal Hocko, Rik van Riel,
Vlastimil Babka, Will Deacon, Ying Huang, linux-arm-kernel,
linux-doc, linux-kernel, linux-mm, page-reclaim, x86
In-Reply-To: <YdV4k1+zEbtzmUkK@google.com>
On Wed, 5 Jan 2022 03:53:07 -0700 Yu Zhao <yuzhao@google.com> wrote:
> On Wed, Jan 05, 2022 at 08:55:34AM +0000, SeongJae Park wrote:
> > Hi Yu,
> >
> > On Tue, 4 Jan 2022 13:22:19 -0700 Yu Zhao <yuzhao@google.com> wrote:
[...]
> > I think similar works are already available out of the box with the latest
> > mainline tree, though it might be suboptimal in some cases.
>
> Ok, I will sound harsh because I hate it when people challenge facts
> while having no idea what they are talking about.
>
> Our jobs are help the leadership make best decisions by providing them
> with facts, not feeding them crap.
I was using the word "similar", to represent this is only for a rough concept
level similarity, rather than detailed facts. But, seems it was not enough,
sorry. Anyway, I will not talk more and thus disturb you having the important
discussion with leaders here, as you are asking.
>
> Don't get me wrong -- you are welcome to start another thread and have
> a casual discussion with me. But this thread is not for that; it's for
> the leadership and stakeholder to make a decision. Check who are in
> "To" and "Cc" and what my request is.
Haha. Ok, good luck for you.
Thanks,
SJ
[...]
^ permalink raw reply
* Re: [OE-core][PATCH] populate_sdk_base: remove useless dirs such as /dev
From: Richard Purdie @ 2022-01-05 11:24 UTC (permalink / raw)
To: Chen Qi, openembedded-core
In-Reply-To: <20220105060154.68560-1-Qi.Chen@windriver.com>
On Tue, 2022-01-04 at 22:01 -0800, Chen Qi wrote:
> We met a problem that core-image-tiny-initramfs's SDK cannot be
> installed. The error message is like below.
>
> tar: ./sysroots/core2-64-poky-linux/dev/console: Cannot mknod: Operation not permitted
>
> In fact, the '/dev' direcotry is useless for SDK. So remove it.
>
> This patches uses a variable, SDK_USELESS_DIRS, to hold useless dir entries
> so that it could be extended. For example, '/usr/bin' could be added if wanted.
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
> meta/classes/populate_sdk_base.bbclass | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/populate_sdk_base.bbclass b/meta/classes/populate_sdk_base.bbclass
> index fafdd96749..d4065b9b13 100644
> --- a/meta/classes/populate_sdk_base.bbclass
> +++ b/meta/classes/populate_sdk_base.bbclass
> @@ -92,6 +92,8 @@ SDK_HOST_MANIFEST = "${SDKDEPLOYDIR}/${TOOLCHAIN_OUTPUTNAME}.host.manifest"
> SDK_EXT_TARGET_MANIFEST = "${SDK_DEPLOY}/${TOOLCHAINEXT_OUTPUTNAME}.target.manifest"
> SDK_EXT_HOST_MANIFEST = "${SDK_DEPLOY}/${TOOLCHAINEXT_OUTPUTNAME}.host.manifest"
>
> +SDK_USELESS_DIRS ?= "/dev"
> +
I think this is the better approach to solving the issue but I don't agree with
the name "USELESS". Something like SDK_PRUNE_SYSROOT_DIRS would probably better
describe what the code is doing.
> python write_target_sdk_manifest () {
> from oe.sdk import sdk_list_installed_packages
> from oe.utils import format_pkg_list
> @@ -103,6 +105,12 @@ python write_target_sdk_manifest () {
> output.write(format_pkg_list(pkgs, 'ver'))
> }
>
> +delete_useless () {
This needs some prefix so we know it is sdk related as well as the useless name
change.
Cheers,
Richard
^ permalink raw reply
* PCI MSI issue for maxcpus=1
From: John Garry @ 2022-01-05 11:23 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner
Cc: chenxiang, Shameer Kolothum, linux-kernel@vger.kernel.org,
liuqi (BA)
Hi Marc,
Just a heads up, I noticed that commit 4c457e8cb75e ("genirq/msi:
Activate Multi-MSI early when MSI_FLAG_ACTIVATE_EARLY is set") is
causing an issue on our arm64 D06 board where the SAS driver probe fails
for maxcpus=1.
This seems different to issue [0].
So it's the driver call to pci_alloc_irq_vectors_affinity() which errors
[1]:
[ 9.619070] hisi_sas_v3_hw: probe of 0000:74:02.0 failed with error -2
Some details:
- device supports 32 MSI
- min and max msi for that function is 17 and 32, respect.
- affd pre and post are 16 and 0, respect.
I haven't checked to see what the issue is yet and I think that the
pci_alloc_irq_vectors_affinity() usage is ok...
[0]
https://lore.kernel.org/lkml/ea730f9b-c635-317d-c70d-4057590b1d1a@huawei.com/
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c?h=v5.11#n2388
Cheers,
John
^ permalink raw reply
* Re: [PATCH] i2c: add Rockchip i2c controller support
From: Sascha Hauer @ 2022-01-05 11:20 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox
In-Reply-To: <20220103120015.1727051-1-a.fatoum@pengutronix.de>
On Mon, Jan 03, 2022 at 01:00:15PM +0100, Ahmad Fatoum wrote:
> For addressing the PMIC on Rockchip platforms, add an i2c controller
> driver. Tested with rk3399 and rk808.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> drivers/i2c/busses/Kconfig | 8 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-rockchip.c | 472 ++++++++++++++++++++++++++++++
> 3 files changed, 481 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-rockchip.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index c1e76a04096e..a551df537a69 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -62,4 +62,12 @@ config I2C_STM32
> depends on HAVE_CLK
> depends on ARCH_STM32MP || COMPILE_TEST
>
> +config I2C_RK3X
> + tristate "Rockchip RK3xxx I2C adapter"
> + depends on HAVE_CLK
> + depends on ARCH_ROCKCHIP || COMPILE_TEST
> + help
> + Say Y here to include support for the I2C adapter in Rockchip RK3xxx
> + SoCs.
> +
> endmenu
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 34a12fcbea96..d6273f3d8679 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_I2C_TEGRA) += i2c-tegra.o
> obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o
> obj-$(CONFIG_I2C_DESIGNWARE) += i2c-designware.o
> obj-$(CONFIG_I2C_STM32) += i2c-stm32.o
> +obj-$(CONFIG_I2C_RK3X) += i2c-rockchip.o
> diff --git a/drivers/i2c/busses/i2c-rockchip.c b/drivers/i2c/busses/i2c-rockchip.c
> new file mode 100644
> index 000000000000..ba7397b19967
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-rockchip.c
> @@ -0,0 +1,472 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * (C) Copyright 2015 Google, Inc
> + *
> + * (C) Copyright 2008-2014 Rockchip Electronics
> + * Peter, Software Engineering, <superpeter.cai@gmail.com>.
> + */
> +
> +#include <common.h>
> +#include <i2c/i2c.h>
> +#include <linux/iopoll.h>
> +#include <errno.h>
> +#include <linux/err.h>
> +#include <driver.h>
> +#include <io.h>
> +#include <linux/clk.h>
> +#include <mfd/syscon.h>
> +#include <regmap.h>
> +#include <linux/sizes.h>
> +
> +struct i2c_regs {
> + u32 con;
> + u32 clkdiv;
> + u32 mrxaddr;
> + u32 mrxraddr;
> + u32 mtxcnt;
> + u32 mrxcnt;
> + u32 ien;
> + u32 ipd;
> + u32 fcnt;
> + u32 reserved0[0x37];
> + u32 txdata[8];
> + u32 reserved1[0x38];
> + u32 rxdata[8];
> +};
> +
> +/* Control register */
> +#define I2C_CON_EN (1 << 0)
> +#define I2C_CON_MOD(mod) ((mod) << 1)
> +#define I2C_MODE_TX 0x00
> +#define I2C_MODE_TRX 0x01
> +#define I2C_MODE_RX 0x02
> +#define I2C_MODE_RRX 0x03
> +#define I2C_CON_MASK (3 << 1)
> +
> +#define I2C_CON_START (1 << 3)
> +#define I2C_CON_STOP (1 << 4)
> +#define I2C_CON_LASTACK (1 << 5)
> +#define I2C_CON_ACTACK (1 << 6)
> +
> +/* Clock dividor register */
s/dividor/divider/
> +#define I2C_CLKDIV_VAL(divl, divh) \
> + (((divl) & 0xffff) | (((divh) << 16) & 0xffff0000))
> +
> +/* the slave address accessed for master rx mode */
> +#define I2C_MRXADDR_SET(vld, addr) (((vld) << 24) | (addr))
> +
> +/* the slave register address accessed for master rx mode */
> +#define I2C_MRXRADDR_SET(vld, raddr) (((vld) << 24) | (raddr))
> +
> +/* interrupt enable register */
> +#define I2C_BTFIEN (1 << 0)
> +#define I2C_BRFIEN (1 << 1)
> +#define I2C_MBTFIEN (1 << 2)
> +#define I2C_MBRFIEN (1 << 3)
> +#define I2C_STARTIEN (1 << 4)
> +#define I2C_STOPIEN (1 << 5)
> +#define I2C_NAKRCVIEN (1 << 6)
> +
> +/* interrupt pending register */
> +#define I2C_BTFIPD (1 << 0)
> +#define I2C_BRFIPD (1 << 1)
> +#define I2C_MBTFIPD (1 << 2)
> +#define I2C_MBRFIPD (1 << 3)
> +#define I2C_STARTIPD (1 << 4)
> +#define I2C_STOPIPD (1 << 5)
> +#define I2C_NAKRCVIPD (1 << 6)
> +#define I2C_IPD_ALL_CLEAN 0x7f
> +
> +/* i2c timerout */
s/timerout/timeout/
> +#define I2C_TIMEOUT (100 * MSECOND)
> +#define I2C_RETRY_COUNT 3
unused
> +
> +/* rk i2c fifo max transfer bytes */
> +#define RK_I2C_FIFO_SIZE 32
> +
> +struct rk_i2c {
> + struct i2c_adapter adapter;
> + struct clk *clk;
> + struct i2c_regs *regs;
> + unsigned int speed;
> +};
> +
> +static inline struct rk_i2c *to_rk_i2c(struct i2c_adapter *adapter)
> +{
> + return container_of(adapter, struct rk_i2c, adapter);
> +}
> +
> +static inline void rk_i2c_get_div(int div, int *divh, int *divl)
> +{
> + *divl = div / 2;
> + if (div % 2 == 0)
> + *divh = div / 2;
Why special case this? DIV_ROUND_UP(div, 2) should work for even numbers
as well.
> + else
> + *divh = DIV_ROUND_UP(div, 2);
> +}
> +
> +/*
> + * SCL Divisor = 8 * (CLKDIVL+1 + CLKDIVH+1)
> + * SCL = PCLK / SCLK Divisor
> + * i2c_rate = PCLK
> + */
> +static void rk_i2c_set_clk(struct rk_i2c *i2c, uint32_t scl_rate)
> +{
> + struct device_d *dev = i2c->adapter.dev.parent;
> + uint32_t i2c_rate;
> + int div, divl, divh;
> +
> + /* First get i2c rate from pclk */
> + i2c_rate = clk_get_rate(i2c->clk);
> +
> + div = DIV_ROUND_UP(i2c_rate, scl_rate * 8) - 2;
> + divh = 0;
> + divl = 0;
> + if (div >= 0)
> + rk_i2c_get_div(div, &divh, &divl);
> + writel(I2C_CLKDIV_VAL(divl, divh), &i2c->regs->clkdiv);
> +
> + dev_dbg(dev, "rk_i2c_set_clk: i2c rate = %d, scl rate = %d\n", i2c_rate,
> + scl_rate);
> + dev_dbg(dev, "set i2c clk div = %d, divh = %d, divl = %d\n", div, divh, divl);
> + dev_dbg(dev, "set clk(I2C_CLKDIV: 0x%08x)\n", readl(&i2c->regs->clkdiv));
> +}
> +
> +static void rk_i2c_show_regs(struct rk_i2c *i2c)
> +{
> + struct device_d *dev = &i2c->adapter.dev;
> + struct i2c_regs *regs = i2c->regs;
> + int i;
> +
> + dev_dbg(dev, "i2c_con: 0x%08x\n", readl(®s->con));
> + dev_dbg(dev, "i2c_clkdiv: 0x%08x\n", readl(®s->clkdiv));
> + dev_dbg(dev, "i2c_mrxaddr: 0x%08x\n", readl(®s->mrxaddr));
> + dev_dbg(dev, "i2c_mrxraddR: 0x%08x\n", readl(®s->mrxraddr));
> + dev_dbg(dev, "i2c_mtxcnt: 0x%08x\n", readl(®s->mtxcnt));
> + dev_dbg(dev, "i2c_mrxcnt: 0x%08x\n", readl(®s->mrxcnt));
> + dev_dbg(dev, "i2c_ien: 0x%08x\n", readl(®s->ien));
> + dev_dbg(dev, "i2c_ipd: 0x%08x\n", readl(®s->ipd));
> + dev_dbg(dev, "i2c_fcnt: 0x%08x\n", readl(®s->fcnt));
> +
> + for (i = 0; i < 8; i++)
> + dev_dbg(dev, "i2c_txdata%d: 0x%08x\n", i, readl(®s->txdata[i]));
> + for (i = 0; i < 8; i++)
> + dev_dbg(dev, "i2c_rxdata%d: 0x%08x\n", i, readl(®s->rxdata[i]));
> +}
> +
> +static int rk_i2c_send_start_bit(struct rk_i2c *i2c)
> +{
> + struct device_d *dev = &i2c->adapter.dev;
> + struct i2c_regs *regs = i2c->regs;
> + u64 start;
> +
> + dev_dbg(dev, "I2c Send Start bit.\n");
> + writel(I2C_IPD_ALL_CLEAN, ®s->ipd);
> +
> + writel(I2C_CON_EN | I2C_CON_START, ®s->con);
> + writel(I2C_STARTIEN, ®s->ien);
> +
> + for (start = get_time_ns(); !is_timeout(start, I2C_TIMEOUT);) {
> + if (readl(®s->ipd) & I2C_STARTIPD) {
> + writel(I2C_STARTIPD, ®s->ipd);
> + return 0;
> + }
> + udelay(1);
Unnecessary udelay()
> + }
> +
> + dev_dbg(dev, "I2C Send Start Bit Timeout\n");
> + rk_i2c_show_regs(i2c);
> + return -ETIMEDOUT;
> +}
> +
> +static int rk_i2c_send_stop_bit(struct rk_i2c *i2c)
> +{
> + struct device_d *dev = &i2c->adapter.dev;
> + struct i2c_regs *regs = i2c->regs;
> + u64 start;
> +
> + dev_dbg(dev, "I2c Send Stop bit.\n");
> + writel(I2C_IPD_ALL_CLEAN, ®s->ipd);
> +
> + writel(I2C_CON_EN | I2C_CON_STOP, ®s->con);
> + writel(I2C_CON_STOP, ®s->ien);
> +
> + for (start = get_time_ns(); !is_timeout(start, I2C_TIMEOUT);) {
> + if (readl(®s->ipd) & I2C_STOPIPD) {
> + writel(I2C_STOPIPD, ®s->ipd);
> + return 0;
> + }
> + udelay(1);
ditto
> + }
> +
> + dev_dbg(dev, "I2C Send Start Bit Timeout\n");
> + rk_i2c_show_regs(i2c);
> + return -ETIMEDOUT;
> +}
> +
> +static inline void rk_i2c_disable(struct rk_i2c *i2c)
> +{
> + writel(0, &i2c->regs->con);
> +}
> +
> +static int rk_i2c_read(struct rk_i2c *i2c, uchar chip, uint reg, uint r_len,
> + uchar *buf, uint b_len)
> +{
> + struct device_d *dev = &i2c->adapter.dev;
> + struct i2c_regs *regs = i2c->regs;
> + uchar *pbuf = buf;
> + uint bytes_remain_len = b_len;
> + uint bytes_xferred = 0;
> + uint words_xferred = 0;
> + u64 start;
> + uint con = 0;
> + uint rxdata;
> + uint i, j;
> + int err;
> + bool snd_chunk = false;
> +
> + dev_dbg(dev, "rk_i2c_read: chip = %d, reg = %d, r_len = %d, b_len = %d\n",
> + chip, reg, r_len, b_len);
> +
> + err = rk_i2c_send_start_bit(i2c);
> + if (err)
> + return err;
> +
> + writel(I2C_MRXADDR_SET(1, chip << 1 | 1), ®s->mrxaddr);
> + if (r_len == 0) {
> + writel(0, ®s->mrxraddr);
> + } else if (r_len < 4) {
> + writel(I2C_MRXRADDR_SET(r_len, reg), ®s->mrxraddr);
> + } else {
> + dev_dbg(dev, "I2C Read: addr len %d not supported\n", r_len);
> + return -EIO;
> + }
> +
> + while (bytes_remain_len) {
> + if (bytes_remain_len > RK_I2C_FIFO_SIZE) {
> + con = I2C_CON_EN;
> + bytes_xferred = 32;
bytes_xferred = RK_I2C_FIFO_SIZE
> + } else {
> + /*
> + * The hw can read up to 32 bytes at a time. If we need
> + * more than one chunk, send an ACK after the last byte.
> + */
> + con = I2C_CON_EN | I2C_CON_LASTACK;
> + bytes_xferred = bytes_remain_len;
> + }
> + words_xferred = DIV_ROUND_UP(bytes_xferred, 4);
> +
> + /*
> + * make sure we are in plain RX mode if we read a second chunk
> + */
> + if (snd_chunk)
> + con |= I2C_CON_MOD(I2C_MODE_RX);
> + else
> + con |= I2C_CON_MOD(I2C_MODE_TRX);
> +
> + writel(con, ®s->con);
> + writel(bytes_xferred, ®s->mrxcnt);
> + writel(I2C_MBRFIEN | I2C_NAKRCVIEN, ®s->ien);
> +
> + err = -ETIMEDOUT;
> + for (start = get_time_ns(); !is_timeout(start, I2C_TIMEOUT);) {
> + if (readl(®s->ipd) & I2C_NAKRCVIPD) {
> + writel(I2C_NAKRCVIPD, ®s->ipd);
> + err = -EREMOTEIO;
Does it make sense to stay in the loop here?
> + }
> + if (readl(®s->ipd) & I2C_MBRFIPD) {
> + writel(I2C_MBRFIPD, ®s->ipd);
> + err = 0;
> + break;
> + }
> + udelay(1);
Unnecessary udelay().
> + }
> +
> + if (err) {
> + dev_dbg(dev, "I2C Read Data Timeout\n");
> + rk_i2c_show_regs(i2c);
> + goto i2c_exit;
> + }
It's easier to follow when you just exit the loop with
if (is_timeout()) {
err = -ETIMEDOUT;
goto i2c_exit;
}
> +
> + for (i = 0; i < words_xferred; i++) {
> + rxdata = readl(®s->rxdata[i]);
> + dev_dbg(dev, "I2c Read RXDATA[%d] = 0x%x\n", i, rxdata);
> + for (j = 0; j < 4; j++) {
> + if ((i * 4 + j) == bytes_xferred)
> + break;
> + *pbuf++ = (rxdata >> (j * 8)) & 0xff;
> + }
> + }
> +
> + bytes_remain_len -= bytes_xferred;
> + snd_chunk = true;
> + dev_dbg(dev, "I2C Read bytes_remain_len %d\n", bytes_remain_len);
> + }
> +
> +i2c_exit:
> + rk_i2c_disable(i2c);
> +
> + return err;
> +}
> +
> +static int rk_i2c_write(struct rk_i2c *i2c, uchar chip, uint reg, uint r_len,
> + uchar *buf, uint b_len)
> +{
> + struct device_d *dev = &i2c->adapter.dev;
> + struct i2c_regs *regs = i2c->regs;
> + int err;
> + uchar *pbuf = buf;
> + uint bytes_remain_len = b_len + r_len + 1;
> + uint bytes_xferred = 0;
> + uint words_xferred = 0;
> + u64 start;
> + uint txdata;
> + uint i, j;
> +
> + dev_dbg(dev, "rk_i2c_write: chip = %d, reg = %d, r_len = %d, b_len = %d\n",
> + chip, reg, r_len, b_len);
> + err = rk_i2c_send_start_bit(i2c);
> + if (err)
> + return err;
> +
> + while (bytes_remain_len) {
> + if (bytes_remain_len > RK_I2C_FIFO_SIZE)
> + bytes_xferred = RK_I2C_FIFO_SIZE;
> + else
> + bytes_xferred = bytes_remain_len;
> + words_xferred = DIV_ROUND_UP(bytes_xferred, 4);
> +
> + for (i = 0; i < words_xferred; i++) {
> + txdata = 0;
> + for (j = 0; j < 4; j++) {
> + if ((i * 4 + j) == bytes_xferred)
> + break;
> +
> + if (i == 0 && j == 0 && pbuf == buf) {
> + txdata |= (chip << 1);
> + } else if (i == 0 && j <= r_len && pbuf == buf) {
> + txdata |= (reg &
> + (0xff << ((j - 1) * 8))) << 8;
> + } else {
> + txdata |= (*pbuf++)<<(j * 8);
> + }
> + }
> + writel(txdata, ®s->txdata[i]);
> + dev_dbg(dev, "I2c Write TXDATA[%d] = 0x%08x\n", i, txdata);
> + }
> +
> + writel(I2C_CON_EN | I2C_CON_MOD(I2C_MODE_TX), ®s->con);
> + writel(bytes_xferred, ®s->mtxcnt);
> + writel(I2C_MBTFIEN | I2C_NAKRCVIEN, ®s->ien);
> +
> + err = -ETIMEDOUT;
> + for (start = get_time_ns(); !is_timeout(start, I2C_TIMEOUT);) {
> + if (readl(®s->ipd) & I2C_NAKRCVIPD) {
> + writel(I2C_NAKRCVIPD, ®s->ipd);
> + err = -EREMOTEIO;
> + }
Same as above, need to stay in the loop here?
> + if (readl(®s->ipd) & I2C_MBTFIPD) {
> + writel(I2C_MBTFIPD, ®s->ipd);
> + err = 0;
> + break;
> + }
> + udelay(1);
Drop the udelay()
> + }
> +
> + if (err) {
> + dev_dbg(dev, "I2C Write Data Timeout\n");
> + rk_i2c_show_regs(i2c);
> + goto i2c_exit;
> + }
Test for timeout in the loop.
> +
> + bytes_remain_len -= bytes_xferred;
> + dev_dbg(dev, "I2C Write bytes_remain_len %d\n", bytes_remain_len);
> + }
> +
> +i2c_exit:
> + rk_i2c_disable(i2c);
> +
> + return err;
> +}
> +
> +static int rockchip_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msg,
> + int nmsgs)
> +{
> + struct rk_i2c *i2c = to_rk_i2c(adapter);
> + struct device_d *dev = &adapter->dev;
> + int ret;
> +
> + dev_dbg(dev, "i2c_xfer: %d messages\n", nmsgs);
> + for (; nmsgs > 0; nmsgs--, msg++) {
> + dev_dbg(dev, "i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
> + if (msg->flags & I2C_M_RD) {
> + ret = rk_i2c_read(i2c, msg->addr, 0, 0, msg->buf,
> + msg->len);
> + } else {
> + ret = rk_i2c_write(i2c, msg->addr, 0, 0, msg->buf,
> + msg->len);
> + }
> + if (ret) {
> + dev_dbg(dev, "i2c_write: error sending\n");
> + return -EREMOTEIO;
Print the error code? Forward the error?
> + }
> + }
> +
> + rk_i2c_send_stop_bit(i2c);
> + rk_i2c_disable(i2c);
> +
> + return 0;
> +}
> +
> +static int rk_i2c_probe(struct device_d *dev)
> +{
> + struct device_node *np = dev->device_node;
> + struct resource *iores;
> + struct rk_i2c *i2c;
> + unsigned bitrate;
> +
> + iores = dev_request_mem_resource(dev, 0);
> + if (IS_ERR(iores))
> + return PTR_ERR(iores);
> +
> + i2c = kzalloc(sizeof(struct rk_i2c), GFP_KERNEL);
> + if (!i2c)
> + return -ENOMEM;
> +
> + dev->priv = i2c;
> + i2c->regs = IOMEM(iores->start);
> +
> + /* Only one clock to use for bus clock and peripheral clock */
> + i2c->clk = clk_get(dev, NULL);
> + if (IS_ERR(i2c->clk))
> + return dev_err_probe(dev, PTR_ERR(i2c->clk), "Can't get bus clk\n");
> +
> + /* Setup i2c_fsl driver structure */
copy/paste, it's not i2c_fsl
> + i2c->adapter.master_xfer = rockchip_i2c_xfer;
> + i2c->adapter.nr = dev->id;
> + i2c->adapter.dev.parent = dev;
> + i2c->adapter.dev.device_node = np;
> +
> + /* Set up clock divider */
> + bitrate = 100000;
> + of_property_read_u32(np, "clock-frequency", &bitrate);
> +
> + rk_i2c_set_clk(i2c, bitrate);
> +
> + return i2c_add_numbered_adapter(&i2c->adapter);
> +}
> +
> +static const struct of_device_id rk_i2c_match[] = {
> + { .compatible = "rockchip,rv1108-i2c" },
> + { .compatible = "rockchip,rk3228-i2c" },
> + { .compatible = "rockchip,rk3288-i2c" },
> + { .compatible = "rockchip,rk3399-i2c" },
> + {},
> +};
> +
> +static struct driver_d rk_i2c_driver = {
> + .name = "rk3x-i2c",
> + .of_compatible = rk_i2c_match,
> + .probe = rk_i2c_probe,
> +};
> +coredevice_platform_driver(rk_i2c_driver);
> --
> 2.30.2
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply
* RE: [i-g-t 04/14] tests/kms_color: New subtests for Plane gamma
From: Modem, Bhanuprakash @ 2022-01-05 11:21 UTC (permalink / raw)
To: Harry Wentland, igt-dev@lists.freedesktop.org,
dri-devel@lists.freedesktop.org
Cc: Shankar, Uma
In-Reply-To: <d8d00fe5-da3c-cf8a-f7ef-6f525b1d551a@amd.com>
> From: Harry Wentland <harry.wentland@amd.com>
> Sent: Wednesday, January 5, 2022 2:49 AM
> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; igt-
> dev@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Shankar, Uma
> <uma.shankar@intel.com>; ppaalanen@gmail.com
> Subject: Re: [i-g-t 04/14] tests/kms_color: New subtests for Plane gamma
>
> On 2022-01-02 23:05, Modem, Bhanuprakash wrote:
> >> From: Harry Wentland <harry.wentland@amd.com>
> >> Sent: Friday, November 26, 2021 10:25 PM
> >> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; igt-
> >> dev@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Juha-Pekka Heikkila
> >> <juhapekka.heikkila@gmail.com>; Shankar, Uma <uma.shankar@intel.com>
> >> Subject: Re: [i-g-t 04/14] tests/kms_color: New subtests for Plane gamma
> >>
> >> On 2021-11-15 04:47, Bhanuprakash Modem wrote:
> >>> To verify Plane gamma, draw 3 gradient rectangles in red, green and blue,
> >>> with a maxed out gamma LUT and verify we have the same CRC as drawing
> solid
> >>> color rectangles.
> >>>
> >>> Cc: Harry Wentland <harry.wentland@amd.com>
> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >>> Cc: Uma Shankar <uma.shankar@intel.com>
> >>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> >>> ---
> >>> tests/kms_color.c | 179 +++++++++++++++++++++++++++++++++++++++++++++-
> >>> 1 file changed, 178 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tests/kms_color.c b/tests/kms_color.c
> >>> index 775f35964f..b45d66762f 100644
> >>> --- a/tests/kms_color.c
> >>> +++ b/tests/kms_color.c
> >>> @@ -24,7 +24,34 @@
> >>>
> >>> #include "kms_color_helper.h"
> >>>
> >>> -IGT_TEST_DESCRIPTION("Test Color Features at Pipe level");
> >>> +IGT_TEST_DESCRIPTION("Test Color Features at Pipe & Plane level");
> >>> +
> >>> +#define MAX_SUPPORTED_PLANES 7
> >>> +#define SDR_PLANE_BASE 3
> >>> +
> >>> +typedef bool (*test_t)(data_t*, igt_plane_t*);
> >>> +
> >>> +static bool is_hdr_plane(const igt_plane_t *plane)
> >>> +{
> >>> + return plane->index >= 0 && plane->index < SDR_PLANE_BASE;
> >>> +}
> >>> +
> >>> +static bool is_valid_plane(igt_plane_t *plane)
> >>> +{
> >>> + int index = plane->index;
> >>> +
> >>> + if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> >>> + return false;
> >>> +
> >>> + /*
> >>> + * Test 1 HDR plane, 1 SDR plane.
> >>> + *
> >>> + * 0,1,2 HDR planes
> >>> + * 3,4,5,6 SDR planes
> >>> + *
> >>> + */
> >>
> >> This seems to be about Intel HW. AMD HW for example does
> >> not have HDR or SDR planes, only one generic plane.
> >>
> >>> + return index >= 0 && index < MAX_SUPPORTED_PLANES;
> >>> +}
> >>>
> >>> static void test_pipe_degamma(data_t *data,
> >>> igt_plane_t *primary)
> >>> @@ -638,6 +665,122 @@ static void test_pipe_limited_range_ctm(data_t
> *data,
> >>> }
> >>> #endif
> >>>
> >>> +static bool plane_gamma_test(data_t *data, igt_plane_t *plane)
> >>> +{
> >>> + igt_output_t *output;
> >>> + igt_display_t *display = &data->display;
> >>> + drmModeModeInfo *mode;
> >>> + struct igt_fb fb;
> >>> + drmModePropertyPtr gamma_mode = NULL;
> >>> + uint32_t i;
> >>> + bool ret = true;
> >>> + igt_pipe_crc_t *pipe_crc = NULL;
> >>> + color_t red_green_blue[] = {
> >>> + { 1.0, 0.0, 0.0 },
> >>> + { 0.0, 1.0, 0.0 },
> >>> + { 0.0, 0.0, 1.0 }
> >>> + };
> >>> +
> >>> + igt_info("Plane gamma test is running on pipe-%s plane-%s(%s)\n",
> >>> + kmstest_pipe_name(plane->pipe->pipe),
> >>> + kmstest_plane_type_name(plane->type),
> >>> + is_hdr_plane(plane) ? "hdr":"sdr");
> >>> +
> >>
> >> is_hdr_plane is Intel-specific.
> >>
> >>> + igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_MODE));
> >>> + igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_LUT));
> >>> +
> >>> + pipe_crc = igt_pipe_crc_new(data->drm_fd,
> >>> + plane->pipe->pipe,
> >>> + INTEL_PIPE_CRC_SOURCE_AUTO);
> >>> +
> >>> + output = igt_get_single_output_for_pipe(display, plane->pipe->pipe);
> >>> + igt_assert(output);
> >>> +
> >>> + igt_output_set_pipe(output, plane->pipe->pipe);
> >>> + mode = igt_output_get_mode(output);
> >>> +
> >>> + /* Create a framebuffer at the size of the output. */
> >>> + igt_assert(igt_create_fb(data->drm_fd,
> >>> + mode->hdisplay,
> >>> + mode->vdisplay,
> >>> + DRM_FORMAT_XRGB8888,
> >>> + DRM_FORMAT_MOD_LINEAR,
> >>> + &fb));
> >>> + igt_plane_set_fb(plane, &fb);
> >>> +
> >>> + /* Disable Pipe color props. */
> >>> + disable_ctm(plane->pipe);
> >>> + disable_degamma(plane->pipe);
> >>> + disable_gamma(plane->pipe);
> >>> +
> >>> + disable_plane_ctm(plane);
> >>> + disable_plane_degamma(plane);
> >>> + igt_display_commit2(display, display->is_atomic ?
> >>> + COMMIT_ATOMIC : COMMIT_LEGACY);
> >>> +
> >>> + gamma_mode = get_plane_gamma_degamma_mode(plane, IGT_PLANE_GAMMA_MODE);
> >>> +
> >>> + /* Iterate all supported gamma modes. */
> >>> + for (i = 0; i < gamma_mode->count_enums; i++) {
> >>> + igt_crc_t crc_gamma, crc_fullcolors;
> >>> + segment_data_t *segment_info = NULL;
> >>> + struct drm_color_lut_ext *lut = NULL;
> >>> + uint32_t lut_size = 0;
> >>> +
> >>> + /* Ignore 'no gamma' from enum list. */
> >>> + if (!strcmp(gamma_mode->enums[i].name, "no gamma"))
> >>> + continue;
> >>> +
> >>
> >> It might still make sense to test that this is doing bypass.
> >
> > As we know gamma_mode->enum[i].name represents the name of the
> > gamma mode and gamma_mode->enum[i].value would be the LUT blob
> > address of that particular gamma_mode.
> >
> > For "no gamma" case the blob address is NULL, so we can't commit
> > this mode. Hence skipping this mode.
> >
>
> I was thinking it'd be good to test the "no gamma" case as well
> here, i.e. the case were we set a NULL blob. I'm not sure what
> you mean when you say "we can't commit this mode."
Sorry for the confusion, "no gamma" is intentional to disable the gamma.
I think, we just need to check that we can able to flip with this mode.
So, we need to update disable_plane_gamma() to use this mode.
Or are you thinking any specific usecase for this?
- Bhanu
>
> Harry
>
> >>
> >>> + igt_info("Trying to use gamma mode: \'%s\'\n", gamma_mode-
> >>> enums[i].name);
> >>> +
> >>> + /* Draw solid colors with no gamma transformation. */
> >>> + disable_plane_gamma(plane);
> >>> + paint_rectangles(data, mode, red_green_blue, &fb);
> >>> + igt_plane_set_fb(plane, &fb);
> >>> + igt_display_commit2(display, display->is_atomic ?
> >>> + COMMIT_ATOMIC : COMMIT_LEGACY);
> >>> + igt_wait_for_vblank(data->drm_fd,
> >>> + display->pipes[plane->pipe->pipe].crtc_offset);
> >>> + igt_pipe_crc_collect_crc(pipe_crc, &crc_fullcolors);
> >>> +
> >>> + /* Draw gradient colors with gamma LUT to remap all
> >>> + * values to max red/green/blue.
> >>> + */
> >>> + segment_info = get_segment_data(data, gamma_mode->enums[i].value,
> >>> + gamma_mode->enums[i].name);
> >>> + lut_size = sizeof(struct drm_color_lut_ext) * segment_info-
> >>> entries_count;
> >>> + lut = create_max_lut(segment_info);
> >>> + set_plane_gamma(plane, gamma_mode->enums[i].name, lut, lut_size);
> >>> +
> >>> + paint_gradient_rectangles(data, mode, red_green_blue, &fb);
> >>> + igt_plane_set_fb(plane, &fb);
> >>> + igt_display_commit2(display, display->is_atomic ?
> >>> + COMMIT_ATOMIC : COMMIT_LEGACY);
> >>> + igt_wait_for_vblank(data->drm_fd,
> >>> + display->pipes[plane->pipe->pipe].crtc_offset);
> >>> + igt_pipe_crc_collect_crc(pipe_crc, &crc_gamma);
> >>> +
> >>> + /* Verify that the CRC of the software computed output is
> >>> + * equal to the CRC of the gamma LUT transformation output.
> >>> + */
> >>> + ret &= igt_check_crc_equal(&crc_gamma, &crc_fullcolors);
> >>> +
> >>> + free(lut);
> >>> + clear_segment_data(segment_info);
> >>> + }
> >>> +
> >>> + disable_plane_gamma(plane);
> >>> + igt_plane_set_fb(plane, NULL);
> >>> + igt_output_set_pipe(output, PIPE_NONE);
> >>> + igt_display_commit2(display, display->is_atomic ?
> >>> + COMMIT_ATOMIC : COMMIT_LEGACY);
> >>> +
> >>> + igt_pipe_crc_free(pipe_crc);
> >>> + drmModeFreeProperty(gamma_mode);
> >>> +
> >>> + return ret;
> >>> +}
> >>> +
> >>> static void
> >>> prep_pipe(data_t *data, enum pipe p)
> >>> {
> >>> @@ -890,6 +1033,37 @@ run_invalid_tests_for_pipe(data_t *data, enum pipe
> p)
> >>> invalid_ctm_matrix_sizes(data, p);
> >>> }
> >>>
> >>> +static void run_plane_color_test(data_t *data, enum pipe pipe, test_t
> test)
> >>> +{
> >>> + igt_plane_t *plane;
> >>> + int count = 0;
> >>> +
> >>> + for_each_plane_on_pipe(&data->display, pipe, plane) {
> >>> + if (!is_valid_plane(plane))
> >>> + continue;
> >>> +
> >>> + igt_assert(test(data, plane));
> >>> +
> >>> + count++;
> >>> + }
> >>> +
> >>> + igt_require_f(count, "No valid planes found.\n");
> >>> +}
> >>> +
> >>> +static void run_tests_for_plane(data_t *data, enum pipe pipe)
> >>> +{
> >>> + igt_fixture {
> >>> + igt_require_pipe(&data->display, pipe);
> >>> + igt_require_pipe_crc(data->drm_fd);
> >>> + igt_require(data->display.pipes[pipe].n_planes > 0);
> >>> + igt_display_require_output_on_pipe(&data->display, pipe);
> >>> + }
> >>> +
> >>> + igt_describe("Compare maxed out plane gamma LUT and solid color linear
> >> LUT");
> >>
> >> I can't seem to see the linear LUT test. This only seems to test
> >> the max LUT.
> >>
> >> Harry
> >>
> >>> + igt_subtest_f("pipe-%s-plane-gamma", kmstest_pipe_name(pipe))
> >>> + run_plane_color_test(data, pipe, plane_gamma_test);
> >>> +}
> >>> +
> >>> igt_main
> >>> {
> >>> data_t data = {};
> >>> @@ -910,6 +1084,9 @@ igt_main
> >>>
> >>> igt_subtest_group
> >>> run_invalid_tests_for_pipe(&data, pipe);
> >>> +
> >>> + igt_subtest_group
> >>> + run_tests_for_plane(&data, pipe);
> >>> }
> >>>
> >>> igt_fixture {
> >>>
> >
^ permalink raw reply
* Re: [igt-dev] [i-g-t 04/14] tests/kms_color: New subtests for Plane gamma
From: Modem, Bhanuprakash @ 2022-01-05 11:21 UTC (permalink / raw)
To: Harry Wentland, igt-dev@lists.freedesktop.org,
dri-devel@lists.freedesktop.org
Cc: ppaalanen@gmail.com
In-Reply-To: <d8d00fe5-da3c-cf8a-f7ef-6f525b1d551a@amd.com>
> From: Harry Wentland <harry.wentland@amd.com>
> Sent: Wednesday, January 5, 2022 2:49 AM
> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; igt-
> dev@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Shankar, Uma
> <uma.shankar@intel.com>; ppaalanen@gmail.com
> Subject: Re: [i-g-t 04/14] tests/kms_color: New subtests for Plane gamma
>
> On 2022-01-02 23:05, Modem, Bhanuprakash wrote:
> >> From: Harry Wentland <harry.wentland@amd.com>
> >> Sent: Friday, November 26, 2021 10:25 PM
> >> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; igt-
> >> dev@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Juha-Pekka Heikkila
> >> <juhapekka.heikkila@gmail.com>; Shankar, Uma <uma.shankar@intel.com>
> >> Subject: Re: [i-g-t 04/14] tests/kms_color: New subtests for Plane gamma
> >>
> >> On 2021-11-15 04:47, Bhanuprakash Modem wrote:
> >>> To verify Plane gamma, draw 3 gradient rectangles in red, green and blue,
> >>> with a maxed out gamma LUT and verify we have the same CRC as drawing
> solid
> >>> color rectangles.
> >>>
> >>> Cc: Harry Wentland <harry.wentland@amd.com>
> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >>> Cc: Uma Shankar <uma.shankar@intel.com>
> >>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> >>> ---
> >>> tests/kms_color.c | 179 +++++++++++++++++++++++++++++++++++++++++++++-
> >>> 1 file changed, 178 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tests/kms_color.c b/tests/kms_color.c
> >>> index 775f35964f..b45d66762f 100644
> >>> --- a/tests/kms_color.c
> >>> +++ b/tests/kms_color.c
> >>> @@ -24,7 +24,34 @@
> >>>
> >>> #include "kms_color_helper.h"
> >>>
> >>> -IGT_TEST_DESCRIPTION("Test Color Features at Pipe level");
> >>> +IGT_TEST_DESCRIPTION("Test Color Features at Pipe & Plane level");
> >>> +
> >>> +#define MAX_SUPPORTED_PLANES 7
> >>> +#define SDR_PLANE_BASE 3
> >>> +
> >>> +typedef bool (*test_t)(data_t*, igt_plane_t*);
> >>> +
> >>> +static bool is_hdr_plane(const igt_plane_t *plane)
> >>> +{
> >>> + return plane->index >= 0 && plane->index < SDR_PLANE_BASE;
> >>> +}
> >>> +
> >>> +static bool is_valid_plane(igt_plane_t *plane)
> >>> +{
> >>> + int index = plane->index;
> >>> +
> >>> + if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> >>> + return false;
> >>> +
> >>> + /*
> >>> + * Test 1 HDR plane, 1 SDR plane.
> >>> + *
> >>> + * 0,1,2 HDR planes
> >>> + * 3,4,5,6 SDR planes
> >>> + *
> >>> + */
> >>
> >> This seems to be about Intel HW. AMD HW for example does
> >> not have HDR or SDR planes, only one generic plane.
> >>
> >>> + return index >= 0 && index < MAX_SUPPORTED_PLANES;
> >>> +}
> >>>
> >>> static void test_pipe_degamma(data_t *data,
> >>> igt_plane_t *primary)
> >>> @@ -638,6 +665,122 @@ static void test_pipe_limited_range_ctm(data_t
> *data,
> >>> }
> >>> #endif
> >>>
> >>> +static bool plane_gamma_test(data_t *data, igt_plane_t *plane)
> >>> +{
> >>> + igt_output_t *output;
> >>> + igt_display_t *display = &data->display;
> >>> + drmModeModeInfo *mode;
> >>> + struct igt_fb fb;
> >>> + drmModePropertyPtr gamma_mode = NULL;
> >>> + uint32_t i;
> >>> + bool ret = true;
> >>> + igt_pipe_crc_t *pipe_crc = NULL;
> >>> + color_t red_green_blue[] = {
> >>> + { 1.0, 0.0, 0.0 },
> >>> + { 0.0, 1.0, 0.0 },
> >>> + { 0.0, 0.0, 1.0 }
> >>> + };
> >>> +
> >>> + igt_info("Plane gamma test is running on pipe-%s plane-%s(%s)\n",
> >>> + kmstest_pipe_name(plane->pipe->pipe),
> >>> + kmstest_plane_type_name(plane->type),
> >>> + is_hdr_plane(plane) ? "hdr":"sdr");
> >>> +
> >>
> >> is_hdr_plane is Intel-specific.
> >>
> >>> + igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_MODE));
> >>> + igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_LUT));
> >>> +
> >>> + pipe_crc = igt_pipe_crc_new(data->drm_fd,
> >>> + plane->pipe->pipe,
> >>> + INTEL_PIPE_CRC_SOURCE_AUTO);
> >>> +
> >>> + output = igt_get_single_output_for_pipe(display, plane->pipe->pipe);
> >>> + igt_assert(output);
> >>> +
> >>> + igt_output_set_pipe(output, plane->pipe->pipe);
> >>> + mode = igt_output_get_mode(output);
> >>> +
> >>> + /* Create a framebuffer at the size of the output. */
> >>> + igt_assert(igt_create_fb(data->drm_fd,
> >>> + mode->hdisplay,
> >>> + mode->vdisplay,
> >>> + DRM_FORMAT_XRGB8888,
> >>> + DRM_FORMAT_MOD_LINEAR,
> >>> + &fb));
> >>> + igt_plane_set_fb(plane, &fb);
> >>> +
> >>> + /* Disable Pipe color props. */
> >>> + disable_ctm(plane->pipe);
> >>> + disable_degamma(plane->pipe);
> >>> + disable_gamma(plane->pipe);
> >>> +
> >>> + disable_plane_ctm(plane);
> >>> + disable_plane_degamma(plane);
> >>> + igt_display_commit2(display, display->is_atomic ?
> >>> + COMMIT_ATOMIC : COMMIT_LEGACY);
> >>> +
> >>> + gamma_mode = get_plane_gamma_degamma_mode(plane, IGT_PLANE_GAMMA_MODE);
> >>> +
> >>> + /* Iterate all supported gamma modes. */
> >>> + for (i = 0; i < gamma_mode->count_enums; i++) {
> >>> + igt_crc_t crc_gamma, crc_fullcolors;
> >>> + segment_data_t *segment_info = NULL;
> >>> + struct drm_color_lut_ext *lut = NULL;
> >>> + uint32_t lut_size = 0;
> >>> +
> >>> + /* Ignore 'no gamma' from enum list. */
> >>> + if (!strcmp(gamma_mode->enums[i].name, "no gamma"))
> >>> + continue;
> >>> +
> >>
> >> It might still make sense to test that this is doing bypass.
> >
> > As we know gamma_mode->enum[i].name represents the name of the
> > gamma mode and gamma_mode->enum[i].value would be the LUT blob
> > address of that particular gamma_mode.
> >
> > For "no gamma" case the blob address is NULL, so we can't commit
> > this mode. Hence skipping this mode.
> >
>
> I was thinking it'd be good to test the "no gamma" case as well
> here, i.e. the case were we set a NULL blob. I'm not sure what
> you mean when you say "we can't commit this mode."
Sorry for the confusion, "no gamma" is intentional to disable the gamma.
I think, we just need to check that we can able to flip with this mode.
So, we need to update disable_plane_gamma() to use this mode.
Or are you thinking any specific usecase for this?
- Bhanu
>
> Harry
>
> >>
> >>> + igt_info("Trying to use gamma mode: \'%s\'\n", gamma_mode-
> >>> enums[i].name);
> >>> +
> >>> + /* Draw solid colors with no gamma transformation. */
> >>> + disable_plane_gamma(plane);
> >>> + paint_rectangles(data, mode, red_green_blue, &fb);
> >>> + igt_plane_set_fb(plane, &fb);
> >>> + igt_display_commit2(display, display->is_atomic ?
> >>> + COMMIT_ATOMIC : COMMIT_LEGACY);
> >>> + igt_wait_for_vblank(data->drm_fd,
> >>> + display->pipes[plane->pipe->pipe].crtc_offset);
> >>> + igt_pipe_crc_collect_crc(pipe_crc, &crc_fullcolors);
> >>> +
> >>> + /* Draw gradient colors with gamma LUT to remap all
> >>> + * values to max red/green/blue.
> >>> + */
> >>> + segment_info = get_segment_data(data, gamma_mode->enums[i].value,
> >>> + gamma_mode->enums[i].name);
> >>> + lut_size = sizeof(struct drm_color_lut_ext) * segment_info-
> >>> entries_count;
> >>> + lut = create_max_lut(segment_info);
> >>> + set_plane_gamma(plane, gamma_mode->enums[i].name, lut, lut_size);
> >>> +
> >>> + paint_gradient_rectangles(data, mode, red_green_blue, &fb);
> >>> + igt_plane_set_fb(plane, &fb);
> >>> + igt_display_commit2(display, display->is_atomic ?
> >>> + COMMIT_ATOMIC : COMMIT_LEGACY);
> >>> + igt_wait_for_vblank(data->drm_fd,
> >>> + display->pipes[plane->pipe->pipe].crtc_offset);
> >>> + igt_pipe_crc_collect_crc(pipe_crc, &crc_gamma);
> >>> +
> >>> + /* Verify that the CRC of the software computed output is
> >>> + * equal to the CRC of the gamma LUT transformation output.
> >>> + */
> >>> + ret &= igt_check_crc_equal(&crc_gamma, &crc_fullcolors);
> >>> +
> >>> + free(lut);
> >>> + clear_segment_data(segment_info);
> >>> + }
> >>> +
> >>> + disable_plane_gamma(plane);
> >>> + igt_plane_set_fb(plane, NULL);
> >>> + igt_output_set_pipe(output, PIPE_NONE);
> >>> + igt_display_commit2(display, display->is_atomic ?
> >>> + COMMIT_ATOMIC : COMMIT_LEGACY);
> >>> +
> >>> + igt_pipe_crc_free(pipe_crc);
> >>> + drmModeFreeProperty(gamma_mode);
> >>> +
> >>> + return ret;
> >>> +}
> >>> +
> >>> static void
> >>> prep_pipe(data_t *data, enum pipe p)
> >>> {
> >>> @@ -890,6 +1033,37 @@ run_invalid_tests_for_pipe(data_t *data, enum pipe
> p)
> >>> invalid_ctm_matrix_sizes(data, p);
> >>> }
> >>>
> >>> +static void run_plane_color_test(data_t *data, enum pipe pipe, test_t
> test)
> >>> +{
> >>> + igt_plane_t *plane;
> >>> + int count = 0;
> >>> +
> >>> + for_each_plane_on_pipe(&data->display, pipe, plane) {
> >>> + if (!is_valid_plane(plane))
> >>> + continue;
> >>> +
> >>> + igt_assert(test(data, plane));
> >>> +
> >>> + count++;
> >>> + }
> >>> +
> >>> + igt_require_f(count, "No valid planes found.\n");
> >>> +}
> >>> +
> >>> +static void run_tests_for_plane(data_t *data, enum pipe pipe)
> >>> +{
> >>> + igt_fixture {
> >>> + igt_require_pipe(&data->display, pipe);
> >>> + igt_require_pipe_crc(data->drm_fd);
> >>> + igt_require(data->display.pipes[pipe].n_planes > 0);
> >>> + igt_display_require_output_on_pipe(&data->display, pipe);
> >>> + }
> >>> +
> >>> + igt_describe("Compare maxed out plane gamma LUT and solid color linear
> >> LUT");
> >>
> >> I can't seem to see the linear LUT test. This only seems to test
> >> the max LUT.
> >>
> >> Harry
> >>
> >>> + igt_subtest_f("pipe-%s-plane-gamma", kmstest_pipe_name(pipe))
> >>> + run_plane_color_test(data, pipe, plane_gamma_test);
> >>> +}
> >>> +
> >>> igt_main
> >>> {
> >>> data_t data = {};
> >>> @@ -910,6 +1084,9 @@ igt_main
> >>>
> >>> igt_subtest_group
> >>> run_invalid_tests_for_pipe(&data, pipe);
> >>> +
> >>> + igt_subtest_group
> >>> + run_tests_for_plane(&data, pipe);
> >>> }
> >>>
> >>> igt_fixture {
> >>>
> >
^ permalink raw reply
* Re: [PATCH 1/1] builtin/pull.c: use config value of autostash
From: Phillip Wood @ 2022-01-05 11:21 UTC (permalink / raw)
To: Junio C Hamano, John Cai; +Cc: git, Tilman Vogel, Philippe Blain
In-Reply-To: <xmqqbl0r9l0l.fsf@gitster.g>
Hi John and Junio
On 04/01/2022 22:46, Junio C Hamano wrote:
> John Cai <johncai86@gmail.com> writes:
>
>> diff --git a/builtin/pull.c b/builtin/pull.c
>> index 100cbf9fb8..fb700c2d39 100644
>> --- a/builtin/pull.c
>> +++ b/builtin/pull.c
>> @@ -86,7 +86,8 @@ static char *opt_ff;
>> static char *opt_verify_signatures;
>> static char *opt_verify;
>> static int opt_autostash = -1;
>> -static int config_autostash;
>> +static int rebase_autostash = 0;
>> +static int cfg_rebase_autostash;
>
> Do not explicitly initialize statics to '0' (or NULL for that matter).
>
> But more importantly, I have a feeling that you are making a piece
> of code that is already hard to read impossible to follow by adding
> yet another variable.
>
>> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
>> index 66cfcb09c5..28f551db8e 100755
>> --- a/t/t5521-pull-options.sh
>> +++ b/t/t5521-pull-options.sh
>> @@ -251,5 +251,56 @@ test_expect_success 'git pull --no-verify --verify passed to merge' '
>> test_commit -C src two &&
>> test_must_fail git -C dst pull --no-ff --no-verify --verify
>> '
>> +test_expect_success 'git pull --rebase --autostash succeeds on ff' '
>
> Missing blank line between tests.
>
>
> I thought that the root cause of the problem is that run_merge() is
> called even when rebase was asked for (either via pull.rebase=true
> configuration or "pull --rebase" option), when the other side is a
> descendant of HEAD. The basic idea behind that behaviour may be
> sound (i.e. if we do not have any of our own development on top of
> their history, rebase vs merge shouldn't make any difference while
> fast-forwarding HEAD to their history), except that rebase vs merge
> look at different configuration variables.
>
> I wonder if the following two-liner patch is a simpler fix that is
> easier to understand. run_merge() decides if we should pass the
> "--[no-]autostash" option based on the value of opt_autostash, and
> we know the value of rebase.autostash in config_autostash variable.
>
> It appears to pass all four tests your patch adds.
I think this is a better approach - it's simpler and it is clear that we
only use rebase.autostash when a rebase was requested.
Best Wishes
Phillip
> builtin/pull.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git c/builtin/pull.c w/builtin/pull.c
> index 100cbf9fb8..d459a91a64 100644
> --- c/builtin/pull.c
> +++ w/builtin/pull.c
> @@ -1133,7 +1133,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
> die(_("cannot rebase with locally recorded submodule modifications"));
>
> if (can_ff) {
> - /* we can fast-forward this without invoking rebase */
> + /*
> + * We can fast-forward without invoking
> + * rebase, by calling run_merge(). But we
> + * have to allow rebase.autostash=true to kick
> + * in.
> + */
> + if (opt_autostash < 0)
> + opt_autostash = config_autostash;
> opt_ff = "--ff-only";
> ret = run_merge();
> } else {
>
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 2/2] drm/i915/uncore: rename i915_reg_read_ioctl intel_uncore_reg_read_ioctl
From: Tvrtko Ursulin @ 2022-01-05 11:20 UTC (permalink / raw)
To: Jani Nikula, intel-gfx
In-Reply-To: <875yqytqv4.fsf@intel.com>
On 05/01/2022 10:32, Jani Nikula wrote:
> On Wed, 05 Jan 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> On 05/01/2022 10:05, Jani Nikula wrote:
>>> Follow the usual naming convention.
>>
>> But intel_uncore_ prefix usually means functions takes intel_uncore as
>> the first argument.
>>
>> Maybe solution here is that i915_reg_read_ioctl does not belong in
>> intel_uncore.c, it being the UAPI layer thing? I guess arguments could
>> be made for either way.
>
> My position is that the function and file prefixes go hand in
> hand. You'll always know where to place a function, and you'll always
> know where the function is to be found.
>
> If you can *also* make the context argument follow the pattern, it's
> obviously better, and indicates the division to files is working out
> nicely. However, in a lot of cases you'll need to pass struct
> drm_i915_private or similar as the first parameter to e.g. init
> functions. It can't be the rigid rule.
>
> I'm fine with moving the entire function somewhere else, as long as the
> declaration is not in i915_drv.h. There's no longer a i915_drv.c, and
> i915_drv.h should not have function declarations at all.
Yes I agree it cannot be a rigid rule. I just that it feels
intel_uncore.[hc] is too low level to me to hold an ioctl
implementation, and header actually feels wrong to have the declaration.
Not least it is about _one_ of the uncores, while the ioctl is not
operating on that level, albeit undefined at the moment how exactly it
would work for multi-tile.
Would it be too early, or unwarranted at this point, to maybe consider
adding i915_ioctls.[hc]?
I like the i915_ prefix of ioctls for consistency.. i915_getparam_ioctl,
i915_query_ioctl, i915_perf_..., i915_gem_....
Regards,
Tvrtko
>
> BR,
> Jani.
>
>>
>> Regards,
>>
>> Tvrtko
>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_driver.c | 2 +-
>>> drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
>>> drivers/gpu/drm/i915/intel_uncore.h | 4 ++--
>>> 3 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>>> index 95174938b160..f9a494e159dc 100644
>>> --- a/drivers/gpu/drm/i915/i915_driver.c
>>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>>> @@ -1805,7 +1805,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
>>> DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_RENDER_ALLOW),
>>> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE_EXT, i915_gem_context_create_ioctl, DRM_RENDER_ALLOW),
>>> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_RENDER_ALLOW),
>>> - DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_RENDER_ALLOW),
>>> + DRM_IOCTL_DEF_DRV(I915_REG_READ, intel_uncore_reg_read_ioctl, DRM_RENDER_ALLOW),
>>> DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_gem_context_reset_stats_ioctl, DRM_RENDER_ALLOW),
>>> DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_RENDER_ALLOW),
>>> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
>>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>>> index fc25ebf1a593..33f95bb2d3d5 100644
>>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>>> @@ -2269,8 +2269,8 @@ static const struct reg_whitelist {
>>> .size = 8
>>> } };
>>>
>>> -int i915_reg_read_ioctl(struct drm_device *dev,
>>> - void *data, struct drm_file *file)
>>> +int intel_uncore_reg_read_ioctl(struct drm_device *dev,
>>> + void *data, struct drm_file *file)
>>> {
>>> struct drm_i915_private *i915 = to_i915(dev);
>>> struct intel_uncore *uncore = &i915->uncore;
>>> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
>>> index 3a87bbd906f8..697ac4586159 100644
>>> --- a/drivers/gpu/drm/i915/intel_uncore.h
>>> +++ b/drivers/gpu/drm/i915/intel_uncore.h
>>> @@ -457,7 +457,7 @@ static inline int intel_uncore_write_and_verify(struct intel_uncore *uncore,
>>> #define raw_reg_write(base, reg, value) \
>>> writel(value, base + i915_mmio_reg_offset(reg))
>>>
>>> -int i915_reg_read_ioctl(struct drm_device *dev, void *data,
>>> - struct drm_file *file);
>>> +int intel_uncore_reg_read_ioctl(struct drm_device *dev, void *data,
>>> + struct drm_file *file);
>>>
>>> #endif /* !__INTEL_UNCORE_H__ */
>>>
>
^ permalink raw reply
* Re: [RFC 5/5] libvhost-user: handle removal of identical regions
From: Stefan Hajnoczi @ 2022-01-05 11:18 UTC (permalink / raw)
To: Raphael Norwitz
Cc: marcandre.lureau@redhat.com, david@redhat.com,
qemu-devel@nongnu.org, raphael.s.norwitz@gmail.com,
mst@redhat.com
In-Reply-To: <20211215222939.24738-6-raphael.norwitz@nutanix.com>
[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]
On Wed, Dec 15, 2021 at 10:29:55PM +0000, Raphael Norwitz wrote:
> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
> index 74a9980194..2f465a4f0e 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -809,6 +809,7 @@ static bool
> vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
> VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m;
> int i;
> + bool found = false;
>
> if (vmsg->fd_num != 1 ||
> vmsg->size != sizeof(vmsg->payload.memreg)) {
> @@ -831,25 +832,25 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
> VuDevRegion *r = &dev->regions[i];
> void *m = (void *) (uintptr_t) r->mmap_addr;
>
> - if (m) {
> + if (m && !found) {
> munmap(m, r->size + r->mmap_offset);
> }
Why is only the first region unmapped? My interpretation of
vu_add_mem_reg() is that it mmaps duplicate regions to unique mmap_addr
addresses, so we need to munmap each of them.
>
> - break;
> + /*
> + * Shift all affected entries by 1 to close the hole at index i and
> + * zero out the last entry.
> + */
> + memmove(dev->regions + i, dev->regions + i + 1,
> + sizeof(VuDevRegion) * (dev->nregions - i - 1));
> + memset(dev->regions + dev->nregions - 1, 0, sizeof(VuDevRegion));
> + DPRINT("Successfully removed a region\n");
> + dev->nregions--;
> +
> + found = true;
> }
i-- is missing. dev->regions[] has been shortened so we need to check
the same element again.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [syzbot] kernel BUG in pskb_expand_head
From: syzbot @ 2022-01-05 11:20 UTC (permalink / raw)
To: anthony.l.nguyen, changbin.du, christian.brauner, davem, edumazet,
eric.dumazet, hawk, hkallweit1, intel-wired-lan-owner,
intel-wired-lan, jesse.brandeburg, kuba, linux-can, linux-kernel,
mkl, netdev, socketcan, syzkaller-bugs, yajun.deng
In-Reply-To: <0000000000007ea16705d0cfbb53@google.com>
syzbot has found a reproducer for the following issue on:
HEAD commit: c9e6606c7fe9 Linux 5.16-rc8
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=148351c3b00000
kernel config: https://syzkaller.appspot.com/x/.config?x=32f9fa260d7413b4
dashboard link: https://syzkaller.appspot.com/bug?extid=4c63f36709a642f801c5
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15435e2bb00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12f4508db00000
The issue was bisected to:
commit e4b8954074f6d0db01c8c97d338a67f9389c042f
Author: Eric Dumazet <edumazet@google.com>
Date: Tue Dec 7 01:30:37 2021 +0000
netlink: add net device refcount tracker to struct ethnl_req_info
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=109e6fcbb00000
final oops: https://syzkaller.appspot.com/x/report.txt?x=129e6fcbb00000
console output: https://syzkaller.appspot.com/x/log.txt?x=149e6fcbb00000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+4c63f36709a642f801c5@syzkaller.appspotmail.com
Fixes: e4b8954074f6 ("netlink: add net device refcount tracker to struct ethnl_req_info")
skbuff: skb_over_panic: text:ffffffff88235fb8 len:4096 put:4096 head:ffff888021cb8400 data:ffff888021cb8400 tail:0x1000 end:0xc0 dev:<NULL>
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:113!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 19 Comm: ksoftirqd/1 Not tainted 5.16.0-rc8-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:skb_panic+0x16c/0x16e net/core/skbuff.c:113
Code: f8 4c 8b 4c 24 10 8b 4b 70 41 56 45 89 e8 4c 89 e2 41 57 48 89 ee 48 c7 c7 e0 4b ad 8a ff 74 24 10 ff 74 24 20 e8 6e 24 c2 ff <0f> 0b e8 74 92 38 f8 4c 8b 64 24 18 e8 da 47 7f f8 48 c7 c1 80 58
RSP: 0018:ffffc90000d979e0 EFLAGS: 00010286
RAX: 000000000000008b RBX: ffff888021ccb500 RCX: 0000000000000000
RDX: ffff88801196d700 RSI: ffffffff815f0948 RDI: fffff520001b2f2e
RBP: ffffffff8aad58c0 R08: 000000000000008b R09: 0000000000000000
R10: ffffffff815ea6ee R11: 0000000000000000 R12: ffffffff88235fb8
R13: 0000000000001000 R14: ffffffff8aad4ba0 R15: 00000000000000c0
FS: 0000000000000000(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f886c8cc718 CR3: 000000007ad6d000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
skb_over_panic net/core/skbuff.c:118 [inline]
skb_put.cold+0x24/0x24 net/core/skbuff.c:1990
isotp_rcv_cf net/can/isotp.c:570 [inline]
isotp_rcv+0xa38/0x1e30 net/can/isotp.c:668
deliver net/can/af_can.c:574 [inline]
can_rcv_filter+0x445/0x8d0 net/can/af_can.c:635
can_receive+0x31d/0x580 net/can/af_can.c:665
can_rcv+0x120/0x1c0 net/can/af_can.c:696
__netif_receive_skb_one_core+0x114/0x180 net/core/dev.c:5465
__netif_receive_skb+0x24/0x1b0 net/core/dev.c:5579
process_backlog+0x2a5/0x6c0 net/core/dev.c:6455
__napi_poll+0xaf/0x440 net/core/dev.c:7023
napi_poll net/core/dev.c:7090 [inline]
net_rx_action+0x801/0xb40 net/core/dev.c:7177
__do_softirq+0x29b/0x9c2 kernel/softirq.c:558
run_ksoftirqd kernel/softirq.c:921 [inline]
run_ksoftirqd+0x2d/0x60 kernel/softirq.c:913
smpboot_thread_fn+0x645/0x9c0 kernel/smpboot.c:164
kthread+0x405/0x4f0 kernel/kthread.c:327
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
</TASK>
Modules linked in:
---[ end trace 9f06028ec4daf4be ]---
RIP: 0010:skb_panic+0x16c/0x16e net/core/skbuff.c:113
Code: f8 4c 8b 4c 24 10 8b 4b 70 41 56 45 89 e8 4c 89 e2 41 57 48 89 ee 48 c7 c7 e0 4b ad 8a ff 74 24 10 ff 74 24 20 e8 6e 24 c2 ff <0f> 0b e8 74 92 38 f8 4c 8b 64 24 18 e8 da 47 7f f8 48 c7 c1 80 58
RSP: 0018:ffffc90000d979e0 EFLAGS: 00010286
RAX: 000000000000008b RBX: ffff888021ccb500 RCX: 0000000000000000
RDX: ffff88801196d700 RSI: ffffffff815f0948 RDI: fffff520001b2f2e
RBP: ffffffff8aad58c0 R08: 000000000000008b R09: 0000000000000000
R10: ffffffff815ea6ee R11: 0000000000000000 R12: ffffffff88235fb8
R13: 0000000000001000 R14: ffffffff8aad4ba0 R15: 00000000000000c0
FS: 0000000000000000(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f886c8cc718 CR3: 000000007ad6d000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
^ permalink raw reply
* [Intel-wired-lan] [syzbot] kernel BUG in pskb_expand_head
From: syzbot @ 2022-01-05 11:20 UTC (permalink / raw)
To: intel-wired-lan
In-Reply-To: <0000000000007ea16705d0cfbb53@google.com>
syzbot has found a reproducer for the following issue on:
HEAD commit: c9e6606c7fe9 Linux 5.16-rc8
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=148351c3b00000
kernel config: https://syzkaller.appspot.com/x/.config?x=32f9fa260d7413b4
dashboard link: https://syzkaller.appspot.com/bug?extid=4c63f36709a642f801c5
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15435e2bb00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12f4508db00000
The issue was bisected to:
commit e4b8954074f6d0db01c8c97d338a67f9389c042f
Author: Eric Dumazet <edumazet@google.com>
Date: Tue Dec 7 01:30:37 2021 +0000
netlink: add net device refcount tracker to struct ethnl_req_info
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=109e6fcbb00000
final oops: https://syzkaller.appspot.com/x/report.txt?x=129e6fcbb00000
console output: https://syzkaller.appspot.com/x/log.txt?x=149e6fcbb00000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+4c63f36709a642f801c5 at syzkaller.appspotmail.com
Fixes: e4b8954074f6 ("netlink: add net device refcount tracker to struct ethnl_req_info")
skbuff: skb_over_panic: text:ffffffff88235fb8 len:4096 put:4096 head:ffff888021cb8400 data:ffff888021cb8400 tail:0x1000 end:0xc0 dev:<NULL>
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:113!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 19 Comm: ksoftirqd/1 Not tainted 5.16.0-rc8-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:skb_panic+0x16c/0x16e net/core/skbuff.c:113
Code: f8 4c 8b 4c 24 10 8b 4b 70 41 56 45 89 e8 4c 89 e2 41 57 48 89 ee 48 c7 c7 e0 4b ad 8a ff 74 24 10 ff 74 24 20 e8 6e 24 c2 ff <0f> 0b e8 74 92 38 f8 4c 8b 64 24 18 e8 da 47 7f f8 48 c7 c1 80 58
RSP: 0018:ffffc90000d979e0 EFLAGS: 00010286
RAX: 000000000000008b RBX: ffff888021ccb500 RCX: 0000000000000000
RDX: ffff88801196d700 RSI: ffffffff815f0948 RDI: fffff520001b2f2e
RBP: ffffffff8aad58c0 R08: 000000000000008b R09: 0000000000000000
R10: ffffffff815ea6ee R11: 0000000000000000 R12: ffffffff88235fb8
R13: 0000000000001000 R14: ffffffff8aad4ba0 R15: 00000000000000c0
FS: 0000000000000000(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f886c8cc718 CR3: 000000007ad6d000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
skb_over_panic net/core/skbuff.c:118 [inline]
skb_put.cold+0x24/0x24 net/core/skbuff.c:1990
isotp_rcv_cf net/can/isotp.c:570 [inline]
isotp_rcv+0xa38/0x1e30 net/can/isotp.c:668
deliver net/can/af_can.c:574 [inline]
can_rcv_filter+0x445/0x8d0 net/can/af_can.c:635
can_receive+0x31d/0x580 net/can/af_can.c:665
can_rcv+0x120/0x1c0 net/can/af_can.c:696
__netif_receive_skb_one_core+0x114/0x180 net/core/dev.c:5465
__netif_receive_skb+0x24/0x1b0 net/core/dev.c:5579
process_backlog+0x2a5/0x6c0 net/core/dev.c:6455
__napi_poll+0xaf/0x440 net/core/dev.c:7023
napi_poll net/core/dev.c:7090 [inline]
net_rx_action+0x801/0xb40 net/core/dev.c:7177
__do_softirq+0x29b/0x9c2 kernel/softirq.c:558
run_ksoftirqd kernel/softirq.c:921 [inline]
run_ksoftirqd+0x2d/0x60 kernel/softirq.c:913
smpboot_thread_fn+0x645/0x9c0 kernel/smpboot.c:164
kthread+0x405/0x4f0 kernel/kthread.c:327
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
</TASK>
Modules linked in:
---[ end trace 9f06028ec4daf4be ]---
RIP: 0010:skb_panic+0x16c/0x16e net/core/skbuff.c:113
Code: f8 4c 8b 4c 24 10 8b 4b 70 41 56 45 89 e8 4c 89 e2 41 57 48 89 ee 48 c7 c7 e0 4b ad 8a ff 74 24 10 ff 74 24 20 e8 6e 24 c2 ff <0f> 0b e8 74 92 38 f8 4c 8b 64 24 18 e8 da 47 7f f8 48 c7 c1 80 58
RSP: 0018:ffffc90000d979e0 EFLAGS: 00010286
RAX: 000000000000008b RBX: ffff888021ccb500 RCX: 0000000000000000
RDX: ffff88801196d700 RSI: ffffffff815f0948 RDI: fffff520001b2f2e
RBP: ffffffff8aad58c0 R08: 000000000000008b R09: 0000000000000000
R10: ffffffff815ea6ee R11: 0000000000000000 R12: ffffffff88235fb8
R13: 0000000000001000 R14: ffffffff8aad4ba0 R15: 00000000000000c0
FS: 0000000000000000(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f886c8cc718 CR3: 000000007ad6d000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
^ permalink raw reply
* [igt-dev] ✓ Fi.CI.BAT: success for tests/i915/gem_ctx_exec: Skip test for RCS+CCS (rev2)
From: Patchwork @ 2022-01-05 11:20 UTC (permalink / raw)
To: priyanka.dandamudi; +Cc: igt-dev
In-Reply-To: <20220105101022.1604358-1-priyanka.dandamudi@intel.com>
[-- Attachment #1: Type: text/plain, Size: 2221 bytes --]
== Series Details ==
Series: tests/i915/gem_ctx_exec: Skip test for RCS+CCS (rev2)
URL : https://patchwork.freedesktop.org/series/98396/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11047 -> IGTPW_6538
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6538/index.html
Participating hosts (48 -> 36)
------------------------------
Missing (12): fi-ilk-m540 bat-dg1-6 bat-dg1-5 fi-hsw-4200u fi-bsw-cyan bat-adlp-6 bat-adlp-4 fi-ctg-p8600 bat-rpls-1 fi-bdw-samus bat-jsl-2 bat-jsl-1
Known issues
------------
Here are the changes found in IGTPW_6538 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_psr@primary_page_flip:
- fi-skl-6600u: [PASS][1] -> [FAIL][2] ([i915#4547])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11047/fi-skl-6600u/igt@kms_psr@primary_page_flip.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6538/fi-skl-6600u/igt@kms_psr@primary_page_flip.html
#### Warnings ####
* igt@runner@aborted:
- fi-skl-6600u: [FAIL][3] ([i915#1436] / [i915#2722] / [i915#4312]) -> [FAIL][4] ([i915#4312])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11047/fi-skl-6600u/igt@runner@aborted.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6538/fi-skl-6600u/igt@runner@aborted.html
[i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
[i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_6323 -> IGTPW_6538
CI-20190529: 20190529
CI_DRM_11047: f50e5d7abd1873b3488081da88dc8412584280b9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_6538: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6538/index.html
IGT_6323: 9dbaa0d5be7a859cda9b7d54c20ba96a596f43bd @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6538/index.html
[-- Attachment #2: Type: text/html, Size: 2891 bytes --]
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: remove typo from REALTEK OTTO WATCHDOG section
From: Wim Van Sebroeck @ 2022-01-05 9:42 UTC (permalink / raw)
To: Lukas Bulwahn
Cc: Sander Vanheule, Wim Van Sebroeck, Guenter Roeck, linux-watchdog,
kernel-janitors, linux-kernel
In-Reply-To: <20220104154414.21496-1-lukas.bulwahn@gmail.com>
Hi Lukas,
> Commit 489119bf75e6 ("watchdog: Add Realtek Otto watchdog timer") adds the
> REALTEK OTTO WATCHDOG section in MAINTAINERS and one file entry refers to
> driver/watchdog/realtek_otto_wdt.c. The actual top-level directory name is
> drivers, not driver, though.
>
> Hence, ./scripts/get_maintainer.pl --self-test=patterns complains:
>
> warning: no file matches F: driver/watchdog/realtek_otto_wdt.c
>
> Remove this obvious typo in the file entry.
>
> Fixes: 489119bf75e6 ("watchdog: Add Realtek Otto watchdog timer")
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> ---
> MAINTAINERS | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f7bf491409cf..b4fcc2bb7c54 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16307,7 +16307,7 @@ M: Sander Vanheule <sander@svanheule.net>
> L: linux-watchdog@vger.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/watchdog/realtek,otto-wdt.yaml
> -F: driver/watchdog/realtek_otto_wdt.c
> +F: drivers/watchdog/realtek_otto_wdt.c
>
> REALTEK RTL83xx SMI DSA ROUTER CHIPS
> M: Linus Walleij <linus.walleij@linaro.org>
> --
> 2.17.1
>
Since I had to do some other changes on the tree, I changed it directly in the original patch.
Kind regards,
Wim.
^ permalink raw reply
* Re: [RFC 06/10] vdpa-dev: implement the unrealize interface
From: Stefano Garzarella @ 2022-01-05 11:16 UTC (permalink / raw)
To: Longpeng(Mike)
Cc: mst, jasowang, cohuck, qemu-devel, yechuan, arei.gonglei,
huangzhichao, stefanha, pbonzini
In-Reply-To: <20220105005900.860-7-longpeng2@huawei.com>
On Wed, Jan 05, 2022 at 08:58:56AM +0800, Longpeng(Mike) wrote:
>From: Longpeng <longpeng2@huawei.com>
>
>Implements the .unrealize interface.
>
>Signed-off-by: Longpeng <longpeng2@huawei.com>
>---
> hw/virtio/vdpa-dev.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
>diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
>index 2d534d837a..4e4dd3d201 100644
>--- a/hw/virtio/vdpa-dev.c
>+++ b/hw/virtio/vdpa-dev.c
>@@ -133,9 +133,29 @@ out:
> close(fd);
> }
>
>+static void vhost_vdpa_vdev_unrealize(VhostVdpaDevice *s)
>+{
>+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
>+ int i;
>+
>+ for (i = 0; i < s->num_queues; i++) {
^
`s->num_queues` seems uninitialized to me, maybe we could just remove
the num_queues field from VhostVdpaDevice, and use `s->dev.nvqs` as in
vhost_vdpa_device_realize().
>+ virtio_delete_queue(s->virtqs[i]);
>+ }
>+ g_free(s->virtqs);
>+ virtio_cleanup(vdev);
>+
>+ g_free(s->config);
>+}
>+
> static void vhost_vdpa_device_unrealize(DeviceState *dev)
> {
>- return;
>+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev);
>+
>+ virtio_set_status(vdev, 0);
>+ vhost_dev_cleanup(&s->dev);
If we will use `s->dev.nvqs` in vhost_vdpa_vdev_unrealize(), we should
call vhost_dev_cleanup() after it, just before close() as we already do
in the error path of vhost_vdpa_device_realize().
>+ vhost_vdpa_vdev_unrealize(s);
>+ close(s->vdpa.device_fd);
> }
>
> static void
>--
>2.23.0
>
^ permalink raw reply
* Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states
From: Anshuman Khandual @ 2022-01-05 11:16 UTC (permalink / raw)
To: Suzuki K Poulose, linux-arm-kernel
Cc: Catalin Marinas, Will Deacon, Mathieu Poirier, coresight,
linux-doc, linux-kernel
In-Reply-To: <77d59823-b975-e3ba-3aa4-fac5c61bb69f@arm.com>
On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
> Hi Anshuman
>
> On 05/01/2022 05:05, Anshuman Khandual wrote:
>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>> an inconsistent view on whether trace is prohibited within the CPU. As a
>> result, the trace buffer or trace buffer state might be corrupted. This
>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>> followed by just a single context synchronization event before execution
>> changes from a context, in which trace is prohibited to one where it isn't,
>> or vice versa. In these mentioned conditions, the view of whether trace is
>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>> or the trace buffer state might be corrupted.
>>
>> Work around this problem in the TRBE driver by preventing an inconsistent
>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>> instruction before an ERET, or two ISB instructions if no ERET is to take
>> place. This adds a new cpu errata in arm64 errata framework and also
>> updates TRBE driver as required.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> Documentation/arm64/silicon-errata.rst | 2 +
>> arch/arm64/Kconfig | 23 ++++++++++
>> arch/arm64/kernel/cpu_errata.c | 9 ++++
>> arch/arm64/tools/cpucaps | 1 +
>> drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>> 5 files changed, 72 insertions(+), 10 deletions(-)
>
> As with the previous patch, it may be a good idea to split the
> patch to arm64 and trbe parts.
Sure, will do.
>
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -54,6 +54,8 @@ stable kernels.
>> +----------------+-----------------+-----------------+-----------------------------+
>> | ARM | Cortex-A510 | #2064142 | ARM64_ERRATUM_2064142 |
>> +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM | Cortex-A510 | #2038923 | ARM64_ERRATUM_2038923 |
>> ++----------------+-----------------+-----------------+-----------------------------+
>> | ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 |
>> +----------------+-----------------+-----------------+-----------------------------+
>> | ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 2105b68d88db..026e34fb6fad 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>> If unsure, say Y.
>> +config ARM64_ERRATUM_2038923
>> + bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>> + depends on CORESIGHT_TRBE
>> + default y
>> + help
>> + This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>> +
>> + Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>> + prohibited within the CPU. As a result, the trace buffer or trace buffer state
>> + might be corrupted. This happens after TRBE buffer has been enabled by setting
>> + TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>> + execution changes from a context, in which trace is prohibited to one where it
>> + isn't, or vice versa. In these mentioned conditions, the view of whether trace
>> + is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>> + the trace buffer state might be corrupted.
>> +
>> + Work around this in the driver by preventing an inconsistent view of whether the
>> + trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>> + change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>> + two ISB instructions if no ERET is to take place.
>> +
>> + If unsure, say Y.
>> +
>> config CAVIUM_ERRATUM_22375
>> bool "Cavium erratum 22375, 24313"
>> default y
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index cbb7d5a9aee7..60b0c1f1d912 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>> ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>> },
>> #endif
>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>> + {
>> + .desc = "ARM erratum 2038923",
>> + .capability = ARM64_WORKAROUND_2038923,
>> +
>> + /* Cortex-A510 r0p0 - r0p2 */
>> + ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>> + },
>> +#endif
>> {
>> }
>> };
>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>> index fca3cb329e1d..45a06d36d080 100644
>> --- a/arch/arm64/tools/cpucaps
>> +++ b/arch/arm64/tools/cpucaps
>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>> WORKAROUND_1508412
>> WORKAROUND_1542419
>> WORKAROUND_2064142
>> +WORKAROUND_2038923
>> WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>> WORKAROUND_TSB_FLUSH_FAILURE
>> WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index ec24b62b2cec..0689c6dab96d 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -92,11 +92,13 @@ struct trbe_buf {
>> #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE 0
>> #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE 1
>> #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE 2
>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE 3
>> static int trbe_errata_cpucaps[] = {
>> [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>> [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>> [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>> + [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>> -1, /* Sentinel, must be the last entry */
>> };
>> @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>> return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>> }
>> +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>> +{
>
> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?
trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.
>
>> + return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>> +}
>> +
>> static int trbe_alloc_node(struct perf_event *event)
>> {
>> if (event->cpu == -1)
>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>> dsb(nsh);
>> }
>> +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>> +{
>> + u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>
> minor nit: This implies we do the TRBE programming in the following
> manner in the common case (i.e, TRBE enabled in the beginning of a
> session).
> -> set TRBE LIMIT
> -> read TRBE LIMIT
> -> set TRBE ENABLED
>
> Could we please optimize this ? I believe the buf->trbe_limit
> must hold the LIMITR value at any point in time. And thus this
But is not bit risky though ! We have got the following places where
given trbe_limit instance changes its value.
drivers/../coresight-trbe.c: buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
drivers/../coresight-trbe.c: buf->trbe_limit = compute_trbe_buffer_limit(handle);
drivers/../coresight-trbe.c: buf->trbe_limit -= PAGE_SIZE;
> function could simply be :
>
> set_trbe_enabled(trbe_buf)
> {
> limitr = trbe_buf->limit | LIMITR_ENABLE
> write(limitr, TRBLIMITR_EL1);
> ...
> }
Is the potential for performance improvement here, out weigh possible
risks of using buf->trbe_limit directly while enabling the TRBE ?
>
> Otherwise looks good to me
>
> Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] mt76: mt7915: add support for passing chip/firmware debug data to user space
From: Felix Fietkau @ 2022-01-05 11:16 UTC (permalink / raw)
To: linux-wireless
This can be used to assist in debugging driver or firmware tx/rx issues.
The data is streamed to user space using a relay file in debugfs
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
.../wireless/mediatek/mt76/mt7915/debugfs.c | 142 +++++++++++++++++-
.../net/wireless/mediatek/mt76/mt7915/mac.c | 6 +
.../net/wireless/mediatek/mt76/mt7915/mac.h | 1 +
.../net/wireless/mediatek/mt76/mt7915/mcu.c | 7 +-
.../net/wireless/mediatek/mt76/mt7915/mmio.c | 2 +
.../wireless/mediatek/mt76/mt7915/mt7915.h | 6 +
.../net/wireless/mediatek/mt76/mt7915/regs.h | 2 +
7 files changed, 162 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c b/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c
index c59ef08a5306..dad25dfb946c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c
@@ -1,9 +1,13 @@
// SPDX-License-Identifier: ISC
/* Copyright (C) 2020 MediaTek Inc. */
+#include <linux/relay.h>
#include "mt7915.h"
#include "eeprom.h"
#include "mcu.h"
+#include "mac.h"
+
+#define FW_BIN_LOG_MAGIC 0x44e98caf
/** global debugfs **/
@@ -311,16 +315,31 @@ mt7915_fw_debug_wm_set(void *data, u64 val)
DEBUG_SPL,
DEBUG_RPT_RX,
} debug;
+ bool tx, rx, en;
int ret;
dev->fw_debug_wm = val ? MCU_FW_LOG_TO_HOST : 0;
- ret = mt7915_mcu_fw_log_2_host(dev, MCU_FW_LOG_WM, dev->fw_debug_wm);
+ if (dev->fw_debug_bin)
+ val = 16;
+ else
+ val = dev->fw_debug_wm;
+
+ tx = dev->fw_debug_wm || (dev->fw_debug_bin & BIT(1));
+ rx = dev->fw_debug_wm || (dev->fw_debug_bin & BIT(2));
+ en = dev->fw_debug_wm || (dev->fw_debug_bin & BIT(0));
+
+ ret = mt7915_mcu_fw_log_2_host(dev, MCU_FW_LOG_WM, val);
if (ret)
return ret;
for (debug = DEBUG_TXCMD; debug <= DEBUG_RPT_RX; debug++) {
- ret = mt7915_mcu_fw_dbg_ctrl(dev, debug, !!dev->fw_debug_wm);
+ if (debug == DEBUG_RPT_RX)
+ val = en && rx;
+ else
+ val = en && tx;
+
+ ret = mt7915_mcu_fw_dbg_ctrl(dev, debug, val);
if (ret)
return ret;
}
@@ -376,6 +395,65 @@ mt7915_fw_debug_wa_get(void *data, u64 *val)
DEFINE_DEBUGFS_ATTRIBUTE(fops_fw_debug_wa, mt7915_fw_debug_wa_get,
mt7915_fw_debug_wa_set, "%lld\n");
+static struct dentry *
+create_buf_file_cb(const char *filename, struct dentry *parent, umode_t mode,
+ struct rchan_buf *buf, int *is_global)
+{
+ struct dentry *f;
+
+ f = debugfs_create_file("fwlog_data", mode, parent, buf,
+ &relay_file_operations);
+ if (IS_ERR(f))
+ return NULL;
+
+ *is_global = 1;
+
+ return f;
+}
+
+static int
+remove_buf_file_cb(struct dentry *f)
+{
+ debugfs_remove(f);
+
+ return 0;
+}
+
+static int
+mt7915_fw_debug_bin_set(void *data, u64 val)
+{
+ static struct rchan_callbacks relay_cb = {
+ .create_buf_file = create_buf_file_cb,
+ .remove_buf_file = remove_buf_file_cb,
+ };
+ struct mt7915_dev *dev = data;
+
+ if (!dev->relay_fwlog)
+ dev->relay_fwlog = relay_open("fwlog_data", dev->debugfs_dir,
+ 1500, 512, &relay_cb, NULL);
+ if (!dev->relay_fwlog)
+ return -ENOMEM;
+
+ dev->fw_debug_bin = val;
+
+ relay_reset(dev->relay_fwlog);
+
+ return mt7915_fw_debug_wm_set(dev, dev->fw_debug_wm);
+}
+
+static int
+mt7915_fw_debug_bin_get(void *data, u64 *val)
+{
+ struct mt7915_dev *dev = data;
+
+ *val = dev->fw_debug_bin;
+
+ return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_fw_debug_bin, mt7915_fw_debug_bin_get,
+ mt7915_fw_debug_bin_set, "%lld\n");
+
static int
mt7915_fw_util_wm_show(struct seq_file *file, void *data)
{
@@ -757,6 +835,7 @@ int mt7915_init_debugfs(struct mt7915_phy *phy)
debugfs_create_file("tx_stats", 0400, dir, phy, &mt7915_tx_stats_fops);
debugfs_create_file("fw_debug_wm", 0600, dir, dev, &fops_fw_debug_wm);
debugfs_create_file("fw_debug_wa", 0600, dir, dev, &fops_fw_debug_wa);
+ debugfs_create_file("fw_debug_bin", 0600, dir, dev, &fops_fw_debug_bin);
debugfs_create_file("fw_util_wm", 0400, dir, dev,
&mt7915_fw_util_wm_fops);
debugfs_create_file("fw_util_wa", 0400, dir, dev,
@@ -775,9 +854,68 @@ int mt7915_init_debugfs(struct mt7915_phy *phy)
&fops_radar_trigger);
}
+ if (!ext_phy)
+ dev->debugfs_dir = dir;
+
return 0;
}
+static void
+mt7915_debugfs_write_fwlog(struct mt7915_dev *dev, const void *hdr, int hdrlen,
+ const void *data, int len)
+{
+ static DEFINE_SPINLOCK(lock);
+ unsigned long flags;
+ void *dest;
+
+ spin_lock_irqsave(&lock, flags);
+ dest = relay_reserve(dev->relay_fwlog, hdrlen + len + 4);
+ if (dest) {
+ *(u32 *)dest = hdrlen + len;
+ dest += 4;
+
+ if (hdrlen) {
+ memcpy(dest, hdr, hdrlen);
+ dest += hdrlen;
+ }
+
+ memcpy(dest, data, len);
+ relay_flush(dev->relay_fwlog);
+ }
+ spin_unlock_irqrestore(&lock, flags);
+}
+
+void mt7915_debugfs_rx_ics(struct mt7915_dev *dev, const void *data, int len)
+{
+ struct {
+ __le32 magic;
+ __le32 timestamp;
+ __le16 msg_type;
+ __le16 len;
+ } hdr = {
+ .magic = cpu_to_le32(FW_BIN_LOG_MAGIC),
+ .msg_type = PKT_TYPE_RX_ICS,
+ };
+
+ if (!dev->relay_fwlog)
+ return;
+
+ hdr.timestamp = mt76_rr(dev, MT_LPON_FRCR(0));
+ hdr.len = *(__le16 *)data;
+ mt7915_debugfs_write_fwlog(dev, &hdr, sizeof(hdr), data, len);
+}
+
+bool mt7915_debugfs_rx_log(struct mt7915_dev *dev, const void *data, int len)
+{
+ if (get_unaligned_le32(data) != FW_BIN_LOG_MAGIC)
+ return false;
+
+ if (dev->relay_fwlog)
+ mt7915_debugfs_write_fwlog(dev, NULL, 0, data, len);
+
+ return true;
+}
+
#ifdef CONFIG_MAC80211_DEBUGFS
/** per-station debugfs **/
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
index e0200f84a2f9..a6770fd40b34 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
@@ -1691,6 +1691,9 @@ bool mt7915_rx_check(struct mt76_dev *mdev, void *data, int len)
for (rxd += 2; rxd + 8 <= end; rxd += 8)
mt7915_mac_add_txs(dev, rxd);
return false;
+ case PKT_TYPE_RX_ICS:
+ mt7915_debugfs_rx_ics(dev, data, len);
+ return false;
default:
return true;
}
@@ -1722,6 +1725,9 @@ void mt7915_queue_rx_skb(struct mt76_dev *mdev, enum mt76_rxq_id q,
mt7915_mac_add_txs(dev, rxd);
dev_kfree_skb(skb);
break;
+ case PKT_TYPE_RX_ICS:
+ mt7915_debugfs_rx_ics(dev, skb->data, skb->len);
+ break;
case PKT_TYPE_NORMAL:
if (!mt7915_mac_fill_rx(dev, skb)) {
mt76_rx(&dev->mt76, q, skb);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.h b/drivers/net/wireless/mediatek/mt76/mt7915/mac.h
index d79f0a56535f..7fef2f8fad65 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.h
@@ -23,6 +23,7 @@ enum rx_pkt_type {
PKT_TYPE_RETRIEVE,
PKT_TYPE_TXRX_NOTIFY,
PKT_TYPE_RX_EVENT,
+ PKT_TYPE_RX_ICS = 0x0c,
};
/* RXD DW1 */
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
index 74cdfd3d13b9..66f8daf3168c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
@@ -368,9 +368,13 @@ mt7915_mcu_rx_log_message(struct mt7915_dev *dev, struct sk_buff *skb)
struct mt7915_mcu_rxd *rxd = (struct mt7915_mcu_rxd *)skb->data;
const char *data = (char *)&rxd[1];
const char *type;
+ int len = skb->len - sizeof(*rxd);
switch (rxd->s2d_index) {
case 0:
+ if (mt7915_debugfs_rx_log(dev, data, len))
+ return;
+
type = "WM";
break;
case 2:
@@ -381,8 +385,7 @@ mt7915_mcu_rx_log_message(struct mt7915_dev *dev, struct sk_buff *skb)
break;
}
- wiphy_info(mt76_hw(dev)->wiphy, "%s: %.*s", type,
- (int)(skb->len - sizeof(*rxd)), data);
+ wiphy_info(mt76_hw(dev)->wiphy, "%s: %.*s", type, len, data);
}
static void
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c b/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
index f1568f9059d8..e8ff686bd3f3 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
@@ -50,6 +50,7 @@ static const u32 mt7915_offs[] = {
[AGG_ATCR3] = 0x0f4,
[LPON_UTTR0] = 0x080,
[LPON_UTTR1] = 0x084,
+ [LPON_FRCR] = 0x314,
[MIB_SDR3] = 0x014,
[MIB_SDR4] = 0x018,
[MIB_SDR5] = 0x01c,
@@ -121,6 +122,7 @@ static const u32 mt7916_offs[] = {
[AGG_ATCR3] = 0x080,
[LPON_UTTR0] = 0x360,
[LPON_UTTR1] = 0x364,
+ [LPON_FRCR] = 0x37c,
[MIB_SDR3] = 0x698,
[MIB_SDR4] = 0x788,
[MIB_SDR5] = 0x780,
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h b/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h
index 5adde022d4e2..fff3065a3453 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h
@@ -291,6 +291,10 @@ struct mt7915_dev {
bool ibf;
u8 fw_debug_wm;
u8 fw_debug_wa;
+ u8 fw_debug_bin;
+
+ struct dentry *debugfs_dir;
+ struct rchan *relay_fwlog;
void *cal;
@@ -537,6 +541,8 @@ void mt7915_update_channel(struct mt76_phy *mphy);
int mt7915_mcu_muru_debug_set(struct mt7915_dev *dev, bool enable);
int mt7915_mcu_muru_debug_get(struct mt7915_phy *phy, void *ms);
int mt7915_init_debugfs(struct mt7915_phy *phy);
+void mt7915_debugfs_rx_ics(struct mt7915_dev *dev, const void *data, int len);
+bool mt7915_debugfs_rx_log(struct mt7915_dev *dev, const void *data, int len);
#ifdef CONFIG_MAC80211_DEBUGFS
void mt7915_sta_add_debugfs(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
struct ieee80211_sta *sta, struct dentry *dir);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/regs.h b/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
index aa19e5940070..6a0f68180396 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
@@ -50,6 +50,7 @@ enum offs_rev {
AGG_ATCR3,
LPON_UTTR0,
LPON_UTTR1,
+ LPON_FRCR,
MIB_SDR3,
MIB_SDR4,
MIB_SDR5,
@@ -238,6 +239,7 @@ enum offs_rev {
#define MT_LPON_UTTR0(_band) MT_WF_LPON(_band, __OFFS(LPON_UTTR0))
#define MT_LPON_UTTR1(_band) MT_WF_LPON(_band, __OFFS(LPON_UTTR1))
+#define MT_LPON_FRCR(_band) MT_WF_LPON(_band, __OFFS(LPON_FRCR))
#define MT_LPON_TCR(_band, n) MT_WF_LPON(_band, 0x0a8 + \
(((n) * 4) << 1))
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states
From: Anshuman Khandual @ 2022-01-05 11:16 UTC (permalink / raw)
To: Suzuki K Poulose, linux-arm-kernel
Cc: Catalin Marinas, Will Deacon, Mathieu Poirier, coresight,
linux-doc, linux-kernel
In-Reply-To: <77d59823-b975-e3ba-3aa4-fac5c61bb69f@arm.com>
On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
> Hi Anshuman
>
> On 05/01/2022 05:05, Anshuman Khandual wrote:
>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>> an inconsistent view on whether trace is prohibited within the CPU. As a
>> result, the trace buffer or trace buffer state might be corrupted. This
>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>> followed by just a single context synchronization event before execution
>> changes from a context, in which trace is prohibited to one where it isn't,
>> or vice versa. In these mentioned conditions, the view of whether trace is
>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>> or the trace buffer state might be corrupted.
>>
>> Work around this problem in the TRBE driver by preventing an inconsistent
>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>> instruction before an ERET, or two ISB instructions if no ERET is to take
>> place. This adds a new cpu errata in arm64 errata framework and also
>> updates TRBE driver as required.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> Documentation/arm64/silicon-errata.rst | 2 +
>> arch/arm64/Kconfig | 23 ++++++++++
>> arch/arm64/kernel/cpu_errata.c | 9 ++++
>> arch/arm64/tools/cpucaps | 1 +
>> drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>> 5 files changed, 72 insertions(+), 10 deletions(-)
>
> As with the previous patch, it may be a good idea to split the
> patch to arm64 and trbe parts.
Sure, will do.
>
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -54,6 +54,8 @@ stable kernels.
>> +----------------+-----------------+-----------------+-----------------------------+
>> | ARM | Cortex-A510 | #2064142 | ARM64_ERRATUM_2064142 |
>> +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM | Cortex-A510 | #2038923 | ARM64_ERRATUM_2038923 |
>> ++----------------+-----------------+-----------------+-----------------------------+
>> | ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 |
>> +----------------+-----------------+-----------------+-----------------------------+
>> | ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 2105b68d88db..026e34fb6fad 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>> If unsure, say Y.
>> +config ARM64_ERRATUM_2038923
>> + bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>> + depends on CORESIGHT_TRBE
>> + default y
>> + help
>> + This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>> +
>> + Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>> + prohibited within the CPU. As a result, the trace buffer or trace buffer state
>> + might be corrupted. This happens after TRBE buffer has been enabled by setting
>> + TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>> + execution changes from a context, in which trace is prohibited to one where it
>> + isn't, or vice versa. In these mentioned conditions, the view of whether trace
>> + is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>> + the trace buffer state might be corrupted.
>> +
>> + Work around this in the driver by preventing an inconsistent view of whether the
>> + trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>> + change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>> + two ISB instructions if no ERET is to take place.
>> +
>> + If unsure, say Y.
>> +
>> config CAVIUM_ERRATUM_22375
>> bool "Cavium erratum 22375, 24313"
>> default y
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index cbb7d5a9aee7..60b0c1f1d912 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>> ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>> },
>> #endif
>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>> + {
>> + .desc = "ARM erratum 2038923",
>> + .capability = ARM64_WORKAROUND_2038923,
>> +
>> + /* Cortex-A510 r0p0 - r0p2 */
>> + ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>> + },
>> +#endif
>> {
>> }
>> };
>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>> index fca3cb329e1d..45a06d36d080 100644
>> --- a/arch/arm64/tools/cpucaps
>> +++ b/arch/arm64/tools/cpucaps
>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>> WORKAROUND_1508412
>> WORKAROUND_1542419
>> WORKAROUND_2064142
>> +WORKAROUND_2038923
>> WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>> WORKAROUND_TSB_FLUSH_FAILURE
>> WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index ec24b62b2cec..0689c6dab96d 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -92,11 +92,13 @@ struct trbe_buf {
>> #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE 0
>> #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE 1
>> #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE 2
>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE 3
>> static int trbe_errata_cpucaps[] = {
>> [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>> [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>> [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>> + [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>> -1, /* Sentinel, must be the last entry */
>> };
>> @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>> return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>> }
>> +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>> +{
>
> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?
trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.
>
>> + return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>> +}
>> +
>> static int trbe_alloc_node(struct perf_event *event)
>> {
>> if (event->cpu == -1)
>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>> dsb(nsh);
>> }
>> +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>> +{
>> + u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>
> minor nit: This implies we do the TRBE programming in the following
> manner in the common case (i.e, TRBE enabled in the beginning of a
> session).
> -> set TRBE LIMIT
> -> read TRBE LIMIT
> -> set TRBE ENABLED
>
> Could we please optimize this ? I believe the buf->trbe_limit
> must hold the LIMITR value at any point in time. And thus this
But is not bit risky though ! We have got the following places where
given trbe_limit instance changes its value.
drivers/../coresight-trbe.c: buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
drivers/../coresight-trbe.c: buf->trbe_limit = compute_trbe_buffer_limit(handle);
drivers/../coresight-trbe.c: buf->trbe_limit -= PAGE_SIZE;
> function could simply be :
>
> set_trbe_enabled(trbe_buf)
> {
> limitr = trbe_buf->limit | LIMITR_ENABLE
> write(limitr, TRBLIMITR_EL1);
> ...
> }
Is the potential for performance improvement here, out weigh possible
risks of using buf->trbe_limit directly while enabling the TRBE ?
>
> Otherwise looks good to me
>
> Suzuki
^ permalink raw reply
* [PATCH] ide: Explicitly poll for BHs on cancel
From: Hanna Reitz @ 2022-01-05 11:13 UTC (permalink / raw)
To: qemu-block; +Cc: Hanna Reitz, John Snow, qemu-devel
When we still have an AIOCB registered for DMA operations, we try to
settle the respective operation by draining the BlockBackend associated
with the IDE device.
However, this assumes that every DMA operation is associated with some
I/O operation on the BlockBackend, and so settling the latter will
settle the former. That is not the case; for example, the guest is free
to issue a zero-length TRIM operation that will not result in any I/O
operation forwarded to the BlockBackend. In such a case, blk_drain()
will be a no-op if no other operations are in flight.
It is clear that if blk_drain() is a no-op, the value of
s->bus->dma->aiocb will not change between checking it in the `if`
condition and asserting that it is NULL after blk_drain().
To settle the DMA operation, we will thus need to explicitly invoke
aio_poll() ourselves, which will run any outstanding BHs (like
ide_trim_bh_cb()), until s->bus->dma->aiocb is NULL. To stop this from
being an infinite loop, assert that we made progress with every
aio_poll() call (i.e., invoked some BH).
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2029980
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
Perhaps for a lack of being aware of all the kinds of tests we have, I
found it impossible to write a reproducer in any of our current test
frameworks: From how I understand the issue, to reproduce it, you need
to issue a TRIM request and immediately cancel it, before
ide_trim_bh_cb() (scheduled as a BH) can run.
I wanted to do this via qtest, but that does not work, because every
port I/O operation is done via a qtest command, and QEMU will happily
poll the main context between each qtest command, which means that you
cannot cancel an ongoing IDE request before a BH scheduled by it is run.
Therefore, I wrote an x86 boot sector that sets up a no-op TRIM request
(i.e. one where all TRIM ranges have length 0) and immediately cancels
it by setting SRST. It is attached to the BZ linked above, and can be
tested as follows:
$ TEST_BIN=test.bin
$ (sleep 1; echo 'info registers'; echo quit) \
| ./qemu-system-x86_64 \
-drive if=ide,file=$TEST_BIN,format=raw \
-monitor stdio \
| grep EIP= \
| sed -e 's/ EFL.*//'
The result should be:
EIP=00007c72
Not:
qemu-system-x86_64: ../hw/ide/core.c:734: ide_cancel_dma_sync: Assertion
`s->bus->dma->aiocb == NULL' failed.
---
hw/ide/core.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index e28f8aad61..c7f7a1016c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -731,7 +731,17 @@ void ide_cancel_dma_sync(IDEState *s)
if (s->bus->dma->aiocb) {
trace_ide_cancel_dma_sync_remaining();
blk_drain(s->blk);
- assert(s->bus->dma->aiocb == NULL);
+
+ /*
+ * Wait for potentially still-scheduled BHs, like ide_trim_bh_cb()
+ * (blk_drain() will only poll if there are in-flight requests on the
+ * BlockBackend, which there may not necessarily be, e.g. when the
+ * guest has issued a zero-length TRIM request)
+ */
+ while (s->bus->dma->aiocb) {
+ bool progress = aio_poll(qemu_get_aio_context(), true);
+ assert(progress);
+ }
}
}
--
2.33.1
^ permalink raw reply related
* Re: [PATCH v4 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
From: Phillip Wood @ 2022-01-05 11:15 UTC (permalink / raw)
To: John Cai via GitGitGadget, git
Cc: Johannes Schindelin, John Cai, Junio C Hamano
In-Reply-To: <4e9200922a4c2c91e69e3b497fbf4c8702046a27.1641307776.git.gitgitgadget@gmail.com>
Hi John
On 04/01/2022 14:49, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
> [...]
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index 27f60153a6c..21370afdaf9 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)
> int cmd_name_rev(int argc, const char **argv, const char *prefix)
> {
> struct object_array revs = OBJECT_ARRAY_INIT;
> - int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
> + int all = 0, annotate_stdin = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
> struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
> struct option opts[] = {
> OPT_BOOL(0, "name-only", &data.name_only, N_("print only ref-based names (no object names)")),
> @@ -539,6 +539,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
> OPT_GROUP(""),
> OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
> OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),
If the intention is to deprecate this option then it might be worth
marking it as PARSE_OPT_HIDDEN so that it is not shown by 'git name-rev
-h'. (You need to change OPT_BOOL to OPT_BOOL_F to pass the flag)
> + OPT_BOOL(0, "annotate-stdin", &annotate_stdin, N_("annotate text from stdin")),
> OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
> OPT_BOOL(0, "always", &always,
> N_("show abbreviated commit object as fallback")),
> @@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
> init_commit_rev_name(&rev_names);
> git_config(git_default_config, NULL);
> argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
> - if (all + transform_stdin + !!argc > 1) {
> +
> + if (transform_stdin) {
> + warning("--stdin is deprecated. Please use --annotate-stdin instead, "
> + "which is functionally equivalent.\n"
> + "This option will be removed in a future release.");
> + annotate_stdin = 1;
> + }
> +
> + if (all + annotate_stdin + !!argc > 1) {
> error("Specify either a list, or --all, not both!");
> usage_with_options(name_rev_usage, opts);
> }
> - if (all || transform_stdin)
> + if (all || annotate_stdin)
> cutoff = 0;
>
> for (; argc; argc--, argv++) {
> @@ -613,8 +622,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
> for_each_ref(name_ref, &data);
> name_tips();
>
> - if (transform_stdin) {
> - char buffer[2048];
> + if (annotate_stdin) {
> + struct strbuf sb = STRBUF_INIT;
I think this hunk belongs in the next patch. Before posting a patch
series I find it helpful to run
git rebase --keep-base -x'make -j4 git && cd t && prove -j4 <tests
I think might fail> :: --root=/dev/shm'
to check that the individual patches compile and pass the relevant
tests. I've never got round to trying it but git-test[1] also lets you
test all the commits in a series
Best Wishes
Phillip
[1] https://github.com/mhagger/git-test
> while (!feof(stdin)) {
> char *p = fgets(buffer, sizeof(buffer), stdin);
> diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
> index 19c6f4acbf6..1e9f7833dd6 100755
> --- a/t/t3412-rebase-root.sh
> +++ b/t/t3412-rebase-root.sh
> @@ -11,7 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> log_with_names () {
> git rev-list --topo-order --parents --pretty="tformat:%s" HEAD |
> - git name-rev --stdin --name-only --refs=refs/heads/$1
> + git name-rev --annotate-stdin --name-only --refs=refs/heads/$1
> }
>
>
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 50495598619..dc884107de4 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -659,7 +659,7 @@ EOF
>
> test_expect_success 'log --graph with full output' '
> git log --graph --date-order --pretty=short |
> - git name-rev --name-only --stdin |
> + git name-rev --name-only --annotate-stdin |
> sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual &&
> test_cmp expect actual
> '
> diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh
> index aebe4b69e13..6f3e5439771 100755
> --- a/t/t6007-rev-list-cherry-pick-file.sh
> +++ b/t/t6007-rev-list-cherry-pick-file.sh
> @@ -58,7 +58,7 @@ EOF
>
> test_expect_success '--left-right' '
> git rev-list --left-right B...C > actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" \
> < actual > actual.named &&
> test_cmp expect actual.named
> '
> @@ -78,14 +78,14 @@ EOF
>
> test_expect_success '--cherry-pick bar does not come up empty' '
> git rev-list --left-right --cherry-pick B...C -- bar > actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" \
> < actual > actual.named &&
> test_cmp expect actual.named
> '
>
> test_expect_success 'bar does not come up empty' '
> git rev-list --left-right B...C -- bar > actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" \
> < actual > actual.named &&
> test_cmp expect actual.named
> '
> @@ -97,14 +97,14 @@ EOF
>
> test_expect_success '--cherry-pick bar does not come up empty (II)' '
> git rev-list --left-right --cherry-pick F...E -- bar > actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" \
> < actual > actual.named &&
> test_cmp expect actual.named
> '
>
> test_expect_success 'name-rev multiple --refs combine inclusive' '
> git rev-list --left-right --cherry-pick F...E -- bar >actual &&
> - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \
> <actual >actual.named &&
> test_cmp expect actual.named
> '
> @@ -116,7 +116,7 @@ EOF
> test_expect_success 'name-rev --refs excludes non-matched patterns' '
> git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect &&
> git rev-list --left-right --cherry-pick F...E -- bar >actual &&
> - git name-rev --stdin --name-only --refs="*tags/F" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/F" \
> <actual >actual.named &&
> test_cmp expect actual.named
> '
> @@ -128,14 +128,14 @@ EOF
> test_expect_success 'name-rev --exclude excludes matched patterns' '
> git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect &&
> git rev-list --left-right --cherry-pick F...E -- bar >actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \
> <actual >actual.named &&
> test_cmp expect actual.named
> '
>
> test_expect_success 'name-rev --no-refs clears the refs list' '
> git rev-list --left-right --cherry-pick F...E -- bar >expect &&
> - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \
> <expect >actual &&
> test_cmp expect actual
> '
> @@ -149,7 +149,7 @@ EOF
>
> test_expect_success '--cherry-mark' '
> git rev-list --cherry-mark F...E -- bar > actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" \
> < actual > actual.named &&
> test_cmp expect actual.named
> '
> @@ -163,7 +163,7 @@ EOF
>
> test_expect_success '--cherry-mark --left-right' '
> git rev-list --cherry-mark --left-right F...E -- bar > actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" \
> < actual > actual.named &&
> test_cmp expect actual.named
> '
> @@ -174,14 +174,14 @@ EOF
>
> test_expect_success '--cherry-pick --right-only' '
> git rev-list --cherry-pick --right-only F...E -- bar > actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" \
> < actual > actual.named &&
> test_cmp expect actual.named
> '
>
> test_expect_success '--cherry-pick --left-only' '
> git rev-list --cherry-pick --left-only E...F -- bar > actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" \
> < actual > actual.named &&
> test_cmp expect actual.named
> '
> @@ -193,7 +193,7 @@ EOF
>
> test_expect_success '--cherry' '
> git rev-list --cherry F...E -- bar > actual &&
> - git name-rev --stdin --name-only --refs="*tags/*" \
> + git name-rev --annotate-stdin --name-only --refs="*tags/*" \
> < actual > actual.named &&
> test_cmp expect actual.named
> '
> diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
> index 4f7fa8b6c03..4fedc614fa6 100755
> --- a/t/t6012-rev-list-simplify.sh
> +++ b/t/t6012-rev-list-simplify.sh
> @@ -12,7 +12,7 @@ note () {
> }
>
> unnote () {
> - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g"
> + git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g"
> }
>
> #
> diff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh
> index e07b6070e0e..90ff1416400 100755
> --- a/t/t6111-rev-list-treesame.sh
> +++ b/t/t6111-rev-list-treesame.sh
> @@ -23,7 +23,8 @@ note () {
> }
>
> unnote () {
> - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g"
> + git name-rev --tags --annotate-stdin | \
> + sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g"
> }
>
> test_expect_success setup '
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index d8af2bb9d2b..9781b92aedd 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -270,7 +270,7 @@ test_expect_success 'name-rev --all' '
> test_cmp expect actual
> '
>
> -test_expect_success 'name-rev --stdin' '
> +test_expect_success 'name-rev --annotate-stdin' '
> >expect.unsorted &&
> for rev in $(git rev-list --all)
> do
> @@ -278,11 +278,16 @@ test_expect_success 'name-rev --stdin' '
> echo "$rev ($name)" >>expect.unsorted || return 1
> done &&
> sort <expect.unsorted >expect &&
> - git rev-list --all | git name-rev --stdin >actual.unsorted &&
> + git rev-list --all | git name-rev --annotate-stdin >actual.unsorted &&
> sort <actual.unsorted >actual &&
> test_cmp expect actual
> '
>
> +test_expect_success 'name-rev --stdin deprecated' "
> + git rev-list --all | git name-rev --stdin 2>actual &&
> + grep -E 'warning: --stdin is deprecated' actual
> +"
> +
> test_expect_success 'describe --contains with the exact tags' '
> echo "A^0" >expect &&
> tag_object=$(git rev-parse refs/tags/A) &&
>
^ permalink raw reply
* Re: [PATCH v6 0/9] Multigenerational LRU Framework
From: Borislav Petkov @ 2022-01-05 11:12 UTC (permalink / raw)
To: Yu Zhao
Cc: SeongJae Park, Andrew Morton, Linus Torvalds, Andi Kleen,
Catalin Marinas, Dave Hansen, Hillf Danton, Jens Axboe,
Jesse Barnes, Johannes Weiner, Jonathan Corbet, Matthew Wilcox,
Mel Gorman, Michael Larabel, Michal Hocko, Rik van Riel,
Vlastimil Babka, Will Deacon, Ying Huang, linux-arm-kernel,
linux-doc, linux-kernel, linux-mm, page-reclaim, x86
In-Reply-To: <YdV4k1+zEbtzmUkK@google.com>
On Wed, Jan 05, 2022 at 03:53:07AM -0700, Yu Zhao wrote:
> Look, I'm open to your suggestion. I probably should have been nicer.
> So I'm sorry. I just don't appreciate alternative facts.
Yes, you should've been *much* nicer. I'm reading lkml for pretty much
20 years now and you just made my eyebrows go up - something which
pretty much never happens these days.
So you need to check yourself before replying. Looking at git history,
you're not a newbie so you've probably picked up - at least from the
sidelines - all those code of conduct discussions. And I'm not going to
point you to it - I'm sure you can find it yourself and peruse it at
your own convenience.
Long story short: we all try to be civil to each other now, even if it
is hard sometimes.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.