[PATCH v2 0/2] efi: arm: add support for earlycon on EFI framebuffer
Repurpose the existing EFI earlyprintk code to implement support for 'earlycon=efi' for arm64 systems, allowing the graphical console to be used instead of the serial port for early debug output. Changes since v1: - Rename earlycon= argument to 'efifb' to emphasize that this is specific to the EFI framebuffer. - Replace earlyprintk=efi entirely, rather than keep it alongside earlycon. Since earlycon is typically enabled (along with the infrastructure), and earlyprintk isn't, this does not result in a lot more code to be included, but does make it more likely that a given [distro] kernel has support for this enabled out of the box. - Switch to write-combine mappings by default. This is the default for efifb, and is actually required on arm64, since device attributes do not tolerate unaligned accesses or other operations (such as DC ZVA) that rely on memory semantics. This requires ARCH_USE_MEMREMAP_PROT to be wired up, which is why a new patch #1 has been added. - Since adding the 'ram' parameter for framebuffers in shared memory on cache coherent devices is trivial after the switch to WC mappings, fold the change into the main patch. Cc: cor...@lwn.net Cc: leif.lindh...@linaro.org Cc: graeme.greg...@linaro.org Cc: mi...@redhat.com Cc: t...@linutronix.de Cc: linux-doc@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: ag...@suse.de Cc: pjo...@redhat.com Ard Biesheuvel (2): x86: make ARCH_USE_MEMREMAP_PROT a generic Kconfig symbol efi: x86: convert x86 EFI earlyprintk into generic earlycon implementation Documentation/admin-guide/kernel-parameters.txt | 8 +- arch/Kconfig| 3 + arch/x86/Kconfig| 5 +- arch/x86/Kconfig.debug | 10 - arch/x86/include/asm/efi.h | 1 - arch/x86/kernel/early_printk.c | 4 - arch/x86/platform/efi/Makefile | 1 - arch/x86/platform/efi/early_printk.c| 240 drivers/firmware/efi/Kconfig| 6 + drivers/firmware/efi/Makefile | 1 + drivers/firmware/efi/earlycon.c | 208 + 11 files changed, 226 insertions(+), 261 deletions(-) delete mode 100644 arch/x86/platform/efi/early_printk.c create mode 100644 drivers/firmware/efi/earlycon.c -- 2.20.1
[PATCH v2 1/2] x86: make ARCH_USE_MEMREMAP_PROT a generic Kconfig symbol
Turn ARCH_USE_MEMREMAP_PROT into a generic Kconfig symbol, and fix the dependency expression to reflect that AMD_MEM_ENCRYPT depends on it, instead of the other way around. This will permit ARCH_USE_MEMREMAP_PROT to be selected by other architectures. Signed-off-by: Ard Biesheuvel --- arch/Kconfig | 3 +++ arch/x86/Kconfig | 5 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 4cfb6de48f79..9f0213213da8 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -885,6 +885,9 @@ config HAVE_ARCH_PREL32_RELOCATIONS architectures, and don't require runtime relocation on relocatable kernels. +config ARCH_USE_MEMREMAP_PROT + bool + source "kernel/gcov/Kconfig" source "scripts/gcc-plugins/Kconfig" diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 26387c7bf305..c0e9a77dc089 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1510,6 +1510,7 @@ config AMD_MEM_ENCRYPT bool "AMD Secure Memory Encryption (SME) support" depends on X86_64 && CPU_SUP_AMD select DYNAMIC_PHYSICAL_MASK + select ARCH_USE_MEMREMAP_PROT ---help--- Say yes to enable support for the encryption of system memory. This requires an AMD processor that supports Secure Memory @@ -1529,10 +1530,6 @@ config AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT If set to N, then the encryption of system memory can be activated with the mem_encrypt=on command line option. -config ARCH_USE_MEMREMAP_PROT - def_bool y - depends on AMD_MEM_ENCRYPT - # Common NUMA Features config NUMA bool "Numa Memory Allocation and Scheduler Support" -- 2.20.1
[PATCH v2 2/2] efi: x86: convert x86 EFI earlyprintk into generic earlycon implementation
Move the x86 EFI earlyprintk implementation to a shared location under drivers/firmware and tweak it slightly so we can expose it as an earlycon implementation (which is generic) rather than earlyprintk (which is only implemented for a few architectures) This also involves switching to write-combine mappings by default (which is required on ARM since device mappings lack memory semantics, and so memcpy/memset may not be used on them), and adding support for shared memory framebuffers on cache coherent non-x86 systems (which do not tolerate mismatched attributes) Note that 32-bit ARM does not populate its struct screen_info early enough for earlycon=efifb to work, so it is disabled there. Signed-off-by: Ard Biesheuvel --- Documentation/admin-guide/kernel-parameters.txt | 8 +- arch/x86/Kconfig.debug | 10 - arch/x86/include/asm/efi.h | 1 - arch/x86/kernel/early_printk.c | 4 - arch/x86/platform/efi/Makefile | 1 - arch/x86/platform/efi/early_printk.c| 240 drivers/firmware/efi/Kconfig| 6 + drivers/firmware/efi/Makefile | 1 + drivers/firmware/efi/earlycon.c | 208 + 9 files changed, 222 insertions(+), 257 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index b799bcf67d7b..76dd3baa31e0 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1073,9 +1073,15 @@ specified address. The serial port must already be setup and configured. Options are not yet supported. + efifb,[options] + Start an early, unaccelerated console on the EFI + memory mapped framebuffer (if available). On cache + coherent non-x86 systems that use system memory for + the framebuffer, pass the 'ram' option so that it is + mapped with the correct attributes. + earlyprintk=[X86,SH,ARM,M68k,S390] earlyprintk=vga - earlyprintk=efi earlyprintk=sclp earlyprintk=xen earlyprintk=serial[,ttySn[,baudrate]] diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug index 0723dff17e6c..15d0fbe27872 100644 --- a/arch/x86/Kconfig.debug +++ b/arch/x86/Kconfig.debug @@ -40,16 +40,6 @@ config EARLY_PRINTK_DBGP with klogd/syslogd or the X server. You should normally say N here, unless you want to debug such a crash. You need usb debug device. -config EARLY_PRINTK_EFI - bool "Early printk via the EFI framebuffer" - depends on EFI && EARLY_PRINTK - select FONT_SUPPORT - ---help--- - Write kernel log output directly into the EFI framebuffer. - - This is useful for kernel debugging when your machine crashes very - early before the console code is initialized. - config EARLY_PRINTK_USB_XDBC bool "Early printk via the xHCI debug port" depends on EARLY_PRINTK && PCI diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h index 107283b1eb1e..606a4b6a9812 100644 --- a/arch/x86/include/asm/efi.h +++ b/arch/x86/include/asm/efi.h @@ -170,7 +170,6 @@ static inline bool efi_runtime_supported(void) return false; } -extern struct console early_efi_console; extern void parse_efi_setup(u64 phys_addr, u32 data_len); extern void efifb_setup_from_dmi(struct screen_info *si, const char *opt); diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c index 374a52fa5296..9b33904251a9 100644 --- a/arch/x86/kernel/early_printk.c +++ b/arch/x86/kernel/early_printk.c @@ -388,10 +388,6 @@ static int __init setup_early_printk(char *buf) if (!strncmp(buf, "xen", 3)) early_console_register(&xenboot_console, keep); #endif -#ifdef CONFIG_EARLY_PRINTK_EFI - if (!strncmp(buf, "efi", 3)) - early_console_register(&early_efi_console, keep); -#endif #ifdef CONFIG_EARLY_PRINTK_USB_XDBC if (!strncmp(buf, "xdbc", 4)) early_xdbc_parse_parameter(buf + 4); diff --git a/arch/x86/platform/efi/Makefile b/arch/x86/platform/efi/Makefile index e4dc3862d423..fe29f3f5d384 100644 --- a/arch/x86/platform/efi/Makefile +++ b/arch/x86/platform/efi/Makefile @@ -3,5 +3,4 @@ OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y obj-$(CONFIG_EFI) += quirks.o efi.o efi_$(BITS).o efi_stub_$(BITS).o -obj-$(CONFIG_EARLY_PRINTK_EFI) += early_printk.o obj-$(CONFIG_EFI_MIXED)+= efi_thunk_$(BITS).o diff --git a/arch/x86/platform/efi/early_printk.c b/arch/x86/pla
Re: [PATCH v3 5/5] psi: introduce psi monitor
On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote:
> static void psi_update_work(struct work_struct *work)
> {
> struct delayed_work *dwork;
> struct psi_group *group;
> + bool first_pass = true;
> + u64 next_update;
> + u32 change_mask;
> + int polling;
> bool nonidle;
> + u64 now;
>
> dwork = to_delayed_work(work);
> group = container_of(dwork, struct psi_group, clock_work);
>
> + now = sched_clock();
> +
> + mutex_lock(&group->update_lock);
actually acquiring a mutex can take a fairly long while; would it not
make more sense to take the @now timestanp _after_ it, instead of
before?
[PATCH] vim2m: fix driver for it to handle handle different fourcc formats
Despite vim2m is reporting that it supports RGB565BE and YUYV,
that's not true.
Right now, it just says that it supports both format, but it
doesn't actually support them.
Also, horizontal flip is not properly implemented. It sounds
that it was designed to do a pseudo-horizontal flip using 8
tiles. Yet, as it doesn't do format conversion, the result
is a mess.
I suspect that it was done this way in order to save CPU time,
at the time of OMAP2 days.
That's messy and doesn't really help if someone wants to
use vim2m to test a pipeline.
Worse than that, the unique RGB format it says it supports is
RGB565BE, with is not supported by Gstreamer. That prevents
practical usage of it, even for tests.
So, instead, properly implement fourcc format conversions,
adding a few more RGB formats:
- RGB and BGR with 24 bits
- RGB565LE (known as RGB16 at gstreamer)
Also allows using any of the 5 supported formats as either
capture or output.
Note: The YUYV conversion routines are based on the conversion code
written by Hans de Goede inside libv4lconvert (part of v4l-utils),
released under LGPGL 2.1 (GPL 2.0 compatible).
Tested all possible format combinations except for RGB565BE,
as Gstreamer currently doesn't support it.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/platform/vim2m.c | 380 +
1 file changed, 240 insertions(+), 140 deletions(-)
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index a7a152fb3075..ccd0576c766e 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -60,8 +60,6 @@ MODULE_PARM_DESC(debug, "activates debug info");
/* Default transaction time in msec */
#define MEM2MEM_DEF_TRANSTIME 40
-#define MEM2MEM_COLOR_STEP (0xff >> 4)
-#define MEM2MEM_NUM_TILES 8
/* Flags that indicate processing mode */
#define MEM2MEM_HFLIP (1 << 0)
@@ -82,22 +80,24 @@ static struct platform_device vim2m_pdev = {
struct vim2m_fmt {
u32 fourcc;
int depth;
- /* Types the format can be used for */
- u32 types;
};
static struct vim2m_fmt formats[] = {
{
- .fourcc = V4L2_PIX_FMT_RGB565X, /* rggg gggb */
+ .fourcc = V4L2_PIX_FMT_RGB565, /* rggg gggb */
.depth = 16,
- /* Both capture and output format */
- .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
- },
- {
+ }, {
+ .fourcc = V4L2_PIX_FMT_RGB565X, /* gggb rggg */
+ .depth = 16,
+ }, {
+ .fourcc = V4L2_PIX_FMT_RGB24,
+ .depth = 24,
+ }, {
+ .fourcc = V4L2_PIX_FMT_BGR24,
+ .depth = 24,
+ }, {
.fourcc = V4L2_PIX_FMT_YUYV,
.depth = 16,
- /* Output-only format */
- .types = MEM2MEM_OUTPUT,
},
};
@@ -201,23 +201,222 @@ static struct vim2m_q_data *get_q_data(struct vim2m_ctx
*ctx,
return NULL;
}
+#define CLIP(color) \
+ (u8)(((color) > 0xFF) ? 0xff : (((color) < 0) ? 0 : (color)))
+
+static void copy_two_pixels(struct vim2m_fmt *in, struct vim2m_fmt *out,
+ u8 **src, u8 **dst, bool reverse)
+{
+ u8 _r[2], _g[2], _b[2], *r, *g, *b;
+ int i, step;
+
+ // If format is the same just copy the data, respecting the width
+ if (in->fourcc == out->fourcc) {
+ int depth = out->depth >> 3;
+
+ if (reverse) {
+ if (in->fourcc == V4L2_PIX_FMT_YUYV) {
+ int u, v, y, y1;
+
+ *src -= 2;
+
+ y1 = (*src)[0]; /* copy as second point */
+ u = (*src)[1];
+ y = (*src)[2]; /* copy as first point */
+ v = (*src)[3];
+
+ *src -= 2;
+
+ *(*dst)++ = y;
+ *(*dst)++ = u;
+ *(*dst)++ = y1;
+ *(*dst)++ = v;
+ return;
+ }
+
+ memcpy(*dst, *src, depth);
+ memcpy(*dst + depth, *src - depth, depth);
+ *src -= depth << 1;
+ } else {
+ memcpy(*dst, *src, depth << 1);
+ *src += depth << 1;
+ }
+ *dst += depth << 1;
+ return;
+ }
+
+ /* Step 1: read two consecutive pixels from src pointer */
+
+ r = _r;
+ g = _g;
+ b = _b;
+
+ if (reverse)
+ step = -1;
+ else
+ step = 1;
+
+ switch (in->fourcc) {
+ case V4L2_PIX_FMT_RGB565: /* rggg gggb */
+ for (i = 0; i < 2; i++) {
+
Re: [PATCH v3 5/5] psi: introduce psi monitor
On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote:
> + atomic_set(&group->polling, polling);
> + /*
> + * Memory barrier is needed to order group->polling
> + * write before times[] read in collect_percpu_times()
> + */
> + smp_mb__after_atomic();
That's broken, smp_mb__{before,after}_atomic() can only be used on
atomic RmW operations, something atomic_set() is _not_.
Re: [PATCH v2 2/2] efi: x86: convert x86 EFI earlyprintk into generic earlycon implementation
On 01/29/2019 10:21 AM, Ard Biesheuvel wrote:
Move the x86 EFI earlyprintk implementation to a shared location under
drivers/firmware and tweak it slightly so we can expose it as an earlycon
implementation (which is generic) rather than earlyprintk (which is only
implemented for a few architectures)
This also involves switching to write-combine mappings by default (which
is required on ARM since device mappings lack memory semantics, and so
memcpy/memset may not be used on them), and adding support for shared
memory framebuffers on cache coherent non-x86 systems (which do not
tolerate mismatched attributes)
Note that 32-bit ARM does not populate its struct screen_info early
enough for earlycon=efifb to work, so it is disabled there.
Signed-off-by: Ard Biesheuvel
---
Documentation/admin-guide/kernel-parameters.txt | 8 +-
arch/x86/Kconfig.debug | 10 -
arch/x86/include/asm/efi.h | 1 -
arch/x86/kernel/early_printk.c | 4 -
arch/x86/platform/efi/Makefile | 1 -
arch/x86/platform/efi/early_printk.c| 240
drivers/firmware/efi/Kconfig| 6 +
drivers/firmware/efi/Makefile | 1 +
drivers/firmware/efi/earlycon.c | 208 +
9 files changed, 222 insertions(+), 257 deletions(-)
[...]
+static int __init efi_earlycon_setup(struct earlycon_device *device,
+const char *opt)
+{
+ struct screen_info *si;
+ u16 xres, yres;
+ u32 i;
+
+ if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
+ return -ENODEV;
+
+ fb_base = screen_info.lfb_base;
+ if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+ fb_base |= (u64)screen_info.ext_lfb_base << 32;
+
+ if (opt && !strcmp(opt, "ram"))
+ fb_prot = PAGE_KERNEL;
+ else
+ fb_prot = pgprot_writecombine(PAGE_KERNEL);
Can you determine the default from the UEFI memory map?
Also, doesn't the current logic map it as WC on x86 too? Is that
intentional?
Alex
+
+ si = &screen_info;
+ xres = si->lfb_width;
+ yres = si->lfb_height;
+
+ /*
+* efi_earlycon_write_char() implicitly assumes a framebuffer with
+* 32-bits per pixel.
+*/
+ if (si->lfb_depth != 32)
+ return -ENODEV;
+
+ font = get_default_font(xres, yres, -1, -1);
+ if (!font)
+ return -ENODEV;
+
+ efi_y = rounddown(yres, font->height) - font->height;
+ for (i = 0; i < (yres - efi_y) / font->height; i++)
+ efi_earlycon_scroll_up();
+
+ device->con->write = efi_earlycon_write;
+ return 0;
+}
+EARLYCON_DECLARE(efifb, efi_earlycon_setup);
Re: [PATCH v2 2/2] efi: x86: convert x86 EFI earlyprintk into generic earlycon implementation
Hi Alex,
On Tue, 29 Jan 2019 at 14:37, Alexander Graf wrote:
>
> On 01/29/2019 10:21 AM, Ard Biesheuvel wrote:
> > Move the x86 EFI earlyprintk implementation to a shared location under
> > drivers/firmware and tweak it slightly so we can expose it as an earlycon
> > implementation (which is generic) rather than earlyprintk (which is only
> > implemented for a few architectures)
> >
> > This also involves switching to write-combine mappings by default (which
> > is required on ARM since device mappings lack memory semantics, and so
> > memcpy/memset may not be used on them), and adding support for shared
> > memory framebuffers on cache coherent non-x86 systems (which do not
> > tolerate mismatched attributes)
> >
> > Note that 32-bit ARM does not populate its struct screen_info early
> > enough for earlycon=efifb to work, so it is disabled there.
> >
> > Signed-off-by: Ard Biesheuvel
> > ---
> > Documentation/admin-guide/kernel-parameters.txt | 8 +-
> > arch/x86/Kconfig.debug | 10 -
> > arch/x86/include/asm/efi.h | 1 -
> > arch/x86/kernel/early_printk.c | 4 -
> > arch/x86/platform/efi/Makefile | 1 -
> > arch/x86/platform/efi/early_printk.c| 240
> > drivers/firmware/efi/Kconfig| 6 +
> > drivers/firmware/efi/Makefile | 1 +
> > drivers/firmware/efi/earlycon.c | 208 +
> > 9 files changed, 222 insertions(+), 257 deletions(-)
> >
>
> [...]
>
> > +static int __init efi_earlycon_setup(struct earlycon_device *device,
> > + const char *opt)
> > +{
> > + struct screen_info *si;
> > + u16 xres, yres;
> > + u32 i;
> > +
> > + if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
> > + return -ENODEV;
> > +
> > + fb_base = screen_info.lfb_base;
> > + if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
> > + fb_base |= (u64)screen_info.ext_lfb_base << 32;
> > +
> > + if (opt && !strcmp(opt, "ram"))
> > + fb_prot = PAGE_KERNEL;
> > + else
> > + fb_prot = pgprot_writecombine(PAGE_KERNEL);
>
> Can you determine the default from the UEFI memory map?
>
No. This is being called way before we parse the system table and the
memory map. Given that this is debug code, duplicating a significant
chunk of that work here (and run the risk of crashing here due to
unexpected contents in those tables) is not a great idea imo.
> Also, doesn't the current logic map it as WC on x86 too? Is that
> intentional?
>
Yes. As mentioned in the cover letter, this aligns it with efifb which
also uses WC by default (although there, it can be overridden for
performance reasons, but due to the debug nature of earlycon, this
doesn't matter, since higher performance only makes it more difficult
to capture the log on your phone camera)
> > +
> > + si = &screen_info;
> > + xres = si->lfb_width;
> > + yres = si->lfb_height;
> > +
> > + /*
> > + * efi_earlycon_write_char() implicitly assumes a framebuffer with
> > + * 32-bits per pixel.
> > + */
> > + if (si->lfb_depth != 32)
> > + return -ENODEV;
> > +
> > + font = get_default_font(xres, yres, -1, -1);
> > + if (!font)
> > + return -ENODEV;
> > +
> > + efi_y = rounddown(yres, font->height) - font->height;
> > + for (i = 0; i < (yres - efi_y) / font->height; i++)
> > + efi_earlycon_scroll_up();
> > +
> > + device->con->write = efi_earlycon_write;
> > + return 0;
> > +}
> > +EARLYCON_DECLARE(efifb, efi_earlycon_setup);
>
>
Re: [PATCH v2 2/2] efi: x86: convert x86 EFI earlyprintk into generic earlycon implementation
On 01/29/2019 02:41 PM, Ard Biesheuvel wrote:
Hi Alex,
On Tue, 29 Jan 2019 at 14:37, Alexander Graf wrote:
On 01/29/2019 10:21 AM, Ard Biesheuvel wrote:
Move the x86 EFI earlyprintk implementation to a shared location under
drivers/firmware and tweak it slightly so we can expose it as an earlycon
implementation (which is generic) rather than earlyprintk (which is only
implemented for a few architectures)
This also involves switching to write-combine mappings by default (which
is required on ARM since device mappings lack memory semantics, and so
memcpy/memset may not be used on them), and adding support for shared
memory framebuffers on cache coherent non-x86 systems (which do not
tolerate mismatched attributes)
Note that 32-bit ARM does not populate its struct screen_info early
enough for earlycon=efifb to work, so it is disabled there.
Signed-off-by: Ard Biesheuvel
---
Documentation/admin-guide/kernel-parameters.txt | 8 +-
arch/x86/Kconfig.debug | 10 -
arch/x86/include/asm/efi.h | 1 -
arch/x86/kernel/early_printk.c | 4 -
arch/x86/platform/efi/Makefile | 1 -
arch/x86/platform/efi/early_printk.c| 240
drivers/firmware/efi/Kconfig| 6 +
drivers/firmware/efi/Makefile | 1 +
drivers/firmware/efi/earlycon.c | 208 +
9 files changed, 222 insertions(+), 257 deletions(-)
[...]
+static int __init efi_earlycon_setup(struct earlycon_device *device,
+ const char *opt)
+{
+ struct screen_info *si;
+ u16 xres, yres;
+ u32 i;
+
+ if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
+ return -ENODEV;
+
+ fb_base = screen_info.lfb_base;
+ if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+ fb_base |= (u64)screen_info.ext_lfb_base << 32;
+
+ if (opt && !strcmp(opt, "ram"))
+ fb_prot = PAGE_KERNEL;
+ else
+ fb_prot = pgprot_writecombine(PAGE_KERNEL);
Can you determine the default from the UEFI memory map?
No. This is being called way before we parse the system table and the
memory map. Given that this is debug code, duplicating a significant
chunk of that work here (and run the risk of crashing here due to
unexpected contents in those tables) is not a great idea imo.
I see. Maybe we will want to have something there, but I tend to agree
that for now we should keep bits as simple as possible.
Also, doesn't the current logic map it as WC on x86 too? Is that
intentional?
Yes. As mentioned in the cover letter, this aligns it with efifb which
also uses WC by default (although there, it can be overridden for
performance reasons, but due to the debug nature of earlycon, this
doesn't matter, since higher performance only makes it more difficult
to capture the log on your phone camera)
Well, the cover letter really only talks about arm :). But yeah, I think
it's probably a good idea to map it WC regardless.
Overall, I would've preferred to have a larger patch set with more, but
smaller changes that refactor the code. But it seems to be reviewable
enough still. Let's cross our fingers it doesn't break :).
Reviewed-by: Alexander Graf
Thanks,
Alex
[PATCH] doc: Change LXR references to elixir.bootlin.com
Recently, Free Electrons was renamed to Bootlin[1]. Less recently, the Linux Cross Reference (LXR) at lxr.free-electrons.com was replaced by Elixir[2], and lxr.free-electrons.com redirected first to elixir.free-electrons.com and now to elixir.bootlin.com. [1]: https://bootlin.com/blog/free-electrons-becomes-bootlin/ [2]: https://github.com/free-electrons/elixir Signed-off-by: Jonathan Neuschäfer --- Documentation/input/devices/xpad.rst | 2 +- Documentation/process/howto.rst| 2 +- Documentation/process/kernel-docs.rst | 2 +- Documentation/translations/it_IT/process/howto.rst | 2 +- Documentation/translations/ja_JP/howto.rst | 2 +- Documentation/translations/ko_KR/howto.rst | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/input/devices/xpad.rst b/Documentation/input/devices/xpad.rst index b8bd65962dd8..95d410b27a96 100644 --- a/Documentation/input/devices/xpad.rst +++ b/Documentation/input/devices/xpad.rst @@ -218,7 +218,7 @@ References .. [1] http://euc.jp/periphs/xbox-controller.ja.html (ITO Takayuki) .. [2] http://xpad.xbox-scene.com/ .. [3] http://www.markosweb.com/www/xboxhackz.com/ -.. [4] http://lxr.free-electrons.com/ident?i=xpad_device +.. [4] https://elixir.bootlin.com/ident?i=xpad_device Historic Edits diff --git a/Documentation/process/howto.rst b/Documentation/process/howto.rst index 58b2f46c4f98..bdd2eda81a1c 100644 --- a/Documentation/process/howto.rst +++ b/Documentation/process/howto.rst @@ -225,7 +225,7 @@ Cross-Reference project, which is able to present source code in a self-referential, indexed webpage format. An excellent up-to-date repository of the kernel code may be found at: - http://lxr.free-electrons.com/ + https://elixir.bootlin.com/ The development process diff --git a/Documentation/process/kernel-docs.rst b/Documentation/process/kernel-docs.rst index 3fb28de556e4..ab12dddc773e 100644 --- a/Documentation/process/kernel-docs.rst +++ b/Documentation/process/kernel-docs.rst @@ -565,7 +565,7 @@ Miscellaneous * Name: **Cross-Referencing Linux** - :URL: http://lxr.free-electrons.com/ + :URL: https://elixir.bootlin.com/ :Keywords: Browsing source code. :Description: Another web-based Linux kernel source code browser. Lots of cross references to variables and functions. You can see diff --git a/Documentation/translations/it_IT/process/howto.rst b/Documentation/translations/it_IT/process/howto.rst index 909e6a55bc43..b323c4b309a5 100644 --- a/Documentation/translations/it_IT/process/howto.rst +++ b/Documentation/translations/it_IT/process/howto.rst @@ -234,7 +234,7 @@ il progetto Linux Cross-Reference, che è in grado di presentare codice sorgente in un formato autoreferenziale ed indicizzato. Un eccellente ed aggiornata fonte di consultazione del codice del kernel la potete trovare qui: - http://lxr.free-electrons.com/ + https://elixir.bootlin.com Il processo di sviluppo diff --git a/Documentation/translations/ja_JP/howto.rst b/Documentation/translations/ja_JP/howto.rst index f3116381c26b..2ca9389d5915 100644 --- a/Documentation/translations/ja_JP/howto.rst +++ b/Documentation/translations/ja_JP/howto.rst @@ -245,7 +245,7 @@ Linux カーネルソースツリーの中に含まれる、きれいにし、 できます。この最新の素晴しいカーネルコードのリポジトリは以下で見つかり ます - - http://lxr.free-electrons.com/ + https://elixir.bootlin.com/ 開発プロセス diff --git a/Documentation/translations/ko_KR/howto.rst b/Documentation/translations/ko_KR/howto.rst index a8197e072599..9fc869087e9c 100644 --- a/Documentation/translations/ko_KR/howto.rst +++ b/Documentation/translations/ko_KR/howto.rst @@ -235,7 +235,7 @@ ReST 마크업을 사용하는 문서들은 Documentation/output 에 생성된 소스코드를 인덱스된 웹 페이지들의 형태로 보여준다. 최신의 멋진 커널 코드 저장소는 다음을 통하여 참조할 수 있다. - http://lxr.free-electrons.com/ + https://elixir.bootlin.com/ 개발 프로세스 -- 2.20.1
Re: [PATCH v2 2/2] efi: x86: convert x86 EFI earlyprintk into generic earlycon implementation
On Tue, 29 Jan 2019 at 15:04, Alexander Graf wrote:
>
> On 01/29/2019 02:41 PM, Ard Biesheuvel wrote:
> > Hi Alex,
> >
> > On Tue, 29 Jan 2019 at 14:37, Alexander Graf wrote:
> >> On 01/29/2019 10:21 AM, Ard Biesheuvel wrote:
> >>> Move the x86 EFI earlyprintk implementation to a shared location under
> >>> drivers/firmware and tweak it slightly so we can expose it as an earlycon
> >>> implementation (which is generic) rather than earlyprintk (which is only
> >>> implemented for a few architectures)
> >>>
> >>> This also involves switching to write-combine mappings by default (which
> >>> is required on ARM since device mappings lack memory semantics, and so
> >>> memcpy/memset may not be used on them), and adding support for shared
> >>> memory framebuffers on cache coherent non-x86 systems (which do not
> >>> tolerate mismatched attributes)
> >>>
> >>> Note that 32-bit ARM does not populate its struct screen_info early
> >>> enough for earlycon=efifb to work, so it is disabled there.
> >>>
> >>> Signed-off-by: Ard Biesheuvel
> >>> ---
> >>>Documentation/admin-guide/kernel-parameters.txt | 8 +-
> >>>arch/x86/Kconfig.debug | 10 -
> >>>arch/x86/include/asm/efi.h | 1 -
> >>>arch/x86/kernel/early_printk.c | 4 -
> >>>arch/x86/platform/efi/Makefile | 1 -
> >>>arch/x86/platform/efi/early_printk.c| 240
> >>>
> >>>drivers/firmware/efi/Kconfig| 6 +
> >>>drivers/firmware/efi/Makefile | 1 +
> >>>drivers/firmware/efi/earlycon.c | 208 +
> >>>9 files changed, 222 insertions(+), 257 deletions(-)
> >>>
> >> [...]
> >>
> >>> +static int __init efi_earlycon_setup(struct earlycon_device *device,
> >>> + const char *opt)
> >>> +{
> >>> + struct screen_info *si;
> >>> + u16 xres, yres;
> >>> + u32 i;
> >>> +
> >>> + if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
> >>> + return -ENODEV;
> >>> +
> >>> + fb_base = screen_info.lfb_base;
> >>> + if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
> >>> + fb_base |= (u64)screen_info.ext_lfb_base << 32;
> >>> +
> >>> + if (opt && !strcmp(opt, "ram"))
> >>> + fb_prot = PAGE_KERNEL;
> >>> + else
> >>> + fb_prot = pgprot_writecombine(PAGE_KERNEL);
> >> Can you determine the default from the UEFI memory map?
> >>
> > No. This is being called way before we parse the system table and the
> > memory map. Given that this is debug code, duplicating a significant
> > chunk of that work here (and run the risk of crashing here due to
> > unexpected contents in those tables) is not a great idea imo.
>
> I see. Maybe we will want to have something there, but I tend to agree
> that for now we should keep bits as simple as possible.
>
> >
> >> Also, doesn't the current logic map it as WC on x86 too? Is that
> >> intentional?
> >>
> > Yes. As mentioned in the cover letter, this aligns it with efifb which
> > also uses WC by default (although there, it can be overridden for
> > performance reasons, but due to the debug nature of earlycon, this
> > doesn't matter, since higher performance only makes it more difficult
> > to capture the log on your phone camera)
>
> Well, the cover letter really only talks about arm :). But yeah, I think
> it's probably a good idea to map it WC regardless.
>
Fair enough.
> Overall, I would've preferred to have a larger patch set with more, but
> smaller changes that refactor the code. But it seems to be reviewable
> enough still. Let's cross our fingers it doesn't break :).
>
>
> Reviewed-by: Alexander Graf
>
Thanks
AW: [PATCH] doc: Change LXR references to elixir.bootlin.com
Von: linux-input-ow...@vger.kernel.org [linux-input-ow...@vger.kernel.org] im Auftrag von Jonathan Neuschäfer [j.neuschae...@gmx.net] Gesendet: Dienstag, 29. Jänner 2019 15:29 An: linux-doc@vger.kernel.org Cc: Jonathan Neuschäfer; Dmitry Torokhov; Jonathan Corbet; Federico Vaga; Markus Heiser; Minghui Liu; Michael Rodin; Grigory Shipunov; Alessia Mantegazza; Stephen Boyd; Mauro Carvalho Chehab; Takashi Iwai; Matthias Brugger; Guenter Roeck; Jeff Kirsher; linux-in...@vger.kernel.org; linux-ker...@vger.kernel.org Betreff: [PATCH] doc: Change LXR references to elixir.bootlin.com Recently, Free Electrons was renamed to Bootlin[1]. Less recently, the Linux Cross Reference (LXR) at lxr.free-electrons.com was replaced by Elixir[2], and lxr.free-electrons.com redirected first to elixir.free-electrons.com and now to elixir.bootlin.com. [1]: https://bootlin.com/blog/free-electrons-becomes-bootlin/ [2]: https://github.com/free-electrons/elixir Signed-off-by: Jonathan Neuschäfer --- Documentation/input/devices/xpad.rst | 2 +- Documentation/process/howto.rst| 2 +- Documentation/process/kernel-docs.rst | 2 +- Documentation/translations/it_IT/process/howto.rst | 2 +- Documentation/translations/ja_JP/howto.rst | 2 +- Documentation/translations/ko_KR/howto.rst | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/input/devices/xpad.rst b/Documentation/input/devices/xpad.rst index b8bd65962dd8..95d410b27a96 100644 --- a/Documentation/input/devices/xpad.rst +++ b/Documentation/input/devices/xpad.rst @@ -218,7 +218,7 @@ References .. [1] http://euc.jp/periphs/xbox-controller.ja.html (ITO Takayuki) .. [2] http://xpad.xbox-scene.com/ .. [3] http://www.markosweb.com/www/xboxhackz.com/ -.. [4] http://lxr.free-electrons.com/ident?i=xpad_device +.. [4] https://elixir.bootlin.com/ident?i=xpad_device that seems to be wrong. I think it's https://elixir.bootlin.com/linux/latest/ident/xpad_device other than that, good. thanks, martin smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v3 5/5] psi: introduce psi monitor
On Tue, Jan 29, 2019 at 01:38:43PM +0100, Peter Zijlstra wrote:
> On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote:
> > + atomic_set(&group->polling, polling);
> > + /*
> > +* Memory barrier is needed to order group->polling
> > +* write before times[] read in collect_percpu_times()
> > +*/
> > + smp_mb__after_atomic();
>
> That's broken, smp_mb__{before,after}_atomic() can only be used on
> atomic RmW operations, something atomic_set() is _not_.
Also; the comment should explain _why_ not only what.
Re: [PATCH] doc: Change LXR references to elixir.bootlin.com
On Tue, Jan 29, 2019 at 03:01:18PM +, Kepplinger Martin wrote: [...] > -.. [4] http://lxr.free-electrons.com/ident?i=xpad_device > +.. [4] https://elixir.bootlin.com/ident?i=xpad_device > > that seems to be wrong. I think it's > https://elixir.bootlin.com/linux/latest/ident/xpad_device Oops. Indeed, it doesn't work with the old syntax. I'll fix this in v2. > other than that, good. Thanks, Jonathan Neuschäfer signature.asc Description: PGP signature
[PATCH 2/3] media: vim2m: use per-file handler work queue
It doesn't make sense to have a per-device work queue, as the
scheduler should be called per file handler. Having a single
one causes failures if multiple streams are filtered by vim2m.
So, move it to be inside the context structure.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/platform/vim2m.c | 38 +-
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index ccd0576c766e..a9e43070567e 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -146,9 +146,6 @@ struct vim2m_dev {
atomic_tnum_inst;
struct mutexdev_mutex;
- spinlock_t irqlock;
-
- struct delayed_work work_run;
struct v4l2_m2m_dev *m2m_dev;
};
@@ -167,6 +164,10 @@ struct vim2m_ctx {
/* Transaction time (i.e. simulated processing time) in milliseconds */
u32 transtime;
+ struct mutexvb_mutex;
+ struct delayed_work work_run;
+ spinlock_t irqlock;
+
/* Abort requested by m2m */
int aborting;
@@ -490,7 +491,6 @@ static void job_abort(void *priv)
static void device_run(void *priv)
{
struct vim2m_ctx *ctx = priv;
- struct vim2m_dev *dev = ctx->dev;
struct vb2_v4l2_buffer *src_buf, *dst_buf;
src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
@@ -507,18 +507,18 @@ static void device_run(void *priv)
&ctx->hdl);
/* Run delayed work, which simulates a hardware irq */
- schedule_delayed_work(&dev->work_run, msecs_to_jiffies(ctx->transtime));
+ schedule_delayed_work(&ctx->work_run, msecs_to_jiffies(ctx->transtime));
}
static void device_work(struct work_struct *w)
{
- struct vim2m_dev *vim2m_dev =
- container_of(w, struct vim2m_dev, work_run.work);
struct vim2m_ctx *curr_ctx;
+ struct vim2m_dev *vim2m_dev;
struct vb2_v4l2_buffer *src_vb, *dst_vb;
unsigned long flags;
- curr_ctx = v4l2_m2m_get_curr_priv(vim2m_dev->m2m_dev);
+ curr_ctx = container_of(w, struct vim2m_ctx, work_run.work);
+ vim2m_dev = curr_ctx->dev;
if (NULL == curr_ctx) {
pr_err("Instance released before the end of transaction\n");
@@ -530,10 +530,10 @@ static void device_work(struct work_struct *w)
curr_ctx->num_processed++;
- spin_lock_irqsave(&vim2m_dev->irqlock, flags);
+ spin_lock_irqsave(&curr_ctx->irqlock, flags);
v4l2_m2m_buf_done(src_vb, VB2_BUF_STATE_DONE);
v4l2_m2m_buf_done(dst_vb, VB2_BUF_STATE_DONE);
- spin_unlock_irqrestore(&vim2m_dev->irqlock, flags);
+ spin_unlock_irqrestore(&curr_ctx->irqlock, flags);
if (curr_ctx->num_processed == curr_ctx->translen
|| curr_ctx->aborting) {
@@ -893,11 +893,10 @@ static int vim2m_start_streaming(struct vb2_queue *q,
unsigned count)
static void vim2m_stop_streaming(struct vb2_queue *q)
{
struct vim2m_ctx *ctx = vb2_get_drv_priv(q);
- struct vim2m_dev *dev = ctx->dev;
struct vb2_v4l2_buffer *vbuf;
unsigned long flags;
- cancel_delayed_work_sync(&dev->work_run);
+ cancel_delayed_work_sync(&ctx->work_run);
for (;;) {
if (V4L2_TYPE_IS_OUTPUT(q->type))
vbuf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
@@ -907,9 +906,9 @@ static void vim2m_stop_streaming(struct vb2_queue *q)
return;
v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req,
&ctx->hdl);
- spin_lock_irqsave(&ctx->dev->irqlock, flags);
+ spin_lock_irqsave(&ctx->irqlock, flags);
v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
- spin_unlock_irqrestore(&ctx->dev->irqlock, flags);
+ spin_unlock_irqrestore(&ctx->irqlock, flags);
}
}
@@ -943,7 +942,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
struct vb2_queue *ds
src_vq->ops = &vim2m_qops;
src_vq->mem_ops = &vb2_vmalloc_memops;
src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
- src_vq->lock = &ctx->dev->dev_mutex;
+ src_vq->lock = &ctx->vb_mutex;
src_vq->supports_requests = true;
ret = vb2_queue_init(src_vq);
@@ -957,7 +956,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
struct vb2_queue *ds
dst_vq->ops = &vim2m_qops;
dst_vq->mem_ops = &vb2_vmalloc_memops;
dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
- dst_vq->lock = &ctx->dev->dev_mutex;
+ dst_vq->lock = &ctx->vb_mutex;
return vb2_queue_init(dst_vq);
}
@@ -1032,6 +1031,10 @@ static int vim2m_open(struct file *file)
ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_de
[PATCH 0/3] vim2m: make it work properly
The vim2m driver has some issues... It currently fakes supporting two video formats, when in fact, it just copies the data to the buffer; It says it supports hflip, when, in fact, it does a 8 tiles flip... that doesn't end well, though, due to the lack of proper video format; If more than one open() is called, it sometimes go to some dead lock, preventing to stop all pipelines; By default, it can be used only one instance, as it takes too long to generate data (40 msecs). This is actually by purpose, as it uses a delay work queue for that. This patch series solve all the above issues. For the last one, a new modprobe parameter was added, in order to allow changing the default. For example, with this: # sudo modprobe vim2m default_transtime=1 the delay is reduced to 1 ms. On my tests with this pipeline: $ gst-launch-1.0 videotestsrc ! video/x-raw,format=YUY2 ! v4l2video0convert disable-passthrough=1 extra-controls="s,horizontal_flip=0,vertical_flip=0" ! video/x-raw,format=YUY2 ! videoconvert ! fpsdisplaysink and a similar one: $ gst-launch-1.0 videotestsrc ! video/x-raw,format=YUY2 ! v4l2video0convert disable-passthrough=1 extra-controls="s,horizontal_flip=1,vertical_flip=1" ! video/x-raw,format=RGB16 ! videoconvert ! ximagesink I was able to create 17 such pipelines keeping the frame rate at 30 frames per second, and up to 27 pipelines without losing frames, with a framerate close to 20 fps. My tests were done on a 3rd generation i7core machine (i7-3630QM). So, it sounds good enough to be used for testing m2m, even on nowadays CPUs with less performance. I opted to keep the default time to 40 ms to 1 ms, in order to allow multiple streams, but, in practice, I suspect that just one instance should be enough for most usecases. So, I ended by keping the 40 ms timing. PS.: the first patch is identical to the one I submitted before, except for a minor change on its description. This patch series can be found on my development tree: https://git.linuxtv.org/mchehab/experimental.git/log/?h=vim2m Mauro Carvalho Chehab (3): media: vim2m: fix driver for it to handle different fourcc formats media: vim2m: use per-file handler work queue media: vim2m: allow setting the default transaction time via parameter drivers/media/platform/vim2m.c | 434 - 1 file changed, 270 insertions(+), 164 deletions(-) -- 2.20.1
[PATCH 1/3] media: vim2m: fix driver for it to handle different fourcc formats
Despite vim2m is reporting that it supports RGB565BE and YUYV,
that's not true.
Right now, it just says that it supports both format, but it
doesn't actually support them.
Also, horizontal flip is not properly implemented. It sounds
that it was designed to do a pseudo-horizontal flip using 8
tiles. Yet, as it doesn't do format conversion, the result
is a mess.
I suspect that it was done this way in order to save CPU time,
at the time of OMAP2 days.
That's messy and doesn't really help if someone wants to
use vim2m to test a pipeline.
Worse than that, the unique RGB format it says it supports is
RGB565BE, with is not supported by Gstreamer. That prevents
practical usage of it, even for tests.
So, instead, properly implement fourcc format conversions,
adding a few more RGB formats:
- RGB and BGR with 24 bits
- RGB565LE (known as RGB16 at gstreamer)
Also allows using any of the 5 supported formats as either
capture or output.
Note: The YUYV conversion routines are based on the conversion code
written by Hans de Goede inside libv4lconvert (part of v4l-utils),
released under LGPGL 2.1 (GPL 2.0 compatible).
Tested all possible format combinations except for RGB565BE,
as Gstreamer currently doesn't support it.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/platform/vim2m.c | 380 +
1 file changed, 240 insertions(+), 140 deletions(-)
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index a7a152fb3075..ccd0576c766e 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -60,8 +60,6 @@ MODULE_PARM_DESC(debug, "activates debug info");
/* Default transaction time in msec */
#define MEM2MEM_DEF_TRANSTIME 40
-#define MEM2MEM_COLOR_STEP (0xff >> 4)
-#define MEM2MEM_NUM_TILES 8
/* Flags that indicate processing mode */
#define MEM2MEM_HFLIP (1 << 0)
@@ -82,22 +80,24 @@ static struct platform_device vim2m_pdev = {
struct vim2m_fmt {
u32 fourcc;
int depth;
- /* Types the format can be used for */
- u32 types;
};
static struct vim2m_fmt formats[] = {
{
- .fourcc = V4L2_PIX_FMT_RGB565X, /* rggg gggb */
+ .fourcc = V4L2_PIX_FMT_RGB565, /* rggg gggb */
.depth = 16,
- /* Both capture and output format */
- .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
- },
- {
+ }, {
+ .fourcc = V4L2_PIX_FMT_RGB565X, /* gggb rggg */
+ .depth = 16,
+ }, {
+ .fourcc = V4L2_PIX_FMT_RGB24,
+ .depth = 24,
+ }, {
+ .fourcc = V4L2_PIX_FMT_BGR24,
+ .depth = 24,
+ }, {
.fourcc = V4L2_PIX_FMT_YUYV,
.depth = 16,
- /* Output-only format */
- .types = MEM2MEM_OUTPUT,
},
};
@@ -201,23 +201,222 @@ static struct vim2m_q_data *get_q_data(struct vim2m_ctx
*ctx,
return NULL;
}
+#define CLIP(color) \
+ (u8)(((color) > 0xFF) ? 0xff : (((color) < 0) ? 0 : (color)))
+
+static void copy_two_pixels(struct vim2m_fmt *in, struct vim2m_fmt *out,
+ u8 **src, u8 **dst, bool reverse)
+{
+ u8 _r[2], _g[2], _b[2], *r, *g, *b;
+ int i, step;
+
+ // If format is the same just copy the data, respecting the width
+ if (in->fourcc == out->fourcc) {
+ int depth = out->depth >> 3;
+
+ if (reverse) {
+ if (in->fourcc == V4L2_PIX_FMT_YUYV) {
+ int u, v, y, y1;
+
+ *src -= 2;
+
+ y1 = (*src)[0]; /* copy as second point */
+ u = (*src)[1];
+ y = (*src)[2]; /* copy as first point */
+ v = (*src)[3];
+
+ *src -= 2;
+
+ *(*dst)++ = y;
+ *(*dst)++ = u;
+ *(*dst)++ = y1;
+ *(*dst)++ = v;
+ return;
+ }
+
+ memcpy(*dst, *src, depth);
+ memcpy(*dst + depth, *src - depth, depth);
+ *src -= depth << 1;
+ } else {
+ memcpy(*dst, *src, depth << 1);
+ *src += depth << 1;
+ }
+ *dst += depth << 1;
+ return;
+ }
+
+ /* Step 1: read two consecutive pixels from src pointer */
+
+ r = _r;
+ g = _g;
+ b = _b;
+
+ if (reverse)
+ step = -1;
+ else
+ step = 1;
+
+ switch (in->fourcc) {
+ case V4L2_PIX_FMT_RGB565: /* rggg gggb */
+ for (i = 0; i < 2; i++) {
+
[PATCH 3/3] media: vim2m: allow setting the default transaction time via parameter
While there's a control to allow setting it at runtime, as the
control handler is per file handler, only the application setting
the m2m device can change it. As this is a custom control, it is
unlikely that existing apps would be able to set it.
Due to that, and due to the fact that v4l2-mem2mem serializes all
accesses to a m2m device, trying to setup two GStreamer
v4l2videoconvert instance at the same time will cause frame drops.
So, add an alternate way of setting its default via a modprobe parameter.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/platform/vim2m.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index a9e43070567e..0e7814b2327e 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -41,6 +41,12 @@ static unsigned debug;
module_param(debug, uint, 0644);
MODULE_PARM_DESC(debug, "activates debug info");
+/* Default transaction time in msec */
+static unsigned default_transtime = 40; /* Max 25 fps */
+module_param(default_transtime, uint, 0644);
+MODULE_PARM_DESC(default_transtime, "default transaction time in ms");
+
+
#define MIN_W 32
#define MIN_H 32
#define MAX_W 640
@@ -58,9 +64,6 @@ MODULE_PARM_DESC(debug, "activates debug info");
/* In bytes, per queue */
#define MEM2MEM_VID_MEM_LIMIT (16 * 1024 * 1024)
-/* Default transaction time in msec */
-#define MEM2MEM_DEF_TRANSTIME 40
-
/* Flags that indicate processing mode */
#define MEM2MEM_HFLIP (1 << 0)
#define MEM2MEM_VFLIP (1 << 1)
@@ -764,6 +767,8 @@ static int vim2m_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_TRANS_TIME_MSEC:
ctx->transtime = ctrl->val;
+ if (ctx->transtime < 1)
+ ctx->transtime = 1;
break;
case V4L2_CID_TRANS_NUM_BUFS:
@@ -961,12 +966,11 @@ static int queue_init(void *priv, struct vb2_queue
*src_vq, struct vb2_queue *ds
return vb2_queue_init(dst_vq);
}
-static const struct v4l2_ctrl_config vim2m_ctrl_trans_time_msec = {
+static struct v4l2_ctrl_config vim2m_ctrl_trans_time_msec = {
.ops = &vim2m_ctrl_ops,
.id = V4L2_CID_TRANS_TIME_MSEC,
.name = "Transaction Time (msec)",
.type = V4L2_CTRL_TYPE_INTEGER,
- .def = MEM2MEM_DEF_TRANSTIME,
.min = 1,
.max = 10001,
.step = 1,
@@ -1008,6 +1012,8 @@ static int vim2m_open(struct file *file)
v4l2_ctrl_handler_init(hdl, 4);
v4l2_ctrl_new_std(hdl, &vim2m_ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
v4l2_ctrl_new_std(hdl, &vim2m_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
+
+ vim2m_ctrl_trans_time_msec.def = default_transtime;
v4l2_ctrl_new_custom(hdl, &vim2m_ctrl_trans_time_msec, NULL);
v4l2_ctrl_new_custom(hdl, &vim2m_ctrl_trans_num_bufs, NULL);
if (hdl->error) {
--
2.20.1
[PATCH] drm/TODO: Add vrefresh replacement to the todo
From: Sean Paul Suggested-by: Daniel Vetter Signed-off-by: Sean Paul --- Documentation/gpu/todo.rst | 15 +++ 1 file changed, 15 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 38360ede12215..7fc30380eaf6c 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -262,6 +262,21 @@ As a reference, take a look at the conversions already completed in drm core. Contact: Sean Paul, respective driver maintainers +Convert direct mode.vrefresh accesses to use drm_mode_vrefresh() + + +drm_display_mode.vrefresh isn't guaranteed to be populated. As such, using it +is risky and has been known to cause div-by-zero bugs. Fortunately, drm core +has helper which will use mode.vrefresh if it's !0 and will calculate it from +the timings when it's 0. + +Use simple search/replace, or (more fun) cocci to replace instances of direct +vrefresh access with a call to the helper. Check out +https://lists.freedesktop.org/archives/dri-devel/2019-January/205186.html for +inspiration. + +Contact: Sean Paul + Core refactorings = -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH] drm/TODO: Add vrefresh replacement to the todo
On Tue, Jan 29, 2019 at 11:15:51AM -0500, Sean Paul wrote: > From: Sean Paul > > Suggested-by: Daniel Vetter > Signed-off-by: Sean Paul Reviewed-by: Sam Ravnborg > --- > Documentation/gpu/todo.rst | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 38360ede12215..7fc30380eaf6c 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -262,6 +262,21 @@ As a reference, take a look at the conversions already > completed in drm core. > > Contact: Sean Paul, respective driver maintainers > > +Convert direct mode.vrefresh accesses to use drm_mode_vrefresh() > + > + > +drm_display_mode.vrefresh isn't guaranteed to be populated. As such, using it > +is risky and has been known to cause div-by-zero bugs. Fortunately, drm core > +has helper which will use mode.vrefresh if it's !0 and will calculate it from > +the timings when it's 0. > + > +Use simple search/replace, or (more fun) cocci to replace instances of direct > +vrefresh access with a call to the helper. Check out > +https://lists.freedesktop.org/archives/dri-devel/2019-January/205186.html for > +inspiration. Bonus points for the link! Sam
Re: [PATCH] drm/TODO: Add vrefresh replacement to the todo
On Tue, Jan 29, 2019 at 11:15:51AM -0500, Sean Paul wrote: > From: Sean Paul > > Suggested-by: Daniel Vetter > Signed-off-by: Sean Paul > --- > Documentation/gpu/todo.rst | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 38360ede12215..7fc30380eaf6c 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -262,6 +262,21 @@ As a reference, take a look at the conversions already > completed in drm core. > > Contact: Sean Paul, respective driver maintainers > > +Convert direct mode.vrefresh accesses to use drm_mode_vrefresh() > + > + > +drm_display_mode.vrefresh isn't guaranteed to be populated. As such, using it > +is risky and has been known to cause div-by-zero bugs. Fortunately, drm core > +has helper which will use mode.vrefresh if it's !0 and will calculate it from > +the timings when it's 0. > + > +Use simple search/replace, or (more fun) cocci to replace instances of direct > +vrefresh access with a call to the helper. Check out > +https://lists.freedesktop.org/archives/dri-devel/2019-January/205186.html for > +inspiration. I would suggest just nuking mode.vrefresh entirely because we don't want to risk drm_mode_vrefresh() returning some stale value. > + > +Contact: Sean Paul > + > Core refactorings > = > > -- > Sean Paul, Software Engineer, Google / Chromium OS > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel
[PATCH v2] drm/TODO: Add vrefresh replacement to the todo
From: Sean Paul Changes in v2: - Add drm_display_mode.vrefresh removal (Ville) - Add Sam's R-b and bonus points Cc: Ville Syrjälä Suggested-by: Daniel Vetter Reviewed-by: Sam Ravnborg Bonus-points-awarded-by: Sam Ravnborg Signed-off-by: Sean Paul --- Documentation/gpu/todo.rst | 18 ++ 1 file changed, 18 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 38360ede12215..1bbfc5e1b2a46 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -262,6 +262,24 @@ As a reference, take a look at the conversions already completed in drm core. Contact: Sean Paul, respective driver maintainers +Convert direct mode.vrefresh accesses to use drm_mode_vrefresh() + + +drm_display_mode.vrefresh isn't guaranteed to be populated. As such, using it +is risky and has been known to cause div-by-zero bugs. Fortunately, drm core +has helper which will use mode.vrefresh if it's !0 and will calculate it from +the timings when it's 0. + +Use simple search/replace, or (more fun) cocci to replace instances of direct +vrefresh access with a call to the helper. Check out +https://lists.freedesktop.org/archives/dri-devel/2019-January/205186.html for +inspiration. + +Once all instances of vrefresh have been converted, consider removing vrefresh +from drm_display_mode to avoid future use. + +Contact: Sean Paul + Core refactorings = -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH v2] drm/TODO: Add vrefresh replacement to the todo
On Tue, Jan 29, 2019 at 11:45:10AM -0500, Sean Paul wrote: > From: Sean Paul > > Changes in v2: > - Add drm_display_mode.vrefresh removal (Ville) > - Add Sam's R-b and bonus points hsync has the same problem, maybe add that too. With that: Reviewed-by: Daniel Vetter > Cc: Ville Syrjälä > Suggested-by: Daniel Vetter > Reviewed-by: Sam Ravnborg > Bonus-points-awarded-by: Sam Ravnborg > Signed-off-by: Sean Paul > --- > Documentation/gpu/todo.rst | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 38360ede12215..1bbfc5e1b2a46 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -262,6 +262,24 @@ As a reference, take a look at the conversions already > completed in drm core. > > Contact: Sean Paul, respective driver maintainers > > +Convert direct mode.vrefresh accesses to use drm_mode_vrefresh() > + > + > +drm_display_mode.vrefresh isn't guaranteed to be populated. As such, using it > +is risky and has been known to cause div-by-zero bugs. Fortunately, drm core > +has helper which will use mode.vrefresh if it's !0 and will calculate it from > +the timings when it's 0. > + > +Use simple search/replace, or (more fun) cocci to replace instances of direct > +vrefresh access with a call to the helper. Check out > +https://lists.freedesktop.org/archives/dri-devel/2019-January/205186.html for > +inspiration. > + > +Once all instances of vrefresh have been converted, consider removing > vrefresh > +from drm_display_mode to avoid future use. > + > +Contact: Sean Paul > + > Core refactorings > = > > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v3 5/5] psi: introduce psi monitor
Thanks for review Peter!
On Tue, Jan 29, 2019 at 2:44 AM Peter Zijlstra wrote:
>
> On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote:
> > static void psi_update_work(struct work_struct *work)
> > {
> > struct delayed_work *dwork;
> > struct psi_group *group;
> > + bool first_pass = true;
> > + u64 next_update;
> > + u32 change_mask;
> > + int polling;
> > bool nonidle;
> > + u64 now;
> >
> > dwork = to_delayed_work(work);
> > group = container_of(dwork, struct psi_group, clock_work);
> >
> > + now = sched_clock();
> > +
> > + mutex_lock(&group->update_lock);
>
> actually acquiring a mutex can take a fairly long while; would it not
> make more sense to take the @now timestanp _after_ it, instead of
> before?
Yes, that makes sense. As long as *now* is set before the *retry*
label, otherwise the retry mechanism would get even more complicated
to understand with floating *now* timestamp. Will move the assignment
right after the mutex_lock()
> --
> You received this message because you are subscribed to the Google Groups
> "kernel-team" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kernel-team+unsubscr...@android.com.
>
Re: [PATCH v3 5/5] psi: introduce psi monitor
On Tue, Jan 29, 2019 at 4:38 AM Peter Zijlstra wrote:
>
> On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote:
> > + atomic_set(&group->polling, polling);
> > + /*
> > + * Memory barrier is needed to order group->polling
> > + * write before times[] read in collect_percpu_times()
> > + */
> > + smp_mb__after_atomic();
>
> That's broken, smp_mb__{before,after}_atomic() can only be used on
> atomic RmW operations, something atomic_set() is _not_.
Oh, I didn't realize that. After reading the following example from
atomic_ops.txt I was under impression that smp_mb__after_atomic()
would make changes done by atomic_set() visible:
/* All memory operations before this call will
* be globally visible before the clear_bit().
*/
smp_mb__before_atomic();
clear_bit( ... );
/* The clear_bit() will be visible before all
* subsequent memory operations.
*/
smp_mb__after_atomic();
but I'm probably missing something. Is there a more detailed
description of these rules anywhere else?
Meanwhile I'll change smp_mb__after_atomic() into smp_mb(). Would that
fix the ordering?
> --
> You received this message because you are subscribed to the Google Groups
> "kernel-team" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kernel-team+unsubscr...@android.com.
>
Re: [PATCH v3 5/5] psi: introduce psi monitor
On Tue, Jan 29, 2019 at 7:16 AM Peter Zijlstra wrote:
>
> On Tue, Jan 29, 2019 at 01:38:43PM +0100, Peter Zijlstra wrote:
> > On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote:
> > > + atomic_set(&group->polling, polling);
> > > + /*
> > > +* Memory barrier is needed to order group->polling
> > > +* write before times[] read in collect_percpu_times()
> > > +*/
> > > + smp_mb__after_atomic();
> >
> > That's broken, smp_mb__{before,after}_atomic() can only be used on
> > atomic RmW operations, something atomic_set() is _not_.
>
> Also; the comment should explain _why_ not only what.
Got it. Will change the comment to something like:
Order group->polling=0 before reading times[] in
collect_percpu_times() to detect possible race with hotpath that
modifies times[] before it sets group->polling=1 (see Race #1 in the
comments at the top).
> --
> You received this message because you are subscribed to the Google Groups
> "kernel-team" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kernel-team+unsubscr...@android.com.
>
Re: [PATCH v3 5/5] psi: introduce psi monitor
On Tue, Jan 29, 2019 at 10:18 AM Suren Baghdasaryan wrote:
>
> On Tue, Jan 29, 2019 at 4:38 AM Peter Zijlstra wrote:
> >
> > On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote:
> > > + atomic_set(&group->polling, polling);
> > > + /*
> > > + * Memory barrier is needed to order group->polling
> > > + * write before times[] read in
> > > collect_percpu_times()
> > > + */
> > > + smp_mb__after_atomic();
> >
> > That's broken, smp_mb__{before,after}_atomic() can only be used on
> > atomic RmW operations, something atomic_set() is _not_.
>
> Oh, I didn't realize that. After reading the following example from
> atomic_ops.txt I was under impression that smp_mb__after_atomic()
> would make changes done by atomic_set() visible:
>
> /* All memory operations before this call will
> * be globally visible before the clear_bit().
> */
> smp_mb__before_atomic();
> clear_bit( ... );
> /* The clear_bit() will be visible before all
> * subsequent memory operations.
> */
> smp_mb__after_atomic();
>
> but I'm probably missing something. Is there a more detailed
> description of these rules anywhere else?
I was referred to memory-barriers.txt that explains this clearly
stating that "These functions do not imply memory barriers.". Thanks
for noticing! Will change to smp_mb().
> Meanwhile I'll change smp_mb__after_atomic() into smp_mb(). Would that
> fix the ordering?
>
> > --
> > You received this message because you are subscribed to the Google Groups
> > "kernel-team" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to kernel-team+unsubscr...@android.com.
> >
Re: [PATCH v3 5/5] psi: introduce psi monitor
Hi Minchan,
good to see your name on the lists again :)
On Tue, Jan 29, 2019 at 08:53:58AM +0900, Minchan Kim wrote:
> On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote:
> > @@ -68,6 +69,50 @@ struct psi_group_cpu {
> > u32 times_prev[NR_PSI_STATES] cacheline_aligned_in_smp;
> > };
> >
> > +/* PSI growth tracking window */
> > +struct psi_window {
> > + /* Window size in ns */
> > + u64 size;
>
> As rest of field are time, how about "interval" instead of size?
If it were "size" on its own, I would agree, but "window size" is an
existing term that works pretty well here. "window interval" wouldn't.
> > + /* Start time of the current window in ns */
> > + u64 start_time;
> > +
> > + /* Value at the start of the window */
> > + u64 start_value;
>
> "value" is rather vague. starting_stall?
I'm not a fan of using stall here, because it reads like an event,
when it's really a time interval we're interested in.
For an abstraction that samples time intervals, value seems like a
pretty good, straight-forward name...
> > +
> > + /* Value growth per previous window(s) */
> > + u64 per_win_growth;
>
> Rather than per, prev would be more meaninful, I think.
> How about prev_win_stall?
Agreed on the "per", but still not loving the stall. How about
prev_delta? prev_growth?
> > +struct psi_trigger {
> > + /* PSI state being monitored by the trigger */
> > + enum psi_states state;
> > +
> > + /* User-spacified threshold in ns */
> > + u64 threshold;
> > +
> > + /* List node inside triggers list */
> > + struct list_head node;
> > +
> > + /* Backpointer needed during trigger destruction */
> > + struct psi_group *group;
> > +
> > + /* Wait queue for polling */
> > + wait_queue_head_t event_wait;
> > +
> > + /* Pending event flag */
> > + int event;
> > +
> > + /* Tracking window */
> > + struct psi_window win;
> > +
> > + /*
> > +* Time last event was generated. Used for rate-limiting
> > +* events to one per window
> > +*/
> > + u64 last_event_time;
> > +};
> > +
> > struct psi_group {
> > /* Protects data used by the aggregator */
> > struct mutex update_lock;
> > @@ -75,6 +120,8 @@ struct psi_group {
> > /* Per-cpu task state & time tracking */
> > struct psi_group_cpu __percpu *pcpu;
> >
> > + /* Periodic work control */
> > + atomic_t polling;
> > struct delayed_work clock_work;
> >
> > /* Total stall times observed */
> > @@ -85,6 +132,18 @@ struct psi_group {
> > u64 avg_last_update;
> > u64 avg_next_update;
> > unsigned long avg[NR_PSI_STATES - 1][3];
> > +
> > + /* Configured polling triggers */
> > + struct list_head triggers;
> > + u32 nr_triggers[NR_PSI_STATES - 1];
> > + u32 trigger_mask;
>
> This is a state we have an interest.
> How about trigger_states?
Sounds good to me, I'd also switch change_mask below to
changed_states:
if (changed_states & trigger_states)
/* engage! */
[ After reading the rest, I see Minchan proposed the same. ]
> > + u64 trigger_min_period;
> > +
> > + /* Polling state */
> > + /* Total stall times at the start of monitor activation */
> > + u64 polling_total[NR_PSI_STATES - 1];
> > + u64 polling_next_update;
> > + u64 polling_until;
> > };
> >
> > #else /* CONFIG_PSI */
> > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> > index e8cd12c6a553..de3ac22a5e23 100644
> > --- a/kernel/cgroup/cgroup.c
> > +++ b/kernel/cgroup/cgroup.c
> > @@ -3464,7 +3464,101 @@ static int cgroup_cpu_pressure_show(struct seq_file
> > *seq, void *v)
> > {
> > return psi_show(seq, &seq_css(seq)->cgroup->psi, PSI_CPU);
> > }
> > -#endif
> > +
> > +static ssize_t cgroup_pressure_write(struct kernfs_open_file *of, char
> > *buf,
> > + size_t nbytes, enum psi_res res)
> > +{
> > + enum psi_states state;
> > + struct psi_trigger *old;
> > + struct psi_trigger *new;
> > + struct cgroup *cgrp;
> > + u32 threshold_us;
> > + u32 win_sz_us;
>
> window_us?
We don't really encode units in variables in the rest of the code,
maybe we can drop it here as well.
Btw, it looks like the original reason for splitting up trigger_parse
and trigger_create seems gone from the code. Can we merge them again
and keep all those details out of the filesystem ->write methods?
new = psi_trigger_create(group, buf, nbytes, res);
> > + ssize_t ret;
> > +
> > + cgrp = cgroup_kn_lock_live(of->kn, false);
> > + if (!cgrp)
> > + return -ENODEV;
> > +
> > + cgroup_get(cgrp);
> > + cgroup_kn_unlock(of->kn);
> > +
> > + ret = psi_trigger_parse(buf, nbytes, res,
> > + &state, &threshold_us, &win_sz_us);
> > + if (ret) {
> > + cgroup_put(cgrp);
> > + return ret;
> > + }
> > +
> > + new = psi_trigger_create(&cgrp->psi,
> > + state, threshold_us, win_sz_us);
> > + if (IS_ERR(new
Re: [PATCH v3 5/5] psi: introduce psi monitor
On Tue, Jan 29, 2019 at 10:18:20AM -0800, Suren Baghdasaryan wrote:
> On Tue, Jan 29, 2019 at 4:38 AM Peter Zijlstra wrote:
> >
> > On Thu, Jan 24, 2019 at 01:15:18PM -0800, Suren Baghdasaryan wrote:
> > > + atomic_set(&group->polling, polling);
> > > + /*
> > > + * Memory barrier is needed to order group->polling
> > > + * write before times[] read in
> > > collect_percpu_times()
> > > + */
> > > + smp_mb__after_atomic();
> >
> > That's broken, smp_mb__{before,after}_atomic() can only be used on
> > atomic RmW operations, something atomic_set() is _not_.
>
> Oh, I didn't realize that. After reading the following example from
> atomic_ops.txt
That document it woefully out of date (and I should double check, but I
think we can actually delete it now). Please see
Documentation/atomic_t.txt
> I was under impression that smp_mb__after_atomic()
> would make changes done by atomic_set() visible:
>
> /* All memory operations before this call will
> * be globally visible before the clear_bit().
> */
> smp_mb__before_atomic();
> clear_bit( ... );
> /* The clear_bit() will be visible before all
> * subsequent memory operations.
> */
> smp_mb__after_atomic();
>
> but I'm probably missing something. Is there a more detailed
> description of these rules anywhere else?
See atomic_t.txt; but the difference is that clear_bit() is a RmW, while
atomic_set() is just a plain store.
> Meanwhile I'll change smp_mb__after_atomic() into smp_mb(). Would that
> fix the ordering?
It would work here; but I'm still trying to actually understand all
this. So while the detail would be fine, I'm not ready to judge the
over-all thing.
[PATCH v3] drm/TODO: Add drm_display_mode.hsync/vrefresh removal
From: Sean Paul Drivers shouldn't be using these values, add a TODO so someone removes them. Changes in v2: - Add drm_display_mode.vrefresh removal (Ville) - Add Sam's R-b and bonus points Changes in v3: - Add hsync removal todo item (Daniel) - Change vrefresh wording to make removal less optional Cc: Ville Syrjälä Suggested-by: Daniel Vetter Reviewed-by: Daniel Vetter Reviewed-by: Sam Ravnborg Bonus-points-awarded-by: Sam Ravnborg Signed-off-by: Sean Paul --- This time with feeling. Documentation/gpu/todo.rst | 28 1 file changed, 28 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 38360ede12215..f67c84b92873c 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -262,6 +262,34 @@ As a reference, take a look at the conversions already completed in drm core. Contact: Sean Paul, respective driver maintainers +Convert direct mode.vrefresh accesses to use drm_mode_vrefresh() + + +drm_display_mode.vrefresh isn't guaranteed to be populated. As such, using it +is risky and has been known to cause div-by-zero bugs. Fortunately, drm core +has helper which will use mode.vrefresh if it's !0 and will calculate it from +the timings when it's 0. + +Use simple search/replace, or (more fun) cocci to replace instances of direct +vrefresh access with a call to the helper. Check out +https://lists.freedesktop.org/archives/dri-devel/2019-January/205186.html for +inspiration. + +Once all instances of vrefresh have been converted, remove vrefresh from +drm_display_mode to avoid future use. + +Contact: Sean Paul + +Remove drm_display_mode.hsync +- + +We have drm_mode_hsync() to calculate this from hsync_start/end, since drivers +shouldn't/don't use this, remove this member to avoid any temptations to use it +in the future. If there is any debug code using drm_display_mode.hsync, convert +it to use drm_mode_hsync() instead. + +Contact: Sean Paul + Core refactorings = -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH v9 1/3] watchdog: introduce watchdog.open_timeout commandline parameter
On 22/01/2019 18.29, Guenter Roeck wrote:
> On Mon, Jan 21, 2019 at 08:45:39PM +, Rasmus Villemoes wrote:
>> The watchdog framework takes care of feeding a hardware watchdog until
>> userspace opens /dev/watchdogN. If that never happens for some reason
>> (buggy init script, corrupt root filesystem or whatnot) but the kernel
>> itself is fine, the machine stays up indefinitely. This patch allows
>> setting an upper limit for how long the kernel will take care of the
>> watchdog, thus ensuring that the watchdog will eventually reset the
>> machine.
>>
>> A value of 0 (the default) means infinite timeout, preserving the
>> current behaviour.
>>
>> This is particularly useful for embedded devices where some fallback
>> logic is implemented in the bootloader (e.g., use a different root
>> partition, boot from network, ...).
>>
>> There is already handle_boot_enabled serving a similar purpose. However,
>> such a binary choice is unsuitable if the hardware watchdog cannot be
>> programmed by the bootloader to provide a timeout long enough for
>> userspace to get up and running. Many of the embedded devices we see use
>> external (gpio-triggered) watchdogs with a fixed timeout of the order of
>> 1-2 seconds.
>>
>> The open timeout is also used as a maximum time for an application to
>> re-open /dev/watchdogN after closing it. Again, while the kernel already
>> has a nowayout mechanism, using that means userspace is at the mercy of
>> whatever timeout the hardware has.
>>
>> Being a module parameter, one can revert to the ordinary behaviour of
>> having the kernel maintain the watchdog indefinitely by simply writing 0
>> to /sys/... after initially opening /dev/watchdog; conversely, one can
>> of course also have the current behaviour of allowing indefinite time
>> until the first open, and then set that module parameter.
>>
>> Signed-off-by: Rasmus Villemoes
>> ---
>> .../watchdog/watchdog-parameters.txt | 8 +
>> drivers/watchdog/watchdog_dev.c | 30 +--
>> 2 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/watchdog/watchdog-parameters.txt
>> b/Documentation/watchdog/watchdog-parameters.txt
>> index 0b88e333f9e1..907c4bb13810 100644
>> --- a/Documentation/watchdog/watchdog-parameters.txt
>> +++ b/Documentation/watchdog/watchdog-parameters.txt
>> @@ -8,6 +8,14 @@ See Documentation/admin-guide/kernel-parameters.rst for
>> information on
>> providing kernel parameters for builtin drivers versus loadable
>> modules.
>>
>> +The watchdog core parameter watchdog.open_timeout is the maximum time,
>> +in seconds, for which the watchdog framework will take care of pinging
>> +a hardware watchdog until userspace opens the corresponding
>> +/dev/watchdogN device. A value of 0 (the default) means an infinite
>> +timeout. Setting this to a non-zero value can be useful to ensure that
>> +either userspace comes up properly, or the board gets reset and allows
>> +fallback logic in the bootloader to try something else.
>> +
>
> This is misleading. Unless I am missing something, the above only applies
> if the watchdog is already runnning at boot,
well, yes, if it's not running at boot, there's nothing for the kernel
to take care of. I can do s/a hardware watchdog/a running hardware
watchdog/ if that makes it clearer.
and after it has been opened
> and closed once.
>
> FWIW, I find this operation quite confusing. What is the rationale for not
> starting the watchdog at boot time if it is not running and open_timeout
> is set, but then refusing to stop it after it has been started once ?
I guess you have a point that there's a certain asymmetry there. In
practice, the cases where one wants this kind of robustness guarantee do
not rely on a watchdog that can/must be started and stopped by software.
>>
>> static void watchdog_ping_work(struct kthread_work *work)
>> @@ -297,7 +317,7 @@ static int watchdog_stop(struct watchdog_device *wdd)
>> return -EBUSY;
>> }
>>
>> -if (wdd->ops->stop) {
>> +if (wdd->ops->stop && !open_timeout) {
>
> This changes the semantics of WDIOC_SETOPTIONS / WDIOS_DISABLECARD.
> "Turn off the watchdog timer" is well defined and doesn't leave
> the option of setting a timeout on it.
I can drop this hunk, since it's mostly irrelevant to the actual use
cases I have in mind. It makes testing the feature on reference boards a
little more awkward, but I can live with that.
>> +
>> +module_param(open_timeout, uint, 0644);
>> +MODULE_PARM_DESC(open_timeout,
>> +"Maximum time (in seconds, 0 means infinity) for userspace to take over
>> a running watchdog (default=0)");
>
> The description is misleading. After the initial open, a subsequent close no
> longer really stops the watchdog.
If I drop the "&& !open_timeout" above, this would be accurate, no?
Rasmus
[PATCH] rcu docs: repair some whitespace damage
While reading the docs I noticed some whitespace damage in diagram. Let's fix it up to be consistent with elsewhere in the document: use one leading tab, followed by spaces for any additional whitespace required. Signed-off-by: Tycho Andersen --- Documentation/RCU/whatisRCU.txt | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt index 4a6854318b17..419ec18b87bd 100644 --- a/Documentation/RCU/whatisRCU.txt +++ b/Documentation/RCU/whatisRCU.txt @@ -310,7 +310,7 @@ reader, updater, and reclaimer. rcu_assign_pointer() - ++ + ++ +-->| reader |-+ | ++ | | | | @@ -318,12 +318,12 @@ reader, updater, and reclaimer. | | | rcu_read_lock() | | | rcu_read_unlock() |rcu_dereference() | | - +-+ | | - | updater |<-+ | - +-+ V + +-+ | | + | updater |<+ | + +-+V |+---+ +--->| reclaimer | -+---+ ++---+ Defer: synchronize_rcu() & call_rcu() -- 2.19.1
Re: [PATCH v9 1/3] watchdog: introduce watchdog.open_timeout commandline parameter
On 29/01/2019 21.35, Rasmus Villemoes wrote:
> On 22/01/2019 18.29, Guenter Roeck wrote:
>> On Mon, Jan 21, 2019 at 08:45:39PM +, Rasmus Villemoes wrote:
>>>
>>> static void watchdog_ping_work(struct kthread_work *work)
>>> @@ -297,7 +317,7 @@ static int watchdog_stop(struct watchdog_device *wdd)
>>> return -EBUSY;
>>> }
>>>
>>> - if (wdd->ops->stop) {
>>> + if (wdd->ops->stop && !open_timeout) {
>>
>> This changes the semantics of WDIOC_SETOPTIONS / WDIOS_DISABLECARD.
>> "Turn off the watchdog timer" is well defined and doesn't leave
>> the option of setting a timeout on it.
>
> I can drop this hunk, since it's mostly irrelevant to the actual use
> cases I have in mind. It makes testing the feature on reference boards a
> little more awkward, but I can live with that.
Actually, that's not enough - if there was a non-zero open_timeout at
boot, the device has had its ->open_deadline set to some finite (not
KTIME_MAX) value, and ->open_deadline does not get updated on the
WDIOC_SETOPTIONS / WDIOS_DISABLECARD path.
So, I think one way to preserve the semantics of WDIOS_DISABLECARD
(which, for a non-stopable watchdog really means "please let the kernel
take care of this indefinitely, or until WDIOS_ENABLECARD") is to extend
watchdog_stop with a timeout parameter, and do the call of
watchdog_set_open_deadline() from watchdog_stop() using the passed
timeout value. When called from the WDIOS_DISABLECARD case, we'd pass 0
for that timeout, while the call from _release would pass the module
parameter open_timeout. [Obviously, watchdog_set_open_deadline() would
also take the timeout as a parameter instead of referring to the
open_timeout directly.]
Thanks for pointing out the WDIOS_DISABLECARD case. I'll try to make the
above into code to see how it looks.
Rasmus
