commit a481daa88fd (drm/radeon: always apply pci shutdown callbacks) breaks reboot

2016-10-12 Thread Markus Trippelsdorf 
Since:

commit a481daa88fd4d6b54f25348972bba10b5f6a84d0
Author: Alex Deucher 
Date:   Thu Sep 22 14:43:50 2016 -0400

drm/radeon: always apply pci shutdown callbacks

We can't properly detect all hypervisors and we
need this to properly tear down the hardware.

I cannot reboot my machine anymore. Before reboot the monitor goes blank and
the machine stays in that state until I press the reset button.

Hardware is RS780.

-- 
Markus

[bug report] drm/vc4: Add support for drawing 3D frames.

2016-10-12 Thread Eric Anholt 
Dan Carpenter  writes:

> Hello Eric Anholt,
>
> The patch d5b1a78a772f: "drm/vc4: Add support for drawing 3D frames."
> from Nov 30, 2015, leads to the following static checker warning:
>
>   drivers/gpu/drm/vc4/vc4_gem.c:797 vc4_wait_for_seqno_ioctl_helper()
>   warn: ret is never (-4)
>
> drivers/gpu/drm/vc4/vc4_gem.c
>790  static int
>791  vc4_wait_for_seqno_ioctl_helper(struct drm_device *dev,
>792  uint64_t seqno,
>793  uint64_t *timeout_ns)
>794  {
>795  unsigned long start = jiffies;
>796  int ret = vc4_wait_for_seqno(dev, seqno, *timeout_ns, true);
>797  
>798  if ((ret == -EINTR || ret == -ERESTARTSYS) && *timeout_ns != 
> ~0ull) {
>  ^
> Presumably this could be removed?

I think so.  Want to send the patch?
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161012/6d50a618/attachment.sig>

[Intel-gfx] drm/i915: WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)

2016-10-12 Thread Paul Bolle 
On Wed, 2016-10-12 at 14:06 +0200, Paul Bolle wrote:
> That might take some time. Because bisecting always takes a long time
> and especially since hitting this WARNING sometimes takes over an hour.
> Anyhow, please prod me if I stay silent for too long.

For the record: I just had to power cycle this laptop because it got
into that lovely state where it just locks without accepting any input
(no, I don't have netconsole enabled).

Assuming this lockup is related: this could be more urgent than I
thought.


Paul Bolle

commit a481daa88fd (drm/radeon: always apply pci shutdown callbacks) breaks reboot

2016-10-12 Thread Deucher, Alexander 
> -Original Message-
> From: Markus Trippelsdorf [mailto:markus at trippelsdorf.de]
> Sent: Wednesday, October 12, 2016 4:40 PM
> To: Deucher, Alexander
> Cc: Koenig, Christian; dri-devel at lists.freedesktop.org
> Subject: commit a481daa88fd (drm/radeon: always apply pci shutdown
> callbacks) breaks reboot
> 
> Since:
> 
> commit a481daa88fd4d6b54f25348972bba10b5f6a84d0
> Author: Alex Deucher 
> Date:   Thu Sep 22 14:43:50 2016 -0400
> 
> drm/radeon: always apply pci shutdown callbacks
> 
> We can't properly detect all hypervisors and we
> need this to properly tear down the hardware.
> 
> I cannot reboot my machine anymore. Before reboot the monitor goes blank
> and
> the machine stays in that state until I press the reset button.
> 
> Hardware is RS780.

Fixed with this series:
https://lists.freedesktop.org/archives/amd-gfx/2016-October/002833.html

Alex

> 
> --
> Markus

[bug report] drm/vc4: Add support for drawing 3D frames.

2016-10-12 Thread Dan Carpenter 
Hello Eric Anholt,

The patch d5b1a78a772f: "drm/vc4: Add support for drawing 3D frames."
from Nov 30, 2015, leads to the following static checker warning:

drivers/gpu/drm/vc4/vc4_gem.c:797 vc4_wait_for_seqno_ioctl_helper()
warn: ret is never (-4)

drivers/gpu/drm/vc4/vc4_gem.c
   790  static int
   791  vc4_wait_for_seqno_ioctl_helper(struct drm_device *dev,
   792  uint64_t seqno,
   793  uint64_t *timeout_ns)
   794  {
   795  unsigned long start = jiffies;
   796  int ret = vc4_wait_for_seqno(dev, seqno, *timeout_ns, true);
   797  
   798  if ((ret == -EINTR || ret == -ERESTARTSYS) && *timeout_ns != 
~0ull) {
 ^
Presumably this could be removed?

   799  uint64_t delta = jiffies_to_nsecs(jiffies - start);
   800  
   801  if (*timeout_ns >= delta)
   802  *timeout_ns -= delta;
   803  }
   804  
   805  return ret;
   806  }


regards,
dan carpenter

[RFC] drm/fb-helper: reject any changes to the fbdev

2016-10-12 Thread Ville Syrjälä 
On Wed, Oct 12, 2016 at 08:55:45AM -0700, Stefan Agner wrote:
> On 2016-10-12 03:42, Ville Syrjälä wrote:
> > On Tue, Oct 11, 2016 at 04:15:04PM -0700, Stefan Agner wrote:
> >> The current fbdev emulation does not allow to push back changes in
> >> width, height or depth to KMS, hence reject any changes with an
> >> error. This makes sure that fbdev ioctl's fail properly and user
> >> space does not assume that changes succeeded.
> >>
> >> Signed-off-by: Stefan Agner 
> >> ---
> >> This rejects reconfiguration of framebuffer like
> >> fbset -rgba 5,6,5 -depth 16 (when in 24 bit mode by default)
> >> fbset -xres 123
> >>
> >> I think all users of drm_fb_helper_check_var use also the generic
> >> drm_fb_helper_set_par (or do otherwise not support changing size/
> >> depth). Hence, afaict, the change should be the right thing to do
> >> for all driver...
> >>
> >>  drivers/gpu/drm/drm_fb_helper.c | 13 -
> >>  1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> >> b/drivers/gpu/drm/drm_fb_helper.c
> >> index 03414bd..596c056 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -1211,11 +1211,14 @@ int drm_fb_helper_check_var(struct 
> >> fb_var_screeninfo *var,
> >>if (var->pixclock != 0 || in_dbg_master())
> >>return -EINVAL;
> >>
> >> -  /* Need to resize the fb object !!! */
> >> -  if (var->bits_per_pixel > fb->bits_per_pixel ||
> >> -  var->xres > fb->width || var->yres > fb->height ||
> >> -  var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
> >> -  DRM_DEBUG("fb userspace requested width/height/bpp is greater 
> >> than current fb "
> >> +  /*
> >> +   * Changes struct fb_var_screeninfo are currently not pushed back
> >> +   * to KMS, hence fail if different settings are requested.
> >> +   */
> >> +  if (var->bits_per_pixel != fb->bits_per_pixel ||
> >> +  var->xres != fb->width || var->yres != fb->height ||
> >> +  var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
> > 
> > This still looks somewhat incomplete. Sounds like we should just
> > reject changes to everything except xoffset/yoffset.
> > 
> 
> What other parameters would you check? struct fb_var_screeninfo also has
> timings, but that is not something which is handy right here...

We should check the timings I think since we don't allow chaning them.
Though I suppose we don't even populate them. But then the user
shouldn't be allowed try to set them either I guess.

> 
> One parameter we could check too is the depth calculated from the bit
> information, e.g.
> 
> 
>   switch (var->bits_per_pixel) {
>   case 16:
>   depth = (var->green.length == 6) ? 16 : 15;
>   break;
>   case 32:
>   depth = (var->transp.length > 0) ? 32 : 24;
>   break;
>   default:
>   depth = var->bits_per_pixel;
>   break;
>   }
> +
> + if (depth != fb->depth) {
> + DRM_DEBUG("fb userspace requested depth different than current 
> fb "
> +   "request %d vs. %dx\n", depth, fb->depth);
> + return -EINVAL;
> + }
> 
> Or, maybe better, we could use drm_mode_legacy_fb_format to convert
> bpp/depth to fourcc and check that against the pixel_format field...

Also the red/green/... definitions for the actual bits of the color
channels. .grayscale is there well, and some colorspace thing which
I don't even know what it does. And rotation...

So I think in the end it pretty much boils down to everything except
xoffset/yoffset ;)

> 
> --
> Stefan
> 
> 
> >> +  DRM_DEBUG("fb userspace requested width/height/bpp different 
> >> than current fb "
> >>  "request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
> >>  var->xres, var->yres, var->bits_per_pixel,
> >>  var->xres_virtual, var->yres_virtual,
> >> --
> >> 2.10.0
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

[PATCH libdrm] Add support for DRM_MODE_PAGE_FLIP_TARGET_* flags

2016-10-12 Thread Michel Dänzer 
From: Michel Dänzer 

Signed-off-by: Michel Dänzer 
---

The corresponding kernel changes have landed in Linus' tree.

 include/drm/drm.h  |  1 +
 include/drm/drm_mode.h | 39 ---
 xf86drmMode.c  | 16 
 xf86drmMode.h  |  3 +++
 4 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index b4ebaa9..3c5181d 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -636,6 +636,7 @@ struct drm_gem_open {
 #define DRM_CAP_CURSOR_WIDTH   0x8
 #define DRM_CAP_CURSOR_HEIGHT  0x9
 #define DRM_CAP_ADDFB2_MODIFIERS   0x10
+#define DRM_CAP_PAGE_FLIP_TARGET   0x11

 /** DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 7a7856e..e15a74d 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -514,7 +514,13 @@ struct drm_color_lut {

 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
-#define DRM_MODE_PAGE_FLIP_FLAGS 
(DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
+#define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
+#define DRM_MODE_PAGE_FLIP_TARGET_RELATIVE 0x8
+#define DRM_MODE_PAGE_FLIP_TARGET (DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE | \
+  DRM_MODE_PAGE_FLIP_TARGET_RELATIVE)
+#define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT | \
+ DRM_MODE_PAGE_FLIP_ASYNC | \
+ DRM_MODE_PAGE_FLIP_TARGET)

 /*
  * Request a page flip on the specified crtc.
@@ -537,8 +543,7 @@ struct drm_color_lut {
  * 'as soon as possible', meaning that it not delay waiting for vblank.
  * This may cause tearing on the screen.
  *
- * The reserved field must be zero until we figure out something
- * clever to use it for.
+ * The reserved field must be zero.
  */

 struct drm_mode_crtc_page_flip {
@@ -549,6 +554,34 @@ struct drm_mode_crtc_page_flip {
__u64 user_data;
 };

+/*
+ * Request a page flip on the specified crtc.
+ *
+ * Same as struct drm_mode_crtc_page_flip, but supports new flags and
+ * re-purposes the reserved field:
+ *
+ * The sequence field must be zero unless either of the
+ * DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags is specified. When
+ * the ABSOLUTE flag is specified, the sequence field denotes the absolute
+ * vblank sequence when the flip should take effect. When the RELATIVE
+ * flag is specified, the sequence field denotes the relative (to the
+ * current one when the ioctl is called) vblank sequence when the flip
+ * should take effect. NOTE: DRM_IOCTL_WAIT_VBLANK must still be used to
+ * make sure the vblank sequence before the target one has passed before
+ * calling this ioctl. The purpose of the
+ * DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags is merely to clarify
+ * the target for when code dealing with a page flip runs during a
+ * vertical blank period.
+ */
+
+struct drm_mode_crtc_page_flip_target {
+   __u32 crtc_id;
+   __u32 fb_id;
+   __u32 flags;
+   __u32 sequence;
+   __u64 user_data;
+};
+
 /* create a dumb scanout buffer */
 struct drm_mode_create_dumb {
__u32 height;
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 228c6e4..fb22f68 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -948,6 +948,22 @@ int drmModePageFlip(int fd, uint32_t crtc_id, uint32_t 
fb_id,
return DRM_IOCTL(fd, DRM_IOCTL_MODE_PAGE_FLIP, );
 }

+int drmModePageFlipTarget(int fd, uint32_t crtc_id, uint32_t fb_id,
+ uint32_t flags, void *user_data,
+ uint32_t target_vblank)
+{
+   struct drm_mode_crtc_page_flip_target flip_target;
+
+   memclear(flip_target);
+   flip_target.fb_id = fb_id;
+   flip_target.crtc_id = crtc_id;
+   flip_target.user_data = VOID2U64(user_data);
+   flip_target.flags = flags;
+   flip_target.sequence = target_vblank;
+
+   return DRM_IOCTL(fd, DRM_IOCTL_MODE_PAGE_FLIP, _target);
+}
+
 int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
uint32_t fb_id, uint32_t flags,
int32_t crtc_x, int32_t crtc_y,
diff --git a/xf86drmMode.h b/xf86drmMode.h
index 1a02fed..b684967 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -473,6 +473,9 @@ extern int drmModeCrtcGetGamma(int fd, uint32_t crtc_id, 
uint32_t size,
   uint16_t *red, uint16_t *green, uint16_t *blue);
 extern int drmModePageFlip(int fd, uint32_t crtc_id, uint32_t fb_id,
   uint32_t flags, void *user_data);
+extern int drmModePageFlipTarget(int fd, uint32_t crtc_id, uint32_t fb_id,
+uint32_t flags, void *user_data,
+uint32_t target_vblank);

 extern drmModePlaneResPtr drmModeGetPlaneResources(int fd);
 extern drmModePlanePtr drmModeGetPlane(int fd, uint32_t plane_id);
-- 
2.9.3

Question: Re: [PATCH] drm/bridge: analogix: protect power when get_modes or detect

2016-10-12 Thread Mark yao 
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161012/e2118efc/attachment.html>

[PATCH] drm/bridge: analogix: protect power when get_modes or detect

2016-10-12 Thread Mark Yao 
The drm callback ->detect and ->get_modes seems is not power safe,
they may be called when device is power off, do register access on
detect or get_modes will cause system die.

Here is the path call ->detect before analogix_dp power on
[] analogix_dp_detect+0x44/0xdc
[] 
drm_helper_probe_single_connector_modes_merge_bits+0xe8/0x41c
[] drm_helper_probe_single_connector_modes+0x10/0x18
[] drm_mode_getconnector+0xf4/0x304
[] drm_ioctl+0x23c/0x390
[] do_vfs_ioctl+0x4b8/0x58c
[] SyS_ioctl+0x60/0x88

Cc: Inki Dae 
Cc: Sean Paul 
Cc: Gustavo Padovan 
Cc: "Ville Syrjälä" 

Signed-off-by: Mark Yao 
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 28 ++
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index efac8ab..09dece2 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1062,6 +1062,13 @@ int analogix_dp_get_modes(struct drm_connector 
*connector)
return 0;
}

+   if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
+   pm_runtime_get_sync(dp->dev);
+
+   if (dp->plat_data->power_on)
+   dp->plat_data->power_on(dp->plat_data);
+   }
+
if (analogix_dp_handle_edid(dp) == 0) {
drm_mode_connector_update_edid_property(>connector, edid);
num_modes += drm_add_edid_modes(>connector, edid);
@@ -1073,6 +1080,13 @@ int analogix_dp_get_modes(struct drm_connector 
*connector)
if (dp->plat_data->get_modes)
num_modes += dp->plat_data->get_modes(dp->plat_data, connector);

+   if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
+   if (dp->plat_data->power_off)
+   dp->plat_data->power_off(dp->plat_data);
+
+   pm_runtime_put_sync(dp->dev);
+   }
+
ret = analogix_dp_prepare_panel(dp, false, false);
if (ret)
DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
@@ -1106,9 +1120,23 @@ analogix_dp_detect(struct drm_connector *connector, bool 
force)
return connector_status_disconnected;
}

+   if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
+   pm_runtime_get_sync(dp->dev);
+
+   if (dp->plat_data->power_on)
+   dp->plat_data->power_on(dp->plat_data);
+   }
+
if (!analogix_dp_detect_hpd(dp))
status = connector_status_connected;

+   if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
+   if (dp->plat_data->power_off)
+   dp->plat_data->power_off(dp->plat_data);
+
+   pm_runtime_put_sync(dp->dev);
+   }
+
ret = analogix_dp_prepare_panel(dp, false, false);
if (ret)
DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
-- 
1.9.1

[Intel-gfx] drm/i915: WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)

2016-10-12 Thread Jani Nikula 
On Wed, 12 Oct 2016, Paul Bolle  wrote:
> On Wed, 2016-10-12 at 14:08 +0300, Joonas Lahtinen wrote:
>> Bisecting the offending commit between v4.8 and v4.8.1 would be a good
>> start.
>
> That would be between v4.7 and v4.8. (I guess my report was ambiguous.)
>
> That might take some time. Because bisecting always takes a long time
> and especially since hitting this WARNING sometimes takes over an hour.
> Anyhow, please prod me if I stay silent for too long.

In the mean time, please file a bug over at [1] so we don't lose track.

BR,
Jani.

[1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to "tristate".

2016-10-12 Thread Jani Nikula 
On Wed, 12 Oct 2016, Emil Velikov  wrote:
> On 11 October 2016 at 10:33, Jani Nikula  
> wrote:
>> On Tue, 11 Oct 2016, "Sun, Jing A"  wrote:
>>> It's needed that DRM Driver module could be removed and reloaded after
>>> kernel booting on the projects that I have been working on, and I hope
>>> such module type change could be accepted. Looks like Iwai has similar
>>> change request as well. Would you please review it and let us know if
>>> any concerns?
>>
>> Looking at the Kconfig, selecting CONFIG_DRM_MIPI_DSI is against the
>> recommendations of Documentation/kbuild/kconfig-language.txt:
>>
>> select should be used with care. select will force
>> a symbol to a value without visiting the dependencies.
>> By abusing select you are able to select a symbol FOO even
>> if FOO depends on BAR that is not set.
>> In general use select only for non-visible symbols
>> (no prompts anywhere) and for symbols with no dependencies.
>> That will limit the usefulness but on the other hand avoid
>> the illegal configurations all over.
>>
>> Indeed, you may end up with CONFIG_DRM_MIPI_DSI=y and CONFIG_DRM=m,
>> which violates DRM_MIPI_DSI dependency on CONFIG_DRM. This is broken and
>> should be fixed. The suggested patch does *not* fix this issue.
>>
> Jani, git log suggests you as the unfortunate author of the select
> DRM_MIPI_DSI/select DRM_PANEL hunks in i915 ;-)

/o\

As much as my present self would like to scold my past self for all his
mistakes, I have to remind myself that it is the mistakes that have
given me invaluable experience that my past self didn't have. I can only
hope my future self will have time to fix even a fraction of the
mistakes.

Anyway, as Andrzej pointed out, all configs that select DRM_MIPI_DSI
also depend on DRM, so this problem can't currently occur. Once dsi bus
un-registration gets addressed, we can turn DRM_MIPI_DSI into a tristate
config (i.e. a loadable module).

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH] exynos-drm: Fix display manager failing to start without IOMMU problem

2016-10-12 Thread Shuah Khan 
On 10/12/2016 05:11 PM, Shuah Khan wrote:

+ Fixing Krzysztof Kozlowski address.

> Hi Inki,
> 
> On 08/15/2016 10:40 PM, Inki Dae wrote:
> 
>>>
>>> okay the very first commit that added IOMMU support
>>> introduced the code that rejects non-contig gem memory
>>> type without IOMMU.
>>>
>>> commit 0519f9a12d0113caab78980c48a7902d2bd40c2c
>>> Author: Inki Dae 
>>> Date:   Sat Oct 20 07:53:42 2012 -0700
>>>
>>> drm/exynos: add iommu support for exynos drm framework
>>>
> 
> I haven't given up on this yet. I am still seeing the following failure:
> 
> Additional debug messages I added:
> [   15.287403] exynos_drm_gem_create_ioctl() 1
> [   15.287419] exynos_drm_gem_create() flags 1
> 
> [   15.311511] [drm:exynos_drm_framebuffer_init] *ERROR* Non-contiguous GEM 
> memory is not supported.
> 
> Additional debug message I added:
> [   15.318981] [drm:exynos_user_fb_create] *ERROR* failed to initialize 
> framebuffer
> 
> This is what happens:
> 
> 1. exynos_drm_gem_create_ioctl() gets called with EXYNOS_BO_NONCONTIG request
> 2. exynos_drm_gem_create(0 goes ahead and creates the GEM buffers
> 3. exynos_user_fb_create() tries to associate GEM to fb and fails during
>check_fb_gem_memory_type()
> 
> At this point, there is no recovery and lightdm fails
> 
> xf86-video-armsoc/src/drmmode_exynos/drmmode_exynos.c assumes contiguous
> allocations are not supported in some exynos drm versions: The following
> commit introduced this change:
> 
> https://git.linaro.org/arm/xorg/driver/xf86-video-armsoc.git/commitdiff/3be1f6273441fe95dd442f44064387322e16b7e9
> 
> excerpts from the diff:-   if (create_gem->buf_type == ARMSOC_BO_SCANOUT)
> -   create_exynos.flags = EXYNOS_BO_CONTIG;
> -   else
> -   create_exynos.flags = EXYNOS_BO_NONCONTIG;
> +
> +   /* Contiguous allocations are not supported in some exynos drm 
> versions.
> +* When they are supported all allocations are effectively contiguous
> +* anyway, so for simplicity we always request non contiguous buffers.
> +*/
> +   create_exynos.flags = EXYNOS_BO_NONCONTIG;
> 
> There might have been logic on exynos_drm that forced Contig when it coudn't
> support NONCONTIG. At least, that is what this comment suggests. This 
> assumption
> doesn't appear to be a good one and not sure if this change was made to fix a 
> bug.
> 
> After the IOMMU support, this assumption is no longer true. Hence, with IOMMU
> support, latest kernels have a mismatch with the installed xf86-video-armsoc
> 
> This is what I am running into. This leads to the following question:
> 
> 1. How do we ensure exynos_drm kernel changes don't break user-space
>specifically xf86-video-armsoc
> 2. This seems to have gone undetected for a while. I see a change in
>exynos_drm_gem_dumb_create() that is probably addressing this type
>of breakage. Commit 122beea84bb90236b1ae545f08267af58591c21b adds
>handling for IOMMU NONCONTIG case.
> 
> Anyway, I am interested in getting the exynos_drm kernel side code
> and xf86-video-armsoc in sync to resolve the issue.
> 
> Could you recommend a going forward plan?
> 
> I can submit a patch to xf86-video-armsoc. I am also looking ahead to
> see if we can avoid such breaks in the future by keeping kernel and
> xf86-video-armsoc in sync.
> 
> thanks,
> -- Shuah
>

[PATCH] exynos-drm: Fix display manager failing to start without IOMMU problem

2016-10-12 Thread Shuah Khan 
Hi Inki,

On 08/15/2016 10:40 PM, Inki Dae wrote:

>>
>> okay the very first commit that added IOMMU support
>> introduced the code that rejects non-contig gem memory
>> type without IOMMU.
>>
>> commit 0519f9a12d0113caab78980c48a7902d2bd40c2c
>> Author: Inki Dae 
>> Date:   Sat Oct 20 07:53:42 2012 -0700
>>
>> drm/exynos: add iommu support for exynos drm framework
>>

I haven't given up on this yet. I am still seeing the following failure:

Additional debug messages I added:
[   15.287403] exynos_drm_gem_create_ioctl() 1
[   15.287419] exynos_drm_gem_create() flags 1

[   15.311511] [drm:exynos_drm_framebuffer_init] *ERROR* Non-contiguous GEM 
memory is not supported.

Additional debug message I added:
[   15.318981] [drm:exynos_user_fb_create] *ERROR* failed to initialize 
framebuffer

This is what happens:

1. exynos_drm_gem_create_ioctl() gets called with EXYNOS_BO_NONCONTIG request
2. exynos_drm_gem_create(0 goes ahead and creates the GEM buffers
3. exynos_user_fb_create() tries to associate GEM to fb and fails during
   check_fb_gem_memory_type()

At this point, there is no recovery and lightdm fails

xf86-video-armsoc/src/drmmode_exynos/drmmode_exynos.c assumes contiguous
allocations are not supported in some exynos drm versions: The following
commit introduced this change:

https://git.linaro.org/arm/xorg/driver/xf86-video-armsoc.git/commitdiff/3be1f6273441fe95dd442f44064387322e16b7e9

excerpts from the diff:-   if (create_gem->buf_type == ARMSOC_BO_SCANOUT)
-   create_exynos.flags = EXYNOS_BO_CONTIG;
-   else
-   create_exynos.flags = EXYNOS_BO_NONCONTIG;
+
+   /* Contiguous allocations are not supported in some exynos drm versions.
+* When they are supported all allocations are effectively contiguous
+* anyway, so for simplicity we always request non contiguous buffers.
+*/
+   create_exynos.flags = EXYNOS_BO_NONCONTIG;

There might have been logic on exynos_drm that forced Contig when it coudn't
support NONCONTIG. At least, that is what this comment suggests. This assumption
doesn't appear to be a good one and not sure if this change was made to fix a 
bug.

After the IOMMU support, this assumption is no longer true. Hence, with IOMMU
support, latest kernels have a mismatch with the installed xf86-video-armsoc

This is what I am running into. This leads to the following question:

1. How do we ensure exynos_drm kernel changes don't break user-space
   specifically xf86-video-armsoc
2. This seems to have gone undetected for a while. I see a change in
   exynos_drm_gem_dumb_create() that is probably addressing this type
   of breakage. Commit 122beea84bb90236b1ae545f08267af58591c21b adds
   handling for IOMMU NONCONTIG case.

Anyway, I am interested in getting the exynos_drm kernel side code
and xf86-video-armsoc in sync to resolve the issue.

Could you recommend a going forward plan?

I can submit a patch to xf86-video-armsoc. I am also looking ahead to
see if we can avoid such breaks in the future by keeping kernel and
xf86-video-armsoc in sync.

thanks,
-- Shuah

[PATCH libdrm] Add support for DRM_MODE_PAGE_FLIP_TARGET_* flags

2016-10-12 Thread Emil Velikov 
Hi Michel,

On 12 October 2016 at 10:41, Michel Dänzer  wrote:
> From: Michel Dänzer 
>
> Signed-off-by: Michel Dänzer 
> ---
>
> The corresponding kernel changes have landed in Linus' tree.
>
>  include/drm/drm.h  |  1 +
>  include/drm/drm_mode.h | 39 ---
For these two (patch 1/2) please follow the pre-existing pattern (git
log -- include/drm/), barring the top commit of course ;-) Namely: use
make headers_install & reference the tree/sha that files are based on.

If you can skim through & handle radeon/amdgpu_drm.h that'll be
greatly appreciated.

>  xf86drmMode.c  | 16 
>  xf86drmMode.h  |  3 +++
And here (patch 2/2) please mention some of the users.

Thanks
Emil

[Intel-gfx] drm/i915: WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)

2016-10-12 Thread Paul Bolle 
On Wed, 2016-10-12 at 17:34 +0300, Jani Nikula wrote:
> In the mean time, please file a bug over at [1] so we don't lose
> track.

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


Paul Bolle

[Bug 67713] Freezes on Trinity 7500G

2016-10-12 Thread bugzilla-dae...@freedesktop.org 
https://bugs.freedesktop.org/show_bug.cgi?id=67713

Erik  changed:

   What|Removed |Added

 Resolution|INVALID |---
 Status|RESOLVED|REOPENED

--- Comment #12 from Erik  ---
Hi guys, seems that the problem arises again. Now I've updated the kernel
4.4.23-1-lts #1, mesa 12.0.3-2.

Same error appears, while I try to extend my desktop to both monitors.

[  193.473517] [drm:atom_op_jump [radeon]] *ERROR* atombios stuck in loop for
more than 5secs aborting
[  193.473551] [drm:atom_execute_table_locked [radeon]] *ERROR* atombios stuck
executing 3336 (len 2642, WS 0, PS 8) @ 0x393E

Still with RADEON_VA=0 option (also tried without it).

-- 
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/20161012/29951c2a/attachment.html>

[Intel-gfx] [PATCH] drm/i915: Add i915 perf infrastructure

2016-10-12 Thread Joonas Lahtinen 
On ti, 2016-10-11 at 12:03 -0700, Robert Bragg wrote:
> > > +               case DRM_I915_PERF_PROP_MAX:
> > > +                       BUG();
> > 
> > We already handle this case above, but I guess we still need this in
> > order to silence gcc...
> 
> right, and preferable to having a default: case, for the future compiler 
> warning to handle any new properties here.

Please, do use MISSING_CASE instead. Daniel is known to get upset for
far less ;)

Generally consensus is that BUG() is used only when there're no other options 
to back out.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

[Intel-gfx] drm/i915: WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)

2016-10-12 Thread Joonas Lahtinen 
On ke, 2016-10-12 at 11:56 +0200, Paul Bolle wrote:
> On a laptop that tracks the latest stable release (Ie, it now runs
> v4.8.1) I see this WARNING
>     WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)
> 
> Full trace pasted below. I never saw this WARNING before v4.8. Since
> v4.8 I've had it in all (four, actually) boots.
> 
> What am I expected to do about this WARNING?
> 

Bisecting the offending commit between v4.8 and v4.8.1 would be a good
start.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

[Intel-gfx] drm/i915: WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)

2016-10-12 Thread Paul Bolle 
On Wed, 2016-10-12 at 14:08 +0300, Joonas Lahtinen wrote:
> Bisecting the offending commit between v4.8 and v4.8.1 would be a good
> start.

That would be between v4.7 and v4.8. (I guess my report was ambiguous.)

That might take some time. Because bisecting always takes a long time
and especially since hitting this WARNING sometimes takes over an hour.
Anyhow, please prod me if I stay silent for too long.

Thanks,


Paul Bolle

GPU-DRM-Savage: Less function calls in savage_bci_cmdbuf() after error detection

2016-10-12 Thread SF Markus Elfring 
>> Date: Thu, 18 Aug 2016 21:28:58 +0200
>>
>> The kfree() function was called in a few cases by the
>> savage_bci_cmdbuf() function during error handling
>> even if a passed variable contained a null pointer.
>>
>> Adjust jump targets according to the Linux coding style convention.
>>
>> Signed-off-by: Markus Elfring 
> 
> Not sure this is worth it, I'll pass. Patch 1 merged.

Unfortunately, it seems that this selection of only one update step
from this small patch series has got unwanted consequences.

Will the update suggestion “[patch] drm/savage: dereferencing an error 
pointer”
by Dan Carpenter (from today) trigger further software development discussions?

https://patchwork.kernel.org/patch/9372127/
https://lkml.kernel.org/r/<20161012062227.GU12841 at mwanda>


Will an update step like “[PATCH 2/2] GPU-DRM-Savage: Less function calls in
savage_bci_cmdbuf() after error detection” (from 2016-08-18) become worth
for related consideratons once more?

https://patchwork.kernel.org/patch/9289183/
https://lkml.kernel.org/r/

Regards,
Markus

[PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to "tristate".

2016-10-12 Thread Jani Nikula 
On Wed, 12 Oct 2016, "Sun, Jing A"  wrote:
> I think "installing a kernel with my changes for both drm and i915"
> takes more time and effort to complete than "only updating DRM/i915
> modules without rebuilding the whole kernel". In some cases, that's
> beneficial.

It's possible to change and rebuild and update just the drm and i915,
but you need to be careful to build against the same tree as the ones
you are replacing. This is like using out-of-tree modules (which is
something I can't recommend no matter what, but that's another
discussion).

However, this is completely different from planning to update drm and
i915 modules on a running production system by unloading the old ones
and probing the new ones. Don't do that. It will be a disaster.

> Also reloadablility is always a good thing to have and I truly hope
> Hajda/Iwai's patches would be accepted and merged.  No downside of it
> after all.

I think it's good to be able to unload and reload modules for debugging
and development, but not for normal use.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH] drm/amd/powerplay: don't give up if DPM is already running

2016-10-12 Thread Zhu, Rex 
Hi Grazvydas and Alex,

We needed to disable dpm when rmmod amdgpu for this issue.

I am checking the function of disable dpm task. 

Best Regards
Rex

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com] 
Sent: Wednesday, October 12, 2016 4:01 AM
To: Grazvydas Ignotas; Zhu, Rex
Cc: Maling list - DRI developers; amd-gfx list
Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already running

+Rex to review this.

Alex

On Sun, Oct 9, 2016 at 3:23 PM, Grazvydas Ignotas  wrote:
> Currently the driver crashes if smu7_enable_dpm_tasks() returns early, 
> which it does if DPM is already active. It seems to be better just to 
> continue anyway, at least I haven't noticed any ill effects. It's also 
> unclear at what state the hardware was left by the previous driver, so 
> IMO it's better to always fully initialize.
>
> Way to reproduce:
> $ modprobe amdgpu
> $ rmmod amdgpu
> $ modprobe amdgpu
> ...
> DPM is already running right now, no need to enable DPM!
> ...
> failed to send message 18b ret is 0
> BUG: unable to handle kernel paging request at ed01fc9ab21f Call 
> Trace:
>  smu7_set_power_state_tasks+0x499/0x1940 [amdgpu]
>  phm_set_power_state+0xcb/0x120 [amdgpu]
>  psm_adjust_power_state_dynamic+0x11e/0x1b0 [amdgpu]
>  pem_task_adjust_power_state+0xb9/0xd0 [amdgpu]
>  pem_excute_event_chain+0x7d/0xe0 [amdgpu]
>  pem_handle_event_unlocked+0x49/0x60 [amdgpu]
>  pem_handle_event+0xe/0x10 [amdgpu]
>  pp_dpm_dispatch_tasks+0xe0/0x190 [amdgpu]
>  amdgpu_pm_compute_clocks+0x10c/0xc60 [amdgpu]
>  dce_v11_0_crtc_dpms+0x7d/0x150 [amdgpu]
>  dce_v11_0_crtc_disable+0x90/0x4a0 [amdgpu]
>  drm_helper_disable_unused_functions+0x67/0x80 [drm_kms_helper]
>  amdgpu_fbdev_init+0x13e/0x170 [amdgpu]
>  amdgpu_device_init+0x1aeb/0x2490 [amdgpu]
>
> Signed-off-by: Grazvydas Ignotas 
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index f6afa6a..327030b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -1166,8 +1166,8 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr 
> *hwmgr)
>
> tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
> PP_ASSERT_WITH_CODE(tmp_result == 0,
> -   "DPM is already running right now, no need to enable 
> DPM!",
> -   return 0);
> +   "DPM is already running",
> +   );
>
> if (smu7_voltage_control(hwmgr)) {
> tmp_result = smu7_enable_voltage_control(hwmgr);
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[RFC] drm/fb-helper: reject any changes to the fbdev

2016-10-12 Thread Ville Syrjälä 
On Tue, Oct 11, 2016 at 04:15:04PM -0700, Stefan Agner wrote:
> The current fbdev emulation does not allow to push back changes in
> width, height or depth to KMS, hence reject any changes with an
> error. This makes sure that fbdev ioctl's fail properly and user
> space does not assume that changes succeeded.
> 
> Signed-off-by: Stefan Agner 
> ---
> This rejects reconfiguration of framebuffer like
> fbset -rgba 5,6,5 -depth 16 (when in 24 bit mode by default)
> fbset -xres 123
> 
> I think all users of drm_fb_helper_check_var use also the generic
> drm_fb_helper_set_par (or do otherwise not support changing size/
> depth). Hence, afaict, the change should be the right thing to do
> for all driver...
> 
>  drivers/gpu/drm/drm_fb_helper.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 03414bd..596c056 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1211,11 +1211,14 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo 
> *var,
>   if (var->pixclock != 0 || in_dbg_master())
>   return -EINVAL;
>  
> - /* Need to resize the fb object !!! */
> - if (var->bits_per_pixel > fb->bits_per_pixel ||
> - var->xres > fb->width || var->yres > fb->height ||
> - var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
> - DRM_DEBUG("fb userspace requested width/height/bpp is greater 
> than current fb "
> + /*
> +  * Changes struct fb_var_screeninfo are currently not pushed back
> +  * to KMS, hence fail if different settings are requested.
> +  */
> + if (var->bits_per_pixel != fb->bits_per_pixel ||
> + var->xres != fb->width || var->yres != fb->height ||
> + var->xres_virtual != fb->width || var->yres_virtual != fb->height) {

This still looks somewhat incomplete. Sounds like we should just
reject changes to everything except xoffset/yoffset.

> + DRM_DEBUG("fb userspace requested width/height/bpp different 
> than current fb "
> "request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
> var->xres, var->yres, var->bits_per_pixel,
> var->xres_virtual, var->yres_virtual,
> -- 
> 2.10.0
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

[Bug 93826] 2560x1440 @144Hz graphic glitches and bad refresh rate

2016-10-12 Thread bugzilla-dae...@freedesktop.org 
https://bugs.freedesktop.org/show_bug.cgi?id=93826

--- Comment #30 from Alex Deucher  ---
Does forcing the mclk to high help? As root:
echo high > /sys/class/drm/card0/device/power_dpm_force_performance_level

-- 
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/20161012/527f1b70/attachment.html>

[patch] drm/savage: dereferencing an error pointer

2016-10-12 Thread SF Markus Elfring 
> A recent cleanup changed the kmalloc() + copy_from_user() to
> memdup_user() but the error handling wasn't updated so we might call
> kfree(-EFAULT) and crash.
> 
> Fixes: a6e3918bcdb1 ('GPU-DRM-Savage: Use memdup_user() rather than 
> duplicating')
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/gpu/drm/savage/savage_state.c 
> b/drivers/gpu/drm/savage/savage_state.c
> index 3dc0d8f..2db89be 100644
> --- a/drivers/gpu/drm/savage/savage_state.c
> +++ b/drivers/gpu/drm/savage/savage_state.c
> @@ -1004,6 +1004,7 @@ int savage_bci_cmdbuf(struct drm_device *dev, void 
> *data, struct drm_file *file_
>   kvb_addr = memdup_user(cmdbuf->vb_addr, cmdbuf->vb_size);
>   if (IS_ERR(kvb_addr)) {
>   ret = PTR_ERR(kvb_addr);
> + kvb_addr = NULL;
>   goto done;
>   }
>   cmdbuf->vb_addr = kvb_addr;
> 

Thanks for this update suggestion.

Can it be that I offered an other approach for a corresponding software 
correction
by the update step “[PATCH 2/2] GPU-DRM-Savage: Less function calls in
savage_bci_cmdbuf() after error detection” (on 2016-08-18)?

https://patchwork.kernel.org/patch/9289183/
https://lkml.kernel.org/r/

Will this one become worth for further development consideratons once more?

Can the shown resetting of an error pointer to a safe null pointer be omitted
in such use cases when the jump targets will be accordingly configured as it is
desired for efficient exception handling implementations?

Regards,
Markus

[patch] drm/amdgpu: potential NULL dereference in debugfs code

2016-10-12 Thread Alex Deucher 
On Wed, Oct 12, 2016 at 5:20 AM, Christian König
 wrote:
> Am 12.10.2016 um 08:17 schrieb Dan Carpenter:
>>
>> debugfs_create_file() returns NULL on error, it only returns error
>> pointers if debugfs isn't enabled in the config and we checked for that
>> earlier so it can't happen.
>>
>> Fixes: 4f4824b55650 ('drm/amd/amdgpu: Convert ring debugfs entries to
>> binary')
>> Signed-off-by: Dan Carpenter 
>
>
> Reviewed-by: Christian König .
>

Applied.  thanks!

Alex

>
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>> index 85aeb0a..8d16eaf 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>> @@ -371,8 +371,8 @@ static int amdgpu_debugfs_ring_init(struct
>> amdgpu_device *adev,
>> ent = debugfs_create_file(name,
>>   S_IFREG | S_IRUGO, root,
>>   ring, _debugfs_ring_fops);
>> -   if (IS_ERR(ent))
>> -   return PTR_ERR(ent);
>> +   if (!ent)
>> +   return -ENOMEM;
>> i_size_write(ent->d_inode, ring->ring_size + 12);
>> ring->ent = ent;
>
>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC] drm/fb-helper: reject any changes to the fbdev

2016-10-12 Thread Tomi Valkeinen 
On 12/10/16 02:15, Stefan Agner wrote:
> The current fbdev emulation does not allow to push back changes in
> width, height or depth to KMS, hence reject any changes with an
> error. This makes sure that fbdev ioctl's fail properly and user
> space does not assume that changes succeeded.

The description is a bit confusing. Don't you mean that currently the
fbdev emulation does allow changes, but doesn't actually push those
changes to KMS? And this patch makes the driver reject those changes.

The change itself sounds good to me.

Reviewed-by: Tomi Valkeinen 

> Signed-off-by: Stefan Agner 
> ---
> This rejects reconfiguration of framebuffer like
> fbset -rgba 5,6,5 -depth 16 (when in 24 bit mode by default)
> fbset -xres 123
> 
> I think all users of drm_fb_helper_check_var use also the generic
> drm_fb_helper_set_par (or do otherwise not support changing size/
> depth). Hence, afaict, the change should be the right thing to do
> for all driver...
> 
>  drivers/gpu/drm/drm_fb_helper.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 03414bd..596c056 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1211,11 +1211,14 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo 
> *var,
>   if (var->pixclock != 0 || in_dbg_master())
>   return -EINVAL;
>  
> - /* Need to resize the fb object !!! */
> - if (var->bits_per_pixel > fb->bits_per_pixel ||
> - var->xres > fb->width || var->yres > fb->height ||
> - var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
> - DRM_DEBUG("fb userspace requested width/height/bpp is greater 
> than current fb "
> + /*
> +  * Changes struct fb_var_screeninfo are currently not pushed back
> +  * to KMS, hence fail if different settings are requested.
> +  */
> + if (var->bits_per_pixel != fb->bits_per_pixel ||
> + var->xres != fb->width || var->yres != fb->height ||
> + var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
> + DRM_DEBUG("fb userspace requested width/height/bpp different 
> than current fb "
> "request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
> var->xres, var->yres, var->bits_per_pixel,
> var->xres_virtual, var->yres_virtual,
> 

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161012/51786b07/attachment.sig>

[PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to "tristate".

2016-10-12 Thread Emil Velikov 
On 11 October 2016 at 10:33, Jani Nikula  wrote:
> On Tue, 11 Oct 2016, "Sun, Jing A"  wrote:
>> It's needed that DRM Driver module could be removed and reloaded after
>> kernel booting on the projects that I have been working on, and I hope
>> such module type change could be accepted. Looks like Iwai has similar
>> change request as well. Would you please review it and let us know if
>> any concerns?
>
> Looking at the Kconfig, selecting CONFIG_DRM_MIPI_DSI is against the
> recommendations of Documentation/kbuild/kconfig-language.txt:
>
> select should be used with care. select will force
> a symbol to a value without visiting the dependencies.
> By abusing select you are able to select a symbol FOO even
> if FOO depends on BAR that is not set.
> In general use select only for non-visible symbols
> (no prompts anywhere) and for symbols with no dependencies.
> That will limit the usefulness but on the other hand avoid
> the illegal configurations all over.
>
> Indeed, you may end up with CONFIG_DRM_MIPI_DSI=y and CONFIG_DRM=m,
> which violates DRM_MIPI_DSI dependency on CONFIG_DRM. This is broken and
> should be fixed. The suggested patch does *not* fix this issue.
>
Jani, git log suggests you as the unfortunate author of the select
DRM_MIPI_DSI/select DRM_PANEL hunks in i915 ;-)

>From a cutesy skim through panel/ there are a handful of things to
squash - unused select/depend on, s/select/depend on/ etc. Sadly I
don't have the time to address these :-\

Regards,
Emil

drm/i915: WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)

2016-10-12 Thread Paul Bolle 
On a laptop that tracks the latest stable release (Ie, it now runs
v4.8.1) I see this WARNING
    WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)

Full trace pasted below. I never saw this WARNING before v4.8. Since
v4.8 I've had it in all (four, actually) boots.

What am I expected to do about this WARNING?

Thanks,


Paul Bolle

WARNING: CPU: 3 PID: 1368 at drivers/gpu/drm/i915/intel_display.c:14178 
skl_max_scale.part.120+0x75/0x80 [i915]
WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)
Modules linked in:
 rfcomm fuse nf_conntrack_netbios_ns nf_conntrack_broadcast ip6t_rpfilter 
ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat 
ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 cmac 
nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw ip6table_security 
iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack 
iptable_mangle iptable_raw iptable_security ebtable_filter ebtables 
ip6table_filter ip6_tables bnep vfat fat arc4 snd_hda_codec_hdmi snd_soc_skl 
dell_led snd_soc_skl_ipc snd_soc_sst_ipc snd_soc_sst_dsp snd_hda_ext_core 
snd_soc_sst_match snd_soc_core intel_rapl snd_hda_codec_realtek 
snd_hda_codec_generic x86_pkg_temp_thermal coretemp kvm_intel snd_compress 
snd_pcm_dmaengine ac97_bus kvm snd_hda_intel iwlmvm snd_hda_codec mac80211 
iTCO_wdt
 iTCO_vendor_support uvcvideo snd_hda_core snd_hwdep snd_seq irqbypass 
dell_laptop i2c_designware_platform i2c_designware_core dell_wmi 
crct10dif_pclmul dell_smbios dcdbas crc32_pclmul snd_seq_device iwlwifi 
videobuf2_vmalloc videobuf2_memops ghash_clmulni_intel snd_pcm videobuf2_v4l2 
videobuf2_core cfg80211 videodev media joydev pcspkr mei_me rtsx_pci_ms 
memstick snd_timer i2c_i801 i2c_smbus mei snd btusb soundcore shpchp hci_uart 
btrtl btbcm btqca idma64 btintel bluetooth intel_pch_thermal 
processor_thermal_device intel_lpss_pci intel_soc_dts_iosf wmi 
pinctrl_sunrisepoint intel_lpss_acpi rfkill pinctrl_intel intel_lpss 
int3400_thermal acpi_als int3403_thermal int340x_thermal_zone kfifo_buf 
acpi_thermal_rel intel_hid industrialio sparse_keymap acpi_pad tpm_tis 
tpm_tis_core tpm nfsd auth_rpcgss
 nfs_acl lockd grace sunrpc hid_multitouch i915 rtsx_pci_sdmmc mmc_core 
i2c_algo_bit drm_kms_helper crc32c_intel drm serio_raw nvme rtsx_pci nvme_core 
i2c_hid video fjes
CPU: 3 PID: 1368 Comm: Xorg Not tainted 4.8.1-1.local1.fc24.x86_64 #1
Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.4.4 06/14/2016
 0286 df2f374c a31528d53910 b83e5cfd
 a31528d53960  a31528d53950 b80a7d5b
 3762c72b3010 a3151e4d8cc0 a31526c23800 a31526e6
Call Trace:
 [] dump_stack+0x63/0x86
 [] __warn+0xcb/0xf0
 [] warn_slowpath_fmt+0x5f/0x80
 [] ? sort+0x147/0x220
 [] ? drm_atomic_helper_normalize_zpos+0x264/0x300 
[drm_kms_helper]
 [] skl_max_scale.part.120+0x75/0x80 [i915]
 [] intel_check_primary_plane+0xc6/0xe0 [i915]
 [] ? drm_atomic_helper_normalize_zpos+0x264/0x300 
[drm_kms_helper]
 [] intel_plane_atomic_check+0x132/0x1f0 [i915]
 [] drm_atomic_helper_check_planes+0x84/0x200 [drm_kms_helper]
 [] intel_atomic_check+0x9a7/0x11a0 [i915]
 [] ? __kmalloc_track_caller+0x17a/0x210
 [] drm_atomic_check_only+0x187/0x610 [drm]
 [] ? drm_atomic_get_crtc_state+0x88/0x100 [drm]
 [] drm_atomic_commit+0x17/0x60 [drm]
 [] drm_atomic_helper_update_plane+0xec/0x130 [drm_kms_helper]
 [] __setplane_internal+0x22b/0x270 [drm]
 [] drm_mode_cursor_universal+0x139/0x240 [drm]
 [] drm_mode_cursor_common+0x7e/0x180 [drm]
 [] drm_mode_cursor2_ioctl+0xe/0x10 [drm]
 [] drm_ioctl+0x1da/0x4b0 [drm]
 [] ? drm_mode_cursor_ioctl+0x70/0x70 [drm]
 [] ? enqueue_hrtimer+0x3d/0x80
 [] do_vfs_ioctl+0xa3/0x5e0
 [] ? __sys_recvmsg+0x51/0x90
 [] SyS_ioctl+0x79/0x90
 [] entry_SYSCALL_64_fastpath+0x1a/0xa4

[RFC 6/6] drivers: staging: ion: add ION_IOC_TAG ioctl

2016-10-12 Thread Hillf Danton 
On Wednesday, October 12, 2016 7:50 AM Ruchi Kandoi wrote:
> +/**
> + * struct ion_fd_data - metadata passed from userspace for a handle

s/fd/tag/ ?

> + * @handle:  a handle
> + * @tag: a string describing the buffer
> + *
> + * For ION_IOC_TAG userspace populates the handle field with
> + * the handle returned from ion alloc and type contains the memtrack_type 
> which
> + * accurately describes the usage for the memory.
> + */
> +struct ion_tag_data {
> + ion_user_handle_t handle;
> + char tag[ION_MAX_TAG_LEN];
> +};
> +

[patch] drm/amdgpu: potential NULL dereference in debugfs code

2016-10-12 Thread Christian König 
Am 12.10.2016 um 08:17 schrieb Dan Carpenter:
> debugfs_create_file() returns NULL on error, it only returns error
> pointers if debugfs isn't enabled in the config and we checked for that
> earlier so it can't happen.
>
> Fixes: 4f4824b55650 ('drm/amd/amdgpu: Convert ring debugfs entries to binary')
> Signed-off-by: Dan Carpenter 

Reviewed-by: Christian König .

>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 85aeb0a..8d16eaf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -371,8 +371,8 @@ static int amdgpu_debugfs_ring_init(struct amdgpu_device 
> *adev,
>   ent = debugfs_create_file(name,
> S_IFREG | S_IRUGO, root,
> ring, _debugfs_ring_fops);
> - if (IS_ERR(ent))
> - return PTR_ERR(ent);
> + if (!ent)
> + return -ENOMEM;
>   
>   i_size_write(ent->d_inode, ring->ring_size + 12);
>   ring->ent = ent;

[RFC 0/6] Module for tracking/accounting shared memory buffers

2016-10-12 Thread Christian König 
Am 12.10.2016 um 01:50 schrieb Ruchi Kandoi:
> This patchstack adds memtrack hooks into dma-buf and ion.  If there's upstream
> interest in memtrack, it can be extended to other memory allocators as well,
> such as GEM implementations.
We have run into similar problems before. Because of this I already 
proposed a solution for this quite a while ago, but never pushed on 
upstreaming this since it was only done for a special use case.

Instead of keeping track of how much memory a process has bound (which 
is very fragile) my solution  only added some more debugging info on a 
per fd basis (e.g. how much memory is bound to this fd).

This information was then used by the OOM killer (for example) to make a 
better decision on which process to reap.

Shouldn't be to hard to expose this through debugfs or maybe a new fcntl 
to userspace for debugging.

I haven't looked at the code in detail, but messing with the per process 
memory accounting like you did in this proposal is clearly not a good 
idea if you ask me.

Regards,
Christian.

Question: Re: [PATCH] drm/bridge: analogix: protect power when get_modes or detect

2016-10-12 Thread Sean Paul 
On Wed, Oct 12, 2016 at 6:22 AM, Mark yao  wrote:
>
> I'm not familiar with the analogix driver, maybe use a power reference count
> would better then direct power on/off analogix_dp.
>
> Does anyone has the idea to protect detect and get_modes context?
>

I'm not sure a reference count is going to help here. The common
pattern is to call detect() followed by get_modes() and then modeset.
However, it's not guaranteed that any one of those functions will be
called after the other. So, if you leave things on after detect or
get_modes, you might be wasting power (or worse).

I recently ran into this exact problem with a panel we're using. Check
out "0b8b059a7: drm/bridge: analogix_dp: Ensure the panel is properly
prepared/unprepared". Perhaps you can piggyback on that function to
add your pm_runtime and plat_data callbacks (since using dpms_mode
might be racey).

Sean


> I found many other connector driver also direct access register on detect or
> get_modes, no problem for it?
>
> On 2016年10月12日 18:00, Mark Yao wrote:
>
> The drm callback ->detect and ->get_modes seems is not power safe,
> they may be called when device is power off, do register access on
> detect or get_modes will cause system die.
>
> Here is the path call ->detect before analogix_dp power on
> [] analogix_dp_detect+0x44/0xdc
> []
> drm_helper_probe_single_connector_modes_merge_bits+0xe8/0x41c
> [] drm_helper_probe_single_connector_modes+0x10/0x18
> [] drm_mode_getconnector+0xf4/0x304
> [] drm_ioctl+0x23c/0x390
> [] do_vfs_ioctl+0x4b8/0x58c
> [] SyS_ioctl+0x60/0x88
>
> Cc: Inki Dae 
> Cc: Sean Paul 
> Cc: Gustavo Padovan 
> Cc: "Ville Syrjälä" 
>
> Signed-off-by: Mark Yao 
> ---
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 28
> ++
>  1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index efac8ab..09dece2 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1062,6 +1062,13 @@ int analogix_dp_get_modes(struct drm_connector
> *connector)
>   return 0;
>   }
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + pm_runtime_get_sync(dp->dev);
> +
> + if (dp->plat_data->power_on)
> + dp->plat_data->power_on(dp->plat_data);
> + }
> +
>   if (analogix_dp_handle_edid(dp) == 0) {
>   drm_mode_connector_update_edid_property(>connector, edid);
>   num_modes += drm_add_edid_modes(>connector, edid);
> @@ -1073,6 +1080,13 @@ int analogix_dp_get_modes(struct drm_connector
> *connector)
>   if (dp->plat_data->get_modes)
>   num_modes += dp->plat_data->get_modes(dp->plat_data, connector);
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + if (dp->plat_data->power_off)
> + dp->plat_data->power_off(dp->plat_data);
> +
> + pm_runtime_put_sync(dp->dev);
> + }
> +
>   ret = analogix_dp_prepare_panel(dp, false, false);
>   if (ret)
>   DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
> @@ -1106,9 +1120,23 @@ analogix_dp_detect(struct drm_connector *connector,
> bool force)
>   return connector_status_disconnected;
>   }
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + pm_runtime_get_sync(dp->dev);
> +
> + if (dp->plat_data->power_on)
> + dp->plat_data->power_on(dp->plat_data);
> + }
> +
>   if (!analogix_dp_detect_hpd(dp))
>   status = connector_status_connected;
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + if (dp->plat_data->power_off)
> + dp->plat_data->power_off(dp->plat_data);
> +
> + pm_runtime_put_sync(dp->dev);
> + }
> +
>   ret = analogix_dp_prepare_panel(dp, false, false);
>   if (ret)
>   DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
>
>
>
> --
> ï¼­ark Yao

[PATCH] drm/gma500: add comments for new parameters

2016-10-12 Thread Jiang Biao 
Added comments for new parameters.

Signed-off-by: Jiang Biao 
---
 drivers/gpu/drm/gma500/gtt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 8f69225..76aea2e 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -76,6 +76,7 @@ static u32 __iomem *psb_gtt_entry(struct drm_device *dev, 
struct gtt_range *r)
  * psb_gtt_insert  -   put an object into the GTT
  * @dev: our DRM device
  * @r: our GTT range
+ * @resume: on resume
  *
  * Take our preallocated GTT range and insert the GEM object into
  * the GTT. This is protected via the gtt mutex which the caller
@@ -321,6 +322,7 @@ out:
  * @len: length (bytes) of address space required
  * @name: resource name
  * @backed: resource should be backed by stolen pages
+ * @align: requested alignment
  *
  * Ask the kernel core to find us a suitable range of addresses
  * to use for a GTT mapping.
-- 
2.1.0

[RFC 0/6] Module for tracking/accounting shared memory buffers

2016-10-12 Thread Dave Hansen 
On 10/11/2016 04:50 PM, Ruchi Kandoi wrote:
> Any process holding a reference to these buffers will keep the kernel from
> reclaiming its backing pages.  mm counters don't provide a complete picture of
> these allocations, since they only account for pages that are mapped into a
> process's address space.  This problem is especially bad for systems like
> Android that use dma-buf fds to share graphics and multimedia buffers between
> processes: these allocations are often large, have complex sharing patterns,
> and are rarely mapped into every process that holds a reference to them.

What do you end up _doing_ with all this new information that you have
here?  You know which processes have "pinned" these shared buffers, and
exported that information in /proc.  But, then what?

[RFC] drm/fb-helper: reject any changes to the fbdev

2016-10-12 Thread Stefan Agner 
On 2016-10-12 09:12, Ville Syrjälä wrote:
> On Wed, Oct 12, 2016 at 08:55:45AM -0700, Stefan Agner wrote:
>> On 2016-10-12 03:42, Ville Syrjälä wrote:
>> > On Tue, Oct 11, 2016 at 04:15:04PM -0700, Stefan Agner wrote:
>> >> The current fbdev emulation does not allow to push back changes in
>> >> width, height or depth to KMS, hence reject any changes with an
>> >> error. This makes sure that fbdev ioctl's fail properly and user
>> >> space does not assume that changes succeeded.
>> >>
>> >> Signed-off-by: Stefan Agner 
>> >> ---
>> >> This rejects reconfiguration of framebuffer like
>> >> fbset -rgba 5,6,5 -depth 16 (when in 24 bit mode by default)
>> >> fbset -xres 123
>> >>
>> >> I think all users of drm_fb_helper_check_var use also the generic
>> >> drm_fb_helper_set_par (or do otherwise not support changing size/
>> >> depth). Hence, afaict, the change should be the right thing to do
>> >> for all driver...
>> >>
>> >>  drivers/gpu/drm/drm_fb_helper.c | 13 -
>> >>  1 file changed, 8 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
>> >> b/drivers/gpu/drm/drm_fb_helper.c
>> >> index 03414bd..596c056 100644
>> >> --- a/drivers/gpu/drm/drm_fb_helper.c
>> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> >> @@ -1211,11 +1211,14 @@ int drm_fb_helper_check_var(struct 
>> >> fb_var_screeninfo *var,
>> >>   if (var->pixclock != 0 || in_dbg_master())
>> >>   return -EINVAL;
>> >>
>> >> - /* Need to resize the fb object !!! */
>> >> - if (var->bits_per_pixel > fb->bits_per_pixel ||
>> >> - var->xres > fb->width || var->yres > fb->height ||
>> >> - var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
>> >> - DRM_DEBUG("fb userspace requested width/height/bpp is greater 
>> >> than current fb "
>> >> + /*
>> >> +  * Changes struct fb_var_screeninfo are currently not pushed back
>> >> +  * to KMS, hence fail if different settings are requested.
>> >> +  */
>> >> + if (var->bits_per_pixel != fb->bits_per_pixel ||
>> >> + var->xres != fb->width || var->yres != fb->height ||
>> >> + var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
>> >
>> > This still looks somewhat incomplete. Sounds like we should just
>> > reject changes to everything except xoffset/yoffset.
>> >
>>
>> What other parameters would you check? struct fb_var_screeninfo also has
>> timings, but that is not something which is handy right here...
> 
> We should check the timings I think since we don't allow chaning them.
> Though I suppose we don't even populate them. But then the user
> shouldn't be allowed try to set them either I guess.
> 

Ultimately we should do that agreed.

However, the framebuffer settings are a bigger problem currently: It is
not just that we silently not apply settings, but because we don't error
out on check_var the fbcon subsystem assumes the changes have been
applied to the display controller and redraws the console with the new
format... Timings do not affect the drawing of fbcon, hence this is less
of a problem. I probably should have mentioned that in the commit log
too.

The callback is also used for initial configuration (which I guess is
some kind of struct fb_var_screeninfo populated by the KMS subsystem?).
Checking more parameters increases the risk that we wrongfully reject
initial configurations which would left people with a broken fbdev
emulation.

>>
>> One parameter we could check too is the depth calculated from the bit
>> information, e.g.
>>
>>
>>  switch (var->bits_per_pixel) {
>>  case 16:
>>  depth = (var->green.length == 6) ? 16 : 15;
>>  break;
>>  case 32:
>>  depth = (var->transp.length > 0) ? 32 : 24;
>>  break;
>>  default:
>>  depth = var->bits_per_pixel;
>>  break;
>>  }
>> +
>> +if (depth != fb->depth) {
>> +DRM_DEBUG("fb userspace requested depth different than current 
>> fb "
>> +  "request %d vs. %dx\n", depth, fb->depth);
>> +return -EINVAL;
>> +}
>>
>> Or, maybe better, we could use drm_mode_legacy_fb_format to convert
>> bpp/depth to fourcc and check that against the pixel_format field...
> 
> Also the red/green/... definitions for the actual bits of the color
> channels. .grayscale is there well, and some colorspace thing which
> I don't even know what it does. And rotation...

We can only easily check what is in the intersection of struct
drm_framebuffer and struct fb_var_screeninfo.

Not sure how the color bit definitions actually map to DRM/KMS stuff, I
guess just through the pixel_format field?

Rotation is something which is per plane, which again is not easily
accessible.

I don't disagree, we should check all this. But we haven't checked any
of this so far, and I would rather start with a small subset (e.g. only
check the stuff easily convertible and available in struct
drm_framebuffer).

--
Stefan

> 
> So I think in the end

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

2016-10-12 Thread Michel Dänzer 
On 11/10/16 09:04 PM, Christian König wrote:
> Am 11.10.2016 um 05:58 schrieb Michel Dänzer:
>> On 07/10/16 09:34 PM, Mike Lothian wrote:
>>> This has discussion has gone a little quiet
>>>
>>> Was there any agreement about what needed doing to get this working
>>> for i965/amdgpu?
>> Christian, do you see anything which could prevent the solution I
>> outlined from working?
> 
> I thought about that approach as well, but unfortunately it also has a
> couple of downsides. Especially keeping the exclusive fence set while we
> actually don't need it isn't really clean either.

I was wondering if it's possible to have a singleton pseudo exclusive
fence which is used for all BOs. That might keep the overhead acceptably
low.


> I'm currently a bit busy with other tasks and so put Nayan on a road to
> get a bit into the kernel driver (he asked for that anyway).
> 
> Implementing the simple workaround to sync when we export the DMA-buf
> should be something like 20 lines of code and fortunately Nayan has an
> I+A system and so can actually test it.
> 
> If it turns out to be more problematic or somebody really starts to need
> it I'm going to hack on that myself a bit more.

If you mean only syncing when a DMA-buf is exported, that's not enough,
as I explained before. The BOs being shared are long-lived, and
synchronization between GPUs is required for every command stream
submission.


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

[Bug 97639] Intermittent flickering artifacts with AMD R7 260x

2016-10-12 Thread bugzilla-dae...@freedesktop.org 
https://bugs.freedesktop.org/show_bug.cgi?id=97639

--- Comment #11 from Michel Dänzer  ---
Alternatively, you can generate a diff between 4.8-rc8 and current drm-next-4.9
with

 git diff v4.8-rc8..8036617e92e3fad49eef9bbe868b661c58249aff

and apply that to 4.8.

-- 
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/20161012/d66af789/attachment.html>

[patch] drm/savage: dereferencing an error pointer

2016-10-12 Thread Dan Carpenter 
A recent cleanup changed the kmalloc() + copy_from_user() to
memdup_user() but the error handling wasn't updated so we might call
kfree(-EFAULT) and crash.

Fixes: a6e3918bcdb1 ('GPU-DRM-Savage: Use memdup_user() rather than 
duplicating')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/savage/savage_state.c 
b/drivers/gpu/drm/savage/savage_state.c
index 3dc0d8f..2db89be 100644
--- a/drivers/gpu/drm/savage/savage_state.c
+++ b/drivers/gpu/drm/savage/savage_state.c
@@ -1004,6 +1004,7 @@ int savage_bci_cmdbuf(struct drm_device *dev, void *data, 
struct drm_file *file_
kvb_addr = memdup_user(cmdbuf->vb_addr, cmdbuf->vb_size);
if (IS_ERR(kvb_addr)) {
ret = PTR_ERR(kvb_addr);
+   kvb_addr = NULL;
goto done;
}
cmdbuf->vb_addr = kvb_addr;

[patch] drm/amdgpu: potential NULL dereference in debugfs code

2016-10-12 Thread Dan Carpenter 
debugfs_create_file() returns NULL on error, it only returns error
pointers if debugfs isn't enabled in the config and we checked for that
earlier so it can't happen.

Fixes: 4f4824b55650 ('drm/amd/amdgpu: Convert ring debugfs entries to binary')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 85aeb0a..8d16eaf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -371,8 +371,8 @@ static int amdgpu_debugfs_ring_init(struct amdgpu_device 
*adev,
ent = debugfs_create_file(name,
  S_IFREG | S_IRUGO, root,
  ring, _debugfs_ring_fops);
-   if (IS_ERR(ent))
-   return PTR_ERR(ent);
+   if (!ent)
+   return -ENOMEM;

i_size_write(ent->d_inode, ring->ring_size + 12);
ring->ent = ent;

[Bug 177041] When browsing Google Map's Satellite view in Chrome or Firefox the screen freezes and goes black, occasionally control is returned to user

2016-10-12 Thread bugzilla-dae...@bugzilla.kernel.org 
https://bugzilla.kernel.org/show_bug.cgi?id=177041

--- Comment #1 from Michel Dänzer  ---
Can you try a newer kernel, ideally 4.8.y? I seem to remember running into this
before, but no longer with current kernels. Unfortunately, I don't remember
specifically how this was fixed.

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

[bug report] drm/amd/powerplay: implement smu7 hwmgr to manager asics with smu ip version 7.

2016-10-12 Thread Dan Carpenter 
Hello Rex Zhu,

This is a semi-automatic email about new static checker warnings.

The patch 599a7e9fe1b6: "drm/amd/powerplay: implement smu7 hwmgr to 
manager asics with smu ip version 7." from Sep 9, 2016, leads to the 
following Smatch complaint:

drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c:1463 
smu7_get_evv_voltages()
 error: we previously assumed 'table_info' could be null (see line 1455)

drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c
  1454  
  1455  if (table_info != NULL)
 ^
Check for non-NULL.

  1456  sclk_table = table_info->vdd_dep_on_sclk;
  1457  
  1458  for (i = 0; i < SMU7_MAX_LEAKAGE_COUNT; i++) {
  1459  vv_id = ATOM_VIRTUAL_VOLTAGE_ID0 + i;
  1460  
  1461  if (data->vdd_gfx_control == 
SMU7_VOLTAGE_CONTROL_BY_SVID2) {
  1462  if (0 == phm_get_sclk_for_voltage_evv(hwmgr,
  1463  
table_info->vddgfx_lookup_table, vv_id, )) {

^^^
Unchecked dereference.

  1464  if 
(phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
  1465  
PHM_PlatformCaps_ClockStretcher)) {

regards,
dan carpenter

[PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to "tristate".

2016-10-12 Thread Sun, Jing A 
I think "installing a kernel with my changes for both drm and i915" takes more 
time and effort to complete than "only updating DRM/i915 modules without 
rebuilding the whole kernel". In some cases, that's beneficial.

Also reloadablility is always a good thing to have and I truly hope 
Hajda/Iwai's patches would be accepted and merged.
No downside of it after all.

Regards,
Sun, Jing

-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, October 12, 2016 2:52 PM
To: Sun, Jing A
Cc: Andrzej Hajda; Jani Nikula; Takashi Iwai; Emil Velikov; linux-kernel at 
vger.kernel.org; dri-devel at lists.freedesktop.org; Vetter, Daniel; Thierry 
Reding
Subject: Re: [PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to 
"tristate".

On Wed, Oct 12, 2016 at 03:08:24AM +, Sun, Jing A wrote:
> Interestingly, I am able to reload i915 and drm. Our CI has tests for
> i915 unload/reload, but does not check drm. In any case the config 
> problem should not impact the reloadability of i915.
> ==
> Sorry that I didn't make myself clear. In order to replace the default
> i915 module with an updated one, the related DRM modules also need to 
> be updated to match the updated i915, hence the restriction.

Just to avoid tears in the future: If you plan to ship this in product, you 
won't ship.

And for debugging, just install a kernel with your changes for both drm and 
i915.

In short, your use-case isn't really valid (but we could still make the dsi 
code modular if people feel like).
-Daniel

> 
> Regards,
> Sun, Jing
> 
> 
> -Original Message-
> From: Andrzej Hajda [mailto:a.hajda at samsung.com]
> Sent: Tuesday, October 11, 2016 5:53 PM
> To: Jani Nikula; Sun, Jing A; Takashi Iwai
> Cc: airlied at linux.ie; Vetter, Daniel; linux-kernel at vger.kernel.org; 
> dri-devel at lists.freedesktop.org; Thierry Reding; Emil Velikov
> Subject: Re: [PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to 
> "tristate".
> 
> On 11.10.2016 11:33, Jani Nikula wrote:
> > On Tue, 11 Oct 2016, "Sun, Jing A"  wrote:
> >> It's needed that DRM Driver module could be removed and reloaded 
> >> after kernel booting on the projects that I have been working on, 
> >> and I hope such module type change could be accepted. Looks like 
> >> Iwai has similar change request as well. Would you please review it 
> >> and let us know if any concerns?
> > Looking at the Kconfig, selecting CONFIG_DRM_MIPI_DSI is against the 
> > recommendations of Documentation/kbuild/kconfig-language.txt:
> >
> > select should be used with care. select will force
> > a symbol to a value without visiting the dependencies.
> > By abusing select you are able to select a symbol FOO even
> > if FOO depends on BAR that is not set.
> > In general use select only for non-visible symbols
> > (no prompts anywhere) and for symbols with no dependencies.
> > That will limit the usefulness but on the other hand avoid
> > the illegal configurations all over.
> 
> All existing drivers which selects DRM_MIPI_DSI also depends on DRM.
> So the dependency is always true. I am not sure if it could not change 
> in the future, but in such case mipi_dsi bus should be completely 
> detached from DRM framework, I hope we have not such case yet :)
> 
> >
> > Indeed, you may end up with CONFIG_DRM_MIPI_DSI=y and CONFIG_DRM=m, 
> > which violates DRM_MIPI_DSI dependency on CONFIG_DRM. This is broken 
> > and should be fixed. The suggested patch does *not* fix this issue.
> 
> At the moment it should not be possible.
> 
> Regards
> Andrzej
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Bug 93826] 2560x1440 @144Hz graphic glitches and bad refresh rate

2016-10-12 Thread bugzilla-dae...@freedesktop.org 
https://bugs.freedesktop.org/show_bug.cgi?id=93826

--- Comment #29 from Michel Dänzer  ---
(In reply to Yves W. from comment #27)
> - with default driver and default kernel (Linux Mint 18 -> 4.4.0-38-generi)
> it's working at 2560x1440 @144hz, no flickering issues.
> 
> -> tried using the Ubuntu 4.8.0 kernel -> flickering at 2560x1440 @144hz,120
> and 100hz. only 60hz works without flickering

Can you bisect, or at least narrow down which version between 4.4.0 and 4.8.0
introduced the problem?

-- 
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/20161012/b1e0e045/attachment.html>

[RFC] drm/fb-helper: reject any changes to the fbdev

2016-10-12 Thread Daniel Vetter 
On Tue, Oct 11, 2016 at 04:15:04PM -0700, Stefan Agner wrote:
> The current fbdev emulation does not allow to push back changes in
> width, height or depth to KMS, hence reject any changes with an
> error. This makes sure that fbdev ioctl's fail properly and user
> space does not assume that changes succeeded.
> 
> Signed-off-by: Stefan Agner 

Make sense to me, but would be great to have an r-b from someone who has
some clue about fbdev ... Tomi, Laurent?
-Daniel

> ---
> This rejects reconfiguration of framebuffer like
> fbset -rgba 5,6,5 -depth 16 (when in 24 bit mode by default)
> fbset -xres 123
> 
> I think all users of drm_fb_helper_check_var use also the generic
> drm_fb_helper_set_par (or do otherwise not support changing size/
> depth). Hence, afaict, the change should be the right thing to do
> for all driver...
> 
>  drivers/gpu/drm/drm_fb_helper.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 03414bd..596c056 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1211,11 +1211,14 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo 
> *var,
>   if (var->pixclock != 0 || in_dbg_master())
>   return -EINVAL;
>  
> - /* Need to resize the fb object !!! */
> - if (var->bits_per_pixel > fb->bits_per_pixel ||
> - var->xres > fb->width || var->yres > fb->height ||
> - var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
> - DRM_DEBUG("fb userspace requested width/height/bpp is greater 
> than current fb "
> + /*
> +  * Changes struct fb_var_screeninfo are currently not pushed back
> +  * to KMS, hence fail if different settings are requested.
> +  */
> + if (var->bits_per_pixel != fb->bits_per_pixel ||
> + var->xres != fb->width || var->yres != fb->height ||
> + var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
> + DRM_DEBUG("fb userspace requested width/height/bpp different 
> than current fb "
> "request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
> var->xres, var->yres, var->bits_per_pixel,
> var->xres_virtual, var->yres_virtual,
> -- 
> 2.10.0
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Bug 97639] Intermittent flickering artifacts with AMD R7 260x

2016-10-12 Thread bugzilla-dae...@freedesktop.org 
https://bugs.freedesktop.org/show_bug.cgi?id=97639

--- Comment #10 from Michel Dänzer  ---
Can you just try the drm-next-4.9 tree? It's based on 4.8-rc8, so should be
pretty stable in general.

-- 
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/20161012/faacb826/attachment.html>

[RFC PATCH 00/11] Introduce writeback connectors

2016-10-12 Thread Daniel Vetter 
On Tue, Oct 11, 2016 at 10:24:23PM +0100, Brian Starkey wrote:
> On Tue, Oct 11, 2016 at 10:02:43PM +0200, Daniel Vetter wrote:
> > The problem with just that is that there's lots of different things
> > that can feed into the overall needs_modeset variable. That's why we
> > split it up into multiple booleans.
> > 
> > So yes you're supposed to clear connectors_changed if there is some
> > change that you can handle without a full modeset. If you want, think
> > of connectors_changed as
> > needs_modeset_due_to_change_in_connnector_state, but that's cumbersome
> > to type and too long ;-)
> > 
> 
> All right, got it :-). This intention wasn't clear to me from the
> comments in the code.

A patch to update the kernel-doc to make it clearer (there's mode_changed,
connectors_changed and active_changed, plus drm_crtc_needs_modeset) would
be awesome. I'm trying to write useful docs, but since I designed this all
I sometimes forget to make the non-obvious assumptions clear enough.

Volunteered?

> > > > tbh I don't like that, I think it'd be better to make this truly
> > > > one-shot. Otherwise we'll have real fun problems with hw where the
> > > > writeback can take longer than a vblank (it happens ...). So one-shot,
> > > > with auto-clearing to NULL/0 is imo the right approach.
> > > 
> > > That's an interesting point about hardware which won't finish within
> > > one frame; but I don't see how "true one-shot" helps.
> > > 
> > > What's the expected behaviour if userspace makes a new atomic commit
> > > with a writeback framebuffer whilst a previous writeback is ongoing?
> > > 
> > > In both cases, you either need to block or fail the commit - whether
> > > the framebuffer gets removed when it's done is immaterial.
> > 
> > See Eric's question. We need to define that, and I think the simplest
> > approach is a completion fence/sync_file. It's destaged now in 4.9, we
> > can use them. I think the simplest uabi would be a pointer property
> > (u64) where we write the fd of the fence we'll signal when write-out
> > completes.
> > 
> 
> That tells userspace that the previous writeback is finished, I agree that's
> needed. It doesn't define any behaviour in case userspace asks for another
> writeback before that fence fires though.

Hm, good point. I guess we could just state that if userspace does a
writeback, and issues a new writeback before both a) the atomic flip and
b) the write back complete fence signalled will lead to undefined
behaviour. Undefined as in: data corruption, rejected atomic commit or
anything else than a kernel crash is allowed. This is similar to doing a
page flip and starting to render to the old buffers before the flip event
signalled completion: Userspace gets the mess it asked for ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[RFC] drm/fb-helper: reject any changes to the fbdev

2016-10-12 Thread Stefan Agner 
On 2016-10-12 03:42, Ville Syrjälä wrote:
> On Tue, Oct 11, 2016 at 04:15:04PM -0700, Stefan Agner wrote:
>> The current fbdev emulation does not allow to push back changes in
>> width, height or depth to KMS, hence reject any changes with an
>> error. This makes sure that fbdev ioctl's fail properly and user
>> space does not assume that changes succeeded.
>>
>> Signed-off-by: Stefan Agner 
>> ---
>> This rejects reconfiguration of framebuffer like
>> fbset -rgba 5,6,5 -depth 16 (when in 24 bit mode by default)
>> fbset -xres 123
>>
>> I think all users of drm_fb_helper_check_var use also the generic
>> drm_fb_helper_set_par (or do otherwise not support changing size/
>> depth). Hence, afaict, the change should be the right thing to do
>> for all driver...
>>
>>  drivers/gpu/drm/drm_fb_helper.c | 13 -
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 03414bd..596c056 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1211,11 +1211,14 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo 
>> *var,
>>  if (var->pixclock != 0 || in_dbg_master())
>>  return -EINVAL;
>>
>> -/* Need to resize the fb object !!! */
>> -if (var->bits_per_pixel > fb->bits_per_pixel ||
>> -var->xres > fb->width || var->yres > fb->height ||
>> -var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
>> -DRM_DEBUG("fb userspace requested width/height/bpp is greater 
>> than current fb "
>> +/*
>> + * Changes struct fb_var_screeninfo are currently not pushed back
>> + * to KMS, hence fail if different settings are requested.
>> + */
>> +if (var->bits_per_pixel != fb->bits_per_pixel ||
>> +var->xres != fb->width || var->yres != fb->height ||
>> +var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
> 
> This still looks somewhat incomplete. Sounds like we should just
> reject changes to everything except xoffset/yoffset.
> 

What other parameters would you check? struct fb_var_screeninfo also has
timings, but that is not something which is handy right here...

One parameter we could check too is the depth calculated from the bit
information, e.g.


switch (var->bits_per_pixel) {
case 16:
depth = (var->green.length == 6) ? 16 : 15;
break;
case 32:
depth = (var->transp.length > 0) ? 32 : 24;
break;
default:
depth = var->bits_per_pixel;
break;
}
+
+   if (depth != fb->depth) {
+   DRM_DEBUG("fb userspace requested depth different than current 
fb "
+ "request %d vs. %dx\n", depth, fb->depth);
+   return -EINVAL;
+   }

Or, maybe better, we could use drm_mode_legacy_fb_format to convert
bpp/depth to fourcc and check that against the pixel_format field...

--
Stefan


>> +DRM_DEBUG("fb userspace requested width/height/bpp different 
>> than current fb "
>>"request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
>>var->xres, var->yres, var->bits_per_pixel,
>>var->xres_virtual, var->yres_virtual,
>> --
>> 2.10.0
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to "tristate".

2016-10-12 Thread Daniel Vetter 
On Wed, Oct 12, 2016 at 03:08:24AM +, Sun, Jing A wrote:
> Interestingly, I am able to reload i915 and drm. Our CI has tests for
> i915 unload/reload, but does not check drm. In any case the config
> problem should not impact the reloadability of i915.
> ==
> Sorry that I didn't make myself clear. In order to replace the default
> i915 module with an updated one, the related DRM modules also need to be
> updated to match the updated i915, hence the restriction.

Just to avoid tears in the future: If you plan to ship this in product,
you won't ship.

And for debugging, just install a kernel with your changes for both drm
and i915.

In short, your use-case isn't really valid (but we could still make the
dsi code modular if people feel like).
-Daniel

> 
> Regards,
> Sun, Jing
> 
> 
> -Original Message-
> From: Andrzej Hajda [mailto:a.hajda at samsung.com] 
> Sent: Tuesday, October 11, 2016 5:53 PM
> To: Jani Nikula; Sun, Jing A; Takashi Iwai
> Cc: airlied at linux.ie; Vetter, Daniel; linux-kernel at vger.kernel.org; 
> dri-devel at lists.freedesktop.org; Thierry Reding; Emil Velikov
> Subject: Re: [PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to 
> "tristate".
> 
> On 11.10.2016 11:33, Jani Nikula wrote:
> > On Tue, 11 Oct 2016, "Sun, Jing A"  wrote:
> >> It's needed that DRM Driver module could be removed and reloaded 
> >> after kernel booting on the projects that I have been working on, and 
> >> I hope such module type change could be accepted. Looks like Iwai has 
> >> similar change request as well. Would you please review it and let us 
> >> know if any concerns?
> > Looking at the Kconfig, selecting CONFIG_DRM_MIPI_DSI is against the 
> > recommendations of Documentation/kbuild/kconfig-language.txt:
> >
> > select should be used with care. select will force
> > a symbol to a value without visiting the dependencies.
> > By abusing select you are able to select a symbol FOO even
> > if FOO depends on BAR that is not set.
> > In general use select only for non-visible symbols
> > (no prompts anywhere) and for symbols with no dependencies.
> > That will limit the usefulness but on the other hand avoid
> > the illegal configurations all over.
> 
> All existing drivers which selects DRM_MIPI_DSI also depends on DRM.
> So the dependency is always true. I am not sure if it could not change in the 
> future, but in such case mipi_dsi bus should be completely detached from DRM 
> framework, I hope we have not such case yet :)
> 
> >
> > Indeed, you may end up with CONFIG_DRM_MIPI_DSI=y and CONFIG_DRM=m, 
> > which violates DRM_MIPI_DSI dependency on CONFIG_DRM. This is broken 
> > and should be fixed. The suggested patch does *not* fix this issue.
> 
> At the moment it should not be possible.
> 
> Regards
> Andrzej
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[RFC PATCH 00/11] Introduce writeback connectors

2016-10-12 Thread Brian Starkey 
Hi Eric,

On Tue, Oct 11, 2016 at 12:01:14PM -0700, Eric Anholt wrote:
>Brian Starkey  writes:
>
>> Hi,
>>
>> This RFC series introduces a new connector type:
>>  DRM_MODE_CONNECTOR_WRITEBACK
>> It is a follow-on from a previous discussion: [1]
>>
>> Writeback connectors are used to expose the memory writeback engines
>> found in some display controllers, which can write a CRTC's
>> composition result to a memory buffer.
>> This is useful e.g. for testing, screen-recording, screenshots,
>> wireless display, display cloning, memory-to-memory composition.
>>
>> Patches 1-7 include the core framework changes required, and patches
>> 8-11 implement a writeback connector for the Mali-DP writeback engine.
>> The Mali-DP patches depend on this other series: [2].
>>
>> The connector is given the FB_ID property for the output framebuffer,
>> and two new read-only properties: PIXEL_FORMATS and
>> PIXEL_FORMATS_SIZE, which expose the supported framebuffer pixel
>> formats of the engine.
>>
>> The EDID property is not exposed for writeback connectors.
>>
>> Writeback connector usage:
>> --
>> Due to connector routing changes being treated as "full modeset"
>> operations, any client which wishes to use a writeback connector
>> should include the connector in every modeset. The writeback will not
>> actually become active until a framebuffer is attached.
>>
>> The writeback itself is enabled by attaching a framebuffer to the
>> FB_ID property of the connector. The driver must then ensure that the
>> CRTC content of that atomic commit is written into the framebuffer.
>>
>> The writeback works in a one-shot mode with each atomic commit. This
>> prevents the same content from being written multiple times.
>> In some cases (front-buffer rendering) there might be a desire for
>> continuous operation - I think a property could be added later for
>> this kind of control.
>>
>> Writeback can be disabled by setting FB_ID to zero.
>
>I think this sounds great, and the interface is just right IMO.
>

Thanks, glad you like it! Hopefully you're equally agreeable with the
changes Daniel has been suggesting.

>I don't really see a use for continuous mode -- a sequence of one-shots
>makes a lot more sense because then you can know what data has changed,
>which anyone trying to use the writeback buffer would need to know.
>

Agreed - we've never found a use for it.

>> Known issues:
>> -
>>  * I'm not sure what "DPMS" should mean for writeback connectors.
>>It could be used to disable writeback (even when a framebuffer is
>>attached), or it could be hidden entirely (which would break the
>>legacy DPMS call for writeback connectors).
>>  * With Daniel's recent re-iteration of the userspace API rules, I
>>fully expect to provide some userspace code to support this. The
>>question is what, and where? We want to use writeback for testing,
>>so perhaps some tests in igt is suitable.
>>  * Documentation. Probably some portion of this cover letter needs to
>>make it into Documentation/
>>  * Synchronisation. Our hardware will finish the writeback by the next
>>vsync. I've not implemented fence support here, but it would be an
>>obvious addition.
>
>My hardware won't necessarily finish by the next vsync -- it trickles
>out at whatever rate it can find memory bandwidth to get the job done,
>and fires an interrupt when it's finished.
>

Is it bounded? You presumably have to finish the write-out before you
can change any input buffers?

>So I would like some definition for how syncing works.  One answer would
>be that these flips don't trigger their pageflip events until the
>writeback is done (so I need to collect both the vsync irq and the
>writeback irq before sending).  Another would be that manage an
>independent fence for the writeback fb, so that you still immediately
>know when framebuffers from the previous scanout-only frame are idle.
>

I much prefer the sound of the explicit fence approach.

Hopefully we can agree that a new atomic commit can't be completed
whilst there's a writeback ongoing, otherwise managing the fence and
framebuffer lifetime sounds really tricky - they'd need to be decoupled
from the atomic_state and outlive the commit that spawned them.

Cheers,
-Brian

>Also, tests for this in igt, please.  Writeback in igt will give us so
>much more ability to cover KMS functionality on non-Intel hardware.

[git pull] drm pull for v4.9

2016-10-12 Thread Dave Airlie 
Hi Linus,

sorry for the delay, had sick wife/baby combo deal last week, which led to
a much reduced sleep pattern. during that I mismerged i915.

this email is all in small letters because my gpg key expired so I couldn't
sign the tag, and it's too early in the morning for me to go do gpg stuff.

there is no nouveau work in this tree, as Ben didn't get a pull request
in, and he was fighting moving to atomic and adding mst support, so maybe
best it waits for a cycle.

Okay I've add some captial letters below.

Dave.

core:
Fence destaging work
DRIVER_LEGACY to split off legacy drm drivers
drm_mm refactoring
Splitting drm_crtc.c into chunks and documenting better
Display info fixes
rbtree support for prime buffer lookup
Simple VGA DAC driver.

panel:
Add Nexus 7 panel.
More simple panels.

i915:
Refactoring GEM naming
Refactored vma/active tracking
Lockless request lookups
Better stolen memory support
FBC fixes
SKL watermark fixes
VGPU improvements
dma-buf fencing support
Better DP dongle support

amdgpu:
Powerplay for Iceland asics
Improved GPU reset support
UVD/VEC powergating support for CZ/ST
Preinitialised VRAM buffer support
Virtual display support
Initial SI support
GTT rework.
PCI shutdown callback support.
HPD IRQ storm fixes.

amdkfd:
bugfixes.

tilcdc:
Atomic modesetting support.

mediatek:
AAL + GAMMA engine support
Hook up gamma LUT
Temporal dithering support

imx:
Pixel clock from devicetree
drm bridge support for LVDS bridges
active plane reconfiguration
VDIC deinterlacer support
Frame synchronisation unit support
Color space conversion support

analogix:
PSR support
Better panel on/off support

rockchip:
rk3399 vop/crtc support
PSR support

vc4:
Interlaced vblank timing
3D rendering CPU overhead reduction
HDMI output fixes

tda998x:
HDMI audio ASoC support

sunxi:
Allwinner A33 support
better TCON support

msm:
DT binding cleanups
Explicit fence-fd support

sti:
remove sti415/416 support

etnaviv:
MMUv2 refactoring
GC3000 support

exynos:
Refactoring HDMI DCC/PHY
G2D pm regression fix
Page fault issues with wait for vblank

The following changes since commit 08895a8b6b06ed2323cd97a36ee40a116b3db8ed:

  Linux 4.8-rc8 (2016-09-25 18:47:13 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~airlied/linux tags/drm-for-v4.9

for you to fetch changes up to 69405d3da98b48633b78a49403e4f9cdb7c6a0f5:

  Merge tag 'topic/drm-misc-2016-10-11' of
git://anongit.freedesktop.org/drm-intel into drm-next (2016-10-12
06:07:38 +1000)


Akash Goel (1):
  drm/i915/gen9: Update i915_drpc_info debugfs for coarse pg &
forcewake info

Alex Deucher (77):
  drm/amdgpu/powerplay: enable powerplay by default on TOPAZ
  drm/amdgpu/gmc7: add missing mullins case
  drm/amdgpu/ci: add mullins to default case for smc ucode
  drm/amdgpu/gfx8: remove stale function declaration
  drm/amdgpu: move all Kconfig options to amdgpu/Kconfig
  drm/amdgpu: move vsync_timer_enabled setup to dce virtual early_init
  drm/amdgpu/virtual_dce: add case for topaz for disable_dce
  drm/amdgpu: add virtual dce support for iceland
  drm/amdgpu: fix IB alignment for UVD
  drm/amdgpu: fix VCE ib alignment value
  drm/amdgpu: add support for UVD_NO_OP register
  drm/radeon: add support for UVD_NO_OP register
  drm/amdgpu: switch UVD code to use UVD_NO_OP for padding
  drm/radeon: switch UVD code to use UVD_NO_OP for padding
  drm/amdgpu: rename suspend_kms and resume_kms
  drm/amdgpu: track the number of vce rings
  drm/amdgpu/vce3: add support for third vce ring
  drm/amdgpu/si: Add updated smc firmware for SI kickers
  drm/amdgpu/gfx6: drop some dead code
  drm/amdgpu: set runtime pm state to active on resume
  drm/amdgpu: skip suspend/resume on DRM_SWITCH_POWER_DYNAMIC_OFF
  drm/radeon: set runtime pm state to active on resume
  drm/radeon: skip suspend/resume on DRM_SWITCH_POWER_DYNAMIC_OFF
  drm/amdgpu: handle runtime pm correctly in amdgpu_driver_open_kms
  drm/radeon: handle runtime pm correctly in amdgpu_driver_open_kms
  drm/amdgpu: handle runtime pm in drm pre/post close
  drm/radeon: handle runtime pm in drm pre/post close
  drm/amdgpu: wire up a pci shutdown callback
  drm/radeon: wire up a pci shutdown callback
  drm/amdgpu/si/dpm: make a bunch of things static
  drm/amdgpu/si/dpm: fix symbol conflicts with radeon
  drm/amdgpu: handle runtime pm in fbcon (v2)
  drm/radeon: handle runtime pm in fbcon (v2)
  drm/amdgpu/si: fix ring size for compute
  drm/amdgpu/gfx6: drop duplicate code
  drm/amdgpu/gfx6: add ring_emit_cntxcntl
  drm/amdgpu/gfx6: drop gds_switch callback

[git pull] drm for 4.8

2016-10-12 Thread Dave Airlie 
On 12 October 2016 at 05:59, Linus Torvalds
 wrote:
> What's the status of the 4.9 merge window pull request? The GPU side
> is the main remaining pile for this merge window according to
> linux-next. I'd hate to get a last-minute pull at the end of the week

I'm lining it up this morning, had sick kid last week so didn't have
enough sleep
to not do something stupid without someone checking it, and thankfully I waited
as I appeared to mismerge i915 in the backmerge.

Dave.

[Bug 60879] [radeonsi] Tahiti LE: GFX block is not functional, CP is okay

2016-10-12 Thread bugzilla-dae...@freedesktop.org 
https://bugs.freedesktop.org/show_bug.cgi?id=60879

--- Comment #158 from Michel Dänzer  ---
(In reply to madmalkav from comment #157)
> I've tried agd5f's 4.9 wip beanch with latest Mesa. Wflinfo fails with an
> "amdgpu: unkown family" error, no Gallium dump log, error on dmesg:

Which Mesa Git commit is that exactly? It looks like it's before SI support was
added to the amdgpu winsys code. Double-check that you're really building
current Git master, and that your self-built radeonsi_dri.so is getting picked
up.

-- 
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/20161012/5f3992c5/attachment.html>

[PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to "tristate".

2016-10-12 Thread Sun, Jing A 
Interestingly, I am able to reload i915 and drm. Our CI has tests for
i915 unload/reload, but does not check drm. In any case the config problem 
should not impact the reloadability of i915.
==
Sorry that I didn't make myself clear. In order to replace the default i915 
module with an updated one, the related DRM modules also need to be updated to 
match the updated i915, hence the restriction.

Regards,
Sun, Jing


-Original Message-
From: Andrzej Hajda [mailto:a.ha...@samsung.com] 
Sent: Tuesday, October 11, 2016 5:53 PM
To: Jani Nikula; Sun, Jing A; Takashi Iwai
Cc: airlied at linux.ie; Vetter, Daniel; linux-kernel at vger.kernel.org; 
dri-devel at lists.freedesktop.org; Thierry Reding; Emil Velikov
Subject: Re: [PATCH]"drm: change DRM_MIPI_DSI module type from "bool" to 
"tristate".

On 11.10.2016 11:33, Jani Nikula wrote:
> On Tue, 11 Oct 2016, "Sun, Jing A"  wrote:
>> It's needed that DRM Driver module could be removed and reloaded 
>> after kernel booting on the projects that I have been working on, and 
>> I hope such module type change could be accepted. Looks like Iwai has 
>> similar change request as well. Would you please review it and let us 
>> know if any concerns?
> Looking at the Kconfig, selecting CONFIG_DRM_MIPI_DSI is against the 
> recommendations of Documentation/kbuild/kconfig-language.txt:
>
>   select should be used with care. select will force
>   a symbol to a value without visiting the dependencies.
>   By abusing select you are able to select a symbol FOO even
>   if FOO depends on BAR that is not set.
>   In general use select only for non-visible symbols
>   (no prompts anywhere) and for symbols with no dependencies.
>   That will limit the usefulness but on the other hand avoid
>   the illegal configurations all over.

All existing drivers which selects DRM_MIPI_DSI also depends on DRM.
So the dependency is always true. I am not sure if it could not change in the 
future, but in such case mipi_dsi bus should be completely detached from DRM 
framework, I hope we have not such case yet :)

>
> Indeed, you may end up with CONFIG_DRM_MIPI_DSI=y and CONFIG_DRM=m, 
> which violates DRM_MIPI_DSI dependency on CONFIG_DRM. This is broken 
> and should be fixed. The suggested patch does *not* fix this issue.

At the moment it should not be possible.

Regards
Andrzej

[RFC 0/6] Module for tracking/accounting shared memory buffers

2016-10-12 Thread Al Viro 
On Tue, Oct 11, 2016 at 04:50:04PM -0700, Ruchi Kandoi wrote:

> memtrack maintains a per-process list of shared buffer references, which is
> exported to userspace as /proc/[pid]/memtrack.  Buffers can be optionally
> "tagged" with a short string: for example, Android userspace would use this
> tag to identify whether buffers were allocated on behalf of the camera stack,
> GL, etc.  memtrack also exports the VMAs associated with these buffers so
> that pages already included in the process's mm counters aren't 
> double-counted.
> 
> Shared-buffer allocators can hook into memtrack by embedding
> struct memtrack_buffer in their buffer metadata, calling
> memtrack_buffer_{init,remove} at buffer allocation and free time, and
> memtrack_buffer_{install,uninstall} when a userspace process takes or
> drops a reference to the buffer.  For fd-backed buffers like dma-bufs, hooks 
> in
> fdtable.c and fork.c automatically notify memtrack when references are added 
> or
> removed from a process's fd table.
> 
> This patchstack adds memtrack hooks into dma-buf and ion.  If there's upstream
> interest in memtrack, it can be extended to other memory allocators as well,
> such as GEM implementations.

No, with a side of Hell, No.  Not to mention anything else,
* descriptor tables do not belong to any specific task_struct and
actions done by one show up in all who share that thing.
* shared descriptor table does not imply belonging to the same
group.
* shared descriptor table can become unshared at any point, invisibly
for that Fine Piece Of Software.
* while we are at it, blocking allocation under several spinlocks
(and with interrupts disabled, for good measure) is generally considered
a bloody bad idea.

That - just from the quick look through that patchset.  Bringing task_struct
into the API is already sufficient for a NAK.