No subject

[Heliogabalo Santos Jugon]

hi,

I'm a new programmer, i just was wondering if i can get some docs to
start learning DRI. I googled a lot but i didn't find a starter
documentation. Some refs will be apreciated

thx

[Bug 98481] [Hawaii] Radeon kernel oops with vfio-pci

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98481

--- Comment #3 from Timothy Pearson  ---
Created attachment 127594
  --> https://bugs.freedesktop.org/attachment.cgi?id=127594=edit
Fix radeon kernel oops when used in vfio-based guest

The amdgpu driver doesn't look quite ready for production use on SI GPUs, but I
was able to work around the issue in the radeon driver via the attached patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/19698d05/attachment-0001.html>

[Bug 51381] [drm:atom_op_jump] *ERROR* atombios stuck in loop for more than 5secs aborting, when disabled via vgaswitcheroo

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=51381

--- Comment #50 from Alex Deucher  ---
Your system does not appear to support powerdown of the dGPU.  From your log:
[9.611183] ATPX version 1, functions 0x0181
Bit 1 of the functions should be set if it does.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 97993] amdgpu_query_gpu_info () segfaults when user does not have permission to open /dev/dri/card*

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=97993

--- Comment #2 from Alex Deucher  ---
Created attachment 127593
  --> https://bugs.freedesktop.org/attachment.cgi?id=127593=edit
possible fix

The patch should fix the acute problem, but may not fix the actual root cause
which is likely in the OCL driver.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/d0ce6ce6/attachment.html>

[Bug 94530] AMD R9 Nano reset problem

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=94530

--- Comment #4 from spam at madsitenet.de ---
It seems, it's not a vfio issue. There are (possibly) some firmware/hw
problems, as it happens also on bare metal Windows (at least on my machine):

https://community.amd.com/thread/204651

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/8f79f97f/attachment.html>

[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

[Christian König]

Am 28.10.2016 um 19:37 schrieb Mario Kleiner:
>
>
> On 10/28/2016 03:34 AM, Michel Dänzer wrote:
>> On 27/10/16 10:33 PM, Mike Lothian wrote:
>>>
>>> Just another gentle ping to see where you are with this?
>>
>> I haven't got a chance to look into this any further.
>>
>>
>
> Fwiw., as a proof of concept, the attached experimental patch does 
> work as tested on Intel HD Haswell + AMD R9 380 Tonga under amdgpu and 
> DRI3/Present when applied to drm-next (updated from a few days ago). 
> With DRI_PRIME=1 tearing for page-flipped fullscreen windows is gone 
> under all loads. The tearing with "windowed" windows now looks as 
> expected for regular tearing not related to Prime.

Yeah, that's pretty much what I had in mind as well. You additionally 
need to wait for the shared fences when you export the BO for the first 
time, but that's only a nitpick.

>
> ftrace confirms the i915 driver's pageflip function is waiting on the 
> fence in reservation_object_wait_timeout_rcu() as it should.
>
> That entry->tv.shared needs to be set false for such buffers in 
> amdgpu_bo_list_set() makes sense to me, as that is part of the buffer 
> validation for command stream submission. There are other places in 
> the driver where tv.shared is set, which i didn't check so far.
>
> I don't know which of these would need to be updated with a "exported 
> bo" check as well, e.g., for video decoding or maybe gpu compute? 
> Adding or removing the check to amdgpu_gem_va_update_vm(), e.g., made 
> no difference. I assume that makes sense because that functions seems 
> to deal with amdgpu internal vm page tables or page table entries for 
> such a bo, not with something visible to external clients?

Yes, exactly. VM updates doesn't matter for anybody else than amdgpu. 
Additional to that we don't even add a fence to the shared slot we 
reserve (could probably drop that for optimization).

Please remove the debugging stuff and the extra code on the VM updates 
and add a reservation_object_wait_timeout_rcu() to the export path and 
we should be good to go.

Regards,
Christian.

>
> All i can say is it fixes 3D rendering under DRI3 + Prime + 
> pageflipping without causing any obvious new problems.
>
> -mario

[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

[Mario Kleiner]

On 10/28/2016 03:34 AM, Michel Dänzer wrote:
> On 27/10/16 10:33 PM, Mike Lothian wrote:
>>
>> Just another gentle ping to see where you are with this?
>
> I haven't got a chance to look into this any further.
>
>

Fwiw., as a proof of concept, the attached experimental patch does work 
as tested on Intel HD Haswell + AMD R9 380 Tonga under amdgpu and 
DRI3/Present when applied to drm-next (updated from a few days ago). 
With DRI_PRIME=1 tearing for page-flipped fullscreen windows is gone 
under all loads. The tearing with "windowed" windows now looks as 
expected for regular tearing not related to Prime.

ftrace confirms the i915 driver's pageflip function is waiting on the 
fence in reservation_object_wait_timeout_rcu() as it should.

That entry->tv.shared needs to be set false for such buffers in 
amdgpu_bo_list_set() makes sense to me, as that is part of the buffer 
validation for command stream submission. There are other places in the 
driver where tv.shared is set, which i didn't check so far.

I don't know which of these would need to be updated with a "exported 
bo" check as well, e.g., for video decoding or maybe gpu compute? Adding 
or removing the check to amdgpu_gem_va_update_vm(), e.g., made no 
difference. I assume that makes sense because that functions seems to 
deal with amdgpu internal vm page tables or page table entries for such 
a bo, not with something visible to external clients?

All i can say is it fixes 3D rendering under DRI3 + Prime + pageflipping 
without causing any obvious new problems.

-mario
-- next part --
A non-text attachment was scrubbed...
Name: 0001-drm-amdgpu-Attach-exclusive-fence-to-prime-exported-.patch
Type: text/x-patch
Size: 3024 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/840350f6/attachment-0001.bin>

[PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2

[Jean-Francois Moine]

On Fri, 28 Oct 2016 00:03:16 +0200
Maxime Ripard  wrote:

> On Tue, Oct 25, 2016 at 04:14:41PM +0200, Jean-Francois Moine wrote:
> > > > +Display controller
> > > > +==
> > > > +
> > > > +Required properties:
> > > > +
> > > > +- compatible: value should be one of the following
> > > > +   "allwinner,sun8i-a83t-display-engine"
> > > > +   "allwinner,sun8i-h3-display-engine"
> > > > +
> > > > +- clocks: must include clock specifiers corresponding to entries in the
> > > > +   clock-names property.
> > > > +
> > > > +- clock-names: must contain
> > > > +   "gate": for DE activation
> > > > +   "clock": DE clock
> > > 
> > > We've been calling them bus and mod.
> > 
> > I can understand "bus" (which is better than "apb"), but why "mod"?
> 
> Allwinner has been calling the clocks that are supposed to generate
> the external signals (depending on where you were looking) module or
> mod clocks (which is also why we have mod in the clock
> compatibles). The module 1 clocks being used for the audio and the
> module 0 for the rest (SPI, MMC, NAND, display, etc.)

I did not find any 'module' in the H3 documentation.
So, is it really a good name?

> > > > +
> > > > +- resets: phandle to the reset of the device
> > > > +
> > > > +- ports: phandle's to the LCD ports
> > > 
> > > Please use the OF graph.
> > 
> > These ports are references to the graph of nodes. See
> > http://www.kernelhub.org/?msg=911825=2
> 
> In an OF-graph, your phandle to the LCD controller would be replaced
> by an output endpoint.

This is the DE controller. There is no endpoint link at this level.
The Device Engine just handles the planes of the LCDs, but, indeed,
the LCDs must know about the DE and the DE must know about the LCDs.
There are 2 ways to realize this knowledge in the DT:
1) either the DE has one or two phandle's to the LCDs,
2) or the LCDs have a phandle to the DE.

I chose the 1st way, the DE ports pointing to the endpoint of the LCDs
which is part of the video link (OF-graph LCD <-> connector).
It would be possible to have phandles to the LCDs themselves, but this
asks for more code.

The second way is also possible, but it also complexifies a bit the
exchanges DE <-> LCD.

> > [snip]
> > > > +struct tcon {
> > > > +   u32 gctl;
> > > > +#defineTCON_GCTL_TCON_En BIT(31)
[snip]
> > > > +   u32 fill_ctl;   /* 0x300 */
> > > > +   u32 fill_start0;
> > > > +   u32 fill_end0;
> > > > +   u32 fill_data0;
> > > > +};
> > > 
> > > Please use defines instead of the structures.
> > 
> > I think that structures are more readable.
> 
> That's not really the point. No one in the kernel uses it (and even
> you use defines for registers offset in some places of that
> patch). And then you have André arguments.

I am not convinced, but I'll do as you said.

> > > > +void de2_disable_vblank(struct drm_device *drm, unsigned crtc)
> > > > +{
> > > > +   struct priv *priv = drm->dev_private;
> > > > +   struct lcd *lcd = priv->lcds[crtc];
> > > > +
> > > > +   tcon_write(lcd->mmio, gint0,
> > > > +tcon_read(lcd->mmio, gint0) &
> > > > +   ~TCON_GINT0_TCON1_Vb_Int_En);
> > > > +}
> > > > +
> > > > +/* panel functions */
> > > 
> > > Panel functions? In the CRTC driver?
> > 
> > Yes, dumb panel.
> 
> What do you mean by that? Using a Parallel/RGB interface?

Sorry, I though this was a well-known name. The 'dump panel' was used
in the documentation of my previous ARM machine as the video frame sent
to the HDMI controller. 'video_frame' is OK for you?

[snip]
> > > > +   ret = clk_prepare_enable(lcd->clk);
> > > > +   if (ret)
> > > > +   goto err2;
> > > 
> > > Is there any reason not to do that in the enable / disable? Leaving
> > > clocks running while the device has no guarantee that it's going to be
> > > used seems like a waste of resources.
> > 
> > If the machine does not need video (network server, router..), it is simpler
> > to prevent the video driver to be loaded (DT, module black list...).
> 
> You might not have control on any of it, or you might just have no
> monitor attached for example. Recompiling the kernel or updating the
> DT when you want to plug an HDMI monitor seems like a poor UX :)

OK, I will check if this works.

> > > > +static const struct {
> > > > +   char chan;
> > > > +   char layer;
> > > > +   char pipe;
> > > > +} plane2layer[DE2_N_PLANES] = {
> > > > +   [DE2_PRIMARY_PLANE] =   {0, 0, 0},
> > > > +   [DE2_CURSOR_PLANE] ={1, 0, 1},
> > > > +   [DE2_VI_PLANE] ={0, 1, 0},
> > > > +};
> > > 
> > > Comments?
> > 
> > This
> > primary plane is channel 0 (VI), layer 0, pipe 0
> > cursor plane is channel 1 (UI), layer 0, pipe 1
> > overlay plane is channel 0 (VI), layer 1, pipe 0
> > or the full explanation:
> > 

[Bug 91880] Radeonsi on Grenada cards (r9 390) exceptionally unstable and poorly performing

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=91880

--- Comment #122 from Lauri Gustafsson  ---
Seconded that 4.8 kernel on Arch has fixed the unstability. Still getting
rather bad artifacts from time to time because of the dynamic memory clock.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/792ccfe5/attachment.html>

[Bug 98481] [Hawaii] Radeon kernel oops with vfio-pci

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98481

--- Comment #2 from Alex Deucher  ---
Most hypervisors do not provide access to pci config registers which the driver
needs access to to determine what pcie speeds are available.  That said, this
should work better with amdgpu and the 4.10-wip kernel:
https://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-next-4.10-wip

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/a862cead/attachment.html>

[Bug 98481] [Hawaii] Radeon kernel oops with vfio-pci

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98481

--- Comment #1 from Timothy Pearson  ---
This appears related:

https://bbs.archlinux.org/viewtopic.php?pid=1537645#p1537645

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/f952e019/attachment.html>

[Bug 98481] [Hawaii] Radeon kernel oops with vfio-pci

[bugzilla-dae...@freedesktop.org]

radeon]
[ 1298.918548] [c00fe123f410] [db3be15c]
drm_dev_register+0xfc/0x140 [drm]
[ 1298.918555] [c00fe123f450] [db3c0798] drm_get_pci_dev+0xf8/0x220
[drm]
[ 1298.918574] [c00fe123f4e0] [db8f0664]
radeon_pci_probe+0x134/0x1b0 [radeon]
[ 1298.918580] [c00fe123f570] [c050471c] local_pci_probe+0x6c/0x140
[ 1298.918583] [c00fe123f600] [c05055a8]
pci_device_probe+0x168/0x200
[ 1298.918588] [c00fe123f660] [c05b4900]
driver_probe_device+0x240/0x550
[ 1298.918591] [c00fe123f6f0] [c05b4d7c]
__driver_attach+0x16c/0x170
[ 1298.918595] [c00fe123f770] [c05b100c]
bus_for_each_dev+0x9c/0x110
[ 1298.918598] [c00fe123f7c0] [c05b3b5c] driver_attach+0x3c/0x60
[ 1298.918626] [c00fe123f7f0] [c05b33a8] bus_add_driver+0x308/0x390
[ 1298.918630] [c00fe123f880] [c05b5d1c] driver_register+0x9c/0x180
[ 1298.918633] [c00fe123f8f0] [c05038dc]
__pci_register_driver+0x6c/0x90
[ 1298.918640] [c00fe123f930] [db3c0a24] drm_pci_init+0x164/0x1b0
[drm]
[ 1298.918659] [c00fe123f9c0] [dba53440] radeon_init+0xc8/0xf8
[radeon]
[ 1298.918663] [c00fe123fa30] [c000b74c] do_one_initcall+0x6c/0x1d0
[ 1298.918667] [c00fe123faf0] [c0822b50] do_init_module+0x94/0x254
[ 1298.918671] [c00fe123fb80] [c01821b0] load_module+0x2380/0x2b30
[ 1298.918674] [c00fe123fd30] [c0182ca0]
SyS_finit_module+0xf0/0x170
[ 1298.918678] [c00fe123fe30] [c0009560] system_call+0x38/0x108
[ 1298.918680] Instruction dump:
[ 1298.918682] f821ff81 7c7e1b78 7c9f2378 4808 e8410018 3920 913f
e93e01e8
[ 1298.918688] 2fa9 41de0044 e9290010 ebc90038  2b891106 419e0030
2b891166
[ 1298.918696] ---[ end trace 6c565dbe73743fb5 ]---

Setting dpm=0 allows the radeon driver to load normally and the card functions
as expected under Xorg.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/7c702ea6/attachment-0001.html>

[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

[Mike Lothian]

Hi Mario

That fixes the tearing, it's been replaced with a strange stutter, like
it's only showing half the number of frames being reported - it's really
noticeable in tomb raider

Thanks for your work on this, the stutter is much more manageable than the
tearing was

I've attached the patch that applies cleanly to 4.10-wip



On Fri, 28 Oct 2016 at 18:37 Mario Kleiner 
wrote:

>
>
> On 10/28/2016 03:34 AM, Michel Dänzer wrote:
> > On 27/10/16 10:33 PM, Mike Lothian wrote:
> >>
> >> Just another gentle ping to see where you are with this?
> >
> > I haven't got a chance to look into this any further.
> >
> >
>
> Fwiw., as a proof of concept, the attached experimental patch does work
> as tested on Intel HD Haswell + AMD R9 380 Tonga under amdgpu and
> DRI3/Present when applied to drm-next (updated from a few days ago).
> With DRI_PRIME=1 tearing for page-flipped fullscreen windows is gone
> under all loads. The tearing with "windowed" windows now looks as
> expected for regular tearing not related to Prime.
>
> ftrace confirms the i915 driver's pageflip function is waiting on the
> fence in reservation_object_wait_timeout_rcu() as it should.
>
> That entry->tv.shared needs to be set false for such buffers in
> amdgpu_bo_list_set() makes sense to me, as that is part of the buffer
> validation for command stream submission. There are other places in the
> driver where tv.shared is set, which i didn't check so far.
>
> I don't know which of these would need to be updated with a "exported
> bo" check as well, e.g., for video decoding or maybe gpu compute? Adding
> or removing the check to amdgpu_gem_va_update_vm(), e.g., made no
> difference. I assume that makes sense because that functions seems to
> deal with amdgpu internal vm page tables or page table entries for such
> a bo, not with something visible to external clients?
>
> All i can say is it fixes 3D rendering under DRI3 + Prime + pageflipping
> without causing any obvious new problems.
>
> -mario
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/73d5e496/attachment.html>
-- next part --
A non-text attachment was scrubbed...
Name: 0001-drm-amdgpu-Attach-exclusive-fence-to-prime-exported-.patch
Type: text/x-patch
Size: 2235 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/73d5e496/attachment.bin>

[PATCH v2 2/4] drm/i915: Set link status property for DP connector

[Manasi Navare]

This defines a helper function to set the property value.
This will be used to set the link status to Bad in case
of link training failures.

v2:
* Simplify the return value (Jani Nikula)

Cc: dri-devel at lists.freedesktop.org
Cc: Jani Nikula 
Cc: Daniel Vetter 
Cc: Ville Syrjala 
Cc: Chris Wilson 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/i915/intel_dp.c  | 11 +++
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1063afe..2b6f51c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4640,6 +4640,17 @@ static int intel_dp_get_modes(struct drm_connector 
*connector)
return 0;
 }

+int
+intel_dp_set_link_status_property(struct drm_connector *connector,
+ uint64_t val)
+{
+   struct drm_device *dev = connector->dev;
+
+   return drm_object_property_set_value(>base,
+
dev->mode_config.link_status_property,
+val);
+}
+
 static int
 intel_dp_connector_register(struct drm_connector *connector)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2616d92..3cb7481 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1391,6 +1391,8 @@ u32 skl_plane_stride(const struct drm_framebuffer *fb, 
int plane,
 bool intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port 
port);
 bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 struct intel_connector *intel_connector);
+int intel_dp_set_link_status_property(struct drm_connector *connector,
+ uint64_t val);
 void intel_dp_set_link_params(struct intel_dp *intel_dp,
  int link_rate, uint8_t lane_count,
  bool link_mst);
-- 
1.9.1

[PATCH v2 1/4] drm: Add a new connector property for link status

[Manasi Navare]

A new default connector property is added for keeping
track of whether the link is good (link training passed) or
link is bad (link training  failed). If the link status property
is not good, then userspace should fire off a new modeset at the current
mode even if there have not been any changes in the mode list
or connector status.
Also add link status connector member corersponding to the
decoded value of link status property.

v3: Add link status member to store property value locally
(Ville Syrjala)
v2:
* Make this a default connector property (Daniel Vetter)

Cc: dri-devel at lists.freedesktop.org
Cc: Jani Nikula 
Cc: Daniel Vetter 
Cc: Ville Syrjala 
Cc: Chris Wilson 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/drm_connector.c | 17 +
 include/drm/drm_connector.h |  4 +++-
 include/drm/drm_crtc.h  |  5 +
 include/uapi/drm/drm_mode.h |  4 
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2db7fb5..d4e852f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
drm_object_attach_property(>base,
  config->dpms_property, 0);

+   drm_object_attach_property(>base,
+  config->link_status_property,
+  0);
+
if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
drm_object_attach_property(>base, 
config->prop_crtc_id, 0);
}
@@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum 
subpixel_order order)
 };
 DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)

+static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
+   { DRM_MODE_LINK_STATUS_GOOD, "Good" },
+   { DRM_MODE_LINK_STATUS_BAD, "Bad" },
+};
+DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
+
 /**
  * drm_display_info_set_bus_formats - set the supported bus formats
  * @info: display info to store bus formats in
@@ -622,6 +632,13 @@ int drm_connector_create_standard_properties(struct 
drm_device *dev)
return -ENOMEM;
dev->mode_config.tile_property = prop;

+   prop = drm_property_create_enum(dev, 0, "link-status",
+   drm_link_status_enum_list,
+   ARRAY_SIZE(drm_link_status_enum_list));
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.link_status_property = prop;
+
return 0;
 }

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ac9d7d8..90387a1 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -682,6 +682,9 @@ struct drm_connector {
uint8_t num_h_tile, num_v_tile;
uint8_t tile_h_loc, tile_v_loc;
uint16_t tile_h_size, tile_v_size;
+
+   /* Connector Link status for link training */
+   bool link_status;
 };

 #define obj_to_connector(x) container_of(x, struct drm_connector, base)
@@ -754,7 +757,6 @@ int drm_mode_create_tv_properties(struct drm_device *dev,
 int drm_mode_create_scaling_mode_property(struct drm_device *dev);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
 int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
-
 int drm_mode_connector_set_path_property(struct drm_connector *connector,
 const char *path);
 int drm_mode_connector_set_tile_property(struct drm_connector *connector);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fa1aa21..b86ca19 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1151,6 +1151,11 @@ struct drm_mode_config {
 */
struct drm_property *tile_property;
/**
+* @link_status_property: Default connector property for link status
+* of a connector as a result of link training.
+*/
+   struct drm_property *link_status_property;
+   /**
 * @plane_type_property: Default plane property to differentiate
 * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
 */
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 084b50a..f1b0afd 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -121,6 +121,10 @@
 #define DRM_MODE_DIRTY_ON   1
 #define DRM_MODE_DIRTY_ANNOTATE 2

+/* Link Status options */
+#define DRM_MODE_LINK_STATUS_GOOD  0
+#define DRM_MODE_LINK_STATUS_BAD   1
+
 struct drm_mode_modeinfo {
__u32 clock;
__u16 hdisplay;
-- 
1.9.1

[PATCH v5] drm/fsl-dcu: Implement gamma_lut atomic crtc properties

[Stefan Agner]

Hi Meng,

On 2016-09-28 01:24, Meng Yi wrote:
> Gamma correction is optional and can be used to adjust the color
> output values to match the gamut of a particular TFT LCD panel
> 
> Split the DCU regs into "regs", "palette", "gamma" and "cursor".
> Create a second regmap for gamma memory space using little endian.
> The registers after the first address space are not accessed yet,
> hence new device trees would even work with old kernels. Just new
> kernel need the new format so we can access the separate gamma
> reg space.
> 
> Suggested-by: Stefan Agner 
> Signed-off-by: Meng Yi 

How did you actually test that? I have a hard time to get anything
useable with this code.

The display looks completely borked (colors are way off, and at random
either too dark or too bright).

I then also added Gamma 1.0 (and different values) to the Monitor
section of xorg.conf, but still not really usable.

I looked a bit more in depth into it, and some questions appeared, see
below:

> ---
> Changes since V1:
> -created a second regmap for gamma
> -updated the DCU DT binding
> -removed Kconfig for gamma and enable gamma when valid data filled.
> -extended and simplified comment lines.
> ---
>  .../devicetree/bindings/display/fsl,dcu.txt| 12 +++-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 33 
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c  | 35 
> +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h  |  7 +
>  4 files changed, 85 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/fsl,dcu.txt
> b/Documentation/devicetree/bindings/display/fsl,dcu.txt
> index 63ec2a6..8140b5d 100644
> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt
> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt
> @@ -6,6 +6,12 @@ Required properties:
>   * "fsl,vf610-dcu".
>  
>  - reg:   Address and length of the register set for dcu.
> + Must contain four address/length tuples:
> + 1. Register address space
> + 2. Palette/Tile address space
> + 3. Gamma address space
> + 4. Cursor address space
> +- reg-names: Should be "regs", "palette", "gamma" and "cursor"
>  - clocks:Handle to "dcu" and "pix" clock (in the order below)
>   This can be the same clock (e.g. LS1021a)
>   See ../clocks/clock-bindings.txt for details.
> @@ -20,7 +26,11 @@ Optional properties:
>  Examples:
>  dcu: dcu at 2ce {
>   compatible = "fsl,ls1021a-dcu";
> - reg = <0x0 0x2ce 0x0 0x1>;
> + reg = <0x0 0x2ce 0x0 0x2000>,
> +   <0x0 0x2ce2000 0x0 0x2000>,
> +   <0x0 0x2ce4000 0x0 0xc00>,
> +   <0x0 0x2ce4c00 0x0 0x400>;
> + reg-names = "regs", "palette", "gamma", "cursor";
>   clocks = <_clk 0>, <_clk 0>;
>   clock-names = "dcu", "pix";
>   big-endian;
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> index 3371635..6371e4d 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> @@ -22,6 +22,31 @@
>  #include "fsl_dcu_drm_drv.h"
>  #include "fsl_dcu_drm_plane.h"
>  
> +static void fsl_crtc_gamma_set(struct drm_crtc *crtc, struct
> drm_color_lut *lut,
> +   uint32_t size)
> +{
> + struct fsl_dcu_drm_device *fsl_dev = crtc->dev->dev_private;
> + unsigned int i;
> +
> + if (crtc->state->gamma_lut->data) {
> + for (i = 0; i < size; i++) {
> + regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_R + 4 * i,
> +  lut[i].red);
> + regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_G + 4 * i,
> +  lut[i].green);
> + regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_B + 4 * i,
> +  lut[i].blue);

I think you should not use the color values directly. They are also too
precise for DCU, DCU only support 8 bit.

You can use drm_color_lut_extract(.., 8) to get the 8-bit precision of
the LUT. See also:

https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html?highlight=gamma_lut#c.drm_color_lut_extract


> + }
> +
> + regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
> +DCU_MODE_EN_GAMMA_MASK,
> +DCU_MODE_GAMMA_ENABLE);
> + } else {
> + regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
> +DCU_MODE_EN_GAMMA_MASK, 0);
> + }
> +}
> +
>  static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> struct drm_crtc_state *old_crtc_state)
>  {
> @@ -37,6 +62,10 @@ static void fsl_dcu_drm_crtc_atomic_flush(struct
> drm_crtc *crtc,
>

[PATCH] drm/rockchip: analogix_dp: add supports for regulators in edp IP

[Randy Li]

On 10/28/2016 05:11 PM, Shawn Lin wrote:
> On 2016/10/23 3:18, Randy Li wrote:
>> I found if eDP_AVDD_1V0 and eDP_AVDD_1V8 are not been power at
>> RK3288, once trying to enable the pclk clock, the kernel would dead.
>> This patch would try to enable them first. The eDP_AVDD_1V8 more
>> likely to be applied to eDP phy, but I have no time to confirmed
>> it yet.
>
> Comfirm it or at least someone should be able to answer your
> question, Mark?
I just forget to ask the IC department, the TRM didn't cover that.
>
> Have you considered to add some details about vcc-supply and vccio-
> supply for your analogix_dp-rockchip.txt ?
>
> From your commit msg, these two properties are more likely to be
> required but the code itself tell me them should be optional(from the
> point of backward compatibility, it should also be optinoal).
Yes, I keep it optional for the same reason. Most of boards won't turn 
off those power supply and may use some fixed regulators.
>
>>
>> Signed-off-by: Randy Li 
>> ---
>>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 25
>> +
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> index 8548e82..6bf0441 100644
>> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> @@ -17,6 +17,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>
>> @@ -70,6 +71,7 @@ struct rockchip_dp_device {
>>  struct clk   *grfclk;
>>  struct regmap*grf;
>>  struct reset_control *rst;
>> +struct regulator_bulk_data supplies[2];
>>
>>  struct work_struct psr_work;
>>  spinlock_t psr_lock;
>> @@ -146,6 +148,13 @@ static int rockchip_dp_poweron(struct
>> analogix_dp_plat_data *plat_data)
>>
>>  cancel_work_sync(>psr_work);
>>
>> +ret = regulator_bulk_enable(ARRAY_SIZE(dp->supplies),
>> +dp->supplies);
>> +if (ret) {
>> +dev_err(dp->dev, "failed to enable vdd supply %d\n", ret);
>> +return ret;
>> +}
>> +
>>  ret = clk_prepare_enable(dp->pclk);
>>  if (ret < 0) {
>>  dev_err(dp->dev, "failed to enable pclk %d\n", ret);
>> @@ -168,6 +177,9 @@ static int rockchip_dp_powerdown(struct
>> analogix_dp_plat_data *plat_data)
>>
>>  clk_disable_unprepare(dp->pclk);
>>
>> +regulator_bulk_disable(ARRAY_SIZE(dp->supplies),
>> +dp->supplies);
>> +
>>  return 0;
>>  }
>>
>> @@ -323,6 +335,19 @@ static int rockchip_dp_init(struct
>> rockchip_dp_device *dp)
>>  return PTR_ERR(dp->rst);
>>  }
>>
>> +dp->supplies[0].supply = "vcc";
>> +dp->supplies[1].supply = "vccio";
>> +ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dp->supplies),
>> +dp->supplies);
>> +if (ret < 0) {
>> +dev_err(dev, "failed to get regulators: %d\n", ret);
>> +}
>> +ret = regulator_bulk_enable(ARRAY_SIZE(dp->supplies),
>> +dp->supplies);
>> +if (ret < 0) {
>> +dev_err(dev, "failed to enable regulators: %d\n", ret);
>> +}
>> +
>>  ret = clk_prepare_enable(dp->pclk);
>>  if (ret < 0) {
>>  dev_err(dp->dev, "failed to enable pclk %d\n", ret);
>>
>
>

-- 
Randy Li
The third produce department
===
This email message, including any attachments, is for the sole
use of the intended recipient(s) and may contain confidential and
privileged information. Any unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient, please
contact the sender by reply e-mail and destroy all copies of the original
message. [Fuzhou Rockchip Electronics, INC. China mainland]
===

[Bug 97993] amdgpu_query_gpu_info () segfaults when user does not have permission to open /dev/dri/card*

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=97993

--- Comment #1 from Vedran Miletić  ---
Still occurs with 16.40.

Simple test:

$ ls -la /dev/dri/* # verify that other does not have rwx
$ sudo chgrp root /dev/dri/* # or any group which the user is not a member of
$ clinfo

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/34b67f2a/attachment.html>

[PATCH] drm/rockchip: analogix_dp: add supports for regulators in edp IP

[Shawn Lin]

On 2016/10/23 3:18, Randy Li wrote:
> I found if eDP_AVDD_1V0 and eDP_AVDD_1V8 are not been power at
> RK3288, once trying to enable the pclk clock, the kernel would dead.
> This patch would try to enable them first. The eDP_AVDD_1V8 more
> likely to be applied to eDP phy, but I have no time to confirmed
> it yet.

Comfirm it or at least someone should be able to answer your
question, Mark?

Have you considered to add some details about vcc-supply and vccio-
supply for your analogix_dp-rockchip.txt ?

 From your commit msg, these two properties are more likely to be
required but the code itself tell me them should be optional(from the
point of backward compatibility, it should also be optinoal).

>
> Signed-off-by: Randy Li 
> ---
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 25 
> +
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 8548e82..6bf0441 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -70,6 +71,7 @@ struct rockchip_dp_device {
>   struct clk   *grfclk;
>   struct regmap*grf;
>   struct reset_control *rst;
> + struct regulator_bulk_data supplies[2];
>
>   struct work_struct   psr_work;
>   spinlock_t   psr_lock;
> @@ -146,6 +148,13 @@ static int rockchip_dp_poweron(struct 
> analogix_dp_plat_data *plat_data)
>
>   cancel_work_sync(>psr_work);
>
> + ret = regulator_bulk_enable(ARRAY_SIZE(dp->supplies),
> + dp->supplies);
> + if (ret) {
> + dev_err(dp->dev, "failed to enable vdd supply %d\n", ret);
> + return ret;
> + }
> +
>   ret = clk_prepare_enable(dp->pclk);
>   if (ret < 0) {
>   dev_err(dp->dev, "failed to enable pclk %d\n", ret);
> @@ -168,6 +177,9 @@ static int rockchip_dp_powerdown(struct 
> analogix_dp_plat_data *plat_data)
>
>   clk_disable_unprepare(dp->pclk);
>
> + regulator_bulk_disable(ARRAY_SIZE(dp->supplies),
> + dp->supplies);
> +
>   return 0;
>  }
>
> @@ -323,6 +335,19 @@ static int rockchip_dp_init(struct rockchip_dp_device 
> *dp)
>   return PTR_ERR(dp->rst);
>   }
>
> + dp->supplies[0].supply = "vcc";
> + dp->supplies[1].supply = "vccio";
> + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dp->supplies),
> + dp->supplies);
> + if (ret < 0) {
> + dev_err(dev, "failed to get regulators: %d\n", ret);
> + }
> + ret = regulator_bulk_enable(ARRAY_SIZE(dp->supplies),
> + dp->supplies);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable regulators: %d\n", ret);
> + }
> +
>   ret = clk_prepare_enable(dp->pclk);
>   if (ret < 0) {
>   dev_err(dp->dev, "failed to enable pclk %d\n", ret);
>


-- 
Best Regards
Shawn Lin

[PATCH] drm/mediatek: fix null pointer dereference

[CK Hu]

Hi, Matthias:

Even though OVL HW would not be enabled before component_add() in
current design, your patch would be safe for any situation.

Acked-by CK Hu 

Regards,
CK

On Wed, 2016-10-26 at 16:09 +0200, Matthias Brugger wrote:
> The probe function requests the interrupt before initializing
> the ddp component. Which leads to a null pointer dereference at boot.
> Fix this by requesting the interrput after all components got
> initialized properly.
> 
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC
> MT8173.")
> Signed-off-by: Matthias Brugger 
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c 
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 019b7ca..1e78159 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -250,13 +250,6 @@ static int mtk_disp_ovl_probe(struct platform_device 
> *pdev)
>   if (irq < 0)
>   return irq;
>  
> - ret = devm_request_irq(dev, irq, mtk_disp_ovl_irq_handler,
> -IRQF_TRIGGER_NONE, dev_name(dev), priv);
> - if (ret < 0) {
> - dev_err(dev, "Failed to request irq %d: %d\n", irq, ret);
> - return ret;
> - }
> -
>   comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DISP_OVL);
>   if (comp_id < 0) {
>   dev_err(dev, "Failed to identify by alias: %d\n", comp_id);
> @@ -272,6 +265,13 @@ static int mtk_disp_ovl_probe(struct platform_device 
> *pdev)
>  
>   platform_set_drvdata(pdev, priv);
>  
> + ret = devm_request_irq(dev, irq, mtk_disp_ovl_irq_handler,
> +IRQF_TRIGGER_NONE, dev_name(dev), priv);
> + if (ret < 0) {
> + dev_err(dev, "Failed to request irq %d: %d\n", irq, ret);
> + return ret;
> + }
> +
>   ret = component_add(dev, _disp_ovl_component_ops);
>   if (ret)
>   dev_err(dev, "Failed to add component: %d\n", ret);

[Bug 94530] AMD R9 Nano reset problem

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=94530

--- Comment #3 from Alex Deucher  ---
Please try the latest 4.9 kernel or my 4.10-wip branch:
https://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-next-4.10-wip

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/72987591/attachment.html>

[PATCH v8 02/12] drm/i915: Add i915 perf infrastructure

[Matthew Auld]

> +/* Note we copy the properties from userspace outside of the i915 perf
> + * mutex to avoid an awkward lockdep with mmap_sem.
> + *
> + * Note this function only validates properties in isolation it doesn't
> + * validate that the combination of properties makes sense or that all
> + * properties necessary for a particular kind of stream have been set.
> + */
> +static int read_properties_unlocked(struct drm_i915_private *dev_priv,
> +   u64 __user *uprops,
> +   u32 n_props,
> +   struct perf_open_properties *props)
> +{
> +   u64 __user *uprop = uprops;
> +   int i;
> +
> +   memset(props, 0, sizeof(struct perf_open_properties));
> +
> +   if (!n_props) {
> +   DRM_ERROR("No i915 perf properties given");
> +   return -EINVAL;
> +   }
> +
> +   if (n_props > DRM_I915_PERF_PROP_MAX) {
Ah but DRM_I915_PERF_PROP_MAX is not a property itself.

[PATCH 1/7] drm: Define a work struct for scheduling a uevent for modeset retry

[Jani Nikula]

On Fri, 28 Oct 2016, Jani Nikula  wrote:
> On Thu, 27 Oct 2016, Manasi Navare  wrote:
>> This work struct will be used to schedule a uevent on a separate
>> thread. This will be scheduled after a link train failure during modeset
>> to indicate a modeset retry request. It will get executed after the
>> current modeset is complete and all locks are released. This was
>> required to avoid deadlock.
>>
>> v2:
>> * Create a generic work func not i915 specific (Daniel Vetter)
>>
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: Jani Nikula 
>> Cc: Daniel Vetter 
>> Cc: Ville Syrjala 
>>
>> Signed-off-by: Manasi Navare 
>> ---
>>  include/drm/drm_connector.h | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index ac9d7d8..fc9d475 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -682,6 +682,11 @@ struct drm_connector {
>>  uint8_t num_h_tile, num_v_tile;
>>  uint8_t tile_h_loc, tile_v_loc;
>>  uint16_t tile_h_size, tile_v_size;
>> +
>> +/* Work struct to schedule a uevent on link train failure for
>> + * DisplayPort.
>> + */
>> +struct work_struct modeset_retry_work;
>
> I think at least for now we should keep this in intel_connector. All the
> code using this will be in i915.

Oh, and you can just squash that in the first patch that uses it,
i.e. patch 7/7 in this series.


>
> BR,
> Jani.
>
>>  };
>>  
>>  #define obj_to_connector(x) container_of(x, struct drm_connector, base)

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH 3/7] drm/i915: Set link status property for DP connector

[Jani Nikula]

On Thu, 27 Oct 2016, Manasi Navare  wrote:
> This defines a helper function to set the property value.
> This will be used to set the link status to Bad in case
> of link training failures.
>
> Cc: dri-devel at lists.freedesktop.org
> Cc: Jani Nikula 
> Cc: Daniel Vetter 
> Cc: Ville Syrjala 
> Cc: Chris Wilson 
> Signed-off-by: Manasi Navare 
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 16 
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  2 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3c2293b..795897c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4673,6 +4673,22 @@ static int intel_dp_get_modes(struct drm_connector 
> *connector)
>   return 0;
>  }
>  
> +int
> +intel_dp_set_link_status_property(struct drm_connector *connector,
> +   uint64_t val)
> +{
> + struct drm_device *dev = connector->dev;
> + int ret = 0;
> +
> + ret = drm_object_property_set_value(>base,
> + 
> dev->mode_config.link_status_property,
> + val);
> + if (ret)
> + return ret;
> +
> + return ret;

That's a difficult way to say

return drm_object_property_set_value(...);

> +}
> +
>  static int
>  intel_dp_connector_register(struct drm_connector *connector)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 7dda79d..f6fe05a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1397,6 +1397,8 @@ u32 skl_plane_stride(const struct drm_framebuffer *fb, 
> int plane,
>  bool intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port 
> port);
>  bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>struct intel_connector *intel_connector);
> +int intel_dp_set_link_status_property(struct drm_connector *connector,
> +   uint64_t val);
>  void intel_dp_set_link_params(struct intel_dp *intel_dp,
> int link_rate, uint8_t lane_count,
> bool link_mst);

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH 1/7] drm: Define a work struct for scheduling a uevent for modeset retry

[Jani Nikula]

On Thu, 27 Oct 2016, Manasi Navare  wrote:
> This work struct will be used to schedule a uevent on a separate
> thread. This will be scheduled after a link train failure during modeset
> to indicate a modeset retry request. It will get executed after the
> current modeset is complete and all locks are released. This was
> required to avoid deadlock.
>
> v2:
> * Create a generic work func not i915 specific (Daniel Vetter)
>
> Cc: dri-devel at lists.freedesktop.org
> Cc: Jani Nikula 
> Cc: Daniel Vetter 
> Cc: Ville Syrjala 
>
> Signed-off-by: Manasi Navare 
> ---
>  include/drm/drm_connector.h | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ac9d7d8..fc9d475 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -682,6 +682,11 @@ struct drm_connector {
>   uint8_t num_h_tile, num_v_tile;
>   uint8_t tile_h_loc, tile_v_loc;
>   uint16_t tile_h_size, tile_v_size;
> +
> + /* Work struct to schedule a uevent on link train failure for
> +  * DisplayPort.
> +  */
> + struct work_struct modeset_retry_work;

I think at least for now we should keep this in intel_connector. All the
code using this will be in i915.

BR,
Jani.

>  };
>  
>  #define obj_to_connector(x) container_of(x, struct drm_connector, base)

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH] drm/amdgpu: release parent fence reference

[Christian König]

Am 28.10.2016 um 13:47 schrieb Christian König:
> Am 24.10.2016 um 11:43 schrieb Grazvydas Ignotas:
>> On Mon, Oct 24, 2016 at 12:13 PM, Christian König
>>  wrote:
>>> Am 24.10.2016 um 04:25 schrieb zhoucm1:


 On 2016年10月24日 02:31, Grazvydas Ignotas wrote:
> It's done in amd_sched_hw_job_reset(), but not in normal job 
> processing.
> Leak reported by CONFIG_SLUB_DEBUG.
>
> Signed-off-by: Grazvydas Ignotas 
> ---
>CONFIG_SLUB_DEBUG reports more leaks related to ioctls,
>but I was unable to track them down...
>
>drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 ++
>1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> index 910b8d5..cfb686e 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> @@ -522,6 +522,8 @@ static void amd_sched_process_job(struct fence 
> *f,
> struct fence_cb *cb)
>  trace_amd_sched_process_job(s_fence);
>fence_put(_fence->finished);
> +fence_put(s_fence->parent);
 If I remember correctly, parent was put in sched fence release.
>>>
>>> Yes, exactly. It's only released in amd_sched_hw_job_reset() because 
>>> we want
>>> to assign a new parent fence.
>> Well then it doesn't work, SLUB_DEBUG detects leaks on teardown, and
>> with my patch they're gone.
>> Perhaps the problem is that when new parent fence is assigned, the old
>> one is not put? I won't be able to check until the weekend, so if
>> anybody else can do it, please go ahead.
>
> Mhm, what steps do you do to reproduce this?

Never mind, I was able to figure out how to trigger it. You actually 
have to create a fence to leak it :)

Regards,
Christian.

>
> I'm looking into this a bit and it sounds like we just doesn't 
> correctly tear down the scheduler on driver unload.
>
> Regards,
> Christian.
>
>>
>> Gražvydas
>
>

[PATCH v4 2/2] drm/panel: simple: add support for Sharp LQ150X1LG11 panels

[Peter Rosin]

On 2016-10-19 14:31, Thierry Reding wrote:
> On Wed, Oct 19, 2016 at 02:27:50PM +0200, Thierry Reding wrote:
>> On Tue, Oct 04, 2016 at 05:29:21PM +0200, Peter Rosin wrote:
>>> From: Gustaf Lindström 
>>>
>>> The Sharp 15" LQ150X1LG11 panel is an XGA TFT LCD panel.
>>>
>>> The simple-panel driver is used to get support for essential
>>> functionality of the panel.
>>>
>>> Signed-off-by: Gustaf Lindström 
>>> Signed-off-by: Peter Rosin 
>>> ---
>>>  drivers/gpu/drm/panel/panel-simple.c | 27 +++
>>>  1 file changed, 27 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
>>> b/drivers/gpu/drm/panel/panel-simple.c
>>> index 85143d1b9b31..58cfe0a7a9d6 100644
>>> --- a/drivers/gpu/drm/panel/panel-simple.c
>>> +++ b/drivers/gpu/drm/panel/panel-simple.c
>>> @@ -1386,6 +1386,30 @@ static const struct panel_desc sharp_lq123p1jx31 = {
>>> },
>>>  };
>>>  
>>> +static const struct drm_display_mode sharp_lq150x1lg11_mode = {
>>> +   .clock = 71100,
>>> +   .hdisplay = 1024,
>>> +   .hsync_start = 1024 + 168,
>>> +   .hsync_end = 1024 + 168 + 64,
>>> +   .htotal = 1024 + 168 + 64 + 88,
>>> +   .vdisplay = 768,
>>> +   .vsync_start = 768 + 37,
>>> +   .vsync_end = 768 + 37 + 2,
>>> +   .vtotal = 768 + 37 + 2 + 8,
>>> +   .vrefresh = 60,
>>> +};
>>> +
>>> +static const struct panel_desc sharp_lq150x1lg11 = {
>>> +   .modes = _lq150x1lg11_mode,
>>> +   .num_modes = 1,
>>> +   .bpc = 8,
>>> +   .size = {
>>> +   .width = 304,
>>> +   .height = 228,
>>> +   },
>>> +   .bus_format = MEDIA_BUS_FMT_RGB565_1X16,
>>
>> This doesn't match the .bpc = 8 above. You'd usually use a .bpc = 6 for
>> RGB565 panels.
> 
> Are you actually using the .bpc field in your driver? If yes, can you
> please test if things work correctly if you set it to 6? If it's unused
> I'm leaning towards just applying this with .bpc set to 6 and hope for
> the best (and fix it if somebody finds that it's broken).
> 
> So no need to resend, just let me know what you think.

Sorry for the delay, but I just tested .bpc = 6 and it was ok too. It
doesn't seem to matter for us, so do as you like.

Cheers,
Peter

[drm/qxl 6/6] qxl: Allow resolution which are not multiple of 8

[Christophe Fergeau]

The use of drm_cvt_mode() in qxl_add_monitors_config_modes() means that
the resolutions we are going to present to user-space are going to be
rounded down to a multiple of 8. In the QXL arbitrary resolution case,
this is not useful.
This commit forces the actual width/height that was requested by the
client in the drm_display_mode structure rather than keeping the
rounded version.

Signed-off-by: Christophe Fergeau 
---

I know this is very hacky, but I have no idea what is important to be set in 
the mode struct,
if there is a better way to create it without getting the rounding to a 
multiple of 8, ...


 drivers/gpu/drm/qxl/qxl_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c 
b/drivers/gpu/drm/qxl/qxl_display.c
index edb90f6..fc5b01e 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -202,6 +202,9 @@ static int qxl_add_monitors_config_modes(struct 
drm_connector *connector,
mode = drm_cvt_mode(dev, head->width, head->height, 60, false, false,
false);
mode->type |= DRM_MODE_TYPE_PREFERRED;
+   mode->hdisplay = head->width;
+   mode->vdisplay = head->height;
+   drm_mode_set_name(mode);
*pwidth = head->width;
*pheight = head->height;
drm_mode_probed_add(connector, mode);
-- 
2.9.3

[drm/qxl 5/6] qxl: Don't notify userspace when monitors config is unchanged

[Christophe Fergeau]

When the QXL driver receives a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt,
we currently always notify userspace that there was some hotplug event.

However, gnome-shell/mutter is reacting to this event by attempting a
resolution change, which it does by issueing drmModeRmFB, drmModeAddFB,
and then drmModeSetCrtc. This has the side-effect of causing
qxl_crtc_mode_set() to tell the QXL virtual hardware that a primary
surface was destroyed and created. After going through QEMU and then the
remote SPICE client, a new identical monitors config message will be
sent, resulting in a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt to
be emitted, and the same scenario occurring again.

As destroying/creating the primary surface causes a visible screen
flicker, this makes the guest hard to use (
https://bugzilla.redhat.com/show_bug.cgi?id=1266484 ).

This commit checks if the screen configuration we received is the same
one as the current one, and does not notify userspace about it if that's
the case.

Signed-off-by: Christophe Fergeau 
---
 drivers/gpu/drm/qxl/qxl_display.c | 65 +--
 1 file changed, 55 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c 
b/drivers/gpu/drm/qxl/qxl_display.c
index 156b7de..edb90f6 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -57,11 +57,18 @@ static void qxl_alloc_client_monitors_config(struct 
qxl_device *qdev, unsigned c
qdev->client_monitors_config->count = count;
 }

+enum MonitorsConfigCopyStatus {
+   MONITORS_CONFIG_COPIED,
+   MONITORS_CONFIG_UNCHANGED,
+   MONITORS_CONFIG_BAD_CRC,
+};
+
 static int qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
 {
int i;
int num_monitors;
uint32_t crc;
+   bool changed = false;

num_monitors = qdev->rom->client_monitors_config.count;
crc = crc32(0, (const uint8_t *)>rom->client_monitors_config,
@@ -70,7 +77,7 @@ static int qxl_display_copy_rom_client_monitors_config(struct 
qxl_device *qdev)
qxl_io_log(qdev, "crc mismatch: have %X (%zd) != %X\n", crc,
   sizeof(qdev->rom->client_monitors_config),
   qdev->rom->client_monitors_config_crc);
-   return 1;
+   return MONITORS_CONFIG_BAD_CRC;
}
if (num_monitors > qdev->monitors_config->max_allowed) {
DRM_DEBUG_KMS("client monitors list will be truncated: %d < 
%d\n",
@@ -79,6 +86,10 @@ static int 
qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
} else {
num_monitors = qdev->rom->client_monitors_config.count;
}
+   if (qdev->client_monitors_config
+ && (num_monitors != qdev->client_monitors_config->count)) {
+   changed = true;
+   }
qxl_alloc_client_monitors_config(qdev, num_monitors);
/* we copy max from the client but it isn't used */
qdev->client_monitors_config->max_allowed =
@@ -88,17 +99,42 @@ static int 
qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
>rom->client_monitors_config.heads[i];
struct qxl_head *client_head =
>client_monitors_config->heads[i];
-   client_head->x = c_rect->left;
-   client_head->y = c_rect->top;
-   client_head->width = c_rect->right - c_rect->left;
-   client_head->height = c_rect->bottom - c_rect->top;
-   client_head->surface_id = 0;
-   client_head->id = i;
-   client_head->flags = 0;
+   if (client_head->x != c_rect->left) {
+   client_head->x = c_rect->left;
+   changed = true;
+   }
+   if (client_head->y != c_rect->top) {
+   client_head->y = c_rect->top;
+   changed = true;
+   }
+   if (client_head->width != c_rect->right - c_rect->left) {
+   client_head->width = c_rect->right - c_rect->left;
+   changed = true;
+   }
+   if (client_head->height != c_rect->bottom - c_rect->top) {
+   client_head->height = c_rect->bottom - c_rect->top;
+   changed = true;
+   }
+   if (client_head->surface_id != 0) {
+   client_head->surface_id = 0;
+   changed = true;
+   }
+   if (client_head->id != i) {
+   client_head->id = i;
+   changed = true;
+   }
+   if (client_head->flags != 0) {
+   client_head->flags = 0;
+   changed = true;
+   }
DRM_DEBUG_KMS("read %dx%d+%d+%d\n", client_head->width, 
client_head->height,
 

[drm/qxl 4/6] qxl: Call qxl_gem_{init,fini}

[Christophe Fergeau]

qdev->gem.objects was initialized directly in qxl_device_init() rather
than going through qxl_gem_init(), and qxl_gem_fini() was never called.

Signed-off-by: Christophe Fergeau 
---
 drivers/gpu/drm/qxl/qxl_kms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index e642242..af685f1 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -131,7 +131,7 @@ static int qxl_device_init(struct qxl_device *qdev,
mutex_init(>update_area_mutex);
mutex_init(>release_mutex);
mutex_init(>surf_evict_mutex);
-   INIT_LIST_HEAD(>gem.objects);
+   qxl_gem_init(qdev);

qdev->rom_base = pci_resource_start(pdev, 2);
qdev->rom_size = pci_resource_len(pdev, 2);
@@ -273,6 +273,7 @@ static void qxl_device_fini(struct qxl_device *qdev)
qxl_ring_free(qdev->command_ring);
qxl_ring_free(qdev->cursor_ring);
qxl_ring_free(qdev->release_ring);
+   qxl_gem_fini(qdev);
qxl_bo_fini(qdev);
io_mapping_free(qdev->surface_mapping);
io_mapping_free(qdev->vram_mapping);
-- 
2.9.3

[drm/qxl 3/6] qxl: Add missing '\n' to qxl_io_log() call

[Christophe Fergeau]

The message has to be terminated by a newline as it's not going to get
added automatically.

Signed-off-by: Christophe Fergeau 
---
 drivers/gpu/drm/qxl/qxl_fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index 2cd879a..0d16107 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -197,7 +197,7 @@ static int qxlfb_framebuffer_dirty(struct drm_framebuffer 
*fb,
/*
 * we are using a shadow draw buffer, at qdev->surface0_shadow
 */
-   qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]", clips->x1, clips->x2,
+   qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]\n", clips->x1, clips->x2,
   clips->y1, clips->y2);
image->dx = clips->x1;
image->dy = clips->y1;
-- 
2.9.3

[drm/qxl 2/6] qxl: Remove unused prototype

[Christophe Fergeau]

qxl_crtc_set_from_monitors_config() is defined in qxl_drv.h but never
implemented.

Signed-off-by: Christophe Fergeau 
---
 drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index da41e1f..590ba25 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -398,9 +398,6 @@ void qxl_display_read_client_monitors_config(struct 
qxl_device *qdev);
 int qxl_create_monitors_object(struct qxl_device *qdev);
 int qxl_destroy_monitors_object(struct qxl_device *qdev);

-/* used by qxl_debugfs only */
-void qxl_crtc_set_from_monitors_config(struct qxl_device *qdev);
-
 /* qxl_gem.c */
 int qxl_gem_init(struct qxl_device *qdev);
 void qxl_gem_fini(struct qxl_device *qdev);
-- 
2.9.3

[drm/qxl 1/6] qxl: Mark some internal functions as static

[Christophe Fergeau]

They are not used outside of their respective source file

Signed-off-by: Christophe Fergeau 
---
 drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
 drivers/gpu/drm/qxl/qxl_display.c | 4 ++--
 drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 04270f5..74fc936 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -578,7 +578,7 @@ int qxl_hw_surface_dealloc(struct qxl_device *qdev,
return 0;
 }

-int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf)
+static int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf)
 {
struct qxl_rect rect;
int ret;
diff --git a/drivers/gpu/drm/qxl/qxl_display.c 
b/drivers/gpu/drm/qxl/qxl_display.c
index a61c0d4..156b7de 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -36,7 +36,7 @@ static bool qxl_head_enabled(struct qxl_head *head)
return head->width && head->height;
 }

-void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned count)
+static void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned 
count)
 {
if (qdev->client_monitors_config &&
count > qdev->client_monitors_config->count) {
@@ -607,7 +607,7 @@ static bool qxl_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
 }

-void
+static void
 qxl_send_monitors_config(struct qxl_device *qdev)
 {
int i;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 5f3e5ad..da41e1f 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -395,13 +395,11 @@ qxl_framebuffer_init(struct drm_device *dev,
 struct drm_gem_object *obj,
 const struct drm_framebuffer_funcs *funcs);
 void qxl_display_read_client_monitors_config(struct qxl_device *qdev);
-void qxl_send_monitors_config(struct qxl_device *qdev);
 int qxl_create_monitors_object(struct qxl_device *qdev);
 int qxl_destroy_monitors_object(struct qxl_device *qdev);

 /* used by qxl_debugfs only */
 void qxl_crtc_set_from_monitors_config(struct qxl_device *qdev);
-void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned count);

 /* qxl_gem.c */
 int qxl_gem_init(struct qxl_device *qdev);
@@ -574,6 +572,5 @@ int qxl_bo_check_id(struct qxl_device *qdev, struct qxl_bo 
*bo);
 struct qxl_drv_surface *
 qxl_surface_lookup(struct drm_device *dev, int surface_id);
 void qxl_surface_evict(struct qxl_device *qdev, struct qxl_bo *surf, bool 
freeing);
-int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf);

 #endif
-- 
2.9.3

[drm/qxl 0/6] Various cleanups/fixes

[Christophe Fergeau]

Hey,

This patch series starts by doing some various small cleanups (patch 1 to 4), 
and
then there are 2 fixes for issues which were noticed in fedora 25, the more 
annoying one
being the one fixed by patch 5 where the VM screen would constantly flicker
when wayland is used. Patch 6 is very hacky, I'm not familiar at all with that 
code,
so I don't know what is the correct way of doing it.

Christophe

[PATCH 2/2] drm: tilcdc: Correct misspelling in error message

[Daniel Schultz]

This error message will be printed when a FIFO underflow irq has
triggered. Since this happens sometimes and the error message will be
displayed on the console, it should have a correct spelling.

Signed-off-by: Daniel Schultz 
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index fe1d088..63caed4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -769,7 +769,7 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
}

if (stat & LCDC_FIFO_UNDERFLOW)
-   dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underfow",
+   dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underflow",
__func__, stat);

/* For revision 2 only */
-- 
1.9.1

[PATCH 1/2] drm: tilcdc: Add revision handling for FB_CEILING

[Daniel Schultz]

The commit d8ff0c63fbcb ("drm/tilcdc: Adjust the FB_CEILING address")
added an adjustment of the FB_CEILING address. This is done by decrementing
the address by one.

On the AM335x (rev 0x4F201000) the framebuffer is rotated left over the
display border, because the ceiling address is 8f276fff instead of
8f277000. Since this adjustment isn't necessary for the LCDC v2, the
origin ceiling address should be used.

Signed-off-by: Daniel Schultz 
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 788999e..fe1d088 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -69,6 +69,7 @@ static void set_scanout(struct drm_crtc *crtc, struct 
drm_framebuffer *fb)
 {
struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
struct drm_device *dev = crtc->dev;
+   struct tilcdc_drm_private *priv = dev->dev_private;
struct drm_gem_cma_object *gem;
unsigned int depth, bpp;
dma_addr_t start, end;
@@ -88,7 +89,10 @@ static void set_scanout(struct drm_crtc *crtc, struct 
drm_framebuffer *fb)
 * unlikely that LCDC would fetch the DMA addresses in the middle of
 * an update.
 */
-   dma_base_and_ceiling = (u64)(end - 1) << 32 | start;
+   if (priv->rev == 1)
+   end -= 1;
+
+   dma_base_and_ceiling = (u64)end << 32 | start;
tilcdc_write64(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, dma_base_and_ceiling);

if (tilcdc_crtc->curr_fb)
-- 
1.9.1

[PATCH] drm/amdgpu: release parent fence reference

[Christian König]

Am 24.10.2016 um 11:43 schrieb Grazvydas Ignotas:
> On Mon, Oct 24, 2016 at 12:13 PM, Christian König
>  wrote:
>> Am 24.10.2016 um 04:25 schrieb zhoucm1:
>>>
>>>
>>> On 2016年10月24日 02:31, Grazvydas Ignotas wrote:
 It's done in amd_sched_hw_job_reset(), but not in normal job processing.
 Leak reported by CONFIG_SLUB_DEBUG.

 Signed-off-by: Grazvydas Ignotas 
 ---
CONFIG_SLUB_DEBUG reports more leaks related to ioctls,
but I was unable to track them down...

drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 ++
1 file changed, 2 insertions(+)

 diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
 b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
 index 910b8d5..cfb686e 100644
 --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
 +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
 @@ -522,6 +522,8 @@ static void amd_sched_process_job(struct fence *f,
 struct fence_cb *cb)
  trace_amd_sched_process_job(s_fence);
fence_put(_fence->finished);
 +fence_put(s_fence->parent);
>>> If I remember correctly, parent was put in sched fence release.
>>
>> Yes, exactly. It's only released in amd_sched_hw_job_reset() because we want
>> to assign a new parent fence.
> Well then it doesn't work, SLUB_DEBUG detects leaks on teardown, and
> with my patch they're gone.
> Perhaps the problem is that when new parent fence is assigned, the old
> one is not put? I won't be able to check until the weekend, so if
> anybody else can do it, please go ahead.

Mhm, what steps do you do to reproduce this?

I'm looking into this a bit and it sounds like we just doesn't correctly 
tear down the scheduler on driver unload.

Regards,
Christian.

>
> Gražvydas

[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=97879

--- Comment #39 from Stefano Cipriani  ---
Playing against bots with an AMD r9 m265x (cape verde) using DRI_PRIME=1
rendered via Intel HD Graphics 4600 the game it's not playable for about 30+
seconds, bot cars start to one every 5~10 seconds. When playing online I get
the hangs for about the same time.

I made a trace of an offline match with the git version of apitrace, and I can
replay it with both git and stable version. You can find it at

https://drive.google.com/open?id=0B41ZZ4daaP9hTmNDaUl2MHFWQlE

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/69385b59/attachment.html>

[Bug 63941] Can't control brightness with ATI 5650/Intel Ironlake when switched to radeon

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=63941

Jani Nikula  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |OBSOLETE

--- Comment #7 from Jani Nikula  ---
Really old bug, closing.

Please file any remaining drm/i915 bugs at
https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH] drm/mediatek: fix null pointer dereference

[Matthias Brugger]

On 10/28/2016 10:34 AM, CK Hu wrote:
> Hi, Matthias:
>
> Even though OVL HW would not be enabled before component_add() in
> current design, your patch would be safe for any situation.

Maybe the FW I use left an interrupt pending before loading the kernel 
and this leads to the case where we enter the handler before all data 
structures are set up.

>
> Acked-by CK Hu 

Thanks!

Matthias

[Bug 178281] wine-staging apps freezes the machine with RX460

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=178281

--- Comment #15 from fin4478 at hotmail.com ---
New Mesa did not fix this. Playing at fullhd TR Legend still hangs xserver in 3
minutes. Tried 2 times, same effect. Works as good as with an 300Hz overclocked
non debug 4.9-rc2 kernel from kernel.org, where Unigine Heaven windows
benchmark runs ok. 

I do not know why my first gameplay after updating mesa was long and did not
hang xserver.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH] drm/tegra: dpaux: Fix error handling

[Christophe JAILLET]

'devm_pinctrl_register()' returns an error pointer or a valid handle. So
checking for NULL here is pointless and can never trigger.

Check the returned value with IS_ERR instead and propagate this value as
done in the other functions which call 'devm_pinctrl_register()'

Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support")

Signed-off-by: Christophe JAILLET 
---
 drivers/gpu/drm/tegra/dpaux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 059f409556d5..2fde44c3a1b3 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -539,9 +539,9 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
dpaux->desc.owner = THIS_MODULE;

dpaux->pinctrl = devm_pinctrl_register(>dev, >desc, dpaux);
-   if (!dpaux->pinctrl) {
+   if (IS_ERR(dpaux->pinctrl)) {
dev_err(>dev, "failed to register pincontrol\n");
-   return -ENODEV;
+   return PTR_ERR(dpaux->pinctrl);
}
 #endif
/* enable and clear all interrupts */
-- 
2.9.3

[git pull] drm/x86 pat regression fix.

[Dave Airlie]

On 26 October 2016 at 16:53, Dave Airlie  wrote:
> Hi Linus,
>
> This is a standalone pull request for the fix for a regression introduced
> in -rc1 by a change to vm_insert_mixed to start using the PAT range tracking
> to validate page protections. With this fix in place, all the VRAM mappings
> for GPU drivers ended up at UC instead of WC.
>
> There are probably better ways to fix this long term, but nothing I'd 
> considered
> for -fixes that wouldn't need more settling in time. So I've just created a 
> new
> arch API that the drivers can reserve all their VRAM aperture ranges as WC.
>
Hey Linus,

was there some problem with this? I've not seen it land yet. I've been
blocking more fixes on this landing.

Dave.

[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

[Michel Dänzer]

On 27/10/16 10:33 PM, Mike Lothian wrote:
> 
> Just another gentle ping to see where you are with this?

I haven't got a chance to look into this any further.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

[PATCH v6 4/6] drm/fence: add in-fences support

[Gustavo Padovan]

2016-10-28 Daniel Vetter :

> On Thu, Oct 27, 2016 at 05:37:09PM -0200, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > There is now a new property called FENCE_FD attached to every plane
> > state that receives the sync_file fd from userspace via the atomic commit
> > IOCTL.
> > 
> > The fd is then translated to a fence (that may be a fence_collection
> > subclass or just a normal fence) and then used by DRM to fence_wait() for
> > all fences in the sync_file to signal. So it only commits when all
> > framebuffers are ready to scanout.
> > 
> > v2: Comments by Daniel Vetter:
> > - remove set state->fence = NULL in destroy phase
> > - accept fence -1 as valid and just return 0
> > - do not call fence_get() - sync_file_fences_get() already calls it
> > - fence_put() if state->fence is already set, in case userspace
> > set the property more than once.
> > 
> > v3: WARN_ON if fence is set but state has no FB
> > 
> > v4: Comment from Maarten Lankhorst
> > - allow set fence with no related fb
> > 
> > v5: rename FENCE_FD to IN_FENCE_FD
> > 
> > v6: Comments by Daniel Vetter:
> > - rename plane_state->in_fence back to "fence"
> > 
> >  - rebase after fence -> dma_fence rename
> > 
> > Signed-off-by: Gustavo Padovan 
> 
> Looks good, but I'll wait with full review with all the fence patches
> until the igts show up. It's much easier to check for gabs in input
> validation code if you also have the testcases at hand. A few comments
> below.
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/Kconfig |  1 +
> >  drivers/gpu/drm/drm_atomic.c| 15 +++
> >  drivers/gpu/drm/drm_atomic_helper.c |  5 +++--
> >  drivers/gpu/drm/drm_crtc.c  |  6 ++
> >  drivers/gpu/drm/drm_plane.c |  1 +
> >  include/drm/drm_crtc.h  |  5 +
> >  6 files changed, 31 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 483059a..43cb33d 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -12,6 +12,7 @@ menuconfig DRM
> > select I2C
> > select I2C_ALGOBIT
> > select DMA_SHARED_BUFFER
> > +   select SYNC_FILE
> > help
> >   Kernel-level support for the Direct Rendering Infrastructure (DRI)
> >   introduced in XFree86 4.0. If you say Y here, you need to select
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 5e73954..28d9366 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -30,6 +30,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include "drm_crtc_internal.h"
> >  
> > @@ -686,6 +687,17 @@ int drm_atomic_plane_set_property(struct drm_plane 
> > *plane,
> > drm_atomic_set_fb_for_plane(state, fb);
> > if (fb)
> > drm_framebuffer_unreference(fb);
> > +   } else if (property == config->prop_in_fence_fd) {
> > +   if (U642I64(val) == -1)
> > +   return 0;
> > +
> > +   if (state->fence)
> > +   dma_fence_put(state->fence);
> > +
> > +   state->fence = sync_file_get_fence(val);
> > +   if (!state->fence)
> > +   return -EINVAL;
> > +
> > } else if (property == config->prop_crtc_id) {
> > struct drm_crtc *crtc = drm_crtc_find(dev, val);
> > return drm_atomic_set_crtc_for_plane(state, crtc);
> > @@ -747,6 +759,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> >  
> > if (property == config->prop_fb_id) {
> > *val = (state->fb) ? state->fb->base.id : 0;
> > +   } else if (property == config->prop_in_fence_fd) {
> > +   *val = -1;
> > } else if (property == config->prop_crtc_id) {
> > *val = (state->crtc) ? state->crtc->base.id : 0;
> > } else if (property == config->prop_crtc_x) {
> > @@ -1752,6 +1766,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > drm_mode_object_unreference(obj);
> > }
> >  
> > +
> 
> Spurios whitespace.
> 
> > if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
> > for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > struct drm_pending_vblank_event *e;
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 75ad01d..c34da9e 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1034,8 +1034,6 @@ int drm_atomic_helper_wait_for_fences(struct 
> > drm_device *dev,
> > if (!plane_state->fence)
> > continue;
> >  
> > -   WARN_ON(!plane_state->fb);
> > -
> 
> Why this? We don't allow a plane to be enabled without an fb, and adding a
> fence to a plane which is disabled sounds likea  bug. We probably should
> have a bit of code in the core atomic check code to make sure userspace
> never asks for a fence when the plane 

[PATCH v6 6/6] drm/fence: add out-fences support

[Brian Starkey]

Hi Gustavo,

On Thu, Oct 27, 2016 at 05:37:11PM -0200, Gustavo Padovan wrote:
>From: Gustavo Padovan 
>
>Support DRM out-fences by creating a sync_file with a fence for each CRTC
>that sets the OUT_FENCE_PTR property.
>
>We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
>the sync_file fd back to userspace.
>
>The sync_file and fd are allocated/created before commit, but the
>fd_install operation only happens after we know that commit succeed.
>
>v2: Comment by Rob Clark:
>   - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
>
>Comment by Daniel Vetter:
>   - Add clean up code for out_fences
>
>v3: Comments by Daniel Vetter:
>   - create DRM_MODE_ATOMIC_EVENT_MASK
>   - userspace should fill out_fences_ptr with the crtc_ids for which
>   it wants fences back.
>
>v4: Create OUT_FENCE_PTR properties and remove old approach.
>
>v5: Comments by Brian Starkey:
>   - Remove extra fence_get() in atomic_ioctl()
>   - Check ret before iterating on the crtc_state
>   - check ret before fd_install
>   - set fence_state to NULL at the beginning
>   - check fence_state->out_fence_ptr before put_user()
>   - change order of fput() and put_unused_fd() on failure
>
> - Add access_ok() check to the out_fence_ptr received
> - Rebase after fence -> dma_fence rename
> - Store out_fence_ptr in the drm_atomic_state
> - Split crtc_setup_out_fence()
> - return -1 as out_fence with TEST_ONLY flag
>
>Signed-off-by: Gustavo Padovan 
>---
> drivers/gpu/drm/drm_atomic.c | 199 ---
> drivers/gpu/drm/drm_crtc.c   |   8 ++
> include/drm/drm_atomic.h |   1 +
> include/drm/drm_crtc.h   |  17 
> 4 files changed, 196 insertions(+), 29 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index 28d9366..9b70a27 100644
>--- a/drivers/gpu/drm/drm_atomic.c
>+++ b/drivers/gpu/drm/drm_atomic.c
>@@ -290,6 +290,46 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>
> /**
>+ * drm_atomic_set_out_fence_for_crtc - set CRTC out fence pointer
>+ * @state: global atomic state object
>+ * @crtc: crtc to set the out fence pointer
>+ * @fence_ptr: the userspace pointer to user
>+ *
>+ * This function stores the out fence pointer in the atomic state.
>+ */
>+static void
>+drm_atomic_set_out_fence_for_crtc(struct drm_atomic_state *state,
>+struct drm_crtc *crtc, u64 __user *fence_ptr)

bikeshed: I'd rather these were called set/get_out_fence_ptr_for_crtc,
as they don't involve struct drm_fence at all.

>+{
>+  state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
>+}
>+
>+/**
>+ * drm_atomic_get_out_fence_for_crtc - get CRTC out fence pointer
>+ * @state: global atomic state object
>+ * @crtc: crtc to set the out fence pointer
>+ *
>+ * Get the user pointer that should be used for to store the out fence fd.
>+ * This function should be called only once per atomic state as it clears
>+ * the out_fence_ptr store there to prevent drivers to access them.
>+ *
>+ * Returns:
>+ *
>+ * The out fence user pointer.
>+ */
>+static u64 __user *
>+drm_atomic_get_out_fence_for_crtc(struct drm_atomic_state *state,
>+   struct drm_crtc *crtc)
>+{
>+  u64 __user *fence_ptr;
>+
>+  fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
>+  state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
>+
>+  return fence_ptr;
>+}
>+
>+/**
>  * drm_atomic_set_mode_for_crtc - set mode for CRTC
>  * @state: the CRTC whose incoming state to update
>  * @mode: kernel-internal mode to use for the CRTC, or NULL to disable
>@@ -490,6 +530,12 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   );
>   state->color_mgmt_changed |= replaced;
>   return ret;
>+  } else if (property == config->prop_out_fence_ptr) {
>+  uint64_t __user *fence_ptr = u64_to_user_ptr(val);
>+  if (!access_ok(VERIFY_WRITE, fence_ptr, sizeof(*fence_ptr)))
>+  return -EFAULT;
>+
>+  drm_atomic_set_out_fence_for_crtc(state->state, crtc, 
>fence_ptr);
>   } else if (crtc->funcs->atomic_set_property)
>   return crtc->funcs->atomic_set_property(crtc, state, property, 
> val);
>   else
>@@ -532,6 +578,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>   *val = (state->ctm) ? state->ctm->base.id : 0;
>   else if (property == config->gamma_lut_property)
>   *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>+  else if (property == config->prop_out_fence_ptr)
>+  *val = 0;
>   else if (crtc->funcs->atomic_get_property)
>   return crtc->funcs->atomic_get_property(crtc, state, property, 
> val);
>   else
>@@ -1506,11 +1554,9 @@ EXPORT_SYMBOL(drm_atomic_nonblocking_commit);

imx-drm hang issue with etnaviv (GC3000)

[Lucas Stach]

Am Donnerstag, den 27.10.2016, 19:26 +0200 schrieb Wladimir J. van der
Laan:
> Hello,
> 
> After running kmscube (or another KMS executable) on a i.MX6 QuadPlus 
> (etnaviv,
> GC3000) a few times on I get the below crash in the drm kernel driver.
> This is on a device with LVDS panel. It is always reproducible, although the
> number of invocations needed differs.
> 
> The only way to get rendering to work again after the crash is to reboot.
> Repeated tries only get the "flip_done timed out".
> 
> This always happens while the program is exiting.
> 
> Versions:
> 
> - mesa: https://github.com/etnaviv/mesa 9a09984
> 
> - libdrm: https://cgit.freedesktop.org/mesa/drm/ fe4579e
> 
> - Kernel: 4.8.0 or 4.8.4 + Pengutronix patches (20161007).
> 
> Does anyone have an idea what could be the problem?
> 
I think I've seen this problem a few times already. I'll have a look at
this today.

Regards,
Lucas

> Regards,
> Wladimir van der Laan
> 
> [  130.026973] [ cut here ]
> [  130.031630] WARNING: CPU: 1 PID: 222 at 
> drivers/gpu/drm/drm_atomic_helper.c:1127 
> drm_atomic_helper_wait_for_vblanks+0x1e4/0x200
> [  130.043149] [CRTC:24] vblank wait timed out
> [  130.047367] Modules linked in: hid_generic usbhid hid ci_hdrc_imx ci_hdrc 
> extcon_core ehci_hcd usbcore usb_common usbmisc_imx coda videobuf2_vmalloc
> [  130.060915] CPU: 1 PID: 222 Comm: kmscube Not tainted 4.8.4+ #1
> [  130.066844] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [  130.073378] Backtrace: 
> [  130.075863] [<8010b6c0>] (dump_backtrace) from [<8010b908>] 
> (show_stack+0x20/0x24)
> [  130.083439]  r7:80b441d8 r6:600c0013 r5: r4:80b441d8
> [  130.089187] [<8010b8e8>] (show_stack) from [<803a33fc>] 
> (dump_stack+0x78/0x94)
> [  130.096422] [<803a3384>] (dump_stack) from [<8011a9b8>] (__warn+0xdc/0x110)
> [  130.103389]  r7:0009 r6:8042fda4 r5: r4:ed47fd08
> [  130.109132] [<8011a8dc>] (__warn) from [<8011aa34>] 
> (warn_slowpath_fmt+0x48/0x50)
> [  130.116620]  r9: r8:ee1b9418 r7:edc76600 r6: r5:edffc500 
> r4:
> [  130.124470] [<8011a9f0>] (warn_slowpath_fmt) from [<8042fda4>] 
> (drm_atomic_helper_wait_for_vblanks+0x1e4/0x200)
> [  130.134563]  r3:0018 r2:80898166
> [  130.138194] [<8042fbc0>] (drm_atomic_helper_wait_for_vblanks) from 
> [<80457fc8>] (imx_drm_atomic_commit_tail+0x58/0x68)
> [  130.148895]  r10:8086686b r9:ee1b923c r8:003f r7:80b6bf22 r6: 
> r5:ee1b9000
> [  130.156823]  r4:edffc500
> [  130.159390] [<80457f70>] (imx_drm_atomic_commit_tail) from [<804323e8>] 
> (commit_tail+0x4c/0x68)
> [  130.168094]  r5:80b4a014 r4:edffc500
> [  130.171719] [<8043239c>] (commit_tail) from [<8043249c>] 
> (drm_atomic_helper_commit+0x98/0xb0)
> [  130.180249]  r5: r4:edffc500
> [  130.183875] [<80432404>] (drm_atomic_helper_commit) from [<8045810c>] 
> (imx_drm_atomic_commit+0x134/0x144)
> [  130.193447]  r7:80b6bf22 r6:edffc800 r5:edffc500 r4:0006
> [  130.199191] [<80457fd8>] (imx_drm_atomic_commit) from [<804556d0>] 
> (drm_atomic_commit+0x60/0x70)
> [  130.207981]  r10:0004 r9:ee1b923c r8:003f r7:ee1b9000 r6:edffc500 
> r5:ee1b9000
> [  130.215908]  r4:edffc500
> [  130.218475] [<80455670>] (drm_atomic_commit) from [<80435660>] 
> (drm_fb_helper_restore_fbdev_mode_unlocked+0x130/0x29c)
> [  130.229175]  r5:eeba9f00 r4:
> [  130.232799] [<80435530>] (drm_fb_helper_restore_fbdev_mode_unlocked) from 
> [<80436904>] (drm_fbdev_cma_restore_mode+0x20/0x24)
> [  130.244107]  r10:400c0013 r9:ede68f88 r8:ee2053c0 r7:ee1b911c r6:ede68f7c 
> r5:80bc5648
> [  130.252033]  r4:ee1b9000
> [  130.254598] [<804368e4>] (drm_fbdev_cma_restore_mode) from [<804581e8>] 
> (imx_drm_driver_lastclose+0x20/0x24)
> [  130.264439] [<804581c8>] (imx_drm_driver_lastclose) from [<8043a5b0>] 
> (drm_lastclose+0x4c/0xfc)
> [  130.273149] [<8043a564>] (drm_lastclose) from [<8043a938>] 
> (drm_release+0x2d8/0x324)
> [  130.280898]  r7:ee1b911c r6:ede68f7c r5:ede68f00 r4:ee1b9000
> [  130.286642] [<8043a660>] (drm_release) from [<801fa8a8>] 
> (__fput+0xe8/0x1bc)
> [  130.293696]  r10:ede069c8 r9:0008 r8:ee21b190 r7:ee428ee0 r6: 
> r5:ee197bc0
> [  130.301622]  r4:ede069c0
> [  130.304183] [<801fa7c0>] (__fput) from [<801fa9ec>] (fput+0x18/0x1c)
> [  130.310890]  r10: r9: r8:80107ac4 r7:ed47ff58 r6:edf36a80 
> r5:80b70eb8
> [  130.318817]  r4:ee0bf800
> [  130.321381] [<801fa9d4>] (fput) from [<80134e6c>] 
> (task_work_run+0xc8/0xdc)
> [  130.328704] [<80134da4>] (task_work_run) from [<8011caec>] 
> (do_exit+0x438/0x960)
> [  130.336105]  r7:ed47ff58 r6:ee0bfc08 r5:eeb88a80 r4:ee0bf800
> [  130.341847] [<8011c6b4>] (do_exit) from [<8011e188>] 
> (do_group_exit+0x5c/0xcc)
> [  130.349075]  r7:e000
> [  130.351638] [<8011e12c>] (do_group_exit) from [<8011e218>] 
> (__wake_up_parent+0x0/0x30)
> [  130.359560]  r7:00f8 r6:76ec6750 r5:0001 r4:0001
> [  130.365305] [<8011e1f8>] (SyS_exit_group) from 

[PATCH] [RFC] drm: Nerf DRM_CONTROL nodes

[Daniel Vetter]

Looking at the ioctl permission checks I noticed that it's impossible
to import gem buffers into a control nodes, and fd2handle/handle2fd
also don't work, so no joy with dma-bufs.

The only way to do anything with a control node is by drawing stuff
into a dumb buffer and displaying that. I suspect control nodes are an
entirely unused thing, and a cursory check shows that there does not
seem to be any callers of drmOpenControl nor of the other drmOpen
functions using DRM_MODE_CONTROL.

Since I don't like dead uabi, let's remove it. But since this would be
a really big change I think it's better to start out small by simply
not registering anything. We can garbage-collect the dead code later
on, once we're sure it's really not used anywhere.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_drv.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 6efdba4993fc..f085e28ffc6f 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -517,12 +517,6 @@ int drm_dev_init(struct drm_device *dev,
goto err_free;
}

-   if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-   ret = drm_minor_alloc(dev, DRM_MINOR_CONTROL);
-   if (ret)
-   goto err_minors;
-   }
-
if (drm_core_check_feature(dev, DRIVER_RENDER)) {
ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
if (ret)
-- 
2.10.1

How to implement a EGL or DRM display in VA-API driver

[Randy Li]

On 10/27/2016 11:03 PM, Xiang, Haihao wrote:
>> -Original Message-
>> From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On Behalf
>> Of Randy Li
>> Sent: Monday, October 24, 2016 3:59 PM
>> To: libva at lists.freedesktop.org
>> Cc: gwenole.beauchesne at intel.com; dri-devel at lists.freedesktop.org; 
>> linux-
>> rockchip at lists.infradead.org; Jaquez, VictorX > intel.com>;
>> eddie.cai ; 林金发 > rock-chips.com>;
>> herman.chen at rock-chips.com; vjaquez at igalia.com
>> Subject: How to implement a EGL or DRM display in VA-API driver
>>
>> Hello:
>>   I am going to implement a EGL and DRM display for Rockchip VA-API driver.
>> We do have a EGL implementation in Rockchip VA-API driver, but it is
>> implemented in the standard way, we did that as a X11 display.
>>   I didn't see the usage of struct VADriverVTableEGL in gstreamer, and I have
>> no idea about where should I implement something functions like
>> eglExportDRMImageMESA().
>
> VADriverVTableEGL is deprecated in libva, we has a more efficient way to use 
> vaapi and egl.
> You can refer to the examples in libyami-utils 
> (https://github.com/01org/libyami-utils.git) for
> how to use vaapi and egl.
I see, thank you.
>
>>   The DRM seems more complex, the reason I want to use the DRM is that,
>> GPU would not work with the 4K video rendering, so the DRM means that
>> directly output the video into video controller in our platform. But still 
>> have no
>> idea what kind of thing I should implement in the VA-API driver. It seems 
>> that
>> the VA-API base library would open a DRM instance for the driver, but leaving
>> those configure for connector, encoder, planes to VA-API driver?
About the DRM, I have implemented a version which pretends a X output, I 
would like to know a better way.
>
> configure for connector, encoder, planes aren't a part of va-api driver.  You 
> should check libdrm and drm/i915.
> You can refer to the test case of modetest in libdrm 
> (git.freedesktop.org/git/mesa/drm)
>
>
>>   Could you guys give me same sample code or example of those kind of
>> display in VA-API or the documents would help(I would not image there is a
>> VA-API documents)
>>
>> --
>> Randy Li
>> The third produce department
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

-- 
Randy Li
The third produce department
===
This email message, including any attachments, is for the sole
use of the intended recipient(s) and may contain confidential and
privileged information. Any unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient, please
contact the sender by reply e-mail and destroy all copies of the original
message. [Fuzhou Rockchip Electronics, INC. China mainland]
===

[PATCH v6 6/6] drm/fence: add out-fences support

[Daniel Vetter]

On Thu, Oct 27, 2016 at 05:37:11PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Support DRM out-fences by creating a sync_file with a fence for each CRTC
> that sets the OUT_FENCE_PTR property.
> 
> We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
> the sync_file fd back to userspace.
> 
> The sync_file and fd are allocated/created before commit, but the
> fd_install operation only happens after we know that commit succeed.
> 
> v2: Comment by Rob Clark:
>   - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
> 
> Comment by Daniel Vetter:
>   - Add clean up code for out_fences
> 
> v3: Comments by Daniel Vetter:
>   - create DRM_MODE_ATOMIC_EVENT_MASK
>   - userspace should fill out_fences_ptr with the crtc_ids for which
>   it wants fences back.
> 
> v4: Create OUT_FENCE_PTR properties and remove old approach.
> 
> v5: Comments by Brian Starkey:
>   - Remove extra fence_get() in atomic_ioctl()
>   - Check ret before iterating on the crtc_state
>   - check ret before fd_install
>   - set fence_state to NULL at the beginning
>   - check fence_state->out_fence_ptr before put_user()
>   - change order of fput() and put_unused_fd() on failure
> 
>  - Add access_ok() check to the out_fence_ptr received
>  - Rebase after fence -> dma_fence rename
>  - Store out_fence_ptr in the drm_atomic_state
>  - Split crtc_setup_out_fence()
>  - return -1 as out_fence with TEST_ONLY flag
> 
> Signed-off-by: Gustavo Padovan 

I think this looks good. Again I'll wait with full in-depth review of all
the input validation until the igt shows up, but I think we're mostly
covereed. A few smaller comments for polish below.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic.c | 199 
> ---
>  drivers/gpu/drm/drm_crtc.c   |   8 ++
>  include/drm/drm_atomic.h |   1 +
>  include/drm/drm_crtc.h   |  17 
>  4 files changed, 196 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 28d9366..9b70a27 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -290,6 +290,46 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
>  EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>  
>  /**
> + * drm_atomic_set_out_fence_for_crtc - set CRTC out fence pointer
> + * @state: global atomic state object
> + * @crtc: crtc to set the out fence pointer
> + * @fence_ptr: the userspace pointer to user
> + *
> + * This function stores the out fence pointer in the atomic state.
> + */
> +static void
> +drm_atomic_set_out_fence_for_crtc(struct drm_atomic_state *state,
> +   struct drm_crtc *crtc, u64 __user *fence_ptr)
> +{
> + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
> +}
> +
> +/**
> + * drm_atomic_get_out_fence_for_crtc - get CRTC out fence pointer
> + * @state: global atomic state object
> + * @crtc: crtc to set the out fence pointer
> + *
> + * Get the user pointer that should be used for to store the out fence fd.
> + * This function should be called only once per atomic state as it clears
> + * the out_fence_ptr store there to prevent drivers to access them.
> + *
> + * Returns:
> + *
> + * The out fence user pointer.
> + */

We don't document non-driver-visble functions and structs, and especially
static functions should be self explanatory. I feel bad for your effort in
typing kernel-doc here :(

> +static u64 __user *
> +drm_atomic_get_out_fence_for_crtc(struct drm_atomic_state *state,
> +  struct drm_crtc *crtc)
> +{
> + u64 __user *fence_ptr;
> +
> + fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
> + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
> +
> + return fence_ptr;
> +}
> +
> +/**
>   * drm_atomic_set_mode_for_crtc - set mode for CRTC
>   * @state: the CRTC whose incoming state to update
>   * @mode: kernel-internal mode to use for the CRTC, or NULL to disable
> @@ -490,6 +530,12 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   );
>   state->color_mgmt_changed |= replaced;
>   return ret;
> + } else if (property == config->prop_out_fence_ptr) {
> + uint64_t __user *fence_ptr = u64_to_user_ptr(val);
> + if (!access_ok(VERIFY_WRITE, fence_ptr, sizeof(*fence_ptr)))
> + return -EFAULT;
> +
> + drm_atomic_set_out_fence_for_crtc(state->state, crtc, 
> fence_ptr);
>   } else if (crtc->funcs->atomic_set_property)
>   return crtc->funcs->atomic_set_property(crtc, state, property, 
> val);
>   else
> @@ -532,6 +578,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>   *val = (state->ctm) ? state->ctm->base.id : 0;
>   else if (property == config->gamma_lut_property)
>   *val = (state->gamma_lut) ? 

[PATCH v6 5/6] drm/fence: add fence timeline to drm_crtc

[Brian Starkey]

On Fri, Oct 28, 2016 at 09:42:12AM +0200, Daniel Vetter wrote:
>On Thu, Oct 27, 2016 at 05:37:10PM -0200, Gustavo Padovan wrote:
>> From: Gustavo Padovan 
>>
>> Create one timeline context for each CRTC to be able to handle out-fences
>> and signal them. It adds a few members to struct drm_crtc: fence_context,
>> where we store the context we get from fence_context_alloc(), the
>> fence seqno and the fence lock, that we pass in fence_init() to be
>> used by the fence.
>>
>> v2: Comment by Daniel Stone:
>>  - add BUG_ON() to fence_to_crtc() macro
>>
>> v3: Comment by Ville Syrjälä
>>  - Use more meaningful name as crtc timeline name
>>
>> v4: Comments by Brian Starkey
>>  - Use even more meaninful name for the crtc timeline
>>  - add doc for timeline_name
>> Comment by Daniel Vetter
>>  - use in-line style for comments
>>
>> - rebase after fence -> dma_fence rename
>>
>> Signed-off-by: Gustavo Padovan 
>> ---
>>  drivers/gpu/drm/drm_crtc.c | 31 +++
>>  include/drm/drm_crtc.h | 39 +++
>>  2 files changed, 70 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 7878bfd..e2a06c8 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -165,6 +165,32 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
>>  #endif
>>  }
>>
>> +static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)
>> +{
>> +struct drm_crtc *crtc = fence_to_crtc(fence);
>> +
>> +return crtc->dev->driver->name;
>> +}
>> +
>> +static const char *drm_crtc_fence_get_timeline_name(struct dma_fence *fence)
>> +{
>> +struct drm_crtc *crtc = fence_to_crtc(fence);
>> +
>> +return crtc->timeline_name;
>> +}
>> +
>> +static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
>> +{
>> +return true;
>> +}
>> +
>> +const struct dma_fence_ops drm_crtc_fence_ops = {
>> +.get_driver_name = drm_crtc_fence_get_driver_name,
>> +.get_timeline_name = drm_crtc_fence_get_timeline_name,
>> +.enable_signaling = drm_crtc_fence_enable_signaling,
>> +.wait = dma_fence_default_wait,
>> +};
>> +
>>  /**
>>   * drm_crtc_init_with_planes - Initialise a new CRTC object with
>>   *specified primary and cursor planes.
>> @@ -222,6 +248,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev, 
>> struct drm_crtc *crtc,
>>  return -ENOMEM;
>>  }
>>
>> +crtc->fence_context = dma_fence_context_alloc(1);
>> +spin_lock_init(>fence_lock);
>> +snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
>> + "CRTC:%d-%s", crtc->base.id, crtc->name);
>> +
>>  crtc->base.properties = >properties;
>>
>>  list_add_tail(>head, >crtc_list);
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 719b6a8..278dbdd 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -32,6 +32,8 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -726,8 +728,45 @@ struct drm_crtc {
>>   */
>>  struct drm_crtc_crc crc;
>>  #endif
>> +
>> +/**
>> + * @fence_context:
>> + *
>> + * timeline context used for fence operations.
>> + */
>> +unsigned int fence_context;
>> +
>> +/**
>> + * @fence_lock:
>> + *
>> + * spinlock to protect the fences in the fence_context.
>> + */
>> +
>> +spinlock_t fence_lock;
>> +/**
>> + * @fence_seqno:
>> + *
>> + * Seqno variable used as monotonic counter for the fences
>> + * created on the CRTC's timeline.
>> + */
>> +unsigned long fence_seqno;
>> +
>> +/**
>> + * @timeline_name:
>> + *
>> + * The name of the CRTC's fence timeline.
>> + */
>> +char timeline_name[32];
>>  };
>>
>> +extern const struct dma_fence_ops drm_crtc_fence_ops;
>> +
>
>Kerneldoc please. With that addressed:
>
>Reviewed-by: Daniel Vetter 
>

For my connector fences I exported a function to get the fence, then
you can keep the ops and fence_to_crtc private to drm_crtc.c. In that
case I think you can drop the BUG_ON.

Either way, lgtm.

Reviewed-by: Brian Starkey 

>> +static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
>> +{
>> +BUG_ON(fence->ops != _crtc_fence_ops);
>> +return container_of(fence->lock, struct drm_crtc, fence_lock);
>> +}
>> +
>>  /**
>>   * struct drm_mode_set - new values for a CRTC config change
>>   * @fb: framebuffer to use for new config
>> --
>> 2.5.5
>>
>
>-- 
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
>

[Bug 94530] AMD R9 Nano reset problem

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=94530

--- Comment #2 from m.fehrenbach at protonmail.com ---
I've got a VM which uses an R9 Nano via passthrough. Like Michael said, my
system hangs both the VM and the host machine when installing AMD drier
updates. Likewise my VM freezes and the fan revs up to maximum upon a normal
resart of the VM, or I change anything in the QEMU startup command (e.g. adding
another hard drive to the VM). And the only way to stop either of those issues
is to power off the host machine.

The only way I have been able to change the driver for my card is during a
fresh install of Windows when no drivers are previously installed (aside from
the basic prepackaged ones are within Windows). Then I can get the lastest
drivers I already downloaded from another hard drive and install them on my
system, and it will work for the most part. However this doesn't address the
core issue.

Whilst I'm unsure if this website is the appropriate place to file this bug
report, the previous two posts are the only place I've seen this exact issue
mentioned.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/5dcbf9ef/attachment.html>

[PATCH 0/5] drm/i915/skl: Backport watermark fixes for 4.8.y

[Greg KH]

On Wed, Oct 26, 2016 at 03:36:32PM -0400, Lyude wrote:
> Now that these have finally made it into 4.9, it's time to finally backport
> these fixes. Skylake has been a mess in multi-monitor setups for a while now
> because up until recently we've been updating the watermarks on Skylake just
> like we would for previous generations of Intel GPUs. This means updating
> attributes for each plane, and then only after they've been updated writing
> their new watermark values.
> 
> The problem with this approach is Skylake has double buffered watermark
> registers that are flipped at the same time as the rest of the plane 
> registers.
> This means that the original approach will leave planes active with new
> attributes but without the required watermark programming that would ensure 
> the
> display pipe reads enough data from each plane. As a result, pipes start to
> underrun and the user's displays starts to flicker heavily. Usually in 
> response
> to plugging in new monitors, or moving cursors from one screen to another
> (which triggers a plane and watermark update).
> 
> Additionally, issues were found with the original code for configuring ddb,
> display data buffer, allocations between display planes on Skylake. On Skylake
> all planes have space allocated to them in the ddb, and the hardware requires
> that these allocations never overlap at any point in time. Because ddb
> allocations were not updated alongside plane attributes despite also being
> double buffered registers armed by plane updates, planes were likely to get
> stuck momentarily with ddb allocations that overlapped one another. This would
> also lead to pipe underruns and display flickering.
> 
> The new approach fixes this problem by making sure that on Skylake, attributes
> for display planes are always updated at the same time as the watermarks, and
> pipes are updated in an order that ensures their ddb allocations don't
> overlap at any point between plane updates. This ensures the display pipes are
> always programmed correctly, and dramatically reduces the chance of display
> flickering.
> 
> (note: my e-mail has changed since these patches were upstreamed, and I 
> updated
> the e-mails in these patches to reflect this. if this is wrong I will be happy
> to update and resend the patches).

Thanks for these, all now queued up.

greg k-h

[PATCH v6 5/6] drm/fence: add fence timeline to drm_crtc

[Daniel Vetter]

On Thu, Oct 27, 2016 at 05:37:10PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Create one timeline context for each CRTC to be able to handle out-fences
> and signal them. It adds a few members to struct drm_crtc: fence_context,
> where we store the context we get from fence_context_alloc(), the
> fence seqno and the fence lock, that we pass in fence_init() to be
> used by the fence.
> 
> v2: Comment by Daniel Stone:
>   - add BUG_ON() to fence_to_crtc() macro
> 
> v3: Comment by Ville Syrjälä
>   - Use more meaningful name as crtc timeline name
> 
> v4: Comments by Brian Starkey
>   - Use even more meaninful name for the crtc timeline
>   - add doc for timeline_name
> Comment by Daniel Vetter
>   - use in-line style for comments
> 
> - rebase after fence -> dma_fence rename
> 
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/gpu/drm/drm_crtc.c | 31 +++
>  include/drm/drm_crtc.h | 39 +++
>  2 files changed, 70 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 7878bfd..e2a06c8 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -165,6 +165,32 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
>  #endif
>  }
>  
> +static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)
> +{
> + struct drm_crtc *crtc = fence_to_crtc(fence);
> +
> + return crtc->dev->driver->name;
> +}
> +
> +static const char *drm_crtc_fence_get_timeline_name(struct dma_fence *fence)
> +{
> + struct drm_crtc *crtc = fence_to_crtc(fence);
> +
> + return crtc->timeline_name;
> +}
> +
> +static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
> +{
> + return true;
> +}
> +
> +const struct dma_fence_ops drm_crtc_fence_ops = {
> + .get_driver_name = drm_crtc_fence_get_driver_name,
> + .get_timeline_name = drm_crtc_fence_get_timeline_name,
> + .enable_signaling = drm_crtc_fence_enable_signaling,
> + .wait = dma_fence_default_wait,
> +};
> +
>  /**
>   * drm_crtc_init_with_planes - Initialise a new CRTC object with
>   *specified primary and cursor planes.
> @@ -222,6 +248,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev, 
> struct drm_crtc *crtc,
>   return -ENOMEM;
>   }
>  
> + crtc->fence_context = dma_fence_context_alloc(1);
> + spin_lock_init(>fence_lock);
> + snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
> +  "CRTC:%d-%s", crtc->base.id, crtc->name);
> +
>   crtc->base.properties = >properties;
>  
>   list_add_tail(>head, >crtc_list);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 719b6a8..278dbdd 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -32,6 +32,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -726,8 +728,45 @@ struct drm_crtc {
>*/
>   struct drm_crtc_crc crc;
>  #endif
> +
> + /**
> +  * @fence_context:
> +  *
> +  * timeline context used for fence operations.
> +  */
> + unsigned int fence_context;
> +
> + /**
> +  * @fence_lock:
> +  *
> +  * spinlock to protect the fences in the fence_context.
> +  */
> +
> + spinlock_t fence_lock;
> + /**
> +  * @fence_seqno:
> +  *
> +  * Seqno variable used as monotonic counter for the fences
> +  * created on the CRTC's timeline.
> +  */
> + unsigned long fence_seqno;
> +
> + /**
> +  * @timeline_name:
> +  *
> +  * The name of the CRTC's fence timeline.
> +  */
> + char timeline_name[32];
>  };
>  
> +extern const struct dma_fence_ops drm_crtc_fence_ops;
> +

Kerneldoc please. With that addressed:

Reviewed-by: Daniel Vetter 

> +static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
> +{
> + BUG_ON(fence->ops != _crtc_fence_ops);
> + return container_of(fence->lock, struct drm_crtc, fence_lock);
> +}
> +
>  /**
>   * struct drm_mode_set - new values for a CRTC config change
>   * @fb: framebuffer to use for new config
> -- 
> 2.5.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v6 4/6] drm/fence: add in-fences support

[Daniel Vetter]

On Thu, Oct 27, 2016 at 05:37:09PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> There is now a new property called FENCE_FD attached to every plane
> state that receives the sync_file fd from userspace via the atomic commit
> IOCTL.
> 
> The fd is then translated to a fence (that may be a fence_collection
> subclass or just a normal fence) and then used by DRM to fence_wait() for
> all fences in the sync_file to signal. So it only commits when all
> framebuffers are ready to scanout.
> 
> v2: Comments by Daniel Vetter:
>   - remove set state->fence = NULL in destroy phase
>   - accept fence -1 as valid and just return 0
>   - do not call fence_get() - sync_file_fences_get() already calls it
>   - fence_put() if state->fence is already set, in case userspace
>   set the property more than once.
> 
> v3: WARN_ON if fence is set but state has no FB
> 
> v4: Comment from Maarten Lankhorst
>   - allow set fence with no related fb
> 
> v5: rename FENCE_FD to IN_FENCE_FD
> 
> v6: Comments by Daniel Vetter:
>   - rename plane_state->in_fence back to "fence"
> 
>  - rebase after fence -> dma_fence rename
> 
> Signed-off-by: Gustavo Padovan 

Looks good, but I'll wait with full review with all the fence patches
until the igts show up. It's much easier to check for gabs in input
validation code if you also have the testcases at hand. A few comments
below.
-Daniel

> ---
>  drivers/gpu/drm/Kconfig |  1 +
>  drivers/gpu/drm/drm_atomic.c| 15 +++
>  drivers/gpu/drm/drm_atomic_helper.c |  5 +++--
>  drivers/gpu/drm/drm_crtc.c  |  6 ++
>  drivers/gpu/drm/drm_plane.c |  1 +
>  include/drm/drm_crtc.h  |  5 +
>  6 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 483059a..43cb33d 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -12,6 +12,7 @@ menuconfig DRM
>   select I2C
>   select I2C_ALGOBIT
>   select DMA_SHARED_BUFFER
> + select SYNC_FILE
>   help
> Kernel-level support for the Direct Rendering Infrastructure (DRI)
> introduced in XFree86 4.0. If you say Y here, you need to select
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5e73954..28d9366 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -30,6 +30,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "drm_crtc_internal.h"
>  
> @@ -686,6 +687,17 @@ int drm_atomic_plane_set_property(struct drm_plane 
> *plane,
>   drm_atomic_set_fb_for_plane(state, fb);
>   if (fb)
>   drm_framebuffer_unreference(fb);
> + } else if (property == config->prop_in_fence_fd) {
> + if (U642I64(val) == -1)
> + return 0;
> +
> + if (state->fence)
> + dma_fence_put(state->fence);
> +
> + state->fence = sync_file_get_fence(val);
> + if (!state->fence)
> + return -EINVAL;
> +
>   } else if (property == config->prop_crtc_id) {
>   struct drm_crtc *crtc = drm_crtc_find(dev, val);
>   return drm_atomic_set_crtc_for_plane(state, crtc);
> @@ -747,6 +759,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  
>   if (property == config->prop_fb_id) {
>   *val = (state->fb) ? state->fb->base.id : 0;
> + } else if (property == config->prop_in_fence_fd) {
> + *val = -1;
>   } else if (property == config->prop_crtc_id) {
>   *val = (state->crtc) ? state->crtc->base.id : 0;
>   } else if (property == config->prop_crtc_x) {
> @@ -1752,6 +1766,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   drm_mode_object_unreference(obj);
>   }
>  
> +

Spurios whitespace.

>   if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
>   for_each_crtc_in_state(state, crtc, crtc_state, i) {
>   struct drm_pending_vblank_event *e;
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 75ad01d..c34da9e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1034,8 +1034,6 @@ int drm_atomic_helper_wait_for_fences(struct drm_device 
> *dev,
>   if (!plane_state->fence)
>   continue;
>  
> - WARN_ON(!plane_state->fb);
> -

Why this? We don't allow a plane to be enabled without an fb, and adding a
fence to a plane which is disabled sounds likea  bug. We probably should
have a bit of code in the core atomic check code to make sure userspace
never asks for a fence when the plane is off.

>   /*
>* If waiting for fences pre-swap (ie: nonblock), userspace can
>* still interrupt the operation. Instead of blocking until the
> @@ 

[PATCH v6 3/6] drm/msm: use drm_atomic_set_fence_for_plane() to set the fence

[Daniel Vetter]

On Thu, Oct 27, 2016 at 05:37:08PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> drm_atomic_set_fence_for_plane() is smart and won't overwrite
> plane_state->fence if the user already set an explicit fence there.
> 
> Signed-off-by: Gustavo Padovan 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/msm/msm_atomic.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c 
> b/drivers/gpu/drm/msm/msm_atomic.c
> index db193f8..4e21e1d 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -217,8 +217,9 @@ int msm_atomic_commit(struct drm_device *dev,
>   if ((plane->state->fb != plane_state->fb) && plane_state->fb) {
>   struct drm_gem_object *obj = 
> msm_framebuffer_bo(plane_state->fb, 0);
>   struct msm_gem_object *msm_obj = to_msm_bo(obj);
> + struct dma_fence *fence = 
> reservation_object_get_excl_rcu(msm_obj->resv);
>  
> - plane_state->fence = 
> reservation_object_get_excl_rcu(msm_obj->resv);
> + drm_atomic_set_fence_for_plane(plane_state, fence);
>   }
>   }
>  
> -- 
> 2.5.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v6 2/6] drm/imx: use drm_atomic_set_fence_for_plane() to set the fence

[Daniel Vetter]

On Thu, Oct 27, 2016 at 05:37:07PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> drm_atomic_set_fence_for_plane() is smart and won't overwrite
> plane_state->fence if the user already set an explicit fence there.
> 
> Signed-off-by: Gustavo Padovan 

Process nit: Please make sure you Cc: driver maintainers for driver
patches. Best done by putting the Cc: into the patch, then you never
forget ;-)

Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/imx/imx-drm-core.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c 
> b/drivers/gpu/drm/imx/imx-drm-core.c
> index 98df09c..07fe955 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -158,6 +158,7 @@ static int imx_drm_atomic_commit(struct drm_device *dev,
>   struct drm_plane_state *plane_state;
>   struct drm_plane *plane;
>   struct dma_buf *dma_buf;
> + struct dma_fence *fence;
>   int i;
>  
>   /*
> @@ -170,8 +171,9 @@ static int imx_drm_atomic_commit(struct drm_device *dev,
>0)->base.dma_buf;
>   if (!dma_buf)
>   continue;
> - plane_state->fence =
> - reservation_object_get_excl_rcu(dma_buf->resv);
> + fence = reservation_object_get_excl_rcu(dma_buf->resv);
> +
> + drm_atomic_set_fence_for_plane(plane_state, fence);
>   }
>   }
>  
> -- 
> 2.5.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v6 1/6] drm/atomic: add drm_atomic_set_fence_for_plane()

[Daniel Vetter]

On Fri, Oct 28, 2016 at 09:30:26AM +0200, Daniel Vetter wrote:
> On Thu, Oct 27, 2016 at 05:37:06PM -0200, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > This new function should be used by drivers when setting a implicit
> > fence for the plane. It abstracts the fact that the user might have
> > chosen explicit fencing instead.
> > 
> > Signed-off-by: Gustavo Padovan 
> 
> Reviewed-by: Daniel Vetter 
> 
> > ---
> >  drivers/gpu/drm/drm_atomic.c | 30 ++
> >  include/drm/drm_atomic.h |  2 ++
> >  include/drm/drm_plane.h  |  2 +-
> >  3 files changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index c32fb3c..5e73954 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1147,6 +1147,36 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state 
> > *plane_state,
> >  EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);
> >  
> >  /**
> > + * drm_atomic_set_fence_for_plane - set fence for plane
> > + * @plane_state: atomic state object for the plane
> > + * @fence: dma_fence to use for the plane
> > + *
> > + * Helper to setup the plane_state fence in case it is not set yet.
> > + * By using this drivers doesn't need to worry if the user choose
> > + * implicit or explicit fencing.
> > + *
> > + * This function will not set the fence to the state if it was set
> > + * via explicit fencing interfaces on the atomic ioctl. It will
> > + * all drope the reference to the fence as we not storing it
> > + * anywhere.
> > + *
> > + * Otherwise, if plane_state->fence is not set this function we
> > + * just set it with the received implict fence.
> > + */
> > +void
> > +drm_atomic_set_fence_for_plane(struct drm_plane_state *plane_state,
> > +  struct dma_fence *fence)
> > +{
> > +   if (plane_state->fence) {
> > +   dma_fence_put(fence);
> > +   return;
> > +   }
> > +
> > +   plane_state->fence = fence;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_set_fence_for_plane);
> > +
> > +/**
> >   * drm_atomic_set_crtc_for_connector - set crtc for connector
> >   * @conn_state: atomic state object for the connector
> >   * @crtc: crtc to use for the connector
> > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> > index fc8af53..2d1e9c9 100644
> > --- a/include/drm/drm_atomic.h
> > +++ b/include/drm/drm_atomic.h
> > @@ -345,6 +345,8 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state 
> > *plane_state,
> >   struct drm_crtc *crtc);
> >  void drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
> >  struct drm_framebuffer *fb);
> > +void drm_atomic_set_fence_for_plane(struct drm_plane_state *plane_state,
> > +   struct dma_fence *fence);
> >  int __must_check
> >  drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
> >   struct drm_crtc *crtc);
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > index c5e8a0d..68f6d22 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -59,7 +59,7 @@ struct drm_plane_state {
> >  
> > struct drm_crtc *crtc;   /* do not write directly, use 
> > drm_atomic_set_crtc_for_plane() */
> > struct drm_framebuffer *fb;  /* do not write directly, use 
> > drm_atomic_set_fb_for_plane() */
> > -   struct dma_fence *fence;
> > +   struct dma_fence *fence; /* do not write directly, use 
> > drm_atomic_set_fence_for_plane() */

Ok, small bikeshed maybe: When you feel the need to add more comments
in-line in a struct, then please switch to the in-line kernel-doc member
documentation style, and merge the existing kerneldoc together with these
additional comments. Would be great to do that with all the others here
too. Follow-up patch to address this would be great.
-Daniel

> >  
> > /* Signed dest location allows it to be partially off screen */
> > int32_t crtc_x, crtc_y;
> > -- 
> > 2.5.5
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v6 1/6] drm/atomic: add drm_atomic_set_fence_for_plane()

[Daniel Vetter]

On Thu, Oct 27, 2016 at 05:37:06PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> This new function should be used by drivers when setting a implicit
> fence for the plane. It abstracts the fact that the user might have
> chosen explicit fencing instead.
> 
> Signed-off-by: Gustavo Padovan 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/drm_atomic.c | 30 ++
>  include/drm/drm_atomic.h |  2 ++
>  include/drm/drm_plane.h  |  2 +-
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index c32fb3c..5e73954 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1147,6 +1147,36 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state 
> *plane_state,
>  EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);
>  
>  /**
> + * drm_atomic_set_fence_for_plane - set fence for plane
> + * @plane_state: atomic state object for the plane
> + * @fence: dma_fence to use for the plane
> + *
> + * Helper to setup the plane_state fence in case it is not set yet.
> + * By using this drivers doesn't need to worry if the user choose
> + * implicit or explicit fencing.
> + *
> + * This function will not set the fence to the state if it was set
> + * via explicit fencing interfaces on the atomic ioctl. It will
> + * all drope the reference to the fence as we not storing it
> + * anywhere.
> + *
> + * Otherwise, if plane_state->fence is not set this function we
> + * just set it with the received implict fence.
> + */
> +void
> +drm_atomic_set_fence_for_plane(struct drm_plane_state *plane_state,
> +struct dma_fence *fence)
> +{
> + if (plane_state->fence) {
> + dma_fence_put(fence);
> + return;
> + }
> +
> + plane_state->fence = fence;
> +}
> +EXPORT_SYMBOL(drm_atomic_set_fence_for_plane);
> +
> +/**
>   * drm_atomic_set_crtc_for_connector - set crtc for connector
>   * @conn_state: atomic state object for the connector
>   * @crtc: crtc to use for the connector
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index fc8af53..2d1e9c9 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -345,6 +345,8 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state 
> *plane_state,
> struct drm_crtc *crtc);
>  void drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
>struct drm_framebuffer *fb);
> +void drm_atomic_set_fence_for_plane(struct drm_plane_state *plane_state,
> + struct dma_fence *fence);
>  int __must_check
>  drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
> struct drm_crtc *crtc);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index c5e8a0d..68f6d22 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -59,7 +59,7 @@ struct drm_plane_state {
>  
>   struct drm_crtc *crtc;   /* do not write directly, use 
> drm_atomic_set_crtc_for_plane() */
>   struct drm_framebuffer *fb;  /* do not write directly, use 
> drm_atomic_set_fb_for_plane() */
> - struct dma_fence *fence;
> + struct dma_fence *fence; /* do not write directly, use 
> drm_atomic_set_fence_for_plane() */
>  
>   /* Signed dest location allows it to be partially off screen */
>   int32_t crtc_x, crtc_y;
> -- 
> 2.5.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[Bug 178281] wine-staging apps freezes the machine with RX460

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=178281

--- Comment #14 from fin4478 at hotmail.com ---
One hour ago published Oibaf Mesa fixed Xserver hanging when playing TR Legend
at fullhd.
https://launchpad.net/~oibaf/+archive/ubuntu/graphics-drivers/+packages

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 178281] wine-staging apps freezes the machine with RX460

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=178281

--- Comment #13 from fin4478 at hotmail.com ---
I tested  latest drm-next-4.10-wip  today that have couple of more patches that
yesterday. Still Xserver hangs when playing Tomb Raider Legend at full hd.

With dmesg, I have these errors at boot:
[3.466266] amdgpu :01:00.0: Invalid PCI ROM header signature: expecting
0xaa55, got 0x
[3.466706] ATOM BIOS: C99401


[3.479722] [drm] amdgpu: irq initialized.
[3.479730] Can't find requested voltage id in vdd_dep_on_sclk table!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 178281] wine-staging apps freezes the machine with RX460

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=178281

--- Comment #12 from fin4478 at hotmail.com ---
With Tomb Raider Legend the game hangs Xserver in one minute when playing at
fullhd.Dropping resolution to 720p allows to play stable. drm-next-4.10-wip and
Oipaf ppa mesa from yesterday used. There is no matter if csmt in wine-staging
is enabled or not.

OpenGL vendor string: X.Org
OpenGL renderer string: Gallium 0.4 on AMD POLARIS11 (DRM 3.9.0 / 4.9.0-rc2+,
LLVM 3.9.0)
OpenGL core profile version string: 4.3 (Core Profile) Mesa 13.1.0-devel
OpenGL core profile shading language version string: 4.30
OpenGL core profile context flags: (none)
OpenGL core profile profile mask: core profile
OpenGL core profile extensions:
OpenGL version string: 3.0 Mesa 13.1.0-devel
OpenGL shading language version string: 1.30
OpenGL context flags: (none)
OpenGL extensions:
OpenGL ES profile version string: OpenGL ES 3.1 Mesa 13.1.0-devel
OpenGL ES profile shading language version string: OpenGL ES GLSL ES 3.10

I needed to configure 4.9 kernel like a stock laptop kernel to make it more
stable when gaming.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH v8 12/12] drm/i915: Add a kerneldoc summary for i915_perf.c

[Robert Bragg]

In particular this tries to capture for posterity some of the early
challenges we had with using the core perf infrastructure in case we
ever want to revisit adapting perf for device metrics.

Cc: Chris Wilson 
Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/i915_perf.c | 163 +++
 1 file changed, 163 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index e3c6f51..621b3aa 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -24,6 +24,169 @@
  *   Robert Bragg 
  */

+
+/**
+ * DOC: i915 Perf, streaming API for GPU metrics
+ *
+ * Gen graphics supports a large number of performance counters that can help
+ * driver and application developers understand and optimize their use of the
+ * GPU.
+ *
+ * This i915 perf interface enables userspace to configure and open a file
+ * descriptor representing a stream of GPU metrics which can then be read() as
+ * a stream of sample records.
+ *
+ * The interface is particularly suited to exposing buffered metrics that are
+ * captured by DMA from the GPU, unsynchronized with and unrelated to the CPU.
+ *
+ * Streams representing a single context are accessible to applications with a
+ * corresponding drm file descriptor, such that OpenGL can use the interface
+ * without special privileges. Access to system-wide metrics requires root
+ * privileges by default, unless changed via the dev.i915.perf_event_paranoid
+ * sysctl option.
+ *
+ *
+ * The interface was initially inspired by the core Perf infrastructure but
+ * some notable differences are:
+ *
+ * i915 perf file descriptors represent a "stream" instead of an "event"; where
+ * a perf event primarily corresponds to a single 64bit value, while a stream
+ * might sample sets of tightly-coupled counters, depending on the
+ * configuration.  For example the Gen OA unit isn't designed to support
+ * orthogonal configurations of individual counters; it's configured for a set
+ * of related counters. Samples for an i915 perf stream capturing OA metrics
+ * will include a set of counter values packed in a compact HW specific format.
+ * The OA unit supports a number of different packing formats which can be
+ * selected by the user opening the stream. Perf has support for grouping
+ * events, but each event in the group is configured, validated and
+ * authenticated individually with separate system calls.
+ *
+ * i915 perf stream configurations are provided as an array of u64 (key,value)
+ * pairs, instead of a fixed struct with multiple miscellaneous config members,
+ * interleaved with event-type specific members.
+ *
+ * i915 perf doesn't support exposing metrics via an mmap'd circular buffer.
+ * The supported metrics are being written to memory by the GPU unsynchronized
+ * with the CPU, using HW specific packing formats for counter sets. Sometimes
+ * the constraints on HW configuration require reports to be filtered before it
+ * would be acceptable to expose them to unprivileged applications - to hide
+ * the metrics of other processes/contexts. For these use cases a read() based
+ * interface is a good fit, and provides an opportunity to filter data as it
+ * gets copied from the GPU mapped buffers to userspace buffers.
+ *
+ *
+ * Some notes regarding Linux Perf:
+ * 
+ *
+ * The first prototype of this driver was based on the core perf
+ * infrastructure, and while we did make that mostly work, with some changes to
+ * perf, we found we were breaking or working around too many assumptions baked
+ * into perf's currently cpu centric design.
+ *
+ * In the end we didn't see a clear benefit to making perf's implementation and
+ * interface more complex by changing design assumptions while we knew we still
+ * wouldn't be able to use any existing perf based userspace tools.
+ *
+ * Also considering the Gen specific nature of the Observability hardware and
+ * how userspace will sometimes need to combine i915 perf OA metrics with
+ * side-band OA data captured via MI_REPORT_PERF_COUNT commands; we're
+ * expecting the interface to be used by a platform specific userspace such as
+ * OpenGL or tools. This is to say; we aren't inherently missing out on having
+ * a standard vendor/architecture agnostic interface by not using perf.
+ *
+ *
+ * For posterity, in case we might re-visit trying to adapt core perf to be
+ * better suited to exposing i915 metrics these were the main pain points we
+ * hit:
+ *
+ * - The perf based OA PMU driver broke some significant design assumptions:
+ *
+ *   Existing perf pmus are used for profiling work on a cpu and we were
+ *   introducing the idea of _IS_DEVICE pmus with different security
+ *   implications, the need to fake cpu-related data (such as user/kernel
+ *   registers) to fit with perf's current design, and adding _DEVICE records
+ *   as a way to forward device-specific status records.
+ 

[PATCH v8 11/12] drm/i915: Add more Haswell OA metric sets

[Robert Bragg]

This adds 'compute', 'compute extended', 'memory reads', 'memory writes'
and 'sampler balance' metric sets for Haswell.

The code is auto generated from an XML description of metric sets,
currently maintained in gputop, ref:

 https://github.com/rib/gputop
 > gputop-data/oa-*.xml
 > scripts/i915-perf-kernelgen.py

 $ make -C gputop-data -f Makefile.xml

Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/i915_oa_hsw.c | 559 -
 1 file changed, 558 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_oa_hsw.c 
b/drivers/gpu/drm/i915/i915_oa_hsw.c
index 6af25cf..4ddf756 100644
--- a/drivers/gpu/drm/i915/i915_oa_hsw.c
+++ b/drivers/gpu/drm/i915/i915_oa_hsw.c
@@ -31,9 +31,14 @@

 enum metric_set_id {
METRIC_SET_ID_RENDER_BASIC = 1,
+   METRIC_SET_ID_COMPUTE_BASIC,
+   METRIC_SET_ID_COMPUTE_EXTENDED,
+   METRIC_SET_ID_MEMORY_READS,
+   METRIC_SET_ID_MEMORY_WRITES,
+   METRIC_SET_ID_SAMPLER_BALANCE,
 };

-int i915_oa_n_builtin_metric_sets_hsw = 1;
+int i915_oa_n_builtin_metric_sets_hsw = 6;

 static const struct i915_oa_reg b_counter_config_render_basic[] = {
{ _MMIO(0x2724), 0x0080 },
@@ -112,6 +117,298 @@ get_render_basic_mux_config(struct drm_i915_private 
*dev_priv,
return mux_config_render_basic;
 }

+static const struct i915_oa_reg b_counter_config_compute_basic[] = {
+   { _MMIO(0x2710), 0x },
+   { _MMIO(0x2714), 0x0080 },
+   { _MMIO(0x2718), 0x },
+   { _MMIO(0x271c), 0x },
+   { _MMIO(0x2720), 0x },
+   { _MMIO(0x2724), 0x0080 },
+   { _MMIO(0x2728), 0x },
+   { _MMIO(0x272c), 0x },
+   { _MMIO(0x2740), 0x },
+   { _MMIO(0x2744), 0x },
+   { _MMIO(0x2748), 0x },
+   { _MMIO(0x274c), 0x },
+   { _MMIO(0x2750), 0x },
+   { _MMIO(0x2754), 0x },
+   { _MMIO(0x2758), 0x },
+   { _MMIO(0x275c), 0x },
+   { _MMIO(0x236c), 0x },
+};
+
+static const struct i915_oa_reg mux_config_compute_basic[] = {
+   { _MMIO(0x253a4), 0x },
+   { _MMIO(0x2681c), 0x01f00800 },
+   { _MMIO(0x26820), 0x1000 },
+   { _MMIO(0x2781c), 0x01f00800 },
+   { _MMIO(0x26520), 0x0007 },
+   { _MMIO(0x265a0), 0x0007 },
+   { _MMIO(0x25380), 0x0010 },
+   { _MMIO(0x2538c), 0x0030 },
+   { _MMIO(0x25384), 0xaa8a },
+   { _MMIO(0x25404), 0x },
+   { _MMIO(0x26800), 0x4202 },
+   { _MMIO(0x26808), 0x00605817 },
+   { _MMIO(0x2680c), 0x10001005 },
+   { _MMIO(0x26804), 0x },
+   { _MMIO(0x27800), 0x0102 },
+   { _MMIO(0x27808), 0x0c0701e0 },
+   { _MMIO(0x2780c), 0x000200a0 },
+   { _MMIO(0x27804), 0x },
+   { _MMIO(0x26484), 0x4400 },
+   { _MMIO(0x26704), 0x4400 },
+   { _MMIO(0x26500), 0x0006 },
+   { _MMIO(0x26510), 0x0001 },
+   { _MMIO(0x26504), 0x8800 },
+   { _MMIO(0x26580), 0x0006 },
+   { _MMIO(0x26590), 0x0020 },
+   { _MMIO(0x26584), 0x },
+   { _MMIO(0x26104), 0x5582 },
+   { _MMIO(0x26184), 0xaa86 },
+   { _MMIO(0x25420), 0x08320c83 },
+   { _MMIO(0x25424), 0x06820c83 },
+   { _MMIO(0x2541c), 0x },
+   { _MMIO(0x25428), 0x0c03 },
+};
+
+static const struct i915_oa_reg *
+get_compute_basic_mux_config(struct drm_i915_private *dev_priv,
+int *len)
+{
+   *len = ARRAY_SIZE(mux_config_compute_basic);
+   return mux_config_compute_basic;
+}
+
+static const struct i915_oa_reg b_counter_config_compute_extended[] = {
+   { _MMIO(0x2724), 0xf080 },
+   { _MMIO(0x2720), 0x },
+   { _MMIO(0x2714), 0xf080 },
+   { _MMIO(0x2710), 0x },
+   { _MMIO(0x2770), 0x0007fe2a },
+   { _MMIO(0x2774), 0xff00 },
+   { _MMIO(0x2778), 0x0007fe6a },
+   { _MMIO(0x277c), 0xff00 },
+   { _MMIO(0x2780), 0x0007fe92 },
+   { _MMIO(0x2784), 0xff00 },
+   { _MMIO(0x2788), 0x0007fea2 },
+   { _MMIO(0x278c), 0xff00 },
+   { _MMIO(0x2790), 0x0007fe32 },
+   { _MMIO(0x2794), 0xff00 },
+   { _MMIO(0x2798), 0x0007fe9a },
+   { _MMIO(0x279c), 0xff00 },
+   { _MMIO(0x27a0), 0x0007ff23 },
+   { _MMIO(0x27a4), 0xff00 },
+   { _MMIO(0x27a8), 0x0007fff3 },
+   { _MMIO(0x27ac), 0xfffe },
+};
+
+static const struct i915_oa_reg mux_config_compute_extended[] = {
+   { _MMIO(0x2681c), 0x3eb00800 },
+   { _MMIO(0x26820), 0x0090 },
+   { _MMIO(0x25384), 0x02aa },
+   { _MMIO(0x25404), 0x03ff },
+   { _MMIO(0x26800), 0x00142284 },
+   { _MMIO(0x26808), 0x0e629062 },
+   { _MMIO(0x2680c), 0x3f6f55cb },
+   { _MMIO(0x26810), 0x0014 },
+   { _MMIO(0x26804), 0x },
+   { _MMIO(0x26104), 

[PATCH v8 10/12] drm/i915: add oa_event_min_timer_exponent sysctl

[Robert Bragg]

The minimal sampling period is now configurable via a
dev.i915.oa_min_timer_exponent sysctl parameter.

Following the precedent set by perf, the default is the minimum that
won't (on its own) exceed the default kernel.perf_event_max_sample_rate
default of 10 samples/s.

Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/i915_perf.c | 42 
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 4e42073..e3c6f51 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -82,6 +82,22 @@ static u32 i915_perf_stream_paranoid = true;
 #define INVALID_CTX_ID 0x


+/* for sysctl proc_dointvec_minmax of i915_oa_min_timer_exponent */
+static int oa_exponent_max = OA_EXPONENT_MAX;
+
+/* Theoretically we can program the OA unit to sample every 160ns but don't
+ * allow that by default unless root...
+ *
+ * The period is derived from the exponent as:
+ *
+ *   period = 80ns * 2^(exponent + 1)
+ *
+ * Referring to perf's kernel.perf_event_max_sample_rate for a precedent
+ * (10 by default); with an OA exponent of 6 we get a period of 10.240
+ * microseconds - just under 10Hz
+ */
+static u32 i915_oa_min_timer_exponent = 6;
+
 /* XXX: beware if future OA HW adds new report formats that the current
  * code assumes all reports have a power-of-two size and ~(size - 1) can
  * be used as a mask to align the OA tail pointer.
@@ -1353,21 +1369,14 @@ static int read_properties_unlocked(struct 
drm_i915_private *dev_priv,
return -EINVAL;
}

-   /* NB: The exponent represents a period as follows:
-*
-*   80ns * 2^(period_exponent + 1)
-*
-* Theoretically we can program the OA unit to sample
+   /* Theoretically we can program the OA unit to sample
 * every 160ns but don't allow that by default unless
 * root.
-*
-* Referring to perf's
-* kernel.perf_event_max_sample_rate for a precedent
-* (10 by default); with an OA exponent of 6 we get
-* a period of 10.240 microseconds -just under 10Hz
 */
-   if (value < 6 && !capable(CAP_SYS_ADMIN)) {
-   DRM_ERROR("Minimum OA sampling exponent is 6 
without root privileges\n");
+   if (value < i915_oa_min_timer_exponent &&
+   !capable(CAP_SYS_ADMIN)) {
+   DRM_ERROR("Minimum OA sampling exponent (sysctl 
dev.i915.oa_min_timer_exponent) is %u without root privileges\n",
+ i915_oa_min_timer_exponent);
return -EACCES;
}

@@ -1475,6 +1484,15 @@ static struct ctl_table oa_table[] = {
 .extra1 = ,
 .extra2 = ,
 },
+   {
+.procname = "oa_min_timer_exponent",
+.data = _oa_min_timer_exponent,
+.maxlen = sizeof(i915_oa_min_timer_exponent),
+.mode = 0644,
+.proc_handler = proc_dointvec_minmax,
+.extra1 = ,
+.extra2 = _exponent_max,
+},
{}
 };

-- 
2.10.1

[PATCH v8 09/12] drm/i915: Add dev.i915.perf_stream_paranoid sysctl option

[Robert Bragg]

Consistent with the kernel.perf_event_paranoid sysctl option that can
allow non-root users to access system wide cpu metrics, this can
optionally allow non-root users to access system wide OA counter metrics
from Gen graphics hardware.

Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/i915_perf.c | 50 +++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 01438fb..a138f86 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2171,6 +2171,7 @@ struct drm_i915_private {
bool initialized;

struct kobject *metrics_kobj;
+   struct ctl_table_header *sysctl_header;

struct mutex lock;
struct list_head streams;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 8d07c41..4e42073 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -64,6 +64,11 @@
 #define POLL_FREQUENCY 200
 #define POLL_PERIOD (NSEC_PER_SEC / POLL_FREQUENCY)

+/* for sysctl proc_dointvec_minmax of dev.i915.perf_stream_paranoid */
+static int zero;
+static int one = 1;
+static u32 i915_perf_stream_paranoid = true;
+
 /* The maximum exponent the hardware accepts is 63 (essentially it selects one
  * of the 64bit timestamp bits to trigger reports from) but there's currently
  * no known use case for sampling as infrequently as once per 47 thousand 
years.
@@ -1207,7 +1212,13 @@ i915_perf_open_ioctl_locked(struct drm_i915_private 
*dev_priv,
}
}

-   if (!specific_ctx && !capable(CAP_SYS_ADMIN)) {
+   /* Similar to perf's kernel.perf_paranoid_cpu sysctl option
+* we check a dev.i915.perf_stream_paranoid sysctl option
+* to determine if it's ok to access system wide OA counters
+* without CAP_SYS_ADMIN privileges.
+*/
+   if (!specific_ctx &&
+   i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
DRM_ERROR("Insufficient privileges to open system-wide i915 
perf stream\n");
ret = -EACCES;
goto err_ctx;
@@ -1454,6 +1465,39 @@ void i915_perf_unregister(struct drm_i915_private 
*dev_priv)
dev_priv->perf.metrics_kobj = NULL;
 }

+static struct ctl_table oa_table[] = {
+   {
+.procname = "perf_stream_paranoid",
+.data = _perf_stream_paranoid,
+.maxlen = sizeof(i915_perf_stream_paranoid),
+.mode = 0644,
+.proc_handler = proc_dointvec_minmax,
+.extra1 = ,
+.extra2 = ,
+},
+   {}
+};
+
+static struct ctl_table i915_root[] = {
+   {
+.procname = "i915",
+.maxlen = 0,
+.mode = 0555,
+.child = oa_table,
+},
+   {}
+};
+
+static struct ctl_table dev_root[] = {
+   {
+.procname = "dev",
+.maxlen = 0,
+.mode = 0555,
+.child = i915_root,
+},
+   {}
+};
+
 void i915_perf_init(struct drm_i915_private *dev_priv)
 {
if (!IS_HASWELL(dev_priv))
@@ -1484,6 +1528,8 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
dev_priv->perf.oa.n_builtin_sets =
i915_oa_n_builtin_metric_sets_hsw;

+   dev_priv->perf.sysctl_header = register_sysctl_table(dev_root);
+
dev_priv->perf.initialized = true;
 }

@@ -1492,6 +1538,8 @@ void i915_perf_fini(struct drm_i915_private *dev_priv)
if (!dev_priv->perf.initialized)
return;

+   unregister_sysctl_table(dev_priv->perf.sysctl_header);
+
memset(_priv->perf.oa.ops, 0, sizeof(dev_priv->perf.oa.ops));
dev_priv->perf.initialized = false;
 }
-- 
2.10.1

[PATCH v8 08/12] drm/i915: advertise available metrics via sysfs

[Robert Bragg]

Each metric set is given a sysfs entry like:

/sys/class/drm/card0/metrics//id

This allows userspace to enumerate the specific sets that are available
for the current system. The 'id' file contains an unsigned integer that
can be used to open the associated metric set via
DRM_IOCTL_I915_PERF_OPEN. The  is a globally unique ID for a
specific OA unit register configuration that can be reliably used by
userspace as a key to lookup corresponding counter meta data and
normalization equations.

The guid registry is currently maintained as part of gputop along with
the XML metric set descriptions and code generation scripts, ref:

 https://github.com/rib/gputop
 > gputop-data/guids.xml
 > scripts/update-guids.py
 > gputop-data/oa-*.xml
 > scripts/i915-perf-kernelgen.py

 $ make -C gputop-data -f Makefile.xml SYSFS=1 WHITELIST=RenderBasic

Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/i915_drv.c|  5 
 drivers/gpu/drm/i915/i915_drv.h|  4 +++
 drivers/gpu/drm/i915/i915_oa_hsw.c | 51 +
 drivers/gpu/drm/i915/i915_oa_hsw.h |  4 +++
 drivers/gpu/drm/i915/i915_perf.c   | 52 ++
 5 files changed, 116 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 685c96e..29bc83b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1116,6 +1116,9 @@ static void i915_driver_register(struct drm_i915_private 
*dev_priv)
i915_debugfs_register(dev_priv);
i915_guc_register(dev_priv);
i915_setup_sysfs(dev_priv);
+
+   /* Depends on sysfs having been initialized */
+   i915_perf_register(dev_priv);
} else
DRM_ERROR("Failed to register driver for userspace access!\n");

@@ -1152,6 +1155,8 @@ static void i915_driver_unregister(struct 
drm_i915_private *dev_priv)
acpi_video_unregister();
intel_opregion_unregister(dev_priv);

+   i915_perf_unregister(dev_priv);
+
i915_teardown_sysfs(dev_priv);
i915_guc_unregister(dev_priv);
i915_debugfs_unregister(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dd2b4d3..01438fb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2170,6 +2170,8 @@ struct drm_i915_private {
struct {
bool initialized;

+   struct kobject *metrics_kobj;
+
struct mutex lock;
struct list_head streams;

@@ -3757,6 +3759,8 @@ int intel_engine_cmd_parser(struct intel_engine_cs 
*engine,
 /* i915_perf.c */
 extern void i915_perf_init(struct drm_i915_private *dev_priv);
 extern void i915_perf_fini(struct drm_i915_private *dev_priv);
+extern void i915_perf_register(struct drm_i915_private *dev_priv);
+extern void i915_perf_unregister(struct drm_i915_private *dev_priv);

 /* i915_suspend.c */
 extern int i915_save_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_oa_hsw.c 
b/drivers/gpu/drm/i915/i915_oa_hsw.c
index 8906380..6af25cf 100644
--- a/drivers/gpu/drm/i915/i915_oa_hsw.c
+++ b/drivers/gpu/drm/i915/i915_oa_hsw.c
@@ -24,6 +24,8 @@
  *
  */

+#include 
+
 #include "i915_drv.h"
 #include "i915_oa_hsw.h"

@@ -142,3 +144,52 @@ int i915_oa_select_metric_set_hsw(struct drm_i915_private 
*dev_priv)
return -ENODEV;
}
 }
+
+static ssize_t
+show_render_basic_id(struct device *kdev, struct device_attribute *attr, char 
*buf)
+{
+   return sprintf(buf, "%d\n", METRIC_SET_ID_RENDER_BASIC);
+}
+
+static struct device_attribute dev_attr_render_basic_id = {
+   .attr = { .name = "id", .mode = 0444 },
+   .show = show_render_basic_id,
+   .store = NULL,
+};
+
+static struct attribute *attrs_render_basic[] = {
+   _attr_render_basic_id.attr,
+   NULL,
+};
+
+static struct attribute_group group_render_basic = {
+   .name = "403d8832-1a27-4aa6-a64e-f5389ce7b212",
+   .attrs =  attrs_render_basic,
+};
+
+int
+i915_perf_register_sysfs_hsw(struct drm_i915_private *dev_priv)
+{
+   int mux_len;
+   int ret = 0;
+
+   if (get_render_basic_mux_config(dev_priv, _len)) {
+   ret = sysfs_create_group(dev_priv->perf.metrics_kobj, 
_render_basic);
+   if (ret)
+   goto error_render_basic;
+   }
+
+   return 0;
+
+error_render_basic:
+   return ret;
+}
+
+void
+i915_perf_unregister_sysfs_hsw(struct drm_i915_private *dev_priv)
+{
+   int mux_len;
+
+   if (get_render_basic_mux_config(dev_priv, _len))
+   sysfs_remove_group(dev_priv->perf.metrics_kobj, 
_render_basic);
+}
diff --git a/drivers/gpu/drm/i915/i915_oa_hsw.h 
b/drivers/gpu/drm/i915/i915_oa_hsw.h
index b618a1f..429a229 100644
--- a/drivers/gpu/drm/i915/i915_oa_hsw.h
+++ b/drivers/gpu/drm/i915/i915_oa_hsw.h
@@ -31,4 +31,8 @@ extern int 

[PATCH v8 07/12] drm/i915: Enable i915 perf stream for Haswell OA unit

[Robert Bragg]

Gen graphics hardware can be set up to periodically write snapshots of
performance counters into a circular buffer via its Observation
Architecture and this patch exposes that capability to userspace via the
i915 perf interface.

v2:
   Make sure to initialize ->specific_ctx_id when opening, without
   relying on _pin_notify hook, in case ctx already pinned.
v3:
   Revert back to pinning ctx upfront when opening stream, removing
   need to hook in to pinning and to update OACONTROL on the fly.

Cc: Chris Wilson 
Signed-off-by: Robert Bragg 
Signed-off-by: Zhenyu Wang 
---
 drivers/gpu/drm/i915/i915_drv.h  |   66 ++-
 drivers/gpu/drm/i915/i915_perf.c | 1036 +-
 drivers/gpu/drm/i915/i915_reg.h  |  338 +
 include/uapi/drm/i915_drm.h  |   71 ++-
 4 files changed, 1482 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f22adc4..dd2b4d3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1767,6 +1767,11 @@ struct intel_wm_config {
bool sprites_scaled;
 };

+struct i915_oa_format {
+   u32 format;
+   int size;
+};
+
 struct i915_oa_reg {
i915_reg_t addr;
u32 value;
@@ -1787,11 +1792,6 @@ struct i915_perf_stream_ops {
 */
void (*disable)(struct i915_perf_stream *stream);

-   /* Return: true if any i915 perf records are ready to read()
-* for this stream.
-*/
-   bool (*can_read)(struct i915_perf_stream *stream);
-
/* Call poll_wait, passing a wait queue that will be woken
 * once there is something ready to read() for the stream
 */
@@ -1801,9 +1801,7 @@ struct i915_perf_stream_ops {

/* For handling a blocking read, wait until there is something
 * to ready to read() for the stream. E.g. wait on the same
-* wait queue that would be passed to poll_wait() until
-* ->can_read() returns true (if its safe to call ->can_read()
-* without the i915 perf lock held).
+* wait queue that would be passed to poll_wait().
 */
int (*wait_unlocked)(struct i915_perf_stream *stream);

@@ -1843,11 +1841,28 @@ struct i915_perf_stream {
struct list_head link;

u32 sample_flags;
+   int sample_size;

struct i915_gem_context *ctx;
bool enabled;

-   struct i915_perf_stream_ops *ops;
+   const struct i915_perf_stream_ops *ops;
+};
+
+struct i915_oa_ops {
+   void (*init_oa_buffer)(struct drm_i915_private *dev_priv);
+   int (*enable_metric_set)(struct drm_i915_private *dev_priv);
+   void (*disable_metric_set)(struct drm_i915_private *dev_priv);
+   void (*oa_enable)(struct drm_i915_private *dev_priv);
+   void (*oa_disable)(struct drm_i915_private *dev_priv);
+   void (*update_oacontrol)(struct drm_i915_private *dev_priv);
+   void (*update_hw_ctx_id_locked)(struct drm_i915_private *dev_priv,
+   u32 ctx_id);
+   int (*read)(struct i915_perf_stream *stream,
+   char __user *buf,
+   size_t count,
+   size_t *offset);
+   bool (*oa_buffer_is_empty)(struct drm_i915_private *dev_priv);
 };

 struct drm_i915_private {
@@ -2154,16 +2169,47 @@ struct drm_i915_private {

struct {
bool initialized;
+
struct mutex lock;
struct list_head streams;

+   spinlock_t hook_lock;
+
struct {
-   u32 metrics_set;
+   struct i915_perf_stream *exclusive_stream;
+
+   u32 specific_ctx_id;
+   struct i915_vma *pinned_rcs_vma;
+
+   struct hrtimer poll_check_timer;
+   wait_queue_head_t poll_wq;
+   bool pollin;
+
+   bool periodic;
+   int period_exponent;
+   int timestamp_frequency;
+
+   int tail_margin;
+
+   int metrics_set;

const struct i915_oa_reg *mux_regs;
int mux_regs_len;
const struct i915_oa_reg *b_counter_regs;
int b_counter_regs_len;
+
+   struct {
+   struct i915_vma *vma;
+   u8 *vaddr;
+   int format;
+   int format_size;
+   } oa_buffer;
+
+   u32 gen7_latched_oastatus1;
+
+   struct i915_oa_ops ops;
+   const struct i915_oa_format *oa_formats;
+   int n_builtin_sets;
} oa;
} perf;

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index c45cf92..8b9cf0d 100644
--- 

[PATCH v8 06/12] drm/i915: Add 'render basic' Haswell OA unit config

[Robert Bragg]

Adds a static OA unit, MUX + B Counter configuration for basic render
metrics on Haswell. This is auto generated from an XML
description of metric sets, currently maintained in gputop, ref:

  https://github.com/rib/gputop
  > gputop-data/oa-*.xml
  > scripts/i915-perf-kernelgen.py

  $ make -C gputop-data -f Makefile.xml SYSFS=0 WHITELIST=RenderBasic

Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/Makefile  |   3 +-
 drivers/gpu/drm/i915/i915_drv.h|  14 
 drivers/gpu/drm/i915/i915_oa_hsw.c | 144 +
 drivers/gpu/drm/i915/i915_oa_hsw.h |  34 +
 4 files changed, 194 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/i915_oa_hsw.c
 create mode 100644 drivers/gpu/drm/i915/i915_oa_hsw.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 8d4e25f..ac0c3ad 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -114,7 +114,8 @@ i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
 i915-y += i915_vgpu.o

 # perf code
-i915-y += i915_perf.o
+i915-y += i915_perf.o \
+ i915_oa_hsw.o

 ifeq ($(CONFIG_DRM_I915_GVT),y)
 i915-y += intel_gvt.o
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7a65c0b..f22adc4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1767,6 +1767,11 @@ struct intel_wm_config {
bool sprites_scaled;
 };

+struct i915_oa_reg {
+   i915_reg_t addr;
+   u32 value;
+};
+
 struct i915_perf_stream;

 struct i915_perf_stream_ops {
@@ -2151,6 +2156,15 @@ struct drm_i915_private {
bool initialized;
struct mutex lock;
struct list_head streams;
+
+   struct {
+   u32 metrics_set;
+
+   const struct i915_oa_reg *mux_regs;
+   int mux_regs_len;
+   const struct i915_oa_reg *b_counter_regs;
+   int b_counter_regs_len;
+   } oa;
} perf;

/* Abstract the submission mechanism (legacy ringbuffer or execlists) 
away */
diff --git a/drivers/gpu/drm/i915/i915_oa_hsw.c 
b/drivers/gpu/drm/i915/i915_oa_hsw.c
new file mode 100644
index 000..8906380
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_oa_hsw.c
@@ -0,0 +1,144 @@
+/*
+ * Autogenerated file, DO NOT EDIT manually!
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "i915_drv.h"
+#include "i915_oa_hsw.h"
+
+enum metric_set_id {
+   METRIC_SET_ID_RENDER_BASIC = 1,
+};
+
+int i915_oa_n_builtin_metric_sets_hsw = 1;
+
+static const struct i915_oa_reg b_counter_config_render_basic[] = {
+   { _MMIO(0x2724), 0x0080 },
+   { _MMIO(0x2720), 0x },
+   { _MMIO(0x2714), 0x0080 },
+   { _MMIO(0x2710), 0x },
+};
+
+static const struct i915_oa_reg mux_config_render_basic[] = {
+   { _MMIO(0x253a4), 0x0160 },
+   { _MMIO(0x25440), 0x0010 },
+   { _MMIO(0x25128), 0x },
+   { _MMIO(0x2691c), 0x0800 },
+   { _MMIO(0x26aa0), 0x0150 },
+   { _MMIO(0x26b9c), 0x6000 },
+   { _MMIO(0x2791c), 0x0800 },
+   { _MMIO(0x27aa0), 0x0150 },
+   { _MMIO(0x27b9c), 0x6000 },
+   { _MMIO(0x2641c), 0x0400 },
+   { _MMIO(0x25380), 0x0010 },
+   { _MMIO(0x2538c), 0x },
+   { _MMIO(0x25384), 0x0800 },
+   { _MMIO(0x25400), 0x0004 },
+   { _MMIO(0x2540c), 0x06029000 },
+   { _MMIO(0x25410), 0x0002 },
+   { _MMIO(0x25404), 0x5c30 },
+   { _MMIO(0x25100), 0x0016 },
+   { _MMIO(0x25110), 0x0400 },
+   { _MMIO(0x25104), 0x },
+   { _MMIO(0x26804), 0x1211 },
+   { _MMIO(0x26884), 0x0100 },
+   { _MMIO(0x26900), 

[PATCH v8 05/12] drm/i915: don't whitelist oacontrol in cmd parser

[Robert Bragg]

Being able to program OACONTROL from a non-privileged batch buffer is
not sufficient to be able to configure the OA unit. This was originally
allowed to help enable Mesa to expose OA counters via the
INTEL_performance_query extension, but the current implementation based
on programming OACONTROL via a batch buffer isn't able to report useable
data without a more complete OA unit configuration. Mesa handles the
possibility that writes to OACONTROL may not be allowed and so only
advertises the extension after explicitly testing that a write to
OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
should be ok for userspace.

Removing this simplifies adding a new kernel api for configuring the OA
unit without needing to consider the possibility that userspace might
trample on OACONTROL state which we'd like to start managing within
the kernel instead. In particular running any Mesa based GL application
currently results in clearing OACONTROL when initializing which would
disable the capturing of metrics.

Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 38 ++
 1 file changed, 2 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index c45dd83..5152d6f 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -450,7 +450,6 @@ static const struct drm_i915_reg_descriptor 
gen7_render_regs[] = {
REG64(PS_INVOCATION_COUNT),
REG64(PS_DEPTH_COUNT),
REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
-   REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below. */
REG64(MI_PREDICATE_SRC0),
REG64(MI_PREDICATE_SRC1),
REG32(GEN7_3DPRIM_END_OFFSET),
@@ -1060,8 +1059,7 @@ bool intel_engine_needs_cmd_parser(struct intel_engine_cs 
*engine)
 static bool check_cmd(const struct intel_engine_cs *engine,
  const struct drm_i915_cmd_descriptor *desc,
  const u32 *cmd, u32 length,
- const bool is_master,
- bool *oacontrol_set)
+ const bool is_master)
 {
if (desc->flags & CMD_DESC_SKIP)
return true;
@@ -1099,31 +1097,6 @@ static bool check_cmd(const struct intel_engine_cs 
*engine,
}

/*
-* OACONTROL requires some special handling for
-* writes. We want to make sure that any batch which
-* enables OA also disables it before the end of the
-* batch. The goal is to prevent one process from
-* snooping on the perf data from another process. To do
-* that, we need to check the value that will be written
-* to the register. Hence, limit OACONTROL writes to
-* only MI_LOAD_REGISTER_IMM commands.
-*/
-   if (reg_addr == i915_mmio_reg_offset(GEN7_OACONTROL)) {
-   if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
-   DRM_DEBUG_DRIVER("CMD: Rejected LRM to 
OACONTROL\n");
-   return false;
-   }
-
-   if (desc->cmd.value == MI_LOAD_REGISTER_REG) {
-   DRM_DEBUG_DRIVER("CMD: Rejected LRR to 
OACONTROL\n");
-   return false;
-   }
-
-   if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1))
-   *oacontrol_set = (cmd[offset + 1] != 0);
-   }
-
-   /*
 * Check the value written to the register against the
 * allowed mask/value pair given in the whitelist entry.
 */
@@ -1214,7 +1187,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs 
*engine,
u32 *cmd, *batch_end;
struct drm_i915_cmd_descriptor default_desc = noop_desc;
const struct drm_i915_cmd_descriptor *desc = _desc;
-   bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */
bool needs_clflush_after = false;
int ret = 0;

@@ -1270,8 +1242,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs 
*engine,
break;
}

-   if (!check_cmd(engine, desc, cmd, length, is_master,
-  _set)) {
+   if (!check_cmd(engine, desc, cmd, length, is_master)) {
ret = -EACCES;
break;
}
@@ -1279,11 +1250,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs 
*engine,
cmd += length;
}

-

[PATCH v8 04/12] drm/i915: return EACCES for check_cmd() failures

[Robert Bragg]

check_cmd() is checking whether a command adheres to certain
restrictions that ensure it's safe to execute within a privileged batch
buffer. Returning false implies a privilege problem, not that the
command is invalid.

The distinction makes the difference between allowing the buffer to be
executed as an unprivileged batch buffer or returning an EINVAL error to
userspace without executing anything.

In a case where userspace may want to test whether it can successfully
write to a register that needs privileges the distinction may be
important and an EINVAL error may be considered fatal.

In particular this is currently true for Mesa, which includes a test for
whether OACONTROL can be written too, but Mesa treats any error when
flushing a batch buffer as fatal, calling exit(1).

As it is currently Mesa can gracefully handle a failure to write to
OACONTROL if the command parser is disabled, but if we were to remove
OACONTROL from the parser's whitelist then the returned EINVAL would
break Mesa applications as they attempt an OACONTROL write.

This bumps the command parser version from 7 to 8, as the change is
visible to userspace.

Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index fe34470..c45dd83 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1272,7 +1272,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs 
*engine,

if (!check_cmd(engine, desc, cmd, length, is_master,
   _set)) {
-   ret = -EINVAL;
+   ret = -EACCES;
break;
}

@@ -1333,6 +1333,9 @@ int i915_cmd_parser_get_version(struct drm_i915_private 
*dev_priv)
 * 5. GPGPU dispatch compute indirect registers.
 * 6. TIMESTAMP register and Haswell CS GPR registers
 * 7. Allow MI_LOAD_REGISTER_REG between whitelisted registers.
+* 8. Don't report cmd_check() failures as EINVAL errors to userspace;
+*rely on the HW to NOOP disallowed commands as it would without
+*the parser enabled.
 */
-   return 7;
+   return 8;
 }
-- 
2.10.1

[PATCH v8 03/12] drm/i915: rename OACONTROL GEN7_OACONTROL

[Robert Bragg]

OACONTROL changes quite a bit for gen8, with some bits split out into a
per-context OACTXCONTROL register. Rename now before adding more gen7 OA
registers

Signed-off-by: Robert Bragg 
Reviewed-by: Matthew Auld 
---
 drivers/gpu/drm/i915/gvt/handlers.c| 2 +-
 drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
 drivers/gpu/drm/i915/i915_reg.h| 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
b/drivers/gpu/drm/i915/gvt/handlers.c
index 9ab1f95..4527cb7 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -2177,7 +2177,7 @@ static int init_generic_mmio_info(struct intel_gvt *gvt)
MMIO_DFH(0x1217c, D_ALL, F_CMD_ACCESS, NULL, NULL);

MMIO_F(0x2290, 8, 0, 0, 0, D_HSW_PLUS, NULL, NULL);
-   MMIO_D(OACONTROL, D_HSW);
+   MMIO_D(GEN7_OACONTROL, D_HSW);
MMIO_D(0x2b00, D_BDW_PLUS);
MMIO_D(0x2360, D_BDW_PLUS);
MMIO_F(0x5200, 32, 0, 0, 0, D_ALL, NULL, NULL);
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index f191d7b..fe34470 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -450,7 +450,7 @@ static const struct drm_i915_reg_descriptor 
gen7_render_regs[] = {
REG64(PS_INVOCATION_COUNT),
REG64(PS_DEPTH_COUNT),
REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
-   REG32(OACONTROL), /* Only allowed for LRI and SRM. See below. */
+   REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below. */
REG64(MI_PREDICATE_SRC0),
REG64(MI_PREDICATE_SRC1),
REG32(GEN7_3DPRIM_END_OFFSET),
@@ -1108,7 +1108,7 @@ static bool check_cmd(const struct intel_engine_cs 
*engine,
 * to the register. Hence, limit OACONTROL writes to
 * only MI_LOAD_REGISTER_IMM commands.
 */
-   if (reg_addr == i915_mmio_reg_offset(OACONTROL)) {
+   if (reg_addr == i915_mmio_reg_offset(GEN7_OACONTROL)) {
if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
DRM_DEBUG_DRIVER("CMD: Rejected LRM to 
OACONTROL\n");
return false;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 542e570..59628d5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -615,7 +615,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define HSW_CS_GPR(n)   _MMIO(0x2600 + (n) * 8)
 #define HSW_CS_GPR_UDW(n)   _MMIO(0x2600 + (n) * 8 + 4)

-#define OACONTROL _MMIO(0x2360)
+#define GEN7_OACONTROL _MMIO(0x2360)

 #define _GEN7_PIPEA_DE_LOAD_SL 0x70068
 #define _GEN7_PIPEB_DE_LOAD_SL 0x71068
-- 
2.10.1

[PATCH v8 02/12] drm/i915: Add i915 perf infrastructure

[Robert Bragg]

Adds base i915 perf infrastructure for Gen performance metrics.

This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an array of uint64
properties to configure a stream of metrics and returns a new fd usable
with standard VFS system calls including read() to read typed and sized
records; ioctl() to enable or disable capture and poll() to wait for
data.

A stream is opened something like:

  uint64_t properties[] = {
  /* Single context sampling */
  DRM_I915_PERF_PROP_CTX_HANDLE,ctx_handle,

  /* Include OA reports in samples */
  DRM_I915_PERF_PROP_SAMPLE_OA, true,

  /* OA unit configuration */
  DRM_I915_PERF_PROP_OA_METRICS_SET,metrics_set_id,
  DRM_I915_PERF_PROP_OA_FORMAT, report_format,
  DRM_I915_PERF_PROP_OA_EXPONENT,   period_exponent,
   };
   struct drm_i915_perf_open_param parm = {
  .flags = I915_PERF_FLAG_FD_CLOEXEC |
   I915_PERF_FLAG_FD_NONBLOCK |
   I915_PERF_FLAG_DISABLED,
  .properties_ptr = (uint64_t)properties,
  .num_properties = sizeof(properties) / 16,
   };
   int fd = drmIoctl(drm_fd, DRM_IOCTL_I915_PERF_OPEN, );

Records read all start with a common { type, size } header with
DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample records
contain an extensible number of fields and it's the
DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening that
determine what's included in every sample.

No specific streams are supported yet so any attempt to open a stream
will return an error.

v2:
use i915_gem_context_get() - Chris Wilson
v3:
update read() interface to avoid passing state struct - Chris Wilson
fix some rebase fallout, with i915-perf init/deinit
v4:
s/DRM_IORW/DRM_IOW/ - Emil Velikov

Signed-off-by: Robert Bragg 
---
 drivers/gpu/drm/i915/Makefile|   3 +
 drivers/gpu/drm/i915/i915_drv.c  |   4 +
 drivers/gpu/drm/i915/i915_drv.h  |  91 
 drivers/gpu/drm/i915/i915_perf.c | 443 +++
 include/uapi/drm/i915_drm.h  |  67 ++
 5 files changed, 608 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_perf.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 6123400..8d4e25f 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -113,6 +113,9 @@ i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
 # virtual gpu code
 i915-y += i915_vgpu.o

+# perf code
+i915-y += i915_perf.o
+
 ifeq ($(CONFIG_DRM_I915_GVT),y)
 i915-y += intel_gvt.o
 include $(src)/gvt/Makefile
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index af3559d..685c96e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -836,6 +836,8 @@ static int i915_driver_init_early(struct drm_i915_private 
*dev_priv,

intel_detect_preproduction_hw(dev_priv);

+   i915_perf_init(dev_priv);
+
return 0;

 err_workqueues:
@@ -849,6 +851,7 @@ static int i915_driver_init_early(struct drm_i915_private 
*dev_priv,
  */
 static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
 {
+   i915_perf_fini(dev_priv);
i915_gem_load_cleanup(_priv->drm);
i915_workqueues_cleanup(dev_priv);
 }
@@ -2556,6 +2559,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, 
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, 
i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, 
i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, 
DRM_RENDER_ALLOW),
 };

 static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5a260db..7a65c0b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1767,6 +1767,84 @@ struct intel_wm_config {
bool sprites_scaled;
 };

+struct i915_perf_stream;
+
+struct i915_perf_stream_ops {
+   /* Enables the collection of HW samples, either in response to
+* I915_PERF_IOCTL_ENABLE or implicitly called when stream is
+* opened without I915_PERF_FLAG_DISABLED.
+*/
+   void (*enable)(struct i915_perf_stream *stream);
+
+   /* Disables the collection of HW samples, either in response to
+* I915_PERF_IOCTL_DISABLE or implicitly called before
+* destroying the stream.
+*/
+   void (*disable)(struct i915_perf_stream *stream);
+
+   /* Return: true if any i915 perf records are ready to read()
+* for this stream.
+*/
+   bool (*can_read)(struct i915_perf_stream *stream);
+
+   /* Call poll_wait, passing a wait queue that will be woken
+* once there is something ready to read() for the stream
+*/
+   void (*poll_wait)(struct i915_perf_stream *stream,
+

[PATCH v8 01/12] ctx-pin placeholder from chris

[Robert Bragg]

From: Chris Wilson 

---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem_context.c | 34 ++---
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 55afb66..5a260db 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3437,6 +3437,7 @@ struct drm_i915_gem_object *
 i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
 struct i915_gem_context *
 i915_gem_context_create_gvt(struct drm_device *dev);
+struct i915_vma *i915_gem_context_pin_legacy(struct i915_gem_context *ctx);

 static inline struct i915_gem_context *
 i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 5dca32a..a620e15b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -751,12 +751,31 @@ needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
return false;
 }

+struct i915_vma *i915_gem_context_pin_legacy(struct i915_gem_context *ctx)
+{
+   struct i915_vma *vma = ctx->engine[RCS].state;
+   int ret;
+
+   /* Clear this page out of any CPU caches for coherent swap-in/out. */
+   if (!(vma->flags & I915_VMA_GLOBAL_BIND)) {
+   ret = i915_gem_object_set_to_gtt_domain(vma->obj, false);
+   if (ret)
+   return ERR_PTR(ret);
+   }
+
+   ret = i915_vma_pin(vma, 0, ctx->ggtt_alignment, PIN_GLOBAL);
+   if (ret)
+   return ERR_PTR(ret);
+
+   return vma;
+}
+
 static int do_rcs_switch(struct drm_i915_gem_request *req)
 {
struct i915_gem_context *to = req->ctx;
struct intel_engine_cs *engine = req->engine;
struct i915_hw_ppgtt *ppgtt = to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
-   struct i915_vma *vma = to->engine[RCS].state;
+   struct i915_vma *vma;
struct i915_gem_context *from;
u32 hw_flags;
int ret, i;
@@ -764,17 +783,10 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
if (skip_rcs_switch(ppgtt, engine, to))
return 0;

-   /* Clear this page out of any CPU caches for coherent swap-in/out. */
-   if (!(vma->flags & I915_VMA_GLOBAL_BIND)) {
-   ret = i915_gem_object_set_to_gtt_domain(vma->obj, false);
-   if (ret)
-   return ret;
-   }
-
/* Trying to pin first makes error handling easier. */
-   ret = i915_vma_pin(vma, 0, to->ggtt_alignment, PIN_GLOBAL);
-   if (ret)
-   return ret;
+   vma = i915_gem_context_pin_legacy(to);
+   if (IS_ERR(vma))
+   return PTR_ERR(vma);

/*
 * Pin can switch back to the default context if we end up calling into
-- 
2.10.1

[PATCH v8 00/12] Enable i915 perf stream for Haswell OA unit

[Robert Bragg]

Rebased on nightly, and updated as per review from Matt and Chris

The first patch from Chris adds an i915_gem_context_pin_legacy() utility that
I'm depending on now - though it doesn't really form part of the i915-perf
series proper. I'm assuming Chris plans to send a version of this to the list
himself with a proper commit message.

- Robert

Chris Wilson (1):
  ctx-pin placeholder from chris

Robert Bragg (11):
  drm/i915: Add i915 perf infrastructure
  drm/i915: rename OACONTROL GEN7_OACONTROL
  drm/i915: return EACCES for check_cmd() failures
  drm/i915: don't whitelist oacontrol in cmd parser
  drm/i915: Add 'render basic' Haswell OA unit config
  drm/i915: Enable i915 perf stream for Haswell OA unit
  drm/i915: advertise available metrics via sysfs
  drm/i915: Add dev.i915.perf_stream_paranoid sysctl option
  drm/i915: add oa_event_min_timer_exponent sysctl
  drm/i915: Add more Haswell OA metric sets
  drm/i915: Add a kerneldoc summary for i915_perf.c

 drivers/gpu/drm/i915/Makefile   |4 +
 drivers/gpu/drm/i915/gvt/handlers.c |2 +-
 drivers/gpu/drm/i915/i915_cmd_parser.c  |   45 +-
 drivers/gpu/drm/i915/i915_drv.c |9 +
 drivers/gpu/drm/i915/i915_drv.h |  157 +++
 drivers/gpu/drm/i915/i915_gem_context.c |   34 +-
 drivers/gpu/drm/i915/i915_oa_hsw.c  |  752 ++
 drivers/gpu/drm/i915/i915_oa_hsw.h  |   38 +
 drivers/gpu/drm/i915/i915_perf.c| 1726 +++
 drivers/gpu/drm/i915/i915_reg.h |  340 +-
 include/uapi/drm/i915_drm.h |  134 +++
 11 files changed, 3190 insertions(+), 51 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_oa_hsw.c
 create mode 100644 drivers/gpu/drm/i915/i915_oa_hsw.h
 create mode 100644 drivers/gpu/drm/i915/i915_perf.c

-- 
2.10.1

[PATCH][resend] drm: bridge: add DesignWare HDMI I2S audio support

[Kuninori Morimoto]

From: Kuninori Morimoto 

Current dw-hdmi is supporting sound via AHB bus, but it has
I2S audio feature too. This patch adds I2S audio support to dw-hdmi.
This HDMI I2S is supported by using ALSA SoC common HDMI encoder
driver.

Signed-off-by: Kuninori Morimoto 
---
 drivers/gpu/drm/bridge/Kconfig |   8 ++
 drivers/gpu/drm/bridge/Makefile|   1 +
 drivers/gpu/drm/bridge/dw-hdmi-audio.h |   7 ++
 drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c | 130 +
 drivers/gpu/drm/bridge/dw-hdmi.c   |  22 -
 drivers/gpu/drm/bridge/dw-hdmi.h   |  21 +
 6 files changed, 187 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index b590e67..4ed891e 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -32,6 +32,14 @@ config DRM_DW_HDMI_AHB_AUDIO
  Designware HDMI block.  This is used in conjunction with
  the i.MX6 HDMI driver.

+config DRM_DW_HDMI_I2S_AUDIO
+   tristate "Synopsis Designware I2S Audio interface"
+   depends on DRM_DW_HDMI
+   select SND_SOC_HDMI_CODEC
+   help
+ Support the I2S Audio interface which is part of the Synopsis
+ Designware HDMI block.
+
 config DRM_NXP_PTN3460
tristate "NXP PTN3460 DP/LVDS bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index efdb07e..720e53e 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -3,6 +3,7 @@ ccflags-y := -Iinclude/drm
 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
 obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
+obj-$(CONFIG_DRM_DW_HDMI_I2S_AUDIO) += dw-hdmi-i2s-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
diff --git a/drivers/gpu/drm/bridge/dw-hdmi-audio.h 
b/drivers/gpu/drm/bridge/dw-hdmi-audio.h
index 91f631b..fd1f745 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi-audio.h
+++ b/drivers/gpu/drm/bridge/dw-hdmi-audio.h
@@ -11,4 +11,11 @@ struct dw_hdmi_audio_data {
u8 *eld;
 };

+struct dw_hdmi_i2s_audio_data {
+   struct dw_hdmi *hdmi;
+
+   void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
+   u8 (*read)(struct dw_hdmi *hdmi, int offset);
+};
+
 #endif
diff --git a/drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c 
b/drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c
new file mode 100644
index 000..7dd2091
--- /dev/null
+++ b/drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c
@@ -0,0 +1,130 @@
+/*
+ * dw-hdmi-i2s-audio.c
+ *
+ * Copyright (c) 2016 Kuninori Morimoto 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include 
+
+#include 
+
+#include "dw-hdmi.h"
+#include "dw-hdmi-audio.h"
+
+#define DRIVER_NAME "dw-hdmi-i2s-audio"
+
+static inline void hdmi_write(struct dw_hdmi_i2s_audio_data *audio,
+ u8 val, int offset)
+{
+   struct dw_hdmi *hdmi = audio->hdmi;
+
+   audio->write(hdmi, val, offset);
+}
+
+static inline u8 hdmi_read(struct dw_hdmi_i2s_audio_data *audio, int offset)
+{
+   struct dw_hdmi *hdmi = audio->hdmi;
+
+   return audio->read(hdmi, offset);
+}
+
+static int dw_hdmi_i2s_hw_params(struct device *dev, void *data,
+struct hdmi_codec_daifmt *fmt,
+struct hdmi_codec_params *hparms)
+{
+   struct dw_hdmi_i2s_audio_data *audio = data;
+   struct dw_hdmi *hdmi = audio->hdmi;
+   u8 conf0 = 0;
+   u8 conf1 = 0;
+   u8 inputclkfs = 0;
+
+   /* it cares I2S only */
+   if ((fmt->fmt != HDMI_I2S) ||
+   (fmt->bit_clk_master | fmt->frame_clk_master)) {
+   dev_err(dev, "unsupported format/settings\n");
+   return -EINVAL;
+   }
+
+   inputclkfs  = HDMI_AUD_INPUTCLKFS_64FS;
+   conf0   = HDMI_AUD_CONF0_I2S_ALL_ENABLE;
+
+   switch (hparms->sample_width) {
+   case 16:
+   conf1 = HDMI_AUD_CONF1_WIDTH_16;
+   break;
+   case 24:
+   case 32:
+   conf1 = HDMI_AUD_CONF1_WIDTH_24;
+   break;
+   }
+
+   dw_hdmi_set_sample_rate(hdmi, hparms->sample_rate);
+
+   hdmi_write(audio, inputclkfs, HDMI_AUD_INPUTCLKFS);
+   hdmi_write(audio, conf0, HDMI_AUD_CONF0);
+   hdmi_write(audio, conf1, HDMI_AUD_CONF1);
+
+   dw_hdmi_audio_enable(hdmi);
+
+   return 0;
+}
+
+static void dw_hdmi_i2s_audio_shutdown(struct device *dev, void *data)
+{
+   struct dw_hdmi_i2s_audio_data *audio = data;
+   struct dw_hdmi *hdmi = audio->hdmi;
+
+   

[linux-sunxi] [PATCH v5 2/7] ASoC: sunxi: Add a simple HDMI CODEC

[Chen-Yu Tsai]

On Fri, Oct 21, 2016 at 3:44 PM, Jean-Francois Moine  wrote:
> Allwinner's SoCs include support for both audio and video on HDMI.
> This patch defines a simple audio CODEC which may be used in sunxi
> HDMI video drivers.
>
> Signed-off-by: Jean-Francois Moine 

There's already a driver for basically the same thing:

sound/soc/codec/hdmi-codec.c

You use it by registering a sub-device from your hdmi driver, with the
proper platform_data and callbacks. Grep for HDMI_CODEC_DRV_NAME for
platforms already using it.

ChenYu

> ---
>  include/sound/sunxi_hdmi.h|  23 +
>  sound/soc/codecs/Kconfig  |   9 
>  sound/soc/codecs/Makefile |   2 +
>  sound/soc/codecs/sunxi-hdmi.c | 106 
> ++
>  4 files changed, 140 insertions(+)
>  create mode 100644 include/sound/sunxi_hdmi.h
>  create mode 100644 sound/soc/codecs/sunxi-hdmi.c
>
> diff --git a/include/sound/sunxi_hdmi.h b/include/sound/sunxi_hdmi.h
> new file mode 100644
> index 000..0986bb9
> --- /dev/null
> +++ b/include/sound/sunxi_hdmi.h
> @@ -0,0 +1,23 @@
> +#ifndef __SUNXI_HDMI_H__
> +#define __SUNXI_HDMI_H__
> +/*
> + * Copyright (C) 2016 Jean-François Moine
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +struct sunxi_hdmi_codec {
> +   u8 *eld;
> +   int (*set_audio_input)(struct device *dev,
> +   int enable,
> +   unsigned sample_rate,
> +   unsigned sample_bit);
> +};
> +
> +int sunxi_hdmi_codec_register(struct device *dev);
> +void sunxi_hdmi_codec_unregister(struct device *dev);
> +
> +#endif /* __SUNXI_HDMI_H__ */
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index c67667b..53385b1 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -131,6 +131,7 @@ config SND_SOC_ALL_CODECS
> select SND_SOC_STA529 if I2C
> select SND_SOC_STAC9766 if SND_SOC_AC97_BUS
> select SND_SOC_STI_SAS
> +   select SND_SOC_SUNXI_HDMI
> select SND_SOC_TAS2552 if I2C
> select SND_SOC_TAS5086 if I2C
> select SND_SOC_TAS571X if I2C
> @@ -793,6 +794,14 @@ config SND_SOC_STAC9766
>  config SND_SOC_STI_SAS
> tristate "codec Audio support for STI SAS codec"
>
> +config SND_SOC_SUNXI_HDMI
> +   tristate "Allwinner sunxi HDMI Support"
> +   default m if DRM_SUNXI_DE2_HDMI=m
> +   default y if DRM_SUNXI_DE2_HDMI=y
> +   select SND_PCM_ELD
> +   help
> + Enable HDMI audio output
> +
>  config SND_SOC_TAS2552
> tristate "Texas Instruments TAS2552 Mono Audio amplifier"
> depends on I2C
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index 958cd49..35804eb 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -139,6 +139,7 @@ snd-soc-sta350-objs := sta350.o
>  snd-soc-sta529-objs := sta529.o
>  snd-soc-stac9766-objs := stac9766.o
>  snd-soc-sti-sas-objs := sti-sas.o
> +snd-soc-sunxi-hdmi-objs := sunxi-hdmi.o
>  snd-soc-tas5086-objs := tas5086.o
>  snd-soc-tas571x-objs := tas571x.o
>  snd-soc-tas5720-objs := tas5720.o
> @@ -359,6 +360,7 @@ obj-$(CONFIG_SND_SOC_STA350)   += snd-soc-sta350.o
>  obj-$(CONFIG_SND_SOC_STA529)   += snd-soc-sta529.o
>  obj-$(CONFIG_SND_SOC_STAC9766) += snd-soc-stac9766.o
>  obj-$(CONFIG_SND_SOC_STI_SAS)  += snd-soc-sti-sas.o
> +obj-$(CONFIG_SND_SOC_SUNXI_HDMI)   += snd-soc-sunxi-hdmi.o
>  obj-$(CONFIG_SND_SOC_TAS2552)  += snd-soc-tas2552.o
>  obj-$(CONFIG_SND_SOC_TAS5086)  += snd-soc-tas5086.o
>  obj-$(CONFIG_SND_SOC_TAS571X)  += snd-soc-tas571x.o
> diff --git a/sound/soc/codecs/sunxi-hdmi.c b/sound/soc/codecs/sunxi-hdmi.c
> new file mode 100644
> index 000..0d08676
> --- /dev/null
> +++ b/sound/soc/codecs/sunxi-hdmi.c
> @@ -0,0 +1,106 @@
> +/*
> + * Allwinner HDMI codec
> + *
> + * Copyright (C) 2016 Jean-Francois Moine 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "sound/sunxi_hdmi.h"
> +
> +#define SUNXI_HDMI_FORMATS (SNDRV_PCM_FMTBIT_S8 | \
> + SNDRV_PCM_FMTBIT_S16_LE | \
> + SNDRV_PCM_FMTBIT_S20_3LE | \
> + SNDRV_PCM_FMTBIT_S24_LE | \
> + SNDRV_PCM_FMTBIT_S32_LE)
> +
> +static int sunxi_hdmi_codec_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> +   struct snd_pcm_runtime *runtime = 

[PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2

[Maxime Ripard]

dinates for example.

> > > +void de2_de_plane_disable(struct priv *priv,
> > > + int lcd_num, int plane_ix)
> > > +{
> > > + void __iomem *mux_o = priv->mmio;
> > > + void __iomem *chan_o;
> > > + u32 fcolor;
> > > + int chan, layer, chan_disable = 0;
> > > + unsigned long flags;
> > > +
> > > + chan = plane2layer[plane_ix].chan;
> > > + layer = plane2layer[plane_ix].layer;
> > > +
> > > + mux_o += (lcd_num == 0) ? DE_MUX0_BASE : DE_MUX1_BASE;
> > > + chan_o = mux_o;
> > > + chan_o += DE_MUX_CHAN_REGS + DE_MUX_CHAN_SZ * chan;
> > > +
> > > + /* (only 2 layers) */
> > > + if (chan == 0) {
> > > + if (vi_read(chan_o, cfg[1 - layer].attr) == 0)
> > > + chan_disable = 1;
> > > + } else {
> > > + if (ui_read(chan_o, cfg[1 - layer].attr) == 0)
> > > + chan_disable = 1;
> > > + }
> > > +
> > > + spin_lock_irqsave(_lock, flags);
> > > +
> > > + fcolor = bld_read(mux_o + DE_MUX_BLD_REGS, fcolor_ctl);
> > > +
> > > + de_lcd_select(priv, lcd_num, mux_o);
> > > +
> > > + if (chan == 0)
> > > + vi_write(chan_o, cfg[layer].attr, 0);
> > > + else
> > > + ui_write(chan_o, cfg[layer].attr, 0);
> > > +
> > > + if (chan_disable)
> > > + bld_write(mux_o + DE_MUX_BLD_REGS, fcolor_ctl,
> > > + fcolor & ~(0x100 << plane2layer[plane_ix].pipe));
> > > +
> > > + spin_unlock_irqrestore(_lock, flags);
> > > +}
> > 
> > Can't you just disable it?
> 
> Which 'it'? A layer must be disabled and it is not useful to let the
> DE2 processor to scan a pipe (channel) without any layer.

Oh, so that's what it does.

I really think that you should put as much comments as possible on
what you found out working on this, especially because of the lack of
documentation.

> > > +static int __init de2_drm_init(void)
> > > +{
> > > + int ret;
> > > +
> > > +/* uncomment to activate the drm traces at startup time */
> > > +/*   drm_debug = DRM_UT_CORE | DRM_UT_DRIVER | DRM_UT_KMS |
> > > + DRM_UT_PRIME | DRM_UT_ATOMIC; */
> > 
> > That's useless.
> 
> Right, but it seems that some people don't know how to debug a DRM
> driver. This is only a reminder.
> 
> > > + DRM_DEBUG_DRIVER("\n");
> > > +
> > > + ret = platform_driver_register(_lcd_platform_driver);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + ret = platform_driver_register(_drm_platform_driver);
> > > + if (ret < 0)
> > > + platform_driver_unregister(_lcd_platform_driver);
> > > +
> > > + return ret;
> > > +}
> > 
> > And that really shouldn't be done that way.
> 
> May you explain?

This goes against the whole idea of the device and driver
model. Drivers should only register themselves, device should be
created by buses (or by using some external components if the bus
can't: DT, ACPI, etc.). If there's a match, you get probed.

A driver that creates its own device just to probe itself violates
that.

> > > +int de2_plane_init(struct drm_device *drm, struct lcd *lcd)
> > > +{
> > > + int ret, possible_crtcs = 1 << lcd->crtc_idx;
> > > +
> > > + ret = de2_one_plane_init(drm, >planes[DE2_PRIMARY_PLANE],
> > > + DRM_PLANE_TYPE_PRIMARY, possible_crtcs,
> > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > + if (ret >= 0)
> > > + ret = de2_one_plane_init(drm, >planes[DE2_CURSOR_PLANE],
> > > + DRM_PLANE_TYPE_CURSOR, possible_crtcs,
> > > + ui_formats, ARRAY_SIZE(ui_formats));
> > 
> > Nothing looks really special about that cursor plane. Any reasion not
> > to make it an overlay?
> 
> As explained above (channel/layer/pipe plane definitions), the cursor
> cannot go in a channel lower or equal to the one of the primary plane.
> Then, it must be known and, so, have an explicit plane.

If you were to make it a plane, you could use atomic_check to check
this and make sure this doesn't happen. And you would gain a generic
plane that can be used for other purposes if needed.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161028/82ba1eba/attachment-0001.sig>

[PATCH v3 2/3] drm: zte: add initial vou drm driver

[Shawn Guo]

Hi Gustavo,

Thanks for looking at the patch.

> 2016-10-20 Shawn Guo :
> 
> > It adds the initial ZTE VOU display controller DRM driver.  There are
> > still some features to be added, like overlay plane, scaling, and more
> > output devices support.  But it's already useful with dual CRTCs and
> > HDMI monitor working.
> >
> > Signed-off-by: Shawn Guo 



> > +static void zx_hdmi_connector_destroy(struct drm_connector *connector)
> > +{
> > + drm_connector_unregister(connector);
> 
> drm_connector_unregister() is not needed anymore. DRM core will call it
> for you.
> 
> > + drm_connector_cleanup(connector);
> > +}
> > +
> > +static const struct drm_connector_funcs zx_hdmi_connector_funcs = {
> > + .dpms = drm_atomic_helper_connector_dpms,
> > + .fill_modes = drm_helper_probe_single_connector_modes,
> > + .detect = zx_hdmi_connector_detect,
> > + .destroy = zx_hdmi_connector_destroy,
> 
> Then here you can use drm_connector_cleanup() directly.

Okay, will do.

> > + .reset = drm_atomic_helper_connector_reset,
> > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > +};



> > +static void zx_crtc_atomic_begin(struct drm_crtc *crtc,
> > +  struct drm_crtc_state *state)
> > +{
> > + struct drm_pending_vblank_event *event = crtc->state->event;
> > +
> > + if (!event)
> > + return;
> > +
> > + crtc->state->event = NULL;
> > +
> > + spin_lock_irq(>dev->event_lock);
> > + if (drm_crtc_vblank_get(crtc) == 0)
> > + drm_crtc_arm_vblank_event(crtc, event);
> > + else
> > + drm_crtc_send_vblank_event(crtc, event);
> > + spin_unlock_irq(>dev->event_lock);
> 
> I think you may want to send the vblank event to userspace after you
> commit your planes, and not before.

I guess you are suggesting that the code should be implemented in
.atomic_flush hook instead, right?

> > +}
> > +
> > +static const struct drm_crtc_helper_funcs zx_crtc_helper_funcs = {
> > + .enable = zx_crtc_enable,
> > + .disable = zx_crtc_disable,
> > + .atomic_begin = zx_crtc_atomic_begin,
> > +};
> > +
> > +static const struct drm_crtc_funcs zx_crtc_funcs = {
> > + .destroy = drm_crtc_cleanup,
> > + .set_config = drm_atomic_helper_set_config,
> > + .page_flip = drm_atomic_helper_page_flip,
> > + .reset = drm_atomic_helper_crtc_reset,
> > + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> > + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> > +};
> > +
> > +static int zx_crtc_init(struct drm_device *drm, struct zx_vou_hw *vou,
> > + enum vou_chn_type chn_type)
> > +{
> > + struct device *dev = vou->dev;
> > + struct zx_layer_data data;
> > + struct zx_crtc *zcrtc;
> > + int ret;
> > +
> > + zcrtc = devm_kzalloc(dev, sizeof(*zcrtc), GFP_KERNEL);
> > + if (!zcrtc)
> > + return -ENOMEM;
> > +
> > + zcrtc->vou = vou;
> > + zcrtc->chn_type = chn_type;
> > +
> > + if (chn_type == VOU_CHN_MAIN) {
> > + data.layer = vou->osd + MAIN_GL_OFFSET;
> > + data.csc = vou->osd + MAIN_CSC_OFFSET;
> > + data.hbsc = vou->osd + MAIN_HBSC_OFFSET;
> > + data.rsz = vou->otfppu + MAIN_RSZ_OFFSET;
> > + zcrtc->chnreg = vou->osd + OSD_MAIN_CHN;
> > + zcrtc->regs = _crtc_regs;
> > + zcrtc->bits = _crtc_bits;
> > + } else {
> > + data.layer = vou->osd + AUX_GL_OFFSET;
> > + data.csc = vou->osd + AUX_CSC_OFFSET;
> > + data.hbsc = vou->osd + AUX_HBSC_OFFSET;
> > + data.rsz = vou->otfppu + AUX_RSZ_OFFSET;
> > + zcrtc->chnreg = vou->osd + OSD_AUX_CHN;
> > + zcrtc->regs = _crtc_regs;
> > + zcrtc->bits = _crtc_bits;
> > + }
> > +
> > + zcrtc->pixclk = devm_clk_get(dev, (chn_type == VOU_CHN_MAIN) ?
> > +   "main_wclk" : "aux_wclk");
> > + if (IS_ERR(zcrtc->pixclk)) {
> > + ret = PTR_ERR(zcrtc->pixclk);
> > + dev_err(dev, "failed to get pix clk: %d\n", ret);
> 
> DRM_ERROR() - here and in other places in your patch

I will follow Sean's suggestion to use DRM_DEV_* for these messages.

Shawn