Re: [Intel-gfx] linux-next: manual merge of the drm-misc tree with the arm tree

2016-06-22 Thread Russell King 
On Wed, Jun 22, 2016 at 09:31:18AM +0200, Daniel Vetter wrote:
> On Wed, Jun 22, 2016 at 3:47 AM, Stephen Rothwell  
> wrote:
> > Hi all,
> >
> > Today's linux-next merge of the drm-misc tree got a conflict in:
> >
> >   drivers/gpu/drm/sti/sti_drv.c
> >
> > between commit:
> >
> >   062993b15e8e ("drm: convert DT component matching to 
> > component_match_add_release()")
> 
> Why did that one end up in the arm tree? Should it go in through
> drm-misc instead?

Mine is part of a three part patch series which is part of the component
helper updates (which I'm the author and maintainer of).

Then someone came up with an alternative way of some of part of it.

You can't merge the above DRM part, because that means you also need to
merge patch 1, which is core component stuff.

-- 
Russell King
ARM architecture Linux Kernel maintainer
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] linux-next: manual merge of the drm-misc tree with the arm tree

2016-06-22 Thread Russell King 
On Wed, Jun 22, 2016 at 10:23:36AM +0200, Daniel Vetter wrote:
> On Wed, Jun 22, 2016 at 09:21:11AM +0100, Russell King wrote:
> > On Wed, Jun 22, 2016 at 09:31:18AM +0200, Daniel Vetter wrote:
> > > On Wed, Jun 22, 2016 at 3:47 AM, Stephen Rothwell  
> > > wrote:
> > > > Hi all,
> > > >
> > > > Today's linux-next merge of the drm-misc tree got a conflict in:
> > > >
> > > >   drivers/gpu/drm/sti/sti_drv.c
> > > >
> > > > between commit:
> > > >
> > > >   062993b15e8e ("drm: convert DT component matching to 
> > > > component_match_add_release()")
> > > 
> > > Why did that one end up in the arm tree? Should it go in through
> > > drm-misc instead?
> > 
> > Mine is part of a three part patch series which is part of the component
> > helper updates (which I'm the author and maintainer of).
> > 
> > Then someone came up with an alternative way of some of part of it.
> > 
> > You can't merge the above DRM part, because that means you also need to
> > merge patch 1, which is core component stuff.
> 
> Makes sense, but generally in that case I ask Dave for an explicit ack for
> merging through another tree to avoid confusion. Lack of that is why I
> asked.

It got posted to the appropriate mailing lists with CCs, including David.
Just three people responded.

One of the responses was that people didn't like the duplication.  I
posted v2 the same day, the DT people didn't like the file location, so
I went back to v1.  That then sparked someone to start working _against_
me, cleaning up the existing duplication, and acknowledging that it'll
cause _me_ problems.

So, as it was done maliciously and intentionally to give these porblems,
I'm not budging on this.  Sorry.

There are times when working on the kernel is not very nice.  This is one
of them.

-- 
Russell King
ARM architecture Linux Kernel maintainer
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC PATCH] component: do not leave master devres group open after bind

2021-09-21 Thread Russell King (Oracle) 
On Tue, Sep 21, 2021 at 02:18:10PM +0300, Kai Vehmanen wrote:
> In current code, the devres group for aggregate master is left open
> after call to component_master_add_*(). This leads to problems when the
> master does further managed allocations on its own. When any
> participating driver calls component_del(), this leads to immediate
> release of resources.
> 
> This came up when investigating a page fault occurring with i915 DRM
> driver unbind with 5.15-rc1 kernel. The following sequence occurs:
> 
>  i915_pci_remove()
>-> intel_display_driver_unregister()
>  -> i915_audio_component_cleanup()
>-> component_del()
>  -> component.c:take_down_master()
>-> hdac_component_master_unbind() [via master->ops->unbind()]
>-> devres_release_group(master->parent, NULL)
> 
> With older kernels this has not caused issues, but with audio driver
> moving to use managed interfaces for more of its allocations, this no
> longer works. Devres log shows following to occur:
> 
> component_master_add_with_match()
> [  126.886032] snd_hda_intel :00:1f.3: DEVRES ADD 323ccdc5 
> devm_component_match_release (24 bytes)
> [  126.886045] snd_hda_intel :00:1f.3: DEVRES ADD 865cdb29 grp< 
> (0 bytes)
> [  126.886049] snd_hda_intel :00:1f.3: DEVRES ADD 1b480725 grp< 
> (0 bytes)
> 
> audio driver completes its PCI probe()
> [  126.892238] snd_hda_intel :00:1f.3: DEVRES ADD 1b480725 
> pcim_iomap_release (48 bytes)
> 
> component_del() called() at DRM/i915 unbind()
> [  137.579422] i915 :00:02.0: DEVRES REL ef44c293 grp< (0 bytes)
> [  137.579445] snd_hda_intel :00:1f.3: DEVRES REL 865cdb29 grp< 
> (0 bytes)
> [  137.579458] snd_hda_intel :00:1f.3: DEVRES REL 1b480725 
> pcim_iomap_release (48 bytes)
> 
> So the "devres_release_group(master->parent, NULL)" ends up freeing the
> pcim_iomap allocation. Upon next runtime resume, the audio driver will
> cause a page fault as the iomap alloc was released without the driver
> knowing about it.
> 
> Fix this issue by using the "struct master" pointer as identifier for
> the devres group, and by closing the devres group after the 
> master->ops->bind()
> call is done. This allows devres allocations done by the driver acting as
> master to be isolated from the binding state of the aggregate driver. This
> modifies the logic originally introduced in commit 9e1ccb4a7700
> ("drivers/base: fix devres handling for master device").

Yes, this is the right fix - I did not expect people to be claiming
resources after adding the component master, since that is the point
at which the master can become active. Hence all resources that may
be used should either already be claimed, or (preferred) be claimed
when the master gets the bind call.

However, I think the i915 bits are more complex than that simple view,
so putting the component stuff inside its own devres group and closing
it at the end of try_to_bring_up_master() makes sense.

Acked-by: Russell King (Oracle) 

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [Intel-gfx] [PATCH v2 02/12] drm/armada: Use regular fbdev I/O helpers

2023-05-15 Thread Russell King (Oracle) 
On Mon, May 15, 2023 at 07:55:44PM +0200, Sam Ravnborg wrote:
> Hi Thomas,
> 
> On Mon, May 15, 2023 at 11:40:23AM +0200, Thomas Zimmermann wrote:
> > Use the regular fbdev helpers for framebuffer I/O instead of DRM's
> > helpers. Armada does not use damage handling, so DRM's fbdev helpers
> > are mere wrappers around the fbdev code.
> > 
> > By using fbdev helpers directly within each DRM fbdev emulation,
> > we can eventually remove DRM's wrapper functions entirely.
> > 
> > v2:
> > * use FB_IO_HELPERS option
> > 
> > Signed-off-by: Thomas Zimmermann 
> > Cc: Russell King 
> > ---
> >  drivers/gpu/drm/armada/Kconfig| 1 +
> >  drivers/gpu/drm/armada/armada_fbdev.c | 9 -
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/armada/Kconfig b/drivers/gpu/drm/armada/Kconfig
> > index f5c66d89ba99..5afade25e217 100644
> > --- a/drivers/gpu/drm/armada/Kconfig
> > +++ b/drivers/gpu/drm/armada/Kconfig
> > @@ -3,6 +3,7 @@ config DRM_ARMADA
> > tristate "DRM support for Marvell Armada SoCs"
> > depends on DRM && HAVE_CLK && ARM && MMU
> > select DRM_KMS_HELPER
> > +   select FB_IO_HELPERS if DRM_FBDEV_EMULATION
> > help
> >   Support the "LCD" controllers found on the Marvell Armada 510
> >   devices.  There are two controllers on the device, each controller
> > diff --git a/drivers/gpu/drm/armada/armada_fbdev.c 
> > b/drivers/gpu/drm/armada/armada_fbdev.c
> > index 0a5fd1aa86eb..6c3bbaf53569 100644
> > --- a/drivers/gpu/drm/armada/armada_fbdev.c
> > +++ b/drivers/gpu/drm/armada/armada_fbdev.c
> > @@ -5,6 +5,7 @@
> >   */
> >  
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -34,11 +35,9 @@ static void armada_fbdev_fb_destroy(struct fb_info *info)
> >  static const struct fb_ops armada_fb_ops = {
> > .owner  = THIS_MODULE,
> > DRM_FB_HELPER_DEFAULT_OPS,
> > -   .fb_read= drm_fb_helper_cfb_read,
> > -   .fb_write   = drm_fb_helper_cfb_write,
> I had expected to see
> .fb_read = fb_io_read,
> 
> But maybe this only used when using damage handling?
> 
> Likewise for drm_fb_helper_cfb_write.
> 
> ??
> 
> > -   .fb_fillrect= drm_fb_helper_cfb_fillrect,
> > -   .fb_copyarea= drm_fb_helper_cfb_copyarea,
> > -   .fb_imageblit   = drm_fb_helper_cfb_imageblit,
> > +   .fb_fillrect= cfb_fillrect,
> > +   .fb_copyarea= cfb_copyarea,
> > +   .fb_imageblit   = cfb_imageblit,
> 
> This part is as expected.

Well, to me it looks like this has gone through an entire circular set
of revisions:

commit e8b70e4dd7b5dad7c2379de6e0851587bf86bfd6
Author: Archit Taneja 
Date:   Wed Jul 22 14:58:04 2015 +0530

drm/armada: Use new drm_fb_helper functions

-   .fb_fillrect= cfb_fillrect,
-   .fb_copyarea= cfb_copyarea,
-   .fb_imageblit   = cfb_imageblit,
+   .fb_fillrect= drm_fb_helper_cfb_fillrect,
+   .fb_copyarea= drm_fb_helper_cfb_copyarea,
+   .fb_imageblit   = drm_fb_helper_cfb_imageblit,

commit 983780918c759fdbbf0bf033e701bbff75d2af23
Author: Thomas Zimmermann 
Date:   Thu Nov 3 16:14:40 2022 +0100

drm/fb-helper: Perform all fbdev I/O with the same implementation

+   .fb_read= drm_fb_helper_cfb_read,
+   .fb_write   = drm_fb_helper_cfb_write,

and now effectively those two changes are being reverted, so we'd
now be back to the pre-July 2015 state of affairs. As I believe
the fbdev layer has been stable, this change merely reverts the
driver back to what it once was.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[Intel-gfx] BUG: i915: flickering/temporary artifacts after resume

2022-11-05 Thread Russell King (Oracle) 
Hi,

I have a HP Pavilion 15" laptop that occasionally misbehaves after a
resume from suspend mode. The problem is obvious when the screen
updates e.g. after moving the mouse and the window focus changing, or
when a terminal scrolls, I get a ficker of random short horizontal
white lines over the top of the windows that then disappear. These
appear to be predominantly focused towards the top of the screen,
although they do also occur lower down but less obviously.

Soemtimes these artifacts don't disappear until the next update - I
attempted to capture them, but of course that provokes another screen
update and they disappear.

When this problem occurs, suspending and resuming again doesn't appear
to fix the issue - only a reboot does.

Environment:

00:02.0 VGA compatible controller: Intel Corporation HD Graphics 620 (rev 02) 
(prog-if 00 [VGA controller])
DeviceName: Intel Kabylake HD Graphics ULT GT2
Subsystem: Hewlett-Packard Company HD Graphics 620
Flags: bus master, fast devsel, latency 0, IRQ 130, IOMMU group 1
Memory at a000 (64-bit, non-prefetchable) [size=16M]
Memory at 9000 (64-bit, prefetchable) [size=256M]
I/O ports at 4000 [size=64]
Expansion ROM at 000c [virtual] [disabled] [size=128K]
Capabilities: [40] Vendor Specific Information: Len=0c 
Capabilities: [70] Express Root Complex Integrated Endpoint, MSI 00
Capabilities: [ac] MSI: Enable+ Count=1/1 Maskable- 64bit-
Capabilities: [d0] Power Management version 2
Capabilities: [100] Process Address Space ID (PASID)
Capabilities: [200] Address Translation Service (ATS)
Capabilities: [300] Page Request Interface (PRI)
Kernel driver in use: i915
Kernel modules: i915

Debian Bullseye (stable), Xorg 1.20.11, libdrm 2.4.104, intel xorg
driver 2.99.917+git20200714.

Kernel messages related to DRM:

Linux version 5.10.0-19-amd64 (debian-ker...@lists.debian.org) (gcc-10 (Debian 1
0.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2) #1 SMP Debian
 5.10.149-1 (2022-10-17)
...
i915 :00:02.0: [drm] VT-d active for gfx access
checking generic (9000 408000) vs hw (a000 100)
checking generic (9000 408000) vs hw (9000 1000)
fb0: switching to inteldrmfb from EFI VGA
Console: switching to colour dummy device 80x25
i915 :00:02.0: vgaarb: deactivate vga console
i915 :00:02.0: vgaarb: changed VGA decodes: 
olddecodes=io+mem,decodes=io+mem:owns=io+mem
i915 :00:02.0: firmware: direct-loading firmware i915/kbl_dmc_ver1_04.bin
i915 :00:02.0: [drm] Finished loading DMC firmware i915/kbl_dmc_ver1_04.bin 
(v1.4)
[drm] Initialized i915 1.6.0 20200917 for :00:02.0 on minor 0
ACPI: Video Device [GFX0] (multi-head: yes  rom: no  post: no)
input: Video Bus as 
/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/input/input6
fbcon: i915drmfb (fb0) is primary device
Console: switching to colour frame buffer device 170x48
i915 :00:02.0: [drm] fb0: i915drmfb frame buffer device
mei_hdcp :00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04: bound :00:02.0 
(ops i915_hdcp_component_ops [i915])
snd_hda_intel :00:1f.3: bound :00:02.0 (ops 
i915_audio_component_bind_ops [i915])
(NULL device *): firmware: direct-loading firmware i915/kbl_dmc_ver1_04.bin
mei_hdcp :00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04: bound :00:02.0 (
ops i915_hdcp_component_ops [i915])

The last two lines repeat each time the system is suspended/resumed.

No errors or warnings appear to be logged either from the kernel nor in
the Xorg log file specific to i915 (there's the usual Xorg whinging
about the system being too slow... so an i5-7200U 2.5GHz isn't fast
enough for Xorg!)

It feels like some setting within the Intel GPU hardware that controls
memory access timing isn't being properly restored.

Is this a known issue?

Thanks.

Russell.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [Intel-gfx] BUG: i915: flickering/temporary artifacts after resume

2022-11-07 Thread Russell King (Oracle) 
On Mon, Nov 07, 2022 at 09:29:33AM +, Saarinen, Jani wrote:
> Hi Russell,
> Can you make new gitlab: 
> https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs  ? 

Hi,

Unfortunately, I can't get any more information, and trying to get a
photo of the problem is utterly impossible, beacuse it's almost always
a momentary corruption - out of all the cases, I have only seen it
remain on-screen once.

Also, it can take months of suspend/resume cycles for it to occur,
which makes it difficult to reproduce using the "latest" kernel -
beacuse by the time the latest kernel has shown the problem, there's
a more modern kernel!

Using the intel reg dumper may provide some useful information.

That said, running it from the debian stable repository gives me:

(intel_reg:338236) intel_mmio-CRITICAL: Test assertion failure function
intel_mmio_use_pci_bar, file ../lib/intel_mmio.c:146:
(intel_reg:338236) intel_mmio-CRITICAL: Failed assertion: !(error != 0)
(intel_reg:338236) intel_mmio-CRITICAL: Last errno: 1, Operation not
permitted
(intel_reg:338236) intel_mmio-CRITICAL: Couldn't map MMIO region
Test (null) failed.
 DEBUG 
(intel_reg:338236) intel_chipset-DEBUG: Test requirement passed: pci_dev
(intel_reg:338236) intel_mmio-CRITICAL: Test assertion failure function
intel_mmio_use_pci_bar, file ../lib/intel_mmio.c:146:
(intel_reg:338236) intel_mmio-CRITICAL: Failed assertion: !(error != 0)
(intel_reg:338236) intel_mmio-CRITICAL: Last errno: 1, Operation not
permitted
(intel_reg:338236) intel_mmio-CRITICAL: Couldn't map MMIO region
(intel_reg:338236) igt_core-INFO: Stack trace:
(intel_reg:338236) igt_core-INFO:   #0 [__igt_fail_assert+0x113]
(intel_reg:338236) igt_core-INFO:   #1 [intel_mmio_use_pci_bar+0xc4]
(intel_reg:338236) igt_core-INFO:   #2 [intel_register_access_init+0x1b]
(intel_reg:338236) igt_core-INFO:   #3 [+0xec025c3a]
(intel_reg:338236) igt_core-INFO:   #4 [+0xec0246f6]
(intel_reg:338236) igt_core-INFO:   #5 [__libc_start_main+0xea]
(intel_reg:338236) igt_core-INFO:   #6 [+0xec0247ba]
  END  

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [Intel-gfx] [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

2018-12-05 Thread Russell King - ARM Linux 
On Tue, Nov 20, 2018 at 06:13:42PM +0200, Ville Syrjala wrote:
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c 
> b/drivers/gpu/drm/i2c/tda998x_drv.c
> index a7c39f39793f..38c66fbc8276 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -849,7 +849,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct 
> drm_display_mode *mode)
>  {
>   union hdmi_infoframe frame;
>  
> - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> +  &priv->connector, mode);
>   frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
>  
>   tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);

For this,

Acked-by: Russell King 

Thanks.

-- 
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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 13/29] drm/armada: Use a private mutex to protect priv->linear

2015-11-23 Thread Russell King - ARM Linux 
On Mon, Nov 23, 2015 at 10:32:46AM +0100, Daniel Vetter wrote:
> diff --git a/drivers/gpu/drm/armada/armada_gem.c 
> b/drivers/gpu/drm/armada/armada_gem.c
> index e3a86b96af2a..a43690ab18b9 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -46,7 +46,7 @@ static size_t roundup_gem_size(size_t size)
>   return roundup(size, PAGE_SIZE);
>  }
>  
> -/* dev->struct_mutex is held here */
> +/* dev_priv->linear_lock is held here */
>  void armada_gem_free_object(struct drm_gem_object *obj)

This is wrong (unless there's changes which I'm not aware of.)

This function is called from drm_gem_object_free(), which is called from
drm_gem_object_unreference(), both of which contain:

WARN_ON(!mutex_is_locked(&dev->struct_mutex));

Now, unless you're changing the conditions on which
drm_gem_object_unreference() to be under dev_priv->linear_lock, the
above comment becomes misleading.

It'd also need drm_gem_object_unreference_unlocked() to become per-
driver, so it can take dev_priv->linear_lock, and I don't see anything
in the patches which I've received which does that.

So, I suspect the new comment is basically false.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 12/29] drm/armada: Drop struct_mutex from cursor paths

2015-12-03 Thread Russell King - ARM Linux 
On Mon, Nov 23, 2015 at 10:32:45AM +0100, Daniel Vetter wrote:
> @@ -957,11 +955,9 @@ static int armada_drm_crtc_cursor_move(struct drm_crtc 
> *crtc, int x, int y)
>   if (!dcrtc->variant->has_spu_adv_reg)
>   return -EFAULT;
>  
> - mutex_lock(&dev->struct_mutex);
>   dcrtc->cursor_x = x;
>   dcrtc->cursor_y = y;
>   ret = armada_drm_crtc_cursor_update(dcrtc, false);
> - mutex_unlock(&dev->struct_mutex);
>  
>   return ret;
>  }

drivers/gpu/drm/armada/armada_crtc.c: In function 'armada_drm_crtc_cursor_move':
drivers/gpu/drm/armada/armada_crtc.c:916:21: warning: unused variable 'dev' 
[-Wunused-variable]

Shall I add to your patch a change to remove that variable too? :)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/tda998x: Remove dummy save/restore funcs

2015-12-08 Thread Russell King - ARM Linux 
On Tue, Dec 08, 2015 at 11:11:01AM +0100, Daniel Vetter wrote:
> On Tue, Dec 08, 2015 at 09:30:48AM +, Emil Velikov wrote:
> > On 8 December 2015 at 08:49, Daniel Vetter  wrote:
> > > In my quest to remove all the drm_encoder_helper_funcs->save/restore
> > > hooks I somehow missed that this driver has a dummy set too - I
> > > thought all the i2c drivers only set up the slave_encoder save/restore
> > > hooks, which I've left. Remove them.
> > >
> > There was an identical patch from Rodrigo a couple of days ago
> 
> couple = less than 1 ;-) But thanks for pointing out, merged that patch
> instead.

I'd be nice if Rodrigo's patch was copied to me.  I've some patches
outstanding (the atomic mode set) which I need to send to David, but
this change should not conflict.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/35] drm/armada: Use lockless gem BO free callback

2016-04-28 Thread Russell King - ARM Linux 
On Tue, Apr 26, 2016 at 07:29:44PM +0200, Daniel Vetter wrote:
> No dev->struct_mutex anywhere to be seen.
> 
> Cc: Russell King 

Acked-by: Russell King 

Thanks Daniel.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 16/35] drm/etnaviv: Use lockless gem BO free callback

2016-04-28 Thread Russell King - ARM Linux 
On Tue, Apr 26, 2016 at 07:29:49PM +0200, Daniel Vetter wrote:
> No dev->struct_mutex anywhere to be seen.
> 
> Cc: Christian Gmeiner 
> Cc: Russell King 

Acked-by: Russell King 

Thanks Daniel.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/10] drm/armada: Drop fb gamma_set/get functions

2016-03-30 Thread Russell King - ARM Linux 
On Wed, Mar 30, 2016 at 11:51:18AM +0200, Daniel Vetter wrote:
> The fb helper private gamma_set/get functions are only required when
> the driver supports paletted 8bit mode with fbdev. Armada uses 32bpp
> unconditionally, so this is just dead code. It also doesn't do
> anything really. Let's just remove it.

This comment is misleading: Armada supports 16bpp formats as well as
32bpp, and the hardware does have 8bpp modes to, but I've chosen not
to support the 8bpp modes.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/10] drm/armada: Drop fb gamma_set/get functions

2016-03-30 Thread Russell King - ARM Linux 
On Wed, Mar 30, 2016 at 01:19:21PM +0200, Daniel Vetter wrote:
> On Wed, Mar 30, 2016 at 1:09 PM, Russell King - ARM Linux
>  wrote:
> > On Wed, Mar 30, 2016 at 11:51:18AM +0200, Daniel Vetter wrote:
> >> The fb helper private gamma_set/get functions are only required when
> >> the driver supports paletted 8bit mode with fbdev. Armada uses 32bpp
> >> unconditionally, so this is just dead code. It also doesn't do
> >> anything really. Let's just remove it.
> >
> > This comment is misleading: Armada supports 16bpp formats as well as
> > 32bpp, and the hardware does have 8bpp modes to, but I've chosen not
> > to support the 8bpp modes.
> 
> This is purely about the fbdev emulation (and yeah need to clarify
> that), not about kms support in general. And these two gamma_set/get
> hooks are _only_ used by the fbdev emulation, and only needed if you
> do 8bit paletted mode. Ok if I change the commit message to "Armada
> used 32bpp unconditionally for fbdev emulation, ..."?

I still don't know where you get that from - the armada fbdev code
supports more than just 32bpp.  Please explain.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] i915 render node discovery buggy?

2015-10-27 Thread Russell King - ARM Linux 
In passing, while reading the Intel DDX driver code, I've noticed
that the Intel driver contains code which assumes that the master and
render devices are related by minor device number, eg:

/* Are we a render-node ourselves? */
if (is_render_node(fd, &master))
return NULL;

sprintf(buf, "/dev/dri/renderD%d", (int)((master.st_rdev | 0x80) & 
0xbf));
if (stat(buf, &render) == 0 &&
master.st_mode == render.st_mode &&
render.st_rdev == ((master.st_rdev | 0x80) & 0xbf))
return strdup(buf);

There's also code doing the reverse as well.

From my observations, the assumption that this code is built upon is
false.  I have an ARM platform here (non-Intel graphics) which shows
the problem - we have a KMS-only DRM driver (card0) and a GPU-only
DRM driver (card1).  This populates the /dev/dri subdirectory as
follows:

crw-rw+ 1 root video 226,   0 Oct 27 04:59 card0
crw-rw+ 1 root video 226,   1 Oct 26 20:40 card1
crw-rw  1 root video 226,  64 Oct 26 20:40 controlD64
crw-rw  1 root video 226, 128 Oct 26 20:40 renderD128

and if I look at /sys/class/drm, you can then see who owns which devices:

lrwxrwxrwx 1 root root0 Oct 26 20:40 card0 -> 
../../devices/platform/armada-drm/drm/card0
lrwxrwxrwx 1 root root0 Oct 26 20:40 card0-HDMI-A-1 -> 
../../devices/platform/armada-drm/drm/card0/card0-HDMI-A-1
lrwxrwxrwx 1 root root0 Oct 26 20:40 card1 -> 
../../devices/platform/etnaviv/drm/card1
lrwxrwxrwx 1 root root0 Oct 26 20:40 controlD64 -> 
../../devices/platform/armada-drm/drm/controlD64
lrwxrwxrwx 1 root root0 Oct 26 20:40 renderD128 -> 
../../devices/platform/etnaviv/drm/renderD128

So, renderD128 is card1's render node, which does not conform to the
assumption which the Intel DDX driver makes - (1 | 0x80) & 0xbf is
not 128.  The same thing can happen if there's ever a case on Intel
hardware where a KMS DRM driver registers prior to the i915 driver.

I think the only way to properly determine which render nodes
correspond with which master node is to actually open the device and
check the device names, or parse sysfs - maybe reading the links of
/sys/class/drm, and checking which link dirname corresponds with the
master node.

Any comments?

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/22] drm/armada: Remove NULL open/pre/postclose hooks

2016-01-12 Thread Russell King - ARM Linux 
On Mon, Jan 11, 2016 at 10:41:01PM +0100, Daniel Vetter wrote:
> The compiler will do this, but the void hits when grepping all the
> hooks for a subsystem wide audit are slightly annoying. So remove them
> for next time around.

I'll try to remember to queue this after -rc1, though a reminder
after -rc1 would be useful.

Thanks.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [REGRESSION] component: add support for releasing match data

2016-01-26 Thread Russell King - ARM Linux 
On Tue, Jan 26, 2016 at 02:42:16PM +0100, Maarten Lankhorst wrote:
> Commit ce657b1cddf1f88c56 ("component: add support for releasing match
> data") causes a general protection fault when unloading snd-hda-intel
> with the i915 module loaded on a recent skylake machine.

I'm no good at interpreting x86 oopses.  Can I have some help with
this one?  What causes a "general protection fault"?  What does the
code line disassemble to?  What was the code doing at the point
it caused this fault?  Was it calling mc->release() or was it
trying to access some data?

> This breaks one of the i915 acceptance tests that performs a module
> unload/reload on snd-hda-intel and i915.
> 
> # modprobe -r snd-hda-intel
> 
> [  268.635792] general protection fault:  [#1] PREEMPT SMP 
> DEBUG_PAGEALLOC 
> [  268.635879] Modules linked in: fuse snd_hda_codec_hdmi 
> snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel(-) 
> snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_seq_midi 
> snd_seq_midi_event snd_rawmidi snd_seq input_leds intel_rapl led_class 
> x86_pkg_temp_thermal snd_seq_device snd_timer thermal fan wmi snd 
> soundcore pinctrl_sunrisepoint pinctrl_intel acpi_cpufreq acpi_pad i915 
> processor parport_pc ppdev parport efivarfs autofs4
> [  268.636072] CPU: 3 PID: 2156 Comm: modprobe Tainted: G U  W   
> 4.5.0-rc1+ #4626
> [  268.636101] Hardware name: Intel Corporation Skylake Client 
> platform/Skylake DT DDR4
> [  268.636142] task: 88044242ab80 ti: 880089c7c000 task.ti: 
> 880089c7c000
> [  268.636168] RIP: 0010:[]  [] 
> devm_component_match_release+0x35/0x50
> [  268.636205] RSP: 0018:880089c7fd88  EFLAGS: 00010202
> [  268.636224] RAX: 6b6b6b6b6b6b6b6b RBX:  RCX: 
> 0001
> [  268.636248] RDX: 880453b71260 RSI: 6b6b6b6b6b6b6b6b RDI: 
> 8804592545b8
> [  268.636272] RBP: 880089c7fda0 R08: 88045c0039d0 R09: 
> 0026
> [  268.636296] R10: 00d2 R11: 2c81 R12: 
> 8804592545b8
> [  268.636320] R13: 8804582181b0 R14: 0002 R15: 
> 880089c7fdd0
> [  268.636345] FS:  7fc3b6f74700() GS:88046c6c() 
> knlGS:
> [  268.636373] CS:  0010 DS:  ES:  CR0: 80050033
> [  268.636393] CR2: 559dae9b10e0 CR3: 00045566e000 CR4: 
> 003406e0
> [  268.636417] DR0:  DR1:  DR2: 
> 
> [  268.636442] DR3:  DR6: fffe0ff0 DR7: 
> 0400
> [  268.636468] Stack:
> [  268.636477]  880458218188 8804592545b8 880089c7fdd0 
> 880089c7fe08
> [  268.636509]  81428f50 0286 880459254818 
> 
> [  268.636542]  880459254818 880458218188 880453b71238 
> 8804592545b8
> [  268.636574] Call Trace:
> [  268.636587]  [] release_nodes+0x120/0x1e0
> [  268.636608]  [] devres_release_all+0x37/0x60
> [  268.636630]  [] __device_release_driver+0xa4/0x150
> [  268.636654]  [] driver_detach+0xac/0xb0
> [  268.636674]  [] bus_remove_driver+0x60/0xb0
> [  268.636695]  [] driver_unregister+0x27/0x50
> [  268.636717]  [] pci_unregister_driver+0x25/0x70
> [  268.636740]  [] azx_driver_exit+0x10/0x66f 
> [snd_hda_intel]
> [  268.636767]  [] SyS_delete_module+0x17b/0x240
> [  268.636789]  [] entry_SYSCALL_64_fastpath+0x12/0x6a
> [  268.636811] Code: f5 41 54 49 89 fc 53 31 db 48 83 7e 08 00 74 29 48 
> 8d 14 80 49 8b 45 10 48 8d 14 d0 48 8b 42 10 48 85 c0 74 08 48 8b 32 4c 
> 89 e7  d0 8d 43 01 49 3b 45 08 48 89 c3 72 d7 5b 41 5c 41 5d 
> 5d c3 
> [  268.636993] RIP  [] 
> devm_component_match_release+0x35/0x50
> [  268.637021]  RSP 
> [  268.637043] ---[ end trace 31731dfc9d95562b ]---
> 
> Looking at the code, I see it only happens on newer i915 chips with power 
> well support,
> so haswell/broadwell/skylake are probably affected in the functions
> snd_hdac_i915_init and snd_hdac_i915_exit.
> 
> ~Maarten
> 

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [REGRESSION] component: add support for releasing match data

2016-01-26 Thread Russell King - ARM Linux 
On Tue, Jan 26, 2016 at 03:28:34PM +0100, Maarten Lankhorst wrote:
> Something similar to a segfault. It's trying to call 0x6b6b6b6b6b which
> is POISON_FREE.
> 
> mc appears to be freed already, so calling mc->release would jump to
> invalid data.

It seems that my devm foo wasn't quite up to scratch.  Quite why it
doesn't show here while testing it (I have patches for the etnaviv
GPU driver, which I'm regularly inserting/removing) I'm not sure.

Please test this patch - it seems "no worse" for me (in that it didn't
crash before, and it still doesn't crash.)

Thanks.

 drivers/base/component.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 89f5cf68d80a..05cd26c02449 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -206,6 +206,8 @@ static void component_match_release(struct device *master,
if (mc->release)
mc->release(master, mc->data);
}
+
+   kfree(match->compare);
 }
 
 static void devm_component_match_release(struct device *dev, void *res)
@@ -221,14 +223,14 @@ static int component_match_realloc(struct device *dev,
if (match->alloc == num)
return 0;
 
-   new = devm_kmalloc_array(dev, num, sizeof(*new), GFP_KERNEL);
+   new = kmalloc_array(num, sizeof(*new), GFP_KERNEL);
if (!new)
return -ENOMEM;
 
if (match->compare) {
memcpy(new, match->compare, sizeof(*new) *
min(match->num, num));
-   devm_kfree(dev, match->compare);
+   kfree(match->compare);
}
match->compare = new;
match->alloc = num;


-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 05/15] drm/armada: Use drm_fb_helper_lastclose() and _poll_changed()

2017-10-31 Thread Russell King - ARM Linux 
On Mon, Oct 30, 2017 at 04:39:41PM +0100, Noralf Trønnes wrote:
> -static void armada_output_poll_changed(struct drm_device *dev)
> -{
> - struct armada_private *priv = dev->dev_private;
> - struct drm_fb_helper *fbh = priv->fbdev;
> -
> - if (fbh)
> - drm_fb_helper_hotplug_event(fbh);
> -}
> -
>  const struct drm_mode_config_funcs armada_drm_mode_config_funcs = {
>   .fb_create  = armada_fb_create,
> - .output_poll_changed= armada_output_poll_changed,
> + .output_poll_changed= drm_fb_helper_output_poll_changed,

I think this is fine, because although it gets rid of the NULL checks
for the drm_fb_helper structure, the driver will fail to initialise
unless priv->fbdev and the fb helper successfully initialises.

It would be nice to mention that in the commit message as to why
removing those NULL checks is safe.

In any case,

Acked-by: Russell King 

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/15] drm/armada: Use drm_fb_helper_lastclose() and _poll_changed()

2017-10-23 Thread Russell King - ARM Linux 
On Fri, Oct 20, 2017 at 01:02:03AM +0200, Noralf Trønnes wrote:
> This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
> It can also use drm_fb_helper_output_poll_changed() as its
> .output_poll_changed callback.
> 
> Cc: Russell King 

Acked-by: Russell King 

Thanks.

> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/armada/armada_drm.h   |  1 -
>  drivers/gpu/drm/armada/armada_drv.c   |  8 ++--
>  drivers/gpu/drm/armada/armada_fb.c| 11 +--
>  drivers/gpu/drm/armada/armada_fbdev.c |  8 
>  4 files changed, 3 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_drm.h 
> b/drivers/gpu/drm/armada/armada_drm.h
> index b064879ecdbd..cc4c557c9f66 100644
> --- a/drivers/gpu/drm/armada/armada_drm.h
> +++ b/drivers/gpu/drm/armada/armada_drm.h
> @@ -84,7 +84,6 @@ void armada_drm_queue_unref_work(struct drm_device *,
>  extern const struct drm_mode_config_funcs armada_drm_mode_config_funcs;
>  
>  int armada_fbdev_init(struct drm_device *);
> -void armada_fbdev_lastclose(struct drm_device *);
>  void armada_fbdev_fini(struct drm_device *);
>  
>  int armada_overlay_plane_create(struct drm_device *, unsigned long);
> diff --git a/drivers/gpu/drm/armada/armada_drv.c 
> b/drivers/gpu/drm/armada/armada_drv.c
> index e857b88a9799..4b11b6b52f1d 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include "armada_crtc.h"
>  #include "armada_drm.h"
> @@ -54,15 +55,10 @@ static struct drm_ioctl_desc armada_ioctls[] = {
>   DRM_IOCTL_DEF_DRV(ARMADA_GEM_PWRITE, armada_gem_pwrite_ioctl, 0),
>  };
>  
> -static void armada_drm_lastclose(struct drm_device *dev)
> -{
> - armada_fbdev_lastclose(dev);
> -}
> -
>  DEFINE_DRM_GEM_FOPS(armada_drm_fops);
>  
>  static struct drm_driver armada_drm_driver = {
> - .lastclose  = armada_drm_lastclose,
> + .lastclose  = drm_fb_helper_lastclose,
>   .gem_free_object_unlocked = armada_gem_free_object,
>   .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>   .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> diff --git a/drivers/gpu/drm/armada/armada_fb.c 
> b/drivers/gpu/drm/armada/armada_fb.c
> index a38d5a0892a9..ac92bce07ecd 100644
> --- a/drivers/gpu/drm/armada/armada_fb.c
> +++ b/drivers/gpu/drm/armada/armada_fb.c
> @@ -154,16 +154,7 @@ static struct drm_framebuffer *armada_fb_create(struct 
> drm_device *dev,
>   return ERR_PTR(ret);
>  }
>  
> -static void armada_output_poll_changed(struct drm_device *dev)
> -{
> - struct armada_private *priv = dev->dev_private;
> - struct drm_fb_helper *fbh = priv->fbdev;
> -
> - if (fbh)
> - drm_fb_helper_hotplug_event(fbh);
> -}
> -
>  const struct drm_mode_config_funcs armada_drm_mode_config_funcs = {
>   .fb_create  = armada_fb_create,
> - .output_poll_changed= armada_output_poll_changed,
> + .output_poll_changed= drm_fb_helper_output_poll_changed,
>  };
> diff --git a/drivers/gpu/drm/armada/armada_fbdev.c 
> b/drivers/gpu/drm/armada/armada_fbdev.c
> index a2ce83f84800..2a59db0994b2 100644
> --- a/drivers/gpu/drm/armada/armada_fbdev.c
> +++ b/drivers/gpu/drm/armada/armada_fbdev.c
> @@ -159,14 +159,6 @@ int armada_fbdev_init(struct drm_device *dev)
>   return ret;
>  }
>  
> -void armada_fbdev_lastclose(struct drm_device *dev)
> -{
> - struct armada_private *priv = dev->dev_private;
> -
> - if (priv->fbdev)
> - drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
> -}
> -
>  void armada_fbdev_fini(struct drm_device *dev)
>  {
>   struct armada_private *priv = dev->dev_private;
> -- 
> 2.14.2
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 1/6] drm/armada: Simplify drm_dev_init error log

2018-01-24 Thread Russell King - ARM Linux 
On Wed, Jan 24, 2018 at 04:18:16PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> dev_err will log the device in question and since there is a single caller
> to drm_dev_init inside this driver, the drm prefix and the function name
> can both also be safely dropped.
> 
> Signed-off-by: Tvrtko Ursulin 

Acked-by: Russell King 

Thanks.

> Cc: David Airlie 
> Cc: dri-de...@lists.freedesktop.org
> ---
>  drivers/gpu/drm/armada/armada_drv.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_drv.c 
> b/drivers/gpu/drm/armada/armada_drv.c
> index 4b11b6b52f1d..76a06bf6ebcd 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -115,8 +115,7 @@ static int armada_drm_bind(struct device *dev)
>  
>   ret = drm_dev_init(&priv->drm, &armada_drm_driver, dev);
>   if (ret) {
> - dev_err(dev, "[" DRM_NAME ":%s] drm_dev_init failed: %d\n",
> - __func__, ret);
> + dev_err(dev, "drm_dev_init failed: %d\n", ret);
>   kfree(priv);
>   return ret;
>   }
> -- 
> 2.14.1
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/19] drm: Add acquire ctx parameter to ->update_plane

2017-03-23 Thread Russell King - ARM Linux 
On Wed, Mar 22, 2017 at 10:50:41PM +0100, Daniel Vetter wrote:
> diff --git a/drivers/gpu/drm/armada/armada_overlay.c 
> b/drivers/gpu/drm/armada/armada_overlay.c
> index 34cb73d0db77..b54fd8cbd3a6 100644
> --- a/drivers/gpu/drm/armada/armada_overlay.c
> +++ b/drivers/gpu/drm/armada/armada_overlay.c
> @@ -94,7 +94,8 @@ static int
>  armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
>   struct drm_framebuffer *fb,
>   int crtc_x, int crtc_y, unsigned crtc_w, unsigned crtc_h,
> - uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h)
> + uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h,
> + struct drm_modeset_acquire_ctx *ctx)

I'm rather unhappy that we're ending up with a function taking soo many
arguments.

Most of these have to be stacked on ARM, and I'm guessing most
architectures end up doing something similar.  Is there a reason why we
don't pass pointers to drm_rect's or maybe even consider passing the
drm_plane_state structure in?

I've found that, when cleaning up these code paths in armada, that
storing all the parameters into a drm_plane_state and then validating
it with drm_plane_helper_check_state() is by way the simplest solution,
and of course, it's forward-compatible with atomic modeset.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm: provide a helper for the encoder possible_crtcs mask

2014-01-14 Thread Russell King - ARM Linux 
On Mon, Jan 13, 2014 at 02:50:46PM +0100, David Herrmann wrote:
> Hi Russel
> 
> On Mon, Jan 13, 2014 at 1:47 PM, Russell King - ARM Linux
>  wrote:
> > On Mon, Jan 13, 2014 at 12:58:54PM +0100, David Herrmann wrote:
> >> Does that -1 ever make sense? We don't support mode-object-hotplugging
> >> so all "drm_crtc" objects are known at initialization time. I'd rather
> >> put a BUG() here than a silent -1. This also makes drm_crtc_mask()
> >> redundant.
> >
> > I disagree with that last statement.  drm_crtc_mask() is still useful
> > for converting to the mask, rather than having that open coded all
> > over the place.  It probably makes more sense for drm_crtc_mask() to
> > become a helper in a header file though.
> 
> Thierry renamed your helper to drm_crtc_index() and added
> drm_crtc_mask(). If we remove the -1, drm_crtc_mask() is redundant as
> it just calls drm_crtc_index(). So I cannot follow what you mean by
> "open coded all over the place". I guess you were talking about
> drm_crtc_index()?

+uint32_t drm_crtc_mask(struct drm_crtc *crtc)
+{
+   int i = drm_crtc_index(crtc);
+
+   return i < 0 ? 0 : 1 << i;
+}
+EXPORT_SYMBOL(drm_crtc_mask);

When drm_crtc_index() no longer returns negative numbers, this becomes:

+uint32_t drm_crtc_mask(struct drm_crtc *crtc)
+{
+   return 1 << drm_crtc_index(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_mask);

Notice the conversion from an index returned by drm_crtc_index() to a
bitmask.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm: provide a helper for the encoder possible_crtcs mask

2014-01-14 Thread Russell King - ARM Linux 
On Mon, Jan 13, 2014 at 12:58:54PM +0100, David Herrmann wrote:
> Does that -1 ever make sense? We don't support mode-object-hotplugging
> so all "drm_crtc" objects are known at initialization time. I'd rather
> put a BUG() here than a silent -1. This also makes drm_crtc_mask()
> redundant.

I disagree with that last statement.  drm_crtc_mask() is still useful
for converting to the mask, rather than having that open coded all
over the place.  It probably makes more sense for drm_crtc_mask() to
become a helper in a header file though.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm: share drm_add_fake_info_node

2014-01-14 Thread Russell King - ARM Linux 
On Wed, Jan 15, 2014 at 12:25:10AM +0100, Daniel Vetter wrote:
> On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote:
> > Both i915 and Armada had the exact same implementation. For an upcoming
> > patch, I'd like to call this function from two different source files in
> > i915, and having it available externally helps there too.
> > 
> > While moving, add 'debugfs_' to the name in order to match the other drm
> > debugfs helper functions.
> > 
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Ben Widawsky 
> 
> drm_debugfs_create_files in drm_debugfs.c has the almost same code again.
> Now the problem here is that the interface is a bit botched up, since all
> the users in i915 and armada actaully faile to clean up teh debugfs dentry
> if drm_add_fake_info_node.

That's not correct - armada does clean up these, I think you need to
take a closer look at the code.

We do this by setting node->info_ent to the file operations structure,
which is a unique key to the file being registered.

Upon failure to create the fake node, we appropriately call
drm_debugfs_remove_files() with the first argument being a pointer to
the file operations.  This causes drm_debugfs_remove_files() to match
the fake entry, call debugfs_remove() on the dentry, and remove the
node from the list, and free the node structure we allocated.

Upon driver teardown, we also call drm_debugfs_remove_files() but with
each fops which should be registered, thus cleaning up each fake node
which was created.

So, Armada does clean up these entries properly.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 2/4] drm: Constify struct drm_fb_helper_funcs

2014-06-27 Thread Russell King - ARM Linux 
On Fri, Jun 27, 2014 at 05:19:23PM +0200, Thierry Reding wrote:
> From: Thierry Reding 
> 
> There's no need for this to be modifiable. Make it const so that it can
> be put into the .rodata section.
> 
> Reviewed-by: Daniel Vetter 
> Signed-off-by: Thierry Reding 

Definitely a good thing.  For Armada:

Acked-by: Russell King 

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/mm: Support 4 GiB and larger ranges

2015-01-27 Thread Russell King - ARM Linux 
On Tue, Jan 27, 2015 at 09:31:26AM +0100, Thomas Hellstrom wrote:
> On 01/27/2015 09:15 AM, Thierry Reding wrote:
> > Are you referring to the 4 GiB - 1 comment? The point I was trying to
> > make is not that the granularity of the IOVA space needs to be 1 byte
> > but rather that using an unsigned long for a size on a 32-bit machine
> > will give you 4 GiB - 1 addresses. The IOMMU page size is still 4 KiB
> > for Tegra.
> 
> I was rather referring to that if the range manager (drm_mm) is set up
> to manage pages instead of bytes
> (like, for example, the TTM VM address space), you'd get 4G - 1 pages,
> which, I believe, is sufficient on most 32 bit systems?

It'd probably also be more efficient on 32-bit systems rather than
shifting them over to using 64-bit quantities.

If the allocation granularity were to move to pages, would it be worth
moving to a pfn-based addressing system too, rather than sticking with
a byte address there too?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/18] drm/armada: Don't grab dev->struct_mutex for in mmap offset ioctl

2015-08-10 Thread Russell King - ARM Linux 
On Thu, Jul 09, 2015 at 11:32:43PM +0200, Daniel Vetter wrote:
> Since David Herrmann's mmap vma manager rework we don't need to grab
> dev->struct_mutex any more to prevent races when looking up the mmap
> offset. Drop it and instead don't forget to use the unref_unlocked
> variant (since the drm core still cares).
> 
> While at it also fix a leak when this ioctl is called on an imported
> buffer.

Good catch, thanks Daniel.  I'd prefer these to be two different commits
though, so that the leak can be easily backported to stable kernels if
needed.

I suspect this should go through the same tree that David's work is,
otherwise we end up with random dependencies between trees.  With the
bug fix separated out:

Acked-by: Russell King 

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH RFC 0/4] drm/core: restore suspend/resume calbacks in KMS drm drivers

2014-10-03 Thread Russell King - ARM Linux 
On Fri, Oct 03, 2014 at 01:39:21PM +0200, Daniel Vetter wrote:
> On Fri, Oct 3, 2014 at 11:42 AM, Andrzej Hajda  wrote:
> > But this is an issue closely connected with component framework.
> > Component framework separates master component probe and drm device
> > initialization. As a result PM ops which are synchronized with probe
> > (via device_lock)
> > are no more synchronized with drm initialization which is usually called
> > from
> > .bind callback.
> 
> Now I'm confused. component->bind and drm initialization is about
> driver load, but all this code here is about system suspend/resume
> really. So I'm rather confused what problem you actually want to fix
> ..

The component *helper* will disconnect the bind of the master device
from its probe if the components aren't already known.  If all the
components are known at the point the master device probes, the
master device lock will be held (since we'll be operating within the
master device's probe function.)

The issue here is that while the master device is being probed, the
per-device lock is held.  Beneath that lock there are locks in the
component helper which will be taken, and thus will end up depending
upon the per-device lock.

This means that we pretty much can not guarantee synchronisation between
PM on the master device and the component helper calling the bind
callback - if we tried to take the per-device lock here, we would either
deadlock, or we would end up with AB-BA lock dependencies.

What we /could/ do is expose lock/unlock functions from the component
helper so that PM implementations can synchronise with binds - or I could
look at whether we could integrate the component helper into the device
model.  I suspect the latter would not be met with enthusiasm as it would
mean adding bloat to the driver core structures, which would not be needed
in 99% of cases.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] dma-buf: add struct dma_buf_attach_info v2

2019-04-30 Thread Russell King - ARM Linux admin 
On Tue, Apr 30, 2019 at 01:10:02PM +0200, Christian König wrote:
> Add a structure for the parameters of dma_buf_attach, this makes it much 
> easier
> to add new parameters later on.

I don't understand this reasoning.  What are the "new parameters" that
are being proposed, and why do we need to put them into memory to pass
them across this interface?

If the intention is to make it easier to change the interface, passing
parameters in this manner mean that it's easy for the interface to
change and drivers not to notice the changes, since the compiler will
not warn (unless some member of the structure that the driver is using
gets removed, in which case it will error.)

Additions to the structure will go unnoticed by drivers - what if the
caller is expecting some different kind of behaviour, and the driver
ignores that new addition?

This doesn't seem to me like a good idea.

> 
> v2: rebase cleanup and fix all new implementations as well
> 
> Signed-off-by: Christian König 
> ---
>  drivers/dma-buf/dma-buf.c   | 13 +++--
>  drivers/gpu/drm/armada/armada_gem.c |  6 +-
>  drivers/gpu/drm/drm_prime.c |  6 +-
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c  |  6 +-
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c   |  6 +-
>  drivers/gpu/drm/tegra/gem.c |  6 +-
>  drivers/gpu/drm/udl/udl_dmabuf.c|  6 +-
>  .../common/videobuf2/videobuf2-dma-contig.c |  6 +-
>  .../media/common/videobuf2/videobuf2-dma-sg.c   |  6 +-
>  drivers/misc/fastrpc.c  |  6 +-
>  drivers/staging/media/tegra-vde/tegra-vde.c |  6 +-
>  drivers/xen/gntdev-dmabuf.c |  4 
>  include/linux/dma-buf.h | 17 +++--
>  13 files changed, 76 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 3ae6c0c2cc02..e295e76a8c57 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -535,8 +535,9 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
>  /**
>   * dma_buf_attach - Add the device to dma_buf's attachments list; optionally,
>   * calls attach() of dma_buf_ops to allow device-specific attach 
> functionality
> - * @dmabuf:  [in]buffer to attach device to.
> - * @dev: [in]device to be attached.
> + * @info:[in]holds all the attach related information provided
> + *   by the importer. see &struct dma_buf_attach_info
> + *   for further details.
>   *
>   * Returns struct dma_buf_attachment pointer for this attachment. Attachments
>   * must be cleaned up by calling dma_buf_detach().
> @@ -550,20 +551,20 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
>   * accessible to @dev, and cannot be moved to a more suitable place. This is
>   * indicated with the error code -EBUSY.
>   */
> -struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> -   struct device *dev)
> +struct dma_buf_attachment *dma_buf_attach(const struct dma_buf_attach_info 
> *info)
>  {
> + struct dma_buf *dmabuf = info->dmabuf;
>   struct dma_buf_attachment *attach;
>   int ret;
>  
> - if (WARN_ON(!dmabuf || !dev))
> + if (WARN_ON(!dmabuf || !info->dev))
>   return ERR_PTR(-EINVAL);
>  
>   attach = kzalloc(sizeof(*attach), GFP_KERNEL);
>   if (!attach)
>   return ERR_PTR(-ENOMEM);
>  
> - attach->dev = dev;
> + attach->dev = info->dev;
>   attach->dmabuf = dmabuf;
>  
>   mutex_lock(&dmabuf->lock);
> diff --git a/drivers/gpu/drm/armada/armada_gem.c 
> b/drivers/gpu/drm/armada/armada_gem.c
> index 642d0e70d0f8..19c47821032f 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -501,6 +501,10 @@ armada_gem_prime_export(struct drm_device *dev, struct 
> drm_gem_object *obj,
>  struct drm_gem_object *
>  armada_gem_prime_import(struct drm_device *dev, struct dma_buf *buf)
>  {
> + struct dma_buf_attach_info attach_info = {
> + .dev = dev->dev,
> + .dmabuf = buf
> + };
>   struct dma_buf_attachment *attach;
>   struct armada_gem_object *dobj;
>  
> @@ -516,7 +520,7 @@ armada_gem_prime_import(struct drm_device *dev, struct 
> dma_buf *buf)
>   }
>   }
>  
> - attach = dma_buf_attach(buf, dev->dev);
> + attach = dma_buf_attach(&attach_info);
>   if (IS_ERR(attach))
>   return ERR_CAST(attach);
>  
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index dc079efb3b0f..1dd70fc095ee 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -710,6 +710,10 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct 
> drm_device *dev,
>   struct dma_buf *dma_buf,
>   struct device *attach_dev)
>  {
> +

Re: [Intel-gfx] [PATCH 2/4] components: multiple components for a device

2019-02-08 Thread Russell King - ARM Linux admin 
On Fri, Feb 08, 2019 at 12:27:57AM +0100, Daniel Vetter wrote:
> Component framework is extended to support multiple components for
> a struct device. These will be matched with different masters based on
> its sub component value.
> 
> We are introducing this, as I915 needs two different components
> with different subcomponent value, which will be matched to two
> different component masters(Audio and HDCP) based on the subcomponent
> values.
> 
> v2: Add documenation.
> 
> v3: Rebase on top of updated documenation.
> 
> v4: Review from Rafael:
> - Remove redundant "This" from kerneldoc (also in the previous patch)
> - Streamline the logic in find_component() a bit.
> 
> Signed-off-by: Daniel Vetter  (v1 code)
> Signed-off-by: Ramalingam C  (v1 commit message)
> Cc: Ramalingam C 
> Cc: Greg Kroah-Hartman 
> Cc: Russell King 
> Cc: Rafael J. Wysocki 
> Cc: Jaroslav Kysela 
> Cc: Takashi Iwai 
> Cc: Rodrigo Vivi 
> Cc: Jani Nikula 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/base/component.c  | 158 +-
>  include/linux/component.h |  10 ++-
>  2 files changed, 129 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 1624c2a892a5..7dbc41cccd58 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -47,6 +47,7 @@ struct component;
>  struct component_match_array {
>   void *data;
>   int (*compare)(struct device *, void *);
> + int (*compare_typed)(struct device *, int, void *);
>   void (*release)(struct device *, void *);
>   struct component *component;
>   bool duplicate;
> @@ -74,6 +75,7 @@ struct component {
>   bool bound;
>  
>   const struct component_ops *ops;
> + int subcomponent;
>   struct device *dev;
>  };
>  
> @@ -158,7 +160,7 @@ static struct master *__master_find(struct device *dev,
>  }
>  
>  static struct component *find_component(struct master *master,
> - int (*compare)(struct device *, void *), void *compare_data)
> + struct component_match_array *mc)
>  {
>   struct component *c;
>  
> @@ -166,7 +168,11 @@ static struct component *find_component(struct master 
> *master,
>   if (c->master && c->master != master)
>   continue;
>  
> - if (compare(c->dev, compare_data))
> + if (mc->compare && mc->compare(c->dev, mc->data))
> + return c;
> +
> + if (mc->compare_typed &&
> + mc->compare_typed(c->dev, c->subcomponent, mc->data))
>   return c;
>   }
>  
> @@ -192,7 +198,7 @@ static int find_components(struct master *master)
>   if (match->compare[i].component)
>   continue;
>  
> - c = find_component(master, mc->compare, mc->data);
> + c = find_component(master, mc);
>   if (!c) {
>   ret = -ENXIO;
>   break;
> @@ -327,29 +333,12 @@ static int component_match_realloc(struct device *dev,
>   return 0;
>  }
>  
> -/**
> - * component_match_add_release - add a component match with release callback
> - * @master: device with the aggregate driver
> - * @matchptr: pointer to the list of component matches
> - * @release: release function for @compare_data
> - * @compare: compare function to match against all components
> - * @compare_data: opaque pointer passed to the @compare function
> - *
> - * Adds a new component match to the list stored in @matchptr, which the 
> @master
> - * aggregate driver needs to function. The list of component matches pointed 
> to
> - * by @matchptr must be initialized to NULL before adding the first match.
> - *
> - * The allocated match list in @matchptr is automatically released using devm
> - * actions, where upon @release will be called to free any references held by
> - * @compare_data, e.g. when @compare_data is a &device_node that must be
> - * released with of_node_put().
> - *
> - * See also component_match_add().
> - */
> -void component_match_add_release(struct device *master,
> +static void __component_match_add(struct device *master,
>   struct component_match **matchptr,
>   void (*release)(struct device *, void *),
> - int (*compare)(struct device *, void *), void *compare_data)
> + int (*compare)(struct device *, void *),
> + int (*compare_typed)(struct device *, int, void *),
> + void *compare_data)
>  {
>   struct component_match *match = *matchptr;
>  
> @@ -381,13 +370

Re: [Intel-gfx] [PATCH 05/15] drm/armada: Delete dma_buf->k(un)map implemenation

2019-11-25 Thread Russell King - ARM Linux admin 
On Mon, Nov 25, 2019 at 10:44:43PM +0100, Daniel Vetter wrote:
> On Mon, Nov 18, 2019 at 11:35:26AM +0100, Daniel Vetter wrote:
> > It's a dummy anyway.
> > 
> > Signed-off-by: Daniel Vetter 
> > Cc: Russell King 
> 
> I merged the entire series except this one and the final patch, sill
> waiting a bit more for an ack on this perhaps.

Acked-by: Russell King 

I thought drm trees closed around -rc6?

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/armada/armada_gem.c | 12 
> >  1 file changed, 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/armada/armada_gem.c 
> > b/drivers/gpu/drm/armada/armada_gem.c
> > index 93cf8b8bfcff..976685f2939e 100644
> > --- a/drivers/gpu/drm/armada/armada_gem.c
> > +++ b/drivers/gpu/drm/armada/armada_gem.c
> > @@ -461,16 +461,6 @@ static void armada_gem_prime_unmap_dma_buf(struct 
> > dma_buf_attachment *attach,
> > kfree(sgt);
> >  }
> >  
> > -static void *armada_gem_dmabuf_no_kmap(struct dma_buf *buf, unsigned long 
> > n)
> > -{
> > -   return NULL;
> > -}
> > -
> > -static void
> > -armada_gem_dmabuf_no_kunmap(struct dma_buf *buf, unsigned long n, void 
> > *addr)
> > -{
> > -}
> > -
> >  static int
> >  armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
> >  {
> > @@ -481,8 +471,6 @@ static const struct dma_buf_ops 
> > armada_gem_prime_dmabuf_ops = {
> > .map_dma_buf= armada_gem_prime_map_dma_buf,
> > .unmap_dma_buf  = armada_gem_prime_unmap_dma_buf,
> > .release= drm_gem_dmabuf_release,
> > -   .map= armada_gem_dmabuf_no_kmap,
> > -   .unmap  = armada_gem_dmabuf_no_kunmap,
> > .mmap   = armada_gem_dmabuf_mmap,
> >  };
> >  
> > -- 
> > 2.24.0
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> 

-- 
RMK's Patch system: https://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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 5/5] uaccess: Rename user_access_begin/end() to user_full_access_begin/end()

2020-04-21 Thread Russell King - ARM Linux admin 
On Tue, Apr 21, 2020 at 03:49:19AM +0100, Al Viro wrote:
>   The only source I'd been able to find speeks of >= 60 cycles
> (and possibly much more) for non-pipelined coprocessor instructions;
> the list of such does contain loads and stores to a bunch of registers.
> However, the register in question (p15/c3) has only store mentioned there,
> so loads might be cheap; no obvious reasons for those to be slow.
> That's a question to arm folks, I'm afraid...  rmk?

I have no information on that; instruction timings are not defined
at architecture level (architecture reference manual), nor do I find
information in the CPU technical reference manual (which would be
specific to the CPU). Instruction timings tend to be implementation
dependent.

I've always consulted Will Deacon when I've needed to know whether
an instruction is expensive or not.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 02/21] drm/armada: Introduce GEM object functions

2020-09-15 Thread Russell King - ARM Linux admin 
On Tue, Sep 15, 2020 at 04:59:39PM +0200, Thomas Zimmermann wrote:
> GEM object functions deprecate several similar callback interfaces in
> struct drm_driver. This patch replaces the per-driver callbacks with
> per-instance callbacks in armada.
> 
> Signed-off-by: Thomas Zimmermann 

Acked-by: Russell King 

Thanks.

> ---
>  drivers/gpu/drm/armada/armada_drv.c |  3 ---
>  drivers/gpu/drm/armada/armada_gem.c | 12 +++-
>  drivers/gpu/drm/armada/armada_gem.h |  2 --
>  3 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_drv.c 
> b/drivers/gpu/drm/armada/armada_drv.c
> index 980d3f1f8f16..22247cfce80b 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -37,13 +37,10 @@ DEFINE_DRM_GEM_FOPS(armada_drm_fops);
>  
>  static struct drm_driver armada_drm_driver = {
>   .lastclose  = drm_fb_helper_lastclose,
> - .gem_free_object_unlocked = armada_gem_free_object,
>   .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>   .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> - .gem_prime_export   = armada_gem_prime_export,
>   .gem_prime_import   = armada_gem_prime_import,
>   .dumb_create= armada_gem_dumb_create,
> - .gem_vm_ops = &armada_gem_vm_ops,
>   .major  = 1,
>   .minor  = 0,
>   .name   = "armada-drm",
> diff --git a/drivers/gpu/drm/armada/armada_gem.c 
> b/drivers/gpu/drm/armada/armada_gem.c
> index ecf8a55e93d9..c343fbefe47c 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -25,7 +25,7 @@ static vm_fault_t armada_gem_vm_fault(struct vm_fault *vmf)
>   return vmf_insert_pfn(vmf->vma, vmf->address, pfn);
>  }
>  
> -const struct vm_operations_struct armada_gem_vm_ops = {
> +static const struct vm_operations_struct armada_gem_vm_ops = {
>   .fault  = armada_gem_vm_fault,
>   .open   = drm_gem_vm_open,
>   .close  = drm_gem_vm_close,
> @@ -184,6 +184,12 @@ armada_gem_map_object(struct drm_device *dev, struct 
> armada_gem_object *dobj)
>   return dobj->addr;
>  }
>  
> +static const struct drm_gem_object_funcs armada_gem_object_funcs = {
> + .free = armada_gem_free_object,
> + .export = armada_gem_prime_export,
> + .vm_ops = &armada_gem_vm_ops,
> +};
> +
>  struct armada_gem_object *
>  armada_gem_alloc_private_object(struct drm_device *dev, size_t size)
>  {
> @@ -195,6 +201,8 @@ armada_gem_alloc_private_object(struct drm_device *dev, 
> size_t size)
>   if (!obj)
>   return NULL;
>  
> + obj->obj.funcs = &armada_gem_object_funcs;
> +
>   drm_gem_private_object_init(dev, &obj->obj, size);
>  
>   DRM_DEBUG_DRIVER("alloc private obj %p size %zu\n", obj, size);
> @@ -214,6 +222,8 @@ static struct armada_gem_object 
> *armada_gem_alloc_object(struct drm_device *dev,
>   if (!obj)
>   return NULL;
>  
> + obj->obj.funcs = &armada_gem_object_funcs;
> +
>   if (drm_gem_object_init(dev, &obj->obj, size)) {
>   kfree(obj);
>   return NULL;
> diff --git a/drivers/gpu/drm/armada/armada_gem.h 
> b/drivers/gpu/drm/armada/armada_gem.h
> index de04cc2c8f0e..ffcc7e8dd351 100644
> --- a/drivers/gpu/drm/armada/armada_gem.h
> +++ b/drivers/gpu/drm/armada/armada_gem.h
> @@ -21,8 +21,6 @@ struct armada_gem_object {
>   void*update_data;
>  };
>  
> -extern const struct vm_operations_struct armada_gem_vm_ops;
> -
>  #define drm_to_armada_gem(o) container_of(o, struct armada_gem_object, obj)
>  
>  void armada_gem_free_object(struct drm_gem_object *);
> -- 
> 2.28.0
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 18/51] drm/: Use drmm_add_final_kfree

2020-02-22 Thread Russell King - ARM Linux admin 
On Fri, Feb 21, 2020 at 10:02:46PM +0100, Daniel Vetter wrote:
> These are the leftover drivers that didn't have a ->release hook that
> needed to be updated.
> 
> Signed-off-by: Daniel Vetter 
> Cc: "James (Qian) Wang" 
> Cc: Liviu Dudau 
> Cc: Mihail Atanassov 
> Cc: Russell King 
> Cc: Hans de Goede 
> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 2 ++
>  drivers/gpu/drm/armada/armada_drv.c | 2 ++
>  drivers/gpu/drm/vboxvideo/vbox_drv.c| 2 ++
>  3 files changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/armada/armada_drv.c 
> b/drivers/gpu/drm/armada/armada_drv.c
> index 197dca3fc84c..dd9ed71ed942 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -12,6 +12,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -103,6 +104,7 @@ static int armada_drm_bind(struct device *dev)
>   kfree(priv);
>   return ret;
>   }
> + drmm_add_final_kfree(&priv->drm, priv);
>  
>   /* Remove early framebuffers */
>   ret = drm_fb_helper_remove_conflicting_framebuffers(NULL,

I have no visibility of what the changes behind this are, so I
can't ack this change.

-- 
RMK's Patch system: https://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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx