Re: [PATCH] gpu/docs: Clarify what userspace means for gl

2019-04-24 Thread zhoucm1



On 2019年04月25日 03:22, Eric Anholt wrote:

"Zhou, David(ChunMing)"  writes:


Will linux be only mesa-linux? I thought linux is an  open linux.
Which will impact our opengl/amdvlk(MIT open source), not sure Rocm:
1. how to deal with one uapi that opengl/amdvlk needs but mesa dont need? 
reject?
2. one hw feature that opengl/amdvlk developers work on that but no mesa
developers work on, cannot upstream as well?

I believe these questions are already covered by

"+Other userspace is only admissible if exposing a given feature through OpenGL
or
+OpenGL ES would result in a technically unsound design, incomplete driver or
+an implementation which isn't useful in real world usage."

If OpenGL needs the interface, then you need a Mesa implementation.
It's time for you to work with the community to build that or get it
built.  Or, in AMD's case, work with the Mesa developers that you
already employ.

If OpenGL doesn't need it, but Vulkan needs it, then we don't have a
clear policy in place, and this patch doesn't change that.  I would
personally say that AMDVLK doesn't qualify given that as far as I know
there is not open review of proposed patches to the project as they're
being developed.
Can I understand what you mean is, as soon as the stack is openly 
developed, then which will be able to drive new UAPI?


-David

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110510] Radeon VII HDMI issues: Flicking/system crashing

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110510

--- Comment #3 from Tom B  ---
Further testing after reading this:
https://bugs.freedesktop.org/show_bug.cgi?id=102646

If I run:

echo high > /sys/class/drm/card0/device/power_dpm_force_performance_level

Before setting the refresh rate to 59.94hz it works perfectly! The system does
not freeze at all (at least not for the 5 minutes since I tried it.)


The second I ran 

echo auto > /sys/class/drm/card0/device/power_dpm_force_performance_level

X froze. 


A rather annoying chain of bugs:

1. I have to set the refresh rate to 59.94hz to stop the screen flickering
(Don't really care about this, I'd happily do this if it was the only issue)

2. It works perfectly with a single display

3. With multiple displays, setting the refresh rate to 59.94 causes the system
to freeze and I have to to a hard reset

4. Points 2 and 3 are only relevant if  power_dpm_force_performance_level is
set to auto.


This is an acceptable compromise for now. The additional temperature/power
usage is far more palatable than the flickering screen or system freezes.

-- 
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 108521] RX 580 as eGPU amdgpu: gpu post error!

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108521

--- Comment #53 from Robert Strube  ---
Created attachment 144091
  --> https://bugs.freedesktop.org/attachment.cgi?id=144091=edit
lspci kernel 5.0.x with nividia eGPU

-- 
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 108521] RX 580 as eGPU amdgpu: gpu post error!

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108521

--- Comment #52 from Robert Strube  ---
Created attachment 144090
  --> https://bugs.freedesktop.org/attachment.cgi?id=144090=edit
dmesg log with kernel 5.0.x with nvidia eGPU

-- 
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 108521] RX 580 as eGPU amdgpu: gpu post error!

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108521

--- Comment #51 from Robert Strube  ---
Hello Everyone,

I realize it's been a long time since I updated this bug report, apologies in
advance.  I decided to give up on eGPUs + Linux (over Thunderbolt 3) for a
while, and didn't get a chance to really tackle the problem again until more
recently.

Since my initial report, I have been able to get an eGPU working with my Dell
XPS 9575, but only with a Nvidia GPU (specifically an RTX 2070).  I did try
another AMD card, but ran into the same problems.

I'll attach my dmesg and lspci information in the hopes that this might shed
some light on why the nvidia GPU works correctly (albeit with the proprietary
driver) and certain AMD GPUs don't.

-- 
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 110510] Radeon VII HDMI issues: Flicking/system crashing

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110510

--- Comment #2 from Tom B  ---
After freezing the system, here's the output from journalctl --boot=-1 | grep
amdgpu

Apr 25 00:10:18 desktop kernel: [drm] amdgpu kernel modesetting enabled.
Apr 25 00:10:18 desktop kernel: fb0: switching to amdgpudrmfb from EFI VGA
Apr 25 00:10:18 desktop kernel: amdgpu :44:00.0: No more image in the PCI
ROM
Apr 25 00:10:18 desktop kernel: amdgpu :44:00.0: VRAM: 16368M
0x0080 - 0x0083FEFF (16368M used)
Apr 25 00:10:18 desktop kernel: amdgpu :44:00.0: GART: 512M
0x - 0x1FFF
Apr 25 00:10:18 desktop kernel: amdgpu :44:00.0: AGP: 267894784M
0x0084 - 0x
Apr 25 00:10:18 desktop kernel: [drm] amdgpu: 16368M of VRAM memory ready
Apr 25 00:10:18 desktop kernel: [drm] amdgpu: 16368M of GTT memory ready.
Apr 25 00:10:18 desktop kernel: amdgpu :44:00.0: Direct firmware load for
amdgpu/vega20_ta.bin failed with error -2
Apr 25 00:10:18 desktop kernel: amdgpu :44:00.0: psp v11.0: Failed to load
firmware "amdgpu/vega20_ta.bin"
Apr 25 00:10:19 desktop kernel: fbcon: amdgpudrmfb (fb0) is primary device
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: fb0: amdgpudrmfb frame
buffer device
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring gfx uses VM inv eng 0
on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring comp_1.0.0 uses VM
inv eng 1 on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring comp_1.1.0 uses VM
inv eng 4 on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring comp_1.2.0 uses VM
inv eng 5 on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring comp_1.3.0 uses VM
inv eng 6 on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring comp_1.0.1 uses VM
inv eng 7 on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring comp_1.1.1 uses VM
inv eng 8 on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring comp_1.2.1 uses VM
inv eng 9 on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring comp_1.3.1 uses VM
inv eng 10 on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring kiq_2.1.0 uses VM inv
eng 11 on hub 0
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring sdma0 uses VM inv eng
0 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring page0 uses VM inv eng
1 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring sdma1 uses VM inv eng
4 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring page1 uses VM inv eng
5 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring uvd_0 uses VM inv eng
6 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring uvd_enc_0.0 uses VM
inv eng 7 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring uvd_enc_0.1 uses VM
inv eng 8 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring uvd_1 uses VM inv eng
9 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring uvd_enc_1.0 uses VM
inv eng 10 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring uvd_enc_1.1 uses VM
inv eng 11 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring vce0 uses VM inv eng
12 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring vce1 uses VM inv eng
13 on hub 1
Apr 25 00:10:19 desktop kernel: amdgpu :44:00.0: ring vce2 uses VM inv eng
14 on hub 1
Apr 25 00:10:19 desktop kernel: [drm] Initialized amdgpu 3.27.0 20150101 for
:44:00.0 on minor 0
Apr 25 00:11:10 desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring
sdma0 timeout, signaled seq=590, emitted seq=591
Apr 25 00:11:10 desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring
page0 timeout, signaled seq=2654, emitted seq=2656
Apr 25 00:11:10 desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring
sdma1 timeout, signaled seq=23, emitted seq=25
Apr 25 00:11:10 desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring
page1 timeout, signaled seq=1051, emitted seq=1053
Apr 25 00:11:10 desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR*
Process information: process  pid 0 thread  pid 0
Apr 25 00:11:10 desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR*
Process information: process plasmashell pid 1388 thread plasmashel:cs0 pid
1666
Apr 25 00:11:10 desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR*
Process information: process  pid 0 thread  pid 0
Apr 25 00:11:10 desktop kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR*
Process information: process kwin_x11 pid 1379 thread kwin_x11:cs0 pid 1615
Apr 25 00:11:10 desktop kernel: amdgpu :44:00.0: GPU reset begin!
Apr 25 00:11:10 desktop kernel: amdgpu :44:00.0: GPU reset begin!
Apr 25 00:11:10 desktop kernel: amdgpu :44:00.0: GPU reset begin!
Apr 25 00:11:10 desktop kernel: amdgpu :44:00.0: GPU reset begin!
Apr 25 00:11:21 desktop kernel: [drm:amdgpu_dm_atomic_check [amdgpu]] *ERROR*
[CRTC:47:crtc-0] hw_done 

[Bug 110510] Radeon VII HDMI issues: Flicking/system crashing

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110510

--- Comment #1 from Tom B  ---
This may be relevant: The crash does not happen if I run the HDMI monitor at
30hz. This means the monitors can be run at different refresh rates. 

The DisplayPort monitor does not support 59.94hz or 50hz. My conclusion is that
the crash occurs if the HDMI monitor is set to a refresh rate that the
DisplayPort monitor does not support. 


And that explains why the HDMI monitor works on its own. With only the HDMI
monitor connected, all the connected displays support 59.94 so everything is
fine.


I did try adding 3840x2160 59.94 to the displayport  monitor via xrandr to see
if I could run both displays at 59.94, unfortunately it does not seem to
support it and I just get a black screen.

-- 
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] pci/quirks: Add quirk to reset nvgpu at boot for the Lenovo ThinkPad P50

2019-04-24 Thread Lyude Paul
On Wed, 2019-04-24 at 17:36 -0500, Bjorn Helgaas wrote:
> On Wed, Apr 24, 2019 at 03:16:37PM -0400, Lyude Paul wrote:
> > On Wed, 2019-04-24 at 13:59 -0500, Bjorn Helgaas wrote:
> > > Not being a scheduled work expert, I was unsure if this experiment was
> > > equivalent to what I proposed.
> > > 
> > > I'm always suspicious of singleton solutions like this (using
> > > schedule_work() in runtime_resume()) because usually they seem to be
> > > solving a generic problem that should happen on many kinds of
> > > hardware.  The 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime)
> > > resume") commit log says:
> > > 
> > >   We need to call drm_helper_hpd_irq_event() on resume to properly
> > >   detect monitor connection / disconnection on some laptops, use
> > >   hpd_work for this to avoid deadlocks.
> > > 
> > > The situation of a monitor being connected or disconnected during
> > > suspend can happen to *any* GPU, but the commit only changes nouveau,
> > > which of course raises the question of how we deal with that in other
> > > drivers.  If the Nvidia GPU has some unique behavior related to
> > > monitor connection, that would explain special-case code there, but
> > > the commit doesn't mention anything like that.
> > > 
> > > It should be simple to revert 0b2fe6594fa2 and see whether it changes
> > > the behavior at all (well, simple except for the fact that this
> > > problem isn't 100% reproducible in the first place).
> > 
> > It's not 100% reproducible, but it's at least 90% so it's not
> > difficult for me to test at all.
> > 
> > Also, reverting this commit makes no difference either. 
> 
> OK, great, that makes it crystal clear.  I didn't know you had
> specifically tested that revert.  Thanks for doing that.
> 
> > Note that while that commit only changed nouveau, scheduled_work()
> > is exactly how a number of other drivers (i915 for instance) handle
> > reprobing like this as well.
> 
> OK.  The GPU code would be a lot more approachable if similar things
> were done in similar ways.  I spent an hour or so looking for this
> similar code in i915, but gave up.

We try
> 
> > The reason being that we can't do full connector reprobing in our
> > runtime resume thread because we could deadlock if someone else is
> > holding a modesetting lock we need and waiting on us to resume at
> > the same time (there's a number of other bug fixes in nouveau for
> > other issues caused by the same deadlock scenario). 
> 
> You mention nouveau specifically here, but I assume this is a generic
> deadlock scenario that applies to any GPU, and they all avoid the
> deadlock in the same way.  Right?

Yes-there is only one exception in the tree that I know of (amdgpu/radeon),
but fixing that is on my todo if someone else hasn't already gotten to it yet.

> 
> > I'm confused here though, it sounds like you're running under the
> > assumption that PCI devices like this aren't reset into a clean
> > state during a system reboot, is that correct?
> 
> No, I wasn't trying to say anything about that.  My point here is
> that:
> 
>   - you're reporting a problem that only happens with nouveau and
> only happens during shutdown/reboot
>   - the behavior is similar to a race (not 100% reproducible, seems
> to happen more if shutdown is faster)
>   - shutdown involves resuming the device (see pci_device_shutdown())
>   - nouveau_pmops_runtime_resume() schedules asynchronous work, which
> (to my untrained eye) looks unusual
>   - asynchronous work is inherently subject to races
> 
> So I think that's all somewhat suspicious.  But if the same problem
> happens without the asynchronous work, obviously the issue is
> elsewhere.
> 
> But you *are* right that if the device were actually reset, none of
> this should matter.  It certainly seems that the BIOS neglects to
> reset it in some cases.
> 
> I can sort of imagine a BIOS engineer thinking that if the device
> looks like it's in use, we shouldn't reset it, and it's still
> conceivable that some sort of Linux shutdown race could leave it
> looking like it's in use.  But you've been working with Lenovo on
> this, and it seems like that would be pretty obvious to somebody with
> the BIOS source (though I just demonstrated above that even with the
> source it's easy to miss things).

Yeah, that's what I thought as well! This experience has taught me that
there's a surprising amount of things about BIOSes that BIOS engineers don't
seem to know about…

> 
> I'm out of ideas, so I think your quirk is the best way forward.  I
> trimmed out some of the commit log backtraces and such, added the
> bugzilla, and tweaked the patch to use pci_iomap() instead of
> ioremap().  Would the patch below work for you?

Works for me, tested on the problematic P50 as well.

> 
> 
> commit 18dc5b3c7ddc
> Author: Lyude Paul 
> Date:   Tue Feb 12 17:02:30 2019 -0500
> 
> PCI: Reset Lenovo ThinkPad P50 nvgpu at boot if necessary
> 
> On ThinkPad P50 SKUs with an Nvidia 

[pull] sched, ttm drm-fixes-5.1

2019-04-24 Thread Alex Deucher
Hi Dave, Daniel,

Pretty quiet.  Just two fixes:
- ttm regression fix
- sched documentation fix

The following changes since commit 00fd14ff3017f64a9a03a08291e4be0d87bedc17:

  Merge branch 'drm-fixes-5.1' of git://people.freedesktop.org/~agd5f/linux 
into drm-fixes (2019-04-18 06:56:35 +1000)

are available in the Git repository at:

  git://people.freedesktop.org/~agd5f/linux drm-fixes-5.1

for you to fetch changes up to f5d356328d676deca698d01324000e0d98fba643:

  drm/sched: Fix description of drm_sched_stop (2019-04-23 11:15:38 -0500)


Christian König (1):
  drm/ttm: fix re-init of global structures

Jonathan Neuschäfer (1):
  drm/sched: Fix description of drm_sched_stop

 drivers/gpu/drm/scheduler/sched_main.c |  3 +--
 drivers/gpu/drm/ttm/ttm_bo.c   | 10 +-
 drivers/gpu/drm/ttm/ttm_memory.c   |  5 +++--
 include/drm/ttm/ttm_bo_driver.h|  1 -
 4 files changed, 9 insertions(+), 10 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110510] Radeon VII HDMI issues: Flicking/system crashing

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110510

Bug ID: 110510
   Summary: Radeon VII HDMI issues: Flicking/system crashing
   Product: DRI
   Version: XOrg git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: t...@r.je

Created attachment 144089
  --> https://bugs.freedesktop.org/attachment.cgi?id=144089=edit
dmesg after setting HDMI screen to 59.94hz

I have a radeon VII with two 4k displays. One connected via HDMI and one
connected via DisplayPort. Neither monitor supports freesync.

Linux 5.0.9 
Mesa 19.0.3

The DisplayPort monitor is working fine.

The HDMI monitor has some problems. On its own I can get it working perfectly.
Here's the output from xrandr:

DisplayPort-2 connected primary 3840x2160+0+0 (normal left inverted right x
axis y axis) 521mm x 293mm
   3840x2160 60.00*+  30.0030.0024.0029.9723.98  
   1920x1200 60.00  
   1920x1080 60.0050.0059.9430.0024.0029.9723.98  
   1600x1200 60.00  
   1680x1050 59.95  
   1280x1024 75.0260.02  
   1440x900  59.89  
   1280x960  60.00  
   1280x800  59.81  
   1152x864  75.00  
   1280x768  59.87  
   1280x720  60.0050.0059.94  
   1024x768  75.0370.0760.00  
   832x624   74.55  
   800x600   72.1975.0060.3256.25  
   720x576   50.00  
   720x480   60.0059.94  
   640x480   75.0072.8166.6760.0059.94  
   720x400   70.08  
HDMI-A-0 connected 3840x2160+3840+0 (normal left inverted right x axis y axis)
521mm x 293mm
   3840x2160 60.00*+  60.0050.0059.9430.0030.0024.00   
29.9723.98  
   1920x1200 60.00  
   1920x1080 60.0050.0059.9430.0024.0029.9723.98  
   1600x1200 60.00  
   1680x1050 59.88  
   1280x1024 75.0260.02  
   1440x900  59.90  
   1280x960  60.00  
   1280x800  59.91  
   1152x864  75.00  
   1280x768  59.87  
   1280x720  60.0050.0059.94  
   1024x768  75.0370.0760.00  
   832x624   74.55  
   800x600   72.1975.0060.3256.25  
   720x576   50.00  
   720x480   60.0059.94  
   640x480   75.0072.8166.6760.0059.94  
   720x400   70.08  


If it's connected on it shows two problems intermittently:  

1. It flickers occasionally with visual artifacts
2. The screen goes black for a few seconds then comes back


With just the HDMI screen connected this can be solved. If I set the refresh
rate to 59.94 the problems go away and everything is working flawlessly. 

However, as soon as I connect the second monitor by displayport this fix  no
longer works.

With both monitors connected and both running at 60.0hz, the displayport screen
is fine but the HDMI screen flickers. An easy fix I thought: Run the HMDI
screen at 59.94hz and the flicker will go away.


Unfortunately, what actually happens is the entire session freezes after a few
seconds on both X and Wayland. Usually it requires a complete system reset but
I have managed to recover from it a couple of times and I've attached my dmesg
output.

Sometimes the freeze is instant, other times I get up to 10 seconds before the
system freezes.


I also tried 50hz on the HDMI monitor and the same thing happened.

There appears to be two different issues here:


1. HDMI flickers 
2. Running two monitors at different refresh rates causes the driver to crash

-- 
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] pci/quirks: Add quirk to reset nvgpu at boot for the Lenovo ThinkPad P50

2019-04-24 Thread Bjorn Helgaas
On Wed, Apr 24, 2019 at 03:16:37PM -0400, Lyude Paul wrote:
> On Wed, 2019-04-24 at 13:59 -0500, Bjorn Helgaas wrote:
> > Not being a scheduled work expert, I was unsure if this experiment was
> > equivalent to what I proposed.
> > 
> > I'm always suspicious of singleton solutions like this (using
> > schedule_work() in runtime_resume()) because usually they seem to be
> > solving a generic problem that should happen on many kinds of
> > hardware.  The 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime)
> > resume") commit log says:
> > 
> >   We need to call drm_helper_hpd_irq_event() on resume to properly
> >   detect monitor connection / disconnection on some laptops, use
> >   hpd_work for this to avoid deadlocks.
> > 
> > The situation of a monitor being connected or disconnected during
> > suspend can happen to *any* GPU, but the commit only changes nouveau,
> > which of course raises the question of how we deal with that in other
> > drivers.  If the Nvidia GPU has some unique behavior related to
> > monitor connection, that would explain special-case code there, but
> > the commit doesn't mention anything like that.
> > 
> > It should be simple to revert 0b2fe6594fa2 and see whether it changes
> > the behavior at all (well, simple except for the fact that this
> > problem isn't 100% reproducible in the first place).
> 
> It's not 100% reproducible, but it's at least 90% so it's not
> difficult for me to test at all.
> 
> Also, reverting this commit makes no difference either. 

OK, great, that makes it crystal clear.  I didn't know you had
specifically tested that revert.  Thanks for doing that.

> Note that while that commit only changed nouveau, scheduled_work()
> is exactly how a number of other drivers (i915 for instance) handle
> reprobing like this as well.

OK.  The GPU code would be a lot more approachable if similar things
were done in similar ways.  I spent an hour or so looking for this
similar code in i915, but gave up.

> The reason being that we can't do full connector reprobing in our
> runtime resume thread because we could deadlock if someone else is
> holding a modesetting lock we need and waiting on us to resume at
> the same time (there's a number of other bug fixes in nouveau for
> other issues caused by the same deadlock scenario). 

You mention nouveau specifically here, but I assume this is a generic
deadlock scenario that applies to any GPU, and they all avoid the
deadlock in the same way.  Right?

> I'm confused here though, it sounds like you're running under the
> assumption that PCI devices like this aren't reset into a clean
> state during a system reboot, is that correct?

No, I wasn't trying to say anything about that.  My point here is
that:

  - you're reporting a problem that only happens with nouveau and
only happens during shutdown/reboot
  - the behavior is similar to a race (not 100% reproducible, seems
to happen more if shutdown is faster)
  - shutdown involves resuming the device (see pci_device_shutdown())
  - nouveau_pmops_runtime_resume() schedules asynchronous work, which
(to my untrained eye) looks unusual
  - asynchronous work is inherently subject to races

So I think that's all somewhat suspicious.  But if the same problem
happens without the asynchronous work, obviously the issue is
elsewhere.

But you *are* right that if the device were actually reset, none of
this should matter.  It certainly seems that the BIOS neglects to
reset it in some cases.

I can sort of imagine a BIOS engineer thinking that if the device
looks like it's in use, we shouldn't reset it, and it's still
conceivable that some sort of Linux shutdown race could leave it
looking like it's in use.  But you've been working with Lenovo on
this, and it seems like that would be pretty obvious to somebody with
the BIOS source (though I just demonstrated above that even with the
source it's easy to miss things).

I'm out of ideas, so I think your quirk is the best way forward.  I
trimmed out some of the commit log backtraces and such, added the
bugzilla, and tweaked the patch to use pci_iomap() instead of
ioremap().  Would the patch below work for you?


commit 18dc5b3c7ddc
Author: Lyude Paul 
Date:   Tue Feb 12 17:02:30 2019 -0500

PCI: Reset Lenovo ThinkPad P50 nvgpu at boot if necessary

On ThinkPad P50 SKUs with an Nvidia Quadro M1000M instead of the M2000M
variant, the BIOS does not always reset the secondary Nvidia GPU during
reboot if the laptop is configured in Hybrid Graphics mode.  The reason is
unknown, but the following steps and possibly a good bit of patience will
reproduce the issue:

  1. Boot up the laptop normally in Hybrid Graphics mode
  2. Make sure nouveau is loaded and that the GPU is awake
  2. Allow the Nvidia GPU to runtime suspend itself after being idle
  3. Reboot the machine, the more sudden the better (e.g sysrq-b may help)
  4. If nouveau loads up properly, reboot the machine again and go 

[PATCH 1/2 v2] drm/doc: Allow new UAPI to be used once it's in drm-next/drm-misc-next.

2019-04-24 Thread Eric Anholt
I was trying to figure out if it was permissible to merge the Mesa
side of V3D's CSD support yet while it's in drm-misc-next but not
drm-next, and developers on #dri-devel IRC had differing opinions of
what the requirement was.

v2: Restrict to just drm-next or drm-misc-next on airlied's request.

Signed-off-by: Eric Anholt 
---
 Documentation/gpu/drm-uapi.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index c9fd23efd957..b7a96dc02d21 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -92,9 +92,9 @@ leads to a few additional requirements:
   requirements by doing a quick fork.
 
 - The kernel patch can only be merged after all the above requirements are met,
-  but it **must** be merged **before** the userspace patches land. uAPI always 
flows
-  from the kernel, doing things the other way round risks divergence of the 
uAPI
-  definitions and header files.
+  but it **must** be merged to either drm-next or drm-misc-next **before** the
+  userspace patches land. uAPI always flows from the kernel, doing things the
+  other way round risks divergence of the uAPI definitions and header files.
 
 These are fairly steep requirements, but have grown out from years of shared
 pain and experience with uAPI added hastily, and almost always regretted about
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/gem: Fix sphinx warnings

2019-04-24 Thread Eric Anholt
Sean Paul  writes:

> From: Sean Paul 
>
> Sphinx really wants colons after arguments :/
>
> Fixes the following warnings:
> drm_gem.c:1384: warning: Function parameter or member 'fence_array' not 
> described in 'drm_gem_fence_array_add'
> drm_gem.c:1384: warning: Function parameter or member 'fence' not described 
> in 'drm_gem_fence_array_add'
> drm_gem.c:1435: warning: Function parameter or member 'fence_array' not 
> described in 'drm_gem_fence_array_add_implicit'
> drm_gem.c:1435: warning: Function parameter or member 'obj' not described in 
> 'drm_gem_fence_array_add_implicit'
> drm_gem.c:1435: warning: Function parameter or member 'write' not described 
> in 'drm_gem_fence_array_add_implicit'

Hopefully some day we can move to gitlab and have CI and make sure we
don't screw this stuff up ever again.

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PULL] drm-intel-fixes

2019-04-24 Thread Rodrigo Vivi
Hi Dave and Daniel,

This has been a very quiet week. The only 2 patches here
was queued last week.

drm-intel-fixes-2019-04-24:

A fix for display lanes calculation for BXT and a protection
to avoid enabling FEC without DSC.

Thanks,
Rodrigo.

The following changes since commit 3f5f5d534bd40b666cf37bbeeb48bfe6c2efc1e0:

  Merge tag 'gvt-fixes-2019-04-11' of https://github.com/intel/gvt-linux into 
drm-intel-fixes (2019-04-11 09:18:14 -0700)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2019-04-24

for you to fetch changes up to f5c58ba18ab8ea2169670ed880e4d31ed772ad10:

  drm/i915: Restore correct bxt_ddi_phy_calc_lane_lat_optim_mask() calculation 
(2019-04-15 09:36:42 -0700)


A fix for display lanes calculation for BXT and a protection
to avoid enabling FEC without DSC.


Ville Syrjälä (2):
  drm/i915: Do not enable FEC without DSC
  drm/i915: Restore correct bxt_ddi_phy_calc_lane_lat_optim_mask() 
calculation

 drivers/gpu/drm/i915/intel_ddi.c | 6 --
 drivers/gpu/drm/i915/intel_dp.c  | 6 +++---
 2 files changed, 7 insertions(+), 5 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 24/25] drm: kirin: Pass driver data to crtc init and plane init

2019-04-24 Thread John Stultz
On Wed, Apr 24, 2019 at 2:15 PM Sam Ravnborg  wrote:
> > > I missed where ade_driver_data came from.
> > > This looks an extra patch to intoduce driver_data,
> > > that maybe should be merged with an earlier version?
> >
> > I'm not sure I'm following you here. Can you clarify a bit more?
>
> So I looked at this a bit more - and got the bigger picture in place
> again.
>
> driver_data is assigned using the lookup done at probe() time.
> For now this is just assigned to ade_driver_data as this is the
> only option.
> So an indirection via driver_date or calling ade_driver_data
> direct is the same.
> And you have several patches where you migrate to use driver_data
> rather than calling ade_driver_data direct.
> It confused me that the patch introducing the lookup at probe()
> came before all call sites were migrated to use driver_data.
> But I get it now so it is fine.
>
> Maybe a few words in the commit log like:
>
> This patch refactor to call functions via driver_data,
> rather than hardcoding them via ade_driver_data.
> This is doen so we later can assing another stucture to
> driver_data to support other chips.

Sounds good! I'll integrate this into the change log.

> PS. I did not complain about your spelling mistakes in the
> changelog. I have a similar (or worse) keyboard from a spelling
> point of view.

Oh yes, a deficiency of mine. Good reminder I should run through the
logs w/ the spell checker.

Again, I appreciate the feedback!

thanks
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 24/25] drm: kirin: Pass driver data to crtc init and plane init

2019-04-24 Thread Sam Ravnborg
Hi John.

> 
> > I missed where ade_driver_data came from.
> > This looks an extra patch to intoduce driver_data,
> > that maybe should be merged with an earlier version?
> 
> I'm not sure I'm following you here. Can you clarify a bit more?

So I looked at this a bit more - and got the bigger picture in place
again.

driver_data is assigned using the lookup done at probe() time.
For now this is just assigned to ade_driver_data as this is the
only option.
So an indirection via driver_date or calling ade_driver_data
direct is the same.
And you have several patches where you migrate to use driver_data
rather than calling ade_driver_data direct.
It confused me that the patch introducing the lookup at probe()
came before all call sites were migrated to use driver_data.
But I get it now so it is fine.

Maybe a few words in the commit log like:

This patch refactor to call functions via driver_data,
rather than hardcoding them via ade_driver_data.
This is doen so we later can assing another stucture to
driver_data to support other chips.

PS. I did not complain about your spelling mistakes in the
changelog. I have a similar (or worse) keyboard from a spelling
point of view.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PULL] drm-misc-next-fixes

2019-04-24 Thread Sean Paul

Hi Da.*,
First pull from -next-fixes for 5.2. Mostly lease fixes from Daniel with a NULL
deref from Noralf.

Please pull!


drm-misc-next-fixes-2019-04-24:
- fb_helper: Fix NULL deref in legacy drivers (Noralf)
- leases: Ensure lessees can't connect to objects outside their perview (Daniel)
- leases: Enforce that lessees hold the lease for implicitly set planes (Daniel)
- leases: A few non-functional cleanups (Daniel)

Cc: Daniel Vetter 
Cc: Noralf Trønnes 

Cheers, Sean


The following changes since commit abbc0697d5fbf53f74ce0bcbe936670199764cfa:

  drm/fb: revert the i915 Actually configure untiled displays from master 
(2019-04-24 16:41:03 +1000)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2019-04-24

for you to fetch changes up to 1de7259275ca4ebc66459de6620558d3e38d4142:

  drm/fb-helper: Fix drm_fb_helper_firmware_config() NULL pointer deref 
(2019-04-24 15:57:43 +0200)


- fb_helper: Fix NULL deref in legacy drivers (Noralf)
- leases: Ensure lessees can't connect to objects outside their perview (Daniel)
- leases: Enforce that lessees hold the lease for implicitly set planes (Daniel)
- leases: A few non-functional cleanups (Daniel)

Cc: Daniel Vetter 
Cc: Noralf Trønnes 


Daniel Vetter (7):
  drm/leases: Drop object_id validation for negative ids
  drm/lease: Drop recursive leads checks
  drm/leases: Don't init to 0 in drm_master_create
  drm/lease: Check for lessor outside of locks
  drm/lease: Make sure implicit planes are leased
  drm/atomic: Wire file_priv through for property changes
  drm/atomic: -EACCESS for lease-denied crtc lookup

Noralf Trønnes (1):
  drm/fb-helper: Fix drm_fb_helper_firmware_config() NULL pointer deref

 drivers/gpu/drm/drm_atomic_uapi.c   | 36 +++-
 drivers/gpu/drm/drm_auth.c  |  2 --
 drivers/gpu/drm/drm_crtc.c  |  4 
 drivers/gpu/drm/drm_crtc_internal.h |  1 +
 drivers/gpu/drm/drm_fb_helper.c |  3 +++
 drivers/gpu/drm/drm_lease.c | 13 +++--
 drivers/gpu/drm/drm_mode_object.c   |  5 +++--
 drivers/gpu/drm/drm_plane.c |  8 
 8 files changed, 45 insertions(+), 27 deletions(-)

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/25] drm: Kirin driver cleanups to prep for Kirin960 support

2019-04-24 Thread John Stultz
On Wed, Apr 24, 2019 at 1:54 PM Sam Ravnborg  wrote:
>
> Hi John.
>
> On Tue, Apr 23, 2019 at 04:20:31PM -0700, John Stultz wrote:
> > This patchset contains one fix (in the front, so its easier to
> > eventually backport), and a series of changes from YiPing to
> > refactor the kirin drm driver so that it can be used on both
> > kirin620 based devices (like the original HiKey board) as well
> > as kirin960 based devices (like the HiKey960 board).
> >
> > The full kirin960 drm support is still being refactored, but as
> > this base kirin rework was getting to be substantial, I wanted
> > to send out the first chunk for some initial review, so that the
> > review burden wasn't overwhelming.
> >
> > The full HiKey960 patch stack can be found here:
> >   
> > https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/hikey960-mainline-WIP
>
> On the mailing list we are missing patch 25/25 - I only realize now.

Oof. Somehow I didn't add my Cc: list to the commit message. Apologies!

You can find it here:
  https://lkml.org/lkml/2019/4/23/1140

thanks
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/dp_mst: Expose build_mst_prop_path()

2019-04-24 Thread Lyude Paul
On Wed, 2019-04-24 at 23:52 +0300, Ville Syrjälä wrote:
> On Wed, Apr 24, 2019 at 08:40:30PM +, Li, Sun peng (Leo) wrote:
> > 
> > 
> > On 2019-04-24 1:26 p.m., Lyude Paul wrote:
> > > Closer, but are we sure we want to use the MST prop path for this? Why
> > > not add
> > > a sysfs attribute with the corresponding DRM connector name instead
> > > since the
> > > connector itself will have a path property. That way we can associate
> > > aux
> > > devices for eDP and DP devices with their corresponding connectors as
> > > well
> > 
> > I thought about that as well, but I hit a wall when trying to get the
> > SST connector from the aux device. Perhaps there's a simpler way that
> > I'm overlooking?
> > 
> > It's easier for MST, since the mst_port can be obtained via container_of
> > dp_aux. port->connector would then give what we want.
> > 
> > For SST though, each driver calls drm_aux_register() with an aux struct
> > that they've initialized. I'm not sure how I can reliably get the
> > drm_connector from that.
> 
> On i915 the aux is a child of the connector, so no extra
> attributes/links needed. Maybe other drivers should/could
> follow  that apporach as well?

ooo, good point. Yeah that seems like it would be worth a shot since it'd be a
little nicer then just adding more sysfs attributes. But otherwise if that
doesn't work, adding a connector parameter to drm_dp_aux_register() should be
fine.

> 
-- 
Cheers,
Lyude Paul

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/25] drm: Kirin driver cleanups to prep for Kirin960 support

2019-04-24 Thread Sam Ravnborg
Hi John.

On Tue, Apr 23, 2019 at 04:20:31PM -0700, John Stultz wrote:
> This patchset contains one fix (in the front, so its easier to
> eventually backport), and a series of changes from YiPing to
> refactor the kirin drm driver so that it can be used on both
> kirin620 based devices (like the original HiKey board) as well
> as kirin960 based devices (like the HiKey960 board).
> 
> The full kirin960 drm support is still being refactored, but as
> this base kirin rework was getting to be substantial, I wanted
> to send out the first chunk for some initial review, so that the
> review burden wasn't overwhelming.
> 
> The full HiKey960 patch stack can be found here:
>   
> https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/hikey960-mainline-WIP

On the mailing list we are missing patch 25/25 - I only realize now.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/dp_mst: Expose build_mst_prop_path()

2019-04-24 Thread Ville Syrjälä
On Wed, Apr 24, 2019 at 08:40:30PM +, Li, Sun peng (Leo) wrote:
> 
> 
> 
> On 2019-04-24 1:26 p.m., Lyude Paul wrote:
> > Closer, but are we sure we want to use the MST prop path for this? Why not 
> > add
> > a sysfs attribute with the corresponding DRM connector name instead since 
> > the
> > connector itself will have a path property. That way we can associate aux
> > devices for eDP and DP devices with their corresponding connectors as well
> 
> I thought about that as well, but I hit a wall when trying to get the
> SST connector from the aux device. Perhaps there's a simpler way that
> I'm overlooking?
> 
> It's easier for MST, since the mst_port can be obtained via container_of
> dp_aux. port->connector would then give what we want.
> 
> For SST though, each driver calls drm_aux_register() with an aux struct
> that they've initialized. I'm not sure how I can reliably get the
> drm_connector from that.

On i915 the aux is a child of the connector, so no extra
attributes/links needed. Maybe other drivers should/could
follow  that apporach as well?

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/gem: Fix sphinx warnings

2019-04-24 Thread Sean Paul
From: Sean Paul 

Sphinx really wants colons after arguments :/

Fixes the following warnings:
drm_gem.c:1384: warning: Function parameter or member 'fence_array' not 
described in 'drm_gem_fence_array_add'
drm_gem.c:1384: warning: Function parameter or member 'fence' not described in 
'drm_gem_fence_array_add'
drm_gem.c:1435: warning: Function parameter or member 'fence_array' not 
described in 'drm_gem_fence_array_add_implicit'
drm_gem.c:1435: warning: Function parameter or member 'obj' not described in 
'drm_gem_fence_array_add_implicit'
drm_gem.c:1435: warning: Function parameter or member 'write' not described in 
'drm_gem_fence_array_add_implicit'

Fixes: 5d5a179d3e90 ("drm: Add helpers for setting up an array of dma_fence 
dependencies.")
Cc: Eric Anholt 
Cc: Qiang Yu  (v1)
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/drm_gem.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index fae4676707b6..50de138c89e0 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1372,8 +1372,8 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations);
  * drm_gem_fence_array_add - Adds the fence to an array of fences to be
  * waited on, deduplicating fences from the same context.
  *
- * @fence_array array of dma_fence * for the job to block on.
- * @fence the dma_fence to add to the list of dependencies.
+ * @fence_array: array of dma_fence * for the job to block on.
+ * @fence: the dma_fence to add to the list of dependencies.
  *
  * Returns:
  * 0 on success, or an error on failing to expand the array.
@@ -1423,9 +1423,9 @@ EXPORT_SYMBOL(drm_gem_fence_array_add);
  * GEM objects used in the job but before updating the reservations with your
  * own fences.
  *
- * @fence_array array of dma_fence * for the job to block on.
- * @obj the gem object to add new dependencies from.
- * @write whether the job might write the object (so we need to depend on
+ * @fence_array: array of dma_fence * for the job to block on.
+ * @obj: the gem object to add new dependencies from.
+ * @write: whether the job might write the object (so we need to depend on
  * shared fences in the reservation object).
  */
 int drm_gem_fence_array_add_implicit(struct xarray *fence_array,
-- 
Sean Paul, Software Engineer, Google / Chromium OS

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/dp_mst: Expose build_mst_prop_path()

2019-04-24 Thread Li, Sun peng (Leo)



On 2019-04-24 1:26 p.m., Lyude Paul wrote:
> Closer, but are we sure we want to use the MST prop path for this? Why not add
> a sysfs attribute with the corresponding DRM connector name instead since the
> connector itself will have a path property. That way we can associate aux
> devices for eDP and DP devices with their corresponding connectors as well

I thought about that as well, but I hit a wall when trying to get the
SST connector from the aux device. Perhaps there's a simpler way that
I'm overlooking?

It's easier for MST, since the mst_port can be obtained via container_of
dp_aux. port->connector would then give what we want.

For SST though, each driver calls drm_aux_register() with an aux struct
that they've initialized. I'm not sure how I can reliably get the
drm_connector from that.

Maybe drm_dp_aux should have a back reference to the connector? FWICT
all drivers using drm_aux_register() should be able to provide the
associated connector when calling it.

Leo


> 
> On Mon, 2019-04-22 at 19:56 -0400, sunpeng...@amd.com wrote:
>> From: Leo Li 
>>
>> To give identifiable attributes to MST DP aux devices, we can use the
>> MST relative address. Expose this function for later use.
>>
>> Signed-off-by: Leo Li 
>> ---
>>   drivers/gpu/drm/drm_dp_mst_topology.c | 4 ++--
>>   include/drm/drm_dp_mst_helper.h   | 4 
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
>> b/drivers/gpu/drm/drm_dp_mst_topology.c
>> index 2ab16c9..86ff8e2 100644
>> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
>> @@ -1120,7 +1120,7 @@ static void drm_dp_check_mstb_guid(struct
>> drm_dp_mst_branch *mstb, u8 *guid)
>>  }
>>   }
>>   
>> -static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
>> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
>>  int pnum,
>>  char *proppath,
>>  size_t proppath_size)
>> @@ -1202,7 +1202,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
>> *mstb,
>>  if (created && !port->input) {
>>  char proppath[255];
>>   
>> -build_mst_prop_path(mstb, port->port_num, proppath,
>> sizeof(proppath));
>> +drm_dp_build_mst_prop_path(mstb, port->port_num, proppath,
>> sizeof(proppath));
>>  port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr,
>> port, proppath);
>>  if (!port->connector) {
>>  /* remove it from the port list */
>> diff --git a/include/drm/drm_dp_mst_helper.h
>> b/include/drm/drm_dp_mst_helper.h
>> index 371cc28..81c8d79 100644
>> --- a/include/drm/drm_dp_mst_helper.h
>> +++ b/include/drm/drm_dp_mst_helper.h
>> @@ -602,6 +602,10 @@ void drm_dp_mst_deallocate_vcpi(struct
>> drm_dp_mst_topology_mgr *mgr,
>>   int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
>> int pbn);
>>   
>> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
>> +   int pnum,
>> +   char *proppath,
>> +   size_t proppath_size);
>>   
>>   int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>>   
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] i915: disable framebuffer compression on GeminiLake

2019-04-24 Thread Paulo Zanoni
Em qua, 2019-04-24 às 20:58 +0100, Chris Wilson escreveu:
> Quoting Jian-Hong Pan (2019-04-23 10:28:10)
> > From: Daniel Drake 
> > 
> > On many (all?) the Gemini Lake systems we work with, there is frequent
> > momentary graphical corruption at the top of the screen, and it seems
> > that disabling framebuffer compression can avoid this.
> > 
> > The ticket was reported 6 months ago and has already affected a
> > multitude of users, without any real progress being made. So, lets
> > disable framebuffer compression on GeminiLake until a solution is found.
> > 
> > Buglink: https://bugs.freedesktop.org/show_bug.cgi?id=108085
> > Signed-off-by: Daniel Drake 
> > Signed-off-by: Jian-Hong Pan 
> 
> Fixes: fd7d6c5c8f3e ("drm/i915: enable FBC on gen9+ too") ?
> Cc: Paulo Zanoni 
> Cc: Daniel Vetter 
> Cc: Jani Nikula 
> Cc:  # v4.11+
> 
> glk landed 1 month before, so that seems the earliest broken point.
> 

The bug is well reported, the bug author is helpful and it even has a
description of "steps to reproduce" that looks very easy (although I
didn't try it). Everything suggests this is a bug the display team
could actually solve with not-so-many hours of debugging.

In the meantime, unbreak the systems:
Reviewed-by: Paulo Zanoni 

> > ---
> >  drivers/gpu/drm/i915/intel_fbc.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c 
> > b/drivers/gpu/drm/i915/intel_fbc.c
> > index 656e684e7c9a..fc018f3f53a1 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -1278,6 +1278,10 @@ static int intel_sanitize_fbc_option(struct 
> > drm_i915_private *dev_priv)
> > if (!HAS_FBC(dev_priv))
> > return 0;
> >  
> > +   /* https://bugs.freedesktop.org/show_bug.cgi?id=108085 */
> > +   if (IS_GEMINILAKE(dev_priv))
> > +   return 0;
> > +
> > if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
> > return 1;
> >  
> > -- 
> > 2.21.0
> > 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/doc: Allow new UAPI to be used once it's in the driver's -next.

2019-04-24 Thread Dave Airlie
On Thu, 25 Apr 2019 at 05:35, Daniel Vetter  wrote:
>
> On Wed, Apr 24, 2019 at 11:56:16AM -0700, Eric Anholt wrote:
> > I was trying to figure out if it was permissible to merge the Mesa
> > side of V3D's CSD support yet while it's in drm-misc-next but not
> > drm-next, and developers on #dri-devel IRC had differing opinions of
> > what the requirement was.  Propose a clarification here to see if Dave
> > Airlie agrees.
> >
> > Signed-off-by: Eric Anholt 
> > ---
> >
> > Personally, I thought the rule was "has to be in drm-next", but
> > assuming our review processes aren't totally broken, this should be
> > enough.
>
> Yeah if you end up with a revert on your hands the process failed much
> harder and you get to keep the pieces no matter what. Not sure we should
> clarify whether you need a stable sha1 or not (helps with cross
> referencing uapi header updates), but imo good as is. And matches what
> I've been doing/recommending past few years.
>
> Reviewed-by: Daniel Vetter 
>
> >
> >  Documentation/gpu/drm-uapi.rst | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> > index c9fd23efd957..8e5545dfbf82 100644
> > --- a/Documentation/gpu/drm-uapi.rst
> > +++ b/Documentation/gpu/drm-uapi.rst
> > @@ -92,8 +92,9 @@ leads to a few additional requirements:
> >requirements by doing a quick fork.
> >
> >  - The kernel patch can only be merged after all the above requirements are 
> > met,
> > -  but it **must** be merged **before** the userspace patches land. uAPI 
> > always flows
> > -  from the kernel, doing things the other way round risks divergence of 
> > the uAPI
> > +  but it **must** be merged to the driver's -next tree (as documented in
> > +  MAINTAINERS) **before** the userspace patches land. uAPI always flows 
> > from
> > +  the kernel, doing things the other way round risks divergence of the uAPI
> >definitions and header files.

I'd rather restrict this to drm-next and drm-misc-next, I frankly
don't trust driver trees here to have the review practices in place.

I trust drm-misc-next to have at least had someone unrelated look over
the new api.

Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] i915: disable framebuffer compression on GeminiLake

2019-04-24 Thread Chris Wilson
Quoting Jian-Hong Pan (2019-04-23 10:28:10)
> From: Daniel Drake 
> 
> On many (all?) the Gemini Lake systems we work with, there is frequent
> momentary graphical corruption at the top of the screen, and it seems
> that disabling framebuffer compression can avoid this.
> 
> The ticket was reported 6 months ago and has already affected a
> multitude of users, without any real progress being made. So, lets
> disable framebuffer compression on GeminiLake until a solution is found.
> 
> Buglink: https://bugs.freedesktop.org/show_bug.cgi?id=108085
> Signed-off-by: Daniel Drake 
> Signed-off-by: Jian-Hong Pan 

Fixes: fd7d6c5c8f3e ("drm/i915: enable FBC on gen9+ too") ?
Cc: Paulo Zanoni 
Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc:  # v4.11+

glk landed 1 month before, so that seems the earliest broken point.

> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c 
> b/drivers/gpu/drm/i915/intel_fbc.c
> index 656e684e7c9a..fc018f3f53a1 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -1278,6 +1278,10 @@ static int intel_sanitize_fbc_option(struct 
> drm_i915_private *dev_priv)
> if (!HAS_FBC(dev_priv))
> return 0;
>  
> +   /* https://bugs.freedesktop.org/show_bug.cgi?id=108085 */
> +   if (IS_GEMINILAKE(dev_priv))
> +   return 0;
> +
> if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
> return 1;
>  
> -- 
> 2.21.0
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch V2 19/29] lockdep: Simplify stack trace handling

2019-04-24 Thread Peter Zijlstra
On Thu, Apr 18, 2019 at 10:41:38AM +0200, Thomas Gleixner wrote:
> Replace the indirection through struct stack_trace by using the storage
> array based interfaces and storing the information is a small lockdep
> specific data structure.
> 

Acked-by: Peter Zijlstra (Intel) 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch V2 18/29] lockdep: Move stack trace logic into check_prev_add()

2019-04-24 Thread Peter Zijlstra
On Thu, Apr 18, 2019 at 10:41:37AM +0200, Thomas Gleixner wrote:
> There is only one caller of check_prev_add() which hands in a zeroed struct
> stack trace and a function pointer to save_stack(). Inside check_prev_add()
> the stack_trace struct is checked for being empty, which is always
> true. Based on that one code path stores a stack trace which is unused. The
> comment there does not make sense either. It's all leftovers from
> historical lockdep code (cross release).

I was more or less expecting a revert of:

ce07a9415f26 ("locking/lockdep: Make check_prev_add() able to handle external 
stack_trace")

And then I read the comment that went with the "static struct
stack_trace trace" that got removed (in the above commit) and realized
that your patch will consume more stack entries.

The problem is when the held lock stack in check_prevs_add() has multple
trylock entries on top, in that case we call check_prev_add() multiple
times, and this patch will then save the exact same stack-trace multiple
times, consuming static resources.

Possibly we should copy what stackdepot does (but we cannot use it
directly because stackdepot uses locks; but possible we can share bits),
but that is a patch for another day I think.

So while convoluted, perhaps we should retain this code for now.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 201273] Fatal error during GPU init amdgpu RX560

2019-04-24 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=201273

--- Comment #43 from Alex Deucher (alexdeuc...@gmail.com) ---
Does booting with any of the following options on the kernel command line in
grub help?
amd_iommu=off
idle=nomwait
iommu=pt
pci=noats
Can you also try different IOMMU and SVM settings in the sbios?  E.g., change
from "auto" to "enabled" or "disabled".

-- 
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 00/25] drm: Kirin driver cleanups to prep for Kirin960 support

2019-04-24 Thread John Stultz
On Wed, Apr 24, 2019 at 10:13 AM Sam Ravnborg  wrote:
>
> Hi John.
>
> On Tue, Apr 23, 2019 at 04:20:31PM -0700, John Stultz wrote:
> > This patchset contains one fix (in the front, so its easier to
> > eventually backport), and a series of changes from YiPing to
> > refactor the kirin drm driver so that it can be used on both
> > kirin620 based devices (like the original HiKey board) as well
> > as kirin960 based devices (like the HiKey960 board).
> >
> > The full kirin960 drm support is still being refactored, but as
> > this base kirin rework was getting to be substantial, I wanted
> > to send out the first chunk for some initial review, so that the
> > review burden wasn't overwhelming.
> I have been through all 25 patches and it triggered a few minor
> comments here and there.
> I know nothing about the hisilicon driver but based on the fact
> that I know nothing about the driver you can give them all
> a (smallish)
> Reviewed-by: Sam Ravnborg 
>
> I trust your judgement to decide the feedback you want to address and
> what to ignore.

Thanks again for your time and thoughts here! I really appreciate it!

> One request:
> Could you please in a follow-up patch kill the use of drmP.h.
> We want to get rid of it one day and three fewer users are one
> small step towards this goal.

I'll take a swing at this. Thanks!
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/doc: Document expectation that userspace review looks at kernel uAPI.

2019-04-24 Thread Daniel Vetter
On Wed, Apr 24, 2019 at 11:56:17AM -0700, Eric Anholt wrote:
> The point of this review process is that userspace using the new uAPI
> can actually live with the uAPI being provided, and it's hard to know
> that without having actually looked into a kernel patch yourself.
> 
> Signed-off-by: Eric Anholt 
> Suggested-by: Daniel Vetter 
> ---
>  Documentation/gpu/drm-uapi.rst | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 8e5545dfbf82..298424b98d99 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -85,7 +85,9 @@ leads to a few additional requirements:
>  - The userspace side must be fully reviewed and tested to the standards of 
> that
>userspace project. For e.g. mesa this means piglit testcases and review on 
> the
>mailing list. This is again to ensure that the new interface actually gets 
> the
> -  job done.
> +  job done.  The userspace-side reviewer should also provide at least an
> +  Acked-by on the kernel uAPI patch indicating that they've looked at how the
> +  kernel side is implementing the new feature being used.

Answers a question that just recently came up on merging new kms
properties.

Reviewed-by: Daniel Vetter 

>  
>  - The userspace patches must be against the canonical upstream, not some 
> vendor
>fork. This is to make sure that no one cheats on the review and testing
> -- 
> 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 1/2] drm/doc: Allow new UAPI to be used once it's in the driver's -next.

2019-04-24 Thread Daniel Vetter
On Wed, Apr 24, 2019 at 11:56:16AM -0700, Eric Anholt wrote:
> I was trying to figure out if it was permissible to merge the Mesa
> side of V3D's CSD support yet while it's in drm-misc-next but not
> drm-next, and developers on #dri-devel IRC had differing opinions of
> what the requirement was.  Propose a clarification here to see if Dave
> Airlie agrees.
> 
> Signed-off-by: Eric Anholt 
> ---
> 
> Personally, I thought the rule was "has to be in drm-next", but
> assuming our review processes aren't totally broken, this should be
> enough.

Yeah if you end up with a revert on your hands the process failed much
harder and you get to keep the pieces no matter what. Not sure we should
clarify whether you need a stable sha1 or not (helps with cross
referencing uapi header updates), but imo good as is. And matches what
I've been doing/recommending past few years.

Reviewed-by: Daniel Vetter 

> 
>  Documentation/gpu/drm-uapi.rst | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index c9fd23efd957..8e5545dfbf82 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -92,8 +92,9 @@ leads to a few additional requirements:
>requirements by doing a quick fork.
>  
>  - The kernel patch can only be merged after all the above requirements are 
> met,
> -  but it **must** be merged **before** the userspace patches land. uAPI 
> always flows
> -  from the kernel, doing things the other way round risks divergence of the 
> uAPI
> +  but it **must** be merged to the driver's -next tree (as documented in
> +  MAINTAINERS) **before** the userspace patches land. uAPI always flows from
> +  the kernel, doing things the other way round risks divergence of the uAPI
>definitions and header files.
>  
>  These are fairly steep requirements, but have grown out from years of shared
> -- 
> 2.20.1
> 
> ___
> 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: [PATCH 24/25] drm: kirin: Pass driver data to crtc init and plane init

2019-04-24 Thread John Stultz
On Wed, Apr 24, 2019 at 10:09 AM Sam Ravnborg  wrote:
> On Tue, Apr 23, 2019 at 04:20:55PM -0700, John Stultz wrote:
> >  static int kirin_drm_crtc_init(struct drm_device *dev, struct drm_crtc 
> > *crtc,
> > -  struct drm_plane *plane)
> > + struct drm_plane *plane,
> > + const struct kirin_drm_data *driver_data)
>
> Indent looks wrong here.
>
..
> >   ret = drm_universal_plane_init(dev, >base, 1,
> > + driver_data->plane_funcs,
> > + driver_data->channel_formats,
> > + driver_data->channel_formats_cnt,
> > + NULL, type, NULL);
> Indent looks wrong here.

Thanks! I've now fixed those up.

> I missed where ade_driver_data came from.
> This looks an extra patch to intoduce driver_data,
> that maybe should be merged with an earlier version?

I'm not sure I'm following you here. Can you clarify a bit more?

thanks
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/25] drm: kirin: Move kirin_crtc, kirin_plane, kirin_format to kirin_drm_drv.h

2019-04-24 Thread John Stultz
On Wed, Apr 24, 2019 at 9:50 AM Sam Ravnborg  wrote:
> On Tue, Apr 23, 2019 at 04:20:42PM -0700, John Stultz wrote:
>
> This struct:
> >  /* ade-format info: */
> > -struct ade_format {
> > - u32 pixel_format;
> > - enum ade_fb_format ade_format;
> > -};
> > -
> > -static const struct ade_format ade_formats[] = {
> > +static const struct kirin_format ade_formats[] = {
> >   /* 16bpp RGB: */
> >   { DRM_FORMAT_RGB565, ADE_RGB_565 },
> >   { DRM_FORMAT_BGR565, ADE_BGR_565 },
...
> > +
> > +/* kirin-format translate table */
> > +struct kirin_format {
> > + u32 pixel_format;
> > + u32 hw_format;
> > +};
> Is renamed.
> The member hw_format is renamed and no longer uses an enum.
> (The sole user of this enum type).

So the enum values are still used, but yes, the type here shifts.

> These changes are not included in the changelog - should they be part of
> this patch?
> And also the change from enum to u32 is not understood.

So the intent is to be able to share the kirin_format structure
between both the kirin620 support and kirin960, where as the
ade_fb_format enum values are tied to the kirin620, the kirin960 has
dpe_fb_formats enum which has different values. So the u32 hw_format
value is just the generic storage for enumerated format types of
either device. So its just a map from generic pixel_format definition
-> hardware specific value for that format.

I'll try to make that change more clear in the commit message, but if
you have ideas for a simpler or cleaner way to do the same, let me
know.

thanks
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re:[PATCH] gpu/docs: Clarify what userspace means for gl

2019-04-24 Thread Eric Anholt
"Zhou, David(ChunMing)"  writes:

> Will linux be only mesa-linux? I thought linux is an  open linux.
> Which will impact our opengl/amdvlk(MIT open source), not sure Rocm:
> 1. how to deal with one uapi that opengl/amdvlk needs but mesa dont need? 
> reject?
> 2. one hw feature that opengl/amdvlk developers work on that but no mesa
> developers work on, cannot upstream as well?

I believe these questions are already covered by

"+Other userspace is only admissible if exposing a given feature through OpenGL
or
+OpenGL ES would result in a technically unsound design, incomplete driver or
+an implementation which isn't useful in real world usage."

If OpenGL needs the interface, then you need a Mesa implementation.
It's time for you to work with the community to build that or get it
built.  Or, in AMD's case, work with the Mesa developers that you
already employ.

If OpenGL doesn't need it, but Vulkan needs it, then we don't have a
clear policy in place, and this patch doesn't change that.  I would
personally say that AMDVLK doesn't qualify given that as far as I know
there is not open review of proposed patches to the project as they're
being developed.


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] pci/quirks: Add quirk to reset nvgpu at boot for the Lenovo ThinkPad P50

2019-04-24 Thread Lyude Paul
On Wed, 2019-04-24 at 13:59 -0500, Bjorn Helgaas wrote:
> Not being a scheduled work expert, I was unsure if this experiment was
> equivalent to what I proposed.
> 
> I'm always suspicious of singleton solutions like this (using
> schedule_work() in runtime_resume()) because usually they seem to be
> solving a generic problem that should happen on many kinds of
> hardware.  The 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime)
> resume") commit log says:
> 
>   We need to call drm_helper_hpd_irq_event() on resume to properly
>   detect monitor connection / disconnection on some laptops, use
>   hpd_work for this to avoid deadlocks.
> 
> The situation of a monitor being connected or disconnected during
> suspend can happen to *any* GPU, but the commit only changes nouveau,
> which of course raises the question of how we deal with that in other
> drivers.  If the Nvidia GPU has some unique behavior related to
> monitor connection, that would explain special-case code there, but
> the commit doesn't mention anything like that.
> 
> It should be simple to revert 0b2fe6594fa2 and see whether it changes
> the behavior at all (well, simple except for the fact that this
> problem isn't 100% reproducible in the first place).

It's not 100% reproducible, but it's at least 90% so it's not difficult for me
to test at all.

Also, reverting this commit makes no difference either. Note that while that
commit only changed nouveau, scheduled_work() is exactly how a number of other
drivers (i915 for instance) handle reprobing like this as well. The reason
being that we can't do full connector reprobing in our runtime resume thread
because we could deadlock if someone else is holding a modesetting lock we
need and waiting on us to resume at the same time (there's a number of other
bug fixes in nouveau for other issues caused by the same deadlock scenario). 

I'm confused here though, it sounds like you're running under the assumption
that PCI devices like this aren't reset into a clean state during a system
reboot, is that correct?

> 
> > Do we want to have this discussion on the bz btw, or is this email
> > thread fine?
> 
> Email is fine.
-- 
Cheers,
Lyude Paul

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] pci/quirks: Add quirk to reset nvgpu at boot for the Lenovo ThinkPad P50

2019-04-24 Thread Bjorn Helgaas
On Mon, Apr 15, 2019 at 02:07:18PM -0400, Lyude Paul wrote:
> On Thu, 2019-04-04 at 09:17 -0500, Bjorn Helgaas wrote:
> > [+cc Hans, author of 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime)
> > resume")]
> > 
> > On Fri, Mar 22, 2019 at 06:30:15AM -0500, Bjorn Helgaas wrote:
> > > On Thu, Mar 21, 2019 at 05:48:19PM -0500, Bjorn Helgaas wrote:
> > > > On Wed, Mar 13, 2019 at 06:25:02PM -0400, Lyude Paul wrote:
> > > > > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote:
> > > > > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote:
> > > > > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote:
> > > > > > > > On a very specific subset of ThinkPad P50 SKUs, particularly
> > > > > > > > ones that come with a Quadro M1000M chip instead of the M2000M
> > > > > > > > variant, the BIOS seems to have a very nasty habit of not
> > > > > > > > always resetting the secondary Nvidia GPU between full reboots
> > > > > > > > if the laptop is configured in Hybrid Graphics mode. The
> > > > > > > > reason for this happening is unknown, but the following steps
> > > > > > > > and possibly a good bit of patience will reproduce the issue:
> > > > > > > > 
> > > > > > > > 1. Boot up the laptop normally in Hybrid graphics mode
> > > > > > > > 2. Make sure nouveau is loaded and that the GPU is awake
> > > > > > > > 2. Allow the nvidia GPU to runtime suspend itself after being
> > > > > > > > idle
> > > > > > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b
> > > > > > > > may help)
> > > > > > > > 4. If nouveau loads up properly, reboot the machine again and go
> > > > > > > > back to
> > > > > > > > step 2 until you reproduce the issue
> > > > > > > > 
> > > > > > > > This results in some very strange behavior: the GPU will quite
> > > > > > > > literally be left in exactly the same state it was in when the
> > > > > > > > previously booted kernel started the reboot. This has all
> > > > > > > > sorts of bad sideaffects: for starters, this completely breaks
> > > > > > > > nouveau starting with a mysterious EVO channel failure that
> > > > > > > > happens well before we've actually used the EVO channel for
> > > > > > > > anything:
> > > > 
> > > > Thanks for the hybrid tutorial (snipped from this response).  IIUC,
> > > > what you said was that in hybrid mode, the Intel GPU drives the
> > > > built-in display and the Nvidia GPU drives any external displays and
> > > > may be used for DRI PRIME rendering (whatever that is).  But since you
> > > > say the Nvidia device gets runtime suspended, I assume there's no
> > > > external display here and you're not using DRI PRIME.
> > > > 
> > > > I wonder if it's related to the fact that the Nvidia GPU has been
> > > > runtime suspended before you do the reboot.  Can you try turning of
> > > > runtime power management for the GPU by setting the runpm module
> > > > parameter to 0?  I *think* this would be booting with
> > > > "nouveau.runpm=0".
> > > 
> > > Sorry, I wasn't really thinking here.  You already *said* this is
> > > related to runtime suspend.  It only happens when the Nvidia GPU has
> > > been suspended.
> > > 
> > > I don't know that much about suspend, but ISTR seeing comments about
> > > resuming devices before we shutdown.  If we do that, maybe there's
> > > some kind of race between that resume and the reboot?
> > 
> > I think we do in fact resume PCI devices before shutdown.  Here's the
> > path I'm looking at:
> > 
> >   device_shutdown
> > pm_runtime_get_noresume
> > pm_runtime_barrier
> > dev->bus->shutdown
> >   pci_device_shutdown
> > pm_runtime_resume
> >   __pm_runtime_resume(dev, 0)
> > rpm_resume(dev, 0)
> >   __update_runtime_status(dev, RPM_RESUMING)
> >   callback = RPM_GET_CALLBACK(dev, runtime_resume)
> >   rpm_callback(callback, dev)
> > __rpm_callback
> >   pci_pm_runtime_resume
> > drv->pm->runtime_resume
> >   nouveau_pmops_runtime_resume
> > nouveau_do_resume
> > schedule_work(hpd_work)   # <---
> > ...
> > nouveau_display_hpd_work
> >   pm_runtime_get_sync
> >   drm_helper_hpd_irq_event
> >   pm_runtime_mark_last_busy
> >   pm_runtime_put_sync
> > 
> > I'm curious about that "schedule_work(hpd_work)" near the end because
> > no other drivers seem to use schedule_work() in the runtime_resume
> > path, and I don't know how that synchronizes with the shutdown
> > process.  I don't see anything that waits for
> > nouveau_display_hpd_work() to complete, so it seems like something
> > that could be a race.
> > 
> > I wonder this problem would be easier to reproduce if you added a
> > sleep in nouveau_display_hpd_work() as in the first hunk below, and I
> > wonder if the 

[PATCH 2/2] drm/doc: Document expectation that userspace review looks at kernel uAPI.

2019-04-24 Thread Eric Anholt
The point of this review process is that userspace using the new uAPI
can actually live with the uAPI being provided, and it's hard to know
that without having actually looked into a kernel patch yourself.

Signed-off-by: Eric Anholt 
Suggested-by: Daniel Vetter 
---
 Documentation/gpu/drm-uapi.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 8e5545dfbf82..298424b98d99 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -85,7 +85,9 @@ leads to a few additional requirements:
 - The userspace side must be fully reviewed and tested to the standards of that
   userspace project. For e.g. mesa this means piglit testcases and review on 
the
   mailing list. This is again to ensure that the new interface actually gets 
the
-  job done.
+  job done.  The userspace-side reviewer should also provide at least an
+  Acked-by on the kernel uAPI patch indicating that they've looked at how the
+  kernel side is implementing the new feature being used.
 
 - The userspace patches must be against the canonical upstream, not some vendor
   fork. This is to make sure that no one cheats on the review and testing
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/doc: Allow new UAPI to be used once it's in the driver's -next.

2019-04-24 Thread Eric Anholt
I was trying to figure out if it was permissible to merge the Mesa
side of V3D's CSD support yet while it's in drm-misc-next but not
drm-next, and developers on #dri-devel IRC had differing opinions of
what the requirement was.  Propose a clarification here to see if Dave
Airlie agrees.

Signed-off-by: Eric Anholt 
---

Personally, I thought the rule was "has to be in drm-next", but
assuming our review processes aren't totally broken, this should be
enough.

 Documentation/gpu/drm-uapi.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index c9fd23efd957..8e5545dfbf82 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -92,8 +92,9 @@ leads to a few additional requirements:
   requirements by doing a quick fork.
 
 - The kernel patch can only be merged after all the above requirements are met,
-  but it **must** be merged **before** the userspace patches land. uAPI always 
flows
-  from the kernel, doing things the other way round risks divergence of the 
uAPI
+  but it **must** be merged to the driver's -next tree (as documented in
+  MAINTAINERS) **before** the userspace patches land. uAPI always flows from
+  the kernel, doing things the other way round risks divergence of the uAPI
   definitions and header files.
 
 These are fairly steep requirements, but have grown out from years of shared
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6 2/3] dt-bindings: backlight: add lm3630a bindings

2019-04-24 Thread Pavel Machek
On Wed 2019-04-24 10:57:50, Rob Herring wrote:
> On Wed, Apr 24, 2019 at 9:20 AM Pavel Machek  wrote:
> >
> > Hi!
> >
> > > --- /dev/null
> > > +++ 
> > > b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> > > @@ -0,0 +1,129 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/leds/backlight/lm3630a-backlight.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >
> > I'm not a great fan on links to external websites. But this is comment
> > for Rob.
> 
> This is simply how json-schema works. $schema says what meta-schema
> this file should validate against so it's versioned if we change the
> schema format. $id is just a unique identifier and is used to resolve
> $ref values. Note that these aren't live urls either (but could be at
> some point in the future).

Umm. Interesting design :-).

> > > +title: TI LM3630A High-Efficiency Dual-String White LED
> > > +
> > > +maintainers:
> > > +  - Lee Jones 
> > > +  - Daniel Thompson 
> > > +  - Jingoo Han 
> >
> > Not a great fan of duplicating MAINTAINERS information here. But this
> > is a comment for Rob, again.
> 
> Hopefully this is temporary until we move to per directory MAINTAINERS
> files. Not sure what happened with that.

Aha, ok.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/mcde: Add new driver for ST-Ericsson MCDE

2019-04-24 Thread Sam Ravnborg
Hi Linus.

A few repeated comments from last review, and then a bunch
of trivial nits below.
One major thing to consider is to use a regmap for all the register
access.
Another bigger item is the excessive use of logging. Is this relevant
now the driver works or just leftovers which should really be dropped?


> +++ b/drivers/gpu/drm/mcde/mcde_display.c
> @@ -0,0 +1,1292 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Linus Walleij 
> + * Parts of this file were based on the MCDE driver by Marcus Lorentzon
> + * (C) ST-Ericsson SA 2013
> + */
> +#include 
> +#include 
> +#include 
> +#include 
Sort includes alphabetically

> +
> +#include 
Do not use drmP in new drivers.
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
Also sort these alphabetically.

> +#include "mcde_drm.h"
> +

> +#define MCDE_DSIVID0DELAY1 0x0E18
> +#define MCDE_DSICMD0DELAY1 0x0E38
> +#define MCDE_DSIVID1DELAY1 0x0E58
> +#define MCDE_DSICMD1DELAY1 0x0E78
> +#define MCDE_DSIVID2DELAY1 0x0E98
> +#define MCDE_DSICMD2DELAY1 0x0EB8
Consider to move all the register definitiosn to a seperate .h
file - local to this driver.
Then the _display, _drv and _dpmi files contains mainly src.


> +void mcde_display_irq(struct mcde *mcde)
> +{
> + u32 mispp, misovl, mischnl;
> + bool vblank;
> +
> + /* Handle display IRQs */
> + mispp = readl(mcde->regs + MCDE_MISPP);
> + misovl = readl(mcde->regs + MCDE_MISOVL);
> + mischnl = readl(mcde->regs + MCDE_MISCHNL);
> +
> + /*
> +  * Handle IRQs from the DSI link. All IRQs from the DSI links
> +  * are just latched onto the MCDE IRQ line, so we need to traverse
> +  * any active DSI masters and check if an IRQ is originating from
> +  * them.
> +  *
> +  * TODO: Currently only one DSI link is supported.
> +  */
> + if (mcde_dsi_irq(mcde->mdsi)) {
> + u32 val;
> +
> + /*
> +  * In oneshot mode we do not send continuous updates
> +  * to the display, instead we only push out updates when
> +  * the update function is called, then we disable the
> +  * flow on the channel once we get the TE IRQ.
> +  */
> + if (mcde->oneshot_mode) {
> + spin_lock(>flow_lock);
> + if (--mcde->flow_active == 0) {
> + dev_dbg(mcde->dev, "TE0 IRQ\n");
> + /* Disable FIFO A flow */
> + val = readl(mcde->regs + MCDE_CRA0);
> + val &= ~MCDE_CRX0_FLOEN;
> + writel(val, mcde->regs + MCDE_CRA0);
> + }
> + spin_unlock(>flow_lock);
All the code that deals with flow_active would be better
in a few well-named helper functions.
Then is would be more obvious what is going on.
Also there seems to be a little code duplication
around flow_active later in this patch.

The dev_dbg(...) seems as it can flood the terminal if enabled.
Is it maybe a little to excessive logging?
See below for example.

> + }
> + }
> +
> + /* Vblank from one of the channels */
> + if (mispp & MCDE_PP_VCMPA) {
> + dev_dbg(mcde->dev, "chnl A vblank IRQ\n");
> + vblank = true;
> + }
> + if (mispp & MCDE_PP_VCMPB) {
> + dev_dbg(mcde->dev, "chnl B vblank IRQ\n");
> + vblank = true;
> + }
> + if (mispp & MCDE_PP_VCMPC0)
> + dev_dbg(mcde->dev, "chnl C0 vblank IRQ\n");
> + if (mispp & MCDE_PP_VCMPC1)
> + dev_dbg(mcde->dev, "chnl C1 vblank IRQ\n");
> + if (mispp & MCDE_PP_VSCC0)
> + dev_dbg(mcde->dev, "chnl C0 TE IRQ\n");
> + if (mispp & MCDE_PP_VSCC1)
> + dev_dbg(mcde->dev, "chnl C1 TE IRQ\n");
> + writel(mispp, mcde->regs + MCDE_RISPP);
> +
> + if (vblank)
> + drm_crtc_handle_vblank(>pipe.crtc);
> +
> + if (misovl)
> + dev_info(mcde->dev, "some stray overlay IRQ %08x\n", misovl);
> + writel(misovl, mcde->regs + MCDE_RISOVL);
> +
> + if (mischnl)
> + dev_info(mcde->dev, "some stray channel error IRQ %08x\n",
> +  mischnl);
> + writel(mischnl, mcde->regs + MCDE_RISCHNL);
> +}
> +
> +void mcde_display_disable_irqs(struct mcde *mcde)
> +{
> + /* Disable all IRQs */
> + writel(0, mcde->regs + MCDE_IMSCPP);
> + writel(0, mcde->regs + MCDE_IMSCOVL);
> + writel(0, mcde->regs + MCDE_IMSCCHNL);
> +
> + /* Clear any pending IRQs */
> + writel(0x, mcde->regs + MCDE_RISPP);
> + writel(0x, mcde->regs + MCDE_RISOVL);
> + writel(0x, mcde->regs + MCDE_RISCHNL);
> +}
> +
> +static int mcde_display_check(struct drm_simple_display_pipe *pipe,
> +struct drm_plane_state *pstate,
> +struct drm_crtc_state *cstate)
> 

[Bug 110117] Waking from Suspend causes screen to appear with grey static (like a TV with no signal)

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110117

--- Comment #8 from Craig  ---
I have updated to Ubuntu 19.04 and still getting the same result.  Please let
me know what steps to take next to get this issue resolved.

-- 
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] pci/quirks: Add quirk to reset nvgpu at boot for the Lenovo ThinkPad P50

2019-04-24 Thread Bjorn Helgaas
On Wed, Apr 24, 2019 at 01:31:09PM -0400, Lyude Paul wrote:
> Any update on this? This has been waiting for a while now

Oh, sorry, I guess we were both waiting for the other.  Let me respond to
the email with context.

> On Thu, 2019-04-04 at 09:17 -0500, Bjorn Helgaas wrote:
> > [+cc Hans, author of 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime)
> > resume")]
> -- 
> Cheers,
>   Lyude Paul
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: a word on regressions

2019-04-24 Thread Eric Anholt
Dave Airlie  writes:

> I've been looking a bit at 5.0 for a few things recently, and I've
> noticed it shipped with a bunch of regressions, that I'm trying to
> smash.
>
> udl driver regression due to gem unlocked cleanup
> udl driver unload regression due to other unplug changes
> i915 + atomic x.org modesetting driver break
> i915 eDP faster/stronger/wider patch fallout
> virtio-gpu prime removal broke DRI3
>
> Now in some of the cases here a revert has had to be argued for by me,
> and in some cases a revert hasn't happened as quickly as I'd like.
>
> But one thing I've seen in a couple of cases is regression nitpicking,
> which isn't acceptable to Linus and hence to me either.
>
> if the sequence happens:
> kernel A does something
> apply patch X
> kernel A+X doesn't do the same thing
>
> then patch X is broken and needs to be reverted
> - no matter what patch X fixes
> - no matter if the fault patch X uncovers is somewhere in userspace or
> in magic other lands,
> - no matter if patch X revert regresses something patch X fixes.
>
> I do not want to hear excuses that this patch isn't at fault, or it's
> the other userspace problem, it's your problem to engineer around
> that, wait 5 years, new CONFIG options, detecting the buggy userspace,
> whatever, you are engineering something, go engineer it.
>
> For anyone interested I've reverted the i915 atomic fbdev fix, the
> virtio-gpu prime removal, i915 team already did the eDP reverts, and I
> did two fixes for udl.

Thank you for tracking these and making sure the reverts happen!


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110509] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110509

james.dut...@gmail.com changed:

   What|Removed |Added

 Attachment #144086|0   |1
is obsolete||

--- Comment #4 from james.dut...@gmail.com ---
Created attachment 144087
  --> https://bugs.freedesktop.org/attachment.cgi?id=144087=edit
dmesg

dmesg

-- 
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 110509] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110509

--- Comment #3 from james.dut...@gmail.com ---
Created attachment 144086
  --> https://bugs.freedesktop.org/attachment.cgi?id=144086=edit
dmesg

dmesg during reset.

-- 
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 110509] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110509

--- Comment #2 from james.dut...@gmail.com ---
Created attachment 144085
  --> https://bugs.freedesktop.org/attachment.cgi?id=144085=edit
/usr/src/umr/build/src/app/umr -wa

Output of the wave.

-- 
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 110509] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110509

--- Comment #1 from james.dut...@gmail.com ---
Created attachment 144084
  --> https://bugs.freedesktop.org/attachment.cgi?id=144084=edit
./umr -O bits -r *.*.mmGRBM_STATUS

Output while GPU failed to reset.

-- 
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] pci/quirks: Add quirk to reset nvgpu at boot for the Lenovo ThinkPad P50

2019-04-24 Thread Lyude Paul
Any update on this? This has been waiting for a while now

On Thu, 2019-04-04 at 09:17 -0500, Bjorn Helgaas wrote:
> [+cc Hans, author of 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime)
> resume")]
-- 
Cheers,
Lyude Paul

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110509] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110509

Bug ID: 110509
   Summary: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx
timeout
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: james.dut...@gmail.com
QA Contact: dri-devel@lists.freedesktop.org

AMD Vega 56 fails to reset:
[  188.771043] Evicting PASID 32782 queues
[  188.782094] Restoring PASID 32782 queues
[  214.563362] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout,
signaled seq=19285, emitted seq=19287
[  214.563432] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information:
process ACOdyssey.exe pid 3761 thread ACOdyssey.exe pid 3761
[  214.563439] amdgpu :43:00.0: GPU reset begin!
[  214.563445] Evicting PASID 32782 queues
[  224.793032] [drm:amdgpu_dm_atomic_check [amdgpu]] *ERROR* [CRTC:49:crtc-0]
hw_done or flip_done timed out


How do I go about diagnosing this problem?

-- 
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 2/3] drm/dp_mst: Expose build_mst_prop_path()

2019-04-24 Thread Lyude Paul
Closer, but are we sure we want to use the MST prop path for this? Why not add
a sysfs attribute with the corresponding DRM connector name instead since the
connector itself will have a path property. That way we can associate aux
devices for eDP and DP devices with their corresponding connectors as well

On Mon, 2019-04-22 at 19:56 -0400, sunpeng...@amd.com wrote:
> From: Leo Li 
> 
> To give identifiable attributes to MST DP aux devices, we can use the
> MST relative address. Expose this function for later use.
> 
> Signed-off-by: Leo Li 
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 4 ++--
>  include/drm/drm_dp_mst_helper.h   | 4 
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 2ab16c9..86ff8e2 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1120,7 +1120,7 @@ static void drm_dp_check_mstb_guid(struct
> drm_dp_mst_branch *mstb, u8 *guid)
>   }
>  }
>  
> -static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
>   int pnum,
>   char *proppath,
>   size_t proppath_size)
> @@ -1202,7 +1202,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
> *mstb,
>   if (created && !port->input) {
>   char proppath[255];
>  
> - build_mst_prop_path(mstb, port->port_num, proppath,
> sizeof(proppath));
> + drm_dp_build_mst_prop_path(mstb, port->port_num, proppath,
> sizeof(proppath));
>   port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr,
> port, proppath);
>   if (!port->connector) {
>   /* remove it from the port list */
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> index 371cc28..81c8d79 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -602,6 +602,10 @@ void drm_dp_mst_deallocate_vcpi(struct
> drm_dp_mst_topology_mgr *mgr,
>  int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
>  int pbn);
>  
> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
> +int pnum,
> +char *proppath,
> +size_t proppath_size);
>  
>  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>  
-- 
Cheers,
Lyude Paul

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] ARM: dts: exynos: Increase minimal ACLK400_DISP1 frequency on Exynos542x

2019-04-24 Thread Krzysztof Kozlowski
On Tue, Mar 19, 2019 at 02:26:01PM +0100, Marek Szyprowski wrote:
> ACLK400_DISP1 bus feeds some internal buses of the display subsystem, some
> of which are also related to TV/Mixer hardware modules. When that bus
> is set to 120MHz, Exynos Mixer is not able to properly handle two XRGB
> display planes at FullHD-60MHz. DMA underrun happens, which in turn might
> result in reading data out of the configured buffer, what causes IOMMU
> page fault and kernel panic.
> 
> This change fixes the following IOMMU fault, observed, when 2 Mixer planes
> were enabled:
> 
> exynos-sysmmu 1465.sysmmu: 1445.mixer: PAGE FAULT occurred at 
> 0x20fe9000
> [ cut here ]
> kernel BUG at ../drivers/iommu/exynos-iommu.c:450!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 5 PID: 0 Comm: swapper/5 Not tainted 5.0.0-3-g1b03088168ea #149
> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> PC is at exynos_sysmmu_irq+0x1c0/0x264
> LR is at lock_is_held_type+0x44/0x64
> ...
> 
> Reported-by: Marian Mihailescu 
> Fixes: 5d99cc59a3c6 ("ARM: dts: exynos: Move Exynos5250 and Exynos5420 nodes 
> under soc")
> Fixes: b04a62d3ade3 ("ARM: dts: exynos: Add bus nodes using VDD_INT for 
> Exynos542x SoC")
> Signed-off-by: Marek Szyprowski 
> ---
>  arch/arm/boot/dts/exynos5420.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi 
> b/arch/arm/boot/dts/exynos5420.dtsi
> index aaff15880761..250f4d7182e0 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -1294,7 +1294,7 @@
>   compatible = "operating-points-v2";
>  
>   opp00 {
> - opp-hz = /bits/ 64 <12000>;
> + opp-hz = /bits/ 64 <15000>;

Some time ago we talked about all these changes on IRC. I understand
that implementing proper QoS (or fix devfreq/PPMU events to properly
indicate busy mixer) might be a big task so let's go with this
workaround. However how about adding a TODO comment about reason of
bumping the frequency?

Best regards,
Krzysztof

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/dp: Use non-cyclic idr

2019-04-24 Thread Lyude Paul
lgtm

Reviewed-by: Lyude Paul 

On Mon, 2019-04-22 at 19:56 -0400, sunpeng...@amd.com wrote:
> From: Leo Li 
> 
> In preparation for adding aux devices for DP MST, make the IDR
> non-cyclic. That way, hotplug cycling MST devices won't needlessly
> increment the minor version index.
> 
> Signed-off-by: Leo Li 
> ---
>  drivers/gpu/drm/drm_dp_aux_dev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> b/drivers/gpu/drm/drm_dp_aux_dev.c
> index 0e4f25d..6d84611 100644
> --- a/drivers/gpu/drm/drm_dp_aux_dev.c
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -80,8 +80,7 @@ static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct
> drm_dp_aux *aux)
>   kref_init(_dev->refcount);
>  
>   mutex_lock(_idr_mutex);
> - index = idr_alloc_cyclic(_idr, aux_dev, 0, DRM_AUX_MINORS,
> -  GFP_KERNEL);
> + index = idr_alloc(_idr, aux_dev, 0, DRM_AUX_MINORS, GFP_KERNEL);
>   mutex_unlock(_idr_mutex);
>   if (index < 0) {
>   kfree(aux_dev);
-- 
Cheers,
Lyude Paul

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH v3 00/11] drm/fb-helper: Move modesetting code to drm_client

2019-04-24 Thread Noralf Trønnes


Den 23.04.2019 13.04, skrev Martin Peres:
> On 20/04/2019 20:24, Noralf Trønnes wrote:
>>
>>
>> Den 20.04.2019 12.45, skrev Noralf Trønnes:
>>> This moves the modesetting code from drm_fb_helper to drm_client so it
>>> can be shared by all internal clients.
>>>
>>> Changes this time:
>>> - Use full drm_client_init/release for the modesets (Daniel Vetter)
>>> - drm_client_for_each_modeset: use lockdep_assert_held (Daniel Vetter)
>>> - Hook up to Documentation/gpu/drm-client.rst (Daniel Vetter)
>>>
>>
>> I got Fi.CI.IGT failures on this one:
>>
>>   * igt@kms_fbcon_fbt@psr:
>> - shard-skl:  PASS -> FAIL
>>
>>   * igt@kms_fbcon_fbt@psr-suspend:
>> - shard-iclb: PASS -> FAIL +1
>> - shard-skl:  NOTRUN -> FAIL
>>
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12850/
>> https://patchwork.freedesktop.org/series/58597/
>>
>> The previous version of this series reported success:
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12720/
>> But AFAICT those fbcon tests didn't succeed when I look at the details.
>>
>> I'd appreciate if someone with Intel CI knowledge could have a look at this.
> 
> The issue is real, but I honestly can't tell if this is due to your
> patches or not. There was a regression last week and we reworked some
> filters that may not apply anymore to the base that was selected to test
> your patches.
> 
> I queued a re-run! We should have the results in the next 6-12 hours.
> 

igt@kms_fbcon_fbt@psr doesn't regress anymore, but @psr-suspend still does:

  * igt@kms_fbcon_fbt@psr-suspend:
- shard-iclb: [PASS][1] -> [FAIL][2] +1 similar issue
   [1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5971/shard-iclb5/igt@kms_fbcon_...@psr-suspend.html
   [2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12854/shard-iclb2/igt@kms_fbcon_...@psr-suspend.html
- shard-skl:  [PASS][3] -> [FAIL][4] +1 similar issue
   [3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5971/shard-skl4/igt@kms_fbcon_...@psr-suspend.html
   [4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12854/shard-skl4/igt@kms_fbcon_...@psr-suspend.html

I think I'll split up the series and feed it to the CI piece by piece so
I can find the offending patch.

Thanks for taking a look.

Noralf.

> Sorry for the delay!
> Martin
> 
>>
>> Noralf.
>>
>>> Noralf.
>>>
>>> Noralf Trønnes (11):
>>>   drm/atomic: Move __drm_atomic_helper_disable_plane/set_config()
>>>   drm/fb-helper: Avoid race with DRM userspace
>>>   drm/fb-helper: No need to cache rotation and sw_rotations
>>>   drm/fb-helper: Remove drm_fb_helper_crtc->{x,y,desired_mode}
>>>   drm/fb-helper: Remove drm_fb_helper_crtc
>>>   drm/fb-helper: Prepare to move out commit code
>>>   drm/fb-helper: Move out commit code
>>>   drm/fb-helper: Remove drm_fb_helper_connector
>>>   drm/fb-helper: Prepare to move out modeset config code
>>>   drm/fb-helper: Move out modeset config code
>>>   drm/client: Hack: Add bootsplash example
>>>
>>>  Documentation/gpu/drm-client.rst |3 +
>>>  Documentation/gpu/todo.rst   |   10 +
>>>  drivers/gpu/drm/Kconfig  |5 +
>>>  drivers/gpu/drm/Makefile |3 +-
>>>  drivers/gpu/drm/drm_atomic.c |  168 
>>>  drivers/gpu/drm/drm_atomic_helper.c  |  164 ---
>>>  drivers/gpu/drm/drm_auth.c   |   20 +
>>>  drivers/gpu/drm/drm_bootsplash.c |  362 +++
>>>  drivers/gpu/drm/drm_client.c |   17 +-
>>>  drivers/gpu/drm/drm_client_modeset.c | 1085 
>>>  drivers/gpu/drm/drm_crtc_internal.h  |5 +
>>>  drivers/gpu/drm/drm_drv.c|4 +
>>>  drivers/gpu/drm/drm_fb_helper.c  | 1381 +++---
>>>  drivers/gpu/drm/drm_internal.h   |2 +
>>>  include/drm/drm_atomic_helper.h  |4 -
>>>  include/drm/drm_client.h |   49 +
>>>  include/drm/drm_fb_helper.h  |  102 +-
>>>  17 files changed, 1864 insertions(+), 1520 deletions(-)
>>>  create mode 100644 drivers/gpu/drm/drm_bootsplash.c
>>>  create mode 100644 drivers/gpu/drm/drm_client_modeset.c
>>>
>> ___
>> Intel-gfx mailing list
>> intel-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/25] drm: Kirin driver cleanups to prep for Kirin960 support

2019-04-24 Thread Sam Ravnborg
Hi John.

On Tue, Apr 23, 2019 at 04:20:31PM -0700, John Stultz wrote:
> This patchset contains one fix (in the front, so its easier to
> eventually backport), and a series of changes from YiPing to
> refactor the kirin drm driver so that it can be used on both
> kirin620 based devices (like the original HiKey board) as well
> as kirin960 based devices (like the HiKey960 board).
> 
> The full kirin960 drm support is still being refactored, but as
> this base kirin rework was getting to be substantial, I wanted
> to send out the first chunk for some initial review, so that the
> review burden wasn't overwhelming.
I have been through all 25 patches and it triggered a few minor
comments here and there.
I know nothing about the hisilicon driver but based on the fact
that I know nothing about the driver you can give them all
a (smallish)
Reviewed-by: Sam Ravnborg 

I trust your judgement to decide the feedback you want to address and
what to ignore.

One request:
Could you please in a follow-up patch kill the use of drmP.h.
We want to get rid of it one day and three fewer users are one
small step towards this goal.

Thanks,

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Support for 2D engines/blitters in V4L2 and DRM

2019-04-24 Thread Michel Dänzer
On 2019-04-24 2:19 p.m., Paul Kocialkowski wrote:
> On Wed, 2019-04-24 at 10:31 +0200, Michel Dänzer wrote:
>> On 2019-04-19 10:38 a.m., Paul Kocialkowski wrote:
>>> On Thu, 2019-04-18 at 20:30 -0400, Nicolas Dufresne wrote:
 Le jeudi 18 avril 2019 à 10:18 +0200, Daniel Vetter a écrit :
>> It would be cool if both could be used concurrently and not just return
>> -EBUSY when the device is used with the other subsystem.
>
> We live in this world already :-) I think there's even patches (or merged
> already) to add fences to v4l, for Android.

 This work is currently suspended. It will require some feature on DRM
 display to really make this useful, but there is also a lot of
 challanges in V4L2. In GFX space, most of the use case are about
 rendering as soon as possible. Though, in multimedia we have two
 problems, we need to synchronize the frame rendering with the audio,
 and output buffers may comes out of order due to how video CODECs are
 made.
>>>
>>> Definitely, it feels like the DRM display side is currently a good fit
>>> for render use cases, but not so much for precise display cases where
>>> we want to try and display a buffer at a given vblank target instead of
>>> "as soon as possible".
>>>
>>> I have a userspace project where I've implemented a page flip queue,
>>> which only schedules the next flip when relevant and keeps ready
>>> buffers in the queue until then. This requires explicit vblank
>>> syncronisation (which DRM offsers, but pretty much all other display
>>> APIs, that are higher-level don't, so I'm just using a refresh-rate
>>> timer for them) and flip done notification.
>>>
>>> I haven't looked too much at how to flip with a target vblank with DRM
>>> directly but maybe the atomic API already has the bits in for that (but
>>> I haven't heard of such a thing as a buffer queue, so that makes me
>>> doubt it).
>>
>> Not directly. What's available is that if userspace waits for vblank n
>> and then submits a flip, the flip will complete in vblank n+1 (or a
>> later vblank, depending on when the flip is submitted and when the
>> fences the flip depends on signal).
>>
>> There is reluctance allowing more than one flip to be queued in the
>> kernel, as it would considerably increase complexity in the kernel. It
>> would probably only be considered if there was a compelling use-case
>> which was outright impossible otherwise.
> 
> Well, I think it's just less boilerplace for userspace. This is indeed
> quite complex, and I prefer to see that complexity done once and well
> in Linux rather than duplicated in userspace with more or less reliable
> implementations.

That's not the only trade-off to consider, e.g. I suspect handling this
in the kernel is more complex than in userspace.


>>> Well, I need to handle stuff like SDL in my userspace project, so I have
>>> to have all that queuing stuff in software anyway, but it would be good
>>> if each project didn't have to implement that. Worst case, it could be
>>> in libdrm too.
>>
>> Usually, this kind of queuing will be handled in a display server such
>> as Xorg or a Wayland compositor, not by the application such as a video
>> player itself, or any library in the latter's address space. I'm not
>> sure there's much potential for sharing code between display servers for
>> this.
> 
> This assumes that you are using a display server, which is definitely
> not always the case (there is e.g. Kodi GBM). Well, I'm not saying it
> is essential to have it in the kernel, but it would avoid code
> duplication and lower the complexity in userspace.

For code duplication, my suggestion would be to use a display server
instead of duplicating its functionality.


 In the first, we'd need a mechanism where we can schedule a render at a
 specific time or vblank. We can of course already implement this in
 software, but with fences, the scheduling would need to be done in the
 driver. Then if the fence is signalled earlier, the driver should hold
 on until the delay is met. If the fence got signalled late, we also
 need to think of a workflow. As we can't schedule more then one render
 in DRM at one time, I don't really see yet how to make that work.
>>>
>>> Indeed, that's also one of the main issues I've spotted. Before using
>>> an implicit fence, we basically have to make sure the frame is due for
>>> display at the next vblank. Otherwise, we need to refrain from using
>>> the fence and schedule the flip later, which is kind of counter-
>>> productive.
>>
>> [...]
> 
>>> I feel like specifying a target vblank would be a good unit for that,
>>
>> The mechanism described above works for that.
> 
> I still don't see any fence-based mechanism that can work to achieve
> that, but maybe I'm missing your point.

It's not fence based, just good old waiting for the previous vblank
before submitting the flip to the kernel.


>>> since it's our native granularity after all 

[PATCH 5/6 v2] libdrm: reduce number of reallocations in drmModeAtomicAddProperty

2019-04-24 Thread John Stultz
From: Adrian Salido 

When calling drmModeAtomicAddProperty allocation of memory
happens as needed in increments of 16 elements. This can be very
slow if there are multiple properties to be updated in an Atomic
Commit call.

Increase this to as many as can fit in a memory PAGE to avoid
having to reallocate memory too often.

Also this patch has a small one line perf tweak in
drmModeAtomicDuplicate() to only memcpy items to the cursor
position in order avoid copying the entire item array if its
mostly empty.

Cc: Emil Velikov 
Cc: Sean Paul 
Cc: Alistair Strachan 
Cc: Marissa Wall 
Reviewed-by: Alex Deucher 
Reviewed-by: Emil Velikov 
[jstultz: Expanded commit message]
Signed-off-by: John Stultz 
---
v2: Improved commit message
---
 xf86drmMode.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xf86drmMode.c b/xf86drmMode.c
index 8f8633e..c878d9e 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -1259,7 +1259,7 @@ drm_public drmModeAtomicReqPtr 
drmModeAtomicDuplicate(drmModeAtomicReqPtr old)
return NULL;
}
memcpy(new->items, old->items,
-  old->size_items * sizeof(*new->items));
+  old->cursor * sizeof(*new->items));
} else {
new->items = NULL;
}
@@ -1322,12 +1322,13 @@ drm_public int 
drmModeAtomicAddProperty(drmModeAtomicReqPtr req,
return -EINVAL;
 
if (req->cursor >= req->size_items) {
+   const uint32_t item_size_inc = getpagesize() / 
sizeof(*req->items);
drmModeAtomicReqItemPtr new;
 
-   req->size_items += 16;
+   req->size_items += item_size_inc;
new = realloc(req->items, req->size_items * 
sizeof(*req->items));
if (!new) {
-   req->size_items -= 16;
+   req->size_items -= item_size_inc;
return -ENOMEM;
}
req->items = new;
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 24/25] drm: kirin: Pass driver data to crtc init and plane init

2019-04-24 Thread Sam Ravnborg
Hi John.

On Tue, Apr 23, 2019 at 04:20:55PM -0700, John Stultz wrote:
> From: Xu YiPing 
> 
> As part of refactoring the kirin driver to better support
> different hardware revisions, this patch changes funcitons
> to pass the kirin_driver_data as a prameter.
> 
> This will allow those funcitons to be later moved to the
> generic kirin_drm_drv.c
> 
> Cc: Xinliang Liu 
> Cc: Rongrong Zou 
> Cc: Xinwei Kong 
> Cc: Chen Feng 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel 
> Signed-off-by: Xu YiPing 
> [jstultz: Reworded commit message]
> Signed-off-by: John Stultz 
> ---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 25 
> ++---
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 
> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index 71671f8..876e25b 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -572,7 +572,8 @@ static const struct drm_crtc_funcs ade_crtc_funcs = {
>  };
>  
>  static int kirin_drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> -  struct drm_plane *plane)
> + struct drm_plane *plane,
> + const struct kirin_drm_data *driver_data)

Indent looks wrong here.

>  {
>   struct device_node *port;
>   int ret;
> @@ -589,13 +590,13 @@ static int kirin_drm_crtc_init(struct drm_device *dev, 
> struct drm_crtc *crtc,
>   crtc->port = port;
>  
>   ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,
> - ade_driver_data.crtc_funcs, NULL);
> + driver_data->crtc_funcs, NULL);
>   if (ret) {
>   DRM_ERROR("failed to init crtc.\n");
>   return ret;
>   }
>  
> - drm_crtc_helper_add(crtc, ade_driver_data.crtc_helper_funcs);
> + drm_crtc_helper_add(crtc, driver_data->crtc_helper_funcs);
>  
>   return 0;
>  }


> @@ -894,21 +895,22 @@ static struct drm_plane_funcs ade_plane_funcs = {
>  
>  static int kirin_drm_plane_init(struct drm_device *dev,
>   struct kirin_plane *kplane,
> - enum drm_plane_type type)
> + enum drm_plane_type type,
> + const struct kirin_drm_data *driver_data)
>  {
>   int ret = 0;
>  
>   ret = drm_universal_plane_init(dev, >base, 1,
> + driver_data->plane_funcs,
> + driver_data->channel_formats,
> + driver_data->channel_formats_cnt,
> + NULL, type, NULL);
Indent looks wrong here.

I missed where ade_driver_data came from.
This looks an extra patch to intoduce driver_data,
that maybe should be merged with an earlier version?

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6 v2] libdrm: omap: Add DRM_RDWR flag to dmabuf export

2019-04-24 Thread John Stultz
From: Hemant Hariyani 

Allows mmap on dmabuf fd with MAP_SHARED and PROT_WRITE.

This fixes boot failures with Android (likely w/ closed source
user-space drivers) that were caused due to mmap() returning
error.

Cc: Emil Velikov 
Cc: Sean Paul 
Cc: Alistair Strachan 
Cc: Marissa Wall 
Signed-off-by: Hemant Hariyani 
[picked and updated commitmsg from 
http://git.ti.com/cgit/cgit.cgi/android/external-libdrm.git/]
Signed-off-by: Praneeth Bajjuri 
Signed-off-by: Alistair Strachan 
[jstultz: Tweaked commit message]
Signed-off-by: John Stultz 
---
v2: Tweaked commit message
---
 omap/omap_drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/omap/omap_drm.c b/omap/omap_drm.c
index 3aed4e0..ffacea6 100644
--- a/omap/omap_drm.c
+++ b/omap/omap_drm.c
@@ -414,7 +414,7 @@ drm_public int omap_bo_dmabuf(struct omap_bo *bo)
if (bo->fd < 0) {
struct drm_prime_handle req = {
.handle = bo->handle,
-   .flags = DRM_CLOEXEC,
+   .flags = DRM_CLOEXEC | DRM_RDWR,
};
int ret;
 
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/6 v2] libdrm: Avoid additional drm open close

2019-04-24 Thread John Stultz
From: Prabhanjan Kandula 

Avoid additional drm device open and close.

Cc: Emil Velikov 
Cc: Sean Paul 
Cc: Alistair Strachan 
Cc: Marissa Wall 
Reviewed-by: Alex Deucher 
Signed-off-by: John Stultz 
---
 xf86drm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index fe822ca..2c19376 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -750,8 +750,8 @@ drm_public int drmOpen(const char *name, const char *busid)
  */
 drm_public int drmOpenWithType(const char *name, const char *busid, int type)
 {
-if (!drmAvailable() && name != NULL && drm_server_info &&
-drm_server_info->load_module) {
+if (name != NULL && drm_server_info &&
+drm_server_info->load_module && !drmAvailable()) {
 /* try to load the kernel module */
 if (!drm_server_info->load_module(name)) {
 drmMsg("[drm] failed to load kernel module \"%s\"\n", name);
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/6 v2] libdrm: Android.mk: Add minimal Android platform check

2019-04-24 Thread John Stultz
Add a check to error out on Android version K(4.4) or
lower.

This is due to dependency added in a previous commit on mmap64,
which was introduced with Android L.

Cc: Emil Velikov 
Cc: Sean Paul 
Cc: Alistair Strachan 
Cc: Marissa Wall 
Suggested-by: Emil Velikov 
Signed-off-by: John Stultz 
---
NOTE: This change was suggested by Emil, and I've implemented
it as suggested, but due to the fact that the Android.mk files
are no longer usable with AOSP/master, I'm not able to test
this change. Help in validating would be appreciated.
---
 Android.mk | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Android.mk b/Android.mk
index 1b77c53..0ab6f0f 100644
--- a/Android.mk
+++ b/Android.mk
@@ -21,6 +21,11 @@
 # IN THE SOFTWARE.
 #
 
+LIBDRM_ANDROID_MAJOR_VERSION := $(word 1, $(subst ., , $(PLATFORM_VERSION)))
+ifneq ($(filter 2 4, $(LIBDRM_ANDROID_MAJOR_VERSION)),)
+$(error "Android 4.4 and earlier not supported")
+endif
+
 LIBDRM_COMMON_MK := $(call my-dir)/Android.common.mk
 
 LOCAL_PATH := $(call my-dir)
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/6 v2] libdrm: Patches from AOSP

2019-04-24 Thread John Stultz
Recently I've been trying to sync the AOSP libdrm tree with
the upstream freedesktop branch.

Thanks to input from Sean, Alistair and Marissa, we've managed
to drop a bunch of stale patches AOSP was carrying, and get
the AOSP libdrm updated to 2.4.97

I've gone through the remaining patch delta and wanted to submit
the non-Android.bp changes, which seemed like they might be
relevant for upstream, for review.

thanks
-john

Cc: Emil Velikov 
Cc: Sean Paul 
Cc: Alistair Strachan 
Cc: Marissa Wall 

Adrian Salido (1):
  libdrm: reduce number of reallocations in drmModeAtomicAddProperty

Hemant Hariyani (1):
  libdrm: omap: Add DRM_RDWR flag to dmabuf export

John Stultz (2):
  libdrm: Android.mk: Add minimal Android platform check
  libdrm: amdgpu: Initialize unions with memset rather than "= {0}"

Prabhanjan Kandula (1):
  libdrm: Avoid additional drm open close

Sean Paul (1):
  libdrm: Use mmap64 instead of __mmap2

 Android.mk | 5 +
 amdgpu/amdgpu_cs.c | 9 ++---
 libdrm_macros.h| 4 +---
 omap/omap_drm.c| 2 +-
 xf86drm.c  | 4 ++--
 xf86drmMode.c  | 7 ---
 6 files changed, 19 insertions(+), 12 deletions(-)

-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/6 v2] libdrm: Use mmap64 instead of __mmap2

2019-04-24 Thread John Stultz
From: Sean Paul 

__mmap2 isn't supported on all platforms, mmap64 is the right way
to do this in android.

Also folds in a fix from Stéphane Marchesin 

Cc: Emil Velikov 
Cc: Sean Paul 
Cc: Alistair Strachan 
Cc: Marissa Wall 
Acked-by: Alex Deucher 
Reviewed-by: Emil Velikov 
Signed-off-by: Sean Paul 
[jstultz: Folded in Stéphane's fix]
Signed-off-by: John Stultz 
---
 libdrm_macros.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libdrm_macros.h b/libdrm_macros.h
index 95f0ef5..0dca827 100644
--- a/libdrm_macros.h
+++ b/libdrm_macros.h
@@ -48,8 +48,6 @@
 #if defined(ANDROID) && !defined(__LP64__)
 #include  /* for EINVAL */
 
-extern void *__mmap2(void *, size_t, int, int, int, size_t);
-
 static inline void *drm_mmap(void *addr, size_t length, int prot, int flags,
  int fd, loff_t offset)
 {
@@ -59,7 +57,7 @@ static inline void *drm_mmap(void *addr, size_t length, int 
prot, int flags,
   return MAP_FAILED;
}
 
-   return __mmap2(addr, length, prot, flags, fd, (size_t) (offset >> 12));
+   return mmap64(addr, length, prot, flags, fd, offset);
 }
 
 #  define drm_munmap(addr, length) \
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6 v2] libdrm: amdgpu: Initialize unions with memset rather than "= {0}"

2019-04-24 Thread John Stultz
Clang complains when initializing unions using "= {0}"
so instead use memset.

Cc: Emil Velikov 
Cc: Sean Paul 
Cc: Alistair Strachan 
Cc: Marissa Wall 
Reviewed-by: Alex Deucher 
Reviewed-by: Emil Velikov 
Reviewed-by: Christian König 
Signed-off-by: John Stultz 
---
 amdgpu/amdgpu_cs.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
index 7ee844f..7c5b9d1 100644
--- a/amdgpu/amdgpu_cs.c
+++ b/amdgpu/amdgpu_cs.c
@@ -733,12 +733,13 @@ drm_public int amdgpu_cs_submit_raw(amdgpu_device_handle 
dev,
struct drm_amdgpu_cs_chunk *chunks,
uint64_t *seq_no)
 {
-   union drm_amdgpu_cs cs = {0};
+   union drm_amdgpu_cs cs;
uint64_t *chunk_array;
int i, r;
if (num_chunks == 0)
return -EINVAL;
 
+   memset(, 0, sizeof(cs));
chunk_array = alloca(sizeof(uint64_t) * num_chunks);
for (i = 0; i < num_chunks; i++)
chunk_array[i] = (uint64_t)(uintptr_t)[i];
@@ -763,10 +764,11 @@ drm_public int amdgpu_cs_submit_raw2(amdgpu_device_handle 
dev,
 struct drm_amdgpu_cs_chunk *chunks,
 uint64_t *seq_no)
 {
-   union drm_amdgpu_cs cs = {0};
+   union drm_amdgpu_cs cs;
uint64_t *chunk_array;
int i, r;
 
+   memset(, 0, sizeof(cs));
chunk_array = alloca(sizeof(uint64_t) * num_chunks);
for (i = 0; i < num_chunks; i++)
chunk_array[i] = (uint64_t)(uintptr_t)[i];
@@ -803,9 +805,10 @@ drm_public int 
amdgpu_cs_fence_to_handle(amdgpu_device_handle dev,
 uint32_t what,
 uint32_t *out_handle)
 {
-   union drm_amdgpu_fence_to_handle fth = {0};
+   union drm_amdgpu_fence_to_handle fth;
int r;
 
+   memset(, 0, sizeof(fth));
fth.in.fence.ctx_id = fence->context->id;
fth.in.fence.ip_type = fence->ip_type;
fth.in.fence.ip_instance = fence->ip_instance;
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/25] drm: kirin: Reanme dc_ops to kirin_drm_data

2019-04-24 Thread Sam Ravnborg
Hi again.

On Wed, Apr 24, 2019 at 06:52:26PM +0200, Sam Ravnborg wrote:
> Hi John.
> 
> On Tue, Apr 23, 2019 at 04:20:43PM -0700, John Stultz wrote:
> > From: Xu YiPing 
> > 
> > As part of refactoring the kirin driver to better support
> > different hardware revisions, this patch renames the
> > struct kirin_dc_ops to struct kirin_drm_data and cleans
> > up the related variable names.
> > 
> > Cc: Xinliang Liu 
> > Cc: Rongrong Zou 
> > Cc: Xinwei Kong 
> > Cc: Chen Feng 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: dri-devel 
> > Signed-off-by: Xu YiPing 
> > [jstultz: reworded commit message]
> > Signed-off-by: John Stultz 
> > ---
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  2 +-
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 16 
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h |  4 ++--
> >  3 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 
> > b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > index 69604ad..221bfbb 100644
> > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > @@ -1055,7 +1055,7 @@ static void ade_drm_cleanup(struct platform_device 
> > *pdev)
> >  {
> >  }
> >  
> > -const struct kirin_dc_ops ade_dc_ops = {
> > +struct kirin_drm_data ade_driver_data = {
> > .init = ade_drm_init,
> > .cleanup = ade_drm_cleanup
> >  };
> This rename does not help readability. An _ops often/always hold
> function pointers. Where _data hold data.
> So it looks wrong to name this _data.
Reading later pathes the rename makes good sense, so disregard this.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/25] drm: kirin: Remove HISI_KIRIN_DW_DSI config option

2019-04-24 Thread Sam Ravnborg
Hi John.

> >
> > Nice simplification. We are now down to two very small Kconfig files.
> > Consider to merge them into one Kconfig file in the top-level dir.
> >
> 
> Part of this cleanup is so that we can add another device option in a
> later commit (though not in this series), so unless folks are
> generally wanting to consolidate Kconfigs into the top level, it may
> be premature to do so now only to later undo it when there are more
> kirin specific options. I'm fine with whichever, I just want to make
> sure that's clear.
I am just passing my reflections forward in a mail as I read the
patches.
Unless there is something that resemble a real bug then always do what
you consider best. In this case keep the two Kconfig files.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Support for 2D engines/blitters in V4L2 and DRM

2019-04-24 Thread Michel Dänzer
On 2019-04-24 5:44 p.m., Nicolas Dufresne wrote:
> Le mercredi 24 avril 2019 à 17:06 +0200, Daniel Vetter a écrit :
>> On Wed, Apr 24, 2019 at 4:41 PM Paul Kocialkowski
>>  wrote:
>>> On Wed, 2019-04-24 at 16:39 +0200, Michel Dänzer wrote:
 On 2019-04-24 2:01 p.m., Nicolas Dufresne wrote:
>
> Rendering a video stream is more complex then what you describe here.
> Whenever there is a unexpected delay (late delivery of a frame as an
> example) you may endup in situation where one frame is ready after the
> targeted vblank. If there is another frame that targets the following
> vblank that gets ready on-time, the previous frame should be replaced
> by the most recent one.
>
> With fences, what happens is that even if you received the next frame
> on time, naively replacing it is not possible, because we don't know
> when the fence for the next frame will be signalled. If you simply
> always replace the current frame, you may endup skipping a lot more
> vblank then what you expect, and that results in jumpy playback.

 So you want to be able to replace a queued flip with another one then.
 That doesn't necessarily require allowing more than one flip to be
 queued ahead of time.
>>>
>>> There might be other ways to do it, but this one has plenty of
>>> advantages.
>>
>> The point of kms (well one of the reasons) was to separate the
>> implementation of modesetting for specific hw from policy decisions
>> like which frames to drop and how to schedule them. Kernel gives
>> tools, userspace implements the actual protocols.
>>
>> There's definitely a bit a gap around scheduling flips for a specific
>> frame or allowing to cancel/overwrite an already scheduled flip, but
>> no one yet has come up with a clear proposal for new uapi + example
>> implementation + userspace implementation + big enough support from
>> other compositors that this is what they want too.

Actually, the ATOMIC_AMEND patches propose a way to replace a scheduled
flip?


 Note that this can also be done in userspace with explicit fencing (by
 only selecting a frame and submitting it to the kernel after all
 corresponding fences have signalled), at least to some degree, but the
 kernel should be able to do it up to a later point in time and more
 reliably, with less risk of missing a flip for a frame which becomes
 ready just in time.
>>>
>>> Indeed, but it would be great if we could do that with implicit fencing
>>> as well.
>>
>> 1. extract implicit fences from dma-buf. This part is just an idea,
>> but easy to implement once we have someone who actually wants this.
>> All we need is a new ioctl on the dma-buf to export the fences from
>> the reservation_object as a sync_file (either the exclusive or the
>> shared ones, selected with a flag).
>> 2. do the exact same frame scheduling as with explicit fencing
>> 3. supply explicit fences in your atomic ioctl calls - these should
>> overrule any implicit fences (assuming correct kernel drivers, but we
>> have helpers so you can assume they all work correctly).
>>
>> By design this is possible, it's just that no one yet bothered enough
>> to make it happen.
>> -Daniel
> 
> I'm not sure I understand the workflow of this one. I'm all in favour
> leaving the hard work to userspace. Note that I have assumed explicit
> fences from the start, I don't think implicit fence will ever exist in
> v4l2, but I might be wrong. What I understood is that there was a
> previous attempt in the past but it raised more issues then it actually
> solved. So that being said, how do handle exactly the follow use cases:
> 
>  - A frame was lost by capture driver, but it was schedule as being the
> next buffer to render (normally previous frame should remain).

Userspace just doesn't call into the kernel to flip to the lost frame,
so the previous one remains.

>  - The scheduled frame is late for the next vblank (didn't signal on-
> time), a new one may be better for the next vlbank, but we will only
> know when it's fence is signaled.

Userspace only selects a frame and submits it to the kernel after all
its fences have signalled.

> Better in this context means the the presentation time of this frame is
> closer to the next vblank time. Keep in mind that the idea is to
> schedule the frames before they are signal, in order to make the usage
> of the fence useful in lowering the latency.

Fences are about signalling completion, not about low latency.

With a display server, the client can send frames to the display server
ahead of time, only the display server needs to wait for fences to
signal before submitting frames to the kernel.


> Of course as Michel said, we could just always wait on the fence and
> just schedule. But if you do that, why would you care implementing the
> fence in v4l2 to start with, DQBuf does just that already.

A fence is more likely to work out of the box with non-V4L-related code
than DQBuf?


-- 

Re: [PATCH 12/25] drm: kirin: Reanme dc_ops to kirin_drm_data

2019-04-24 Thread Sam Ravnborg
Hi John.

On Tue, Apr 23, 2019 at 04:20:43PM -0700, John Stultz wrote:
> From: Xu YiPing 
> 
> As part of refactoring the kirin driver to better support
> different hardware revisions, this patch renames the
> struct kirin_dc_ops to struct kirin_drm_data and cleans
> up the related variable names.
> 
> Cc: Xinliang Liu 
> Cc: Rongrong Zou 
> Cc: Xinwei Kong 
> Cc: Chen Feng 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel 
> Signed-off-by: Xu YiPing 
> [jstultz: reworded commit message]
> Signed-off-by: John Stultz 
> ---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  2 +-
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 16 
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h |  4 ++--
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 
> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index 69604ad..221bfbb 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -1055,7 +1055,7 @@ static void ade_drm_cleanup(struct platform_device 
> *pdev)
>  {
>  }
>  
> -const struct kirin_dc_ops ade_dc_ops = {
> +struct kirin_drm_data ade_driver_data = {
>   .init = ade_drm_init,
>   .cleanup = ade_drm_cleanup
>  };
This rename does not help readability. An _ops often/always hold
function pointers. Where _data hold data.
So it looks wrong to name this _data.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/25] drm: kirin: Move workqueue to ade_hw_ctx structure

2019-04-24 Thread John Stultz
On Wed, Apr 24, 2019 at 9:46 AM Sam Ravnborg  wrote:
>
> Hi John.
>
> On Tue, Apr 23, 2019 at 04:20:41PM -0700, John Stultz wrote:
> > The workqueue used to reset the display when we hit an LDI
> > underflow error is ADE specific, so since this patch series
> > works to make the kirin_crtc structure more generic, move the
> > workqueue to the ade_hw_ctx structure instead.
> >
> > Cc: Xinliang Liu 
> > Cc: Rongrong Zou 
> > Cc: Xinwei Kong 
> > Cc: Chen Feng 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: dri-devel 
> > Signed-off-by: John Stultz 
> > ---
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 12 +---
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 
> > b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > index 94dcad0..f54cf99 100644
> > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> > @@ -52,6 +52,7 @@ struct ade_hw_ctx {
> >   struct clk *media_noc_clk;
> >   struct clk *ade_pix_clk;
> >   struct reset_control *reset;
> > + struct work_struct drm_device_wq;
>
> The comment probarly belongs to 01/25...
> The name drm_device_wq is not at all descriptive.
> Consider something like: display_reset_wq

I agree, that sounds like a nice improvement! I'll tweak it in patch 1/25.

thanks for the review and feedback!
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/25] drm: kirin: Move kirin_crtc, kirin_plane, kirin_format to kirin_drm_drv.h

2019-04-24 Thread Sam Ravnborg
Hi John.

On Tue, Apr 23, 2019 at 04:20:42PM -0700, John Stultz wrote:
> From: Xu YiPing 
> 
> As part of refactoring the kirin driver to better support
> different hardware revisions, this patch moves some shared
> structures and helpers to the common kirin_drm_drv.h
> 
> These structures will later used by both kirin620 and
> future kirin960 driver
> 
> Cc: Xinliang Liu 
> Cc: Rongrong Zou 
> Cc: Xinwei Kong 
> Cc: Chen Feng 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel 
> Signed-off-by: Xu YiPing 
> [jstultz: reworded commit message]
> Signed-off-by: John Stultz 
> ---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 27 
> ++---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 24 ++
>  2 files changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 
> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index f54cf99..69604ad 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -38,12 +38,6 @@
>  #define OUT_OVLY ADE_OVLY2 /* output overlay compositor */
>  #define ADE_DEBUG1
>  
> -#define to_kirin_crtc(crtc) \
> - container_of(crtc, struct kirin_crtc, base)
> -
> -#define to_kirin_plane(plane) \
> - container_of(plane, struct kirin_plane, base)
> -
>  
>  struct ade_hw_ctx {
>   void __iomem  *base;
> @@ -59,18 +53,6 @@ struct ade_hw_ctx {
>   struct drm_crtc *crtc;
>  };
>  
> -struct kirin_crtc {
> - struct drm_crtc base;
> - void *hw_ctx;
> - bool enable;
> -};
> -
> -struct kirin_plane {
> - struct drm_plane base;
> - void *hw_ctx;
> - u32 ch;
> -};
> -
>  struct ade_data {
>   struct kirin_crtc crtc;
>   struct kirin_plane planes[ADE_CH_NUM];
> @@ -78,12 +60,7 @@ struct ade_data {
>  };


This struct: 
>  /* ade-format info: */
> -struct ade_format {
> - u32 pixel_format;
> - enum ade_fb_format ade_format;
> -};
> -
> -static const struct ade_format ade_formats[] = {
> +static const struct kirin_format ade_formats[] = {
>   /* 16bpp RGB: */
>   { DRM_FORMAT_RGB565, ADE_RGB_565 },
>   { DRM_FORMAT_BGR565, ADE_BGR_565 },
> @@ -127,7 +104,7 @@ static u32 ade_get_format(u32 pixel_format)
>  
>   for (i = 0; i < ARRAY_SIZE(ade_formats); i++)
>   if (ade_formats[i].pixel_format == pixel_format)
> - return ade_formats[i].ade_format;
> + return ade_formats[i].hw_format;
>  
>   /* not found */
>   DRM_ERROR("Not found pixel format!!fourcc_format= %d\n",
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h 
> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
> index ad027d1..b6626f5 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
> @@ -13,6 +13,30 @@
>  
>  #define MAX_CRTC 2
>  
> +#define to_kirin_crtc(crtc) \
> + container_of(crtc, struct kirin_crtc, base)
> +
> +#define to_kirin_plane(plane) \
> + container_of(plane, struct kirin_plane, base)
> +
> +/* kirin-format translate table */
> +struct kirin_format {
> + u32 pixel_format;
> + u32 hw_format;
> +};
Is renamed.
The member hw_format is renamed and no longer uses an enum.
(The sole user of this enum type).

These changes are not included in the changelog - should they be part of
this patch?
And also the change from enum to u32 is not understood.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/25] drm: kirin: Remove HISI_KIRIN_DW_DSI config option

2019-04-24 Thread John Stultz
On Wed, Apr 24, 2019 at 9:39 AM Sam Ravnborg  wrote:
>
> Hi John.
>
> On Tue, Apr 23, 2019 at 04:20:33PM -0700, John Stultz wrote:
> > The CONFIG_HISI_KIRIN_DW_DSI option is only used w/ kirin
> > driver, so cut out the middleman and condense the config
> > logic down.
> >
> > Cc: Xinliang Liu 
> > Cc: Rongrong Zou 
> > Cc: Xinwei Kong 
> > Cc: Chen Feng 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: dri-devel 
> > Signed-off-by: John Stultz 
> > ---
> >  drivers/gpu/drm/hisilicon/kirin/Kconfig  | 10 +-
> >  drivers/gpu/drm/hisilicon/kirin/Makefile |  4 ++--
> >  2 files changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/hisilicon/kirin/Kconfig 
> > b/drivers/gpu/drm/hisilicon/kirin/Kconfig
> > index 499f644..6ef7906 100644
> > --- a/drivers/gpu/drm/hisilicon/kirin/Kconfig
> > +++ b/drivers/gpu/drm/hisilicon/kirin/Kconfig
> > @@ -4,16 +4,8 @@ config DRM_HISI_KIRIN
> >   select DRM_KMS_HELPER
> >   select DRM_GEM_CMA_HELPER
> >   select DRM_KMS_CMA_HELPER
> > - select HISI_KIRIN_DW_DSI
> > + select DRM_MIPI_DSI
> >   help
> > Choose this option if you have a hisilicon Kirin chipsets(hi6220).
> > If M is selected the module will be called kirin-drm.
> >
> > -config HISI_KIRIN_DW_DSI
> > - tristate "HiSilicon Kirin specific extensions for Synopsys DW MIPI 
> > DSI"
> > - depends on DRM_HISI_KIRIN
> > - select DRM_MIPI_DSI
> > - help
> > -  This selects support for HiSilicon Kirin SoC specific extensions for
> > -  the Synopsys DesignWare DSI driver. If you want to enable MIPI DSI on
> > -  hi6220 based SoC, you should selet this option.
>
> Nice simplification. We are now down to two very small Kconfig files.
> Consider to merge them into one Kconfig file in the top-level dir.
>

Part of this cleanup is so that we can add another device option in a
later commit (though not in this series), so unless folks are
generally wanting to consolidate Kconfigs into the top level, it may
be premature to do so now only to later undo it when there are more
kirin specific options. I'm fine with whichever, I just want to make
sure that's clear.

thanks
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/25] drm: kirin: Move workqueue to ade_hw_ctx structure

2019-04-24 Thread Sam Ravnborg
Hi John.

On Tue, Apr 23, 2019 at 04:20:41PM -0700, John Stultz wrote:
> The workqueue used to reset the display when we hit an LDI
> underflow error is ADE specific, so since this patch series
> works to make the kirin_crtc structure more generic, move the
> workqueue to the ade_hw_ctx structure instead.
> 
> Cc: Xinliang Liu 
> Cc: Rongrong Zou 
> Cc: Xinwei Kong 
> Cc: Chen Feng 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel 
> Signed-off-by: John Stultz 
> ---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 
> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index 94dcad0..f54cf99 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -52,6 +52,7 @@ struct ade_hw_ctx {
>   struct clk *media_noc_clk;
>   struct clk *ade_pix_clk;
>   struct reset_control *reset;
> + struct work_struct drm_device_wq;

The comment probarly belongs to 01/25...
The name drm_device_wq is not at all descriptive.
Consider something like: display_reset_wq

Then when the workqueue is started it is much more obvious what it does.

>   bool power_on;
>   int irq;
>  
> @@ -61,7 +62,6 @@ struct ade_hw_ctx {
>  struct kirin_crtc {
>   struct drm_crtc base;
>   void *hw_ctx;
> - struct work_struct drm_device_wq;
>   bool enable;
>  };
>  
> @@ -349,9 +349,9 @@ static void ade_crtc_disable_vblank(struct drm_crtc *crtc)
>  
>  static void drm_underflow_wq(struct work_struct *work)
>  {
> - struct kirin_crtc *acrtc = container_of(work, struct kirin_crtc,
> + struct ade_hw_ctx *ctx = container_of(work, struct ade_hw_ctx,
> drm_device_wq);
> - struct drm_device *drm_dev = (>base)->dev;
> + struct drm_device *drm_dev = ctx->crtc->dev;
>   struct drm_atomic_state *state;
>  
>   state = drm_atomic_helper_suspend(drm_dev);
> @@ -362,7 +362,6 @@ static irqreturn_t ade_irq_handler(int irq, void *data)
>  {
>   struct ade_hw_ctx *ctx = data;
>   struct drm_crtc *crtc = ctx->crtc;
> - struct kirin_crtc *kcrtc = to_kirin_crtc(crtc);
>   void __iomem *base = ctx->base;
>   u32 status;
>  
> @@ -379,7 +378,7 @@ static irqreturn_t ade_irq_handler(int irq, void *data)
>   ade_update_bits(base + LDI_INT_CLR, UNDERFLOW_INT_EN_OFST,
>   MASK(1), 1);
>   DRM_ERROR("LDI underflow!");
> - schedule_work(>drm_device_wq);

Compare:
> + schedule_work(>drm_device_wq);

With
> + schedule_work(>display_reset_wq);

The latter wins in readability.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/25] drm: kirin: Remove HISI_KIRIN_DW_DSI config option

2019-04-24 Thread Sam Ravnborg
Hi John.

On Tue, Apr 23, 2019 at 04:20:33PM -0700, John Stultz wrote:
> The CONFIG_HISI_KIRIN_DW_DSI option is only used w/ kirin
> driver, so cut out the middleman and condense the config
> logic down.
> 
> Cc: Xinliang Liu 
> Cc: Rongrong Zou 
> Cc: Xinwei Kong 
> Cc: Chen Feng 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel 
> Signed-off-by: John Stultz 
> ---
>  drivers/gpu/drm/hisilicon/kirin/Kconfig  | 10 +-
>  drivers/gpu/drm/hisilicon/kirin/Makefile |  4 ++--
>  2 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/kirin/Kconfig 
> b/drivers/gpu/drm/hisilicon/kirin/Kconfig
> index 499f644..6ef7906 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/Kconfig
> +++ b/drivers/gpu/drm/hisilicon/kirin/Kconfig
> @@ -4,16 +4,8 @@ config DRM_HISI_KIRIN
>   select DRM_KMS_HELPER
>   select DRM_GEM_CMA_HELPER
>   select DRM_KMS_CMA_HELPER
> - select HISI_KIRIN_DW_DSI
> + select DRM_MIPI_DSI
>   help
> Choose this option if you have a hisilicon Kirin chipsets(hi6220).
> If M is selected the module will be called kirin-drm.
>  
> -config HISI_KIRIN_DW_DSI
> - tristate "HiSilicon Kirin specific extensions for Synopsys DW MIPI DSI"
> - depends on DRM_HISI_KIRIN
> - select DRM_MIPI_DSI
> - help
> -  This selects support for HiSilicon Kirin SoC specific extensions for
> -  the Synopsys DesignWare DSI driver. If you want to enable MIPI DSI on
> -  hi6220 based SoC, you should selet this option.

Nice simplification. We are now down to two very small Kconfig files.
Consider to merge them into one Kconfig file in the top-level dir.

Sam


> diff --git a/drivers/gpu/drm/hisilicon/kirin/Makefile 
> b/drivers/gpu/drm/hisilicon/kirin/Makefile
> index cdf6158..3585327 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/Makefile
> +++ b/drivers/gpu/drm/hisilicon/kirin/Makefile
> @@ -1,6 +1,6 @@
>  kirin-drm-y := kirin_drm_drv.o \
> -kirin_drm_ade.o
> +kirin_drm_ade.o \
> +dw_drm_dsi.o
>  
>  obj-$(CONFIG_DRM_HISI_KIRIN) += kirin-drm.o
>  
> -obj-$(CONFIG_HISI_KIRIN_DW_DSI) += dw_drm_dsi.o
> -- 
> 2.7.4
> 
> ___
> 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

Re: [PATCH 01/25] drm: kirin: Fix for hikey620 display offset problem

2019-04-24 Thread Sam Ravnborg
Hi John.

On Tue, Apr 23, 2019 at 04:20:32PM -0700, John Stultz wrote:
> From: Da Lv 
> 
> The original HiKey (620) board has had a long running issue
> where when using a 1080p montior, the display would occasionally
> blink and come come back with a horizontal offset (usually also
> shifting the colors, depending on the value of the offset%4).
> 
> After lots of analysis by HiSi developers, they found the issue
> was due to when running at 1080p, it was possible to hit the
> device memory bandwidth limits, which could cause the DSI signal
> to get out of sync.
> 
> Unfortunately the DSI logic doesn't have the ability to
> automatically recover from this situation, but we can get a an
> LDI underflow interrupt when it happens.
> 
> To then correct the issue, when we get an LDI underflow irq, we
> we can simply suspend and resume the display, which resets the
> hardware.
> 
> Thus, this patch enables the ldi underflow interrupt, and
> initializes a workqueue that is used to suspend/resume the
> display to recover. Then when the irq occurs we clear it and
> schedule the workqueue to reset display engine.
> 
> Cc: Xinliang Liu 
> Cc: Rongrong Zou 
> Cc: Xinwei Kong 
> Cc: Chen Feng 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel 
> Signed-off-by: Da Lv 
> Signed-off-by: Yidong Lin 
> [jstultz: Reworded the commit message, checkpatch cleanups]
> Signed-off-by: John Stultz 
> ---
> v2: Minor cleanups
> ---
>  drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h |  6 ++
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 22 ++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h 
> b/drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h
> index 4cf281b7..ced40c6 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_ade_reg.h
> @@ -87,6 +87,7 @@
>  #define VSIZE_OFST   20
>  #define LDI_INT_EN   0x741C
>  #define FRAME_END_INT_EN_OFST1
> +#define UNDERFLOW_INT_EN_OFST2
>  #define LDI_CTRL 0x7420
>  #define BPP_OFST 3
>  #define DATA_GATE_EN BIT(2)
> @@ -97,6 +98,11 @@
>  #define LDI_HDMI_DSI_GT  0x7434
>  
>  /*
> + *BIT_LDI_UNFLOW
> + */
> +#define BIT_LDI_UNFLOW BIT(2)

The definition of this bit looks not like anything surrounding it.
And it is not obvious that this bit is part of LDI_MSK_INT.
Consider to reformat this so it is obvious where this bit belongs
when reading the .h file.


> +
> +/*
>   * ADE media bus service regs
>   */
>  #define ADE0_QOSGENERATOR_MODE   0x010C
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 
> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index 73611a9..beb2a3c 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -58,6 +58,7 @@ struct ade_hw_ctx {
>  struct ade_crtc {
>   struct drm_crtc base;
>   struct ade_hw_ctx *ctx;
> + struct work_struct drm_device_wq;
>   bool enable;
>   u32 out_format;
>  };
> @@ -176,6 +177,7 @@ static void ade_init(struct ade_hw_ctx *ctx)
>*/
>   ade_update_bits(base + ADE_CTRL, FRM_END_START_OFST,
>   FRM_END_START_MASK, REG_EFFECTIVE_IN_ADEEN_FRMEND);
> + ade_update_bits(base + LDI_INT_EN, UNDERFLOW_INT_EN_OFST, MASK(1), 1);
>  }
>  
>  static bool ade_crtc_mode_fixup(struct drm_crtc *crtc,
> @@ -345,6 +347,17 @@ static void ade_crtc_disable_vblank(struct drm_crtc 
> *crtc)
>   MASK(1), 0);
>  }
>  
> +static void drm_underflow_wq(struct work_struct *work)
> +{
> + struct ade_crtc *acrtc = container_of(work, struct ade_crtc,
> +   drm_device_wq);
> + struct drm_device *drm_dev = (>base)->dev;
> + struct drm_atomic_state *state;
> +
> + state = drm_atomic_helper_suspend(drm_dev);
> + drm_atomic_helper_resume(drm_dev, state);
> +}
> +
>  static irqreturn_t ade_irq_handler(int irq, void *data)
>  {
>   struct ade_crtc *acrtc = data;
> @@ -362,6 +375,12 @@ static irqreturn_t ade_irq_handler(int irq, void *data)
>   MASK(1), 1);
>   drm_crtc_handle_vblank(crtc);
>   }
> + if (status & BIT_LDI_UNFLOW) {
> + ade_update_bits(base + LDI_INT_CLR, UNDERFLOW_INT_EN_OFST,
> + MASK(1), 1);
A general comment here.
It is not obvious from reading this code that the code will clear
LDI_UNFLOW bit in the LDI_INT_CLR register.

I think this driver could see readability improvements
if converted to use regmaps.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109526] [CARRIZO] amdgpu fails to resume from S3, atombios stuck executing C554 (len 629, WS 0, PS 0)

2019-04-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109526

--- Comment #5 from Johannes Hirte  ---
Created attachment 144083
  --> https://bugs.freedesktop.org/attachment.cgi?id=144083=edit
dmesg log from suspend->resume->suspend->resume

When this bug occurs, from sddm it is possible to put the system into suspend
again via hotkey. In most cases the second resume works. Attaching a dmesg
output from such a suspend->resume->suspend->resume cycle. Maybe this helps
debugging this.

-- 
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 v6 2/3] dt-bindings: backlight: add lm3630a bindings

2019-04-24 Thread Rob Herring
On Wed, Apr 24, 2019 at 9:20 AM Pavel Machek  wrote:
>
> Hi!
>
> > --- /dev/null
> > +++ 
> > b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> > @@ -0,0 +1,129 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/lm3630a-backlight.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>
> I'm not a great fan on links to external websites. But this is comment
> for Rob.

This is simply how json-schema works. $schema says what meta-schema
this file should validate against so it's versioned if we change the
schema format. $id is just a unique identifier and is used to resolve
$ref values. Note that these aren't live urls either (but could be at
some point in the future).

> > +title: TI LM3630A High-Efficiency Dual-String White LED
> > +
> > +maintainers:
> > +  - Lee Jones 
> > +  - Daniel Thompson 
> > +  - Jingoo Han 
>
> Not a great fan of duplicating MAINTAINERS information here. But this
> is a comment for Rob, again.

Hopefully this is temporary until we move to per directory MAINTAINERS
files. Not sure what happened with that.

Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/2] drm/stm: ltdc: manage error cases in probe

2019-04-24 Thread Fabien Dessenne
This patchset adds some check of the returned error code in probe.

Fabien Dessenne (2):
  drm/stm: ltdc: manage the get_irq probe defer case
  drm/stm: ltdc: return appropriate error code during probe

 drivers/gpu/drm/stm/ltdc.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vc4: Fix compilation error reported by kbuild test bot

2019-04-24 Thread Maarten Lankhorst
Op 24-04-2019 om 17:06 schreef Maarten Lankhorst:
> Op 24-04-2019 om 15:12 schreef kbuild test robot:
>> tree:   git://anongit.freedesktop.org/drm/drm-misc for-linux-next-fixes
>> head:   d08106796a78a4273e39e1bbdf538dc4334b2635
>> commit: d08106796a78a4273e39e1bbdf538dc4334b2635 [1/1] drm/vc4: Fix memory 
>> leak during gpu reset.
>> reproduce:
>> # apt-get install sparse
>> git checkout d08106796a78a4273e39e1bbdf538dc4334b2635
>> make ARCH=x86_64 allmodconfig
>> make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
>>
>> If you fix the issue, kindly add following tag
>> Reported-by: kbuild test robot 
>>
>> sparse warnings: (new ones prefixed by >>)
>>
 drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse: sparse: incorrect type in 
 argument 1 (different base types) @@expected struct drm_crtc *crtc @@  
   got structstruct drm_crtc *crtc @@
 drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse:expected struct 
 drm_crtc *crtc
 drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse:got struct 
 drm_crtc_state *state
 drivers/gpu/drm/vc4/vc4_crtc.c:1045:39: sparse: sparse: not enough 
 arguments for function vc4_crtc_destroy_state
>> vim +1045 drivers/gpu/drm/vc4/vc4_crtc.c
>>
>>   1040   
>>   1041   static void
>>   1042   vc4_crtc_reset(struct drm_crtc *crtc)
>>   1043   {
>>   1044   if (crtc->state)
>>> 1045vc4_crtc_destroy_state(crtc->state);
>>   1046   
>>   1047   crtc->state = kzalloc(sizeof(struct vc4_crtc_state), 
>> GFP_KERNEL);
>>   1048   if (crtc->state)
>>   1049   crtc->state->crtc = crtc;
>>   1050   }
>>   1051   
>>
>> ---
>> 0-DAY kernel test infrastructureOpen Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> -8<
> A pointer to crtc was missing, resulting in the following build error:
> drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse: sparse: incorrect type in 
> argument 1 (different base types)
> drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse:expected struct drm_crtc 
> *crtc
> drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse:got struct drm_crtc_state 
> *state
> drivers/gpu/drm/vc4/vc4_crtc.c:1045:39: sparse: sparse: not enough arguments 
> for function vc4_crtc_destroy_state
>
> Signed-off-by: Maarten Lankhorst 
> Reported-by: kbuild test robot 
> Cc: Eric Anholt 
> ---
>  drivers/gpu/drm/vc4/vc4_crtc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index e7c04a9eb219..1baa10e94484 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -1042,7 +1042,7 @@ static void
>  vc4_crtc_reset(struct drm_crtc *crtc)
>  {
>   if (crtc->state)
> - vc4_crtc_destroy_state(crtc->state);
> + vc4_crtc_destroy_state(crtc, crtc->state);
>  
>   crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>   if (crtc->state)

Pushed with Daniel's irc ack. :)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] gpu/docs: Clarify what userspace means for gl

2019-04-24 Thread Chunming Zhou

在 2019/4/24 22:36, Daniel Vetter 写道:
> On Wed, Apr 24, 2019 at 4:28 PM Daniel Vetter  wrote:
>> On Wed, Apr 24, 2019 at 4:24 PM Zhou, David(ChunMing)
>>  wrote:
>>> Will linux be only mesa-linux? I thought linux is an  open linux.
>>> Which will impact our opengl/amdvlk(MIT open source), not sure Rocm:
>>> 1. how to deal with one uapi that opengl/amdvlk needs but mesa dont need? 
>>> reject?
>>> 2. one hw feature that opengl/amdvlk developers work on that but no mesa 
>>> developers work on, cannot upstream as well?
>>>
>>> I think above two would easily happen, because there are many employees 
>>> working on company project with many customer kinds of reqiurements, but 
>>> mesa not.
>> Well those are the questions I tried to have a clear answer for, to
>> avoid confusion, but looks like the agreement isn't perfect. But
>> there's pretty clear favour towards cross vendor userspace that's
>> collaboratively and openly developed like mesa. So whenever one of the
>> above cases you bring up happen we'll get to have a big thread on
>> dri-devel I guess.
> Aside, I think there's a pretty huge difference between rocm and
> amdvlk. rocm is developed openly and all that, the one nit there is is
> that it's not cross vendor. But then, there is no other vendor really
> (but amd still controls it all despite the usual "totally an open
> standard" lipstick in the marketing material). amdvlk otoh is open
> source in the narrowest legal sense only, and nothing else.

Do you mean if amdvlk is developed openly like Rocm does, amdvlk will be 
able to drive uapi without mesa admissable?


-David

>   And yes
> intel has some equally bad projects on the openess metric too with
> some of the more recent-ish changes in the userspace stack.
> -Daniel
>
>> -Daniel
>>
>>
>>> -David
>>>
>>>  Original Message 
>>> Subject: [PATCH] gpu/docs: Clarify what userspace means for gl
>>> From: Daniel Vetter
>>> To: DRI Development ,Mesa Dev
>>> CC: Jérôme Glisse ,Daniel Vetter ,Karol Herbst ,Kenneth Graunke ,Ben Skeggs 
>>> ,Daniel Vetter ,Sean Paul
>>>
>>> Clear rules avoid arguing.
>>>
>>> Note that this just aims to document current expectations. If that
>>> shifts (e.g. because gl isn't the main api anymore, replaced by vk),
>>> then we need to update this text.
>>>
>>> I think it'd be good to have an equally solid list on the kms side.
>>> But kms is much more meant to be a standard, and the list of userspace
>>> projects we've accepted in the past is constantly shifting and
>>> adjusting. So I figured I'll leave that as an exercise for later on.
>>>
>>> v2: Try to clarify that we don't want a mesa driver just for mesa's
>>> sake, and more clearly exclude anything that just doesn't make sense
>>> technically.  Example would be a compute driver that makes sense to be
>>> merged into drm (for kernel side code-sharing), but where the intended
>>> use is some single-source CUDA-style compute without ever bothering
>>> about any of the 3D/rendering side baggage that comes with gl/vk.
>>>
>>> v3: Drop vulkan for now, the situation there isn't as obviously
>>> clear-cut as on the gl side, and I don't want to tank this idea on a
>>> hot discussion about vk and mesa. Plus I think once we have 1-2 more
>>> vk drivers in mesa the situation on the vk side is clear-cut too, and
>>> we can do a follow-up patch to add vk to the list where we expect the
>>> userspace to be in upstream mesa. That's would give nice precedence to
>>> make it clear that this isn't cast in stone, but meant to reflect
>>> reality and should be adjusted as needed.
>>>
>>> v4: Fix typo.
>>>
>>> v5: Add a note to the commit message that this text needs to be
>>> updated when the situation changes.
>>>
>>> v6: Add a sentence why mesa will give the most meaningful review on gl
>>> stuff - it's a very active project with lots of developers.
>>>
>>> Acked-by: Dave Airlie  (v4)
>>> Acked-by: Eric Anholt  (v4)
>>> Acked-by: Alex Deucher  (v5)
>>> Acked-by: Sean Paul  (v5)
>>> Acked-by: Kenneth Graunke  (v5)
>>> Acked-by: Karol Herbst  (v5)
>>> Acked-by: Rob Clark 
>>> Acked-by: Jérôme Glisse 
>>> Acked-by: Bas Nieuwenhuizen 
>>> Acked-by: Ben Skeggs 
>>> Cc: Dave Airlie 
>>> Cc: Eric Anholt 
>>> Cc: Alex Deucher 
>>> Cc: Sean Paul 
>>> Cc: Kenneth Graunke 
>>> Cc: Karol Herbst 
>>> Cc: Rob Clark 
>>> Cc: Jérôme Glisse 
>>> Cc: Bas Nieuwenhuizen 
>>> Cc: Ben Skeggs 
>>> Signed-off-by: Daniel Vetter 
>>> ---
>>> I chatted with a pile of people in private, and there's clearly some
>>> solid support for this. But there's also some big concerns brought up
>>> by other people. The main one summed up is "what if everyone just
>>> ships vk, with a generic gl-on-vk like ANGLE?", but there's other
>>> concerns too.
>>>
>>> So all together I think this doesn't clear the bar of (almost)
>>> unanimous support which we need to make documentation actually help
>>> with clarifying what's expected. And if/when someone comes up with a
>>> more creative userspace approach for 

Re: Support for 2D engines/blitters in V4L2 and DRM

2019-04-24 Thread Daniel Vetter
On Wed, Apr 24, 2019 at 4:41 PM Paul Kocialkowski
 wrote:
>
> Hi,
>
> On Wed, 2019-04-24 at 16:39 +0200, Michel Dänzer wrote:
> > On 2019-04-24 2:01 p.m., Nicolas Dufresne wrote:
> > > Le mercredi 24 avril 2019 à 10:31 +0200, Michel Dänzer a écrit :
> > > > On 2019-04-19 10:38 a.m., Paul Kocialkowski wrote:
> > > > > On Thu, 2019-04-18 at 20:30 -0400, Nicolas Dufresne wrote:
> > > > > > Le jeudi 18 avril 2019 à 10:18 +0200, Daniel Vetter a écrit :
> > > > > > In the first, we'd need a mechanism where we can schedule a render 
> > > > > > at a
> > > > > > specific time or vblank. We can of course already implement this in
> > > > > > software, but with fences, the scheduling would need to be done in 
> > > > > > the
> > > > > > driver. Then if the fence is signalled earlier, the driver should 
> > > > > > hold
> > > > > > on until the delay is met. If the fence got signalled late, we also
> > > > > > need to think of a workflow. As we can't schedule more then one 
> > > > > > render
> > > > > > in DRM at one time, I don't really see yet how to make that work.
> > > > >
> > > > > Indeed, that's also one of the main issues I've spotted. Before using
> > > > > an implicit fence, we basically have to make sure the frame is due for
> > > > > display at the next vblank. Otherwise, we need to refrain from using
> > > > > the fence and schedule the flip later, which is kind of counter-
> > > > > productive.
> > > >
> > > > Fences are about signalling that the contents of a frame are "done" and
> > > > ready to be presented. They're not about specifying which frame is to be
> > > > presented when.
> > > >
> > > >
> > > > > I feel like specifying a target vblank would be a good unit for that,
> > > >
> > > > The mechanism described above works for that.
> > > >
> > > > > since it's our native granularity after all (while a timestamp is 
> > > > > not).
> > > >
> > > > Note that variable refresh rate (Adaptive Sync / FreeSync / G-Sync)
> > > > changes things in this regard. It makes the vblank length variable, and
> > > > if you wait for multiple vblanks between flips, you get the maximum
> > > > vblank length corresponding to the minimum refresh rate / timing
> > > > granularity. Thus, it would be useful to allow userspace to specify a
> > > > timestamp corresponding to the earliest time when the flip is to
> > > > complete. The kernel could then try to hit that as closely as possible.
> > >
> > > Rendering a video stream is more complex then what you describe here.
> > > Whenever there is a unexpected delay (late delivery of a frame as an
> > > example) you may endup in situation where one frame is ready after the
> > > targeted vblank. If there is another frame that targets the following
> > > vblank that gets ready on-time, the previous frame should be replaced
> > > by the most recent one.
> > >
> > > With fences, what happens is that even if you received the next frame
> > > on time, naively replacing it is not possible, because we don't know
> > > when the fence for the next frame will be signalled. If you simply
> > > always replace the current frame, you may endup skipping a lot more
> > > vblank then what you expect, and that results in jumpy playback.
> >
> > So you want to be able to replace a queued flip with another one then.
> > That doesn't necessarily require allowing more than one flip to be
> > queued ahead of time.
>
> There might be other ways to do it, but this one has plenty of
> advantages.

The point of kms (well one of the reasons) was to separate the
implementation of modesetting for specific hw from policy decisions
like which frames to drop and how to schedule them. Kernel gives
tools, userspace implements the actual protocols.

There's definitely a bit a gap around scheduling flips for a specific
frame or allowing to cancel/overwrite an already scheduled flip, but
no one yet has come up with a clear proposal for new uapi + example
implementation + userspace implementation + big enough support from
other compositors that this is what they want too.

And yes writing a really good compositor is really hard, and I think a
lot of people underestimate that and just create something useful for
their niche. If userspace can't come up with a shared library of
helpers, I don't think baking it in as kernel uapi with 10+ years
regression free api guarantees is going to make it any better.

> > Note that this can also be done in userspace with explicit fencing (by
> > only selecting a frame and submitting it to the kernel after all
> > corresponding fences have signalled), at least to some degree, but the
> > kernel should be able to do it up to a later point in time and more
> > reliably, with less risk of missing a flip for a frame which becomes
> > ready just in time.
>
> Indeed, but it would be great if we could do that with implicit fencing
> as well.

1. extract implicit fences from dma-buf. This part is just an idea,
but easy to implement once we have someone who actually wants this.
All we 

[PATCH] drm/vc4: Fix compilation error reported by kbuild test bot

2019-04-24 Thread Maarten Lankhorst
Op 24-04-2019 om 15:12 schreef kbuild test robot:
> tree:   git://anongit.freedesktop.org/drm/drm-misc for-linux-next-fixes
> head:   d08106796a78a4273e39e1bbdf538dc4334b2635
> commit: d08106796a78a4273e39e1bbdf538dc4334b2635 [1/1] drm/vc4: Fix memory 
> leak during gpu reset.
> reproduce:
> # apt-get install sparse
> git checkout d08106796a78a4273e39e1bbdf538dc4334b2635
> make ARCH=x86_64 allmodconfig
> make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot 
>
> sparse warnings: (new ones prefixed by >>)
>
>>> drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse: sparse: incorrect type in 
>>> argument 1 (different base types) @@expected struct drm_crtc *crtc @@   
>>>  got structstruct drm_crtc *crtc @@
>>> drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse:expected struct drm_crtc 
>>> *crtc
>>> drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse:got struct 
>>> drm_crtc_state *state
>>> drivers/gpu/drm/vc4/vc4_crtc.c:1045:39: sparse: sparse: not enough 
>>> arguments for function vc4_crtc_destroy_state
> vim +1045 drivers/gpu/drm/vc4/vc4_crtc.c
>
>   1040
>   1041static void
>   1042vc4_crtc_reset(struct drm_crtc *crtc)
>   1043{
>   1044if (crtc->state)
>> 1045 vc4_crtc_destroy_state(crtc->state);
>   1046
>   1047crtc->state = kzalloc(sizeof(struct vc4_crtc_state), 
> GFP_KERNEL);
>   1048if (crtc->state)
>   1049crtc->state->crtc = crtc;
>   1050}
>   1051
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

-8<
A pointer to crtc was missing, resulting in the following build error:
drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse: sparse: incorrect type in 
argument 1 (different base types)
drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse:expected struct drm_crtc 
*crtc
drivers/gpu/drm/vc4/vc4_crtc.c:1045:44: sparse:got struct drm_crtc_state 
*state
drivers/gpu/drm/vc4/vc4_crtc.c:1045:39: sparse: sparse: not enough arguments 
for function vc4_crtc_destroy_state

Signed-off-by: Maarten Lankhorst 
Reported-by: kbuild test robot 
Cc: Eric Anholt 
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index e7c04a9eb219..1baa10e94484 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1042,7 +1042,7 @@ static void
 vc4_crtc_reset(struct drm_crtc *crtc)
 {
if (crtc->state)
-   vc4_crtc_destroy_state(crtc->state);
+   vc4_crtc_destroy_state(crtc, crtc->state);
 
crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
if (crtc->state)
-- 
2.20.1


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/stm: ltdc: manage the get_irq probe defer case

2019-04-24 Thread Fabien Dessenne
Manage the -EPROBE_DEFER error case for the ltdc IRQ.

Signed-off-by: Fabien Dessenne 
---
 drivers/gpu/drm/stm/ltdc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 566b0d8..521ba83 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -1174,6 +1174,9 @@ int ltdc_load(struct drm_device *ddev)
 
for (i = 0; i < MAX_IRQ; i++) {
irq = platform_get_irq(pdev, i);
+   if (irq == -EPROBE_DEFER)
+   goto err;
+
if (irq < 0)
continue;
 
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Support for 2D engines/blitters in V4L2 and DRM

2019-04-24 Thread Paul Kocialkowski
Hi,

On Wed, 2019-04-24 at 16:39 +0200, Michel Dänzer wrote:
> On 2019-04-24 2:01 p.m., Nicolas Dufresne wrote:
> > Le mercredi 24 avril 2019 à 10:31 +0200, Michel Dänzer a écrit :
> > > On 2019-04-19 10:38 a.m., Paul Kocialkowski wrote:
> > > > On Thu, 2019-04-18 at 20:30 -0400, Nicolas Dufresne wrote:
> > > > > Le jeudi 18 avril 2019 à 10:18 +0200, Daniel Vetter a écrit :
> > > > > In the first, we'd need a mechanism where we can schedule a render at 
> > > > > a
> > > > > specific time or vblank. We can of course already implement this in
> > > > > software, but with fences, the scheduling would need to be done in the
> > > > > driver. Then if the fence is signalled earlier, the driver should hold
> > > > > on until the delay is met. If the fence got signalled late, we also
> > > > > need to think of a workflow. As we can't schedule more then one render
> > > > > in DRM at one time, I don't really see yet how to make that work.
> > > > 
> > > > Indeed, that's also one of the main issues I've spotted. Before using
> > > > an implicit fence, we basically have to make sure the frame is due for
> > > > display at the next vblank. Otherwise, we need to refrain from using
> > > > the fence and schedule the flip later, which is kind of counter-
> > > > productive.
> > > 
> > > Fences are about signalling that the contents of a frame are "done" and
> > > ready to be presented. They're not about specifying which frame is to be
> > > presented when.
> > > 
> > > 
> > > > I feel like specifying a target vblank would be a good unit for that,
> > > 
> > > The mechanism described above works for that.
> > > 
> > > > since it's our native granularity after all (while a timestamp is not).
> > > 
> > > Note that variable refresh rate (Adaptive Sync / FreeSync / G-Sync)
> > > changes things in this regard. It makes the vblank length variable, and
> > > if you wait for multiple vblanks between flips, you get the maximum
> > > vblank length corresponding to the minimum refresh rate / timing
> > > granularity. Thus, it would be useful to allow userspace to specify a
> > > timestamp corresponding to the earliest time when the flip is to
> > > complete. The kernel could then try to hit that as closely as possible.
> > 
> > Rendering a video stream is more complex then what you describe here.
> > Whenever there is a unexpected delay (late delivery of a frame as an
> > example) you may endup in situation where one frame is ready after the
> > targeted vblank. If there is another frame that targets the following
> > vblank that gets ready on-time, the previous frame should be replaced
> > by the most recent one.
> > 
> > With fences, what happens is that even if you received the next frame
> > on time, naively replacing it is not possible, because we don't know
> > when the fence for the next frame will be signalled. If you simply
> > always replace the current frame, you may endup skipping a lot more
> > vblank then what you expect, and that results in jumpy playback.
> 
> So you want to be able to replace a queued flip with another one then.
> That doesn't necessarily require allowing more than one flip to be
> queued ahead of time.

There might be other ways to do it, but this one has plenty of
advantages.

> Note that this can also be done in userspace with explicit fencing (by
> only selecting a frame and submitting it to the kernel after all
> corresponding fences have signalled), at least to some degree, but the
> kernel should be able to do it up to a later point in time and more
> reliably, with less risk of missing a flip for a frame which becomes
> ready just in time.

Indeed, but it would be great if we could do that with implicit fencing
as well.

> > Render queues with timestamp are used to smooth rendering and handle
> > rendering collision so that the latency is kept low (like when you have
> > a 100fps video over a 60Hz display). This is normally done in
> > userspace, but with fences, you ask the kernel to render something in
> > an unpredictable future, so we loose the ability to make the final
> > decision.
> 
> That's just not what fences are intended to be used for with the current
> KMS UAPI.

Yes, and I think we're discussing towards changing that in the future.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/stm: ltdc: return appropriate error code during probe

2019-04-24 Thread Fabien Dessenne
During probe, return the "clk_get" error value instead of -ENODEV.

Signed-off-by: Fabien Dessenne 
---
 drivers/gpu/drm/stm/ltdc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 521ba83..97912e2 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -1145,8 +1145,9 @@ int ltdc_load(struct drm_device *ddev)
 
ldev->pixel_clk = devm_clk_get(dev, "lcd");
if (IS_ERR(ldev->pixel_clk)) {
-   DRM_ERROR("Unable to get lcd clock\n");
-   return -ENODEV;
+   if (PTR_ERR(ldev->pixel_clk) != -EPROBE_DEFER)
+   DRM_ERROR("Unable to get lcd clock\n");
+   return PTR_ERR(ldev->pixel_clk);
}
 
if (clk_prepare_enable(ldev->pixel_clk)) {
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Support for 2D engines/blitters in V4L2 and DRM

2019-04-24 Thread Michel Dänzer
On 2019-04-24 2:01 p.m., Nicolas Dufresne wrote:
> Le mercredi 24 avril 2019 à 10:31 +0200, Michel Dänzer a écrit :
>> On 2019-04-19 10:38 a.m., Paul Kocialkowski wrote:
>>> On Thu, 2019-04-18 at 20:30 -0400, Nicolas Dufresne wrote:
 Le jeudi 18 avril 2019 à 10:18 +0200, Daniel Vetter a écrit :
>>
 In the first, we'd need a mechanism where we can schedule a render at a
 specific time or vblank. We can of course already implement this in
 software, but with fences, the scheduling would need to be done in the
 driver. Then if the fence is signalled earlier, the driver should hold
 on until the delay is met. If the fence got signalled late, we also
 need to think of a workflow. As we can't schedule more then one render
 in DRM at one time, I don't really see yet how to make that work.
>>>
>>> Indeed, that's also one of the main issues I've spotted. Before using
>>> an implicit fence, we basically have to make sure the frame is due for
>>> display at the next vblank. Otherwise, we need to refrain from using
>>> the fence and schedule the flip later, which is kind of counter-
>>> productive.
>>
>> Fences are about signalling that the contents of a frame are "done" and
>> ready to be presented. They're not about specifying which frame is to be
>> presented when.
>>
>>
>>> I feel like specifying a target vblank would be a good unit for that,
>>
>> The mechanism described above works for that.
>>
>>> since it's our native granularity after all (while a timestamp is not).
>>
>> Note that variable refresh rate (Adaptive Sync / FreeSync / G-Sync)
>> changes things in this regard. It makes the vblank length variable, and
>> if you wait for multiple vblanks between flips, you get the maximum
>> vblank length corresponding to the minimum refresh rate / timing
>> granularity. Thus, it would be useful to allow userspace to specify a
>> timestamp corresponding to the earliest time when the flip is to
>> complete. The kernel could then try to hit that as closely as possible.
> 
> Rendering a video stream is more complex then what you describe here.
> Whenever there is a unexpected delay (late delivery of a frame as an
> example) you may endup in situation where one frame is ready after the
> targeted vblank. If there is another frame that targets the following
> vblank that gets ready on-time, the previous frame should be replaced
> by the most recent one.
> 
> With fences, what happens is that even if you received the next frame
> on time, naively replacing it is not possible, because we don't know
> when the fence for the next frame will be signalled. If you simply
> always replace the current frame, you may endup skipping a lot more
> vblank then what you expect, and that results in jumpy playback.

So you want to be able to replace a queued flip with another one then.
That doesn't necessarily require allowing more than one flip to be
queued ahead of time.

Note that this can also be done in userspace with explicit fencing (by
only selecting a frame and submitting it to the kernel after all
corresponding fences have signalled), at least to some degree, but the
kernel should be able to do it up to a later point in time and more
reliably, with less risk of missing a flip for a frame which becomes
ready just in time.


> Render queues with timestamp are used to smooth rendering and handle
> rendering collision so that the latency is kept low (like when you have
> a 100fps video over a 60Hz display). This is normally done in
> userspace, but with fences, you ask the kernel to render something in
> an unpredictable future, so we loose the ability to make the final
> decision.

That's just not what fences are intended to be used for with the current
KMS UAPI.


-- 
Earthling Michel Dänzer   |  https://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: [PATCH] drm/panfrost: Add sanity checks to submit IOCTL

2019-04-24 Thread Tomeu Vizoso
On Wed, 24 Apr 2019 at 15:20, Daniel Vetter  wrote:
>
> On Wed, Apr 24, 2019 at 03:13:53PM +0200, Tomeu Vizoso wrote:
> > So userspace can get feedback on any error conditions, instead of going
> > ahead and things breaking later.
> >
> > Signed-off-by: Tomeu Vizoso 
>
> Bonus: some igts to exercise these corner cases. It helps a lot with uapi
> review and testing.

I'm rebasing that igt branch as we speak :)

Tomeu

> -Daniel
>
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_drv.c | 35 +
> >  1 file changed, 24 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c 
> > b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index c06af78ab833..0f2863cb8077 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -172,13 +172,27 @@ static int panfrost_ioctl_submit(struct drm_device 
> > *dev, void *data,
> >  {
> >   struct panfrost_device *pfdev = dev->dev_private;
> >   struct drm_panfrost_submit *args = data;
> > - struct drm_syncobj *sync_out;
> > + struct drm_syncobj *sync_out = NULL;
> >   struct panfrost_job *job;
> >   int ret = 0;
> >
> > + if (!args->jc)
> > + return -EINVAL;
> > +
> > + if (args->requirements && args->requirements != PANFROST_JD_REQ_FS)
> > + return -EINVAL;
> > +
> > + if (args->out_sync > 0) {
> > + sync_out = drm_syncobj_find(file, args->out_sync);
> > + if (!sync_out)
> > + return -ENODEV;
> > + }
> > +
> >   job = kzalloc(sizeof(*job), GFP_KERNEL);
> > - if (!job)
> > - return -ENOMEM;
> > + if (!job) {
> > + ret = -ENOMEM;
> > + goto fail_out_sync;
> > + }
> >
> >   kref_init(>refcount);
> >
> > @@ -190,25 +204,24 @@ static int panfrost_ioctl_submit(struct drm_device 
> > *dev, void *data,
> >
> >   ret = panfrost_copy_in_sync(dev, file, args, job);
> >   if (ret)
> > - goto fail;
> > + goto fail_job;
> >
> >   ret = panfrost_lookup_bos(dev, file, args, job);
> >   if (ret)
> > - goto fail;
> > + goto fail_job;
> >
> >   ret = panfrost_job_push(job);
> >   if (ret)
> > - goto fail;
> > + goto fail_job;
> >
> >   /* Update the return sync object for the job */
> > - sync_out = drm_syncobj_find(file, args->out_sync);
> > - if (sync_out) {
> > + if (sync_out)
> >   drm_syncobj_replace_fence(sync_out, job->render_done_fence);
> > - drm_syncobj_put(sync_out);
> > - }
> >
> > -fail:
> > +fail_job:
> >   panfrost_job_put(job);
> > +fail_out_sync:
> > + drm_syncobj_put(sync_out);
> >
> >   return ret;
> >  }
> > --
> > 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] gpu/docs: Clarify what userspace means for gl

2019-04-24 Thread Daniel Vetter
On Wed, Apr 24, 2019 at 4:28 PM Daniel Vetter  wrote:
>
> On Wed, Apr 24, 2019 at 4:24 PM Zhou, David(ChunMing)
>  wrote:
> > Will linux be only mesa-linux? I thought linux is an  open linux.
> > Which will impact our opengl/amdvlk(MIT open source), not sure Rocm:
> > 1. how to deal with one uapi that opengl/amdvlk needs but mesa dont need? 
> > reject?
> > 2. one hw feature that opengl/amdvlk developers work on that but no mesa 
> > developers work on, cannot upstream as well?
> >
> > I think above two would easily happen, because there are many employees 
> > working on company project with many customer kinds of reqiurements, but 
> > mesa not.
>
> Well those are the questions I tried to have a clear answer for, to
> avoid confusion, but looks like the agreement isn't perfect. But
> there's pretty clear favour towards cross vendor userspace that's
> collaboratively and openly developed like mesa. So whenever one of the
> above cases you bring up happen we'll get to have a big thread on
> dri-devel I guess.

Aside, I think there's a pretty huge difference between rocm and
amdvlk. rocm is developed openly and all that, the one nit there is is
that it's not cross vendor. But then, there is no other vendor really
(but amd still controls it all despite the usual "totally an open
standard" lipstick in the marketing material). amdvlk otoh is open
source in the narrowest legal sense only, and nothing else. And yes
intel has some equally bad projects on the openess metric too with
some of the more recent-ish changes in the userspace stack.
-Daniel

> -Daniel
>
>
> > -David
> >
> >  Original Message 
> > Subject: [PATCH] gpu/docs: Clarify what userspace means for gl
> > From: Daniel Vetter
> > To: DRI Development ,Mesa Dev
> > CC: Jérôme Glisse ,Daniel Vetter ,Karol Herbst ,Kenneth Graunke ,Ben Skeggs 
> > ,Daniel Vetter ,Sean Paul
> >
> > Clear rules avoid arguing.
> >
> > Note that this just aims to document current expectations. If that
> > shifts (e.g. because gl isn't the main api anymore, replaced by vk),
> > then we need to update this text.
> >
> > I think it'd be good to have an equally solid list on the kms side.
> > But kms is much more meant to be a standard, and the list of userspace
> > projects we've accepted in the past is constantly shifting and
> > adjusting. So I figured I'll leave that as an exercise for later on.
> >
> > v2: Try to clarify that we don't want a mesa driver just for mesa's
> > sake, and more clearly exclude anything that just doesn't make sense
> > technically.  Example would be a compute driver that makes sense to be
> > merged into drm (for kernel side code-sharing), but where the intended
> > use is some single-source CUDA-style compute without ever bothering
> > about any of the 3D/rendering side baggage that comes with gl/vk.
> >
> > v3: Drop vulkan for now, the situation there isn't as obviously
> > clear-cut as on the gl side, and I don't want to tank this idea on a
> > hot discussion about vk and mesa. Plus I think once we have 1-2 more
> > vk drivers in mesa the situation on the vk side is clear-cut too, and
> > we can do a follow-up patch to add vk to the list where we expect the
> > userspace to be in upstream mesa. That's would give nice precedence to
> > make it clear that this isn't cast in stone, but meant to reflect
> > reality and should be adjusted as needed.
> >
> > v4: Fix typo.
> >
> > v5: Add a note to the commit message that this text needs to be
> > updated when the situation changes.
> >
> > v6: Add a sentence why mesa will give the most meaningful review on gl
> > stuff - it's a very active project with lots of developers.
> >
> > Acked-by: Dave Airlie  (v4)
> > Acked-by: Eric Anholt  (v4)
> > Acked-by: Alex Deucher  (v5)
> > Acked-by: Sean Paul  (v5)
> > Acked-by: Kenneth Graunke  (v5)
> > Acked-by: Karol Herbst  (v5)
> > Acked-by: Rob Clark 
> > Acked-by: Jérôme Glisse 
> > Acked-by: Bas Nieuwenhuizen 
> > Acked-by: Ben Skeggs 
> > Cc: Dave Airlie 
> > Cc: Eric Anholt 
> > Cc: Alex Deucher 
> > Cc: Sean Paul 
> > Cc: Kenneth Graunke 
> > Cc: Karol Herbst 
> > Cc: Rob Clark 
> > Cc: Jérôme Glisse 
> > Cc: Bas Nieuwenhuizen 
> > Cc: Ben Skeggs 
> > Signed-off-by: Daniel Vetter 
> > ---
> > I chatted with a pile of people in private, and there's clearly some
> > solid support for this. But there's also some big concerns brought up
> > by other people. The main one summed up is "what if everyone just
> > ships vk, with a generic gl-on-vk like ANGLE?", but there's other
> > concerns too.
> >
> > So all together I think this doesn't clear the bar of (almost)
> > unanimous support which we need to make documentation actually help
> > with clarifying what's expected. And if/when someone comes up with a
> > more creative userspace approach for gl/vk we'll need to figure this
> > all out with the time honored tradition of having a few massive
> > threads on dri-devel :-)
> >
> > Hence this is more fyi as a guidance I guess, not a strict rule.

Re: [PATCH 4/4] drm/amd/display: Compensate for pre-DCE12 BTR-VRR hw limitations.

2019-04-24 Thread Kazlauskas, Nicholas
On 4/17/19 11:51 PM, Mario Kleiner wrote:
> Pre-DCE12 needs special treatment for BTR / low framerate
> compensation for more stable behaviour:
> 
> According to comments in the code and some testing on DCE-8
> and DCE-11, DCE-11 and earlier only apply VTOTAL_MIN/MAX
> programming with a lag of one frame, so the special BTR hw
> programming for intermediate fixed duration frames must be
> done inside the current frame at flip submission in atomic
> commit tail, ie. one vblank earlier, and the fixed refresh
> intermediate frame mode must be also terminated one vblank
> earlier on pre-DCE12 display engines.
> 
> To achieve proper termination on < DCE-12 shift the point
> when the switch-back from fixed vblank duration to variable
> vblank duration happens from the start of VBLANK (vblank irq,
> as done on DCE-12+) to back-porch or end of VBLANK (handled
> by vupdate irq handler). We must leave the switch-back code
> inside VBLANK irq for DCE12+, as before.
> 
> Doing this, we get much better behaviour of BTR for up-sweeps,
> ie. going from short to long frame durations (~high to low fps)
> and for constant framerate flips, as tested on DCE-8 and
> DCE-11. Behaviour is still not quite as good as on DCN-1
> though.
> 
> On down-sweeps, going from long to short frame durations
> (low fps to high fps) < DCE-12 is a little bit improved,
> although by far not as much as for up-sweeps and constant
> fps.
> 
> Signed-off-by: Mario Kleiner 
> ---

I did some debugging/testing with this patch and it certainly does 
improve things quite a bit for pre DCE12 (since this really should have 
been handled in VUPDATE before).

I have one comment inline below:


>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++-
>   1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 76b6e621793f..9c8c94f82b35 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -364,6 +364,7 @@ static void dm_vupdate_high_irq(void *interrupt_params)
>   struct amdgpu_device *adev = irq_params->adev;
>   struct amdgpu_crtc *acrtc;
>   struct dm_crtc_state *acrtc_state;
> + unsigned long flags;
>   
>   acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - 
> IRQ_TYPE_VUPDATE);
>   
> @@ -381,6 +382,22 @@ static void dm_vupdate_high_irq(void *interrupt_params)
>*/
>   if (amdgpu_dm_vrr_active(acrtc_state))
>   drm_crtc_handle_vblank(>base);
> +
> + if (acrtc_state->stream && adev->family < AMDGPU_FAMILY_AI &&
> + acrtc_state->vrr_params.supported &&
> + acrtc_state->freesync_config.state == 
> VRR_STATE_ACTIVE_VARIABLE) {
> + spin_lock_irqsave(>ddev->event_lock, flags);
> + mod_freesync_handle_v_update(
> + adev->dm.freesync_module,
> + acrtc_state->stream,
> + _state->vrr_params);
> +
> + dc_stream_adjust_vmin_vmax(
> + adev->dm.dc,
> + acrtc_state->stream,
> + _state->vrr_params.adjust);
> + spin_unlock_irqrestore(>ddev->event_lock, flags);
> + }
>   }
>   }
>   
> @@ -390,6 +407,7 @@ static void dm_crtc_high_irq(void *interrupt_params)
>   struct amdgpu_device *adev = irq_params->adev;
>   struct amdgpu_crtc *acrtc;
>   struct dm_crtc_state *acrtc_state;
> + unsigned long flags;
>   
>   acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - 
> IRQ_TYPE_VBLANK);
>   
> @@ -412,9 +430,10 @@ static void dm_crtc_high_irq(void *interrupt_params)
>*/
>   amdgpu_dm_crtc_handle_crc_irq(>base);
>   
> - if (acrtc_state->stream &&
> + if (acrtc_state->stream && adev->family >= AMDGPU_FAMILY_AI &&
>   acrtc_state->vrr_params.supported &&
>   acrtc_state->freesync_config.state == 
> VRR_STATE_ACTIVE_VARIABLE) {
> + spin_lock_irqsave(>ddev->event_lock, flags);
>   mod_freesync_handle_v_update(
>   adev->dm.freesync_module,
>   acrtc_state->stream,
> @@ -424,6 +443,7 @@ static void dm_crtc_high_irq(void *interrupt_params)
>   adev->dm.dc,
>   acrtc_state->stream,
>   _state->vrr_params.adjust);
> + spin_unlock_irqrestore(>ddev->event_lock, flags);
>   }
>   }
>   }
> @@ -4880,6 +4900,8 @@ static void update_freesync_state_on_stream(
>   {
>   struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
>   struct dc_info_packet vrr_infopacket = {0};
> + struct 

Re: [PATCH] staging: ion: solve warning symbol was not declared

2019-04-24 Thread Dan Carpenter
On Mon, Apr 22, 2019 at 08:49:27PM +0200, Oscar Gomez Fuente wrote:
> These changes solve warning symbol was not declared in the functions:
> ion_carveout_heap_create and ion_chunk_heap_create
> 
> Signed-off-by: Oscar Gomez Fuente 
> ---
>  drivers/staging/android/ion/ion_carveout_heap.c | 2 +-
>  drivers/staging/android/ion/ion_chunk_heap.c| 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion_carveout_heap.c 
> b/drivers/staging/android/ion/ion_carveout_heap.c
> index bb9d614..3f359ae 100644
> --- a/drivers/staging/android/ion/ion_carveout_heap.c
> +++ b/drivers/staging/android/ion/ion_carveout_heap.c
> @@ -103,7 +103,7 @@ static struct ion_heap_ops carveout_heap_ops = {
>   .unmap_kernel = ion_heap_unmap_kernel,
>  };
>  
> -struct ion_heap *ion_carveout_heap_create(phys_addr_t base, size_t size)
> +static inline struct ion_heap *ion_carveout_heap_create(phys_addr_t base, 
> size_t size)

Why are you making it inline?  Btw, normally we just leave it for the
compiler to choose which functions to make inline.

regards,
dan carpenter

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] gpu/docs: Clarify what userspace means for gl

2019-04-24 Thread Daniel Vetter
On Wed, Apr 24, 2019 at 4:24 PM Zhou, David(ChunMing)
 wrote:
> Will linux be only mesa-linux? I thought linux is an  open linux.
> Which will impact our opengl/amdvlk(MIT open source), not sure Rocm:
> 1. how to deal with one uapi that opengl/amdvlk needs but mesa dont need? 
> reject?
> 2. one hw feature that opengl/amdvlk developers work on that but no mesa 
> developers work on, cannot upstream as well?
>
> I think above two would easily happen, because there are many employees 
> working on company project with many customer kinds of reqiurements, but mesa 
> not.

Well those are the questions I tried to have a clear answer for, to
avoid confusion, but looks like the agreement isn't perfect. But
there's pretty clear favour towards cross vendor userspace that's
collaboratively and openly developed like mesa. So whenever one of the
above cases you bring up happen we'll get to have a big thread on
dri-devel I guess.
-Daniel


> -David
>
>  Original Message 
> Subject: [PATCH] gpu/docs: Clarify what userspace means for gl
> From: Daniel Vetter
> To: DRI Development ,Mesa Dev
> CC: Jérôme Glisse ,Daniel Vetter ,Karol Herbst ,Kenneth Graunke ,Ben Skeggs 
> ,Daniel Vetter ,Sean Paul
>
> Clear rules avoid arguing.
>
> Note that this just aims to document current expectations. If that
> shifts (e.g. because gl isn't the main api anymore, replaced by vk),
> then we need to update this text.
>
> I think it'd be good to have an equally solid list on the kms side.
> But kms is much more meant to be a standard, and the list of userspace
> projects we've accepted in the past is constantly shifting and
> adjusting. So I figured I'll leave that as an exercise for later on.
>
> v2: Try to clarify that we don't want a mesa driver just for mesa's
> sake, and more clearly exclude anything that just doesn't make sense
> technically.  Example would be a compute driver that makes sense to be
> merged into drm (for kernel side code-sharing), but where the intended
> use is some single-source CUDA-style compute without ever bothering
> about any of the 3D/rendering side baggage that comes with gl/vk.
>
> v3: Drop vulkan for now, the situation there isn't as obviously
> clear-cut as on the gl side, and I don't want to tank this idea on a
> hot discussion about vk and mesa. Plus I think once we have 1-2 more
> vk drivers in mesa the situation on the vk side is clear-cut too, and
> we can do a follow-up patch to add vk to the list where we expect the
> userspace to be in upstream mesa. That's would give nice precedence to
> make it clear that this isn't cast in stone, but meant to reflect
> reality and should be adjusted as needed.
>
> v4: Fix typo.
>
> v5: Add a note to the commit message that this text needs to be
> updated when the situation changes.
>
> v6: Add a sentence why mesa will give the most meaningful review on gl
> stuff - it's a very active project with lots of developers.
>
> Acked-by: Dave Airlie  (v4)
> Acked-by: Eric Anholt  (v4)
> Acked-by: Alex Deucher  (v5)
> Acked-by: Sean Paul  (v5)
> Acked-by: Kenneth Graunke  (v5)
> Acked-by: Karol Herbst  (v5)
> Acked-by: Rob Clark 
> Acked-by: Jérôme Glisse 
> Acked-by: Bas Nieuwenhuizen 
> Acked-by: Ben Skeggs 
> Cc: Dave Airlie 
> Cc: Eric Anholt 
> Cc: Alex Deucher 
> Cc: Sean Paul 
> Cc: Kenneth Graunke 
> Cc: Karol Herbst 
> Cc: Rob Clark 
> Cc: Jérôme Glisse 
> Cc: Bas Nieuwenhuizen 
> Cc: Ben Skeggs 
> Signed-off-by: Daniel Vetter 
> ---
> I chatted with a pile of people in private, and there's clearly some
> solid support for this. But there's also some big concerns brought up
> by other people. The main one summed up is "what if everyone just
> ships vk, with a generic gl-on-vk like ANGLE?", but there's other
> concerns too.
>
> So all together I think this doesn't clear the bar of (almost)
> unanimous support which we need to make documentation actually help
> with clarifying what's expected. And if/when someone comes up with a
> more creative userspace approach for gl/vk we'll need to figure this
> all out with the time honored tradition of having a few massive
> threads on dri-devel :-)
>
> Hence this is more fyi as a guidance I guess, not a strict rule.
> And I don't plan on merging this.
>
> Cheers, Daniel
> ---
>  Documentation/gpu/drm-uapi.rst | 25 +
>  1 file changed, 25 insertions(+)
>
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index c9fd23efd957..0f767cfd5db6 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -105,6 +105,31 @@ is already rather painful for the DRM subsystem, with 
> multiple different uAPIs
>  for the same thing co-existing. If we add a few more complete mistakes into 
> the
>  mix every year it would be entirely unmanageable.
>
> +Below some clarifications what this means for specific areas in DRM.
> +
> +Compute Userspace
> +---
> +
> +Userspace API for enabling compute and rendering blocks 

Re:[PATCH] gpu/docs: Clarify what userspace means for gl

2019-04-24 Thread Zhou, David(ChunMing)
Will linux be only mesa-linux? I thought linux is an  open linux.
Which will impact our opengl/amdvlk(MIT open source), not sure Rocm:
1. how to deal with one uapi that opengl/amdvlk needs but mesa dont need? 
reject?
2. one hw feature that opengl/amdvlk developers work on that but no mesa 
developers work on, cannot upstream as well?

I think above two would easily happen, because there are many employees working 
on company project with many customer kinds of reqiurements, but mesa not.

-David

 Original Message 
Subject: [PATCH] gpu/docs: Clarify what userspace means for gl
From: Daniel Vetter
To: DRI Development ,Mesa Dev
CC: Jérôme Glisse ,Daniel Vetter ,Karol Herbst ,Kenneth Graunke ,Ben Skeggs 
,Daniel Vetter ,Sean Paul

Clear rules avoid arguing.

Note that this just aims to document current expectations. If that
shifts (e.g. because gl isn't the main api anymore, replaced by vk),
then we need to update this text.

I think it'd be good to have an equally solid list on the kms side.
But kms is much more meant to be a standard, and the list of userspace
projects we've accepted in the past is constantly shifting and
adjusting. So I figured I'll leave that as an exercise for later on.

v2: Try to clarify that we don't want a mesa driver just for mesa's
sake, and more clearly exclude anything that just doesn't make sense
technically.  Example would be a compute driver that makes sense to be
merged into drm (for kernel side code-sharing), but where the intended
use is some single-source CUDA-style compute without ever bothering
about any of the 3D/rendering side baggage that comes with gl/vk.

v3: Drop vulkan for now, the situation there isn't as obviously
clear-cut as on the gl side, and I don't want to tank this idea on a
hot discussion about vk and mesa. Plus I think once we have 1-2 more
vk drivers in mesa the situation on the vk side is clear-cut too, and
we can do a follow-up patch to add vk to the list where we expect the
userspace to be in upstream mesa. That's would give nice precedence to
make it clear that this isn't cast in stone, but meant to reflect
reality and should be adjusted as needed.

v4: Fix typo.

v5: Add a note to the commit message that this text needs to be
updated when the situation changes.

v6: Add a sentence why mesa will give the most meaningful review on gl
stuff - it's a very active project with lots of developers.

Acked-by: Dave Airlie  (v4)
Acked-by: Eric Anholt  (v4)
Acked-by: Alex Deucher  (v5)
Acked-by: Sean Paul  (v5)
Acked-by: Kenneth Graunke  (v5)
Acked-by: Karol Herbst  (v5)
Acked-by: Rob Clark 
Acked-by: Jérôme Glisse 
Acked-by: Bas Nieuwenhuizen 
Acked-by: Ben Skeggs 
Cc: Dave Airlie 
Cc: Eric Anholt 
Cc: Alex Deucher 
Cc: Sean Paul 
Cc: Kenneth Graunke 
Cc: Karol Herbst 
Cc: Rob Clark 
Cc: Jérôme Glisse 
Cc: Bas Nieuwenhuizen 
Cc: Ben Skeggs 
Signed-off-by: Daniel Vetter 
---
I chatted with a pile of people in private, and there's clearly some
solid support for this. But there's also some big concerns brought up
by other people. The main one summed up is "what if everyone just
ships vk, with a generic gl-on-vk like ANGLE?", but there's other
concerns too.

So all together I think this doesn't clear the bar of (almost)
unanimous support which we need to make documentation actually help
with clarifying what's expected. And if/when someone comes up with a
more creative userspace approach for gl/vk we'll need to figure this
all out with the time honored tradition of having a few massive
threads on dri-devel :-)

Hence this is more fyi as a guidance I guess, not a strict rule.
And I don't plan on merging this.

Cheers, Daniel
---
 Documentation/gpu/drm-uapi.rst | 25 +
 1 file changed, 25 insertions(+)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index c9fd23efd957..0f767cfd5db6 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -105,6 +105,31 @@ is already rather painful for the DRM subsystem, with 
multiple different uAPIs
 for the same thing co-existing. If we add a few more complete mistakes into the
 mix every year it would be entirely unmanageable.

+Below some clarifications what this means for specific areas in DRM.
+
+Compute Userspace
+---
+
+Userspace API for enabling compute and rendering blocks which are capable of at
+least supporting one of the OpenGL or OpenGL ES standards from Khronos need to
+be enabled in the upstream `Mesa3D project`.
+
+Mesa3D is the canonical upstream for these areas because it is a fully
+compliant, performant and cross-vendor implementation that supports all kernel
+drivers in DRM. It is also an active project with plenty of developers who
+can perform meaningful review. It is therefore the best platform to validate
+userspace API and especially make sure that cross-vendor interoperation is
+assured.
+
+Other userspace is only admissible if exposing a given feature 

Re: [PATCH v6 3/3] backlight: lm3630a: add firmware node support

2019-04-24 Thread Pavel Machek
On Wed 2019-04-24 05:25:05, Brian Masney wrote:
> Add fwnode support to the lm3630a driver and optionally allow
> configuring the label, default brightness level, and maximum brightness
> level. The two outputs can be controlled by bank A and B independently
> or bank A can control both outputs.
> 
> If the platform data was not configured, then the driver defaults to
> enabling both banks. This patch changes the default value to disable
> both banks before parsing the firmware node so that just a single bank
> can be enabled if desired. There are no in-tree users of this driver.
> 
> Driver was tested on a LG Nexus 5 (hammerhead) phone.
> 
> Signed-off-by: Brian Masney 
> Reviewed-by: Dan Murphy 

Acked-by: Pavel Machek 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/fb-helper: Fix drm_fb_helper_firmware_config() NULL pointer deref

2019-04-24 Thread Daniel Vetter
On Wed, Apr 24, 2019 at 4:06 PM Noralf Trønnes  wrote:
>
>
>
> Den 23.04.2019 21.01, skrev Daniel Vetter:
> > On Tue, Apr 23, 2019 at 04:53:53PM +0200, Noralf Trønnes wrote:
> >> Non-atomic drivers like ast doesn't have connector->state set resulting
> >> in a NULL pointer deref:
> >>
> >> [   29.609593] BUG: unable to handle kernel NULL pointer dereference at 
> >> 0010
> >> [   29.609619] Call Trace:
> >> [   29.609630]  ? drm_helper_probe_single_connector_modes+0x27f/0x680
> >> [   29.609640]  drm_setup_crtcs+0x431/0xd80 [drm_kms_helper]
> >> [   29.753065]  __drm_fb_helper_initial_config_and_unlock+0x6f/0x6a0
> >> [   29.753160]  ? drm_modeset_unlock_all+0x31/0x50 [drm]
> >> [   29.765758]  ast_fbdev_init+0xa8/0xc0 [ast]
> >> [   29.765762]  ast_driver_load.cold.7+0x2b3/0xe11 [ast]
> >> [   29.765775]  drm_dev_register+0x111/0x150 [drm]
> >>
> >> Fix by bailing out if the driver does not support atomic modesetting.
> >>
> >> Fixes: 09ded8af57bc ("drm/i915/fbdev: Move intel_fb_initial_config() to 
> >> fbdev helper")
> >> Reported-by: Thomas Zimmermann 
> >> Cc: Daniel Vetter 
> >> Cc: Jani Nikula 
> >> Signed-off-by: Noralf Trønnes 
> >> ---
> >>  drivers/gpu/drm/drm_fb_helper.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> >> b/drivers/gpu/drm/drm_fb_helper.c
> >> index 2339f0f8f5a8..899c2eca26d1 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -2588,6 +2588,9 @@ static bool drm_fb_helper_firmware_config(struct 
> >> drm_fb_helper *fb_helper,
> >>  int num_connectors_detected = 0;
> >>  struct drm_modeset_acquire_ctx ctx;
> >>
> >> +if (!drm_drv_uses_atomic_modeset(dev))
> >> +return false;
> >
> > Reviewed-by: Daniel Vetter 
> >
> > I think for merging we're already past feature freeze, but
> > drm-misc-next-fixes hasn't been rolled forward yet. I think you need to
> > wait for drm-misc maintainers to do that (I pinged them already), and then
> > put this one there.
>
> I saw that -fixes was updated including Dave's i915 backported revert,
> so I have applied this to drm-misc-next-fixes.
>
> How/when will this show up in drm-misc-next? My drm_fb_helper
> refactoring needs to be rebased on this.

Ask drm-misc maintainers that they need to send out a pull request and
then backmerge drm-next into drm-misc-next for you. Adding them.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/bridge/synopsys: dsi: Allow VPG to be enabled via debugfs

2019-04-24 Thread Matt Redfearn
The Synopsys MIPI DSI IP contains a video test pattern generator which
is helpful in debugging video timing with connected displays.
Add a debugfs directory containing files which allow the VPG to be
enabled and disabled, and it's orientation to be changed.

Signed-off-by: Matt Redfearn 

---

 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 55 +++
 1 file changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 0ee440216b8..a1ee2306382 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -10,6 +10,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -86,6 +87,8 @@
 #define VID_MODE_TYPE_NON_BURST_SYNC_EVENTS0x1
 #define VID_MODE_TYPE_BURST0x2
 #define VID_MODE_TYPE_MASK 0x3
+#define VID_MODE_VPG_ENABLEBIT(16)
+#define VID_MODE_VPG_HORIZONTALBIT(24)
 
 #define DSI_VID_PKT_SIZE   0x3c
 #define VID_PKT_SIZE(p)((p) & 0x3fff)
@@ -234,6 +237,15 @@ struct dw_mipi_dsi {
u32 format;
unsigned long mode_flags;
 
+#ifdef CONFIG_DEBUG_FS
+   struct dentry *debugfs_root;
+   struct dentry *debugfs_vpg;
+   struct dentry *debugfs_vpg_horizontal;
+
+   bool vpg;
+   bool vpg_horizontal;
+#endif /* CONFIG_DEBUG_FS */
+
struct dw_mipi_dsi *master; /* dual-dsi master ptr */
struct dw_mipi_dsi *slave; /* dual-dsi slave ptr */
 
@@ -525,6 +537,11 @@ static void dw_mipi_dsi_video_mode_config(struct 
dw_mipi_dsi *dsi)
else
val |= VID_MODE_TYPE_NON_BURST_SYNC_EVENTS;
 
+   if (dsi->vpg) {
+   val |= VID_MODE_VPG_ENABLE;
+   val |= dsi->vpg_horizontal ? VID_MODE_VPG_HORIZONTAL : 0;
+   }
+
dsi_write(dsi, DSI_VID_MODE_CFG, val);
 }
 
@@ -935,6 +952,41 @@ static const struct drm_bridge_funcs 
dw_mipi_dsi_bridge_funcs = {
.attach   = dw_mipi_dsi_bridge_attach,
 };
 
+#ifdef CONFIG_DEBUG_FS
+
+static void dw_mipi_dsi_debugfs_init(struct dw_mipi_dsi *dsi)
+{
+   struct dentry *d;
+
+   d = debugfs_create_dir(dev_name(dsi->dev), NULL);
+   if (IS_ERR(d)) {
+   dev_err(dsi->dev, "failed to create debugfs root\n");
+   return;
+   }
+   dsi->debugfs_root = d;
+
+   d = debugfs_create_bool("vpg", 0660, dsi->debugfs_root, >vpg);
+   dsi->debugfs_vpg = d;
+
+   d = debugfs_create_bool("vpg_horizontal", 0660, dsi->debugfs_root,
+   >vpg_horizontal);
+   dsi->debugfs_vpg_horizontal = d;
+}
+
+static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi)
+{
+   debugfs_remove(dsi->debugfs_vpg_horizontal);
+   debugfs_remove(dsi->debugfs_vpg);
+   debugfs_remove(dsi->debugfs_root);
+}
+
+#else
+
+static void dw_mipi_dsi_debugfs_init(struct dw_mipi_dsi *dsi) { }
+static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi) { }
+
+#endif /* CONFIG_DEBUG_FS */
+
 static struct dw_mipi_dsi *
 __dw_mipi_dsi_probe(struct platform_device *pdev,
const struct dw_mipi_dsi_plat_data *plat_data)
@@ -1005,6 +1057,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
clk_disable_unprepare(dsi->pclk);
}
 
+   dw_mipi_dsi_debugfs_init(dsi);
pm_runtime_enable(dev);
 
dsi->dsi_host.ops = _mipi_dsi_host_ops;
@@ -1012,6 +1065,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
ret = mipi_dsi_host_register(>dsi_host);
if (ret) {
dev_err(dev, "Failed to register MIPI host: %d\n", ret);
+   dw_mipi_dsi_debugfs_remove(dsi);
return ERR_PTR(ret);
}
 
@@ -1029,6 +1083,7 @@ static void __dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi)
mipi_dsi_host_unregister(>dsi_host);
 
pm_runtime_disable(dsi->dev);
+   dw_mipi_dsi_debugfs_remove(dsi);
 }
 
 void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi *slave)
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/bridge/synopsys: dsi: Don't blindly call post_disable

2019-04-24 Thread Matt Redfearn
The DRM documentation states that post_disable is an optional callback.
As such an implementing device may not populate it. To avoid panicing
the kernel by calling a NULL function pointer, we should NULL check it
before blindy calling it.

Signed-off-by: Matt Redfearn 

---

 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 38e88071363..0ee440216b8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -805,7 +805,8 @@ static void dw_mipi_dsi_bridge_post_disable(struct 
drm_bridge *bridge)
 * This needs to be fixed in the drm_bridge framework and the API
 * needs to be updated to manage our own call chains...
 */
-   dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge);
+   if (dsi->panel_bridge->funcs->post_disable)
+   dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge);
 
if (dsi->slave) {
dw_mipi_dsi_disable(dsi->slave);
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/bridge/synopsys: dsi: Wait for all active lanes to reach stop

2019-04-24 Thread Matt Redfearn
The Synopsys manual states that software should wait for all active
lanes to reach stop state (User manual section 3.1.5). Currently the
driver only waits for / checks that the clock lane is in stop state. Fix
this by waiting for the mask of PHY STATUS bits corresponding to the
active lanes to be set.

Signed-off-by: Matt Redfearn 

---

 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index bd15c21a177..38e88071363 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -189,6 +189,10 @@
 #define DSI_PHY_TX_TRIGGERS0xac
 
 #define DSI_PHY_STATUS 0xb0
+#define PHY_STOP_STATE_LANE_3  BIT(11)
+#define PHY_STOP_STATE_LANE_2  BIT(9)
+#define PHY_STOP_STATE_LANE_1  BIT(7)
+#define PHY_STOP_STATE_LANE_0  BIT(4)
 #define PHY_STOP_STATE_CLK_LANEBIT(2)
 #define PHY_LOCK   BIT(0)
 
@@ -752,7 +756,7 @@ static void dw_mipi_dsi_dphy_init(struct dw_mipi_dsi *dsi)
 
 static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
 {
-   u32 val;
+   u32 val, mask;
int ret;
 
dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
@@ -763,11 +767,16 @@ static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi 
*dsi)
if (ret)
DRM_DEBUG_DRIVER("failed to wait phy lock state\n");
 
+   mask = PHY_STOP_STATE_CLK_LANE | PHY_STOP_STATE_LANE_0;
+   mask |= (dsi->lanes >= 2) ? PHY_STOP_STATE_LANE_1 : 0;
+   mask |= (dsi->lanes >= 3) ? PHY_STOP_STATE_LANE_2 : 0;
+   mask |= (dsi->lanes == 4) ? PHY_STOP_STATE_LANE_3 : 0;
+
ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
-val, val & PHY_STOP_STATE_CLK_LANE, 1000,
+val, (val & mask) == mask, 1000,
 PHY_STATUS_TIMEOUT_US);
if (ret)
-   DRM_DEBUG_DRIVER("failed to wait phy clk lane stop state\n");
+   DRM_DEBUG_DRIVER("failed to wait phy stop state\n");
 }
 
 static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi)
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6 2/3] dt-bindings: backlight: add lm3630a bindings

2019-04-24 Thread Pavel Machek
Hi!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/lm3630a-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

I'm not a great fan on links to external websites. But this is comment
for Rob.

> +title: TI LM3630A High-Efficiency Dual-String White LED
> +
> +maintainers:
> +  - Lee Jones 
> +  - Daniel Thompson 
> +  - Jingoo Han 

Not a great fan of duplicating MAINTAINERS information here. But this
is a comment for Rob, again.

Acked-by: Pavel Machek 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/9] drm/ttm: Allow the driver to provide the ttm struct vm_operations_struct

2019-04-24 Thread Thomas Hellstrom
On Wed, 2019-04-24 at 14:10 +, Koenig, Christian wrote:
> Am 24.04.19 um 14:00 schrieb Thomas Hellstrom:
> > Add a pointer to the struct vm_operations_struct in the bo_device,
> > and
> > assign that pointer to the default value currently used.
> > 
> > The driver can then optionally modify that pointer and the new
> > value
> > can be used for each new vma created.
> > 
> > Cc: "Christian König" 
> > 
> > Signed-off-by: Thomas Hellstrom 
> > Reviewed-by: Christian König 
> 
> Going to pick those two TTM patches up for amd-staging-drm-next.

Will you be relying on either patch for related work? Otherwise it
would be simpler for us to use vmwgfx-next for the whole series,
targeting 5.3.

Thomas

> 
> Christian.
> 
> > ---
> >   drivers/gpu/drm/ttm/ttm_bo.c| 1 +
> >   drivers/gpu/drm/ttm/ttm_bo_vm.c | 6 +++---
> >   include/drm/ttm/ttm_bo_driver.h | 6 ++
> >   3 files changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> > b/drivers/gpu/drm/ttm/ttm_bo.c
> > index 3f56647cdb35..1c85bec00472 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > @@ -1656,6 +1656,7 @@ int ttm_bo_device_init(struct ttm_bo_device
> > *bdev,
> > mutex_lock(_global_mutex);
> > list_add_tail(>device_list, >device_list);
> > mutex_unlock(_global_mutex);
> > +   bdev->vm_ops = _bo_vm_ops;
> >   
> > return 0;
> >   out_no_sys:
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> > b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> > index e86a29a1e51f..bfb25b81fed7 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> > @@ -395,7 +395,7 @@ static int ttm_bo_vm_access(struct
> > vm_area_struct *vma, unsigned long addr,
> > return ret;
> >   }
> >   
> > -static const struct vm_operations_struct ttm_bo_vm_ops = {
> > +const struct vm_operations_struct ttm_bo_vm_ops = {
> > .fault = ttm_bo_vm_fault,
> > .open = ttm_bo_vm_open,
> > .close = ttm_bo_vm_close,
> > @@ -445,7 +445,7 @@ int ttm_bo_mmap(struct file *filp, struct
> > vm_area_struct *vma,
> > if (unlikely(ret != 0))
> > goto out_unref;
> >   
> > -   vma->vm_ops = _bo_vm_ops;
> > +   vma->vm_ops = bdev->vm_ops;
> >   
> > /*
> >  * Note: We're transferring the bo reference to
> > @@ -477,7 +477,7 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma,
> > struct ttm_buffer_object *bo)
> >   
> > ttm_bo_get(bo);
> >   
> > -   vma->vm_ops = _bo_vm_ops;
> > +   vma->vm_ops = bo->bdev->vm_ops;
> > vma->vm_private_data = bo;
> > vma->vm_flags |= VM_MIXEDMAP;
> > vma->vm_flags |= VM_IO | VM_DONTEXPAND;
> > diff --git a/include/drm/ttm/ttm_bo_driver.h
> > b/include/drm/ttm/ttm_bo_driver.h
> > index cbf3180cb612..cfeaff5d9706 100644
> > --- a/include/drm/ttm/ttm_bo_driver.h
> > +++ b/include/drm/ttm/ttm_bo_driver.h
> > @@ -443,6 +443,9 @@ extern struct ttm_bo_global {
> >* @driver: Pointer to a struct ttm_bo_driver struct setup by the
> > driver.
> >* @man: An array of mem_type_managers.
> >* @vma_manager: Address space manager
> > + * @vm_ops: Pointer to the struct vm_operations_struct used for
> > this
> > + * device's VM operations. The driver may override this before the
> > first
> > + * mmap() call.
> >* lru_lock: Spinlock that protects the buffer+device lru lists
> > and
> >* ddestroy lists.
> >* @dev_mapping: A pointer to the struct address_space
> > representing the
> > @@ -461,6 +464,7 @@ struct ttm_bo_device {
> > struct ttm_bo_global *glob;
> > struct ttm_bo_driver *driver;
> > struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES];
> > +   const struct vm_operations_struct *vm_ops;
> >   
> > /*
> >  * Protected by internal locks.
> > @@ -489,6 +493,8 @@ struct ttm_bo_device {
> > bool no_retry;
> >   };
> >   
> > +extern const struct vm_operations_struct ttm_bo_vm_ops;
> > +
> >   /**
> >* struct ttm_lru_bulk_move_pos
> >*
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 02/17] drm: Add |struct drm_gem_vram_object| callbacks for |struct ttm_bo_driver|

2019-04-24 Thread Koenig, Christian
Am 24.04.19 um 14:05 schrieb Thomas Zimmermann:
> Hi Christian,
>
> Am 24.04.19 um 13:48 schrieb Thomas Zimmermann:
>> +
>> +/*
>> + * Helpers for struct ttm_bo_driver
>> + */
>> +
>> +static bool drm_is_gem_vram(struct ttm_buffer_object *bo)
>> +{
>> +return (bo->destroy == ttm_buffer_object_destroy);
>> +}
>> +
>> +/**
>> + * drm_gem_vram_bo_driver_evict_flags() - \
>> +Implements  ttm_bo_driver.evict_flags
>> + * @bo: TTM buffer object. Refers to  drm_gem_vram_object.bo
>> + * @pl: TTM placement information.
>> + */
>> +void drm_gem_vram_bo_driver_evict_flags(struct ttm_buffer_object *bo,
>> +struct ttm_placement *pl)
>> +{
>> +struct drm_gem_vram_object *gbo;
>> +
>> +/* TTM may pass BOs that are not GEM VRAM BOs. */
>> +if (!drm_is_gem_vram(bo))
>> +return;
>> +
>> +gbo = drm_gem_vram_of_bo(bo);
>> +drm_gem_vram_placement(gbo, TTM_PL_FLAG_SYSTEM);
>> +*pl = gbo->placement;
>> +}
> For drm_is_gem_vram(), I'm not quite sure what else to test for. So
> there are still a few things I'd like to discuss.
>
>   1) If this test is about the placement flags, then it's unrelated to
> the actual DRM driver. All buffers of type |struct drm_gem_vram_object|
> share the same placement flags.
>
>   2) I tested the code to work with ast and mgag200.
>
>   3) If this test is really about individual instances of each DRM
> driver, then the current implementations are already broken. In this
> scenario TTM should sort out BOs with a BO device different from the one
> that triggered the eviction process; or pass the original BO device to
> the evict_flags callback, so we can sort out buffers here.

Just go ahead with the current approach for now. It could be that we see 
fallout, but that is really unlikely.

Christian.

>
> Best regards
> Thomas
>

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/9] drm/ttm: Allow the driver to provide the ttm struct vm_operations_struct

2019-04-24 Thread Koenig, Christian
Am 24.04.19 um 14:00 schrieb Thomas Hellstrom:
> Add a pointer to the struct vm_operations_struct in the bo_device, and
> assign that pointer to the default value currently used.
>
> The driver can then optionally modify that pointer and the new value
> can be used for each new vma created.
>
> Cc: "Christian König" 
>
> Signed-off-by: Thomas Hellstrom 
> Reviewed-by: Christian König 

Going to pick those two TTM patches up for amd-staging-drm-next.

Christian.

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c| 1 +
>   drivers/gpu/drm/ttm/ttm_bo_vm.c | 6 +++---
>   include/drm/ttm/ttm_bo_driver.h | 6 ++
>   3 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 3f56647cdb35..1c85bec00472 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1656,6 +1656,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>   mutex_lock(_global_mutex);
>   list_add_tail(>device_list, >device_list);
>   mutex_unlock(_global_mutex);
> + bdev->vm_ops = _bo_vm_ops;
>   
>   return 0;
>   out_no_sys:
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index e86a29a1e51f..bfb25b81fed7 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -395,7 +395,7 @@ static int ttm_bo_vm_access(struct vm_area_struct *vma, 
> unsigned long addr,
>   return ret;
>   }
>   
> -static const struct vm_operations_struct ttm_bo_vm_ops = {
> +const struct vm_operations_struct ttm_bo_vm_ops = {
>   .fault = ttm_bo_vm_fault,
>   .open = ttm_bo_vm_open,
>   .close = ttm_bo_vm_close,
> @@ -445,7 +445,7 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct 
> *vma,
>   if (unlikely(ret != 0))
>   goto out_unref;
>   
> - vma->vm_ops = _bo_vm_ops;
> + vma->vm_ops = bdev->vm_ops;
>   
>   /*
>* Note: We're transferring the bo reference to
> @@ -477,7 +477,7 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct 
> ttm_buffer_object *bo)
>   
>   ttm_bo_get(bo);
>   
> - vma->vm_ops = _bo_vm_ops;
> + vma->vm_ops = bo->bdev->vm_ops;
>   vma->vm_private_data = bo;
>   vma->vm_flags |= VM_MIXEDMAP;
>   vma->vm_flags |= VM_IO | VM_DONTEXPAND;
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index cbf3180cb612..cfeaff5d9706 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -443,6 +443,9 @@ extern struct ttm_bo_global {
>* @driver: Pointer to a struct ttm_bo_driver struct setup by the driver.
>* @man: An array of mem_type_managers.
>* @vma_manager: Address space manager
> + * @vm_ops: Pointer to the struct vm_operations_struct used for this
> + * device's VM operations. The driver may override this before the first
> + * mmap() call.
>* lru_lock: Spinlock that protects the buffer+device lru lists and
>* ddestroy lists.
>* @dev_mapping: A pointer to the struct address_space representing the
> @@ -461,6 +464,7 @@ struct ttm_bo_device {
>   struct ttm_bo_global *glob;
>   struct ttm_bo_driver *driver;
>   struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES];
> + const struct vm_operations_struct *vm_ops;
>   
>   /*
>* Protected by internal locks.
> @@ -489,6 +493,8 @@ struct ttm_bo_device {
>   bool no_retry;
>   };
>   
> +extern const struct vm_operations_struct ttm_bo_vm_ops;
> +
>   /**
>* struct ttm_lru_bulk_move_pos
>*

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  1   2   3   >