Re: WARNING: lock held when returning to user space in set_property_atomic
On 2019/01/03 18:04, Dmitry Vyukov wrote:
> On Thu, Jan 3, 2019 at 9:55 AM Maarten Lankhorst
> wrote:
>> Just guessing..
>>
>> Does this help?
Yes it will. And while at it, let's fix another one together.
From 291e42211e3cc6d85c915772717dd08d40fb5fed Mon Sep 17 00:00:00 2001
From: Tetsuo Handa
Date: Fri, 4 Jan 2019 15:23:47 +0900
Subject: [PATCH] gpu/drm: Fix lock held when returning to user space.
We need to call drm_modeset_acquire_fini() when drm_atomic_state_alloc()
failed or call drm_modeset_acquire_init() after drm_atomic_state_alloc()
succeeded.
Signed-off-by: Tetsuo Handa
Reported-by: syzbot
---
drivers/gpu/drm/drm_atomic_uapi.c | 3 +--
drivers/gpu/drm/drm_mode_object.c | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index c408898..9a1f41a 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1296,12 +1296,11 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
(arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
return -EINVAL;
- drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
-
state = drm_atomic_state_alloc(dev);
if (!state)
return -ENOMEM;
+ drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
state->acquire_ctx = &ctx;
state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
diff --git a/drivers/gpu/drm/drm_mode_object.c
b/drivers/gpu/drm/drm_mode_object.c
index cd9bc0c..004191d 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -459,11 +459,11 @@ static int set_property_atomic(struct drm_mode_object
*obj,
struct drm_modeset_acquire_ctx ctx;
int ret;
- drm_modeset_acquire_init(&ctx, 0);
-
state = drm_atomic_state_alloc(dev);
if (!state)
return -ENOMEM;
+
+ drm_modeset_acquire_init(&ctx, 0);
state->acquire_ctx = &ctx;
retry:
if (prop == state->dev->mode_config.dpms_property) {
--
1.8.3.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: "flip_done timed out" messages causing huge increase in boot time
I should really get to bed ... of course, I meant 516a49cc19467e298d08a404f73a6e311f4548d1 ;-) On Sat, 5 Jan 2019 at 01:16, Daniel Kamil Kozar wrote: > > Small omission on my part : I meant 4.19.12, not 4.19.2 as the last > working version. > Also, I can confirm that reverting > 13947d150bae871bd880565ada318b0bcd0e690e from the current HEAD of > linux-stable fixes the issue. > > On Sat, 5 Jan 2019 at 00:47, Daniel Kamil Kozar wrote: > > > > Hello. > > After upgrading the kernel to 4.20, I noticed that the boot time > > increased from about 5 seconds to 25 seconds. My machine is an Asus > > K53SV with an Intel i7-2630QM, i.e. Sandy Bridge. I have an external > > display connected to it via HDMI. If the display is unplugged when > > booting the machine, the boot time stays at its old value. > > The kernel log shows two messages like this : > > > > [ 15.747206] [drm:drm_atomic_helper_wait_for_flip_done > > [drm_kms_helper]] *ERROR* [CRTC:39:pipe A] flip_done timed out > > [ 26.198968] [drm:drm_atomic_helper_wait_for_dependencies > > [drm_kms_helper]] *ERROR* [CRTC:39:pipe A] flip_done timed out > > > > Which are the only ones that don't appear in 4.19.2, where the boot > > time was smaller. Bisecting the commits between these two versions > > leads to commit 516a49cc19467e298d08a404f73a6e311f4548d1. > > > > Let me know if you need more information. > > > > Kind regards, > > Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Sat, Jan 05, 2019 at 10:37:17AM +0800, kbuild test robot wrote: > Hi Jason, > > I love your patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [also build test ERROR on v4.20 next-20190103] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Jason-Gunthorpe/lib-scatterlist-Provide-a-DMA-page-iterator/20190105-081739 > config: x86_64-randconfig-x017-201900 (attached as .config) > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All error/warnings (new ones prefixed by >>): > >In file included from lib/scatterlist.c:9:0: > >> include/linux/export.h:81:20: error: redefinition of > >> '__kstrtab___sg_page_iter_next' > static const char __kstrtab_##sym[]\ >^ >include/linux/export.h:120:25: note: in expansion of macro > '___EXPORT_SYMBOL' > #define __EXPORT_SYMBOL ___EXPORT_SYMBOL > ^~~~ >include/linux/export.h:124:2: note: in expansion of macro '__EXPORT_SYMBOL' > __EXPORT_SYMBOL(sym, "") > ^~~ > >> lib/scatterlist.c:652:1: note: in expansion of macro 'EXPORT_SYMBOL' > EXPORT_SYMBOL(__sg_page_iter_next); > ^ Woops, should be __sg_page_dma_iter_next.. Will resend after getting some feedback. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
"flip_done timed out" messages causing huge increase in boot time
Hello. After upgrading the kernel to 4.20, I noticed that the boot time increased from about 5 seconds to 25 seconds. My machine is an Asus K53SV with an Intel i7-2630QM, i.e. Sandy Bridge. I have an external display connected to it via HDMI. If the display is unplugged when booting the machine, the boot time stays at its old value. The kernel log shows two messages like this : [ 15.747206] [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:39:pipe A] flip_done timed out [ 26.198968] [drm:drm_atomic_helper_wait_for_dependencies [drm_kms_helper]] *ERROR* [CRTC:39:pipe A] flip_done timed out Which are the only ones that don't appear in 4.19.2, where the boot time was smaller. Bisecting the commits between these two versions leads to commit 516a49cc19467e298d08a404f73a6e311f4548d1. Let me know if you need more information. Kind regards, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/2] drm/amd: validate user pitch alignment
On Thu, Jan 03, 2019 at 05:33:19PM +0100, Michel Dänzer wrote:
> On 2018-12-30 2:00 a.m., Yu Zhao wrote:
> > Userspace may request pitch alignment that is not supported by GPU.
> > Some requests 32, but GPU ignores it and uses default 64 when cpp is
> > 4. If GEM object is allocated based on the smaller alignment, GPU
> > DMA will go out of bound.
> >
> > Cc: sta...@vger.kernel.org # v4.2+
> > Signed-off-by: Yu Zhao
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > index 15ce7e681d67..16af80ccd0a0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > @@ -527,6 +527,22 @@ amdgpu_display_user_framebuffer_create(struct
> > drm_device *dev,
> > struct drm_gem_object *obj;
> > struct amdgpu_framebuffer *amdgpu_fb;
> > int ret;
> > + struct amdgpu_device *adev = dev->dev_private;
> > + int cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
> > + int pitch = mode_cmd->pitches[0] / cpp;
> > +
> > + if (pitch < mode_cmd->width) {
> > + DRM_DEBUG_KMS("Expecting pitch(%d)/cpp(%d) >= width(%d)\n",
> > + mode_cmd->pitches[0], cpp, mode_cmd->width);
> > + return ERR_PTR(-EINVAL);
> > + }
>
> This should possibly be tested in drm_internal_framebuffer_create instead.
It seems to me drm_format_info_min_pitch() in framebuffer_check()
already does it. We could just remove the check here?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte()
On Sun, 30 Dec 2018, I wrote:
> On Sat, 29 Dec 2018, LEROY Christophe wrote:
>
> > Finn Thain a ?crit?:
> >
> > > Make use of arch_nvram_ops in device drivers so that the nvram_* function
> > > exports can be removed.
> > >
> > > Since they are no longer global symbols, rename the PPC32 nvram_*
> > > functions
> > > appropriately.
> > >
> > > Signed-off-by: Finn Thain
> > > ---
> > > arch/powerpc/kernel/setup_32.c | 8
> > > drivers/char/generic_nvram.c | 4 ++--
> > > drivers/video/fbdev/controlfb.c| 4 ++--
> > > drivers/video/fbdev/imsttfb.c | 4 ++--
> > > drivers/video/fbdev/matrox/matroxfb_base.c | 2 +-
> > > drivers/video/fbdev/platinumfb.c | 4 ++--
> > > drivers/video/fbdev/valkyriefb.c | 4 ++--
> > > 7 files changed, 15 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kernel/setup_32.c
> > > b/arch/powerpc/kernel/setup_32.c
> > > index e0d045677472..bdbe6acbef11 100644
> > > --- a/arch/powerpc/kernel/setup_32.c
> > > +++ b/arch/powerpc/kernel/setup_32.c
> > > @@ -152,20 +152,18 @@ __setup("l3cr=", ppc_setup_l3cr);
> > >
> > > #ifdef CONFIG_GENERIC_NVRAM
> > >
> > > -unsigned char nvram_read_byte(int addr)
> > > +static unsigned char ppc_nvram_read_byte(int addr)
> > > {
> > > if (ppc_md.nvram_read_val)
> > > return ppc_md.nvram_read_val(addr);
> > > return 0xff;
> > > }
> > > -EXPORT_SYMBOL(nvram_read_byte);
> > >
> > > -void nvram_write_byte(unsigned char val, int addr)
> > > +static void ppc_nvram_write_byte(unsigned char val, int addr)
> > > {
> > > if (ppc_md.nvram_write_val)
> > > ppc_md.nvram_write_val(addr, val);
> > > }
> > > -EXPORT_SYMBOL(nvram_write_byte);
> > >
> > > static ssize_t ppc_nvram_get_size(void)
> > > {
> > > @@ -182,6 +180,8 @@ static long ppc_nvram_sync(void)
> > > }
> > >
> > > const struct nvram_ops arch_nvram_ops = {
> > > + .read_byte = ppc_nvram_read_byte,
> > > + .write_byte = ppc_nvram_write_byte,
> > > .get_size = ppc_nvram_get_size,
> > > .sync = ppc_nvram_sync,
> > > };
> > > diff --git a/drivers/char/generic_nvram.c b/drivers/char/generic_nvram.c
> > > index f32d5663de95..41b76bf9614e 100644
> > > --- a/drivers/char/generic_nvram.c
> > > +++ b/drivers/char/generic_nvram.c
> > > @@ -48,7 +48,7 @@ static ssize_t read_nvram(struct file *file, char __user
> > > *buf,
> > > if (*ppos >= nvram_len)
> > > return 0;
> > > for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count)
> > > - if (__put_user(nvram_read_byte(i), p))
> > > + if (__put_user(arch_nvram_ops.read_byte(i), p))
> >
> > Instead of modifying all drivers (in this patch and previous ones related to
> > other arches), wouldn't it be better to add helpers like the following in
> > nvram.h:
> >
> > Static inline unsigned char nvram_read_byte(int addr)
> > {
> >return arch_nvram_ops.read_byte(addr);
> > }
> >
>
> Is there some benefit, or is that just personal taste?
>
> Avoiding changes to call sites avoids code review, but I think 1) the
> thinkpad_acpi changes have already been reviewed and 2) the fbdev changes
> need review anyway.
>
> Your suggesion would add several new entities and one extra layer of
> indirection.
>
Contrary to what I said above, this kind of double indirection could be
useful if it allows us to avoid the kind of double indirection which Arnd
objected to (which arises when an arch_nvram_ops method invokes a ppc_md
method). For example,
static inline unsigned char nvram_read_byte(int addr)
{
#ifdef CONFIG_PPC
return ppc_md.nvram_read_byte(addr);
#else
return arch_nvram_ops.read_byte(addr);
#endif
}
I'll try this approach for v9 if there are no objections. It may be the
least invasive approach. Also, arch_nvram_ops can remain const.
--
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 1/2] drm/fb-helper: Bring back workaround for bugs of SDL 1.2
On Fri, 2018-12-28 at 13:15 +0100, Daniel Vetter wrote:
> On Fri, Dec 28, 2018 at 04:13:07AM +0500, Ivan Mironov wrote:
> > SDL 1.2 sets all fields related to the pixel format to zero in some
> > cases[1]. Prior to commit db05c48197759 ("drm: fb-helper: Reject all
> > pixel format changing requests"), there was an unintentional workaround
> > for this that existed for more than a decade. First in device-specific DRM
> > drivers, then here in drm_fb_helper.c.
> >
> > Previous code containing this workaround just ignores pixel format fields
> > from userspace code. Not a good thing either, as this way, driver may
> > silently use pixel format different from what client actually requested,
> > and this in turn will lead to displaying garbage on the screen. I think
> > that returning EINVAL to userspace in this particular case is the right
> > option, so I decided to left code from problematic commit untouched
> > instead of just reverting it entirely.
> >
> > Here is the steps required to reproduce this problem exactly:
> > 1) Compile fceux[2] with SDL 1.2.15 and without GTK or OpenGL
> >support. SDL should be compiled with fbdev support (which is
> >on by default).
> > 2) Create /etc/fb.modes with following contents (values seems
> >not used, and just required to trigger problematic code in
> >SDL):
> >
> > mode "test"
> > geometry 1 1 1 1 1
> > timings 1 1 1 1 1 1 1
> > endmode
> >
> > 3) Create ~/.fceux/fceux.cfg with following contents:
> >
> > SDL.Hotkeys.Quit = 27
> > SDL.DoubleBuffering = 1
> >
> > 4) Ensure that screen resolution is at least 1280x960 (e.g.
> >append "video=Virtual-1:1280x960-32" to the kernel cmdline
> >for qemu/QXL).
> >
> > 5) Try to run fceux on VT with some ROM file[3]:
> >
> > # ./fceux color_test.nes
> >
> > [1] SDL 1.2.15 source code, src/video/fbcon/SDL_fbvideo.c,
> > FB_SetVideoMode()
> > [2] http://www.fceux.com
> > [3] Example ROM:
> > https://github.com/bokuweb/rustynes/blob/master/roms/color_test.nes
> >
> > Reported-by: saahriktu
> > Suggested-by: saahriktu
> > Cc: sta...@vger.kernel.org
> > Fixes: db05c48197759 ("drm: fb-helper: Reject all pixel format changing
> > requests")
> > Signed-off-by: Ivan Mironov
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 146
> > 1 file changed, 93 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > b/drivers/gpu/drm/drm_fb_helper.c
> > index d3af098b0922..aff576c3c4fb 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -1621,6 +1621,64 @@ static bool drm_fb_pixel_format_equal(const struct
> > fb_var_screeninfo *var_1,
> >var_1->transp.msb_right == var_2->transp.msb_right;
> > }
> >
> > +static void drm_fb_helper_fill_pixel_fmt(struct fb_var_screeninfo *var,
> > +u8 depth)
> > +{
> > + switch (depth) {
> > + case 8:
> > + var->red.offset = 0;
> > + var->green.offset = 0;
> > + var->blue.offset = 0;
> > + var->red.length = 8; /* 8bit DAC */
> > + var->green.length = 8;
> > + var->blue.length = 8;
> > + var->transp.offset = 0;
> > + var->transp.length = 0;
> > + break;
> > + case 15:
> > + var->red.offset = 10;
> > + var->green.offset = 5;
> > + var->blue.offset = 0;
> > + var->red.length = 5;
> > + var->green.length = 5;
> > + var->blue.length = 5;
> > + var->transp.offset = 15;
> > + var->transp.length = 1;
> > + break;
> > + case 16:
> > + var->red.offset = 11;
> > + var->green.offset = 5;
> > + var->blue.offset = 0;
> > + var->red.length = 5;
> > + var->green.length = 6;
> > + var->blue.length = 5;
> > + var->transp.offset = 0;
> > + break;
> > + case 24:
> > + var->red.offset = 16;
> > + var->green.offset = 8;
> > + var->blue.offset = 0;
> > + var->red.length = 8;
> > + var->green.length = 8;
> > + var->blue.length = 8;
> > + var->transp.offset = 0;
> > + var->transp.length = 0;
> > + break;
> > + case 32:
> > + var->red.offset = 16;
> > + var->green.offset = 8;
> > + var->blue.offset = 0;
> > + var->red.length = 8;
> > + var->green.length = 8;
> > + var->blue.length = 8;
> > + var->transp.offset = 24;
> > + var->transp.length = 8;
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > /**
> > * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var
> > * @var: screeninfo to check
> > @@ -1654,6 +1712,40 @@ in
Re: [PATCH v2] drm/amdgpu_vm: fix boolean expressions
Hi Ezequiel,
On 1/4/19 10:40 AM, Ezequiel Garcia wrote:
Hey Gustavo,
On Thu, 3 Jan 2019 at 17:25, Gustavo A. R. Silva wrote:
Fix boolean expressions by using logical AND operator '&&'
instead of bitwise operator '&'.
This issue was detected with the help of Coccinelle.
Fixes: 9a4b7d4c769e ("drm/amdgpu: Add vm context module param")
Cc: sta...@vger.kernel.org
Reviewed-by: Felix Kuehling
Signed-off-by: Gustavo A. R. Silva
---
Changes in v2:
- Update Fixes tag.
This just fixes a user warning that would be wrongly reported, but
doesn't fix any other issue.
Why stable?
Yeah. In this case the tag can be removed.
Happy new year.
Thanks
--
Gustavo
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/fb-helper: Ignore the value of fb_var_screeninfo.pixclock
On Fri, 2018-12-28 at 13:06 +0100, Daniel Vetter wrote:
> On Fri, Dec 28, 2018 at 04:13:08AM +0500, Ivan Mironov wrote:
> > Strict requirement of pixclock to be zero breaks support of SDL 1.2
> > which contains hardcoded table of supported video modes with non-zero
> > pixclock values[1].
> >
> > To better understand which pixclock values are considered valid and how
> > driver should handle these values, I briefly examined few existing fbdev
> > drivers and documentation in Documentation/fb/. And it looks like there
> > are no strict rules on that and actual behaviour varies:
> >
> > * some drivers treat (pixclock == 0) as "use defaults" (uvesafb.c);
> > * some treat (pixclock == 0) as invalid value which leads to
> > -EINVAL (clps711x-fb.c);
> > * some pass converted pixclock value to hardware (uvesafb.c);
> > * some are trying to find nearest value from predefined table
> > (vga16fb.c, video_gx.c).
> >
> > Given this, I believe that it should be safe to just ignore this value if
> > changing is not supported. It seems that any portable fbdev application
> > which was not written only for one specific device working under one
> > specific kernel version should not rely on any particular behaviour of
> > pixclock anyway.
> >
> > However, while enabling SDL1 applications to work out of the box when
> > there is no /etc/fb.modes with valid settings, this change affects the
> > video mode choosing logic in SDL. Depending on current screen
> > resolution, contents of /etc/fb.modes and resolution requested by
> > application, this may lead to user-visible difference (not always):
> > image will be displayed in a right way, but it will be aligned to the
> > left instead of center. There is no "right behaviour" here as well, as
> > emulated fbdev, opposing to old fbdev drivers, simply ignores any
> > requsts of video mode changes with resolutions smaller than current.
> >
> > Feel free to NAK this patch if you think that it causes breakage of
> > user-space =).
>
> It's a regression, we don't nack regression fixes :-)
>
> > The easiest way to reproduce this problem is to install sdl-sopwith[2],
> > remove /etc/fb.modes file if it exists, and then try to run sopwith
> > from console without X. At least in Fedora 29, sopwith may be simply
> > installed from standard repositories.
> >
> > [1] SDL 1.2.15 source code, src/video/fbcon/SDL_fbvideo.c, vesa_timings
> > [2] http://sdl-sopwith.sourceforge.net/
> >
> > Signed-off-by: Ivan Mironov
>
> I thought this is also a regression fix, so also needs Fixes: and cc:
> stable?
> -Daniel
This particular patch fixes a bug that existed for 10 years unnoticed.
Are "Fixes:" and "Cc: stable" really required?
"pixclock != 0" check was introduced in this file by 785b93ef8c309
("drm/kms: move driver specific fb common code to helper functions
(v2)"). But actually similar code existed in the device-specific
drivers even earlier.
>
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > b/drivers/gpu/drm/drm_fb_helper.c
> > index aff576c3c4fb..b95a0c23c7c8 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -1690,9 +1690,14 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo
> > *var,
> > struct drm_fb_helper *fb_helper = info->par;
> > struct drm_framebuffer *fb = fb_helper->fb;
> >
> > - if (var->pixclock != 0 || in_dbg_master())
> > + if (in_dbg_master())
> > return -EINVAL;
> >
> > + if (var->pixclock != 0) {
> > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel
> > clock, value of pixclock is ignored\n");
> > + var->pixclock = 0;
> > + }
> > +
> > if ((drm_format_info_block_width(fb->format, 0) > 1) ||
> > (drm_format_info_block_height(fb->format, 0) > 1))
> > return -EINVAL;
> > --
> > 2.20.1
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: "flip_done timed out" messages causing huge increase in boot time
Small omission on my part : I meant 4.19.12, not 4.19.2 as the last working version. Also, I can confirm that reverting 13947d150bae871bd880565ada318b0bcd0e690e from the current HEAD of linux-stable fixes the issue. On Sat, 5 Jan 2019 at 00:47, Daniel Kamil Kozar wrote: > > Hello. > After upgrading the kernel to 4.20, I noticed that the boot time > increased from about 5 seconds to 25 seconds. My machine is an Asus > K53SV with an Intel i7-2630QM, i.e. Sandy Bridge. I have an external > display connected to it via HDMI. If the display is unplugged when > booting the machine, the boot time stays at its old value. > The kernel log shows two messages like this : > > [ 15.747206] [drm:drm_atomic_helper_wait_for_flip_done > [drm_kms_helper]] *ERROR* [CRTC:39:pipe A] flip_done timed out > [ 26.198968] [drm:drm_atomic_helper_wait_for_dependencies > [drm_kms_helper]] *ERROR* [CRTC:39:pipe A] flip_done timed out > > Which are the only ones that don't appear in 4.19.2, where the boot > time was smaller. Bisecting the commits between these two versions > leads to commit 516a49cc19467e298d08a404f73a6e311f4548d1. > > Let me know if you need more information. > > Kind regards, > Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] v2 bakclight: fix missing checks in ambient_light_zone_store
In adp8870_bl_ambient_light_zone_store, set, clear, and write
can return an error but are not checked. The fix adds a check for these
cases and returns -1 to match the return type (ssize_t).
Signed-off-by: Aditya Pakki
---
drivers/video/backlight/adp8870_bl.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/video/backlight/adp8870_bl.c
b/drivers/video/backlight/adp8870_bl.c
index 8d50e0299578..56a640587a82 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -800,10 +800,14 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct
device *dev,
if (val == 0) {
/* Enable automatic ambient light sensing */
- adp8870_set_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
+ ret = adp8870_set_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
+ if (ret < 0)
+ goto adp8870_bl_err;
} else if ((val > 0) && (val < 6)) {
/* Disable automatic ambient light sensing */
- adp8870_clr_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
+ ret = adp8870_clr_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
+ if (ret < 0)
+ goto adp8870_bl_err;
/* Set user supplied ambient light zone */
mutex_lock(&data->lock);
@@ -811,12 +815,18 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct
device *dev,
if (!ret) {
reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
reg_val |= (val - 1) << CFGR_BLV_SHIFT;
- adp8870_write(data->client, ADP8870_CFGR, reg_val);
+ ret = adp8870_write(data->client, ADP8870_CFGR,
+ reg_val);
}
mutex_unlock(&data->lock);
+ if (ret < 0)
+ goto adp8870_bl_err;
}
return count;
+
+adp8870_bl_err:
+ return -1;
}
static DEVICE_ATTR(ambient_light_zone, 0664,
adp8870_bl_ambient_light_zone_show,
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] lib/scatterlist: Provide a DMA page iterator
Commit 2db76d7c3c6d ("lib/scatterlist: sg_page_iter: support sg lists w/o
backing pages") introduced the sg_page_iter_dma_address() function without
providing a way to use it in the general case. If the sg_dma_len is not
equal to the dma_length callers cannot safely use the
for_each_sg_page/sg_page_iter_dma_address combination.
Resolve this API mistake by providing a DMA specific iterator,
for_each_sg_dma_page(), that uses the right length so
sg_page_iter_dma_address() works as expected with all sglists. A new
iterator type is introduced to provide compile-time safety against wrongly
mixing accessors and iterators.
Signed-off-by: Jason Gunthorpe
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h| 26 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_mob.c| 26 +++-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 42 +--
drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 +-
include/linux/scatterlist.h| 49 ++
lib/scatterlist.c | 26
6 files changed, 134 insertions(+), 39 deletions(-)
I'd like to run this patch through the RDMA tree as we have another
series in the works that wants to use the for_each_sg_dma_page() API.
The changes to vmwgfx make me nervous, it would be great if someone
could test and ack them?
Changes since the RFC:
- Rework vmwgfx too [CH]
- Use a distinct type for the DMA page iterator [CH]
- Do not have a #ifdef [CH]
Thanks,
Jason
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 59f614225bcd72..3c6d71e13a9342 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -297,7 +297,10 @@ struct vmw_sg_table {
struct vmw_piter {
struct page **pages;
const dma_addr_t *addrs;
- struct sg_page_iter iter;
+ union {
+ struct sg_page_iter iter;
+ struct sg_dma_page_iter dma_iter;
+ };
unsigned long i;
unsigned long num_pages;
bool (*next)(struct vmw_piter *);
@@ -869,9 +872,24 @@ extern int vmw_bo_map_dma(struct ttm_buffer_object *bo);
extern void vmw_bo_unmap_dma(struct ttm_buffer_object *bo);
extern const struct vmw_sg_table *
vmw_bo_sg_table(struct ttm_buffer_object *bo);
-extern void vmw_piter_start(struct vmw_piter *viter,
- const struct vmw_sg_table *vsgt,
- unsigned long p_offs);
+void _vmw_piter_start(struct vmw_piter *viter, const struct vmw_sg_table *vsgt,
+ unsigned long p_offs, bool for_dma);
+
+/* Create a piter that can call vmw_piter_dma_addr() */
+static inline void vmw_piter_start(struct vmw_piter *viter,
+ const struct vmw_sg_table *vsgt,
+ unsigned long p_offs)
+{
+ _vmw_piter_start(viter, vsgt, p_offs, true);
+}
+
+/* Create a piter that can call vmw_piter_page() */
+static inline void vmw_piter_cpu_start(struct vmw_piter *viter,
+ const struct vmw_sg_table *vsgt,
+ unsigned long p_offs)
+{
+ _vmw_piter_start(viter, vsgt, p_offs, false);
+}
/**
* vmw_piter_next - Advance the iterator one page.
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
index 7ed179d30ec51f..a13788017ad608 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
@@ -503,7 +503,8 @@ static void vmw_mob_assign_ppn(u32 **addr, dma_addr_t val)
*/
static unsigned long vmw_mob_build_pt(struct vmw_piter *data_iter,
unsigned long num_data_pages,
- struct vmw_piter *pt_iter)
+ struct vmw_piter *pt_iter_cpu,
+ struct vmw_piter *pt_iter_dma)
{
unsigned long pt_size = num_data_pages * VMW_PPN_SIZE;
unsigned long num_pt_pages = DIV_ROUND_UP(pt_size, PAGE_SIZE);
@@ -513,7 +514,7 @@ static unsigned long vmw_mob_build_pt(struct vmw_piter
*data_iter,
struct page *page;
for (pt_page = 0; pt_page < num_pt_pages; ++pt_page) {
- page = vmw_piter_page(pt_iter);
+ page = vmw_piter_page(pt_iter_cpu);
save_addr = addr = kmap_atomic(page);
@@ -525,7 +526,8 @@ static unsigned long vmw_mob_build_pt(struct vmw_piter
*data_iter,
WARN_ON(!vmw_piter_next(data_iter));
}
kunmap_atomic(save_addr);
- vmw_piter_next(pt_iter);
+ vmw_piter_next(pt_iter_cpu);
+ vmw_piter_next(pt_iter_dma);
}
return num_pt_pages;
@@ -547,29 +549,31 @@ static void vmw_mob_pt_setup(struct vmw_mob *mob,
{
unsigned long num_pt_pages = 0;
struct ttm_buffer_object *bo = mob->pt_bo;
- struct vmw_piter save_pt_iter;
- struct vmw_piter pt_iter;
+
Re: [PATCH] qcom-scm: Include header
On Wed, Dec 26, 2018 at 10:06:19AM -0200, Fabio Estevam wrote:
> Since commit e6f6d63ed14c ("drm/msm: add headless gpu device for imx5")
> the DRM_MSM symbol can be selected by SOC_IMX5 causing the following
> error when building imx_v6_v7_defconfig:
>
> In file included from ../drivers/gpu/drm/msm/adreno/a5xx_gpu.c:17:0:
> ../include/linux/qcom_scm.h: In function 'qcom_scm_set_cold_boot_addr':
> ../include/linux/qcom_scm.h:73:10: error: 'ENODEV' undeclared (first use in
> this function)
> return -ENODEV;
>
> Include the header file to fix this problem.
>
> Reported-by: kernelci.org bot
> Fixes: e6f6d63ed14c ("drm/msm: add headless gpu device for imx5")
> Signed-off-by: Fabio Estevam
> Reviewed-by: Bjorn Andersson
Tested-by: Guenter Roeck
Still broken upstream, and the commit window is about to close.
Any chance to send this patch upstream anytime soon ?
Guenter
> ---
> include/linux/qcom_scm.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 06996ad4f2bc..ce5a476fd733 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -13,6 +13,7 @@
> #ifndef __QCOM_SCM_H
> #define __QCOM_SCM_H
>
> +#include
> #include
> #include
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line, option
Hi Peter,
On Mon, Jan 7, 2019 at 9:26 AM Peter Rosin wrote:
> On 2019-01-06 10:33, Geert Uytterhoeven wrote:
> > On Mon, Nov 26, 2018 at 10:59 PM Peter Rosin wrote:
> >> If there are extra logos (CONFIG_FB_LOGO_EXTRA) the heights of these
> >> extra logos are not considered when centering the first logo vertically.
> >>
> >> Signed-off-by: Peter Rosin
> >
> >> --- a/drivers/video/logo/Kconfig
> >> +++ b/drivers/video/logo/Kconfig
> >> @@ -10,6 +10,15 @@ menuconfig LOGO
> >>
> >> if LOGO
> >>
> >> +config FB_LOGO_CENTER
> >> + bool "Center the logo"
> >> + depends on FB=y
> >> + help
> >> + When this option is selected, the bootup logo is centered both
> >> + horizontally and vertically. If more than one logo is displayed
> >> + due to multiple CPUs, the collected line of logos is centered
> >> + as a whole.
> >> +
> >
> > Isn't a kernel command line option more suitable to configure the position
> > of the logo?
>
> That didn't occur to me previously, but it does make sense now that you
> mention it. This is on top of v5.0-rc1, and if applied before v5.0 we
> can avoid possible regressions for folks who might start to rely on
> CONFIG_FB_LOGO_CENTER if v5.0 is released w/o this.
>
> Cheers,
> Peter
>
> From de7353ab519ba9b5c9ea3f62d607bb8e94b687cc Mon Sep 17 00:00:00 2001
> From: Peter Rosin
> Cc: Bartlomiej Zolnierkiewicz
> Cc: Jonathan Corbet
> Cc: Peter Rosin
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-fb...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> To: linux-ker...@vger.kernel.org
> Date: Mon, 7 Jan 2019 08:35:26 +0100
> Subject: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line
> option
>
> A command line option is much more flexible than a config option and
> the supporting code is small. Gets rid of #ifdefs in the code too...
>
> Suggested-by: Geert Uytterhoeven
> Signed-off-by: Peter Rosin
Thanks for your patch!
> --- a/Documentation/fb/fbcon.txt
> +++ b/Documentation/fb/fbcon.txt
> @@ -163,6 +163,12 @@ C. Boot options
> be preserved until there actually is some text is output to the
> console.
> This option causes fbcon to bind immediately to the fbdev device.
>
> +7. fbcon=center-logo
> +
> + When this option is selected, the bootup logo is centered both
> + horizontally and vertically. If more than one logo is displayed due to
> + multiple CPUs, the collected line of logos is centered as a whole.
> +
What about making this more generic, than an option specific for centering?
Else people want fbcon=left-logo or fbcon=right-logo soon?
fbcon=logo-pos:
With being "center", "left", "top", "right", "bottom", and combinations
like "top,center"? Or is that complicating stuff too much?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: fix some off by one bugs
Hi Daniel,
Am Montag, den 24.12.2018, 10:32 +0100 schrieb Daniel Vetter:
> On Fri, Dec 21, 2018 at 9:24 PM Dan Carpenter om> wrote:
> >
> > I don't think anyone responded to this one?
>
> Maybe time to move etnaviv into drm-misc so that there's a notch more
> redundancy in maintainers? Lucas, Christian, others?
Sorry, but no thanks. The current model guarantees that we have at
least some testing of the patches flowing through the etnaviv tree with
realworld use-cases. We certainly don't have the resources to track a
rapidly changing target like drm-misc with our testing.
Regards,
Lucas
> -Daniel
>
> >
> > regards,
> > dan carpenter
> >
> > On Fri, Jul 13, 2018 at 06:00:18PM +0300, Dan Carpenter wrote:
> > > The ->nr_signal is the supposed to be the number of elements in
> > > the
> > > ->signal array. There was one place where it was 5 but it was
> > > supposed
> > > to be 4. That looks like a copy and paste bug. There were also
> > > two
> > > checks that were off by one.
> > >
> > > Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query
> > > perf counter")
> > > Signed-off-by: Dan Carpenter
> > > ---
> > > Not tested.
> > >
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > index 9980d81a26e3..4227a4006c34 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > @@ -113,7 +113,7 @@ static const struct etnaviv_pm_domain
> > > doms_3d[] = {
> > > .name = "PE",
> > > .profile_read = VIVS_MC_PROFILE_PE_READ,
> > > .profile_config = VIVS_MC_PROFILE_CONFIG0,
> > > - .nr_signals = 5,
> > > + .nr_signals = 4,
> > > .signal = (const struct etnaviv_pm_signal[]) {
> > > {
> > > "PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
> > > @@ -435,7 +435,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu
> > > *gpu,
> > >
> > > dom = meta->domains + signal->domain;
> > >
> > > - if (signal->iter > dom->nr_signals)
> > > + if (signal->iter >= dom->nr_signals)
> > > return -EINVAL;
> > >
> > > sig = &dom->signal[signal->iter];
> > > @@ -461,7 +461,7 @@ int etnaviv_pm_req_validate(const struct
> > > drm_etnaviv_gem_submit_pmr *r,
> > >
> > > dom = meta->domains + r->domain;
> > >
> > > - if (r->signal > dom->nr_signals)
> > > + if (r->signal >= dom->nr_signals)
> > > return -EINVAL;
> > >
> > > return 0;
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109161] Kernel crash shortly after gnome-shell login - refcount_t: increment on 0; use-after-free
https://bugs.freedesktop.org/show_bug.cgi?id=109161 --- Comment #5 from mikhail.v.gavri...@gmail.com --- Looks like I hurried to report that all is well. With prolonged working, hangs still happens. 1) [24501.462105] general protection fault: [#1] SMP NOPTI [24501.462112] CPU: 0 PID: 2147 Comm: gnome-shell Tainted: GWC 4.21.0-0.rc0.git7.2.fc30.x86_64 #1 [24501.462115] Hardware name: System manufacturer System Product Name/ROG STRIX X470-I GAMING, BIOS 1103 11/16/2018 [24501.462122] RIP: 0010:__memcpy+0x12/0x20 [24501.462125] Code: ff 0f 31 48 c1 e2 20 48 09 c2 48 31 d3 e9 79 ff ff ff 90 90 90 90 90 90 0f 1f 44 00 00 48 89 f8 48 89 d1 48 c1 e9 03 83 e2 07 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48 89 d1 f3 a4 [24501.462128] RSP: 0018:ba514d57fbc8 EFLAGS: 00010246 [24501.462131] RAX: 47ff9014d115ff00 RBX: d4745000 RCX: 0200 [24501.462134] RDX: RSI: 8de714745000 RDI: 47ff9014d115ff00 [24501.462136] RBP: 8dee37f1d0b0 R08: 8de64000 R09: [24501.462138] R10: R11: R12: 1000 [24501.462140] R13: R14: 8dee37f1d0b0 R15: 0008 [24501.462143] FS: 7fe6b56dad00() GS:8dee3cc0() knlGS: [24501.462146] CS: 0010 DS: ES: CR0: 80050033 [24501.462148] CR2: 7fe6b429f000 CR3: 00077fef2000 CR4: 003406f0 [24501.462150] Call Trace: [24501.462156] dma_direct_unmap_page+0x7a/0x80 [24501.462165] ttm_unmap_and_unpopulate_pages+0x10f/0x120 [ttm] [24501.462173] ttm_tt_destroy.part.12+0x49/0x50 [ttm] [24501.462179] ttm_bo_cleanup_memtype_use+0x2e/0x70 [ttm] [24501.462186] ttm_bo_put+0x2bf/0x3f0 [ttm] [24501.462247] amdgpu_bo_unref+0x1a/0x30 [amdgpu] [24501.462301] amdgpu_gem_object_free+0x33/0x50 [amdgpu] [24501.462315] drm_gem_object_release_handle+0x59/0xc0 [drm] [24501.462325] drm_gem_handle_delete+0x61/0x90 [drm] [24501.462336] ? drm_gem_handle_create+0x40/0x40 [drm] [24501.462346] drm_ioctl_kernel+0xa9/0xf0 [drm] [24501.462357] drm_ioctl+0x201/0x3a0 [drm] [24501.462367] ? drm_gem_handle_create+0x40/0x40 [drm] [24501.462370] ? sched_clock+0x5/0x10 [24501.462373] ? sched_clock_cpu+0xc/0xb0 [24501.462377] ? lockdep_hardirqs_on+0xed/0x180 [24501.462422] amdgpu_drm_ioctl+0x49/0x80 [amdgpu] [24501.462429] do_vfs_ioctl+0xa5/0x6f0 [24501.462434] ksys_ioctl+0x60/0x90 [24501.462438] __x64_sys_ioctl+0x16/0x20 [24501.462442] do_syscall_64+0x60/0x1f0 [24501.462445] entry_SYSCALL_64_after_hwframe+0x49/0xbe [24501.462448] RIP: 0033:0x7fe6b92342fb [24501.462451] Code: 0f 1e fa 48 8b 05 8d 9b 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 5d 9b 0c 00 f7 d8 64 89 01 48 [24501.462453] RSP: 002b:746ce9f8 EFLAGS: 0246 ORIG_RAX: 0010 [24501.462456] RAX: ffda RBX: 55605e605f70 RCX: 7fe6b92342fb [24501.462458] RDX: 746cea30 RSI: 40086409 RDI: 000d [24501.462461] RBP: 746cea30 R08: R09: 0007 [24501.462463] R10: 00ee R11: 0246 R12: 40086409 [24501.462465] R13: 000d R14: 55605c758b80 R15: 55605c75f910 [24501.462471] Modules linked in: vhost_net vhost macvtap macvlan tap nls_utf8 isofs fuse rfcomm xt_CHECKSUM ipt_MASQUERADE devlink tun bridge stp llc nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_nat ip6table_nat nf_nat_ipv6 ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat_ipv4 nf_nat iptable_mangle iptable_raw iptable_security nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables cmac bnep sunrpc xfs vfat fat libcrc32c edac_mce_amd kvm_amd kvm arc4 irqbypass uvcvideo videobuf2_vmalloc joydev snd_hda_codec_realtek videobuf2_memops videobuf2_v4l2 r8822be(C) snd_hda_codec_generic videobuf2_common crct10dif_pclmul snd_usb_audio ledtrig_audio crc32_pclmul eeepc_wmi snd_hda_codec_hdmi asus_wmi videodev snd_usbmidi_lib snd_hda_intel snd_rawmidi sparse_keymap video media ghash_clmulni_intel snd_hda_codec wmi_bmof mac80211 snd_hda_core snd_hwdep snd_seq [24501.462509] snd_seq_device btusb snd_pcm btrtl btbcm btintel bluetooth snd_timer cfg80211 sp5100_tco snd i2c_piix4 k10temp soundcore ecdh_generic ccp rfkill pcc_cpufreq gpio_amdpt gpio_generic acpi_cpufreq binfmt_misc hid_logitech_hidpp amdgpu hid_logitech_dj chash amd_iommu_v2 gpu_sched ttm drm_kms_helper drm crc32c_intel igb nvme dca i2c_algo_bit nvme_core wmi pinctrl_amd uas usb_storage hid_sony ff_memless [24501.462539] ---[ end trace 4adc8adf49e40b60 ]--- [24501.462545] RIP: 0010:__memcpy+0x12/0x20 [24501.462548] Code: ff 0f 31 48 c1 e2 20 48 09 c2 48 31 d3 e9 79 ff ff ff 90 90 90 90 90 90 0f 1f 44 00 00 48 89 f8 48 89 d1 48 c1 e9 03 83 e2 07 48 a5 89
Re: [PATCH 02/10] drm/etnaviv: mmuv2: don't map zero page
Hi Guido, Am Sonntag, den 30.12.2018, 16:49 +0100 schrieb Guido Günther: > Hi Lucas, > On Wed, Dec 19, 2018 at 03:45:38PM +0100, Lucas Stach wrote: > > Keep the page at address 0 as faulting to catch any potential state > > setup issues early. > > This is a nice idea! But applying this and making mesa hit that page > leads to the process hanging in D state over here on GC7000: > > # [ 242.726192] INFO: task kworker/u8:2:37 blocked for more than 120 seconds. > [ 242.733010] Not tainted 4.18.0-00129-gce2b21074b41 #504 > [ 242.738795] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > this message. > [ 242.746638] kworker/u8:2D037 2 0x0028 > [ 242.752144] Workqueue: events_unbound commit_work > [ 242.756860] Call trace: > [ 242.759318] __switch_to+0x94/0xd0 > [ 242.762741] __schedule+0x1c0/0x6b8 > [ 242.766239] schedule+0x40/0xa8 > [ 242.769380] schedule_timeout+0x2f0/0x428 > [ 242.773410] dma_fence_default_wait+0x1cc/0x2b8 > [ 242.777951] dma_fence_wait_timeout+0x44/0x1b0 > [ 242.782403] drm_atomic_helper_wait_for_fences+0x48/0x108 > [ 242.787819] commit_tail+0x30/0x80 > [ 242.791229] commit_work+0x20/0x30 > [ 242.794642] process_one_work+0x1ec/0x458 > [ 242.798659] worker_thread+0x48/0x430 > [ 242.802331] kthread+0x130/0x138 > [ 242.805557] ret_from_fork+0x10/0x1c > > This is in dmesg showing that we hit the first page: > > [ 65.907388] etnaviv-gpu 3800.gpu: MMU fault status 0x0002 > [ 65.913497] etnaviv-gpu 3800.gpu: MMU 0 fault addr 0x0e40 > > Without that patch it's sampling random data from that page but does not hang. GPU hangs after a MMU fault are expected or more accurately, we actively request the GPU to stop by setting the exception bit in the page table. A hanging GPU should trigger the scheduler timeout handler, which then makes sure to get the GPU back into a working state. So if things don't progress after the fault for you either the timeout handler is buggy on GC7000, or the fence signaling is broken somehow. I'll take a look at this. Regards, Lucas > Cheers, > -- Guido > > > > > > > Signed-off-by: Lucas Stach > > --- > > drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c > > b/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c > > index f1c88d8ad5ba..f794e04be9e6 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c > > @@ -320,8 +320,8 @@ etnaviv_iommuv2_domain_alloc(struct etnaviv_gpu *gpu) > > > > domain = &etnaviv_domain->base; > > > > > > domain->dev = gpu->dev; > > > > - domain->base = 0; > > > > - domain->size = (u64)SZ_1G * 4; > > > > + domain->base = SZ_4K; > > > > + domain->size = (u64)SZ_1G * 4 - SZ_4K; > > > > domain->ops = &etnaviv_iommuv2_ops; > > > > > > ret = etnaviv_iommuv2_init(etnaviv_domain); > > -- > > 2.19.1 > > > > ___ > > etnaviv mailing list > > etna...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/etnaviv ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd line, option
Hi Peter,
On Mon, Jan 7, 2019 at 10:03 AM Peter Rosin wrote:
> On 2019-01-07 09:59, Peter Rosin wrote:
> > On 2019-01-07 09:40, Geert Uytterhoeven wrote:
> >> On Mon, Jan 7, 2019 at 9:26 AM Peter Rosin wrote:
> >>> On 2019-01-06 10:33, Geert Uytterhoeven wrote:
> On Mon, Nov 26, 2018 at 10:59 PM Peter Rosin wrote:
> > If there are extra logos (CONFIG_FB_LOGO_EXTRA) the heights of these
> > extra logos are not considered when centering the first logo vertically.
> >
> > Signed-off-by: Peter Rosin
>
> > --- a/drivers/video/logo/Kconfig
> > +++ b/drivers/video/logo/Kconfig
> > @@ -10,6 +10,15 @@ menuconfig LOGO
> >
> > if LOGO
> >
> > +config FB_LOGO_CENTER
> > + bool "Center the logo"
> > + depends on FB=y
> > + help
> > + When this option is selected, the bootup logo is centered both
> > + horizontally and vertically. If more than one logo is
> > displayed
> > + due to multiple CPUs, the collected line of logos is centered
> > + as a whole.
> > +
>
> Isn't a kernel command line option more suitable to configure the
> position
> of the logo?
> >>>
> >>> That didn't occur to me previously, but it does make sense now that you
> >>> mention it. This is on top of v5.0-rc1, and if applied before v5.0 we
> >>> can avoid possible regressions for folks who might start to rely on
> >>> CONFIG_FB_LOGO_CENTER if v5.0 is released w/o this.
> >>>
> >>> Cheers,
> >>> Peter
> >>>
> >>> From de7353ab519ba9b5c9ea3f62d607bb8e94b687cc Mon Sep 17 00:00:00 2001
> >>> From: Peter Rosin
> >>> Cc: Bartlomiej Zolnierkiewicz
> >>> Cc: Jonathan Corbet
> >>> Cc: Peter Rosin
> >>> Cc: dri-devel@lists.freedesktop.org
> >>> Cc: linux-fb...@vger.kernel.org
> >>> Cc: linux-...@vger.kernel.org
> >>> To: linux-ker...@vger.kernel.org
> >>> Date: Mon, 7 Jan 2019 08:35:26 +0100
> >>> Subject: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd
> >>> line
> >>> option
> >>>
> >>> A command line option is much more flexible than a config option and
> >>> the supporting code is small. Gets rid of #ifdefs in the code too...
> >>>
> >>> Suggested-by: Geert Uytterhoeven
> >>> Signed-off-by: Peter Rosin
> >>
> >> Thanks for your patch!
> >>
> >>> --- a/Documentation/fb/fbcon.txt
> >>> +++ b/Documentation/fb/fbcon.txt
> >>> @@ -163,6 +163,12 @@ C. Boot options
> >>> be preserved until there actually is some text is output to the
> >>> console.
> >>> This option causes fbcon to bind immediately to the fbdev device.
> >>>
> >>> +7. fbcon=center-logo
> >>> +
> >>> + When this option is selected, the bootup logo is centered both
> >>> + horizontally and vertically. If more than one logo is displayed
> >>> due to
> >>> + multiple CPUs, the collected line of logos is centered as a whole.
> >>> +
> >>
> >> What about making this more generic, than an option specific for centering?
> >> Else people want fbcon=left-logo or fbcon=right-logo soon?
> >>
> >> fbcon=logo-pos:
> >>
> >> With being "center", "left", "top", "right", "bottom", and
> >> combinations
> >> like "top,center"? Or is that complicating stuff too much?
> >
> > I'm not too keen on implementing all that when I have zero use for it. I can
> > however rename the trigger for the existing fb_center_logo bool to
> >
> > fbcon=logo-pos:center
> >
> > and add a check for "top,left" to reset the bool to false. Then the other
> > variants can be added later by whomever and when needed.
> >
> > Is that good enough?
>
> Ouch, I just realized that using a comma is a no-go, as that is already
> separating fbcon suboptions, so I suggest top-left instead.
Sure, sounds fine to me. I just want to avoid having a parameter system
that complicates future extension.
I think you can drop the check for top-left, as the bool defaults to false.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/10] drm/etnaviv: mmuv2: don't map zero page
Hi, On Mon, Jan 07, 2019 at 09:50:52AM +0100, Lucas Stach wrote: > Hi Guido, > > Am Sonntag, den 30.12.2018, 16:49 +0100 schrieb Guido Günther: > > Hi Lucas, > > On Wed, Dec 19, 2018 at 03:45:38PM +0100, Lucas Stach wrote: > > > Keep the page at address 0 as faulting to catch any potential state > > > setup issues early. > > > > This is a nice idea! But applying this and making mesa hit that page > > leads to the process hanging in D state over here on GC7000: > > > > # [ 242.726192] INFO: task kworker/u8:2:37 blocked for more than 120 > > seconds. > > [ 242.733010] Not tainted 4.18.0-00129-gce2b21074b41 #504 > > [ 242.738795] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > > this message. > > [ 242.746638] kworker/u8:2D037 2 0x0028 > > [ 242.752144] Workqueue: events_unbound commit_work > > [ 242.756860] Call trace: > > [ 242.759318] __switch_to+0x94/0xd0 > > [ 242.762741] __schedule+0x1c0/0x6b8 > > [ 242.766239] schedule+0x40/0xa8 > > [ 242.769380] schedule_timeout+0x2f0/0x428 > > [ 242.773410] dma_fence_default_wait+0x1cc/0x2b8 > > [ 242.777951] dma_fence_wait_timeout+0x44/0x1b0 > > [ 242.782403] drm_atomic_helper_wait_for_fences+0x48/0x108 > > [ 242.787819] commit_tail+0x30/0x80 > > [ 242.791229] commit_work+0x20/0x30 > > [ 242.794642] process_one_work+0x1ec/0x458 > > [ 242.798659] worker_thread+0x48/0x430 > > [ 242.802331] kthread+0x130/0x138 > > [ 242.805557] ret_from_fork+0x10/0x1c > > > > This is in dmesg showing that we hit the first page: > > > > [ 65.907388] etnaviv-gpu 3800.gpu: MMU fault status 0x0002 > > [ 65.913497] etnaviv-gpu 3800.gpu: MMU 0 fault addr 0x0e40 > > > > Without that patch it's sampling random data from that page but does not > > hang. > > GPU hangs after a MMU fault are expected or more accurately, we > actively request the GPU to stop by setting the exception bit in the > page table. Yeah. I put that in to show that this the cause for the trouble above. > > A hanging GPU should trigger the scheduler timeout handler, which then > makes sure to get the GPU back into a working state. So if things don't > progress after the fault for you either the timeout handler is buggy on > GC7000, or the fence signaling is broken somehow. I'll take a look at > this. This isn't a top notch linux-next based tree yet so if you're not seeing this let me forward port our stuff to that and report back again. Cheers, -- Guido ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Block fb changes for async plane updates
On Wed, Jan 02, 2019 at 03:02:04PM +, Kazlauskas, Nicholas wrote: > On 12/24/18 7:15 AM, Daniel Vetter wrote: > > On Fri, Dec 21, 2018 at 09:33:24AM -0500, Nicholas Kazlauskas wrote: > >> The behavior of drm_atomic_helper_cleanup_planes differs depending on > >> whether the commit was an asynchronous update or not. > >> > >> For a typical (non-async) atomic commit prepare_fb is called on the > >> new_plane_state and cleanup_fb is called on the old_plane_state. > >> > >> However, async commits are performed in place and don't swap the state > >> for the plane. The call to prepare_fb happens on the new_plane_state > >> and the call to cleanup_fb is also called on the new_plane_state in > >> this case (since the state hasn't swapped). > >> > >> This behavior can lead to use-after-free or unpin of an active fb. > >> > >> Consider the following sequence of events for interleaving fbs: > >> > >> - Async update, fb1 prepare, fb1 cleanup_fb > >> - Async update, fb2 prepare, fb2 cleanup_fb > >> - Non-async update, fb1 prepare, fb2 cleanup_fb > >> - Async update, fb2 cleanup_fb -> double cleanup, use-after-free > > > > I think I see your bug, but I'm completely lost in your description above. > > > > I think this is ok as a short-term gap, but imo better if it's a separate > > if condition with a FIXME comment. > > > > Long-term we want to fix this, and I think simplest way to do that is if > > we expect drivers to store the old fb in the new_plane_state (and check > > that with a WARN_ON like the others). I think that should work. > > > > We probably also need some locking on top, to prevent races with the > > cleanup_fb calls done by non-blocking commits, to make sure those clean up > > the right fb. > > -Daniel > > Hi Daniel, > > The description is supposed to be saying that the wrong fb is being > cleaned up because in state updates to the plane don't swap the state > pointer - which is required by the cleanup planes helper function. Yeah I inferred that, but the actual commit message isn't really clear on this, and the example you've listed only confused me. We need good commit messages, so that half a year down the road we still know what exactly we've been thinking :-) Especially in an area that's really tricky, like synchronization of the nonblocking atomic updates against normal updates (and each another), which has seen plenty of bugs in the past. > But putting that aside, I think what you suggest here would work best > for "fixing" the problem. Storing the old fb pointer in the new state > looks kind of odd, but as long as it's documented and there's the > WARN_ON in the helper I think it's fine. I think I would prefer this > over a solution that blocks fb changes in the helper altogether. > > I don't think any additional drm level locking is necessary here. The > race with cleanup_fb calls is already something you have to worry about > when dealing with async commits even if the fb hasn't changed. Most > drivers have their own internal locks or ref-counting that can handle this. > > So to summarize, I think I can post a new patch series that addresses > this problem by fixing existing async commit usage in vc4 and amdgpu for > framebuffer changes. The last patch in the series could be the one that > adds a WARN_ON in the drm_atomic_helper_async_commit helper along with a > comment indicating that the old fb should be in the new plane state. > > Does this sound reasonable to you? Let's please do the the minimal bugfixe first (probably cc: stable), and figure out how to properly fix things up long-term. -Daniel > > Nicholas Kazlauskas > > > > >> This situation has been observed in practice for a double buffered > >> cursor when closing an X client. The non-async update occurs because > >> the new_plane_state->crtc != old_plane_state->crtc which forces the > >> non-async path to occur. > >> > >> The simplest fix for this is to block fb updates in > >> drm_atomic_helper_async_check. This guarantees that the framebuffer > >> will have previously been prepared and any subsequent async updates > >> will always call prepare and cleanup_fb like the non-async atomic > >> commit path would. > >> > >> Cc: Michel Dänzer > >> Cc: Daniel Vetter > >> Cc: Andrey Grodzovsky > >> Cc: Harry Wentland > >> Signed-off-by: Nicholas Kazlauskas > >> --- > >> drivers/gpu/drm/drm_atomic_helper.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c > >> b/drivers/gpu/drm/drm_atomic_helper.c > >> index 54e2ae614dcc..d2f80bf14f86 100644 > >> --- a/drivers/gpu/drm/drm_atomic_helper.c > >> +++ b/drivers/gpu/drm/drm_atomic_helper.c > >> @@ -1599,7 +1599,8 @@ int drm_atomic_helper_async_check(struct drm_device > >> *dev, > >>return -EINVAL; > >> > >>if (!new_plane_state->crtc || > >> - old_plane_state->crtc != new_plane_state->crtc) > >> + old_plane_state->crtc != new_plane_state->crtc || > >> + o
Re: [PATCH v5 1/2] drm/amd: validate user pitch alignment
On 2019-01-07 5:00 a.m., Yu Zhao wrote:
> On Thu, Jan 03, 2019 at 05:33:19PM +0100, Michel Dänzer wrote:
>> On 2018-12-30 2:00 a.m., Yu Zhao wrote:
>>> Userspace may request pitch alignment that is not supported by GPU.
>>> Some requests 32, but GPU ignores it and uses default 64 when cpp is
>>> 4. If GEM object is allocated based on the smaller alignment, GPU
>>> DMA will go out of bound.
>>>
>>> Cc: sta...@vger.kernel.org # v4.2+
>>> Signed-off-by: Yu Zhao
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> index 15ce7e681d67..16af80ccd0a0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> @@ -527,6 +527,22 @@ amdgpu_display_user_framebuffer_create(struct
>>> drm_device *dev,
>>> struct drm_gem_object *obj;
>>> struct amdgpu_framebuffer *amdgpu_fb;
>>> int ret;
>>> + struct amdgpu_device *adev = dev->dev_private;
>>> + int cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
>>> + int pitch = mode_cmd->pitches[0] / cpp;
>>> +
>>> + if (pitch < mode_cmd->width) {
>>> + DRM_DEBUG_KMS("Expecting pitch(%d)/cpp(%d) >= width(%d)\n",
>>> + mode_cmd->pitches[0], cpp, mode_cmd->width);
>>> + return ERR_PTR(-EINVAL);
>>> + }
>>
>> This should possibly be tested in drm_internal_framebuffer_create instead.
>
> It seems to me drm_format_info_min_pitch() in framebuffer_check()
> already does it. We could just remove the check here?
Yeah, looks like that should be fine.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop
On Thu, Jan 03, 2019 at 11:03:36AM +0100, Lubomir Rintel wrote: > Hi, > > On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote: > > On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel wrote: > > > Hi, > > > > > > here are patches that make the Armada DRM work on an OLPC XO-1.75. > > > They build on patches previously submitted by Russel King (included here > > > for > > > completeness as well). > > > > > > They certainly need some more love, but I'm not able to improve them until > > > I get to understand some things first. I'm posting a couple of questions > > > below in hope someone is kind enough to help me deal with my confusion. > > > > > > The display pipeline on the laptop looks like this: > > > > > > Armada LCDC > > > --- > > > | > > > v > > > > > > Himax HX8837 "DCON" > > > --- > > > RGB input from LCDC > > > Controlled via I2C > > > Backlight control > > > Can slow down the panel refresh rate to save power > > > Optional dithering for color mode, "swizzling" > > > Has DRAM attached, can freeze a single frame in it > > > Doc: http://wiki.laptop.org/images/0/09/DCON_datasheet_HX8837-A.pdf > > > | > > > v > > > > > > Innolux LS075AT011 Panel > > > > > > 7.5", 1200x900 > > > No datasheet. > > > http://wiki.laptop.org/go/Display > > > Can opreate in two modes: > > > > > >R G B > > >G B R ... or "e-book" daylight readable > > >B R Greflexive B&W > > > . > > > : > > > > > > Here's what's not clear to me: > > > > > > 1.) Is the Himax chip an encoder here? > > > > Since it's an external IP probably better to model it as a bridge. > > > > > 2.) What's the use of an encoder anyways? If a panel was connected > > > directly > > > do the RGB output from the LCDC, would we have to fake one? Is that > > > the > > > point of things like i.MX driver's > > > drivers/gpu/drm/imx/parallel-display.c? > > > > encoder is the thing between crtc and connector. It's exposed to > > userspace (which is a uapi design accident, but can't change that), > > hence why it's required and some drivers have dummy ones. > > > > The real usage is purely internally for the atomic helpers. Most > > callbacks in the encoder should be optional, so a dummy encoder should > > be a lot thinner than your imx example. The imx example should be > > using the panel-bridge I think, which would take care of most of the > > boilerplate (but panel-bridge didn't exist back then I guess). > > > > > 3.) My patch set currently contains the driver for the Himax that is > > > modelled after tda998x. That one implements an encoder. Similar > > > drivers > > > seem to add a bridge too, but it's not entirely clear to me what a > > > bridge > > > is good for? > > > > tda998x predates bridges, which is why there's still some > > encoder_helper usage in there. It's become a proper bridge driver now > > I think (but some drivers still use the old interface). All new > > transcoder chip drivers should be bridges. Did you look at the > > kerneldoc for bridges? If those aren't clear we need to fix them. > > > > > 4.) How shall I expose the fancy functionality of the Himax to the > > > userspace? > > > Notably, the freeze frame. The OLPC laptops with the stock firmware > > > like > > > to suspend the SoC very aggressively to save power (in 20 seconds of > > > inactivity or so). If the display is open (it can also be turned > > > around > > > for a tablet or e-book mode), it makes sense to freeze the picture and > > > keep the panel on, if the laptop is closed, we want to turn it off. > > > Should the behavior be exposed via sysfs (as it is with the current > > > OLPC > > > kernels), or a DRM property? Would it require libdrm support? > > > > I've accidentally seen how that's implemented for fbdev in the older > > olpc drivers, and that's definitely _not_ how we want to support this. > > Essentially what you have here is a fancy self-refresh panel. That's > > already fully supported by DRM uapi, so I'd say first implement that. > > Once we have that we can figure out a special property that tells the > > driver to not shut down the screen on suspend, but just go into > > self-refresh. Implementing the self-refresh stuff in DRM will be the > > big pile of work (since that's something which goes beyond the generic > > drm_bridge interface). > > > > > > > Himax HX8837 "DCON" > > > --- > > > RGB input from LCDC > > > Controlled via I2C > > > > These two shouldn't be a problem. > > > > > Backlight control > > > > We already have backlight interfaces > > > > > Can slow down the panel refresh rate to save power > > > > Just expose this as modes with different vrefresh (and do that > > modeswitch without doing a full modeset). Bonus if you do it > > automatically, which can be done with the self-refresh infrastructure. > > > >
Re: [PATCH] drm/etnaviv: fix some off by one bugs
On Mon, Jan 07, 2019 at 09:42:00AM +0100, Lucas Stach wrote:
> Hi Daniel,
>
> Am Montag, den 24.12.2018, 10:32 +0100 schrieb Daniel Vetter:
> > On Fri, Dec 21, 2018 at 9:24 PM Dan Carpenter > om> wrote:
> > >
> > > I don't think anyone responded to this one?
> >
> > Maybe time to move etnaviv into drm-misc so that there's a notch more
> > redundancy in maintainers? Lucas, Christian, others?
>
> Sorry, but no thanks. The current model guarantees that we have at
> least some testing of the patches flowing through the etnaviv tree with
> realworld use-cases. We certainly don't have the resources to track a
> rapidly changing target like drm-misc with our testing.
Not following here exactly, whether you do your own tree or reuse some
existing infrastructure, upstream moves as fast as it does no matter how
things work. And you can still do all the usual testing before pushing.
And I just brought this up because the separate tree also doesn't seem to
be entirely perfectly managed either. You wouldn't get me nagging if
the etnaviv pull wouldn't have raised questions :-)
-Daniel
> Regards,
> Lucas
>
> > -Daniel
> >
> > >
> > > regards,
> > > dan carpenter
> > >
> > > On Fri, Jul 13, 2018 at 06:00:18PM +0300, Dan Carpenter wrote:
> > > > The ->nr_signal is the supposed to be the number of elements in
> > > > the
> > > > ->signal array. There was one place where it was 5 but it was
> > > > supposed
> > > > to be 4. That looks like a copy and paste bug. There were also
> > > > two
> > > > checks that were off by one.
> > > >
> > > > Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query
> > > > perf counter")
> > > > Signed-off-by: Dan Carpenter
> > > > ---
> > > > Not tested.
> > > >
> > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > index 9980d81a26e3..4227a4006c34 100644
> > > > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > @@ -113,7 +113,7 @@ static const struct etnaviv_pm_domain
> > > > doms_3d[] = {
> > > > .name = "PE",
> > > > .profile_read = VIVS_MC_PROFILE_PE_READ,
> > > > .profile_config = VIVS_MC_PROFILE_CONFIG0,
> > > > - .nr_signals = 5,
> > > > + .nr_signals = 4,
> > > > .signal = (const struct etnaviv_pm_signal[]) {
> > > > {
> > > > "PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
> > > > @@ -435,7 +435,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu
> > > > *gpu,
> > > >
> > > > dom = meta->domains + signal->domain;
> > > >
> > > > - if (signal->iter > dom->nr_signals)
> > > > + if (signal->iter >= dom->nr_signals)
> > > > return -EINVAL;
> > > >
> > > > sig = &dom->signal[signal->iter];
> > > > @@ -461,7 +461,7 @@ int etnaviv_pm_req_validate(const struct
> > > > drm_etnaviv_gem_submit_pmr *r,
> > > >
> > > > dom = meta->domains + r->domain;
> > > >
> > > > - if (r->signal > dom->nr_signals)
> > > > + if (r->signal >= dom->nr_signals)
> > > > return -EINVAL;
> > > >
> > > > return 0;
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Expose more EDID fields to userspace
On Mon, Dec 31, 2018 at 12:57:24PM +, Simon Ser wrote:
> On Monday, December 24, 2018 11:23 AM, Daniel Vetter wrote:
> > On Sun, Dec 23, 2018 at 09:16:13AM +, Simon Ser wrote:
> >
> > > Hi all,
> > > Right now, the kernel parses EDIDs and exposes some of the data to
> > > userspace. For instance, drmModeConnector has mm{Width,Height} and
> > > subpixel.
> > > Generally, userspace also has another EDID parser. For instance,
> > > wlroots uses it just to get the make/model/serial. I've talked about
> > > this at XDC 2018, and someone mentioned it could be a good idea to
> > > de-duplicate the work.
> >
> > Could have been me ...
> >
> > > Would it be reasonable to expose those as DRM connector properties?
> >
> > Yes, very much. Aside from the kernel-side implementation all we need is
> > the userspace per
> >
> > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
> >
> > Important: Don't merge the userspace side into your main branch before
> > it's all reviewed.
> >
> > And ideally some igts (we have infrastructure to inject edids and hence
> > can check that the right stuff comes back):
> >
> > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#testing-and-validation
>
> Thanks! I've began to implement this, but I'm wondering what's the best
> way to expose this piece of information to userspace.
>
> I can think of two solutions:
>
> 1. A single IDENTIFICATION blob containing one struct with
>make/serial/model.
> 2. Three properties: MAKE, MODEL, SERIAL.
>
> I also wonder if it's worth it to expose both the uint32_t for serial
> and the serial string, or just the serial string. Some monitors don't
> have a serial string, but we can generate one from the uint32_t (e.g.
> hex). Same applies to model.
>
> Thoughts?
Best to pull in some other compositor people and get them to agree. From a
kernel pov I'm fine with whatever userspace preferes.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 1/2] drm/fb-helper: Bring back workaround for bugs of SDL 1.2
On Sat, Jan 05, 2019 at 09:11:53PM +0500, Ivan Mironov wrote:
> On Fri, 2018-12-28 at 13:15 +0100, Daniel Vetter wrote:
> > On Fri, Dec 28, 2018 at 04:13:07AM +0500, Ivan Mironov wrote:
> > > SDL 1.2 sets all fields related to the pixel format to zero in some
> > > cases[1]. Prior to commit db05c48197759 ("drm: fb-helper: Reject all
> > > pixel format changing requests"), there was an unintentional workaround
> > > for this that existed for more than a decade. First in device-specific DRM
> > > drivers, then here in drm_fb_helper.c.
> > >
> > > Previous code containing this workaround just ignores pixel format fields
> > > from userspace code. Not a good thing either, as this way, driver may
> > > silently use pixel format different from what client actually requested,
> > > and this in turn will lead to displaying garbage on the screen. I think
> > > that returning EINVAL to userspace in this particular case is the right
> > > option, so I decided to left code from problematic commit untouched
> > > instead of just reverting it entirely.
> > >
> > > Here is the steps required to reproduce this problem exactly:
> > > 1) Compile fceux[2] with SDL 1.2.15 and without GTK or OpenGL
> > > support. SDL should be compiled with fbdev support (which is
> > > on by default).
> > > 2) Create /etc/fb.modes with following contents (values seems
> > > not used, and just required to trigger problematic code in
> > > SDL):
> > >
> > > mode "test"
> > > geometry 1 1 1 1 1
> > > timings 1 1 1 1 1 1 1
> > > endmode
> > >
> > > 3) Create ~/.fceux/fceux.cfg with following contents:
> > >
> > > SDL.Hotkeys.Quit = 27
> > > SDL.DoubleBuffering = 1
> > >
> > > 4) Ensure that screen resolution is at least 1280x960 (e.g.
> > > append "video=Virtual-1:1280x960-32" to the kernel cmdline
> > > for qemu/QXL).
> > >
> > > 5) Try to run fceux on VT with some ROM file[3]:
> > >
> > > # ./fceux color_test.nes
> > >
> > > [1] SDL 1.2.15 source code, src/video/fbcon/SDL_fbvideo.c,
> > > FB_SetVideoMode()
> > > [2] http://www.fceux.com
> > > [3] Example ROM:
> > > https://github.com/bokuweb/rustynes/blob/master/roms/color_test.nes
> > >
> > > Reported-by: saahriktu
> > > Suggested-by: saahriktu
> > > Cc: sta...@vger.kernel.org
> > > Fixes: db05c48197759 ("drm: fb-helper: Reject all pixel format changing
> > > requests")
> > > Signed-off-by: Ivan Mironov
> > > ---
> > > drivers/gpu/drm/drm_fb_helper.c | 146
> > > 1 file changed, 93 insertions(+), 53 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > b/drivers/gpu/drm/drm_fb_helper.c
> > > index d3af098b0922..aff576c3c4fb 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -1621,6 +1621,64 @@ static bool drm_fb_pixel_format_equal(const struct
> > > fb_var_screeninfo *var_1,
> > > var_1->transp.msb_right == var_2->transp.msb_right;
> > > }
> > >
> > > +static void drm_fb_helper_fill_pixel_fmt(struct fb_var_screeninfo *var,
> > > + u8 depth)
> > > +{
> > > + switch (depth) {
> > > + case 8:
> > > + var->red.offset = 0;
> > > + var->green.offset = 0;
> > > + var->blue.offset = 0;
> > > + var->red.length = 8; /* 8bit DAC */
> > > + var->green.length = 8;
> > > + var->blue.length = 8;
> > > + var->transp.offset = 0;
> > > + var->transp.length = 0;
> > > + break;
> > > + case 15:
> > > + var->red.offset = 10;
> > > + var->green.offset = 5;
> > > + var->blue.offset = 0;
> > > + var->red.length = 5;
> > > + var->green.length = 5;
> > > + var->blue.length = 5;
> > > + var->transp.offset = 15;
> > > + var->transp.length = 1;
> > > + break;
> > > + case 16:
> > > + var->red.offset = 11;
> > > + var->green.offset = 5;
> > > + var->blue.offset = 0;
> > > + var->red.length = 5;
> > > + var->green.length = 6;
> > > + var->blue.length = 5;
> > > + var->transp.offset = 0;
> > > + break;
> > > + case 24:
> > > + var->red.offset = 16;
> > > + var->green.offset = 8;
> > > + var->blue.offset = 0;
> > > + var->red.length = 8;
> > > + var->green.length = 8;
> > > + var->blue.length = 8;
> > > + var->transp.offset = 0;
> > > + var->transp.length = 0;
> > > + break;
> > > + case 32:
> > > + var->red.offset = 16;
> > > + var->green.offset = 8;
> > > + var->blue.offset = 0;
> > > + var->red.length = 8;
> > > + var->green.length = 8;
> > > + var->blue.length = 8;
> > > + var->transp.offset = 24;
> > > + var->transp.length = 8;
> > > + break;
> > > + default:
> > > + break;
> > >
Re: [PATCH v1 2/2] drm/fb-helper: Ignore the value of fb_var_screeninfo.pixclock
On Sat, Jan 05, 2019 at 09:21:21PM +0500, Ivan Mironov wrote:
> On Fri, 2018-12-28 at 13:06 +0100, Daniel Vetter wrote:
> > On Fri, Dec 28, 2018 at 04:13:08AM +0500, Ivan Mironov wrote:
> > > Strict requirement of pixclock to be zero breaks support of SDL 1.2
> > > which contains hardcoded table of supported video modes with non-zero
> > > pixclock values[1].
> > >
> > > To better understand which pixclock values are considered valid and how
> > > driver should handle these values, I briefly examined few existing fbdev
> > > drivers and documentation in Documentation/fb/. And it looks like there
> > > are no strict rules on that and actual behaviour varies:
> > >
> > > * some drivers treat (pixclock == 0) as "use defaults" (uvesafb.c);
> > > * some treat (pixclock == 0) as invalid value which leads to
> > > -EINVAL (clps711x-fb.c);
> > > * some pass converted pixclock value to hardware (uvesafb.c);
> > > * some are trying to find nearest value from predefined table
> > > (vga16fb.c, video_gx.c).
> > >
> > > Given this, I believe that it should be safe to just ignore this value if
> > > changing is not supported. It seems that any portable fbdev application
> > > which was not written only for one specific device working under one
> > > specific kernel version should not rely on any particular behaviour of
> > > pixclock anyway.
> > >
> > > However, while enabling SDL1 applications to work out of the box when
> > > there is no /etc/fb.modes with valid settings, this change affects the
> > > video mode choosing logic in SDL. Depending on current screen
> > > resolution, contents of /etc/fb.modes and resolution requested by
> > > application, this may lead to user-visible difference (not always):
> > > image will be displayed in a right way, but it will be aligned to the
> > > left instead of center. There is no "right behaviour" here as well, as
> > > emulated fbdev, opposing to old fbdev drivers, simply ignores any
> > > requsts of video mode changes with resolutions smaller than current.
> > >
> > > Feel free to NAK this patch if you think that it causes breakage of
> > > user-space =).
> >
> > It's a regression, we don't nack regression fixes :-)
> >
> > > The easiest way to reproduce this problem is to install sdl-sopwith[2],
> > > remove /etc/fb.modes file if it exists, and then try to run sopwith
> > > from console without X. At least in Fedora 29, sopwith may be simply
> > > installed from standard repositories.
> > >
> > > [1] SDL 1.2.15 source code, src/video/fbcon/SDL_fbvideo.c, vesa_timings
> > > [2] http://sdl-sopwith.sourceforge.net/
> > >
> > > Signed-off-by: Ivan Mironov
> >
> > I thought this is also a regression fix, so also needs Fixes: and cc:
> > stable?
> > -Daniel
>
> This particular patch fixes a bug that existed for 10 years unnoticed.
> Are "Fixes:" and "Cc: stable" really required?
>
> "pixclock != 0" check was introduced in this file by 785b93ef8c309
> ("drm/kms: move driver specific fb common code to helper functions
> (v2)"). But actually similar code existed in the device-specific
> drivers even earlier.
Afaui 785b93ef8c309 broke existing userspace. Broken userspace, but
existing userspace, and hence we need to fix this regression. That's the
rules with linux upstream, existing userspace is never allowed to break,
even if it does really stupid things. Hence Cc: stable.
-Daniel
>
> >
> > > ---
> > > drivers/gpu/drm/drm_fb_helper.c | 7 ++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > b/drivers/gpu/drm/drm_fb_helper.c
> > > index aff576c3c4fb..b95a0c23c7c8 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -1690,9 +1690,14 @@ int drm_fb_helper_check_var(struct
> > > fb_var_screeninfo *var,
> > > struct drm_fb_helper *fb_helper = info->par;
> > > struct drm_framebuffer *fb = fb_helper->fb;
> > >
> > > - if (var->pixclock != 0 || in_dbg_master())
> > > + if (in_dbg_master())
> > > return -EINVAL;
> > >
> > > + if (var->pixclock != 0) {
> > > + DRM_DEBUG("fbdev emulation doesn't support changing the pixel
> > > clock, value of pixclock is ignored\n");
> > > + var->pixclock = 0;
> > > + }
> > > +
> > > if ((drm_format_info_block_width(fb->format, 0) > 1) ||
> > > (drm_format_info_block_height(fb->format, 0) > 1))
> > > return -EINVAL;
> > > --
> > > 2.20.1
> > >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/fb-helper: Ignore the value of fb_var_screeninfo.pixclock
On Sat, Jan 05, 2019 at 09:21:21PM +0500, Ivan Mironov wrote: > On Fri, 2018-12-28 at 13:06 +0100, Daniel Vetter wrote: > > On Fri, Dec 28, 2018 at 04:13:08AM +0500, Ivan Mironov wrote: > > > Strict requirement of pixclock to be zero breaks support of SDL 1.2 > > > which contains hardcoded table of supported video modes with non-zero > > > pixclock values[1]. > > > > > > To better understand which pixclock values are considered valid and how > > > driver should handle these values, I briefly examined few existing fbdev > > > drivers and documentation in Documentation/fb/. And it looks like there > > > are no strict rules on that and actual behaviour varies: > > > > > > * some drivers treat (pixclock == 0) as "use defaults" (uvesafb.c); > > > * some treat (pixclock == 0) as invalid value which leads to > > > -EINVAL (clps711x-fb.c); > > > * some pass converted pixclock value to hardware (uvesafb.c); > > > * some are trying to find nearest value from predefined table > > > (vga16fb.c, video_gx.c). > > > > > > Given this, I believe that it should be safe to just ignore this value if > > > changing is not supported. It seems that any portable fbdev application > > > which was not written only for one specific device working under one > > > specific kernel version should not rely on any particular behaviour of > > > pixclock anyway. > > > > > > However, while enabling SDL1 applications to work out of the box when > > > there is no /etc/fb.modes with valid settings, this change affects the > > > video mode choosing logic in SDL. Depending on current screen > > > resolution, contents of /etc/fb.modes and resolution requested by > > > application, this may lead to user-visible difference (not always): > > > image will be displayed in a right way, but it will be aligned to the > > > left instead of center. There is no "right behaviour" here as well, as > > > emulated fbdev, opposing to old fbdev drivers, simply ignores any > > > requsts of video mode changes with resolutions smaller than current. > > > > > > Feel free to NAK this patch if you think that it causes breakage of > > > user-space =). > > > > It's a regression, we don't nack regression fixes :-) > > > > > The easiest way to reproduce this problem is to install sdl-sopwith[2], > > > remove /etc/fb.modes file if it exists, and then try to run sopwith > > > from console without X. At least in Fedora 29, sopwith may be simply > > > installed from standard repositories. > > > > > > [1] SDL 1.2.15 source code, src/video/fbcon/SDL_fbvideo.c, vesa_timings > > > [2] http://sdl-sopwith.sourceforge.net/ > > > > > > Signed-off-by: Ivan Mironov > > > > I thought this is also a regression fix, so also needs Fixes: and cc: > > stable? > > -Daniel > > This particular patch fixes a bug that existed for 10 years unnoticed. > Are "Fixes:" and "Cc: stable" really required? If you want it backported into a stable kernel release, then yes, they are needed :) thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/9] drm/fb-helper: Add generic fbdev emulation .fb_probe function
On Thu, Jan 03, 2019 at 06:06:53PM +0100, Noralf Trønnes wrote:
>
>
> Den 28.12.2018 21.38, skrev Daniel Vetter:
> > On Tue, May 29, 2018 at 9:54 AM Daniel Vetter wrote:
> > >
> > > On Fri, May 25, 2018 at 02:42:02PM +0200, Noralf Trønnes wrote:
> > > >
> > > > Den 24.05.2018 11.16, skrev Daniel Vetter:
> > > > > On Wed, May 23, 2018 at 04:34:07PM +0200, Noralf Trønnes wrote:
> > > > > > This is the first step in getting generic fbdev emulation.
> > > > > > A drm_fb_helper_funcs.fb_probe function is added which uses the
> > > > > > DRM client API to get a framebuffer backed by a dumb buffer.
> > > > > >
> > > > > > A transitional hack for tinydrm is needed in order to switch over
> > > > > > all
> > > > > > CMA helper drivers in a later patch. This hack will be removed when
> > > > > > tinydrm moves to vmalloc buffers.
> > > > > >
> > > > > > Signed-off-by: Noralf Trønnes
> > > > > > ---
> > > > > >drivers/gpu/drm/drm_fb_helper.c | 164
> > > > > >
> > > > > >include/drm/drm_fb_helper.h | 26 +++
> > > > > >2 files changed, 190 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > > > > b/drivers/gpu/drm/drm_fb_helper.c
> > > > > > index 2ee1eaa66188..444c2b4040ea 100644
> > > > > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > > > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > > > > @@ -30,11 +30,13 @@
> > > > > >#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > > >#include
> > > > > > +#include
> > > > > >#include
> > > > > >#include
> > > > > >#include
> > > > > >#include
> > > > > >#include
> > > > > > +#include
> > > > > >#include
> > > > > >#include
> > > > > >#include
> > > > > > @@ -2928,6 +2930,168 @@ void
> > > > > > drm_fb_helper_output_poll_changed(struct drm_device *dev)
> > > > > >}
> > > > > >EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
> > > > > > +/* @user: 1=userspace, 0=fbcon */
> > > > > > +static int drm_fbdev_fb_open(struct fb_info *info, int user)
> > > > > > +{
> > > > > > + struct drm_fb_helper *fb_helper = info->par;
> > > > > > +
> > > > > > + if (!try_module_get(fb_helper->dev->driver->fops->owner))
> > > > > > + return -ENODEV;
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int drm_fbdev_fb_release(struct fb_info *info, int user)
> > > > > > +{
> > > > > > + struct drm_fb_helper *fb_helper = info->par;
> > > > > > +
> > > > > > + module_put(fb_helper->dev->driver->fops->owner);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > Hm, I thought earlier versions of your patch series had this
> > > > > separately,
> > > > > for everyone. What's the reasons for merging this into the fb_probe
> > > > > implementation.
> > > >
> > > > This is only necessary when struct fb_ops is defined in a library where
> > > > the owner field is the library module and not the driver module.
> > > > I realised that with this generic version it's highly unlikely that we
> > > > get another library that defines struct fb_ops, so it's only needed
> > > > here.
> > >
> > > Hm, I'm still not 100% convinced we can fully subsume the tinydrm fbdev
> > > code with the generic one, due to the varios slight issues around defio
> > > tracking that we still have.
> >
> > I have a new idea for 100% generic defio support in the fbdev helpers.
> > Haven't tried it thought, but I think this could work around the
> > problem if your mmap isn't struct page backed:
> >
> > - In the drm_fbdev_fb_mmap helper, if we need defio, change the
> > default page prots to write-protected with something like this:
> >
> > vma->vm_page_prot = pgprot_wrprotect(vma->vm_page_prot);
> >
> > - After the driver's mmap function completed, copy the vm_ops
> > structure and WARN_ON if it has an mkwrite and sync callback set.
> > There's no real race here as long as we do all this before we return
> > to userspace.
> >
> > - Set the mkwrite and fsync callbacks with similar implementions to
> > the core fbdev defio stuff. These should all work on plain ptes, they
> > don't actually require a struct page.
> > uff. These should all work on plain ptes, they don't actually require
> > a struct page.
> >
> > - We might also need to overwrite the vma_ops fault callback, just
> > forwarding to the underlying vma_ops instead of copying them would be
> > cleaner. The overhead won't matter, especially not for defio drivers.
> >
> > - Also copypaste all the other bits of the core fbdev defio code we'll
> > need, that would allow us to also clean up the cleanup() warts ...
> >
> > All of this would ofc only be done if the fb has a ->dirty callback.
> >
> > We can also just stuff this into todo.rst.
> >
>
> Hmm, do you think it's worth spending more time on fbdev?
> The point would be to speed it up, right? Avoid the blitting.
>
> My hope is that when I'm done with DRM client, David Herrmann will pick
> up and fin
[PATCH] drm/todo: Better defio support in the generic fbdev emulation
The current one essentially means you need CMA or a vmalloc backed object, which makes fbdev emulation a special case. Since implementing this will be quite a bit of work, capture the idea in a TODO. Cc: Noralf Trønnes Signed-off-by: Daniel Vetter --- Documentation/gpu/todo.rst | 30 ++ 1 file changed, 30 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 41da7b06195c..0a85dad876ae 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -209,6 +209,36 @@ Would be great to refactor this all into a set of small common helpers. Contact: Daniel Vetter +Generic fbdev defio support +--- + +The defio support code in the fbdev core has some very specific requirements, +which means drivers need to have a special framebuffer for fbdev. Which prevents +us from using the generic fbdev emulation code everywhere. The main issue is +that it uses some fields in struct page itself, which breaks shmem gem objects +(and other things). + +Possible solution would be to write our own defio mmap code in the drm fbdev +emulation. It would need to fully wrap the existing mmap ops, forwarding +everything after it has done the write-protect/mkwrite trickery: + +- In the drm_fbdev_fb_mmap helper, if we need defio, change the + default page prots to write-protected with something like this:: + + vma->vm_page_prot = pgprot_wrprotect(vma->vm_page_prot); + +- Set the mkwrite and fsync callbacks with similar implementions to the core + fbdev defio stuff. These should all work on plain ptes, they don't actually + require a struct page. uff. These should all work on plain ptes, they don't + actually require a struct page. + +- Track the dirty pages in a separate structure (bitfield with one bit per page + should work) to avoid clobbering struct page. + +Might be good to also have some igt testcases for this. + +Contact: Daniel Vetter, Noralf Tronnes + Put a reservation_object into drm_gem_object -- 2.20.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm: Fix error handling in drm_legacy_addctx
On Sat, Dec 29, 2018 at 10:49:07AM +0800, YueHaibing wrote:
> 'ctx->handle' is unsigned, it never less than zero.
> This patch use int 'tmp_handle' to handle the err condition.
>
> Fixes: 62968144e673 ("drm: convert drm context code to use Linux idr")
> Signed-off-by: YueHaibing
Queue for 5.1 (since this is essentially dead legacy code there's no
really a need to backport this).
-Daniel
> ---
> drivers/gpu/drm/drm_context.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
> index 506663c..8e73fab 100644
> --- a/drivers/gpu/drm/drm_context.c
> +++ b/drivers/gpu/drm/drm_context.c
> @@ -361,23 +361,26 @@ int drm_legacy_addctx(struct drm_device *dev, void
> *data,
> {
> struct drm_ctx_list *ctx_entry;
> struct drm_ctx *ctx = data;
> + int tmp_handle;
>
> if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
> !drm_core_check_feature(dev, DRIVER_LEGACY))
> return -EOPNOTSUPP;
>
> - ctx->handle = drm_legacy_ctxbitmap_next(dev);
> - if (ctx->handle == DRM_KERNEL_CONTEXT) {
> + tmp_handle = drm_legacy_ctxbitmap_next(dev);
> + if (tmp_handle == DRM_KERNEL_CONTEXT) {
> /* Skip kernel's context and get a new one. */
> - ctx->handle = drm_legacy_ctxbitmap_next(dev);
> + tmp_handle = drm_legacy_ctxbitmap_next(dev);
> }
> - DRM_DEBUG("%d\n", ctx->handle);
> - if (ctx->handle < 0) {
> + DRM_DEBUG("%d\n", tmp_handle);
> + if (tmp_handle < 0) {
> DRM_DEBUG("Not enough free contexts.\n");
> /* Should this return -EBUSY instead? */
> - return -ENOMEM;
> + return tmp_handle;
> }
>
> + ctx->handle = tmp_handle;
> +
> ctx_entry = kmalloc(sizeof(*ctx_entry), GFP_KERNEL);
> if (!ctx_entry) {
> DRM_DEBUG("out of memory\n");
> --
> 2.7.0
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109234] amdgpu random hangs with 4.21+
https://bugs.freedesktop.org/show_bug.cgi?id=109234 --- Comment #2 from Michel Dänzer --- Can you bisect? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Reorder set_property_atomic to avoid returning with an active ww_ctx
On Thu, Jan 03, 2019 at 10:16:54AM +, Chris Wilson wrote:
> Quoting Maarten Lankhorst (2019-01-03 09:03:27)
> > Op 30-12-2018 om 13:28 schreef Chris Wilson:
> > > Delay the drm_modeset_acquire_init() until after we check for an
> > > allocation failure so that we can return immediately upon error without
> > > having to unwind.
> > >
> > > WARNING: lock held when returning to user space!
> > > 4.20.0+ #174 Not tainted
> > >
> > > syz-executor556/8153 is leaving the kernel with locks still held!
> > > 1 lock held by syz-executor556/8153:
> > > #0: 5100c85c (crtc_ww_class_acquire){+.+.}, at:
> > > set_property_atomic+0xb3/0x330 drivers/gpu/drm/drm_mode_object.c:462
> > >
> > > Reported-by: syzbot+6ea337c427f5083eb...@syzkaller.appspotmail.com
> > > Fixes: 144a7999d633 ("drm: Handle properties in the core for atomic
> > > drivers")
> > > Signed-off-by: Chris Wilson
> > > Cc: Daniel Vetter
> > > Cc: Maarten Lankhorst
> > > Cc: Sean Paul
> > > Cc: David Airlie
> > > Cc: # v4.14+
> > > ---
> > > drivers/gpu/drm/drm_mode_object.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_mode_object.c
> > > b/drivers/gpu/drm/drm_mode_object.c
> > > index bb1dd46496cd..a9005c1c2384 100644
> > > --- a/drivers/gpu/drm/drm_mode_object.c
> > > +++ b/drivers/gpu/drm/drm_mode_object.c
> > > @@ -459,12 +459,13 @@ static int set_property_atomic(struct
> > > drm_mode_object *obj,
> > > struct drm_modeset_acquire_ctx ctx;
> > > int ret;
> > >
> > > - drm_modeset_acquire_init(&ctx, 0);
> > > -
> > > state = drm_atomic_state_alloc(dev);
> > > if (!state)
> > > return -ENOMEM;
> > > +
> > > + drm_modeset_acquire_init(&ctx, 0);
> > > state->acquire_ctx = &ctx;
> > > +
> > > retry:
> > > if (prop == state->dev->mode_config.dpms_property) {
> > > if (obj->type != DRM_MODE_OBJECT_CONNECTOR) {
> >
> > Woops only now see you did the same.. :)
>
> I'm impressed that syszbot managed to hit it! Afaict, it is only a
> debugging faux pas with no real user impact, so perhaps the stable is
> overkill.
Yeah, "small allocs can't fail" will make sure this isn't a real world
bug. syzbot uses fault injection stuff to hit these (at least that's what
it did in one of the destilled minimal reproduction cases in some other
very similar report).
> > Reviewed-by: Maarten Lankhorst
>
> Ta, pushed to drm-misc-next
So agreed -next makes sense, no fixes (but I'm sure the autoselect will
pick it up anyway, but that one can't be helped).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 07/14] drm: move EXPORT_SYMBOL_FOR_TESTS_ONLY to drm_util.h
On Thu, Jan 03, 2019 at 12:16:46AM -0800, Christoph Hellwig wrote: > On Sun, Dec 30, 2018 at 06:48:31PM +0100, Sam Ravnborg wrote: > > +/* > > + * Use EXPORT_SYMBOL_FOR_TESTS_ONLY() for functions that shall > > + * only be visible for drmselftests. > > + */ > > +#if defined(CONFIG_DRM_DEBUG_SELFTEST_MODULE) > > +#define EXPORT_SYMBOL_FOR_TESTS_ONLY(x) EXPORT_SYMBOL(x) > > +#else > > +#define EXPORT_SYMBOL_FOR_TESTS_ONLY(x) > > +#endif > > Btw, why isn't this an EXPORT_SYMBOL_GPL if it is only for internal > tests? Yeah EXPORT_SYMBOL_GPL makes sense here I think. Well most of drm is still MIT, so it's a bit hm, but the semantics of _GPL still seem like what we. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 03/11] vga-switcheroo: make PCI dependency explicit
On Sun, Dec 30, 2018 at 07:56:04PM +, Sinan Kaya wrote:
> This driver depends on the PCI infrastructure but the dependency has not
> been explicitly called out.
>
> Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI
> set")
> Signed-off-by: Sinan Kaya
> Reviewed-by: Lukas Wunner
> Acked-by: Daniel Vetter
I'm assuming this goes in through the acpi tree, correct?
-Daniel
> ---
> drivers/gpu/vga/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/vga/Kconfig b/drivers/gpu/vga/Kconfig
> index b677e5d524e6..d5f1d8e1c6f8 100644
> --- a/drivers/gpu/vga/Kconfig
> +++ b/drivers/gpu/vga/Kconfig
> @@ -21,6 +21,7 @@ config VGA_SWITCHEROO
> bool "Laptop Hybrid Graphics - GPU switching support"
> depends on X86
> depends on ACPI
> + depends on PCI
> select VGA_ARB
> help
> Many laptops released in 2008/9/10 have two GPUs with a multiplexer
> --
> 2.19.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)
On Wed, Jan 02, 2019 at 09:49:17AM +0100, Frank Wunderlich wrote:
> From: CK Hu
>
> This patch adds Framebuffer-Driver for Mediatek
>
> currently tested on mt7623, maybe works on other platforms
> MTK-FBDev written by CK Hu and ported from 4.4
>
> based on patchset drm/hdmi for 7623 v5 (except last part included in 4.20)
> https://patchwork.kernel.org/project/linux-mediatek/list/?series=25989
>
> depends on
> dts-patch (resend of 5/5, parts 1-4 already in 4.20):
> "arm: dts: mt7623: add display subsystem related device nodes"
> https://patchwork.kernel.org/patch/10588951/
> 2 bugfix-patches from bibby hsieh
> fix-boot-up-for-720-and-480-but-1080
> using-different-round-rate-for-mt7623
> 1 Patch from Ryder Lee
> http://forum.banana-pi.org/t/kernel-4-19-rc1-for-testers/6618/52
>
> full working tree here for reference:
> https://github.com/frank-w/BPI-R2-4.14/commits/4.20-hdmiv5
>
> Signed-off-by: CK Hu
> Signed-off-by: Alexander Ryabchenko
> Signed-off-by: Frank Wunderlich
> Tested-by: Frank Wunderlich
Would be good to use the new generic fbdev emulation code here, for even
less code. Or at least know why this isn't possible to use for mtk (and
maybe address that in the core code). Hand-rolling fbdev code shouldn't be
needed anymore.
-Daniel
> ---
> drivers/gpu/drm/mediatek/Makefile| 1 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 +
> drivers/gpu/drm/mediatek/mtk_drm_fb.c| 13 ++
> drivers/gpu/drm/mediatek/mtk_drm_fb.h| 3 +
> drivers/gpu/drm/mediatek/mtk_drm_fbdev.c | 178 +++
> drivers/gpu/drm/mediatek/mtk_drm_fbdev.h | 25
> 7 files changed, 230 insertions(+)
> create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.c
> create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fbdev.h
>
> diff --git a/drivers/gpu/drm/mediatek/Makefile
> b/drivers/gpu/drm/mediatek/Makefile
> index 82ae49c64221..d71e57dea77a 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -14,6 +14,7 @@ mediatek-drm-y := mtk_disp_color.o \
> mtk_mipi_tx.o \
> mtk_dpi.o
>
> +mediatek-drm-$(CONFIG_DRM_FBDEV_EMULATION) += mtk_drm_fbdev.o
> obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
>
> mediatek-drm-hdmi-objs := mtk_cec.o \
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 7800822b4db1..51f1ec3412d3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -32,6 +32,7 @@
> #include "mtk_drm_ddp_comp.h"
> #include "mtk_drm_drv.h"
> #include "mtk_drm_fb.h"
> +#include "mtk_drm_fbdev.h"
> #include "mtk_drm_gem.h"
>
> #define DRIVER_NAME "mediatek"
> @@ -299,6 +300,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> drm_kms_helper_poll_init(drm);
> drm_mode_config_reset(drm);
>
> + ret = mtk_fbdev_init(drm);
> + if (ret)
> + goto err_component_unbind;
> +
> return 0;
>
> err_component_unbind:
> @@ -311,6 +316,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>
> static void mtk_drm_kms_deinit(struct drm_device *drm)
> {
> + mtk_fbdev_fini(drm);
> drm_kms_helper_poll_fini(drm);
>
> component_unbind_all(drm->dev, drm);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 256a3ff2e66e..56129a21fb2b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -14,6 +14,7 @@
> #ifndef MTK_DRM_DRV_H
> #define MTK_DRM_DRV_H
>
> +#include
> #include
> #include "mtk_drm_ddp_comp.h"
>
> @@ -43,6 +44,7 @@ struct mtk_drm_private {
> struct drm_device *drm;
> struct device *dma_dev;
>
> + struct drm_crtc *crtc[MAX_CRTC];
> unsigned int num_pipes;
>
> struct device_node *mutex_node;
> @@ -59,6 +61,8 @@ struct mtk_drm_private {
> } commit;
>
> struct drm_atomic_state *suspend_state;
> + struct drm_fb_helper fb_helper;
> + struct drm_gem_object *fbdev_bo;
> };
>
> extern struct platform_driver mtk_ddp_driver;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> index be5f6f1daf55..7533aa4733d2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> @@ -56,6 +56,19 @@ static struct drm_framebuffer
> *mtk_drm_framebuffer_init(struct drm_device *dev,
> return fb;
> }
>
> +struct drm_framebuffer *mtk_drm_framebuffer_create(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode,
> + struct drm_gem_object *obj)
> +{
> + struct drm_framebuffer *mtk_fb;
> +
> + mtk_fb = mtk_drm_framebuffer_init(dev, mode, obj);
> + if (IS_ERR(mtk_fb))
> + return ERR_CAST(mtk_fb);
> +
> + return mtk_fb;
Re: [PATCH 1/1] drm/prime: Use sg_dma_len() macro to get sg's length
Hi Vivek,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc1 next-20190107]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/drm-prime-Use-sg_dma_len-macro-to-get-sg-s-length/20190107-181350
config: x86_64-randconfig-x013-201901 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/gpu/drm/drm_prime.c: In function 'drm_prime_sg_to_page_addr_arrays':
>> drivers/gpu/drm/drm_prime.c:948:9: error: implicit declaration of function
>> 'sg_dma_length'; did you mean 'sg_dma_len'?
>> [-Werror=implicit-function-declaration]
len = sg_dma_length(sg);
^
sg_dma_len
cc1: some warnings being treated as errors
vim +948 drivers/gpu/drm/drm_prime.c
926
927 /**
928 * drm_prime_sg_to_page_addr_arrays - convert an sg table into a page
array
929 * @sgt: scatter-gather table to convert
930 * @pages: optional array of page pointers to store the page array in
931 * @addrs: optional array to store the dma bus address of each page
932 * @max_entries: size of both the passed-in arrays
933 *
934 * Exports an sg table into an array of pages and addresses. This is
currently
935 * required by the TTM driver in order to do correct fault handling.
936 */
937 int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page
**pages,
938 dma_addr_t *addrs, int max_entries)
939 {
940 unsigned count;
941 struct scatterlist *sg;
942 struct page *page;
943 u32 len, index;
944 dma_addr_t addr;
945
946 index = 0;
947 for_each_sg(sgt->sgl, sg, sgt->nents, count) {
> 948 len = sg_dma_length(sg);
949 page = sg_page(sg);
950 addr = sg_dma_address(sg);
951
952 while (len > 0) {
953 if (WARN_ON(index >= max_entries))
954 return -1;
955 if (pages)
956 pages[index] = page;
957 if (addrs)
958 addrs[index] = addr;
959
960 page++;
961 addr += PAGE_SIZE;
962 len -= PAGE_SIZE;
963 index++;
964 }
965 }
966 return 0;
967 }
968 EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays);
969
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Armada DRM: bridge with componentized devices
On Thu, Jan 03, 2019 at 01:11:47PM +, Russell King - ARM Linux wrote: > On Thu, Jan 03, 2019 at 10:47:27AM +0100, Lubomir Rintel wrote: > > Hello, > > > > lately I've been trying to make the Himax HX8837 chip that drives the OLPC > > LCD display work with Armada DRM driver. I've been advised to create a > > bridge driver and not an encoder driver since the silicon is separate from > > the LCDC. > > > > The Armada DRM driver (and, I think, the i.MX one) creates the drm_device > > once the component infrastructure sees the necessary sub-devices appear. > > The sub-devices being the LCDCs and the encoders (not bridges) that it > > expects to be created externally. > > > > Currently, it seems, the only driver that can actually work with this (that > > is -- creates a drm_encoder for a drm_device when the component is bound) > > is the tda998x. All other similar drivers create a drm_bridge instead and > > not use the component infrastructure at all. (In fact, tilcdc driver > > contains a hack to handle tda998x specially.) > > > > I'm wondering how to reconcile the two? > > > > * The tda998x driver has recently been modified to create a bridge on probe > > and eventually encoder on component bind. Is this an okay thing to do in > > a new driver? (this probably means the tilcdc hack can be removed...) > > > > * If a non-componentized bridge were to be used (along with a dummy > > encoder), at what point would it make sense to look for the bridge? > > Would it be a good idea to defer the probe of crtc until a bridge can be > > looked up and the attach it on component bind? What if the bridge goes > > away (a module is unloaded, etc.) in between? > > > > I'd be thankful for opintions and advice before I move ahead with this. > > This is the long-standing problem with the conflict between bridge > support and component support, and I'm not sure that there is really > any answer to it. > > I've gone into the details of the two several times on the list, > particularly about the short-comings of the bridge approach, but it > seems no one cares to fix those short-comings. > > You are re-identifying some of the issues that I've already pointed > out - such as what happens to DRM drives when the bridge driver is > unbound (it's really not about modules being unloaded, and the problem > can't be solved by taking a module reference count - all that the > module reference count does is ensure that the module doesn't go > away unexpected, there is no way to ensure that the device isn't > unbound.) > > The issue of unbinding is precisely the issue which the component > support was created to solve - but everyone seems to prefer the buggy > bridge approach, and no one seems willing to do anything about the > bugs or even acknowledge that it's a problem. It's strange - if one > identifies bugs that result in kernel oops in other kernel subsystems, > one is generally taken seriously and the problem is solved. Unbinding is really not the most important feature, especially for SoC. If you feel different, working together with others, getting some agreement, getting the patches reviewed and finding someone to get them merged is very much appreciated. But just complaining won't move this forward. > The issue about the encoders is something that I've tried to discuss, > and I've pointed out that moving it into the DRM driver adds additional > complexity there, and I'd hoped that my patch set I posted would've > generated discussion, but alas not. > > What I'm not prepared to do is to introduce _known_ bugs into any > driver that I maintain. I thought last time around the idea was to use device links (and teach drm_bridge about them), so that the unloading works correctly. Wrt tda988x: I think it really shouldn't create a drm_encoder, nor register as a component. Fixing that is probably a bit more work. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Auto-set allow_fb_modifiers when given modifiers at plane init
On Fri, Jan 04, 2019 at 09:56:10AM +0100, Paul Kocialkowski wrote: > When drivers pass non-empty lists of modifiers for initializing their > planes, we can infer that they allow framebuffer modifiers and set the > driver's allow_fb_modifiers mode config element. > > In case the allow_fb_modifiers element was not set (some drivers tend > to set them after registering planes), the modifiers will still be > registered but won't be available to userspace unless the flag is set > later. However in that case, the IN_FORMATS blob won't be created. > > In order to avoid this case and generally reduce the trouble associated > with the flag, always set allow_fb_modifiers when a non-empty list of > format modifiers is passed at plane init. > > Signed-off-by: Paul Kocialkowski The automatic primary plane drm_crtc_init() creates doesn't set this, so looks correct in all cases. Reviewed-by: Daniel Vetter (boolin.com has a bunch of drm-misc committers, so I'll leave pushing to them). -Daniel > --- > drivers/gpu/drm/drm_plane.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 5f650d8fc66b..4cfb56893b7f 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -220,6 +220,9 @@ int drm_universal_plane_init(struct drm_device *dev, > struct drm_plane *plane, > format_modifier_count++; > } > > + if (format_modifier_count) > + config->allow_fb_modifiers = true; > + > plane->modifier_count = format_modifier_count; > plane->modifiers = kmalloc_array(format_modifier_count, >sizeof(format_modifiers[0]), > -- > 2.20.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/12] gpu: fix typo
On Fri, Jan 04, 2019 at 10:41:15PM +0100, Matteo Croce wrote:
> Fix spelling mistake: "lenght" -> "length"
>
> Signed-off-by: Matteo Croce
> ---
> drivers/gpu/drm/amd/include/atombios.h | 2 +-
> drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 2 +-
Best to split this up into an amd and omapdrm patch (and send to each
respective maintainer), they're maintained in separate trees.
Thanks, Daniel
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/include/atombios.h
> b/drivers/gpu/drm/amd/include/atombios.h
> index 7931502fa54f..8ba21747b40a 100644
> --- a/drivers/gpu/drm/amd/include/atombios.h
> +++ b/drivers/gpu/drm/amd/include/atombios.h
> @@ -4106,7 +4106,7 @@ typedef struct _ATOM_LCD_MODE_CONTROL_CAP
> typedef struct _ATOM_FAKE_EDID_PATCH_RECORD
> {
>UCHAR ucRecordType;
> - UCHAR ucFakeEDIDLength; // = 128 means EDID lenght is 128 bytes,
> otherwise the EDID length = ucFakeEDIDLength*128
> + UCHAR ucFakeEDIDLength; // = 128 means EDID length is 128 bytes,
> otherwise the EDID length = ucFakeEDIDLength*128
>UCHAR ucFakeEDIDString[1];// This actually has ucFakeEdidLength
> elements.
> } ATOM_FAKE_EDID_PATCH_RECORD;
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> index 813ba42f2753..e384b95ad857 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> @@ -708,7 +708,7 @@ int hdmi4_audio_config(struct hdmi_core_data *core,
> struct hdmi_wp_data *wp,
> else
> acore.i2s_cfg.justification = HDMI_AUDIO_JUSTIFY_RIGHT;
> /*
> - * The I2S input word length is twice the lenght given in the IEC-60958
> + * The I2S input word length is twice the length given in the IEC-60958
>* status word. If the word size is greater than
>* 20 bits, increment by one.
>*/
> --
> 2.20.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 03/11] vga-switcheroo: make PCI dependency explicit
On Mon, Jan 7, 2019 at 11:34 AM Daniel Vetter wrote:
>
> On Sun, Dec 30, 2018 at 07:56:04PM +, Sinan Kaya wrote:
> > This driver depends on the PCI infrastructure but the dependency has not
> > been explicitly called out.
> >
> > Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI
> > set")
> > Signed-off-by: Sinan Kaya
> > Reviewed-by: Lukas Wunner
> > Acked-by: Daniel Vetter
>
> I'm assuming this goes in through the acpi tree, correct?
I'm going to pick it up.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 2/3] drm: Add CRTC background color property (v4)
Hi Matt, On Fri, Dec 28, 2018 at 03:51:30PM -0800, Matt Roper wrote: > Some display controllers can be programmed to present non-black colors > for pixels not covered by any plane (or pixels covered by the > transparent regions of higher planes). Compositors that want a UI with > a solid color background can potentially save memory bandwidth by > setting the CRTC background property and using smaller planes to display > the rest of the content. > > To avoid confusion between different ways of encoding RGB data, we > define a standard 64-bit format that should be used for this property's > value. Helper functions and macros are provided to generate and dissect > values in this standard format with varying component precision values. > > v2: > - Swap internal representation's blue and red bits to make it easier >to read if printed out. (Ville) > - Document bgcolor property in drm_blend.c. (Sean Paul) > - s/background_color/bgcolor/ for consistency between property name and >value storage field. (Sean Paul) > - Add a convenience function to attach property to a given crtc. > > v3: > - Restructure ARGB component extraction macros to be easier to >understand and enclose the parameters in () to avoid calculations >if expressions are passed. (Sean Paul) > - s/rgba/argb/ in helper function/macro names. Even though the idea is >to not worry about the internal representation of the u64, it can >still be confusing to look at code that uses 'rgba' terminology, but >stores values with argb ordering. (Ville) > > v4: > - Drop the bgcolor_changed flag. (Ville, Brian Starkey) > - Clarify in kerneldoc that background color is expected to undergo the >same pipe-level degamma/csc/gamma transformations that planes do. >(Brian Starkey) > - Update kerneldoc to indicate non-opaque colors are allowed, but are >generally only useful in special cases such as when writeback >connectors are used (Brian Starkey / Eric Anholt) The updates LGTM. Reviewed-by: Brian Starkey It would be good to see the Chrome implementation when you have it to share. Cheers, -Brian > > Cc: dri-devel@lists.freedesktop.org > Cc: wei.c...@intel.com > Cc: harish.krupo@intel.com > Cc: Ville Syrjälä > Cc: Sean Paul > Cc: Brian Starkey > Cc: Eric Anholt > Cc: Stéphane Marchesin > Cc: Daniel Vetter > Signed-off-by: Matt Roper > Reviewed-by(v2): Sean Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] v2 bakclight: fix missing checks in ambient_light_zone_store
On Sat, Jan 05, 2019 at 12:48:07PM -0600, Aditya Pakki wrote:
> In adp8870_bl_ambient_light_zone_store, set, clear, and write
I'm curious...
What attracted you to this particular _store function? AFAICT the same
ignoring of return values occurs in other _store functions in this file?
> can return an error but are not checked. The fix adds a check for these
> cases and returns -1 to match the return type (ssize_t).
This isn't right. The caller of the store function expects -errno
values (this is the default for almost all kernel functions).
> Signed-off-by: Aditya Pakki
> ---
> drivers/video/backlight/adp8870_bl.c | 16 +---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/backlight/adp8870_bl.c
> b/drivers/video/backlight/adp8870_bl.c
> index 8d50e0299578..56a640587a82 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -800,10 +800,14 @@ static ssize_t
> adp8870_bl_ambient_light_zone_store(struct device *dev,
>
> if (val == 0) {
> /* Enable automatic ambient light sensing */
> - adp8870_set_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
> + ret = adp8870_set_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
> + if (ret < 0)
> + goto adp8870_bl_err;
There's nothing *wrong* with goto for error handling but is it
really needed here (there is no recovery code)?
> } else if ((val > 0) && (val < 6)) {
> /* Disable automatic ambient light sensing */
> - adp8870_clr_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
> + ret = adp8870_clr_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
> + if (ret < 0)
> + goto adp8870_bl_err;
+1
>
> /* Set user supplied ambient light zone */
> mutex_lock(&data->lock);
> @@ -811,12 +815,18 @@ static ssize_t
> adp8870_bl_ambient_light_zone_store(struct device *dev,
> if (!ret) {
> reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
> reg_val |= (val - 1) << CFGR_BLV_SHIFT;
> - adp8870_write(data->client, ADP8870_CFGR, reg_val);
> + ret = adp8870_write(data->client, ADP8870_CFGR,
> + reg_val);
> }
> mutex_unlock(&data->lock);
> + if (ret < 0)
> + goto adp8870_bl_err;
+1
Daniel.
> }
>
> return count;
> +
> +adp8870_bl_err:
> + return -1;
> }
> static DEVICE_ATTR(ambient_light_zone, 0664,
> adp8870_bl_ambient_light_zone_show,
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] v2 bakclight: fix missing checks in ambient_light_zone_store
On Sat, Jan 05, 2019 at 12:48:07PM -0600, Aditya Pakki wrote:
> In adp8870_bl_ambient_light_zone_store, set, clear, and write
> can return an error but are not checked. The fix adds a check for these
> cases and returns -1 to match the return type (ssize_t).
Sorry... missed this before but there is also a typo in the Subject:
line.
Daniel.
>
> Signed-off-by: Aditya Pakki
> ---
> drivers/video/backlight/adp8870_bl.c | 16 +---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/backlight/adp8870_bl.c
> b/drivers/video/backlight/adp8870_bl.c
> index 8d50e0299578..56a640587a82 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -800,10 +800,14 @@ static ssize_t
> adp8870_bl_ambient_light_zone_store(struct device *dev,
>
> if (val == 0) {
> /* Enable automatic ambient light sensing */
> - adp8870_set_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
> + ret = adp8870_set_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
> + if (ret < 0)
> + goto adp8870_bl_err;
> } else if ((val > 0) && (val < 6)) {
> /* Disable automatic ambient light sensing */
> - adp8870_clr_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
> + ret = adp8870_clr_bits(data->client, ADP8870_MDCR, CMP_AUTOEN);
> + if (ret < 0)
> + goto adp8870_bl_err;
>
> /* Set user supplied ambient light zone */
> mutex_lock(&data->lock);
> @@ -811,12 +815,18 @@ static ssize_t
> adp8870_bl_ambient_light_zone_store(struct device *dev,
> if (!ret) {
> reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
> reg_val |= (val - 1) << CFGR_BLV_SHIFT;
> - adp8870_write(data->client, ADP8870_CFGR, reg_val);
> + ret = adp8870_write(data->client, ADP8870_CFGR,
> + reg_val);
> }
> mutex_unlock(&data->lock);
> + if (ret < 0)
> + goto adp8870_bl_err;
> }
>
> return count;
> +
> +adp8870_bl_err:
> + return -1;
> }
> static DEVICE_ATTR(ambient_light_zone, 0664,
> adp8870_bl_ambient_light_zone_show,
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Armada DRM: bridge with componentized devices
On 07.01.2019 11:45, Daniel Vetter wrote: > On Thu, Jan 03, 2019 at 01:11:47PM +, Russell King - ARM Linux wrote: >> On Thu, Jan 03, 2019 at 10:47:27AM +0100, Lubomir Rintel wrote: >>> Hello, >>> >>> lately I've been trying to make the Himax HX8837 chip that drives the OLPC >>> LCD display work with Armada DRM driver. I've been advised to create a >>> bridge driver and not an encoder driver since the silicon is separate from >>> the LCDC. >>> >>> The Armada DRM driver (and, I think, the i.MX one) creates the drm_device >>> once the component infrastructure sees the necessary sub-devices appear. >>> The sub-devices being the LCDCs and the encoders (not bridges) that it >>> expects to be created externally. >>> >>> Currently, it seems, the only driver that can actually work with this (that >>> is -- creates a drm_encoder for a drm_device when the component is bound) >>> is the tda998x. All other similar drivers create a drm_bridge instead and >>> not use the component infrastructure at all. (In fact, tilcdc driver >>> contains a hack to handle tda998x specially.) >>> >>> I'm wondering how to reconcile the two? >>> >>> * The tda998x driver has recently been modified to create a bridge on probe >>> and eventually encoder on component bind. Is this an okay thing to do in >>> a new driver? (this probably means the tilcdc hack can be removed...) >>> >>> * If a non-componentized bridge were to be used (along with a dummy >>> encoder), at what point would it make sense to look for the bridge? >>> Would it be a good idea to defer the probe of crtc until a bridge can be >>> looked up and the attach it on component bind? What if the bridge goes >>> away (a module is unloaded, etc.) in between? >>> >>> I'd be thankful for opintions and advice before I move ahead with this. >> This is the long-standing problem with the conflict between bridge >> support and component support, and I'm not sure that there is really >> any answer to it. >> >> I've gone into the details of the two several times on the list, >> particularly about the short-comings of the bridge approach, but it >> seems no one cares to fix those short-comings. >> >> You are re-identifying some of the issues that I've already pointed >> out - such as what happens to DRM drives when the bridge driver is >> unbound (it's really not about modules being unloaded, and the problem >> can't be solved by taking a module reference count - all that the >> module reference count does is ensure that the module doesn't go >> away unexpected, there is no way to ensure that the device isn't >> unbound.) >> >> The issue of unbinding is precisely the issue which the component >> support was created to solve - but everyone seems to prefer the buggy >> bridge approach, and no one seems willing to do anything about the >> bugs or even acknowledge that it's a problem. It's strange - if one >> identifies bugs that result in kernel oops in other kernel subsystems, >> one is generally taken seriously and the problem is solved. > Unbinding is really not the most important feature, especially for SoC. If > you feel different, working together with others, getting some agreement, > getting the patches reviewed and finding someone to get them merged is > very much appreciated. But just complaining won't move this forward. > >> The issue about the encoders is something that I've tried to discuss, >> and I've pointed out that moving it into the DRM driver adds additional >> complexity there, and I'd hoped that my patch set I posted would've >> generated discussion, but alas not. >> >> What I'm not prepared to do is to introduce _known_ bugs into any >> driver that I maintain. > I thought last time around the idea was to use device links (and teach > drm_bridge about them), so that the unloading works correctly. With current device_links implementation registering links in probe of the consumer is just incorrect - it can happen that neither consumer neither provider is fully probed and creating device links in such state is wrong according to docs, and my experiments. See [1] for discussion and [2] for docs. [1]: https://patchwork.freedesktop.org/patch/218878/ [2]: https://www.kernel.org/doc/html/latest/driver-api/device_link.html#usage Regards Andrzej > > Wrt tda988x: I think it really shouldn't create a drm_encoder, nor > register as a component. Fixing that is probably a bit more work. > -Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] drm/prime: Use sg_dma_len() macro to get sg's length
Hi Vivek,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc1 next-20190107]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/drm-prime-Use-sg_dma_len-macro-to-get-sg-s-length/20190107-181350
config: i386-randconfig-s3-201901 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/gpu//drm/drm_prime.c: In function 'drm_prime_sg_to_page_addr_arrays':
>> drivers/gpu//drm/drm_prime.c:948:9: error: implicit declaration of function
>> 'sg_dma_length' [-Werror=implicit-function-declaration]
len = sg_dma_length(sg);
^
cc1: some warnings being treated as errors
vim +/sg_dma_length +948 drivers/gpu//drm/drm_prime.c
926
927 /**
928 * drm_prime_sg_to_page_addr_arrays - convert an sg table into a page
array
929 * @sgt: scatter-gather table to convert
930 * @pages: optional array of page pointers to store the page array in
931 * @addrs: optional array to store the dma bus address of each page
932 * @max_entries: size of both the passed-in arrays
933 *
934 * Exports an sg table into an array of pages and addresses. This is
currently
935 * required by the TTM driver in order to do correct fault handling.
936 */
937 int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page
**pages,
938 dma_addr_t *addrs, int max_entries)
939 {
940 unsigned count;
941 struct scatterlist *sg;
942 struct page *page;
943 u32 len, index;
944 dma_addr_t addr;
945
946 index = 0;
947 for_each_sg(sgt->sgl, sg, sgt->nents, count) {
> 948 len = sg_dma_length(sg);
949 page = sg_page(sg);
950 addr = sg_dma_address(sg);
951
952 while (len > 0) {
953 if (WARN_ON(index >= max_entries))
954 return -1;
955 if (pages)
956 pages[index] = page;
957 if (addrs)
958 addrs[index] = addr;
959
960 page++;
961 addr += PAGE_SIZE;
962 len -= PAGE_SIZE;
963 index++;
964 }
965 }
966 return 0;
967 }
968 EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays);
969
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] gpu: ipu-v3: pre: don't trigger update if buffer address doesn't change
Am Freitag, den 21.12.2018, 12:11 +0100 schrieb Philipp Zabel:
> Hi Lucas,
>
> > On Wed, Dec 19, 2018 at 4:40 PM Lucas Stach wrote:
> >
> > On a NOP double buffer update where current buffer address is the same
> > as the next buffer address, the SDW_UPDATE bit clears too late.
>
> What does this mean, exactly? Does the hardware behave differently
> if the writel to IPU_PRE_NEXT_BUF doesn't change the value?
To me it seems like that. When the address changes, the SDW_UPDATE bit
is cleared by the time we handle the EOF IRQ. When the address doesn't
change, it seems the SDW_UPDATE bit clears much later (I've tried the
new frame IRQ without any success), maybe only at the next EOF,
effectively doubling the update period if a plane is flipped with the
same buffer.
>
> > As we
> > are now using this bit to determine when it is safe to signal flip
> > completion to userspace this will delay completion of atomic commits
> > where one plane doesn't change the buffer by a whole frame period.
>
> This sounds like the problem is not the way PRE behaves, but that we are
> setting the SDW_UPDATE flag again and then later use it to check whether
> the previous buffer was released, resulting in a false negative.
>
> > Fix this by remembering the last buffer address and just skip the
> > double buffer update if it would not change the buffer address.
>
> It looks to me like ipu_plane_atomic_update does not have to call
> ipu_prg_channel_configure (which then just relays to ipu_pre_update)
> at all in this case. Should we just skip this in ipuv3-plane.c already?
While we could handle it in ipuv3-plane, this would require more of the
PRE/PRG state tracking to be moved there. Currently a lot of this is
hidden behind the simple ipu_prg_channel_configure() call.
> > Signed-off-by: Lucas Stach
> > ---
> > drivers/gpu/ipu-v3/ipu-pre.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
> > index f933c1911e65..d383e8dbd6cc 100644
> > --- a/drivers/gpu/ipu-v3/ipu-pre.c
> > +++ b/drivers/gpu/ipu-v3/ipu-pre.c
> > @@ -106,6 +106,7 @@ struct ipu_pre {
> > void*buffer_virt;
> > boolin_use;
> > unsigned intsafe_window_end;
> > + unsigned intlast_bufaddr;
>
> This looks a lot like plane state.
>
> > };
> >
> > static DEFINE_MUTEX(ipu_pre_list_mutex);
> > @@ -242,7 +243,11 @@ void ipu_pre_update(struct ipu_pre *pre, unsigned int
> > bufaddr)
> > unsigned short current_yblock;
> > u32 val;
> >
> > + if (bufaddr == pre->last_bufaddr)
> > + return;
>
> Hypothetical question, could this wrongly return if the channel is
> first disabled, leaving
> last_buffaddr set to X, and then the channel is reenabled, which
> doesn't touch last_bufaddr,
> and then the first update contains a buffer at the same address X?
Nope, this code path is only used when doing no-modeset updates of an
active plane. When the plane gets disabled the PRE goes through a
complete reinitialization via ipu_pre_configure() when the channel is
reenabled.
Regards,
Lucas
> > +
> > writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
> > + pre->last_bufaddr = bufaddr;
> >
> > do {
> > if (time_after(jiffies, timeout)) {
> > --
> > 2.19.1
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> regards
> Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.
Am 06.01.19 um 21:29 schrieb Bas Nieuwenhuizen: On Sun, Jan 6, 2019 at 9:23 PM Christian König wrote: Am 06.01.19 um 10:46 schrieb Bas Nieuwenhuizen: For radv we want to be able to pass in a master fd and be sure that the created libdrm_amdgpu device also uses that master fd, so we can use it for prioritized submission. radv does all interaction with other APIs/processes with dma-bufs, so we should not need the functionality in libdrm_amdgpu to only have a single fd for a device in the process. Signed-off-by: Bas Nieuwenhuizen Well NAK. This breaks a couple of design assumptions we used for the kernel driver and is strongly discouraged. What assumptions are these? As far as I can tell everything is per drm fd, not process? Instead radv should not use the master fd for command submission. High priority submission needs to be done through a master fd That assumption is incorrect. See file drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c to answer both your questions at the same time. Additional to the scheduler priority we really don't want more than one fd in a process because of SVM binding overhead. So that whole approach is a really big NAK. Regards, Christian. , so not using a master fd is not an option ... Regards, Christian. --- amdgpu/amdgpu-symbol-check | 1 + amdgpu/amdgpu.h| 37 amdgpu/amdgpu_device.c | 59 -- 3 files changed, 76 insertions(+), 21 deletions(-) diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check index 6f5e0f95..bbf48985 100755 --- a/amdgpu/amdgpu-symbol-check +++ b/amdgpu/amdgpu-symbol-check @@ -56,6 +56,7 @@ amdgpu_cs_wait_fences amdgpu_cs_wait_semaphore amdgpu_device_deinitialize amdgpu_device_initialize +amdgpu_device_initialize2 amdgpu_find_bo_by_cpu_mapping amdgpu_get_marketing_name amdgpu_query_buffer_size_alignment diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index dc51659a..e5ed39bb 100644 --- a/amdgpu/amdgpu.h +++ b/amdgpu/amdgpu.h @@ -66,6 +66,13 @@ struct drm_amdgpu_info_hw_ip; */ #define AMDGPU_QUERY_FENCE_TIMEOUT_IS_ABSOLUTE (1 << 0) +/** + * Uses in amdgpu_device_initialize2(), meaning that the passed in fd should + * not be deduplicated against other libdrm_amdgpu devices referring to the + * same kernel device. + */ +#define AMDGPU_DEVICE_DEDICATED_FD (1 << 0) + /*--*/ /* - Enums */ /*--*/ @@ -526,6 +533,36 @@ int amdgpu_device_initialize(int fd, uint32_t *minor_version, amdgpu_device_handle *device_handle); +/** + * + * \param fd- \c [in] File descriptor for AMD GPU device + * received previously as the result of + * e.g. drmOpen() call. + * For legacy fd type, the DRI2/DRI3 + * authentication should be done before + * calling this function. + * \param flags - \c [in] Bitmask of flags for device creation. + * \param major_version - \c [out] Major version of library. It is assumed + * that adding new functionality will cause + * increase in major version + * \param minor_version - \c [out] Minor version of library + * \param device_handle - \c [out] Pointer to opaque context which should + * be passed as the first parameter on each + * API call + * + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * + * + * \sa amdgpu_device_deinitialize() +*/ +int amdgpu_device_initialize2(int fd, + uint32_t flags, + uint32_t *major_version, + uint32_t *minor_version, + amdgpu_device_handle *device_handle); + /** * * When access to such library does not needed any more the special diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c index 362494b1..b4bf3f76 100644 --- a/amdgpu/amdgpu_device.c +++ b/amdgpu/amdgpu_device.c @@ -100,7 +100,8 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev) pthread_mutex_lock(&fd_mutex); while (*node != dev && (*node)->next) node = &(*node)->next; - *node = (*node)->next; + if (*node == dev) + *node = (*node)->next; pthread_mutex_unlock(&fd_mutex); close(dev->fd); @@ -144,6 +145,16 @@ drm_public int amdgpu_device_initialize(int fd, uint32_t *major_version, uint32_t *minor_versio
Re: [PATCH] drm/fb-helper: generic: Fix setup error path
Hi,
> If register_framebuffer() fails during fbdev setup we will leak the
> framebuffer, the GEM buffer and the shadow buffer for defio. This is
> because drm_fb_helper_fbdev_setup() just calls drm_fb_helper_fini() on
> error not taking into account that register_framebuffer() can fail.
>
> Since the generic emulation uses DRM client for its framebuffer and
> backing buffer in addition to a shadow buffer, it's necessary to open code
> drm_fb_helper_fbdev_setup() to properly handle the error path.
>
> Error cleanup is removed from .fb_probe and is handled by one function for
> all paths.
>
> Fixes: 9060d7f49376 ("drm/fb-helper: Finish the generic fbdev emulation")
> Reported-by: Peter Wu
> Signed-off-by: Noralf Trønnes
Looks sane to me.
Acked-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/drm_fb_helper.c | 98 +++--
> 1 file changed, 58 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 5e9ca6f96379..80136a7a5af1 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2961,18 +2961,16 @@ static int drm_fbdev_fb_release(struct fb_info *info,
> int user)
> return 0;
> }
>
> -/*
> - * fb_ops.fb_destroy is called by the last put_fb_info() call at the end of
> - * unregister_framebuffer() or fb_release().
> - */
> -static void drm_fbdev_fb_destroy(struct fb_info *info)
> +static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
> {
> - struct drm_fb_helper *fb_helper = info->par;
> struct fb_info *fbi = fb_helper->fbdev;
> struct fb_ops *fbops = NULL;
> void *shadow = NULL;
>
> - if (fbi->fbdefio) {
> + if (!fb_helper->dev)
> + return;
> +
> + if (fbi && fbi->fbdefio) {
> fb_deferred_io_cleanup(fbi);
> shadow = fbi->screen_buffer;
> fbops = fbi->fbops;
> @@ -2986,6 +2984,12 @@ static void drm_fbdev_fb_destroy(struct fb_info *info)
> }
>
> drm_client_framebuffer_delete(fb_helper->buffer);
> +}
> +
> +static void drm_fbdev_release(struct drm_fb_helper *fb_helper)
> +{
> + drm_fbdev_cleanup(fb_helper);
> +
> /*
>* FIXME:
>* Remove conditional when all CMA drivers have been moved over to using
> @@ -2997,6 +3001,15 @@ static void drm_fbdev_fb_destroy(struct fb_info *info)
> }
> }
>
> +/*
> + * fb_ops.fb_destroy is called by the last put_fb_info() call at the end of
> + * unregister_framebuffer() or fb_release().
> + */
> +static void drm_fbdev_fb_destroy(struct fb_info *info)
> +{
> + drm_fbdev_release(info->par);
> +}
> +
> static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct
> *vma)
> {
> struct drm_fb_helper *fb_helper = info->par;
> @@ -3047,7 +3060,6 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper
> *fb_helper,
> struct drm_framebuffer *fb;
> struct fb_info *fbi;
> u32 format;
> - int ret;
>
> DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
> sizes->surface_width, sizes->surface_height,
> @@ -3064,10 +3076,8 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper
> *fb_helper,
> fb = buffer->fb;
>
> fbi = drm_fb_helper_alloc_fbi(fb_helper);
> - if (IS_ERR(fbi)) {
> - ret = PTR_ERR(fbi);
> - goto err_free_buffer;
> - }
> + if (IS_ERR(fbi))
> + return PTR_ERR(fbi);
>
> fbi->par = fb_helper;
> fbi->fbops = &drm_fbdev_fb_ops;
> @@ -3098,8 +3108,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper
> *fb_helper,
> if (!fbops || !shadow) {
> kfree(fbops);
> vfree(shadow);
> - ret = -ENOMEM;
> - goto err_fb_info_destroy;
> + return -ENOMEM;
> }
>
> *fbops = *fbi->fbops;
> @@ -3111,13 +3120,6 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper
> *fb_helper,
> }
>
> return 0;
> -
> -err_fb_info_destroy:
> - drm_fb_helper_fini(fb_helper);
> -err_free_buffer:
> - drm_client_framebuffer_delete(buffer);
> -
> - return ret;
> }
> EXPORT_SYMBOL(drm_fb_helper_generic_probe);
>
> @@ -3129,18 +3131,11 @@ static void drm_fbdev_client_unregister(struct
> drm_client_dev *client)
> {
> struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
>
> - if (fb_helper->fbdev) {
> - drm_fb_helper_unregister_fbi(fb_helper);
> + if (fb_helper->fbdev)
> /* drm_fbdev_fb_destroy() takes care of cleanup */
> - return;
> - }
> -
> - /* Did drm_fb_helper_fbdev_setup() run? */
> - if (fb_helper->dev)
> - drm_fb_helper_fini(fb_helper);
> -
> - drm_client_release(client);
> - kfree(fb_helper);
> + drm_fb_helper_unregister_fbi(fb_helper);
> + else
> + drm_fbdev_release
Re: [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.
On Mon, Jan 7, 2019 at 1:23 PM Christian König wrote: > > Am 06.01.19 um 21:29 schrieb Bas Nieuwenhuizen: > > On Sun, Jan 6, 2019 at 9:23 PM Christian König > > wrote: > >> Am 06.01.19 um 10:46 schrieb Bas Nieuwenhuizen: > >>> For radv we want to be able to pass in a master fd and be sure that > >>> the created libdrm_amdgpu device also uses that master fd, so we can > >>> use it for prioritized submission. > >>> > >>> radv does all interaction with other APIs/processes with dma-bufs, > >>> so we should not need the functionality in libdrm_amdgpu to only have > >>> a single fd for a device in the process. > >>> > >>> Signed-off-by: Bas Nieuwenhuizen > >> Well NAK. > >> > >> This breaks a couple of design assumptions we used for the kernel driver > >> and is strongly discouraged. > > What assumptions are these? As far as I can tell everything is per drm > > fd, not process? > >> Instead radv should not use the master fd for command submission. > > High priority submission needs to be done through a master fd > > That assumption is incorrect. See file > drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c to answer both your questions > at the same time. Hmm, I did not know about the AMDGPU_SCHED ioctl. That would work with the permissions. However as it stands we cannot use it, as it is for the entire drm-fd, not per context. Would you be open to a patch adding a context parameter to the struct and a AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE? > > Additional to the scheduler priority we really don't want more than one > fd in a process because of SVM binding overhead. > > So that whole approach is a really big NAK. > > Regards, > Christian. > > > , so not > > using a master fd is not an option ... > > > >> Regards, > >> Christian. > >> > >> > >>> --- > >>>amdgpu/amdgpu-symbol-check | 1 + > >>>amdgpu/amdgpu.h| 37 > >>>amdgpu/amdgpu_device.c | 59 -- > >>>3 files changed, 76 insertions(+), 21 deletions(-) > >>> > >>> diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check > >>> index 6f5e0f95..bbf48985 100755 > >>> --- a/amdgpu/amdgpu-symbol-check > >>> +++ b/amdgpu/amdgpu-symbol-check > >>> @@ -56,6 +56,7 @@ amdgpu_cs_wait_fences > >>>amdgpu_cs_wait_semaphore > >>>amdgpu_device_deinitialize > >>>amdgpu_device_initialize > >>> +amdgpu_device_initialize2 > >>>amdgpu_find_bo_by_cpu_mapping > >>>amdgpu_get_marketing_name > >>>amdgpu_query_buffer_size_alignment > >>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h > >>> index dc51659a..e5ed39bb 100644 > >>> --- a/amdgpu/amdgpu.h > >>> +++ b/amdgpu/amdgpu.h > >>> @@ -66,6 +66,13 @@ struct drm_amdgpu_info_hw_ip; > >>> */ > >>>#define AMDGPU_QUERY_FENCE_TIMEOUT_IS_ABSOLUTE (1 << 0) > >>> > >>> +/** > >>> + * Uses in amdgpu_device_initialize2(), meaning that the passed in fd > >>> should > >>> + * not be deduplicated against other libdrm_amdgpu devices referring to > >>> the > >>> + * same kernel device. > >>> + */ > >>> +#define AMDGPU_DEVICE_DEDICATED_FD (1 << 0) > >>> + > >>> > >>> /*--*/ > >>>/* - Enums > >>> */ > >>> > >>> /*--*/ > >>> @@ -526,6 +533,36 @@ int amdgpu_device_initialize(int fd, > >>> uint32_t *minor_version, > >>> amdgpu_device_handle *device_handle); > >>> > >>> +/** > >>> + * > >>> + * \param fd- \c [in] File descriptor for AMD GPU device > >>> + * received previously as the result of > >>> + * e.g. drmOpen() call. > >>> + * For legacy fd type, the DRI2/DRI3 > >>> + * authentication should be done before > >>> + * calling this function. > >>> + * \param flags - \c [in] Bitmask of flags for device > >>> creation. > >>> + * \param major_version - \c [out] Major version of library. It is > >>> assumed > >>> + * that adding new functionality will > >>> cause > >>> + * increase in major version > >>> + * \param minor_version - \c [out] Minor version of library > >>> + * \param device_handle - \c [out] Pointer to opaque context which > >>> should > >>> + * be passed as the first parameter on > >>> each > >>> + * API call > >>> + * > >>> + * > >>> + * \return 0 on success\n > >>> + * <0 - Negative POSIX Error code > >>> + * > >>> + * > >>> + * \sa amdgpu_device_deinitialize() > >>> +*/ > >>> +int amdgpu_device_initialize2(int fd, > >>> + uint32_t flags, > >>> +
[PATCH] drm/amd: fix typo
Fix spelling mistake: "lenght" -> "length"
Signed-off-by: Matteo Croce
---
drivers/gpu/drm/amd/include/atombios.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/include/atombios.h
b/drivers/gpu/drm/amd/include/atombios.h
index 7931502fa54f..8ba21747b40a 100644
--- a/drivers/gpu/drm/amd/include/atombios.h
+++ b/drivers/gpu/drm/amd/include/atombios.h
@@ -4106,7 +4106,7 @@ typedef struct _ATOM_LCD_MODE_CONTROL_CAP
typedef struct _ATOM_FAKE_EDID_PATCH_RECORD
{
UCHAR ucRecordType;
- UCHAR ucFakeEDIDLength; // = 128 means EDID lenght is 128 bytes,
otherwise the EDID length = ucFakeEDIDLength*128
+ UCHAR ucFakeEDIDLength; // = 128 means EDID length is 128 bytes,
otherwise the EDID length = ucFakeEDIDLength*128
UCHAR ucFakeEDIDString[1];// This actually has ucFakeEdidLength elements.
} ATOM_FAKE_EDID_PATCH_RECORD;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.
Am 07.01.19 um 14:05 schrieb Bas Nieuwenhuizen: > On Mon, Jan 7, 2019 at 1:23 PM Christian König > wrote: >> Am 06.01.19 um 21:29 schrieb Bas Nieuwenhuizen: >>> On Sun, Jan 6, 2019 at 9:23 PM Christian König >>> wrote: Am 06.01.19 um 10:46 schrieb Bas Nieuwenhuizen: > For radv we want to be able to pass in a master fd and be sure that > the created libdrm_amdgpu device also uses that master fd, so we can > use it for prioritized submission. > > radv does all interaction with other APIs/processes with dma-bufs, > so we should not need the functionality in libdrm_amdgpu to only have > a single fd for a device in the process. > > Signed-off-by: Bas Nieuwenhuizen Well NAK. This breaks a couple of design assumptions we used for the kernel driver and is strongly discouraged. >>> What assumptions are these? As far as I can tell everything is per drm >>> fd, not process? Instead radv should not use the master fd for command submission. >>> High priority submission needs to be done through a master fd >> That assumption is incorrect. See file >> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c to answer both your questions >> at the same time. > Hmm, I did not know about the AMDGPU_SCHED ioctl. That would work with > the permissions. However as it stands we cannot use it, as it is for > the entire drm-fd, not per context. > > Would you be open to a patch adding a context parameter to the struct > and a AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE? Certainly yeah. Overriding the whole process priority was never my favorite approach. Regards, Christian. PS: I'm at the start of a bad cold / sinusitis, so sorry if my responses are short and maybe delayed. > >> Additional to the scheduler priority we really don't want more than one >> fd in a process because of SVM binding overhead. >> >> So that whole approach is a really big NAK. >> >> Regards, >> Christian. >> >>> , so not >>> using a master fd is not an option ... >>> Regards, Christian. > --- > amdgpu/amdgpu-symbol-check | 1 + > amdgpu/amdgpu.h| 37 > amdgpu/amdgpu_device.c | 59 -- > 3 files changed, 76 insertions(+), 21 deletions(-) > > diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check > index 6f5e0f95..bbf48985 100755 > --- a/amdgpu/amdgpu-symbol-check > +++ b/amdgpu/amdgpu-symbol-check > @@ -56,6 +56,7 @@ amdgpu_cs_wait_fences > amdgpu_cs_wait_semaphore > amdgpu_device_deinitialize > amdgpu_device_initialize > +amdgpu_device_initialize2 > amdgpu_find_bo_by_cpu_mapping > amdgpu_get_marketing_name > amdgpu_query_buffer_size_alignment > diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h > index dc51659a..e5ed39bb 100644 > --- a/amdgpu/amdgpu.h > +++ b/amdgpu/amdgpu.h > @@ -66,6 +66,13 @@ struct drm_amdgpu_info_hw_ip; > */ > #define AMDGPU_QUERY_FENCE_TIMEOUT_IS_ABSOLUTE (1 << 0) > > +/** > + * Uses in amdgpu_device_initialize2(), meaning that the passed in fd > should > + * not be deduplicated against other libdrm_amdgpu devices referring to > the > + * same kernel device. > + */ > +#define AMDGPU_DEVICE_DEDICATED_FD (1 << 0) > + > > /*--*/ > /* - Enums > */ > > /*--*/ > @@ -526,6 +533,36 @@ int amdgpu_device_initialize(int fd, > uint32_t *minor_version, > amdgpu_device_handle *device_handle); > > +/** > + * > + * \param fd- \c [in] File descriptor for AMD GPU device > + * received previously as the result of > + * e.g. drmOpen() call. > + * For legacy fd type, the DRI2/DRI3 > + * authentication should be done before > + * calling this function. > + * \param flags - \c [in] Bitmask of flags for device > creation. > + * \param major_version - \c [out] Major version of library. It is > assumed > + * that adding new functionality will > cause > + * increase in major version > + * \param minor_version - \c [out] Minor version of library > + * \param device_handle - \c [out] Pointer to opaque context which > should > + * be passed as the first parameter on > each > + *
[PATCH] drm/omap: fix typo
Fix spelling mistake: "lenght" -> "length" Signed-off-by: Matteo Croce --- drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c index 813ba42f2753..e384b95ad857 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c @@ -708,7 +708,7 @@ int hdmi4_audio_config(struct hdmi_core_data *core, struct hdmi_wp_data *wp, else acore.i2s_cfg.justification = HDMI_AUDIO_JUSTIFY_RIGHT; /* -* The I2S input word length is twice the lenght given in the IEC-60958 +* The I2S input word length is twice the length given in the IEC-60958 * status word. If the word size is greater than * 20 bits, increment by one. */ -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/rockchip: Fix YUV buffers color rendering
Hi,
sorry, only now got to test this on actual hardware,
Am Freitag, 14. Dezember 2018, 17:29:20 CET schrieb Ezequiel Garcia:
> From: Daniele Castagna
>
> Currently, YUV hardware overlays are converted to RGB using
> a color space conversion different than BT.601.
>
> The result is that colors of e.g. NV12 buffers don't match
> colors of YUV hardware overlays.
>
> In order to fix this, enable YUV2YUV and set appropriate coefficients
> for formats such as NV12 to be displayed correctly.
>
> This commit was tested using modetest, gstreamer and chromeos (hardware
> accelerated video playback). Before the commit, tests rendering
> with NV12 format resulted in colors not displayed correctly.
>
> Test examples (RK3399 Ficus board connected to HDMI monitor):
>
> $ modetest 39@32:1920x1080@NV12
> $ gst-launch-1.0 videotestrc ! video/x-raw,format=NV12 ! kmssink
>
> Signed-off-by: Daniele Castagna
> [ezequiel: rebase on linux-next and massage commit log]
> Signed-off-by: Ezequiel Garcia
> ---
> v2:
> * Addressed feedback from Sean Paul
> * Rebased on linux-next
>
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 41 -
> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 13 +++
> drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 36 ++
> 3 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..78c7f63a60c0 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -52,6 +52,18 @@
> vop_reg_set(vop, &win->phy->scl->ext->name, \
> win->base, ~0, v, #name)
>
> +#define VOP_WIN_YUV2YUV_SET(x, win_yuv2yuv, name, v) \
> + do { \
> + if (win_yuv2yuv->name.mask) \
> + vop_reg_set(vop, &win_yuv2yuv->name, 0, ~0, v, #name); \
> + } while (0)
> +
> +#define VOP_WIN_YUV2YUV_COEFFICIENT_SET(x, win_yuv2yuv, name, v) \
> + do { \
> + if (win_yuv2yuv->phy->name.mask) \
> + vop_reg_set(vop, &win_yuv2yuv->phy->name,
> win_yuv2yuv->base, ~0, v, #name); \
> + } while (0)
> +
While this seems to work on rk3399, it hangs both my rk3328 (rock64)
and rk3288 (google-pinky) during rockchip-drm probe.
Making this something like
if (win_yuv2yuv && win_yuv2yuv->phy->name.mask) \
aka testing for existence of win_yuv2yuv first, makes them boot again,
so I guess I ran into a (for whatever reason) silent null-ptr-dereference.
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 08fc40af52c8..fe752df4e038 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -637,6 +637,34 @@ static const struct vop_output rk3399_output = {
> .mipi_dual_channel_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 3),
> };
>
> +static const struct vop_yuv2yuv_phy rk3399_yuv2yuv_win01_data = {
> + .y2r_coefficients = {
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 0, 0x, 0),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 0, 0x, 16),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 4, 0x, 0),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 4, 0x, 16),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 8, 0x, 0),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 8, 0x, 16),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 12, 0x, 0),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 12, 0x, 16),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 16, 0x, 0),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 20, 0x, 0),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 24, 0x, 0),
> + VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 28, 0x, 0),
> + },
> +};
> +
> +static const struct vop_yuv2yuv_phy rk3399_yuv2yuv_win23_data = { };
looking at the rk3399 TRM it seems that win2+3 also have yuv2rgb
coefficient registers. I didn't check in depth but are they so different
that they cannot be supported?
Aka what is the difference between win0/1 and win2/3 ?
Heiko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: "flip_done timed out" messages causing huge increase in boot time
On Sat, Jan 05, 2019 at 12:47:12AM +0100, Daniel Kamil Kozar wrote:
> Hello.
> After upgrading the kernel to 4.20, I noticed that the boot time
> increased from about 5 seconds to 25 seconds. My machine is an Asus
> K53SV with an Intel i7-2630QM, i.e. Sandy Bridge. I have an external
> display connected to it via HDMI. If the display is unplugged when
> booting the machine, the boot time stays at its old value.
> The kernel log shows two messages like this :
>
> [ 15.747206] [drm:drm_atomic_helper_wait_for_flip_done
> [drm_kms_helper]] *ERROR* [CRTC:39:pipe A] flip_done timed out
> [ 26.198968] [drm:drm_atomic_helper_wait_for_dependencies
> [drm_kms_helper]] *ERROR* [CRTC:39:pipe A] flip_done timed out
Hrm. With SNB I would be tempted to blame the LP3 watermarks, but
4.20 already has commit 21556350ade3 ("drm/i915: Disable LP3 watermarks
on all SNB machines").
Can you file a bug at
https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel
and attach the full dmesg from the boot with drm.debug=0xe passed to the
kernel cmdline?
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/komeda: Add d71_enum_resources and d71_cleanup
Hi James,
A few minor comments.
On Mon, Dec 24, 2018 at 08:58:46AM +, james qian wang (Arm Technology
China) wrote:
> D71 consists of a number of Register Blocks, every Block controls a
> specific HW function, every block has a common block_header to represent
> its type and pipeline information.
>
> GCU (Global Control Unit) is the first Block which describe the global
> information of D71 HW, Like number of block contained and the number of
> pipeline supported.
>
> So the d71_enum_resources parsed GCU and create pipeline according
> the GCU configuration, and then iterate and detect the blocks that
> indicated by the GCU and block_header.
>
> And this change also added two struct d71_dev/d71_pipeline to extend
> komeda_dev/komeda_pipeline to add some d71 only members.
>
> Signed-off-by: James (Qian) Wang
> ---
-- snip --
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> new file mode 100644
> index ..a43a2410159f
> --- /dev/null
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * (C) COPYRIGHT 2018 ARM Limited. All rights reserved.
> + * Author: James.Qian.Wang
> + *
> + */
> +#include "d71_dev.h"
> +#include "komeda_kms.h"
> +#include "malidp_io.h"
> +
> +static int d71_layer_init(struct d71_dev *d71,
> + struct block_header *blk, u32 __iomem *reg)
> +{
> + DRM_INFO("Detect D71_Layer.\n");
I think all of these can be DRM_DEBUG.
-- snip --
>
> static int d71_enum_resources(struct komeda_dev *mdev)
> {
> - /* TODO add enum resources */
> + struct d71_dev *d71;
> + struct komeda_pipeline *pipe;
> + struct block_header blk;
> + u32 __iomem *blk_base;
> + u32 i, value, offset;
> +
> + d71 = devm_kzalloc(mdev->dev, sizeof(*d71), GFP_KERNEL);
> + if (!d71)
> + return -ENOMEM;
> +
> + mdev->chip_data = d71;
> + d71->mdev = mdev;
> + d71->gcu_addr = mdev->reg_base;
> + d71->periph_addr = mdev->reg_base + (D71_BLOCK_OFFSET_PERIPH >> 2);
> +
> + if (d71_reset(d71)) {
> + DRM_ERROR("Fail to reset d71 device.\n");
> + goto err_cleanup;
> + }
> +
> + /* probe GCU */
> + value = malidp_read32(d71->gcu_addr, GLB_CORE_INFO);
> + d71->num_blocks = value & 0xFF;
> + d71->num_pipelines = (value >> 8) & 0x7;
> +
> + if (d71->num_pipelines > D71_MAX_PIPELINE) {
> + DRM_ERROR("d71 supports %d pipelines, but got: %d.\n",
> + D71_MAX_PIPELINE, d71->num_pipelines);
> + goto err_cleanup;
> + }
> +
> + /* probe PERIPH */
> + value = malidp_read32(d71->periph_addr, BLK_BLOCK_INFO);
> + if (BLOCK_INFO_BLK_TYPE(value) != D71_BLK_TYPE_PERIPH) {
> + DRM_ERROR("access blk periph but got blk: %d.\n",
> + BLOCK_INFO_BLK_TYPE(value));
> + goto err_cleanup;
> + }
> +
> + value = malidp_read32(d71->periph_addr, PERIPH_CONFIGURATION_ID);
> +
> + d71->max_line_size = value & PERIPH_MAX_LINE_SIZE ? 4096 : 2048;
> + d71->max_vsize = 4096;
> + d71->num_rich_layers= value & PERIPH_NUM_RICH_LAYERS ? 2 : 1;
> + d71->supports_dual_link = value & PERIPH_SPLIT_EN ? true : false;
> + d71->integrates_tbu = value & PERIPH_TBU_EN ? true : false;
> +
> + for (i = 0; i < d71->num_pipelines; i++) {
> + pipe = komeda_pipeline_add(mdev, sizeof(struct d71_pipeline),
> +NULL);
> + if (!pipe)
> + goto err_cleanup;
> +
> + d71->pipes[i] = to_d71_pipeline(pipe);
> + }
> +
> + /* loop the register blks and probe */
> + i = 2; /* exclude GCU and PERIPH */
> + offset = D71_BLOCK_SIZE; /* skip GCU */
> + while (i < d71->num_blocks) {
> + blk_base = mdev->reg_base + (offset >> 2);
> +
> + d71_read_block_header(blk_base, &blk);
> + if (BLOCK_INFO_BLK_TYPE(blk.block_info) !=
> D71_BLK_TYPE_RESERVED) {
> + if (d71_probe_block(d71, &blk, blk_base))
> + goto err_cleanup;
> + i++;
> + }
> +
> + offset += D71_BLOCK_SIZE;
> + }
> +
> + DRM_INFO("total %d (out of %d) blocks are found.\n",
> + i, d71->num_blocks);
> +
> + return 0;
> +
> +err_cleanup:
> + d71_cleanup(mdev);
> return -1;
-1 isn't a useful error code, and you return -ENOMEM if allocation
fails. It would probably be better to return proper codes everywhere
(e.g. you might get -EINVAL back from d71_probe_block, but you don't
propagate it).
-- snip --
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> new file mode 100644
> index ..2d5e6d00b42c
> --- /dev/null
> +++ b/dr
Re: [Intel-gfx] [PATCH v5 2/3] drm/i915: Move on the new pm runtime interface
Hi Tvrtko, On Mon, 31 Dec 2018 at 13:32, Tvrtko Ursulin wrote: > > > On 21/12/2018 13:26, Vincent Guittot wrote: > > On Fri, 21 Dec 2018 at 12:33, Tvrtko Ursulin [snip] > >> > >> If it is always monotonic, then worst case we report one wrong sample, > >> which I guess is still not ideal since someone could be querying the PMU > >> with quite low frequency. > >> > >> There are tests which probably can hit this, but to run them > >> automatically your patches would need to be rebased on drm-tip and maybe > >> sent to our trybot. I can do that after the holiday break if you are > >> okay with having the series waiting until then. > > > > yes looks good to me > > Looks good to me as well. And our CI agrees with it. So: > > Reviewed-by: Tvrtko Ursulin Thanks for the review and the test > > I assume you will take the patch through some power related tree and we > will eventually pull it back to drm-tip. Rafael, How do you want to proceed with this patch and the 2 others of the serie ? Regards, Vincent > > Regards, > > Tvrtko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 07/17] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay
On Fri, Dec 21, 2018 at 02:26:11AM +0530, Jagan Teki wrote: > On Tue, Dec 11, 2018 at 10:19 PM Maxime Ripard > wrote: > > > > On Mon, Dec 10, 2018 at 09:47:19PM +0530, Jagan Teki wrote: > > > Video start delay can be computed by subtracting total vertical > > > timing with front porch timing and with adding 1 delay line for TCON. > > > > > > BSP code form BPI-M64-bsp is computing video start delay as > > > (from linux-sunxi/ > > > drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) > > > > > > u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp; > > > => (panel->lcd_vt) - panel->lcd_y - (panel->lcd_vbp) > > > => (timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y) > > >- panel->lcd_y - (panel->lcd_vbp) > > > => timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y > > >- panel->lcd_y - panel->lcd_vbp > > > => timmings->ver_front_porch > > > > > > So, update the start delay computation accordingly. > > > > > > Signed-off-by: Jagan Teki > > > > Even though it's a bit better now on my A33 board and I don't have the > > white stripes on the bottom of the display, there's still some > > flickering with your patches applied. > > > > Bisecting it seems to point at that patch, but reverting it doesn't > > make the issue go away, so it's not really clear which one exactly is > > at fault. > > Is reverting this patch, work fine or not? As I was saying, it doesn't. > This patch is trying to make use of front porch instead of existing > back porch to compute the delay. Since we revert the VBP fix patch[1] > to assume VBP as VFP value look like your setup would also require to > revert this change. But as per BSP or drm_mode details all of my > displays even work with and w/o reverting these two. Again, I cannot help you without the datasheet for the panels you're trying to submit. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/2] drm/sched: Refactor ring mirror list handling.
Am 03.01.19 um 18:42 schrieb Grodzovsky, Andrey:
On 01/03/2019 11:20 AM, Grodzovsky, Andrey wrote:
On 01/03/2019 03:54 AM, Koenig, Christian wrote:
Am 21.12.18 um 21:36 schrieb Grodzovsky, Andrey:
On 12/21/2018 01:37 PM, Christian König wrote:
Am 20.12.18 um 20:23 schrieb Andrey Grodzovsky:
Decauple sched threads stop and start and ring mirror
list handling from the policy of what to do about the
guilty jobs.
When stoppping the sched thread and detaching sched fences
from non signaled HW fenes wait for all signaled HW fences
to complete before rerunning the jobs.
v2: Fix resubmission of guilty job into HW after refactoring.
v4:
Full restart for all the jobs, not only from guilty ring.
Extract karma increase into standalone function.
v5:
Rework waiting for signaled jobs without relying on the job
struct itself as those might already be freed for non 'guilty'
job's schedulers.
Expose karma increase to drivers.
Suggested-by: Christian Koenig
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 +--
drivers/gpu/drm/etnaviv/etnaviv_sched.c | 11 +-
drivers/gpu/drm/scheduler/sched_main.c | 188
+++--
drivers/gpu/drm/v3d/v3d_sched.c | 12 +-
include/drm/gpu_scheduler.h | 10 +-
5 files changed, 151 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 8a078f4..a4bd2d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3298,12 +3298,10 @@ static int
amdgpu_device_pre_asic_reset(struct amdgpu_device *adev,
if (!ring || !ring->sched.thread)
continue;
- kthread_park(ring->sched.thread);
+ drm_sched_stop(&ring->sched, job ? &job->base : NULL);
- if (job && job->base.sched != &ring->sched)
- continue;
-
- drm_sched_hw_job_reset(&ring->sched, job ? &job->base : NULL);
+ if(job)
+ drm_sched_increase_karma(&job->base);
Since we dropped the "job && job->base.sched != &ring->sched" check
above this will now increase the jobs karma multiple times.
Maybe just move that outside of the loop.
/* after all hw jobs are reset, hw fence is meaningless,
so force_completion */
amdgpu_fence_driver_force_completion(ring);
@@ -3454,14 +3452,10 @@ static void
amdgpu_device_post_asic_reset(struct amdgpu_device *adev,
if (!ring || !ring->sched.thread)
continue;
- /* only need recovery sched of the given job's ring
- * or all rings (in the case @job is NULL)
- * after above amdgpu_reset accomplished
- */
- if ((!job || job->base.sched == &ring->sched) &&
!adev->asic_reset_res)
- drm_sched_job_recovery(&ring->sched);
+ if (!adev->asic_reset_res)
+ drm_sched_resubmit_jobs(&ring->sched);
- kthread_unpark(ring->sched.thread);
+ drm_sched_start(&ring->sched, !adev->asic_reset_res);
}
if (!amdgpu_device_has_dc_support(adev)) {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 49a6763..6f1268f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -109,16 +109,19 @@ static void etnaviv_sched_timedout_job(struct
drm_sched_job *sched_job)
}
/* block scheduler */
- kthread_park(gpu->sched.thread);
- drm_sched_hw_job_reset(&gpu->sched, sched_job);
+ drm_sched_stop(&gpu->sched, sched_job);
+
+ if(sched_job)
+ drm_sched_increase_karma(sched_job);
/* get the GPU back into the init state */
etnaviv_core_dump(gpu);
etnaviv_gpu_recover_hang(gpu);
+ drm_sched_resubmit_jobs(&gpu->sched);
+
/* restart scheduler after GPU is usable again */
- drm_sched_job_recovery(&gpu->sched);
- kthread_unpark(gpu->sched.thread);
+ drm_sched_start(&gpu->sched, true);
}
static void etnaviv_sched_free_job(struct drm_sched_job *sched_job)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index dbb6906..b5c5bee 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -60,8 +60,6 @@
static void drm_sched_process_job(struct dma_fence *f, struct
dma_fence_cb *cb);
-static void drm_sched_expel_job_unlocked(struct drm_sched_job
*s_job);
-
/**
* drm_sched_rq_init - initialize a given run queue struct
*
@@ -335,6 +333,42 @@ static void drm_sched_job_timedout(struct
work_struct *work)
spin_unlock_irqrestore(&sched->job_list_lock, flags);
}
Kernel doc here would be nice to have.
+void drm_sched_increase_karma(struct drm_sched_job *bad)
+{
+ int i;
+ struct drm_sched_entity *tmp;
+ st
Re: [Intel-gfx] [PATCH v5 2/3] drm/i915: Move on the new pm runtime interface
On Mon, Jan 7, 2019 at 3:04 PM Vincent Guittot wrote: > > Hi Tvrtko, > > On Mon, 31 Dec 2018 at 13:32, Tvrtko Ursulin > wrote: > > > > > > On 21/12/2018 13:26, Vincent Guittot wrote: > > > On Fri, 21 Dec 2018 at 12:33, Tvrtko Ursulin > > [snip] > > > >> > > >> If it is always monotonic, then worst case we report one wrong sample, > > >> which I guess is still not ideal since someone could be querying the PMU > > >> with quite low frequency. > > >> > > >> There are tests which probably can hit this, but to run them > > >> automatically your patches would need to be rebased on drm-tip and maybe > > >> sent to our trybot. I can do that after the holiday break if you are > > >> okay with having the series waiting until then. > > > > > > yes looks good to me > > > > Looks good to me as well. And our CI agrees with it. So: > > > > Reviewed-by: Tvrtko Ursulin > > Thanks for the review and the test > > > > > I assume you will take the patch through some power related tree and we > > will eventually pull it back to drm-tip. > > Rafael, > How do you want to proceed with this patch and the 2 others of the serie ? I'm going to queue up the whole series for 5.1. Thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 Bug ID: 109239 Summary: Polaris10: Periodic random black screens for 1-2 seconds Product: DRI Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: rocketra...@gmail.com Created attachment 142997 --> https://bugs.freedesktop.org/attachment.cgi?id=142997&action=edit Xorg.0.log with modesetting I have 3 Dell WQHD Screens (2560x1440) screens, connected to a Radeon RX580 (XFX, RX-580 GTS Black Edition, 1425 MHz 8GB), running on Fedora 29. My kernel is: Linux edison 4.19.13-300.fc29.x86_64 #1 SMP Sat Dec 29 22:54:28 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux On 2/3 monitors I get periodic random black screens for 1-2 seconds. The display comes back and all appears normal after this happens. I have tried both with and without modesetting enabled. Nothing at all appears in either the kernel log when this happens, or in Xorg.0.log. I have attached complete Xorg.0.log's and the relevant parts of dmesg (from boot to the very last drm-related message). I was sure this was a hardware issue, and after a debugging process with XFX support in which they had me swapping monitors and cables, they RMAed my card and sent me a new one. However, the new card has very similar behavior (the old card seemed to black screen only one monitor, the new seems to do two of them, but in exactly the same way, and I have also updated my kernel and OS since then, so its possible the hardware is completely unrelated to the minor change in behavior). Given the change in hardware, it seems likely to me this is a driver bug rather than a hardware one. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 --- Comment #1 from Raman Gupta --- Created attachment 142998 --> https://bugs.freedesktop.org/attachment.cgi?id=142998&action=edit Xorg.0.log without modesetting -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] Meson (32-bit): add support for the Mali GPU
On 08/12/2018 18:12, Martin Blumenstingl wrote: > This series adds support for the Mali-450 GPU on Meson8 and Meson8b. > Meson6 uses a Mali-400 GPU but since we don't have a clock driver (and > I don't have a device for testing) Meson6 is left out in this series. > > Meson8 uses a Mali-450 MP6 with six pixel processors. Meson8b (as > cost-reduced SoC) uses a Mali-450 MP2 with two pixel processors. > I tested both using the open source lima driver and a patched mesa > from the lima project as well. Since we don't have display support > on the 32-bit SoCs I used off-screen rendering as described in [0]. > The result is (for example): [1] > > The bootloader (at least on my boards) leaves the Mali clock disabled > by default. The board crashes when trying to access the Mali registers > with the "core" clock disabled. > Thus this series also implements the required clock driver changes. The > Mali clock tree on Meson8b and Meson8m2 is almost identical to the one > on GXBB (see the description of patch #3 for more details). Only Meson8 > is slightly different as it doesn't have a glitch-free mux. Patch #2 > prepares the meson8b clock driver so we can have different clocks per > SoC. > > Dependencies: > - the .dts changes depend on my other series "ARM: dts: meson: add the > APB/APB2 busses" from [2] > - the .dts changes from this series have no compile-time dependency on > the clock driver changes because CLKID_MALI was defined in the > dt-bindings since the first version of the clock driver (but it was > not used until now). > - the .dts changes from this series have a runtime dependency on the > clock driver changes (also from this series) if you have a kernel > patched with the lima driver (without the lima driver there's no > runtime dependency) > > Other notes: > By default the GPU runs off the XTAL clock (24MHz). The lima driver > currently does not update the GPU clock rate. Different frequencies > have to be requested by adding the following properties to the Mali > GPU node (to run it at 510MHz for example): > assigned-clocks = <&clkc CLKID_MALI>; > assigned-clock-rates = <51000>; > > > [0] https://gitlab.freedesktop.org/lima/web/wikis/home > [1] https://abload.de/img/dump0myic0.png > [2] https://patchwork.kernel.org/cover/10719445/ > > > Martin Blumenstingl (5): > dt-bindings: gpu: mali-utgard: add Amlogic Meson8 and Meson8b > compatible > clk: meson: meson8b: use a separate clock table for Meson8 > clk: meson: meson8b: add the GPU clock tree > ARM: dts: meson8: add the Mali-450 MP6 GPU > ARM: dts: meson8b: add the Mali-450 MP2 GPU > > .../bindings/gpu/arm,mali-utgard.txt | 6 + > arch/arm/boot/dts/meson8.dtsi | 58 +++ > arch/arm/boot/dts/meson8b.dtsi| 46 +++ > drivers/clk/meson/meson8b.c | 349 +- > drivers/clk/meson/meson8b.h | 9 +- > 5 files changed, 461 insertions(+), 7 deletions(-) > Applied patches 2 & 3 to next/drivers for Linux 5.1 Kevin, have fun with the other patches ! Neil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 --- Comment #2 from Raman Gupta --- Created attachment 142999 --> https://bugs.freedesktop.org/attachment.cgi?id=142999&action=edit dmesg with modesetting -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 --- Comment #3 from Raman Gupta --- Created attachment 143000 --> https://bugs.freedesktop.org/attachment.cgi?id=143000&action=edit dmesg without modesetting -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/10] drm/etnaviv: mmuv2: don't map zero page
Am Montag, den 07.01.2019, 10:13 +0100 schrieb Guido Günther: > Hi, > On Mon, Jan 07, 2019 at 09:50:52AM +0100, Lucas Stach wrote: > > Hi Guido, > > > > Am Sonntag, den 30.12.2018, 16:49 +0100 schrieb Guido Günther: > > > Hi Lucas, > > > On Wed, Dec 19, 2018 at 03:45:38PM +0100, Lucas Stach wrote: > > > > Keep the page at address 0 as faulting to catch any potential state > > > > setup issues early. > > > > > > This is a nice idea! But applying this and making mesa hit that page > > > leads to the process hanging in D state over here on GC7000: > > > > > > # [ 242.726192] INFO: task kworker/u8:2:37 blocked for more than 120 > > > seconds. > > > [ 242.733010] Not tainted 4.18.0-00129-gce2b21074b41 #504 > > > [ 242.738795] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" > > > disables this message. > > > [ 242.746638] kworker/u8:2D037 2 0x0028 > > > [ 242.752144] Workqueue: events_unbound commit_work > > > [ 242.756860] Call trace: > > > [ 242.759318] __switch_to+0x94/0xd0 > > > [ 242.762741] __schedule+0x1c0/0x6b8 > > > [ 242.766239] schedule+0x40/0xa8 > > > [ 242.769380] schedule_timeout+0x2f0/0x428 > > > [ 242.773410] dma_fence_default_wait+0x1cc/0x2b8 > > > [ 242.777951] dma_fence_wait_timeout+0x44/0x1b0 > > > [ 242.782403] drm_atomic_helper_wait_for_fences+0x48/0x108 > > > [ 242.787819] commit_tail+0x30/0x80 > > > [ 242.791229] commit_work+0x20/0x30 > > > [ 242.794642] process_one_work+0x1ec/0x458 > > > [ 242.798659] worker_thread+0x48/0x430 > > > [ 242.802331] kthread+0x130/0x138 > > > [ 242.805557] ret_from_fork+0x10/0x1c > > > > > > This is in dmesg showing that we hit the first page: > > > > > > [ 65.907388] etnaviv-gpu 3800.gpu: MMU fault status 0x0002 > > > [ 65.913497] etnaviv-gpu 3800.gpu: MMU 0 fault addr 0x0e40 > > > > > > Without that patch it's sampling random data from that page but does not > > > hang. > > > > GPU hangs after a MMU fault are expected or more accurately, we > > actively request the GPU to stop by setting the exception bit in the > > page table. > > Yeah. I put that in to show that this the cause for the trouble above. > > > > > A hanging GPU should trigger the scheduler timeout handler, which then > > makes sure to get the GPU back into a working state. So if things don't > > progress after the fault for you either the timeout handler is buggy on > > GC7000, or the fence signaling is broken somehow. I'll take a look at > > this. > > This isn't a top notch linux-next based tree yet so if you're not seeing this > let me forward port our stuff to that and report back again. I've certainly seen the timeout handler working on GC7000, but with the GC7000 support being relatively lightly tested right now, I wouldn't bet on us handling all corner cases correctly. If this is an issue on a recent kernel, I would certainly love to learn what's going wrong. Regards, Lucas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201815] Mouse Pointer Disappears when touching top of the screen
https://bugzilla.kernel.org/show_bug.cgi?id=201815 --- Comment #12 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) --- There's a fix for this bug coming soon. The bug only affects Raven systems and was a regression introduced in 4.20. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 07/17] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay
On Mon, Jan 7, 2019 at 7:42 PM Maxime Ripard wrote: > > On Fri, Dec 21, 2018 at 02:26:11AM +0530, Jagan Teki wrote: > > On Tue, Dec 11, 2018 at 10:19 PM Maxime Ripard > > wrote: > > > > > > On Mon, Dec 10, 2018 at 09:47:19PM +0530, Jagan Teki wrote: > > > > Video start delay can be computed by subtracting total vertical > > > > timing with front porch timing and with adding 1 delay line for TCON. > > > > > > > > BSP code form BPI-M64-bsp is computing video start delay as > > > > (from linux-sunxi/ > > > > drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) > > > > > > > > u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp; > > > > => (panel->lcd_vt) - panel->lcd_y - (panel->lcd_vbp) > > > > => (timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y) > > > >- panel->lcd_y - (panel->lcd_vbp) > > > > => timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y > > > >- panel->lcd_y - panel->lcd_vbp > > > > => timmings->ver_front_porch > > > > > > > > So, update the start delay computation accordingly. > > > > > > > > Signed-off-by: Jagan Teki > > > > > > Even though it's a bit better now on my A33 board and I don't have the > > > white stripes on the bottom of the display, there's still some > > > flickering with your patches applied. > > > > > > Bisecting it seems to point at that patch, but reverting it doesn't > > > make the issue go away, so it's not really clear which one exactly is > > > at fault. > > > > Is reverting this patch, work fine or not? > > As I was saying, it doesn't. > > > This patch is trying to make use of front porch instead of existing > > back porch to compute the delay. Since we revert the VBP fix patch[1] > > to assume VBP as VFP value look like your setup would also require to > > revert this change. But as per BSP or drm_mode details all of my > > displays even work with and w/o reverting these two. > > Again, I cannot help you without the datasheet for the panels you're > trying to submit. The panel bound with Sitronix ST7701 IC http://www.startek-lcd.com/res/starteklcd/pdres/201705/20170512144242904.pdf ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 199139] System freezes (kernel, amdgpu NULL pointer dereference) when video enters powersafe state
https://bugzilla.kernel.org/show_bug.cgi?id=199139 --- Comment #23 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) --- Does this still occur on kernels for 4.21 and higher? It isn't directly related to power management, but there are many of these classes of edge cases with atomic commit flow that get fixed with the following patches: https://patchwork.freedesktop.org/patch/263411/ https://patchwork.freedesktop.org/series/53324/ So I would be interested in knowing whether this is a separate issue. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/14] drm: make drm_framebuffer.h self contained
Den 30.12.2018 18.48, skrev Sam Ravnborg: > Add forward declaration and pull in include > file to make drm_framebuffer.h self contained. > > While add it order include files alphabetically. > > The use of TASK_COMM_LEN is the reason for including sched.h. > I could not see any good way to avoid this dependency, > and users of drm_framebuffer.comm already use > TASK_COMM_LEN to check for length etc. We can't avoid including it, the macro is used here after all. > > Signed-off-by: Sam Ravnborg > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > --- > include/drm/drm_framebuffer.h | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h > index c94acedfb08e..f639ed527943 100644 > --- a/include/drm/drm_framebuffer.h > +++ b/include/drm/drm_framebuffer.h > @@ -23,13 +23,16 @@ > #ifndef __DRM_FRAMEBUFFER_H__ > #define __DRM_FRAMEBUFFER_H__ > > -#include > #include > +#include > +#include > + > #include > > struct drm_framebuffer; > struct drm_file; > struct drm_device; > +struct drm_clip_rect; I think you can add drm_gem_object to this list. > > /** > * struct drm_framebuffer_funcs - framebuffer hooks > Acked-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 05/14] drm: make drm_file.h self contained
Den 30.12.2018 18.48, skrev Sam Ravnborg: > drm_file.h embed struct idr, so this file need to know > the full type definition. > > With this change users of drm_file.h are no longer forced > to include idr.h - a file they usually get from drmP.h > > This makes it simpler to remove drmP.h includes > > Signed-off-by: Sam Ravnborg > Cc: Jani Nikula > Cc: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > --- Acked-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/14] drm: remove include of drmP.h from drm_encoder_slave.h
Den 30.12.2018 18.48, skrev Sam Ravnborg: > No further changes required. > > Signed-off-by: Sam Ravnborg > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > --- Acked-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Expose more EDID fields to userspace
Daniel Vetter writes: > Best to pull in some other compositor people and get them to agree. From a > kernel pov I'm fine with whatever userspace preferes. Hrm. It would be good to have everyone using the same interpretation of EDID data; in particular, where the kernel has quirks that change the interpretation, user space should be consistent with that. Unless we expose all of the EDID data, then user space may still have to parse EDID. If the kernel has EDID quirks, it might be good to to make those affect the "raw" EDID data visible to use space so that values the kernel supplies separately are consistent with values extracted from the "raw" EDID data. Doing this in the kernel does make it harder to quickly supply fixes for a specific user space application. This will probably lead to kludge-arounds in user space that could depend on kernel version. Perhaps these EDID capabilities in the kernel should be versioned separately? I see good benefits from having user space able to see how the kernel is interpreting EDID so that it can adapt as appropriate, but we should be cautious about moving functionality into the kernel that would be more easily maintained up in user space. -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/14] drm/arc: do not reply on drmP.h from drm_gem_cma_helper.h
Den 30.12.2018 18.48, skrev Sam Ravnborg: > drmP.h was the only header file in the past and a lot > of files rely on that drmP.h defines everything. > The goal is to one day to delete drmP.h and > as a step towards this it will no longer be included in the > headers files in include/drm/ > > To prepare arc/ for this add dependencies that > othwewise was pulled in by drmP.h from drm_gem_cma_helper.h > > Signed-off-by: Sam Ravnborg > Cc: Alexey Brodkin > Cc: Daniel Vetter > Cc: David Airlie > --- Acked-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 12/14] drm/stm: do not reply on drmP.h from drm_gem_cma_helper.h
Den 30.12.2018 18.48, skrev Sam Ravnborg: > drmP.h was the only header file in the past and a lot > of files rely on that drmP.h defines everything. > The goal is to one day to delete drmP.h and > as a step towards this it will no longer be included in the > headers files in include/drm/ > > To prepare stm/ for this add dependencies that > othwewise was pulled in by drmP.h from drm_gem_cma_helper.h > > Sort the include list when we anyway modify it. > > Signed-off-by: Sam Ravnborg > Cc: Yannick Fertre > Cc: Philippe Cornu > Cc: Benjamin Gaignard > Cc: Vincent Abriou > Cc: David Airlie > Cc: Daniel Vetter > --- Acked-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109238] [DC] Polaris10: Missing modes when enabling DC
https://bugs.freedesktop.org/show_bug.cgi?id=109238 Alex Deucher changed: What|Removed |Added Attachment #142991|text/x-log |text/plain mime type|| -- You are receiving this mail because: You are the assignee for the bug. You are on the CC list for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 14/14] drm: remove drmP.h from drm_gem_cma_helper.h
Den 30.12.2018 18.48, skrev Sam Ravnborg: > With all dependencies fixed we can now remove > drmP.h from drm_gem_cma_helper.h. > It is replaced by the include files required, > or forward declarations as appropritate. > > Signed-off-by: Sam Ravnborg > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > --- Acked-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109238] [DC] Polaris10: Missing modes when enabling DC
https://bugs.freedesktop.org/show_bug.cgi?id=109238 Alex Deucher changed: What|Removed |Added Attachment #142992|text/x-log |text/plain mime type|| -- You are receiving this mail because: You are on the CC list for the bug. You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109238] [DC] Polaris10: Missing modes when enabling DC
https://bugs.freedesktop.org/show_bug.cgi?id=109238 Alex Deucher changed: What|Removed |Added Attachment #142994|text/x-log |text/plain mime type|| -- You are receiving this mail because: You are on the CC list for the bug. You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109238] [DC] Polaris10: Missing modes when enabling DC
https://bugs.freedesktop.org/show_bug.cgi?id=109238 Alex Deucher changed: What|Removed |Added Attachment #142993|text/x-log |text/plain mime type|| -- You are receiving this mail because: You are on the CC list for the bug. You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Armada DRM: bridge with componentized devices
On Mon, Jan 07, 2019 at 12:26:58PM +0100, Andrzej Hajda wrote: > On 07.01.2019 11:45, Daniel Vetter wrote: > > On Thu, Jan 03, 2019 at 01:11:47PM +, Russell King - ARM Linux wrote: > >> On Thu, Jan 03, 2019 at 10:47:27AM +0100, Lubomir Rintel wrote: > >>> Hello, > >>> > >>> lately I've been trying to make the Himax HX8837 chip that drives the OLPC > >>> LCD display work with Armada DRM driver. I've been advised to create a > >>> bridge driver and not an encoder driver since the silicon is separate from > >>> the LCDC. > >>> > >>> The Armada DRM driver (and, I think, the i.MX one) creates the drm_device > >>> once the component infrastructure sees the necessary sub-devices appear. > >>> The sub-devices being the LCDCs and the encoders (not bridges) that it > >>> expects to be created externally. > >>> > >>> Currently, it seems, the only driver that can actually work with this > >>> (that > >>> is -- creates a drm_encoder for a drm_device when the component is bound) > >>> is the tda998x. All other similar drivers create a drm_bridge instead and > >>> not use the component infrastructure at all. (In fact, tilcdc driver > >>> contains a hack to handle tda998x specially.) > >>> > >>> I'm wondering how to reconcile the two? > >>> > >>> * The tda998x driver has recently been modified to create a bridge on > >>> probe > >>> and eventually encoder on component bind. Is this an okay thing to do in > >>> a new driver? (this probably means the tilcdc hack can be removed...) > >>> > >>> * If a non-componentized bridge were to be used (along with a dummy > >>> encoder), at what point would it make sense to look for the bridge? > >>> Would it be a good idea to defer the probe of crtc until a bridge can > >>> be > >>> looked up and the attach it on component bind? What if the bridge goes > >>> away (a module is unloaded, etc.) in between? > >>> > >>> I'd be thankful for opintions and advice before I move ahead with this. > >> This is the long-standing problem with the conflict between bridge > >> support and component support, and I'm not sure that there is really > >> any answer to it. > >> > >> I've gone into the details of the two several times on the list, > >> particularly about the short-comings of the bridge approach, but it > >> seems no one cares to fix those short-comings. > >> > >> You are re-identifying some of the issues that I've already pointed > >> out - such as what happens to DRM drives when the bridge driver is > >> unbound (it's really not about modules being unloaded, and the problem > >> can't be solved by taking a module reference count - all that the > >> module reference count does is ensure that the module doesn't go > >> away unexpected, there is no way to ensure that the device isn't > >> unbound.) > >> > >> The issue of unbinding is precisely the issue which the component > >> support was created to solve - but everyone seems to prefer the buggy > >> bridge approach, and no one seems willing to do anything about the > >> bugs or even acknowledge that it's a problem. It's strange - if one > >> identifies bugs that result in kernel oops in other kernel subsystems, > >> one is generally taken seriously and the problem is solved. > > Unbinding is really not the most important feature, especially for SoC. If > > you feel different, working together with others, getting some agreement, > > getting the patches reviewed and finding someone to get them merged is > > very much appreciated. But just complaining won't move this forward. > > > >> The issue about the encoders is something that I've tried to discuss, > >> and I've pointed out that moving it into the DRM driver adds additional > >> complexity there, and I'd hoped that my patch set I posted would've > >> generated discussion, but alas not. > >> > >> What I'm not prepared to do is to introduce _known_ bugs into any > >> driver that I maintain. > > I thought last time around the idea was to use device links (and teach > > drm_bridge about them), so that the unloading works correctly. > > > With current device_links implementation registering links in probe of > the consumer is just incorrect - it can happen that neither consumer > neither provider is fully probed and creating device links in such state > is wrong according to docs, and my experiments. See [1] for discussion > and [2] for docs. We could set up the device link only at drm_dev_register time. At that point we know that driver loading has fully succeeded. We do have a list of bridges at hand, so that's not going to be an issue. The only case where this breaks is if a driver is still using the deprecated ->load callback. But that ->load callback doesn't mix well with EDEFER_PROBE/component framework anyway, so I think not going to be a problem hopefully? -Daniel > > > [1]: https://patchwork.freedesktop.org/patch/218878/ > > [2]: > https://www.kernel.org/doc/html/latest/driver-api/device_link.html#usage > > > Regards > > Andrzej > > > > > > Wrt td
Re: [PATCH v2 13/14] drm/tinydrm: do not reply on drmP.h from drm_gem_cma_helper.h
Den 30.12.2018 18.48, skrev Sam Ravnborg: > drmP.h was the only header file in the past and a lot > of files rely on that drmP.h defines everything. > The goal is to one day to delete drmP.h and > as a step towards this it will no longer be included in the > headers files in include/drm/ > > To prepare tinydrm/ for this add dependencies that > othwewise was pulled in by drmP.h from drm_gem_cma_helper.h > > To avoid that tinydrm.h became "include everything", > push include files to the individual drivers. > > Signed-off-by: Sam Ravnborg > Cc: "Noralf Trønnes" > Cc: David Airlie > Cc: Eric Anholt > Cc: David Lechner > Cc: Daniel Vetter > --- Thanks for doing this dep fix series: Reviewed-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/1] drm/prime: Use sg_dma_len() macro to get sg's length
On Mon, Jan 07, 2019 at 05:37:08PM +0530, Vivek Gautam wrote: > After mapping a sg list the we should use sg_dma_address() and > sg_dma_len() macros to access sg->address and sg->length. Fix > the same for sg->length in drm_prime_sg_to_page_addr_arrays(). > > Signed-off-by: Vivek Gautam > --- > > Changes since v1: > - Fixed compilation error: replaced sg_dma_length() with sg_dma_len(). > > This came while debugging one dmabuf import issue that we are seeing > on sdm845 target. > The dmabuf which is prepared by video (venus in this case), is imported > by drm device. > The import call flow looks like follows: > > drm_gem_prime_import() > - drm_gem_prime_import_dev() >- dma_buf_attach() & dma_buf_map_attachment() > - From dma_buf_map_attachment() >- vb2_dma_sg_dmabuf_ops_map() > - dma_map_sg(): this updates the sg->nents. > > From debugging, the sg table mapping results in sg's 'nents' to be less that > the original nents. Now drm device prepares the page information based on > this sg table, and messes up with the mappings, and we start seeing random > crashes as below from drm's memory space. > > Although this change isn't helping to fix the issue currently, but > this fix seems the right thing to do. > > One thing to notice is that, if we restore the sg->nents to > sg->orig_nents in vb2_dma_sg_dmabuf_ops_map(), we don't see the below > corruptions. > > Any pointers on this will be highly appreciated. > Thanks. > > -- > [ 338.070558] Unable to handle kernel paging request at virtual address > 4038 > [ 338.078751] Mem abort info: > [ 338.081671] ESR = 0x9604 > [ 338.084860] Exception class = DABT (current EL), IL = 32 bits > [ 338.090972] SET = 0, FnV = 0 > [ 338.094139] EA = 0, S1PTW = 0 > [ 338.097393] Data abort info: > [ 338.100375] ISV = 0, ISS = 0x0004 > [ 338.104362] CM = 0, WnR = 0 > [ 338.107446] [4038] address between user and kernel address > ranges > [ 338.114801] Internal error: Oops: 9604 [#1] PREEMPT SMP > [ 338.120527] Modules linked in: rfcomm uinput cdc_ether venus_dec venus_enc > usbnet videobuf2_dma_sg videobuf2_memops hci_uart btqca bluetooth r8152 mii > ath10k_snoc venus_core ath10k_core v4l2_mem2mem videobuf2_v4l2 > videobuf2_common ath mac80211 ecdh_generic qcom_q6v5_mss lzo lzo_compress > qcom_q6v5_adsp qcom_common qcom_q6v5 zram bridge stp llc ipt_MASQUERADE fuse > snd_seq_dummy snd_seq snd_seq_device cfg80211 joydev > [ 338.158192] CPU: 4 PID: 3235 Comm: chrome Tainted: GW > 4.19.0 #2 > [ 338.165700] Hardware name: Google Cheza (rev1) (DT) > [ 338.170720] pstate: 8049 (Nzcv daif +PAN -UAO) > [ 338.175660] pc : drm_mm_insert_node_in_range+0xfc/0x348 > [ 338.181035] lr : drm_mm_insert_node_in_range+0x24/0x348 > [ 338.186407] sp : ff8013033b30 > [ 338.189816] x29: ff8013033bd0 x28: ff8008591894 > [ 338.195275] x27: 0010 x26: > [ 338.200734] x25: x24: > [ 338.206194] x23: x22: ffc0f48b7e08 > [ 338.211656] x21: x20: 005d > [ 338.217118] x19: x18: > [ 338.222581] x17: x16: > [ 338.228046] x15: x14: > [ 338.233511] x13: 0001 x12: ffc0b1da7200 > [ 338.238978] x11: 0010 x10: 0010 > [ 338.244437] x9 : 0008 x8 : 4000 > [ 338.249898] x7 : x6 : > [ 338.255361] x5 : x4 : > [ 338.260823] x3 : x2 : 005d > [ 338.266285] x1 : ffc0b1da7100 x0 : ffc0b0215800 > [ 338.271748] Process chrome (pid: 3235, stack limit = 0x0900f416) > [ 338.278628] Call trace: > [ 338.281151] drm_mm_insert_node_in_range+0xfc/0x348 > [ 338.286168] msm_gem_map_vma+0x60/0xdc > [ 338.290022] msm_gem_get_iova+0xb4/0xf4 > [ 338.293967] msm_ioctl_gem_info+0x90/0xdc > [ 338.298089] drm_ioctl_kernel+0xa8/0xe8 > [ 338.302043] drm_ioctl+0x218/0x384 > [ 338.305547] drm_compat_ioctl+0xd8/0xe8 > [ 338.309503] __arm64_compat_sys_ioctl+0x134/0x20c > [ 338.314339] el0_svc_common+0xa0/0xf0 > [ 338.318108] el0_svc_compat_handler+0x2c/0x38 > [ 338.322588] el0_svc_compat+0x8/0x18 > [ 338.326274] Code: f94066c8 aa1f03e0 321d03e9 321c03ea (f9401d0b) > [ 338.332538] ---[ end trace 5c09e60869887d87 ]--- > [ 338.354633] Kernel panic - not syncing: Fatal exception > [ 338.360018] SMP: stopping secondary CPUs > [ 338.364179] Kernel Offset: disabled > [ 338.367779] CPU features: 0x0,22802a18 > [ 338.371643] Memory Limit: none > -- > > drivers/gpu/drm/drm_prime.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 231e3f6d5f41..aa87ba9c0d7b 100644 > --- a/drivers/gpu/drm/drm_prime.c > +
Re: Armada DRM: bridge with componentized devices
On Mon, Jan 07, 2019 at 11:45:32AM +0100, Daniel Vetter wrote: > On Thu, Jan 03, 2019 at 01:11:47PM +, Russell King - ARM Linux wrote: > > This is the long-standing problem with the conflict between bridge > > support and component support, and I'm not sure that there is really > > any answer to it. > > > > I've gone into the details of the two several times on the list, > > particularly about the short-comings of the bridge approach, but it > > seems no one cares to fix those short-comings. > > > > You are re-identifying some of the issues that I've already pointed > > out - such as what happens to DRM drives when the bridge driver is > > unbound (it's really not about modules being unloaded, and the problem > > can't be solved by taking a module reference count - all that the > > module reference count does is ensure that the module doesn't go > > away unexpected, there is no way to ensure that the device isn't > > unbound.) > > > > The issue of unbinding is precisely the issue which the component > > support was created to solve - but everyone seems to prefer the buggy > > bridge approach, and no one seems willing to do anything about the > > bugs or even acknowledge that it's a problem. It's strange - if one > > identifies bugs that result in kernel oops in other kernel subsystems, > > one is generally taken seriously and the problem is solved. > > Unbinding is really not the most important feature, especially for SoC. If > you feel different, working together with others, getting some agreement, > getting the patches reviewed and finding someone to get them merged is > very much appreciated. But just complaining won't move this forward. Sorry, I disagree. Unbinding is important if the current state results in crashes and oops - the lack of unbinding support in bridge makes it harder to develop without constantly rebooting the target machine. If all you care about is the end user who probably never removes a module, then yes, it's low priority, but if you care about efficient development, then the story is rather different. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Armada DRM: bridge with componentized devices
On 07.01.2019 17:08, Daniel Vetter wrote: > On Mon, Jan 07, 2019 at 12:26:58PM +0100, Andrzej Hajda wrote: >> On 07.01.2019 11:45, Daniel Vetter wrote: >>> On Thu, Jan 03, 2019 at 01:11:47PM +, Russell King - ARM Linux wrote: On Thu, Jan 03, 2019 at 10:47:27AM +0100, Lubomir Rintel wrote: > Hello, > > lately I've been trying to make the Himax HX8837 chip that drives the OLPC > LCD display work with Armada DRM driver. I've been advised to create a > bridge driver and not an encoder driver since the silicon is separate from > the LCDC. > > The Armada DRM driver (and, I think, the i.MX one) creates the drm_device > once the component infrastructure sees the necessary sub-devices appear. > The sub-devices being the LCDCs and the encoders (not bridges) that it > expects to be created externally. > > Currently, it seems, the only driver that can actually work with this > (that > is -- creates a drm_encoder for a drm_device when the component is bound) > is the tda998x. All other similar drivers create a drm_bridge instead and > not use the component infrastructure at all. (In fact, tilcdc driver > contains a hack to handle tda998x specially.) > > I'm wondering how to reconcile the two? > > * The tda998x driver has recently been modified to create a bridge on > probe > and eventually encoder on component bind. Is this an okay thing to do in > a new driver? (this probably means the tilcdc hack can be removed...) > > * If a non-componentized bridge were to be used (along with a dummy > encoder), at what point would it make sense to look for the bridge? > Would it be a good idea to defer the probe of crtc until a bridge can > be > looked up and the attach it on component bind? What if the bridge goes > away (a module is unloaded, etc.) in between? > > I'd be thankful for opintions and advice before I move ahead with this. This is the long-standing problem with the conflict between bridge support and component support, and I'm not sure that there is really any answer to it. I've gone into the details of the two several times on the list, particularly about the short-comings of the bridge approach, but it seems no one cares to fix those short-comings. You are re-identifying some of the issues that I've already pointed out - such as what happens to DRM drives when the bridge driver is unbound (it's really not about modules being unloaded, and the problem can't be solved by taking a module reference count - all that the module reference count does is ensure that the module doesn't go away unexpected, there is no way to ensure that the device isn't unbound.) The issue of unbinding is precisely the issue which the component support was created to solve - but everyone seems to prefer the buggy bridge approach, and no one seems willing to do anything about the bugs or even acknowledge that it's a problem. It's strange - if one identifies bugs that result in kernel oops in other kernel subsystems, one is generally taken seriously and the problem is solved. >>> Unbinding is really not the most important feature, especially for SoC. If >>> you feel different, working together with others, getting some agreement, >>> getting the patches reviewed and finding someone to get them merged is >>> very much appreciated. But just complaining won't move this forward. >>> The issue about the encoders is something that I've tried to discuss, and I've pointed out that moving it into the DRM driver adds additional complexity there, and I'd hoped that my patch set I posted would've generated discussion, but alas not. What I'm not prepared to do is to introduce _known_ bugs into any driver that I maintain. >>> I thought last time around the idea was to use device links (and teach >>> drm_bridge about them), so that the unloading works correctly. >> >> With current device_links implementation registering links in probe of >> the consumer is just incorrect - it can happen that neither consumer >> neither provider is fully probed and creating device links in such state >> is wrong according to docs, and my experiments. See [1] for discussion >> and [2] for docs. > We could set up the device link only at drm_dev_register time. At that point > we know that driver loading has fully succeeded. We do have a list of > bridges at hand, so that's not going to be an issue. > > The only case where this breaks is if a driver is still using the > deprecated ->load callback. But that ->load callback doesn't mix well with > EDEFER_PROBE/component framework anyway, so I think not going to be a > problem hopefully? drm_dev_register usually is called from bind callback, which is called from probe callback of one of the components or master (depending on
Re: [PATCH] drm/todo: Better defio support in the generic fbdev emulation
Den 07.01.2019 11.22, skrev Daniel Vetter: > The current one essentially means you need CMA or a vmalloc backed > object, which makes fbdev emulation a special case. > > Since implementing this will be quite a bit of work, capture the idea > in a TODO. > > Cc: Noralf Trønnes > Signed-off-by: Daniel Vetter > --- Acked-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 Michel Dänzer changed: What|Removed |Added Attachment #142997|text/x-log |text/plain mime type|| -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 Michel Dänzer changed: What|Removed |Added Attachment #142999|text/x-log |text/plain mime type|| -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 Michel Dänzer changed: What|Removed |Added Attachment #143000|text/x-log |text/plain mime type|| -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 Michel Dänzer changed: What|Removed |Added Attachment #142998|text/x-log |text/plain mime type|| -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Expose more EDID fields to userspace
On Mon, Jan 07, 2019 at 07:57:54AM -0800, Keith Packard wrote: > Daniel Vetter writes: > > > Best to pull in some other compositor people and get them to agree. From a > > kernel pov I'm fine with whatever userspace preferes. > > Hrm. It would be good to have everyone using the same interpretation of > EDID data; in particular, where the kernel has quirks that change the > interpretation, user space should be consistent with that. > > Unless we expose all of the EDID data, then user space may still have to > parse EDID. If the kernel has EDID quirks, it might be good to to make > those affect the "raw" EDID data visible to use space so that values the > kernel supplies separately are consistent with values extracted from the > "raw" EDID data. If the quirks can be re-encoded back into an EDID representation, then this sounds like a fairly good approach to me. > > Doing this in the kernel does make it harder to quickly supply fixes for > a specific user space application. This will probably lead to > kludge-arounds in user space that could depend on kernel > version. Perhaps these EDID capabilities in the kernel should be > versioned separately? > > I see good benefits from having user space able to see how the kernel is > interpreting EDID so that it can adapt as appropriate, but we should be > cautious about moving functionality into the kernel that would be more > easily maintained up in user space. > I agree. It seems likely that whatever happens (some) userspace is still going to implement (some) EDID parsing functionality, so it's hard to reason about what belongs where. Shared code in userspace (libdrm?) may well be better than exposing it from the kernel. If it is exposed by the kernel, then it's still non-obvious to me how the kernel exposes that information/interpretation. Adding a property for every potentially-useful field really doesn't scale well, and what fields are useful isn't obvious - e.g. serial string vs serial no., as mentioned by Simon. Uma's recent series: "Add HDR Metadata Parsing and handling in DRM layer"[1] is a good example of more stuff which userspace would want to parse out of the EDID (supported display colorimetry and transfer functions). It would be nice to avoid duplicating all the CEA extension parsing code, but the EDID/CEA data structure is extensible by design. So the kernel API would need to be similarly extensible, or we'll just balloon loads of properties... and then the kernel API would likely just end up just looking similar to the CEA block anyway. Cheers, -Brian [1] https://lists.freedesktop.org/archives/dri-devel/2018-December/200154.html > -- > -keith > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 --- Comment #4 from Michel Dänzer --- When you wrote "modesetting", you meant "DC". So the problem is the same with or without DC? Does it also happen with the Xorg modesetting driver instead of xf86-video-amdgpu, or with a Wayland compositor such as GNOME on Wayland? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] fbdev: offb: Fix OF node name handling
Commit 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
device_node.name") changed how the OF FB driver handles the OF node
name. This missed the case where the node name is passed to
offb_init_palette_hacks(). This results in a NULL ptr dereference
in strncmp and breaks any system except ones using bootx with no display
node.
Fix this by making offb_init_palette_hacks() use the OF node pointer and
use of_node_name_prefix() helper function instead for node name
comparisons. This helps in moving all OF node name accesses to helper
functions in preparation to remove struct device_node.name pointer.
Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
device_node.name")
Reported-by: Mathieu Malaterre
Cc: sta...@vger.kernel.org # v4.19+
Cc: Bartlomiej Zolnierkiewicz
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fb...@vger.kernel.org
Signed-off-by: Rob Herring
---
drivers/video/fbdev/offb.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index 31f769d67195..057d3cdef92e 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -318,28 +318,28 @@ static void __iomem *offb_map_reg(struct device_node *np,
int index,
}
static void offb_init_palette_hacks(struct fb_info *info, struct device_node
*dp,
- const char *name, unsigned long address)
+ unsigned long address)
{
struct offb_par *par = (struct offb_par *) info->par;
- if (dp && !strncmp(name, "ATY,Rage128", 11)) {
+ if (of_node_name_prefix(dp, "ATY,Rage128")) {
par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
if (par->cmap_adr)
par->cmap_type = cmap_r128;
- } else if (dp && (!strncmp(name, "ATY,RageM3pA", 12)
- || !strncmp(name, "ATY,RageM3p12A", 14))) {
+ } else if (of_node_name_prefix(dp, "ATY,RageM3pA") ||
+ of_node_name_prefix(dp, "ATY,RageM3p12A")) {
par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
if (par->cmap_adr)
par->cmap_type = cmap_M3A;
- } else if (dp && !strncmp(name, "ATY,RageM3pB", 12)) {
+ } else if (of_node_name_prefix(dp, "ATY,RageM3pB")) {
par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
if (par->cmap_adr)
par->cmap_type = cmap_M3B;
- } else if (dp && !strncmp(name, "ATY,Rage6", 9)) {
+ } else if (of_node_name_prefix(dp, "ATY,Rage6")) {
par->cmap_adr = offb_map_reg(dp, 1, 0, 0x1fff);
if (par->cmap_adr)
par->cmap_type = cmap_radeon;
- } else if (!strncmp(name, "ATY,", 4)) {
+ } else if (of_node_name_prefix(dp, "ATY,")) {
unsigned long base = address & 0xff00UL;
par->cmap_adr =
ioremap(base + 0x7ff000, 0x1000) + 0xcc0;
@@ -350,7 +350,7 @@ static void offb_init_palette_hacks(struct fb_info *info,
struct device_node *dp
par->cmap_adr = offb_map_reg(dp, 0, 0x6000, 0x1000);
if (par->cmap_adr)
par->cmap_type = cmap_gxt2000;
- } else if (dp && !strncmp(name, "vga,Display-", 12)) {
+ } else if (of_node_name_prefix(dp, "vga,Display-")) {
/* Look for AVIVO initialized by SLOF */
struct device_node *pciparent = of_get_parent(dp);
const u32 *vid, *did;
@@ -438,7 +438,7 @@ static void __init offb_init_fb(const char *name,
par->cmap_type = cmap_unknown;
if (depth == 8)
- offb_init_palette_hacks(info, dp, name, address);
+ offb_init_palette_hacks(info, dp, address);
else
fix->visual = FB_VISUAL_TRUECOLOR;
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: offb: Pass actual name in offb_init_palette_hacks
On Wed, Jan 2, 2019 at 2:03 PM Mathieu Malaterre wrote:
>
> Bartlomiej,
>
> Do you need an Acked-by from Rob, or can you take it in the next round
> of fixes for v4.20 ?
Sorry, I missed this.
>
> Just to repeat myself, previous code would call
> offb_init_palette_hacks(), which in turn would do:
>
>if (dp && !strncmp(name, "ATY,Rage128", 11)) {
>
> with name=NULL.
I prefer to fix this properly using node name helper in
offb_init_palette_hacks(). I'll send a patch shortly.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: Block fb changes for async plane updates
The prepare_fb call always happens on new_plane_state.
The drm_atomic_helper_cleanup_planes checks to see if
plane state pointer has changed when deciding to call cleanup_fb on
either the new_plane_state or the old_plane_state.
For a non-async atomic commit the state pointer is swapped, so this
helper calls prepare_fb on the new_plane_state and cleanup_fb on the
old_plane_state. This makes sense, since we want to prepare the
framebuffer we are going to use and cleanup the the framebuffer we are
no longer using.
For the async atomic update helpers this differs. The async atomic
update helpers perform in-place updates on the existing state. They call
drm_atomic_helper_cleanup_planes but the state pointer is not swapped.
This means that prepare_fb is called on the new_plane_state and
cleanup_fb is called on the new_plane_state (not the old).
In the case where old_plane_state->fb == new_plane_state->fb then
there should be no behavioral difference between an async update
and a non-async commit. But there are issues that arise when
old_plane_state->fb != new_plane_state->fb.
The first is that the new_plane_state->fb is immediately cleaned up
after it has been prepared, so we're using a fb that we shouldn't
be.
The second occurs during a sequence of async atomic updates and
non-async regular atomic commits. Suppose there are two framebuffers
being interleaved in a double-buffering scenario, fb1 and fb2:
- Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1
- Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2
- Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2
We call cleanup_fb on fb2 twice in this example scenario, and any
further use will result in use-after-free.
The simple fix to this problem is to block framebuffer changes
in the drm_atomic_helper_async_check function for now.
Cc: Daniel Vetter
Cc: Harry Wentland
Cc: Andrey Grodzovsky
Cc: # v4.14+
Fixes: fef9df8b5945 ("drm/atomic: initial support for asynchronous plane
update")
Signed-off-by: Nicholas Kazlauskas
---
drivers/gpu/drm/drm_atomic_helper.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 54e2ae614dcc..f4290f6b0c38 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1602,6 +1602,15 @@ int drm_atomic_helper_async_check(struct drm_device *dev,
old_plane_state->crtc != new_plane_state->crtc)
return -EINVAL;
+ /*
+* FIXME: Since prepare_fb and cleanup_fb are always called on
+* the new_plane_state for async updates we need to block framebuffer
+* changes. This prevents use of a fb that's been cleaned up and
+* double cleanups from occuring.
+*/
+ if (old_plane_state->fb != new_plane_state->fb)
+ return -EINVAL;
+
funcs = plane->helper_private;
if (!funcs->atomic_async_update)
return -EINVAL;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109234] amdgpu random hangs with 4.21+
https://bugs.freedesktop.org/show_bug.cgi?id=109234 --- Comment #3 from bmil...@gmail.com --- (In reply to Michel Dänzer from comment #2) > Can you bisect? I could but I don't have a reliable reproduction yet. I don't know what triggers the bug. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next
Hi Dave, Daniel, Here is the first PR for 5.1 for drm-misc-next. This is my first PR ever for drm-misc, so please let me know if I screwed up somehow :) Thanks! Maxime drm-misc-next-2019-01-07-1: drm-misc-next for 5.1: UAPI Changes: Cross-subsystem Changes: - Turn dma-buf fence sequence numbers into 64 bit numbers Core Changes: - Move to a common helper for the DP MST hotplug for radeon, i915 and amdgpu - i2c improvements for drm_dp_mst - Removal of drm_syncobj_cb - Introduction of an helper to create and attach the TV margin properties Driver Changes: - Improve cache flushes for v3d - Reflection support for vc4 - HDMI overscan support for vc4 - Add implicit fencing support for rockchip and sun4i - Switch to generic fbdev emulation for virtio The following changes since commit 0b258ed1a219a9776e8f6967eb34837ae0332e64: drm: revert "expand replace_fence to support timeline point v2" (2018-12-05 11:01:11 +0100) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2019-01-07-1 for you to fetch changes up to 1c95f662fceeb8ae2f34e3de9478e21fd31f09dd: Merge tag 'topic/drmp-cleanup-2019-01-02' of git://anongit.freedesktop.org/drm/drm-intel into drm-misc-next (2019-01-07 16:43:24 +0100) drm-misc-next for 5.1: UAPI Changes: Cross-subsystem Changes: - Turn dma-buf fence sequence numbers into 64 bit numbers Core Changes: - Move to a common helper for the DP MST hotplug for radeon, i915 and amdgpu - i2c improvements for drm_dp_mst - Removal of drm_syncobj_cb - Introduction of an helper to create and attach the TV margin properties Driver Changes: - Improve cache flushes for v3d - Reflection support for vc4 - HDMI overscan support for vc4 - Add implicit fencing support for rockchip and sun4i - Switch to generic fbdev emulation for virtio Archit Taneja (1): MAINTAINERS: drm: Remove myself as drm-bridge maintainer Boris Brezillon (7): drm/vc4: Fix negative X/Y positioning on SAND planes drm/vc4: Add support for X/Y reflection drm/connector: Fix drm_mode_create_tv_properties() doc drm/connector: Clarify the unit of TV margins drm/connector: Allow creation of margin props alone drm/vc4: Take margin setup into account when updating planes drm/vc4: Attach margin props to the HDMI connector Brajeswar Ghosh (1): drm/drm_drv.c: Remove duplicate header Chris Wilson (1): drm: Reorder set_property_atomic to avoid returning with an active ww_ctx Christian König (4): drm/v3d: fix broken build dma-buf: make fence sequence numbers 64 bit v2 drm/etnaviv: fix for 64bit seqno change drm/syncobj: remove drm_syncobj_cb and cleanup Dan Carpenter (2): drm: Fix an error pointer dereference() drm/ati_pcigart: Fix error code in drm_ati_pcigart_init() Daniel Vetter (3): drm/dp-mst-helper: Remove hotplug callback drm/qxl: Don't set the dpms hook drm/xen: Don't set the dpms hook Eric Anholt (6): drm/v3d: Document cache flushing ABI. drm/v3d: Drop unused v3d_flush_caches(). drm/v3d: Don't bother flushing L1TD at job start. drm/v3d: Drop the wait for L2T flush to complete. drm/v3d: Stop trying to flush L2C on V3D 3.3+ drm/v3d: Invalidate the caches from the outside in. Gerd Hoffmann (3): drm/qxl: add spice-devel list to MAINTAINERS drm/virtio: switch to generic fbdev emulation drm/bochs: add edid present check Heiko Stuebner (1): drm/rockchip: Add implicit fencing support for planes Jani Nikula (5): drm: un-inline drm_legacy_findmap() drm: include kernel.h and agp_backend.h from intel-gtt.h drm: include idr.h from drm_file.h drm: include types.h from drm_hdcp.h drm: forward declare struct drm_file in drm_syncobj.h Kuninori Morimoto (1): drm: dw-hdmi-i2s: convert to SPDX identifiers Lyude Paul (3): drm/dp_mst: Fix memory leak in drm_dp_mst_topology_mgr_destroy() drm/dp_mst: Remove bogus conditional in drm_dp_update_payload_part1() drm/dp_mst: Refactor drm_dp_update_payload_part1() Maxime Ripard (1): Merge tag 'topic/drmp-cleanup-2019-01-02' of git://anongit.freedesktop.org/drm/drm-intel into drm-misc-next Mika Kuoppala (1): drm/i915: Compile fix for 64b dma-fence seqno Rob Clark (1): drm/atomic: integrate modeset lock with private objects Sam Ravnborg (1): drm: move DRM_IF_VERSION to drm_internal.h Shayenne da Luz Moura (2): drm: Rename crtc_idr as object_idr to KMS cleanups drm: Remove complete task from TODO documentation Ville Syrjälä (2): drm/dp/mst: Configure no_stop_bit correctly for remote i2c xfers drm/dp/mst: Validate REMOTE_I2C_READ harder Yangtao Li (1): dma-buf: Change to use DEFINE_SHOW_ATTR
Re: [PATCH 0/3] drm/mxsfb: support swapped RGB lanes
Hi Ahmad.
> On 2/1/19 22:37, Sam Ravnborg wrote:
> > The problem with the RED/BLUE lines swapped is something I
> > have encountered while working with DRM support for Atmel at91sam9263 too.
> >
> > The solution selected is to extend the endpoint with
> > a new optional property:
> >
> > - wiring: Wiring of data lines to display.
> > "straight" - normal wiring.
> > "red-blue-reversed" - red and blue lines reversed.
> >
> > (media/video-interfaces.txt)
> >
> >
> > The DT node looks like this:
> >
> >port@0 {
> > reg = <0>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > lcdc_panel_output: endpoint@0 {
> > reg = <0>;
> > wiring = "red-blue-reversed";
> > remote-endpoint = <&panel_input>;
> > };
> > };
> >
> > This allows us to specify the swapping in the endpoint and
> > not in the panel.
> > So we can use the same panel, with the same bus_format, in several
> > designs some with red-blue swapped (reversed), and some not.
>
> A colleague suggested a property in the endpoint as well, but I shied
> away because of the extra hassle. Seems there's won't be a way around it ^^'..
>
> How do you intend to propagate this different wiring setting?
The way I have it implmented is more or less like this:
First find the wiring property:
1) Look up endpoint using of_graph_get_endpoint_by_regs()
2) Get wiring property
3) of_node_put(endpoint);
And then find and attach the panel:
4) drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, &bridge);
5) devm_drm_panel_bridge_add(dev, panel, DRM_MODE_CONNECTOR_DPI);
6) Then based on the wiring property I adjust bus_format
7) drm_simple_display_pipe_init()
8) drm_simple_display_pipe_attach_bridge()
But this is all virgin code that for now can build,
but has not yet seen any testing.
It is a lot of boilerplate for something relatively simple
and I hope there are ways to simplify this.
Relevant parts of the file pasted below.
But the translation of bus_format in a central place may prove a bit
difficult and I assume this as something that can differ
a lot between different HW solutions.
> How about having drm_of_find_panel_or_bridge adjust the
> (*panel)->connector->display_info.bus_formats array to account for the
> different wiring? That way there shouldn't be any need to adjust drivers.
But if you prove me wrong and this fly I am all for it.
Keep in mind that I am novice in the DRM land. So there may be better ways to
do it.
Sam
static int lcdc_get_of_wiring(struct lcdc *lcdc,
const struct device_node *ep)
{
const char *str;
int ret;
ret = of_property_read_string(ep, "wiring", &str);
if (ret)
return ret;
if (strcmp(str, "red-green-reversed") == 0) {
lcdc->wiring_reversed = true;
} else if (strcmp(str, "straight") == 0) {
/* Use default format */
} else {
DRM_DEV_ERROR(lcdc->dev, "unknown \"wiring\" property: %s",
str);
return -EINVAL;
}
return 0;
}
static int lcdc_display_init(struct lcdc *lcdc, struct drm_device *drm)
{
struct drm_display_info *display_info;
const u32 *formats;
size_t nformats;
int ret;
display_info = &lcdc->panel->connector->display_info;
if (!display_info->num_bus_formats || !display_info->bus_formats) {
DRM_DEV_ERROR(lcdc->dev, "missing bus_format from panel");
return -EINVAL;
}
switch (display_info->bus_formats[0]) {
case MEDIA_BUS_FMT_RGB888_1X24:
case MEDIA_BUS_FMT_RGB565_1X16:
lcdc->bus_format = display_info->bus_formats[0];
break;
default:
DRM_DEV_ERROR(lcdc->dev, "unsupported bus_format: %d",
display_info->bus_formats[0]);
return -EINVAL;
}
/* Select formats depending on wiring (from bus_formats + from DT) */
if (lcdc->bus_format == MEDIA_BUS_FMT_RGB888_1X24) {
if (!lcdc->wiring_reversed) {
formats = bgr_formats_24;
nformats = ARRAY_SIZE(bgr_formats_24);
} else {
formats = rgb_formats_24;
nformats = ARRAY_SIZE(rgb_formats_24);
}
} else {
if (!lcdc->wiring_reversed) {
formats = bgr_formats_16;
nformats = ARRAY_SIZE(bgr_formats_16);
} else {
formats = rgb_formats_16;
nformats = ARRAY_S
[Bug 109073] AMDGPU Radeon RX540 system freezes and/or crashes; poor performance with ac adapter plugged in
https://bugs.freedesktop.org/show_bug.cgi?id=109073 --- Comment #6 from whoop --- With v5.0-rc1 the system hangs with a black screen after I login at gdm when the power adapter is connected -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PULL] topic/drmp-cleanup for drm-misc-next and drm-intel-next-queued
Hi Jani, On Wed, Jan 02, 2019 at 12:00:49PM +0200, Jani Nikula wrote: > I embarked on removing drmP.h includes from i915, but that requires a > bunch of drm header cleanup to add relevant includes and forward > declarations. Due to the timing, propagating the patches back to i915 > would take eons, so Daniel suggested a topic branch to be merged both to > drm-misc-next and drm-intel-next-queued. So here it is, with $(git > merge-base drm-misc-next drm-intel-next-queued) as the starting point. > > The topic branch has been part of drm-tip since, uh, last year, but I > did just force push it to update the commit messages to reflect > Laurent's review. No code changes. > > I'll merge the same thing to i915 after it's been pulled to > drm-misc-next, and with this, I should be able to get rid of all drmP.h > includes in i915. I merged the PR, and it's on its way to Daniel and Dave now. Thanks Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109073] AMDGPU Radeon RX540 system freezes and/or crashes; poor performance with ac adapter plugged in
https://bugs.freedesktop.org/show_bug.cgi?id=109073 --- Comment #7 from whoop --- Created attachment 143004 --> https://bugs.freedesktop.org/attachment.cgi?id=143004&action=edit kern.log using kernel v5.0-rc1 with black screen after login with power adapter plugged in -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
