[PATCH v3 1/9] drm/hisilicon/hibmc: Add hisilicon hibmc drm master driver

[Rongrong Zou]

Hi Benjamin,

Thanks for reviewing!

Benjamin Gaignard 於 2016/10/14 16:29 寫道:
> [snip]
>
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>> @@ -0,0 +1,15 @@
>> +config DRM_HISI_HIBMC
>> +   tristate "DRM Support for Hisilicon Hibmc"
>> +   depends on DRM && PCI
>> +   select DRM_KMS_HELPER
>> +   select DRM_KMS_FB_HELPER
>> +   select DRM_GEM_CMA_HELPER
>> +   select DRM_KMS_CMA_HELPER
>
> since you use TTM I don't think that selecting DRM_GEM_CMA_HELPER and
> DRM_KMS_CMA_HELPER
> help you lot here.
> You could add configuration flags step by step in following patches
> that will make you needs more clear (that also true for #include)

will delete them, thanks.

>
>> +   select FB_SYS_FILLRECT
>> +   select FB_SYS_COPYAREA
>> +   select FB_SYS_IMAGEBLIT
>> +   select DRM_TTM
>> +
>> +   help
>> + Choose this option if you have a Hisilicon Hibmc soc chipset.
>> + If M is selected the module will be called hibmc-drm.
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile 
>> b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> new file mode 100644
>> index 000..97cf4a0
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> @@ -0,0 +1,5 @@
>> +ccflags-y := -Iinclude/drm
>> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_power.o
>> +
>> +obj-$(CONFIG_DRM_HISI_HIBMC)   +=hibmc-drm.o
>> +#obj-y += hibmc-drm.o
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c 
>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> new file mode 100644
>> index 000..52c9353
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> @@ -0,0 +1,288 @@
>> +/* Hisilicon Hibmc SoC drm driver
>> + *
>> + * Based on the bochs drm driver.
>> + *
>> + * Copyright (c) 2016 Huawei Limited.
>> + *
>> + * Author:
>> + * Rongrong Zou 
>
> ".com" is missing in you email address (same typo in all other files)

will fix it in next version, thanks. :)

>
>> + * Rongrong Zou 
>> + * Jianhua Li 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>
> cma_helpers look useless since you use TTM, no ?

I add TTM just in this version, and forgot to clean these
cma relevant code, will fix in next version. Thanks.

>
>> +#include 
>> +
>> +#include "hibmc_drm_drv.h"
>> +#include "hibmc_drm_regs.h"
>> +#include "hibmc_drm_power.h"
>> +

Regards,
Rongrong

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

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

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

--- Comment #121 from emilio.moretti at gmail.com ---
Finally working:
I just upgraded to ubuntu 16.10  (kernel 4.8.0-22-generic) and I've been using
the pc all day long without problems (this was impossible before).
I had to use the kernel parameter radeon.dpm=0 in grub in order to get a stable
desktop, and I can confirm I don't need it any more.

-- 
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/20161014/f94f87d9/attachment-0001.html>

Hibernation broken since commit 274ad65c9d02 ("drm/radeon: hard reset r600 and newer GPU when hibernating.")

[Sven Joachim]

On 2016-09-15 11:04 -0400, Jerome Glisse wrote:

>> On Mon, Sep 5, 2016 at 10:25 PM, Jerome Glisse  wrote:
>> >> Recently I got myself a new laptop with the following integrated GPU:
>> >>
>> >> 00:01.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
>> >> Mullins [Radeon R3 Graphics] (rev 40)
>> >>
>> >> I found that hibernation is broken in Linux 4.7+ (it works in Linux 4.6)
>> >> and bisected it to commit 274ad65c9d02 ("drm/radeon: hard reset r600 and
>> >> newer GPU when hibernating.").
>> >>
>> >> This has already been reported three months ago, but for a much older
>> >> GPU, see the thread starting at
>> >> https://lists.freedesktop.org/archives/dri-devel/2016-June/110050.html.
>> >> The symptoms are exactly the same as described by Boris Petkov in that
>> >> thread: after "systemctl hibernate" the screen goes blank, but the
>> >> machine remains powered on and needs to be power-cycled.
>> >>
>> >> Any suggestions would be welcome.
>> >>
>> >> Cheers,
>> >>Sven
>> >
>> > I guess we can invert the logic and only do it for the GPU for which it fix
>> > hibernation.
>> 
>> Do you remember which asics this fixed?
>> 
>
> No that's the issue, i don't have hw anymore but according to bug it is AMD 
> FirePro
> M5170 and M6170 which would make them amdgpu afaict but i remember the hw was 
> using
> radeon so i am not sure how to figure out what GPU it was really.
>
> Bug https://bugzilla.redhat.com/show_bug.cgi?id=1269009

That bug is not public, unfortunately. :-(

Are there any news on this?  As of Linux 4.8.1, I keep reverting commit
274ad65c9d02, and I'd like not having to do that indefinitely.

Cheers,
   Sven

[PATCH 0/6] drm: add atomic state logging and debugfs

[Rob Clark]

On Fri, Oct 14, 2016 at 7:55 PM, Rob Clark  wrote:
> Bit more spiffed out version of the RFC.  Now with Sean's suggestion to
> add vfuncs in plane/crtc/connector funcs for drivers that subclass the
> various state structs.  Plus Ville's suggestion about helper macros
> for printing mode/rect structs (and alignment with how drm_rect printed
> the integer and fixed-point rects).  Plus addition of connector state
> and debugfs.

and jfyi, full state dump now looks like this:


plane[24]: RGB0
crtc=crtc-0
fb=52
format=XB24 little-endian (0x34324258)
size=2048x1152
layers:
pitch[0]=8192
offset[0]=0
modifier[0]=0x0
crtc-pos=2048x1152+0+0
src-pos=2048.00x1152.00+0.00+0.00
rotation=0
premultiplied=0
zpos=1
alpha=255
stage=STAGE_BASE
mode_changed=0
pending=0
plane[29]: RGB1
crtc=(null)
fb=0
crtc-pos=0x0+0+0
src-pos=0.00x0.00+0.00+0.00
rotation=0
premultiplied=0
zpos=1
alpha=255
stage=STAGE_UNUSED
mode_changed=0
pending=0
plane[31]: VIG0
crtc=(null)
fb=0
crtc-pos=0x0+0+0
src-pos=0.00x0.00+0.00+0.00
rotation=0
premultiplied=0
zpos=4
alpha=255
stage=STAGE_UNUSED
mode_changed=0
pending=0
plane[32]: DMA0
crtc=(null)
fb=0
crtc-pos=0x0+0+0
src-pos=0.00x0.00+0.00+0.00
rotation=0
premultiplied=0
zpos=5
alpha=255
stage=STAGE_UNUSED
mode_changed=0
pending=0
crtc[28]: crtc-0
enable=1
active=1
planes_changed=1
mode_changed=0
active_changed=0
connectors_changed=0
color_mgmt_changed=0
plane_mask=1
connector_mask=1
encoder_mask=1
mode: 0:"2048x1152" 60 156750 2048 2096 2128 2208 1152 1155 1160
1185 0x48 0x9
crtc[30]: crtc-1
enable=0
active=0
planes_changed=0
mode_changed=0
active_changed=0
connectors_changed=0
color_mgmt_changed=0
plane_mask=0
connector_mask=0
encoder_mask=0
mode: 0:"" 0 0 0 0 0 0 0 0 0 0 0x0 0x0
connector[35]: HDMI-A-1
crtc=crtc-0


I've added fb parameters, which was another suggestion

BR,
-R

> Rob Clark (6):
>   drm: helper macros to print composite types
>   drm: add helper for printing to log or seq_file
>   drm: add helpers to go from plane state to drm_rect
>   drm/atomic: add new drm_debug bit to dump atomic state before commit
>   drm/atomic: add debugfs file to dump out atomic state
>   drm/msm/mdp5: add atomic_print_state support
>
>  drivers/gpu/drm/Makefile|   3 +-
>  drivers/gpu/drm/drm_atomic.c| 133 
> 
>  drivers/gpu/drm/drm_debugfs.c   |   9 ++
>  drivers/gpu/drm/drm_modes.c |   8 +-
>  drivers/gpu/drm/drm_print.c |  54 +++
>  drivers/gpu/drm/drm_rect.c  |  11 +--
>  drivers/gpu/drm/drm_simple_kms_helper.c |  14 +--
>  drivers/gpu/drm/i915/intel_atomic_plane.c   |  10 +--
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c|  15 +---
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h |  12 +++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  18 +++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  10 +--
>  include/drm/drmP.h  |  22 +
>  include/drm/drm_atomic.h|   4 +
>  include/drm/drm_crtc.h  |  61 +
>  include/drm/drm_print.h |  62 +
>  16 files changed, 387 insertions(+), 59 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_print.c
>  create mode 100644 include/drm/drm_print.h
>
> --
> 2.7.4
>

[PATCH 6/6] drm/msm/mdp5: add atomic_print_state support

[Rob Clark]

We subclass drm_plane_state, so add mdp5_plane_atomic_print_state() to
dump out our own driver specific plane state.

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   | 12 
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 18 +-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h 
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
index e4b3fb3..3502711 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
@@ -120,6 +120,18 @@ static inline u32 mdp5_read(struct mdp5_kms *mdp5_kms, u32 
reg)
return msm_readl(mdp5_kms->mmio + reg);
 }

+static inline const char *stage2name(enum mdp_mixer_stage_id stage)
+{
+   static const char *names[] = {
+#define NAME(n) [n] = #n
+   NAME(STAGE_UNUSED), NAME(STAGE_BASE),
+   NAME(STAGE0), NAME(STAGE1), NAME(STAGE2),
+   NAME(STAGE3), NAME(STAGE4), NAME(STAGE6),
+#undef NAME
+   };
+   return names[stage];
+}
+
 static inline const char *pipe2name(enum mdp5_pipe pipe)
 {
static const char *names[] = {
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index b6f1fc66..9f99b4d 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -16,6 +16,7 @@
  * this program.  If not, see .
  */

+#include 
 #include "mdp5_kms.h"

 struct mdp5_plane {
@@ -187,6 +188,20 @@ done:
 #undef SET_PROPERTY
 }

+static void
+mdp5_plane_atomic_print_state(struct drm_print *p,
+   const struct drm_plane_state *state)
+{
+   struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
+
+   drm_printf(p, "\tpremultiplied=%u\n", pstate->premultiplied);
+   drm_printf(p, "\tzpos=%u\n", pstate->zpos);
+   drm_printf(p, "\talpha=%u\n", pstate->alpha);
+   drm_printf(p, "\tstage=%s\n", stage2name(pstate->stage));
+   drm_printf(p, "\tmode_changed=%u\n", pstate->mode_changed);
+   drm_printf(p, "\tpending=%u\n", pstate->pending);
+}
+
 static void mdp5_plane_reset(struct drm_plane *plane)
 {
struct mdp5_plane_state *mdp5_state;
@@ -250,6 +265,7 @@ static const struct drm_plane_funcs mdp5_plane_funcs = {
.reset = mdp5_plane_reset,
.atomic_duplicate_state = mdp5_plane_duplicate_state,
.atomic_destroy_state = mdp5_plane_destroy_state,
+   .atomic_print_state = mdp5_plane_atomic_print_state,
 };

 static int mdp5_plane_prepare_fb(struct drm_plane *plane,
@@ -907,7 +923,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
ret = drm_universal_plane_init(dev, plane, 0xff, _plane_funcs,
 mdp5_plane->formats, mdp5_plane->nformats,
-type, NULL);
+type, "%s", mdp5_plane->name);
if (ret)
goto fail;

-- 
2.7.4

[PATCH 5/6] drm/atomic: add debugfs file to dump out atomic state

[Rob Clark]

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/drm_atomic.c  | 40 
 drivers/gpu/drm/drm_debugfs.c |  9 +
 include/drm/drm_atomic.h  |  4 
 3 files changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3cc3cb5..117d429 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1565,6 +1565,46 @@ static void drm_atomic_print_state(const struct 
drm_atomic_state *state)
drm_atomic_connector_print_state(, connector_state);
 }

+#ifdef CONFIG_DEBUG_FS
+static int drm_state_info(struct seq_file *m, void *data)
+{
+   struct drm_info_node *node = (struct drm_info_node *) m->private;
+   struct drm_device *dev = node->minor->dev;
+   struct drm_mode_config *config = >mode_config;
+   struct drm_plane *plane;
+   struct drm_crtc *crtc;
+   struct drm_connector *connector;
+   struct drm_print p = drm_print_seq_file(m);
+
+   drm_modeset_lock_all(dev);
+
+   list_for_each_entry(plane, >plane_list, head)
+   drm_atomic_plane_print_state(, plane->state);
+
+   list_for_each_entry(crtc, >crtc_list, head)
+   drm_atomic_crtc_print_state(, crtc->state);
+
+   list_for_each_entry(connector, >connector_list, head)
+   drm_atomic_connector_print_state(, connector->state);
+
+   drm_modeset_unlock_all(dev);
+
+   return 0;
+}
+
+/* any use in debugfs files to dump individual planes/crtc/etc? */
+static const struct drm_info_list drm_atomic_debugfs_list[] = {
+   {"state", drm_state_info, 0},
+};
+
+int drm_atomic_debugfs_init(struct drm_minor *minor)
+{
+   return drm_debugfs_create_files(drm_atomic_debugfs_list,
+   ARRAY_SIZE(drm_atomic_debugfs_list),
+   minor->debugfs_root, minor);
+}
+#endif
+
 /*
  * The big monstor ioctl
  */
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index fa10cef..d8945a9 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "drm_internal.h"

 #if defined(CONFIG_DEBUG_FS)
@@ -163,6 +164,14 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
return ret;
}

+   if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
+   ret = drm_atomic_debugfs_init(minor);
+   if (ret) {
+   DRM_ERROR("Failed to create atomic debugfs files\n");
+   return ret;
+   }
+   }
+
if (dev->driver->debugfs_init) {
ret = dev->driver->debugfs_init(minor);
if (ret) {
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 856a9c8..68422f6 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -181,6 +181,10 @@ int __must_check drm_atomic_check_only(struct 
drm_atomic_state *state);
 int __must_check drm_atomic_commit(struct drm_atomic_state *state);
 int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state);

+#ifdef CONFIG_DEBUG_FS
+int drm_atomic_debugfs_init(struct drm_minor *minor);
+#endif
+
 #define for_each_connector_in_state(__state, connector, connector_state, __i) \
for ((__i) = 0; \
 (__i) < (__state)->num_connector &&
\
-- 
2.7.4

[PATCH 4/6] drm/atomic: add new drm_debug bit to dump atomic state before commit

[Rob Clark]

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/drm_atomic.c | 93 
 include/drm/drmP.h   |  1 +
 include/drm/drm_crtc.h   | 37 ++
 3 files changed, 131 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7ebf375..3cc3cb5 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include "drm_crtc_internal.h"
@@ -608,6 +609,28 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
return 0;
 }

+static void drm_atomic_crtc_print_state(struct drm_print *p,
+   const struct drm_crtc_state *state)
+{
+   struct drm_crtc *crtc = state->crtc;
+
+   drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
+   drm_printf(p, "\tenable=%d\n", state->enable);
+   drm_printf(p, "\tactive=%d\n", state->active);
+   drm_printf(p, "\tplanes_changed=%d\n", state->planes_changed);
+   drm_printf(p, "\tmode_changed=%d\n", state->mode_changed);
+   drm_printf(p, "\tactive_changed=%d\n", state->active_changed);
+   drm_printf(p, "\tconnectors_changed=%d\n", state->connectors_changed);
+   drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
+   drm_printf(p, "\tplane_mask=%x\n", state->plane_mask);
+   drm_printf(p, "\tconnector_mask=%x\n", state->connector_mask);
+   drm_printf(p, "\tencoder_mask=%x\n", state->encoder_mask);
+   drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(>mode));
+
+   if (crtc->funcs->atomic_print_state)
+   crtc->funcs->atomic_print_state(p, state);
+}
+
 /**
  * drm_atomic_get_plane_state - get plane state
  * @state: global atomic state object
@@ -895,6 +918,38 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
return 0;
 }

+static void drm_atomic_plane_print_state(struct drm_print *p,
+   const struct drm_plane_state *state)
+{
+   struct drm_plane *plane = state->plane;
+   struct drm_rect src  = drm_plane_state_src(state);
+   struct drm_rect dest = drm_plane_state_dest(state);
+
+   drm_printf(p, "plane[%u]: %s\n", plane->base.id, plane->name);
+   drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : 
"(null)");
+   drm_printf(p, "\tfb=%u\n", state->fb ? state->fb->base.id : 0);
+   if (state->fb) {
+   struct drm_framebuffer *fb = state->fb;
+   int i, n = drm_format_num_planes(fb->pixel_format);
+
+   drm_printf(p, "\t\tformat=%s\n",
+   drm_get_format_name(fb->pixel_format));
+   drm_printf(p, "\t\tsize=%dx%d\n", fb->width, fb->height);
+   drm_printf(p, "\t\tlayers:\n");
+   for (i = 0; i < n; i++) {
+   drm_printf(p, "\t\t\tpitch[%d]=%u\n", i, 
fb->pitches[i]);
+   drm_printf(p, "\t\t\toffset[%d]=%u\n", i, 
fb->offsets[i]);
+   drm_printf(p, "\t\t\tmodifier[%d]=0x%llx\n", i, 
fb->modifier[i]);
+   }
+   }
+   drm_printf(p, "\tcrtc-pos=" DRM_RECT_FMT "\n", DRM_RECT_ARG());
+   drm_printf(p, "\tsrc-pos=" DRM_RECT_FP_FMT "\n", DRM_RECT_FP_ARG());
+   drm_printf(p, "\trotation=%x\n", state->rotation);
+
+   if (plane->funcs->atomic_print_state)
+   plane->funcs->atomic_print_state(p, state);
+}
+
 /**
  * drm_atomic_get_connector_state - get connector state
  * @state: global atomic state object
@@ -1010,6 +1065,18 @@ int drm_atomic_connector_set_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_atomic_connector_set_property);

+static void drm_atomic_connector_print_state(struct drm_print *p,
+   const struct drm_connector_state *state)
+{
+   struct drm_connector *connector = state->connector;
+
+   drm_printf(p, "connector[%u]: %s\n", connector->base.id, 
connector->name);
+   drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : 
"(null)");
+
+   if (connector->funcs->atomic_print_state)
+   connector->funcs->atomic_print_state(p, state);
+}
+
 /**
  * drm_atomic_connector_get_property - get property value from connector state
  * @connector: the drm connector to set a property on
@@ -1475,6 +1542,29 @@ int drm_atomic_nonblocking_commit(struct 
drm_atomic_state *state)
 }
 EXPORT_SYMBOL(drm_atomic_nonblocking_commit);

+static void drm_atomic_print_state(const struct drm_atomic_state *state)
+{
+   struct drm_print p = drm_print_info();
+   struct drm_plane *plane;
+   struct drm_plane_state *plane_state;
+   struct drm_crtc *crtc;
+   struct drm_crtc_state *crtc_state;
+   struct drm_connector *connector;
+   struct drm_connector_state *connector_state;
+   int i;
+
+   DRM_DEBUG_ATOMIC("checking %p\n", state);
+
+   for_each_plane_in_state(state, plane, plane_state, i)
+   

[PATCH 3/6] drm: add helpers to go from plane state to drm_rect

[Rob Clark]

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/drm_simple_kms_helper.c | 14 ++
 drivers/gpu/drm/i915/intel_atomic_plane.c   | 10 ++
 drivers/gpu/drm/mediatek/mtk_drm_plane.c| 15 ++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 ++
 include/drm/drm_crtc.h  | 24 
 5 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c 
b/drivers/gpu/drm/drm_simple_kms_helper.c
index 0db36d2..9834fc5 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -73,18 +73,8 @@ static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs 
= {
 static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
struct drm_plane_state *plane_state)
 {
-   struct drm_rect src = {
-   .x1 = plane_state->src_x,
-   .y1 = plane_state->src_y,
-   .x2 = plane_state->src_x + plane_state->src_w,
-   .y2 = plane_state->src_y + plane_state->src_h,
-   };
-   struct drm_rect dest = {
-   .x1 = plane_state->crtc_x,
-   .y1 = plane_state->crtc_y,
-   .x2 = plane_state->crtc_x + plane_state->crtc_w,
-   .y2 = plane_state->crtc_y + plane_state->crtc_h,
-   };
+   struct drm_rect src = drm_plane_state_src(plane_state);
+   struct drm_rect dest = drm_plane_state_dest(plane_state);
struct drm_rect clip = { 0 };
struct drm_simple_display_pipe *pipe;
struct drm_crtc_state *crtc_state;
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 7de7721..6682e9b 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -139,14 +139,8 @@ static int intel_plane_atomic_check(struct drm_plane 
*plane,
 * we want to keep another copy internal to our driver that we can
 * clip/modify ourselves.
 */
-   intel_state->src.x1 = state->src_x;
-   intel_state->src.y1 = state->src_y;
-   intel_state->src.x2 = state->src_x + state->src_w;
-   intel_state->src.y2 = state->src_y + state->src_h;
-   intel_state->dst.x1 = state->crtc_x;
-   intel_state->dst.y1 = state->crtc_y;
-   intel_state->dst.x2 = state->crtc_x + state->crtc_w;
-   intel_state->dst.y2 = state->crtc_y + state->crtc_h;
+   intel_state->src = drm_plane_state_src(state);
+   intel_state->dst = drm_plane_state_dest(state);

/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
intel_state->clip.x1 = 0;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c 
b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 3995765..2749e74 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -135,19 +135,8 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
struct drm_framebuffer *fb = state->fb;
struct drm_crtc_state *crtc_state;
bool visible;
-   struct drm_rect dest = {
-   .x1 = state->crtc_x,
-   .y1 = state->crtc_y,
-   .x2 = state->crtc_x + state->crtc_w,
-   .y2 = state->crtc_y + state->crtc_h,
-   };
-   struct drm_rect src = {
-   /* 16.16 fixed point */
-   .x1 = state->src_x,
-   .y1 = state->src_y,
-   .x2 = state->src_x + state->src_w,
-   .y2 = state->src_y + state->src_h,
-   };
+   struct drm_rect dest = drm_plane_state_dest(state);
+   struct drm_rect src  = drm_plane_state_src(state);  /* 16.16 fixed 
point */
struct drm_rect clip = { 0, };

if (!fb)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 6255e5b..31384ca 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -610,14 +610,8 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
if (WARN_ON(!crtc_state))
return -EINVAL;

-   src->x1 = state->src_x;
-   src->y1 = state->src_y;
-   src->x2 = state->src_x + state->src_w;
-   src->y2 = state->src_y + state->src_h;
-   dest->x1 = state->crtc_x;
-   dest->y1 = state->crtc_y;
-   dest->x2 = state->crtc_x + state->crtc_w;
-   dest->y2 = state->crtc_y + state->crtc_h;
+   *src  = drm_plane_state_src(state);
+   *dest = drm_plane_state_dest(state);

clip.x1 = 0;
clip.y1 = 0;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b7d67cc..856fdf8 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 

 struct drm_device;
 struct drm_mode_set;
@@ -1395,6 +1396,29 @@ struct drm_plane_state {
struct drm_atomic_state *state;
 };

+static inline 

[PATCH 2/6] drm: add helper for printing to log or seq_file

[Rob Clark]

Sometimes it is nice not to duplicate equivalent printk() and
seq_printf() code.

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/Makefile|  3 ++-
 drivers/gpu/drm/drm_print.c | 54 +++
 include/drm/drm_print.h | 62 +
 3 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_print.c
 create mode 100644 include/drm/drm_print.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e3dba6f..4894ebb 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -12,7 +12,8 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_info.o drm_debugfs.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
drm_rect.o drm_vma_manager.o drm_flip_work.o \
-   drm_modeset_lock.o drm_atomic.o drm_bridge.o
+   drm_modeset_lock.o drm_atomic.o drm_bridge.o \
+   drm_print.o

 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
new file mode 100644
index 000..ddcb437
--- /dev/null
+++ b/drivers/gpu/drm/drm_print.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2016 Red Hat
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Rob Clark 
+ */
+
+#include 
+#include 
+#include 
+
+void __drm_printfn_seq_file(struct drm_print *p, const char *f, va_list args)
+{
+   seq_vprintf(p->arg, f, args);
+}
+
+void __drm_printfn_info(struct drm_print *p, const char *f, va_list args)
+{
+   const char *hdr = KERN_INFO "[" DRM_NAME "] ";
+   char fmt[strlen(hdr) + strlen(f) + 1];
+
+   memcpy(fmt, hdr, strlen(hdr));
+   memcpy(fmt + strlen(hdr), f, strlen(f));
+   fmt[strlen(hdr) + strlen(f)] = 0;
+
+   vprintk(fmt, args);
+}
+
+void drm_printf(struct drm_print *p, const char *f, ...)
+{
+   va_list args;
+
+   va_start(args, f);
+   p->printfn(p, f, args);
+   va_end(args);
+}
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
new file mode 100644
index 000..f294a9a
--- /dev/null
+++ b/include/drm/drm_print.h
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2016 Red Hat
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Rob Clark 
+ */
+
+#ifndef DRM_PRINT_H_
+#define DRM_PRINT_H_
+
+#include 
+#include 
+
+/* A simple wrapper to abstract seq_file vs printk, so same logging code
+ * does not have to be duplicated.
+ */
+struct drm_print {
+   void (*printfn)(struct drm_print *p, const char *f, va_list args);
+   void *arg;
+};
+
+void __drm_printfn_seq_file(struct drm_print *p, const char *f, va_list args);
+void __drm_printfn_info(struct drm_print *p, const char *f, va_list args);
+
+void drm_printf(struct drm_print *p, const char 

[PATCH 1/6] drm: helper macros to print composite types

[Rob Clark]

I'll want to print things in a similar way in a later patch.  This will
make it easier.

TODO drm_rect_debug_print() doesn't have many call sites, and is kind of
unnecessary now.  Should we just drop it?

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/drm_modes.c |  8 +---
 drivers/gpu/drm/drm_rect.c  | 11 ++-
 include/drm/drmP.h  | 21 +
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index fc5040a..77b0301 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -49,13 +49,7 @@
  */
 void drm_mode_debug_printmodeline(const struct drm_display_mode *mode)
 {
-   DRM_DEBUG_KMS("Modeline %d:\"%s\" %d %d %d %d %d %d %d %d %d %d "
-   "0x%x 0x%x\n",
-   mode->base.id, mode->name, mode->vrefresh, mode->clock,
-   mode->hdisplay, mode->hsync_start,
-   mode->hsync_end, mode->htotal,
-   mode->vdisplay, mode->vsync_start,
-   mode->vsync_end, mode->vtotal, mode->type, mode->flags);
+   DRM_DEBUG_KMS("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
 }
 EXPORT_SYMBOL(drm_mode_debug_printmodeline);

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index a8e2c86..d451112 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -281,17 +281,10 @@ EXPORT_SYMBOL(drm_rect_calc_vscale_relaxed);
  */
 void drm_rect_debug_print(const char *prefix, const struct drm_rect *r, bool 
fixed_point)
 {
-   int w = drm_rect_width(r);
-   int h = drm_rect_height(r);
-
if (fixed_point)
-   DRM_DEBUG_KMS("%s%d.%06ux%d.%06u%+d.%06u%+d.%06u\n", prefix,
- w >> 16, ((w & 0x) * 15625) >> 10,
- h >> 16, ((h & 0x) * 15625) >> 10,
- r->x1 >> 16, ((r->x1 & 0x) * 15625) >> 10,
- r->y1 >> 16, ((r->y1 & 0x) * 15625) >> 10);
+   DRM_DEBUG_KMS("%s" DRM_RECT_FP_FMT "\n", prefix, 
DRM_RECT_FP_ARG(r));
else
-   DRM_DEBUG_KMS("%s%dx%d%+d%+d\n", prefix, w, h, r->x1, r->y1);
+   DRM_DEBUG_KMS("%s" DRM_RECT_FMT "\n", prefix, DRM_RECT_ARG(r));
 }
 EXPORT_SYMBOL(drm_rect_debug_print);

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 28d341a..7ffaa35 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -231,6 +231,27 @@ void drm_err(const char *format, ...);
drm_ut_debug_printk(__func__, fmt, ##args); \
} while (0)

+/* Format strings and argument splitters to simplify printing
+ * various "complex" objects
+ */
+#define DRM_MODE_FMT"%d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x"
+#define DRM_MODE_ARG(m) \
+   (m)->base.id, (m)->name, (m)->vrefresh, (m)->clock, \
+   (m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \
+   (m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \
+   (m)->type, (m)->flags
+
+#define DRM_RECT_FMT"%dx%d%+d%+d"
+#define DRM_RECT_ARG(r) drm_rect_width(r), drm_rect_height(r), (r)->x1, (r)->y1
+
+/* for rect's in fixed-point format: */
+#define DRM_RECT_FP_FMT "%d.%06ux%d.%06u%+d.%06u%+d.%06u"
+#define DRM_RECT_FP_ARG(r) \
+   drm_rect_width(r) >> 16, ((drm_rect_width(r) & 0x) * 15625) 
>> 10, \
+   drm_rect_height(r) >> 16, ((drm_rect_height(r) & 0x) * 
15625) >> 10, \
+   (r)->x1 >> 16, (((r)->x1 & 0x) * 15625) >> 10, \
+   (r)->y1 >> 16, (((r)->y1 & 0x) * 15625) >> 10
+
 /*@}*/

 /***/
-- 
2.7.4

[PATCH 0/6] drm: add atomic state logging and debugfs

[Rob Clark]

Bit more spiffed out version of the RFC.  Now with Sean's suggestion to
add vfuncs in plane/crtc/connector funcs for drivers that subclass the
various state structs.  Plus Ville's suggestion about helper macros
for printing mode/rect structs (and alignment with how drm_rect printed
the integer and fixed-point rects).  Plus addition of connector state
and debugfs.

Rob Clark (6):
  drm: helper macros to print composite types
  drm: add helper for printing to log or seq_file
  drm: add helpers to go from plane state to drm_rect
  drm/atomic: add new drm_debug bit to dump atomic state before commit
  drm/atomic: add debugfs file to dump out atomic state
  drm/msm/mdp5: add atomic_print_state support

 drivers/gpu/drm/Makefile|   3 +-
 drivers/gpu/drm/drm_atomic.c| 133 
 drivers/gpu/drm/drm_debugfs.c   |   9 ++
 drivers/gpu/drm/drm_modes.c |   8 +-
 drivers/gpu/drm/drm_print.c |  54 +++
 drivers/gpu/drm/drm_rect.c  |  11 +--
 drivers/gpu/drm/drm_simple_kms_helper.c |  14 +--
 drivers/gpu/drm/i915/intel_atomic_plane.c   |  10 +--
 drivers/gpu/drm/mediatek/mtk_drm_plane.c|  15 +---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h |  12 +++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  18 +++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  10 +--
 include/drm/drmP.h  |  22 +
 include/drm/drm_atomic.h|   4 +
 include/drm/drm_crtc.h  |  61 +
 include/drm/drm_print.h |  62 +
 16 files changed, 387 insertions(+), 59 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_print.c
 create mode 100644 include/drm/drm_print.h

-- 
2.7.4

[PATCH] drm/msm/mdp5: expose "alpha" property

[Rob Clark]

We had this wired up already internally but initially did not expose the
property pending bikeshed about alpha and color management properties.
I noted that drm-hwc2 was looking for this property, and a couple other
drivers already support it in the same way.  So time to expose it!

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c 
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 432c098..b6f1fc66 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -120,6 +120,7 @@ static void mdp5_plane_install_properties(struct drm_plane 
*plane,
ARRAY_SIZE(name##_prop_enum_list))

INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1);
+   INSTALL_RANGE_PROPERTY(alpha, ALPHA, 0, 255, 255);

mdp5_plane_install_rotation_property(dev, plane);

@@ -148,6 +149,7 @@ static int mdp5_plane_atomic_set_property(struct drm_plane 
*plane,
} while (0)

SET_PROPERTY(zpos, ZPOS, uint8_t);
+   SET_PROPERTY(alpha, ALPHA, uint8_t);

dev_err(dev->dev, "Invalid property\n");
ret = -EINVAL;
@@ -176,6 +178,7 @@ static int mdp5_plane_atomic_get_property(struct drm_plane 
*plane,
} while (0)

GET_PROPERTY(zpos, ZPOS, uint8_t);
+   GET_PROPERTY(alpha, ALPHA, uint8_t);

dev_err(dev->dev, "Invalid property\n");
ret = -EINVAL;
-- 
2.7.4

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

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

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

--- Comment #33 from Yves W.  ---
> But that's still using the radeon driver, right? If so, it's possible to
> narrow down the exact kernel version where the problem was introduced, or
> even the exact Git commit using git bisect.


Yes the original module that used by default - so radeon driver. The info with
amd-gpu-pro driver was just additional info :) it's still beta.. and fps as
much worse than using the radeon driver.

Sorry, but I don't know what you mean by "using bitsect".

-- 
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/20161014/64c5df1d/attachment.html>

[PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Archit Taneja]

On 10/13/2016 10:18 PM, Rob Clark wrote:
> If the bottom-most layer is not fullscreen, we need to use the BASE
> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
> blend_setup() code pretty much handled this already, we just had to
> figure this out in _atomic_check() and assign the stages appropriately.

The patch looks good. I couldn't test the problematic scenario since I
don't have an easy way to reproduce it, but it doesn't break regular
usage (where base layer is full screen).

Reviewed-by: Archit Taneja 

>
> Signed-off-by: Rob Clark 
> ---
> TODO mdp4 might need similar treatment?
>
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44 
> 
>  1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c 
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> index fa2be7c..e42f62d 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>   plane_cnt++;
>   }
>
> - /*
> - * If there is no base layer, enable border color.
> - * Although it's not possbile in current blend logic,
> - * put it here as a reminder.
> - */
>   if (!pstates[STAGE_BASE] && plane_cnt) {
>   ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>   DBG("Border Color is enabled");
> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>   return pa->state->zpos - pb->state->zpos;
>  }
>
> +/* is there a helper for this? */
> +static bool is_fullscreen(struct drm_crtc_state *cstate,
> + struct drm_plane_state *pstate)
> +{
> + return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
> + (pstate->crtc_w == cstate->mode.hdisplay) &&
> + (pstate->crtc_h == cstate->mode.vdisplay);
> +}
> +
>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>   struct drm_crtc_state *state)
>  {
> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>   struct plane_state pstates[STAGE_MAX + 1];
>   const struct mdp5_cfg_hw *hw_cfg;
>   const struct drm_plane_state *pstate;
> - int cnt = 0, i;
> + int cnt = 0, base = 0, i;
>
>   DBG("%s: check", mdp5_crtc->name);
>
> - /* verify that there are not too many planes attached to crtc
> -  * and that we don't have conflicting mixer stages:
> -  */
> - hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>   drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
> - if (cnt >= (hw_cfg->lm.nb_stages)) {
> - dev_err(dev->dev, "too many planes!\n");
> - return -EINVAL;
> - }
> -
> -
>   pstates[cnt].plane = plane;
>   pstates[cnt].state = to_mdp5_plane_state(pstate);
>
> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>   /* assign a stage based on sorted zpos property */
>   sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>
> + /* if the bottom-most layer is not fullscreen, we need to use
> +  * it for solid-color:
> +  */
> + if (!is_fullscreen(state, [0].state->base))
> + base++;
> +
> + /* verify that there are not too many planes attached to crtc
> +  * and that we don't have conflicting mixer stages:
> +  */
> + hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
> +
> + if ((cnt + base) >= hw_cfg->lm.nb_stages) {
> + dev_err(dev->dev, "too many planes!\n");
> + return -EINVAL;
> + }
> +
>   for (i = 0; i < cnt; i++) {
> - pstates[i].state->stage = STAGE_BASE + i;
> + pstates[i].state->stage = STAGE_BASE + i + base;
>   DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>   pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>   pstates[i].state->stage);
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Dan Carpenter]

Hello Rex Zhu,

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 static checker warning:

drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c:2125 
smu7_patch_limits_vddc()
warn: passing casted pointer '>vddc' to 
'smu7_patch_ppt_v0_with_vdd_leakage()' 16 vs 32.

drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c
  2119  static int smu7_patch_limits_vddc(struct pp_hwmgr *hwmgr,
  2120   struct 
phm_clock_and_voltage_limits *tab)
  2121  {
  2122  struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
  2123  
  2124  if (tab) {
  2125  smu7_patch_ppt_v0_with_vdd_leakage(hwmgr, (uint32_t 
*)>vddc,
  2126  
>vddc_leakage);

This call corrupts vddci.

  2127  smu7_patch_ppt_v0_with_vdd_leakage(hwmgr, (uint32_t 
*)>vddci,
  2128  
>vddci_leakage);

But that's fine since we immediately overwrite it here.  But
unfortunately this call corrupt tab->vddgfx.

  2129  }
  2130  
  2131  return 0;
  2132  }

regards,
dan carpenter

[PATCH v2 10/10] drm/i915/gen9: Don't wrap strings in verify_wm_state()

[Lyude]

Wrapping strings is against the guidelines in Documentation/CodingStyle,
chapter 2.

Signed-off-by: Lyude 
Reviewed-by: Paulo Zanoni 
Cc: Maarten Lankhorst 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/intel_display.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ca12f0e..eaec9f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13509,8 +13509,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
sw_ddb_entry = _ddb->plane[pipe][plane];

if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
-   DRM_ERROR("mismatch in DDB state pipe %c plane %d "
- "(expected (%u,%u), found (%u,%u))\n",
+   DRM_ERROR("mismatch in DDB state pipe %c plane %d 
(expected (%u,%u), found (%u,%u))\n",
  pipe_name(pipe), plane + 1,
  sw_ddb_entry->start, sw_ddb_entry->end,
  hw_ddb_entry->start, hw_ddb_entry->end);
@@ -13560,8 +13559,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
sw_ddb_entry = _ddb->plane[pipe][PLANE_CURSOR];

if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
-   DRM_ERROR("mismatch in DDB state pipe %c cursor "
- "(expected (%u,%u), found (%u,%u))\n",
+   DRM_ERROR("mismatch in DDB state pipe %c cursor 
(expected (%u,%u), found (%u,%u))\n",
  pipe_name(pipe),
  sw_ddb_entry->start, sw_ddb_entry->end,
  hw_ddb_entry->start, hw_ddb_entry->end);
-- 
2.7.4

[PATCH v2 09/10] drm/i915/gen9: Actually verify WM levels in verify_wm_state()

[Lyude]

Thanks to Paulo Zanoni for indirectly pointing this out.

Looks like we never actually added any code for checking whether or not
we actually wrote watermark levels properly. Let's fix that.

Changes since v1:
- Use %u instead of %d when printing WM state mismatches

Signed-off-by: Lyude 
Reviewed-by: Paulo Zanoni 
Cc: Maarten Lankhorst 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/intel_display.c | 100 +--
 1 file changed, 84 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 18e82b4..ca12f0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13455,30 +13455,66 @@ static void verify_wm_state(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct skl_ddb_allocation hw_ddb, *sw_ddb;
-   struct skl_ddb_entry *hw_entry, *sw_entry;
+   struct skl_pipe_wm hw_wm, *sw_wm;
+   struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
+   struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
const enum pipe pipe = intel_crtc->pipe;
-   int plane;
+   int plane, level, max_level = ilk_wm_max_level(dev_priv);

if (INTEL_INFO(dev)->gen < 9 || !new_state->active)
return;

+   skl_pipe_wm_get_hw_state(crtc, _wm);
+   sw_wm = _crtc->wm.active.skl;
+
skl_ddb_get_hw_state(dev_priv, _ddb);
sw_ddb = _priv->wm.skl_hw.ddb;

/* planes */
for_each_plane(dev_priv, pipe, plane) {
-   hw_entry = _ddb.plane[pipe][plane];
-   sw_entry = _ddb->plane[pipe][plane];
+   hw_plane_wm = _wm.planes[plane];
+   sw_plane_wm = _wm->planes[plane];

-   if (skl_ddb_entry_equal(hw_entry, sw_entry))
-   continue;
+   /* Watermarks */
+   for (level = 0; level <= max_level; level++) {
+   if (skl_wm_level_equals(_plane_wm->wm[level],
+   _plane_wm->wm[level]))
+   continue;
+
+   DRM_ERROR("mismatch in WM pipe %c plane %d level %d 
(expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe), plane + 1, level,
+ sw_plane_wm->wm[level].plane_en,
+ sw_plane_wm->wm[level].plane_res_b,
+ sw_plane_wm->wm[level].plane_res_l,
+ hw_plane_wm->wm[level].plane_en,
+ hw_plane_wm->wm[level].plane_res_b,
+ hw_plane_wm->wm[level].plane_res_l);
+   }

-   DRM_ERROR("mismatch in DDB state pipe %c plane %d "
- "(expected (%u,%u), found (%u,%u))\n",
- pipe_name(pipe), plane + 1,
- sw_entry->start, sw_entry->end,
- hw_entry->start, hw_entry->end);
+   if (!skl_wm_level_equals(_plane_wm->trans_wm,
+_plane_wm->trans_wm)) {
+   DRM_ERROR("mismatch in trans WM pipe %c plane %d 
(expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe), plane + 1,
+ sw_plane_wm->trans_wm.plane_en,
+ sw_plane_wm->trans_wm.plane_res_b,
+ sw_plane_wm->trans_wm.plane_res_l,
+ hw_plane_wm->trans_wm.plane_en,
+ hw_plane_wm->trans_wm.plane_res_b,
+ hw_plane_wm->trans_wm.plane_res_l);
+   }
+
+   /* DDB */
+   hw_ddb_entry = _ddb.plane[pipe][plane];
+   sw_ddb_entry = _ddb->plane[pipe][plane];
+
+   if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
+   DRM_ERROR("mismatch in DDB state pipe %c plane %d "
+ "(expected (%u,%u), found (%u,%u))\n",
+ pipe_name(pipe), plane + 1,
+ sw_ddb_entry->start, sw_ddb_entry->end,
+ hw_ddb_entry->start, hw_ddb_entry->end);
+   }
}

/*
@@ -13488,15 +13524,47 @@ static void verify_wm_state(struct drm_crtc *crtc,
 * once the plane becomes visible, we can skip this check
 */
if (intel_crtc->cursor_addr) {
-   hw_entry = _ddb.plane[pipe][PLANE_CURSOR];
-   sw_entry = _ddb->plane[pipe][PLANE_CURSOR];
+   hw_plane_wm = _wm.planes[PLANE_CURSOR];
+   sw_plane_wm = _wm->planes[PLANE_CURSOR];
+
+   /* 

[PATCH v2 08/10] drm/i915/gen9: Add skl_wm_level_equals()

[Lyude]

Helper we're going to be using for implementing verification of the wm
levels in skl_verify_wm_level().

Signed-off-by: Lyude 
Reviewed-by: Paulo Zanoni 
Cc: Maarten Lankhorst 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c  | 14 ++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7036310..96963ea 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1761,6 +1761,8 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
 bool intel_can_enable_sagv(struct drm_atomic_state *state);
 int intel_enable_sagv(struct drm_i915_private *dev_priv);
 int intel_disable_sagv(struct drm_i915_private *dev_priv);
+bool skl_wm_level_equals(const struct skl_wm_level *l1,
+const struct skl_wm_level *l2);
 bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
   const struct skl_ddb_allocation *new,
   enum pipe pipe);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6eaeb87..fae3ce4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3857,6 +3857,20 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
>plane[pipe][PLANE_CURSOR]);
 }

+bool skl_wm_level_equals(const struct skl_wm_level *l1,
+const struct skl_wm_level *l2)
+{
+   if (l1->plane_en != l2->plane_en)
+   return false;
+
+   /* If both planes aren't enabled, the rest shouldn't matter */
+   if (!l1->plane_en)
+   return true;
+
+   return (l1->plane_res_l == l2->plane_res_l &&
+   l1->plane_res_b == l2->plane_res_b);
+}
+
 static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
   const struct skl_ddb_entry *b)
 {
-- 
2.7.4

[PATCH v2 07/10] drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable

[Lyude]

There's not much of a reason this should have the locations to read out
the hardware state hardcoded, so allow the caller to specify the
location and add this function to intel_drv.h. As well, we're going to
need this function to be reusable for the next patch.

Changes since v1:
- Fix accidental behavior change in the code that Paulo pointed out

Signed-off-by: Lyude 
Cc: Maarten Lankhorst 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c  | 28 ++--
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a85ce2c..7036310 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1756,6 +1756,8 @@ void ilk_wm_get_hw_state(struct drm_device *dev);
 void skl_wm_get_hw_state(struct drm_device *dev);
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
  struct skl_ddb_allocation *ddb /* out */);
+void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
+ struct skl_pipe_wm *out);
 bool intel_can_enable_sagv(struct drm_atomic_state *state);
 int intel_enable_sagv(struct drm_i915_private *dev_priv);
 int intel_disable_sagv(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2fe851e..6eaeb87 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4288,15 +4288,13 @@ static inline void skl_wm_level_from_reg_val(uint32_t 
val,
PLANE_WM_LINES_MASK;
 }

-static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
+void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
+ struct skl_pipe_wm *out)
 {
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
-   struct skl_wm_values *hw = _priv->wm.skl_hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-   struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
struct intel_plane *intel_plane;
-   struct skl_pipe_wm *active = >wm.skl.optimal;
struct skl_plane_wm *wm;
enum pipe pipe = intel_crtc->pipe;
int level, id, max_level;
@@ -4306,7 +4304,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc 
*crtc)

for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
id = skl_wm_plane_id(intel_plane);
-   wm = >wm.skl.optimal.planes[id];
+   wm = >planes[id];

for (level = 0; level <= max_level; level++) {
if (id != PLANE_CURSOR)
@@ -4328,20 +4326,30 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc 
*crtc)
if (!intel_crtc->active)
return;

-   hw->dirty_pipes |= drm_crtc_mask(crtc);
-   active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
-   intel_crtc->wm.active.skl = *active;
+   out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
 }

 void skl_wm_get_hw_state(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = to_i915(dev);
+   struct skl_wm_values *hw = _priv->wm.skl_hw;
struct skl_ddb_allocation *ddb = _priv->wm.skl_hw.ddb;
struct drm_crtc *crtc;
+   struct intel_crtc *intel_crtc;
+   struct intel_crtc_state *cstate;

skl_ddb_get_hw_state(dev_priv, ddb);
-   list_for_each_entry(crtc, >mode_config.crtc_list, head)
-   skl_pipe_wm_get_hw_state(crtc);
+   list_for_each_entry(crtc, >mode_config.crtc_list, head) {
+   intel_crtc = to_intel_crtc(crtc);
+   cstate = to_intel_crtc_state(crtc->state);
+
+   skl_pipe_wm_get_hw_state(crtc, >wm.skl.optimal);
+
+   if (intel_crtc->active) {
+   hw->dirty_pipes |= drm_crtc_mask(crtc);
+   intel_crtc->wm.active.skl = cstate->wm.skl.optimal;
+   }
+   }

if (dev_priv->active_crtcs) {
/* Fully recompute DDB on first atomic commit */
-- 
2.7.4

[PATCH v3 06/10] drm/i915/gen9: Add ddb changes to atomic debug output

[Lyude]

Finally, add some debugging output for ddb changes in the atomic debug
output. This makes it a lot easier to spot bugs from incorrect ddb
allocations.

Signed-off-by: Lyude 
Reviewed-by: Maarten Lankhorst 
Reviewed-by: Paulo Zanoni 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/intel_pm.c | 54 +
 1 file changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5df5cea..2fe851e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4044,6 +4044,58 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
   sizeof(dst->ddb.plane[pipe]));
 }

+static void
+skl_print_wm_changes(const struct drm_atomic_state *state)
+{
+   const struct drm_device *dev = state->dev;
+   const struct drm_i915_private *dev_priv = to_i915(dev);
+   const struct intel_atomic_state *intel_state =
+   to_intel_atomic_state(state);
+   const struct drm_crtc *crtc;
+   const struct drm_crtc_state *cstate;
+   const struct drm_plane *plane;
+   const struct intel_plane *intel_plane;
+   const struct drm_plane_state *pstate;
+   const struct skl_ddb_allocation *old_ddb = _priv->wm.skl_hw.ddb;
+   const struct skl_ddb_allocation *new_ddb = _state->wm_results.ddb;
+   enum pipe pipe;
+   int id;
+   int i, j;
+
+   for_each_crtc_in_state(state, crtc, cstate, i) {
+   pipe = to_intel_crtc(crtc)->pipe;
+
+   for_each_plane_in_state(state, plane, pstate, j) {
+   const struct skl_ddb_entry *old, *new;
+
+   intel_plane = to_intel_plane(plane);
+   id = skl_wm_plane_id(intel_plane);
+   old = _ddb->plane[pipe][id];
+   new = _ddb->plane[pipe][id];
+
+   if (intel_plane->pipe != pipe)
+   continue;
+
+   if (skl_ddb_entry_equal(old, new))
+   continue;
+
+   if (id != PLANE_CURSOR) {
+   DRM_DEBUG_ATOMIC("[PLANE:%d:plane %d%c] ddb (%d 
- %d) -> (%d - %d)\n",
+plane->base.id, id + 1,
+pipe_name(pipe),
+old->start, old->end,
+new->start, new->end);
+   } else {
+   DRM_DEBUG_ATOMIC("[PLANE:%d:cursor %c] ddb (%d 
- %d) -> (%d - %d)\n",
+plane->base.id,
+pipe_name(pipe),
+old->start, old->end,
+new->start, new->end);
+   }
+   }
+   }
+}
+
 static int
 skl_compute_wm(struct drm_atomic_state *state)
 {
@@ -4105,6 +4157,8 @@ skl_compute_wm(struct drm_atomic_state *state)
intel_cstate->update_wm_pre = true;
}

+   skl_print_wm_changes(state);
+
return 0;
 }

-- 
2.7.4

[PATCH v3 05/10] drm/i915/gen9: Get rid of redundant watermark values

[Lyude]

Now that we've make skl_wm_levels make a little more sense, we can
remove all of the redundant wm information. Up until now we'd been
storing two copies of all of the skl watermarks: one being the
skl_pipe_wm structs, the other being the global wm struct in
drm_i915_private containing the raw register values. This is confusing
and problematic, since it means we're prone to accidentally letting the
two copies go out of sync. So, get rid of all of the functions
responsible for computing the register values and just use a single
helper, skl_write_wm_level(), to convert and write the new watermarks on
the fly.

Changes since v1:
- Fixup skl_write_wm_level()
- Fixup skl_wm_level_from_reg_val()
- Don't forget to copy *active to intel_crtc->wm.active.skl
Changes since v2:
- Fix usage of wrong cstate

Signed-off-by: Lyude 
Reviewed-by: Maarten Lankhorst 
Reviewed-by: Paulo Zanoni 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/i915_drv.h  |   2 -
 drivers/gpu/drm/i915/intel_display.c |  14 ++-
 drivers/gpu/drm/i915/intel_drv.h |   6 +-
 drivers/gpu/drm/i915/intel_pm.c  | 204 ---
 drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
 5 files changed, 91 insertions(+), 143 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4d1133f..929b643 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1648,8 +1648,6 @@ struct skl_ddb_allocation {
 struct skl_wm_values {
unsigned dirty_pipes;
struct skl_ddb_allocation ddb;
-   uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
-   uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
 };

 struct skl_wm_level {
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ced70ad..18e82b4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3378,6 +3378,8 @@ static void skylake_update_primary_plane(struct drm_plane 
*plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
const struct skl_wm_values *wm = _priv->wm.skl_results;
+   const struct skl_plane_wm *p_wm =
+   _state->wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;
u32 plane_ctl;
unsigned int rotation = plane_state->base.rotation;
@@ -3414,7 +3416,7 @@ static void skylake_update_primary_plane(struct drm_plane 
*plane,
intel_crtc->adjusted_y = src_y;

if (wm->dirty_pipes & drm_crtc_mask(_crtc->base))
-   skl_write_plane_wm(intel_crtc, wm, 0);
+   skl_write_plane_wm(intel_crtc, p_wm, >ddb, 0);

I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
@@ -3448,6 +3450,8 @@ static void skylake_disable_primary_plane(struct 
drm_plane *primary,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+   struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+   const struct skl_plane_wm *p_wm = >wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;

/*
@@ -3455,7 +3459,8 @@ static void skylake_disable_primary_plane(struct 
drm_plane *primary,
 * plane's visiblity isn't actually changing neither is its watermarks.
 */
if (!crtc->primary->state->visible)
-   skl_write_plane_wm(intel_crtc, _priv->wm.skl_results, 0);
+   skl_write_plane_wm(intel_crtc, p_wm,
+  _priv->wm.skl_results.ddb, 0);

I915_WRITE(PLANE_CTL(pipe, 0), 0);
I915_WRITE(PLANE_SURF(pipe, 0), 0);
@@ -10826,12 +10831,15 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, 
u32 base,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+   struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
const struct skl_wm_values *wm = _priv->wm.skl_results;
+   const struct skl_plane_wm *p_wm =
+   >wm.skl.optimal.planes[PLANE_CURSOR];
int pipe = intel_crtc->pipe;
uint32_t cntl = 0;

if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc))
-   skl_write_cursor_wm(intel_crtc, wm);
+   skl_write_cursor_wm(intel_crtc, p_wm, >ddb);

if (plane_state && plane_state->base.visible) {
cntl = MCURSOR_GAMMA_ENABLE;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 84301d3..a85ce2c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1765,9 +1765,11 @@ bool skl_ddb_allocation_equals(const struct 
skl_ddb_allocation *old,
 bool skl_ddb_allocation_overlaps(struct 

[PATCH v2 04/10] drm/i915/gen9: Cleanup skl_pipe_wm_active_state

[Lyude]

This function is a wreck, let's help it get its life back together and
cleanup all of the copy pasta here.

Signed-off-by: Lyude 
Reviewed-by: Maarten Lankhorst 
Reviewed-by: Paulo Zanoni 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/intel_pm.c | 52 +++--
 1 file changed, 14 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1bdccc9..2d63a37 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4271,46 +4271,22 @@ static void ilk_optimize_watermarks(struct 
intel_crtc_state *cstate)
 static void skl_pipe_wm_active_state(uint32_t val,
 struct skl_pipe_wm *active,
 bool is_transwm,
-bool is_cursor,
 int i,
 int level)
 {
+   struct skl_plane_wm *plane_wm = >planes[i];
bool is_enabled = (val & PLANE_WM_EN) != 0;

if (!is_transwm) {
-   if (!is_cursor) {
-   active->planes[i].wm[level].plane_en = is_enabled;
-   active->planes[i].wm[level].plane_res_b =
-   val & PLANE_WM_BLOCKS_MASK;
-   active->planes[i].wm[level].plane_res_l =
-   (val >> PLANE_WM_LINES_SHIFT) &
-   PLANE_WM_LINES_MASK;
-   } else {
-   active->planes[PLANE_CURSOR].wm[level].plane_en =
-   is_enabled;
-   active->planes[PLANE_CURSOR].wm[level].plane_res_b =
-   val & PLANE_WM_BLOCKS_MASK;
-   active->planes[PLANE_CURSOR].wm[level].plane_res_l =
-   (val >> PLANE_WM_LINES_SHIFT) &
-   PLANE_WM_LINES_MASK;
-   }
+   plane_wm->wm[level].plane_en = is_enabled;
+   plane_wm->wm[level].plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+   plane_wm->wm[level].plane_res_l =
+   (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
} else {
-   if (!is_cursor) {
-   active->planes[i].trans_wm.plane_en = is_enabled;
-   active->planes[i].trans_wm.plane_res_b =
-   val & PLANE_WM_BLOCKS_MASK;
-   active->planes[i].trans_wm.plane_res_l =
-   (val >> PLANE_WM_LINES_SHIFT) &
-   PLANE_WM_LINES_MASK;
-   } else {
-   active->planes[PLANE_CURSOR].trans_wm.plane_en =
-   is_enabled;
-   active->planes[PLANE_CURSOR].trans_wm.plane_res_b =
-   val & PLANE_WM_BLOCKS_MASK;
-   active->planes[PLANE_CURSOR].trans_wm.plane_res_l =
-   (val >> PLANE_WM_LINES_SHIFT) &
-   PLANE_WM_LINES_MASK;
-   }
+   plane_wm->trans_wm.plane_en = is_enabled;
+   plane_wm->trans_wm.plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+   plane_wm->trans_wm.plane_res_l =
+   (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
}
 }

@@ -4349,20 +4325,20 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc 
*crtc)
for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
temp = hw->plane[pipe][i][level];
-   skl_pipe_wm_active_state(temp, active, false,
-   false, i, level);
+   skl_pipe_wm_active_state(temp, active, false, i, level);
}
temp = hw->plane[pipe][PLANE_CURSOR][level];
-   skl_pipe_wm_active_state(temp, active, false, true, i, level);
+   skl_pipe_wm_active_state(temp, active, false, PLANE_CURSOR,
+level);
}

for (i = 0; i < intel_num_planes(intel_crtc); i++) {
temp = hw->plane_trans[pipe][i];
-   skl_pipe_wm_active_state(temp, active, true, false, i, 0);
+   skl_pipe_wm_active_state(temp, active, true, i, 0);
}

temp = hw->plane_trans[pipe][PLANE_CURSOR];
-   skl_pipe_wm_active_state(temp, active, true, true, i, 0);
+   skl_pipe_wm_active_state(temp, active, true, PLANE_CURSOR, 0);

intel_crtc->wm.active.skl = *active;
 }
-- 
2.7.4

[PATCH v3 03/10] drm/i915/gen9: Make skl_wm_level per-plane

[Lyude]

Having skl_wm_level contain all of the watermarks for each plane is
annoying since it prevents us from having any sort of object to
represent a single watermark level, something we take advantage of in
the next commit to cut down on all of the copy paste code in here.

Changes since v1:
- Style nitpicks
- Fix accidental usage of i vs. PLANE_CURSOR
- Split out skl_pipe_wm_active_state simplification into seperate patch

Signed-off-by: Lyude 
Reviewed-by: Paulo Zanoni 
Reviewed-by: Maarten Lankhorst 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/i915_drv.h  |   6 +-
 drivers/gpu/drm/i915/intel_drv.h |   6 +-
 drivers/gpu/drm/i915/intel_pm.c  | 207 +++
 3 files changed, 111 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e56d4a4..4d1133f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1653,9 +1653,9 @@ struct skl_wm_values {
 };

 struct skl_wm_level {
-   bool plane_en[I915_MAX_PLANES];
-   uint16_t plane_res_b[I915_MAX_PLANES];
-   uint8_t plane_res_l[I915_MAX_PLANES];
+   bool plane_en;
+   uint16_t plane_res_b;
+   uint8_t plane_res_l;
 };

 /*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 10a0cf2..84301d3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -468,9 +468,13 @@ struct intel_pipe_wm {
bool sprites_scaled;
 };

-struct skl_pipe_wm {
+struct skl_plane_wm {
struct skl_wm_level wm[8];
struct skl_wm_level trans_wm;
+};
+
+struct skl_pipe_wm {
+   struct skl_plane_wm planes[I915_MAX_PLANES];
uint32_t linetime;
 };

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7d5721..1bdccc9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3674,67 +3674,52 @@ static int
 skl_compute_wm_level(const struct drm_i915_private *dev_priv,
 struct skl_ddb_allocation *ddb,
 struct intel_crtc_state *cstate,
+struct intel_plane *intel_plane,
 int level,
 struct skl_wm_level *result)
 {
struct drm_atomic_state *state = cstate->base.state;
struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
-   struct drm_plane *plane;
-   struct intel_plane *intel_plane;
-   struct intel_plane_state *intel_pstate;
+   struct drm_plane *plane = _plane->base;
+   struct intel_plane_state *intel_pstate = NULL;
uint16_t ddb_blocks;
enum pipe pipe = intel_crtc->pipe;
int ret;
+   int i = skl_wm_plane_id(intel_plane);
+
+   if (state)
+   intel_pstate =
+   intel_atomic_get_existing_plane_state(state,
+ intel_plane);

/*
-* We'll only calculate watermarks for planes that are actually
-* enabled, so make sure all other planes are set as disabled.
+* Note: If we start supporting multiple pending atomic commits against
+* the same planes/CRTC's in the future, plane->state will no longer be
+* the correct pre-state to use for the calculations here and we'll
+* need to change where we get the 'unchanged' plane data from.
+*
+* For now this is fine because we only allow one queued commit against
+* a CRTC.  Even if the plane isn't modified by this transaction and we
+* don't have a plane lock, we still have the CRTC's lock, so we know
+* that no other transactions are racing with us to update it.
 */
-   memset(result, 0, sizeof(*result));
+   if (!intel_pstate)
+   intel_pstate = to_intel_plane_state(plane->state);

-   for_each_intel_plane_mask(_priv->drm,
- intel_plane,
- cstate->base.plane_mask) {
-   int i = skl_wm_plane_id(intel_plane);
-
-   plane = _plane->base;
-   intel_pstate = NULL;
-   if (state)
-   intel_pstate =
-   intel_atomic_get_existing_plane_state(state,
- 
intel_plane);
+   WARN_ON(!intel_pstate->base.fb);

-   /*
-* Note: If we start supporting multiple pending atomic commits
-* against the same planes/CRTC's in the future, plane->state
-* will no longer be the correct pre-state to use for the
-* calculations here and we'll need to change where we get the
-* 'unchanged' plane data from.
-*
-* For now this is fine because we only allow one queued commit
-* against a CRTC.  Even if the plane isn't 

[PATCH v3 02/10] drm/i915/skl: Remove linetime from skl_wm_values

[Lyude]

Next part of cleaning up the watermark code for skl. This is easy, since
it seems that we never actually needed to keep track of the linetime in
the skl_wm_values struct anyway.

Signed-off-by: Lyude 
Reviewed-by: Paulo Zanoni 
Reviewed-by: Maarten Lankhorst 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/i915_drv.h  | 1 -
 drivers/gpu/drm/i915/intel_display.c | 6 --
 drivers/gpu/drm/i915/intel_pm.c  | 7 +--
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ac4287f99..e56d4a4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1648,7 +1648,6 @@ struct skl_ddb_allocation {
 struct skl_wm_values {
unsigned dirty_pipes;
struct skl_ddb_allocation ddb;
-   uint32_t wm_linetime[I915_MAX_PIPES];
uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
 };
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7a68cc3..ced70ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14844,6 +14844,8 @@ static void intel_begin_crtc_commit(struct drm_crtc 
*crtc,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+   struct intel_crtc_state *intel_cstate =
+   to_intel_crtc_state(crtc->state);
struct intel_crtc_state *old_intel_state =
to_intel_crtc_state(old_crtc_state);
bool modeset = needs_modeset(crtc->state);
@@ -14860,13 +14862,13 @@ static void intel_begin_crtc_commit(struct drm_crtc 
*crtc,
intel_color_load_luts(crtc->state);
}

-   if (to_intel_crtc_state(crtc->state)->update_pipe)
+   if (intel_cstate->update_pipe)
intel_update_pipe_config(intel_crtc, old_intel_state);
else if (INTEL_GEN(dev_priv) >= 9) {
skl_detach_scalers(intel_crtc);

I915_WRITE(PIPE_WM_LINETIME(pipe),
-  dev_priv->wm.skl_hw.wm_linetime[pipe]);
+  intel_cstate->wm.skl.optimal.linetime);
}
 }

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 66586af..a7d5721 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3845,8 +3845,6 @@ static void skl_compute_wm_results(struct drm_device *dev,
temp |= PLANE_WM_EN;

r->plane_trans[pipe][PLANE_CURSOR] = temp;
-
-   r->wm_linetime[pipe] = p_wm->linetime;
 }

 static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
@@ -4081,7 +4079,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
 struct skl_wm_values *src,
 enum pipe pipe)
 {
-   dst->wm_linetime[pipe] = src->wm_linetime[pipe];
memcpy(dst->plane[pipe], src->plane[pipe],
   sizeof(dst->plane[pipe]));
memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
@@ -4332,8 +4329,6 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc 
*crtc)

max_level = ilk_wm_max_level(dev_priv);

-   hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
-
for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++)
hw->plane[pipe][i][level] =
@@ -4350,7 +4345,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc 
*crtc)

hw->dirty_pipes |= drm_crtc_mask(crtc);

-   active->linetime = hw->wm_linetime[pipe];
+   active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));

for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
-- 
2.7.4

[PATCH v3 01/10] drm/i915/skl: Move per-pipe ddb allocations into crtc states

[Lyude]

First part of cleaning up all of the skl watermark code. This moves the
structures for storing the ddb allocations of each pipe into
intel_crtc_state, along with moving the structures for storing the
current ddb allocations active on hardware into intel_crtc.

Changes since v1:
- Don't replace alloc->start = alloc->end = 0;

Signed-off-by: Lyude 
Reviewed-by: Maarten Lankhorst 
Reviewed-by: Paulo Zanoni 
Cc: Ville Syrjälä 
Cc: Matt Roper 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 -
 drivers/gpu/drm/i915/intel_display.c | 16 ---
 drivers/gpu/drm/i915/intel_drv.h |  8 +---
 drivers/gpu/drm/i915/intel_pm.c  | 40 +++-
 4 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fe875b2..ac4287f99 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1641,7 +1641,6 @@ static inline bool skl_ddb_entry_equal(const struct 
skl_ddb_entry *e1,
 }

 struct skl_ddb_allocation {
-   struct skl_ddb_entry pipe[I915_MAX_PIPES];
struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; /* 
packed/uv */
struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
 };
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 60c699e..7a68cc3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14246,12 +14246,11 @@ static void skl_update_crtcs(struct drm_atomic_state 
*state,
 unsigned int *crtc_vblank_mask)
 {
struct drm_device *dev = state->dev;
-   struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_crtc *crtc;
+   struct intel_crtc *intel_crtc;
struct drm_crtc_state *old_crtc_state;
-   struct skl_ddb_allocation *new_ddb = _state->wm_results.ddb;
-   struct skl_ddb_allocation *cur_ddb = _priv->wm.skl_hw.ddb;
+   struct intel_crtc_state *cstate;
unsigned int updated = 0;
bool progress;
enum pipe pipe;
@@ -14269,12 +14268,14 @@ static void skl_update_crtcs(struct drm_atomic_state 
*state,
for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
bool vbl_wait = false;
unsigned int cmask = drm_crtc_mask(crtc);
-   pipe = to_intel_crtc(crtc)->pipe;
+
+   intel_crtc = to_intel_crtc(crtc);
+   cstate = to_intel_crtc_state(crtc->state);
+   pipe = intel_crtc->pipe;

if (updated & cmask || !crtc->state->active)
continue;
-   if (skl_ddb_allocation_overlaps(state, cur_ddb, new_ddb,
-   pipe))
+   if (skl_ddb_allocation_overlaps(state, intel_crtc))
continue;

updated |= cmask;
@@ -14285,7 +14286,8 @@ static void skl_update_crtcs(struct drm_atomic_state 
*state,
 * then we need to wait for a vblank to pass for the
 * new ddb allocation to take effect.
 */
-   if (!skl_ddb_allocation_equals(cur_ddb, new_ddb, pipe) 
&&
+   if (!skl_ddb_entry_equal(>wm.skl.ddb,
+_crtc->hw_ddb) &&
!crtc->state->active_changed &&
intel_state->wm_results.dirty_pipes != updated)
vbl_wait = true;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5bc1154..10a0cf2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -496,6 +496,7 @@ struct intel_crtc_wm_state {
struct {
/* gen9+ only needs 1-step wm programming */
struct skl_pipe_wm optimal;
+   struct skl_ddb_entry ddb;

/* cached plane data rate */
unsigned plane_data_rate[I915_MAX_PLANES];
@@ -733,6 +734,9 @@ struct intel_crtc {
bool cxsr_allowed;
} wm;

+   /* gen9+: ddb allocation currently being used */
+   struct skl_ddb_entry hw_ddb;
+
int scanline_offset;

struct {
@@ -1755,9 +1759,7 @@ bool skl_ddb_allocation_equals(const struct 
skl_ddb_allocation *old,
   const struct skl_ddb_allocation *new,
   enum pipe pipe);
 bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
-const struct skl_ddb_allocation *old,
-const struct skl_ddb_allocation *new,
-enum 

[PATCH v3 00/10] Start of skl watermark cleanup

[Lyude]

While it (mostly) works, the code for handling watermarks on Skylake has been
kind of ugly for a while. As well a lot of it isn't that friendly to atomic
transactions, Lots of copy paste, redundant wm values, etc. While this isn't a
full cleanup, it's a good start. As well, we add a couple of features for
making debugging watermarks a little easier.

Rebased for latest nightly, new r-bs added + some changes

Lyude (10):
  drm/i915/skl: Move per-pipe ddb allocations into crtc states
  drm/i915/skl: Remove linetime from skl_wm_values
  drm/i915/gen9: Make skl_wm_level per-plane
  drm/i915/gen9: Cleanup skl_pipe_wm_active_state
  drm/i915/gen9: Get rid of redundant watermark values
  drm/i915/gen9: Add ddb changes to atomic debug output
  drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
  drm/i915/gen9: Add skl_wm_level_equals()
  drm/i915/gen9: Actually verify WM levels in verify_wm_state()
  drm/i915/gen9: Don't wrap strings in verify_wm_state()

 drivers/gpu/drm/i915/i915_drv.h  |  10 +-
 drivers/gpu/drm/i915/intel_display.c | 138 ---
 drivers/gpu/drm/i915/intel_drv.h |  24 +-
 drivers/gpu/drm/i915/intel_pm.c  | 466 +--
 drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
 5 files changed, 355 insertions(+), 291 deletions(-)

-- 
2.7.4

[RFC PATCH 00/11] Introduce writeback connectors

[Daniel Vetter]

On Fri, Oct 14, 2016 at 2:39 PM, Brian Starkey  wrote:
>> - Besides the above property, writeback hardware can have provisions
>> for scaling, color space conversion and rotation. This would mean that
>> we'd eventually add more writeback specific props/params in
>> drm_connector/drm_connector_state. Would we be okay adding more such
>> props for connectors?
>
>
> I've wondered the same thing about bloating non-writeback connectors
> with writeback-specific stuff. If it does become significant, maybe
> we should subclass drm_connector and add a drm_writeback_state pointer
> to drm_connector_state.

No pionters needed, just embedded drm_connector_state into
drm_writeback_connector_state as the "base" member. Then we can
provide ready-made atomic_set/get_property functions for the aditional
writeback functionality.

But tbh I'd only start doing that once we have a few more. It's purely
an implementation change, with no effect on userspace. And if you go
with my drm_writeback_connector_init idea, it won't even be an issue
for drivers.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[Bug 94354] R9285 Unigine Valley perf regression since radeonsi: use re-Z

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

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

--- Comment #3 from Marek Olšák  ---
Thanks and sorry for missing this bug.

-- 
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/20161014/a4c6fc63/attachment.html>

[PATCH v3 1/9] drm/hisilicon/hibmc: Add hisilicon hibmc drm master driver

[Benjamin Gaignard]

Just by curiosity, why using "old" TTM instead of GEM ? any particular reasons ?

2016-10-14 16:44 GMT+02:00 Rongrong Zou :
> Hi Benjamin,
>
> Thanks for reviewing!
>
> Benjamin Gaignard 於 2016/10/14 16:29 寫道:
>>
>> [snip]
>>
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>>> @@ -0,0 +1,15 @@
>>> +config DRM_HISI_HIBMC
>>> +   tristate "DRM Support for Hisilicon Hibmc"
>>> +   depends on DRM && PCI
>>> +   select DRM_KMS_HELPER
>>> +   select DRM_KMS_FB_HELPER
>>> +   select DRM_GEM_CMA_HELPER
>>> +   select DRM_KMS_CMA_HELPER
>>
>>
>> since you use TTM I don't think that selecting DRM_GEM_CMA_HELPER and
>> DRM_KMS_CMA_HELPER
>> help you lot here.
>> You could add configuration flags step by step in following patches
>> that will make you needs more clear (that also true for #include)
>
>
> will delete them, thanks.
>
>
>>
>>> +   select FB_SYS_FILLRECT
>>> +   select FB_SYS_COPYAREA
>>> +   select FB_SYS_IMAGEBLIT
>>> +   select DRM_TTM
>>> +
>>> +   help
>>> + Choose this option if you have a Hisilicon Hibmc soc chipset.
>>> + If M is selected the module will be called hibmc-drm.
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> new file mode 100644
>>> index 000..97cf4a0
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> @@ -0,0 +1,5 @@
>>> +ccflags-y := -Iinclude/drm
>>> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_power.o
>>> +
>>> +obj-$(CONFIG_DRM_HISI_HIBMC)   +=hibmc-drm.o
>>> +#obj-y += hibmc-drm.o
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> new file mode 100644
>>> index 000..52c9353
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> @@ -0,0 +1,288 @@
>>> +/* Hisilicon Hibmc SoC drm driver
>>> + *
>>> + * Based on the bochs drm driver.
>>> + *
>>> + * Copyright (c) 2016 Huawei Limited.
>>> + *
>>> + * Author:
>>> + * Rongrong Zou 
>>
>>
>> ".com" is missing in you email address (same typo in all other files)
>
>
> will fix it in next version, thanks. :)
>
>>
>>> + * Rongrong Zou 
>>> + * Jianhua Li 
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>
>>
>> cma_helpers look useless since you use TTM, no ?
>
>
> I add TTM just in this version, and forgot to clean these
> cma relevant code, will fix in next version. Thanks.
>
>>
>>> +#include 
>>> +
>>> +#include "hibmc_drm_drv.h"
>>> +#include "hibmc_drm_regs.h"
>>> +#include "hibmc_drm_power.h"
>>> +
>
>
> Regards,
> Rongrong



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

[PATCH] drm/arcpgu: Accommodate adv7511 switch to DRM bridge

[Eugeniy Paltsev]

ARC PGU driver starts crashing on initialization after
'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
of "drm_i2c_encoder_driver" structure, which doesn't exist after
adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
So, when we call "encoder_init" function from this structure driver
crashes.

Bootlog:
->8
[drm] Initialized drm 1.1.0 20060810
arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab
arcpgu e0017000.pgu: assigned reserved memory node frame_buffer at 9e00
Path: (null)
CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-1-gb5642252fa01-dirty #8
task: 9a058000 task.stack: 9a032000

[ECR   ]: 0x00220100 => Invalid Read @ 0x0004 by insn @ 0x803934e8
[EFA   ]: 0x0004
[BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230
[ERET  ]: drm_atomic_helper_connector_dpms+0xa4/0x230
[STAT32]: 0x0846 : K DE   E2 E1
BTA: 0x8016d949  SP: 0x9a033e34  FP: 0x
LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x
r00: 0x8063c118 r01: 0x805b98ac r02: 0x0b11
r03: 0x r04: 0x9a010f54 r05: 0x
r06: 0x0001 r07: 0x r08: 0x0028
r09: 0x0001 r10: 0x0007 r11: 0x0054
r12: 0x720a3033

Stack Trace:
  drm_atomic_helper_connector_dpms+0xa4/0x230
  arcpgu_drm_hdmi_init+0xbc/0x228
  arcpgu_probe+0x168/0x244
  platform_drv_probe+0x26/0x64
  really_probe+0x1f0/0x32c
  __driver_attach+0xa8/0xd0
  bus_for_each_dev+0x3c/0x74
  bus_add_driver+0xc2/0x184
  driver_register+0x50/0xec
  do_one_initcall+0x3a/0x120
  kernel_init_freeable+0x108/0x1a0
->8

Fix ARC PGU driver to be able work with drm bridge hdmi encoder
interface. The hdmi connector code isn't needed anymore as we expect
the adv7511 bridge driver to create/manage the connector.

Signed-off-by: Eugeniy Paltsev 
---
 drivers/gpu/drm/arc/arcpgu_hdmi.c | 144 +-
 1 file changed, 35 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c 
b/drivers/gpu/drm/arc/arcpgu_hdmi.c
index b7a8b2a..48a6c63 100644
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
@@ -14,104 +14,54 @@
  *
  */

+#include 
 #include 
 #include 
 #include 

 #include "arcpgu.h"

-struct arcpgu_drm_connector {
-   struct drm_connector connector;
-   struct drm_encoder_slave *encoder_slave;
-};
-
-static int arcpgu_drm_connector_get_modes(struct drm_connector *connector)
+static void arcpgu_drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
+   struct drm_display_mode *mode,
+   struct drm_display_mode *adjusted_mode)
 {
-   const struct drm_encoder_slave_funcs *sfuncs;
-   struct drm_encoder_slave *slave;
-   struct arcpgu_drm_connector *con =
-   container_of(connector, struct arcpgu_drm_connector, connector);
-
-   slave = con->encoder_slave;
-   if (slave == NULL) {
-   dev_err(connector->dev->dev,
-   "connector_get_modes: cannot find slave encoder for 
connector\n");
-   return 0;
-   }
-
-   sfuncs = slave->slave_funcs;
-   if (sfuncs->get_modes == NULL)
-   return 0;
-
-   return sfuncs->get_modes(>base, connector);
-}
-
-static enum drm_connector_status
-arcpgu_drm_connector_detect(struct drm_connector *connector, bool force)
-{
-   enum drm_connector_status status = connector_status_unknown;
-   const struct drm_encoder_slave_funcs *sfuncs;
-   struct drm_encoder_slave *slave;
-
-   struct arcpgu_drm_connector *con =
-   container_of(connector, struct arcpgu_drm_connector, connector);
-
-   slave = con->encoder_slave;
-   if (slave == NULL) {
-   dev_err(connector->dev->dev,
-   "connector_detect: cannot find slave encoder for 
connector\n");
-   return status;
-   }
+   struct drm_bridge *bridge = encoder->bridge;

-   sfuncs = slave->slave_funcs;
-   if (sfuncs && sfuncs->detect)
-   return sfuncs->detect(>base, connector);
-
-   dev_err(connector->dev->dev, "connector_detect: could not detect slave 
funcs\n");
-   return status;
+   bridge->funcs->mode_set(bridge, mode, adjusted_mode);
 }

-static void arcpgu_drm_connector_destroy(struct drm_connector *connector)
-{
-   drm_connector_unregister(connector);
-   drm_connector_cleanup(connector);
-}
-
-static const struct drm_connector_helper_funcs
-arcpgu_drm_connector_helper_funcs = {
-   .get_modes = arcpgu_drm_connector_get_modes,
-};
-
-static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
-   .dpms = drm_helper_connector_dpms,
-   .reset = drm_atomic_helper_connector_reset,
-   .detect = arcpgu_drm_connector_detect,
-   .fill_modes = 

[RFC PATCH 00/11] Introduce writeback connectors

[Archit Taneja]

Hi Brian,

On 10/11/2016 08:23 PM, Brian Starkey wrote:
> 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.
>
> 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.
>
> See Also:
> -
> [1] https://lists.freedesktop.org/archives/dri-devel/2016-July/113197.html
> [2] https://lists.freedesktop.org/archives/dri-devel/2016-October/120486.html
>
> I welcome any comments, especially if this approach does/doesn't fit
> well with anyone else's hardware.

Thanks for working on this! Some points below.

- Writeback hardware generally allows us to specify the region within
the framebuffer we want to write to. It's analogous to the SRC_X/Y/W/H
plane properties. We could have similar props for the writeback
connectors, and maybe set them to the FB_ID dimensions if they aren't
configured by userspace.

- Besides the above property, writeback hardware can have provisions
for scaling, color space conversion and rotation. This would mean that
we'd eventually add more writeback specific props/params in
drm_connector/drm_connector_state. Would we be okay adding more such
props for connectors?

Thanks,
Archit

>
> Thanks,
>
> -Brian
>
> ---
>
> Brian Starkey (10):
>   drm: add writeback connector type
>   drm/fb-helper: skip writeback connectors
>   drm: extract CRTC/plane disable from drm_framebuffer_remove
>   drm: add __drm_framebuffer_remove_atomic
>   drm: add fb to connector state
>   drm: expose fb_id property for writeback connectors
>   drm: add writeback-connector pixel format properties
>   drm: mali-dp: rename malidp_input_format
>   drm: mali-dp: add RGB writeback formats for DP550/DP650
>   drm: mali-dp: add writeback connector
>
> Liviu Dudau (1):
>   drm: mali-dp: Add support for writeback on DP550/DP650
>
>  drivers/gpu/drm/arm/Makefile|1 +
>  drivers/gpu/drm/arm/malidp_crtc.c   |   10 ++
>  drivers/gpu/drm/arm/malidp_drv.c|   25 +++-
>  drivers/gpu/drm/arm/malidp_drv.h|5 +
>  drivers/gpu/drm/arm/malidp_hw.c |  104 ++
>  drivers/gpu/drm/arm/malidp_hw.h |   27 +++-
>  drivers/gpu/drm/arm/malidp_mw.c |  268 
> +++
>  drivers/gpu/drm/arm/malidp_planes.c |8 +-
>  drivers/gpu/drm/arm/malidp_regs.h   |   15 ++
>  drivers/gpu/drm/drm_atomic.c|   40 ++
>  drivers/gpu/drm/drm_atomic_helper.c |4 +
>  drivers/gpu/drm/drm_connector.c |   79 ++-
>  drivers/gpu/drm/drm_crtc.c  |   14 +-
>  

[PATCH] drivers/gpu/vga: allocate vga_arb_write() buffer on stack

[Ville Syrjälä]

On Fri, Oct 14, 2016 at 02:54:59PM +0200, Dmitry Vyukov wrote:
> Size of kmalloc() in vga_arb_write() is controlled by user.
> Too large kmalloc() size triggers WARNING message on console.
> Allocate the buffer on stack to avoid the WARNING.
> The string must be small (e.g "target PCI:domain:bus:dev.fn").
> 
> Signed-off-by: Dmitry Vyukov 
> Cc: Dave Airlie 
> Cc: Ville Syrjälä 
> Cc: dri-devel at lists.freedesktop.org
> Cc: syzkaller at googlegroups.com
> 
> --
> 
> I've sent a patch that adds __GFP_NOWARN to the kmalloc a while ago:
> https://groups.google.com/forum/#!msg/syzkaller/navdAwe4iCo/mZDhODsnAAAJ
> 
> Ville suggested to allocate the buffer on stack as it must be small:
> 
> "From the looks of things the longest command could be the
> "target PCI:domain:bus:dev.fn" thing. Even assuming something silly like
> having 10 characters for each domain,bus,dev,fn that would still be only
> 55 bytes. So based on that even something like 64 bytes should be more
> than enough AFAICS. "

> 
> Example WARNING:
> 
> WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999
> __alloc_pages_nodemask+0x7d2/0x1760()
> Modules linked in:
> CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB  4.5.0-rc1+ #283
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>   880069eff670 8299a06d 
>  8800658a4740 864985a0 880069eff6b0 8134fcf9
>  8166de32 864985a0 0bb7 024040c0
> Call Trace:
>  [< inline >] __dump_stack lib/dump_stack.c:15
>  [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
>  [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482
>  [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515
>  [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999
>  [] __alloc_pages_nodemask+0x7d2/0x1760 mm/page_alloc.c:3253
>  [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090
>  [< inline >] alloc_pages include/linux/gfp.h:459
>  [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433
>  [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008
>  [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019
>  [< inline >] kmalloc_large include/linux/slab.h:395
>  [] __kmalloc+0x2f4/0x340 mm/slub.c:3557
>  [< inline >] kmalloc include/linux/slab.h:468
>  [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926
>  [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719
>  [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849
>  [] vfs_writev+0x86/0xc0 fs/read_write.c:886
>  [< inline >] SYSC_writev fs/read_write.c:919
>  [] SyS_writev+0x111/0x2b0 fs/read_write.c:911
>  [] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
> ---
>  drivers/gpu/vga/vgaarb.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index 1887f19..0c73083 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -1022,17 +1022,14 @@ static ssize_t vga_arb_write(struct file *file, const 
> char __user *buf,
>  
>   unsigned int io_state;
>  
> - char *kbuf, *curr_pos;
> + char kbuf[64], *curr_pos;
>   size_t remaining = count;
>  
>   int ret_val;
>   int i;
>  
> -
> - kbuf = kmalloc(count + 1, GFP_KERNEL);
> - if (!kbuf)
> - return -ENOMEM;
> -
> + if (count >= sizeof(kbuf))
> + return -EINVAL;
>   if (copy_from_user(kbuf, buf, count)) {
>   kfree(kbuf);

You missed one kfree() here.

Assuming my analysis of the max string length wasn't full of crap
I think this looks fine otherwise.

So with the lingering kfree() nuked this is
Reviewed-by: Ville Syrjälä 

>   return -EFAULT;
> @@ -1259,11 +1256,9 @@ static ssize_t vga_arb_write(struct file *file, const 
> char __user *buf,
>   goto done;
>   }
>   /* If we got here, the message written is not part of the protocol! */
> - kfree(kbuf);
>   return -EPROTO;
>  
>  done:
> - kfree(kbuf);
>   return ret_val;
>  }
>  
> -- 
> 2.8.0.rc3.226.g39d4020

-- 
Ville Syrjälä
Intel OTC

[RFC PATCH 00/11] Introduce writeback connectors

[Ville Syrjälä]

On Fri, Oct 14, 2016 at 01:39:15PM +0100, Brian Starkey wrote:
> Hi Archit,
> 
> On Fri, Oct 14, 2016 at 04:20:14PM +0530, Archit Taneja wrote:
> >Hi Brian,
> >
> >On 10/11/2016 08:23 PM, Brian Starkey wrote:
> >>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.
> >>
> >>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.
> >>
> >>See Also:
> >>-
> >>[1] https://lists.freedesktop.org/archives/dri-devel/2016-July/113197.html
> >>[2] 
> >>https://lists.freedesktop.org/archives/dri-devel/2016-October/120486.html
> >>
> >>I welcome any comments, especially if this approach does/doesn't fit
> >>well with anyone else's hardware.
> >
> >Thanks for working on this! Some points below.
> >
> >- Writeback hardware generally allows us to specify the region within
> >the framebuffer we want to write to. It's analogous to the SRC_X/Y/W/H
> >plane properties. We could have similar props for the writeback
> >connectors, and maybe set them to the FB_ID dimensions if they aren't
> >configured by userspace.
> >
> >- Besides the above property, writeback hardware can have provisions
> >for scaling, color space conversion and rotation. This would mean that
> >we'd eventually add more writeback specific props/params in
> >drm_connector/drm_connector_state. Would we be okay adding more such
> >props for connectors?
> 
> I've wondered the same thing about bloating non-writeback connectors
> with writeback-specific stuff. If it does become significant, maybe
> we should subclass drm_connector and add a drm_writeback_state pointer
> to drm_connector_state.
> 
> Ville touched on scaling support previously, suggesting adding a
> fixed_mode property (for all types of connectors) - on writeback this
> would represent scaling the framebuffer, and on normal connectors it
> could control output scaling (like panel-fitting).

We got some patches [1] posted for i915 recently that added a bunch of new
properties to control post-blending scaling, but I'm not sure I like the
approach since it seems harder to reconcile with the current way we deal
with scaling for eDP/LVDS/DSI/etc. So I'm still somewhat partial to the
fixed mode idea. Just FYI.

[1] https://lists.freedesktop.org/archives/intel-gfx/2016-August/105557.html

> 
> Certainly destination coords, color-space converstion etc. are things
> that are worth adding, but IMO 

[PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Ville Syrjälä]

On Thu, Oct 13, 2016 at 12:48:46PM -0400, Rob Clark wrote:
> If the bottom-most layer is not fullscreen, we need to use the BASE
> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
> blend_setup() code pretty much handled this already, we just had to
> figure this out in _atomic_check() and assign the stages appropriately.
> 
> Signed-off-by: Rob Clark 
> ---
> TODO mdp4 might need similar treatment?
> 
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44 
> 
>  1 file changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c 
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> index fa2be7c..e42f62d 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>   plane_cnt++;
>   }
>  
> - /*
> - * If there is no base layer, enable border color.
> - * Although it's not possbile in current blend logic,
> - * put it here as a reminder.
> - */
>   if (!pstates[STAGE_BASE] && plane_cnt) {
>   ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>   DBG("Border Color is enabled");
> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>   return pa->state->zpos - pb->state->zpos;
>  }
>  
> +/* is there a helper for this? */
> +static bool is_fullscreen(struct drm_crtc_state *cstate,
> + struct drm_plane_state *pstate)
> +{
> + return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
> + (pstate->crtc_w == cstate->mode.hdisplay) &&
> + (pstate->crtc_h == cstate->mode.vdisplay);
> +}

And what if the plane is larger than the screen size while covering it
fully?

> +
>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>   struct drm_crtc_state *state)
>  {
> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>   struct plane_state pstates[STAGE_MAX + 1];
>   const struct mdp5_cfg_hw *hw_cfg;
>   const struct drm_plane_state *pstate;
> - int cnt = 0, i;
> + int cnt = 0, base = 0, i;
>  
>   DBG("%s: check", mdp5_crtc->name);
>  
> - /* verify that there are not too many planes attached to crtc
> -  * and that we don't have conflicting mixer stages:
> -  */
> - hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>   drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
> - if (cnt >= (hw_cfg->lm.nb_stages)) {
> - dev_err(dev->dev, "too many planes!\n");
> - return -EINVAL;
> - }
> -
> -
>   pstates[cnt].plane = plane;
>   pstates[cnt].state = to_mdp5_plane_state(pstate);
>  
> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>   /* assign a stage based on sorted zpos property */
>   sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>  
> + /* if the bottom-most layer is not fullscreen, we need to use
> +  * it for solid-color:
> +  */
> + if (!is_fullscreen(state, [0].state->base))
> + base++;
> +
> + /* verify that there are not too many planes attached to crtc
> +  * and that we don't have conflicting mixer stages:
> +  */
> + hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
> +
> + if ((cnt + base) >= hw_cfg->lm.nb_stages) {
> + dev_err(dev->dev, "too many planes!\n");
> + return -EINVAL;
> + }
> +
>   for (i = 0; i < cnt; i++) {
> - pstates[i].state->stage = STAGE_BASE + i;
> + pstates[i].state->stage = STAGE_BASE + i + base;
>   DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>   pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>   pstates[i].state->stage);
> -- 
> 2.7.4
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

[PATCH] drivers/gpu/vga: allocate vga_arb_write() buffer on stack

[Dmitry Vyukov]

On Fri, Oct 14, 2016 at 3:06 PM, Ville Syrjälä
 wrote:
> On Fri, Oct 14, 2016 at 02:54:59PM +0200, Dmitry Vyukov wrote:
>> Size of kmalloc() in vga_arb_write() is controlled by user.
>> Too large kmalloc() size triggers WARNING message on console.
>> Allocate the buffer on stack to avoid the WARNING.
>> The string must be small (e.g "target PCI:domain:bus:dev.fn").
>>
>> Signed-off-by: Dmitry Vyukov 
>> Cc: Dave Airlie 
>> Cc: Ville Syrjälä 
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: syzkaller at googlegroups.com
>>
>> --
>>
>> I've sent a patch that adds __GFP_NOWARN to the kmalloc a while ago:
>> https://groups.google.com/forum/#!msg/syzkaller/navdAwe4iCo/mZDhODsnAAAJ
>>
>> Ville suggested to allocate the buffer on stack as it must be small:
>>
>> "From the looks of things the longest command could be the
>> "target PCI:domain:bus:dev.fn" thing. Even assuming something silly like
>> having 10 characters for each domain,bus,dev,fn that would still be only
>> 55 bytes. So based on that even something like 64 bytes should be more
>> than enough AFAICS. "
>
>>
>> Example WARNING:
>>
>> WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999
>> __alloc_pages_nodemask+0x7d2/0x1760()
>> Modules linked in:
>> CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB  4.5.0-rc1+ #283
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>   880069eff670 8299a06d 
>>  8800658a4740 864985a0 880069eff6b0 8134fcf9
>>  8166de32 864985a0 0bb7 024040c0
>> Call Trace:
>>  [< inline >] __dump_stack lib/dump_stack.c:15
>>  [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
>>  [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482
>>  [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515
>>  [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999
>>  [] __alloc_pages_nodemask+0x7d2/0x1760 
>> mm/page_alloc.c:3253
>>  [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090
>>  [< inline >] alloc_pages include/linux/gfp.h:459
>>  [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433
>>  [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008
>>  [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019
>>  [< inline >] kmalloc_large include/linux/slab.h:395
>>  [] __kmalloc+0x2f4/0x340 mm/slub.c:3557
>>  [< inline >] kmalloc include/linux/slab.h:468
>>  [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926
>>  [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719
>>  [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849
>>  [] vfs_writev+0x86/0xc0 fs/read_write.c:886
>>  [< inline >] SYSC_writev fs/read_write.c:919
>>  [] SyS_writev+0x111/0x2b0 fs/read_write.c:911
>>  [] entry_SYSCALL_64_fastpath+0x16/0x7a
>> arch/x86/entry/entry_64.S:185
>> ---
>>  drivers/gpu/vga/vgaarb.c | 11 +++
>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
>> index 1887f19..0c73083 100644
>> --- a/drivers/gpu/vga/vgaarb.c
>> +++ b/drivers/gpu/vga/vgaarb.c
>> @@ -1022,17 +1022,14 @@ static ssize_t vga_arb_write(struct file *file, 
>> const char __user *buf,
>>
>>   unsigned int io_state;
>>
>> - char *kbuf, *curr_pos;
>> + char kbuf[64], *curr_pos;
>>   size_t remaining = count;
>>
>>   int ret_val;
>>   int i;
>>
>> -
>> - kbuf = kmalloc(count + 1, GFP_KERNEL);
>> - if (!kbuf)
>> - return -ENOMEM;
>> -
>> + if (count >= sizeof(kbuf))
>> + return -EINVAL;
>>   if (copy_from_user(kbuf, buf, count)) {
>>   kfree(kbuf);
>
> You missed one kfree() here.

Ouch!

> Assuming my analysis of the max string length wasn't full of crap
> I think this looks fine otherwise.
>
> So with the lingering kfree() nuked this is
> Reviewed-by: Ville Syrjälä 

Mailed v2.
Thanks!


>>   return -EFAULT;
>> @@ -1259,11 +1256,9 @@ static ssize_t vga_arb_write(struct file *file, const 
>> char __user *buf,
>>   goto done;
>>   }
>>   /* If we got here, the message written is not part of the protocol! */
>> - kfree(kbuf);
>>   return -EPROTO;
>>
>>  done:
>> - kfree(kbuf);
>>   return ret_val;
>>  }
>>
>> --
>> 2.8.0.rc3.226.g39d4020
>
> --
> Ville Syrjälä
> Intel OTC

[PATCH v2] drivers/gpu/vga: allocate vga_arb_write() buffer on stack

[Dmitry Vyukov]

Size of kmalloc() in vga_arb_write() is controlled by user.
Too large kmalloc() size triggers WARNING message on console.
Allocate the buffer on stack to avoid the WARNING.
The string must be small (e.g "target PCI:domain:bus:dev.fn").

Signed-off-by: Dmitry Vyukov 
Reviewed-by: Ville Syrjälä 
Cc: Dave Airlie 
Cc: Ville Syrjälä 
Cc: dri-devel at lists.freedesktop.org
Cc: syzkaller at googlegroups.com

---

Changes since v1:
 - removed another kfree(kbuf)

I've sent a patch that adds __GFP_NOWARN to the kmalloc a while ago:
https://groups.google.com/forum/#!msg/syzkaller/navdAwe4iCo/mZDhODsnAAAJ

Ville suggested to allocate the buffer on stack as it must be small:

"From the looks of things the longest command could be the
"target PCI:domain:bus:dev.fn" thing. Even assuming something silly like
having 10 characters for each domain,bus,dev,fn that would still be only
55 bytes. So based on that even something like 64 bytes should be more
than enough AFAICS. "

Example WARNING:

WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999
__alloc_pages_nodemask+0x7d2/0x1760()
Modules linked in:
CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB  4.5.0-rc1+ #283
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
  880069eff670 8299a06d 
 8800658a4740 864985a0 880069eff6b0 8134fcf9
 8166de32 864985a0 0bb7 024040c0
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
 [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482
 [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515
 [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999
 [] __alloc_pages_nodemask+0x7d2/0x1760 mm/page_alloc.c:3253
 [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090
 [< inline >] alloc_pages include/linux/gfp.h:459
 [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433
 [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008
 [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019
 [< inline >] kmalloc_large include/linux/slab.h:395
 [] __kmalloc+0x2f4/0x340 mm/slub.c:3557
 [< inline >] kmalloc include/linux/slab.h:468
 [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926
 [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719
 [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849
 [] vfs_writev+0x86/0xc0 fs/read_write.c:886
 [< inline >] SYSC_writev fs/read_write.c:919
 [] SyS_writev+0x111/0x2b0 fs/read_write.c:911
 [] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
---
 drivers/gpu/vga/vgaarb.c | 15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 1887f19..77657a8 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -1022,21 +1022,16 @@ static ssize_t vga_arb_write(struct file *file, const 
char __user *buf,

unsigned int io_state;

-   char *kbuf, *curr_pos;
+   char kbuf[64], *curr_pos;
size_t remaining = count;

int ret_val;
int i;

-
-   kbuf = kmalloc(count + 1, GFP_KERNEL);
-   if (!kbuf)
-   return -ENOMEM;
-
-   if (copy_from_user(kbuf, buf, count)) {
-   kfree(kbuf);
+   if (count >= sizeof(kbuf))
+   return -EINVAL;
+   if (copy_from_user(kbuf, buf, count))
return -EFAULT;
-   }
curr_pos = kbuf;
kbuf[count] = '\0'; /* Just to make sure... */

@@ -1259,11 +1254,9 @@ static ssize_t vga_arb_write(struct file *file, const 
char __user *buf,
goto done;
}
/* If we got here, the message written is not part of the protocol! */
-   kfree(kbuf);
return -EPROTO;

 done:
-   kfree(kbuf);
return ret_val;
 }

-- 
2.8.0.rc3.226.g39d4020

[PATCH] drivers/gpu/vga: allocate vga_arb_write() buffer on stack

[Dmitry Vyukov]

Size of kmalloc() in vga_arb_write() is controlled by user.
Too large kmalloc() size triggers WARNING message on console.
Allocate the buffer on stack to avoid the WARNING.
The string must be small (e.g "target PCI:domain:bus:dev.fn").

Signed-off-by: Dmitry Vyukov 
Cc: Dave Airlie 
Cc: Ville Syrjälä 
Cc: dri-devel at lists.freedesktop.org
Cc: syzkaller at googlegroups.com

--

I've sent a patch that adds __GFP_NOWARN to the kmalloc a while ago:
https://groups.google.com/forum/#!msg/syzkaller/navdAwe4iCo/mZDhODsnAAAJ

Ville suggested to allocate the buffer on stack as it must be small:

"From the looks of things the longest command could be the
"target PCI:domain:bus:dev.fn" thing. Even assuming something silly like
having 10 characters for each domain,bus,dev,fn that would still be only
55 bytes. So based on that even something like 64 bytes should be more
than enough AFAICS. "

Example WARNING:

WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999
__alloc_pages_nodemask+0x7d2/0x1760()
Modules linked in:
CPU: 2 PID: 29322 Comm: syz-executor Tainted: GB  4.5.0-rc1+ #283
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
  880069eff670 8299a06d 
 8800658a4740 864985a0 880069eff6b0 8134fcf9
 8166de32 864985a0 0bb7 024040c0
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
 [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482
 [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515
 [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999
 [] __alloc_pages_nodemask+0x7d2/0x1760 mm/page_alloc.c:3253
 [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090
 [< inline >] alloc_pages include/linux/gfp.h:459
 [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433
 [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008
 [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019
 [< inline >] kmalloc_large include/linux/slab.h:395
 [] __kmalloc+0x2f4/0x340 mm/slub.c:3557
 [< inline >] kmalloc include/linux/slab.h:468
 [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926
 [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719
 [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849
 [] vfs_writev+0x86/0xc0 fs/read_write.c:886
 [< inline >] SYSC_writev fs/read_write.c:919
 [] SyS_writev+0x111/0x2b0 fs/read_write.c:911
 [] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
---
 drivers/gpu/vga/vgaarb.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 1887f19..0c73083 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -1022,17 +1022,14 @@ static ssize_t vga_arb_write(struct file *file, const 
char __user *buf,

unsigned int io_state;

-   char *kbuf, *curr_pos;
+   char kbuf[64], *curr_pos;
size_t remaining = count;

int ret_val;
int i;

-
-   kbuf = kmalloc(count + 1, GFP_KERNEL);
-   if (!kbuf)
-   return -ENOMEM;
-
+   if (count >= sizeof(kbuf))
+   return -EINVAL;
if (copy_from_user(kbuf, buf, count)) {
kfree(kbuf);
return -EFAULT;
@@ -1259,11 +1256,9 @@ static ssize_t vga_arb_write(struct file *file, const 
char __user *buf,
goto done;
}
/* If we got here, the message written is not part of the protocol! */
-   kfree(kbuf);
return -EPROTO;

 done:
-   kfree(kbuf);
return ret_val;
 }

-- 
2.8.0.rc3.226.g39d4020

[Bug 98016] [bisected] Fury fails to boot on drm-next-4.9

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

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

--- Comment #11 from Alex Deucher  ---
Odd.  We haven't seen any failures internally with Fiji and 3 VCE rings
enabled.  Can you provide a log of the failure?  Try manually loading amdgpu
after boot.  E.g., append modprobe.blacklist=amdgpu to the kernel command line
and boot to a non-X runlevel, then manually modprobe amdgpu and capture the
dmesg output. Please attach the dmesg output from a successful boot as well.  
If you have remote access via ssh, that would make it easier.  What VCE
firmware are you using?  Can you try the latest version from git and see if
that helps:
https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git

-- 
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/20161014/65669c70/attachment.html>

[Bug 98016] [bisected] Fury fails to boot on drm-next-4.9

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

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

--- Comment #10 from Ernst Sjöstrand  ---
Created attachment 127299
  --> https://bugs.freedesktop.org/attachment.cgi?id=127299=edit
Patch that fixes the issue.

-- 
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/20161014/f43ab8f8/attachment.html>

[PATCH] drm/edid: Only print the bad edid when aborting

[Ville Syrjälä]

On Thu, Oct 13, 2016 at 08:43:55PM +0100, Chris Wilson wrote:
> Currently, if drm.debug is enabled, we get a DRM_ERROR message on the
> intermediate edid reads. This causes transient failures in CI which
> flags up the sporadic EDID read failures, which are recovered by
> rereading the EDID automatically. This patch combines the reporting done
> by drm_do_get_edid() itself with the bad block printing from
> get_edid_block(), into a single warning associated with the connector
> once all attempts to retrieve the EDID fail.

One question is why have been getting more of these corrupt EDID reads
recently. Due to your gmbus change, or the live status revert perhaps?

> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=98228
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/drm_edid.c | 82 
> +-
>  1 file changed, 44 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index ec77bd3e1f08..51dd10c65b53 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1260,6 +1260,26 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned 
> int block, size_t len)
>   return ret == xfers ? 0 : -1;
>  }
>  
> +static void connector_add_bad_edid(struct drm_connector *connector,
> +u8 *block, int num)
> +{
> + if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS))
> + return;
> +
> + if (drm_edid_is_zero(block, EDID_LENGTH)) {
> + dev_warn(connector->dev->dev,
> +  "%s: EDID block %d is all zeroes.\n",
> +  connector->name, num);
> + } else {
> + dev_warn(connector->dev->dev,
> +  "%s: EDID block %d invalid:\n",
> +  connector->name, num);
> + print_hex_dump(KERN_WARNING,
> +" \t", DUMP_PREFIX_NONE, 16, 1,
> +block, EDID_LENGTH, false);
> + }
> +}
> +
>  /**
>   * drm_do_get_edid - get EDID data using a custom EDID block read function
>   * @connector: connector we're probing
> @@ -1282,20 +1302,19 @@ struct edid *drm_do_get_edid(struct drm_connector 
> *connector,
>   void *data)
>  {
>   int i, j = 0, valid_extensions = 0;
> - u8 *block, *new;
> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & 
> DRM_UT_KMS);
> + u8 *edid, *new;
>  
> - if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
> + if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
>   return NULL;
>  
>   /* base block fetch */
>   for (i = 0; i < 4; i++) {
> - if (get_edid_block(data, block, 0, EDID_LENGTH))
> + if (get_edid_block(data, edid, 0, EDID_LENGTH))
>   goto out;
> - if (drm_edid_block_valid(block, 0, print_bad_edid,
> + if (drm_edid_block_valid(edid, 0, false,
>>edid_corrupt))
>   break;
> - if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
> + if (i == 0 && drm_edid_is_zero(edid, EDID_LENGTH)) {
>   connector->null_edid_counter++;
>   goto carp;
>   }
> @@ -1304,58 +1323,45 @@ struct edid *drm_do_get_edid(struct drm_connector 
> *connector,
>   goto carp;
>  
>   /* if there's no extensions, we're done */
> - if (block[0x7e] == 0)
> - return (struct edid *)block;
> + if (edid[0x7e] == 0)
> + return (struct edid *)edid;
>  
> - new = krealloc(block, (block[0x7e] + 1) * EDID_LENGTH, GFP_KERNEL);
> + new = krealloc(edid, (edid[0x7e] + 1) * EDID_LENGTH, GFP_KERNEL);
>   if (!new)
>   goto out;
> - block = new;
> + edid = new;
> +
> + for (j = 1; j <= edid[0x7e]; j++) {
> + u8 *block = edid + (valid_extensions + 1) * EDID_LENGTH;
>  
> - for (j = 1; j <= block[0x7e]; j++) {
>   for (i = 0; i < 4; i++) {
> - if (get_edid_block(data,
> -   block + (valid_extensions + 1) * EDID_LENGTH,
> -   j, EDID_LENGTH))
> + if (get_edid_block(data, block, j, EDID_LENGTH))
>   goto out;
> - if (drm_edid_block_valid(block + (valid_extensions + 1)
> -  * EDID_LENGTH, j,
> -  print_bad_edid,
> -  NULL)) {
> + if (drm_edid_block_valid(block, j, false, NULL)) {
>   valid_extensions++;
>   break;
>   }
>   }
>  
> - if (i == 4 && print_bad_edid) {
> - dev_warn(connector->dev->dev,
> -   

[RFC PATCH 00/11] Introduce writeback connectors

[Brian Starkey]

Hi Archit,

On Fri, Oct 14, 2016 at 04:20:14PM +0530, Archit Taneja wrote:
>Hi Brian,
>
>On 10/11/2016 08:23 PM, Brian Starkey wrote:
>>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.
>>
>>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.
>>
>>See Also:
>>-
>>[1] https://lists.freedesktop.org/archives/dri-devel/2016-July/113197.html
>>[2] https://lists.freedesktop.org/archives/dri-devel/2016-October/120486.html
>>
>>I welcome any comments, especially if this approach does/doesn't fit
>>well with anyone else's hardware.
>
>Thanks for working on this! Some points below.
>
>- Writeback hardware generally allows us to specify the region within
>the framebuffer we want to write to. It's analogous to the SRC_X/Y/W/H
>plane properties. We could have similar props for the writeback
>connectors, and maybe set them to the FB_ID dimensions if they aren't
>configured by userspace.
>
>- Besides the above property, writeback hardware can have provisions
>for scaling, color space conversion and rotation. This would mean that
>we'd eventually add more writeback specific props/params in
>drm_connector/drm_connector_state. Would we be okay adding more such
>props for connectors?

I've wondered the same thing about bloating non-writeback connectors
with writeback-specific stuff. If it does become significant, maybe
we should subclass drm_connector and add a drm_writeback_state pointer
to drm_connector_state.

Ville touched on scaling support previously, suggesting adding a
fixed_mode property (for all types of connectors) - on writeback this
would represent scaling the framebuffer, and on normal connectors it
could control output scaling (like panel-fitting).

Certainly destination coords, color-space converstion etc. are things
that are worth adding, but IMO I'd rather keep this initial
implementation small so we can enable the basic case right away. For
the most part, the additional things are "just properties" which
should be easily added later without impacting the overall interface.

Cheers,
Brian
>
>Thanks,
>Archit
>
>>
>>Thanks,
>>
>>-Brian
>>
>>---
>>
>>Brian Starkey (10):
>>  drm: add writeback connector type
>>  drm/fb-helper: skip writeback connectors
>>  drm: extract CRTC/plane disable from drm_framebuffer_remove
>>  drm: add __drm_framebuffer_remove_atomic
>>  drm: add fb to connector state
>>  drm: expose fb_id property for writeback connectors
>>  drm: add writeback-connector pixel format 

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

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Zhu, Rex
> Sent: Friday, October 14, 2016 8:12 AM
> To: Alex Deucher
> Cc: Maling list - DRI developers; amd-gfx list
> Subject: RE: [PATCH] drm/amd/powerplay: don't give up if DPM is already
> running
> 
>  Hi Alex,
> 
> Your new attached patch is Tested-by and Reviewed-by: Rex Zhu
> 
> 
> Just one question,
> 
> Do we need to set clock_gate for smu?

At the moment, no, but if we ever have have clockgating for smu related IPs, we 
might as well include it.  It matches the current behavior with respect to IP 
tear down.

Alex

> 
> Best Regards
> Rex
> 
> 
> -Original Message-
> From: Alex Deucher [mailto:alexdeucher at gmail.com]
> Sent: Thursday, October 13, 2016 11:29 PM
> To: Zhu, Rex
> Cc: Grazvydas Ignotas; amd-gfx list; Maling list - DRI developers
> Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already
> running
> 
>  On Thu, Oct 13, 2016 at 3:45 AM, Zhu, Rex  wrote:
> > Hi all,
> >
> > The attached patches were also for this issue.
> > Disable dpm when rmmod amdgpu.
> >
> > Please help to review.
> 
> Patch 1:
> +r = adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->hw_fini((void
> *)adev);
> +if (r)
> +DRM_DEBUG("hw_fini of IP block <%s> failed %d\n",
> +adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->name, r);
> +
> +adev->ip_block_status[AMD_IP_BLOCK_TYPE_SMC].hw = false;
> 
> You can't use AMD_IP_BLOCK_TYPE_* as index.  Some chips may not have
> the IP block.  Maybe something like the attached patch.  That said, I think
> longer term it makes more sense to allows the SOCs to specify the order for
> init, fini, etc. otherwise we'll have lots of variable logic in the common 
> code.
> 
> Patch 2:
> +if (1 == PHM_READ_VFPF_INDIRECT_FIELD(hwmgr->device,
> CGS_IND_REG__SMC, FEATURE_STATUS, AVS_ON)) {
> +PP_ASSERT_WITH_CODE((0 == smum_send_msg_to_smc(hwmgr-
> >smumgr,
> PPSMC_MSG_DisableAvfs)),
> +"Failed to disable AVFS!",
> +return -1;);
> +}
> 
> Use a proper error code there like -EINVAL or something.  With that fixed:
> Reviewed-by: Alex Deucher 
> 
> Alex
> 
> >
> > Best Regards
> > Rex
> >
> > -Original Message-
> > From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> > Of Zhu, Rex
> > Sent: Wednesday, October 12, 2016 9:45 PM
> > To: Alex Deucher; Grazvydas Ignotas
> > Cc: amd-gfx list; Maling list - DRI developers
> > Subject: RE: [PATCH] drm/amd/powerplay: don't give up if DPM is
> > already running
> >
> > 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:alexdeucher at 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
> >> +++ 

[pull] radeon and amdgpu drm-next-4.9

[Alex Deucher]

Hi Dave,

Fixes for radeon and amdgpu for 4.9:
- allow an additional reg in the SI reg checker
- fix thermal sensor readback on CZ/ST
- misc bug fixes

The following changes since commit 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)

are available in the git repository at:

  git://people.freedesktop.org/~agd5f/linux drm-next-4.9

for you to fetch changes up to f28a9b65c9e3697ba8d2ab371fae4fea15638676:

  drm/amd/powerplay: fix bug stop dpm can't work on Vi. (2016-10-14 12:10:00 
-0400)


Alex Deucher (9):
  drm/radeon: fix up dp aux tear down (v2)
  drm/radeon: fix modeset tear down code
  drm/amdgpu/gfx8: fix CGCG_CGLS handling
  drm/amdgpu: clarify UVD/VCE special handling for CG
  drm/radeon: change vblank_time's calculation method to reduce 
computational error.
  drm/amdgpu: fix amdgpu_need_full_reset (v2)
  drm/amdgpu: disable smu hw first on tear down
  drm/amdgpu/powerplay: implement thermal sensor for CZ/ST
  drm/amdgpu/dpm: implement thermal sensor for CZ/ST

Arindam Nath (1):
  drm/amd/amdgpu: enable clockgating only after late init

Dan Carpenter (1):
  drm/amdgpu: potential NULL dereference in debugfs code

Grazvydas Ignotas (1):
  drm/amdgpu: use .early_unregister hook to remove DP AUX i2c

Marek Olšák (1):
  drm/radeon: allow TA_CS_BC_BASE_ADDR on SI

Nicolai Hähnle (1):
  drm/amdgpu: initialize the context reset_counter in amdgpu_ctx_init

Rex Zhu (6):
  drm/amdgpu: change vblank_time's calculation method to reduce 
computational error.
  drm/amd/powerplay: fix static checker warnings in iceland_smc.c
  drm/amd/powerplay: fix static checker warnings in smu7_hwmgr.c
  drm/amd/powerplay: fix static checker warnings in smu7_hwmgr.c
  drm/amd/powerplay: notify smu no display by default.
  drm/amd/powerplay: fix bug stop dpm can't work on Vi.

Tom St Denis (1):
  drm/amdgpu/si_dpm: Limit clocks on HD86xx part

 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 13 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c|  3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 69 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c| 14 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/cz_dpm.c|  8 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 12 +---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 30 ++
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  | 13 ++--
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 14 ++---
 drivers/gpu/drm/amd/amdgpu/si_dpm.c|  6 ++
 drivers/gpu/drm/amd/amdgpu/tonga_ih.c  | 14 ++---
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c  | 14 ++---
 drivers/gpu/drm/amd/amdgpu/vce_v3_0.c  | 15 +++--
 drivers/gpu/drm/amd/include/amd_shared.h   |  2 +-
 .../drm/amd/powerplay/eventmgr/eventactionchains.c |  1 +
 drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 18 ++
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 53 ++---
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c |  2 +-
 drivers/gpu/drm/radeon/r600_dpm.c  | 15 ++---
 drivers/gpu/drm/radeon/radeon_connectors.c | 17 ++
 drivers/gpu/drm/radeon/radeon_display.c| 14 ++---
 drivers/gpu/drm/radeon/radeon_drv.c|  3 +-
 drivers/gpu/drm/radeon/radeon_i2c.c|  3 +-
 drivers/gpu/drm/radeon/si.c|  1 +
 drivers/gpu/drm/radeon/sid.h   |  1 +
 26 files changed, 232 insertions(+), 127 deletions(-)

[PATCH 26/41] drm: Add reference counting to drm_atomic_state

[Chris Wilson]

drm_atomic_state has a complicated single owner model that tracks the
single reference from allocation through to destruction on another
thread - or perhaps on a local error path. We can simplify this tracking
by using reference counting (at a cost of a few more atomics). This is
even more beneficial when the lifetime of the state becomes more
convoluted than being passed to a single worker thread for the commit.

v2: Double check !intel atomic_commit functions for missing gets
v3: Update kerneldocs

Signed-off-by: Chris Wilson 
Cc: Daniel Vetter 
Cc: dri-devel at lists.freedesktop.org
Reviewed-by: Eric Engestrom 
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  3 +-
 drivers/gpu/drm/drm_atomic.c | 25 +++
 drivers/gpu/drm/drm_atomic_helper.c  | 98 +++-
 drivers/gpu/drm/drm_fb_helper.c  |  9 +--
 drivers/gpu/drm/exynos/exynos_drm_drv.c  |  3 +-
 drivers/gpu/drm/i915/i915_debugfs.c  |  5 +-
 drivers/gpu/drm/i915/intel_display.c | 31 +
 drivers/gpu/drm/i915/intel_sprite.c  |  4 +-
 drivers/gpu/drm/mediatek/mtk_drm_drv.c   |  3 +-
 drivers/gpu/drm/msm/msm_atomic.c |  3 +-
 drivers/gpu/drm/omapdrm/omap_drv.c   |  3 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c|  3 +-
 drivers/gpu/drm/sti/sti_drv.c|  3 +-
 drivers/gpu/drm/tegra/drm.c  |  3 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  2 -
 drivers/gpu/drm/vc4/vc4_kms.c|  3 +-
 include/drm/drm_atomic.h | 31 -
 include/drm/drm_plane.h  |  1 -
 18 files changed, 102 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 5f484310bee9..9f6222895212 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -464,7 +464,7 @@ atmel_hlcdc_dc_atomic_complete(struct atmel_hlcdc_dc_commit 
*commit)

drm_atomic_helper_cleanup_planes(dev, old_state);

-   drm_atomic_state_free(old_state);
+   drm_atomic_state_put(old_state);

/* Complete the commit, wake up any waiter. */
spin_lock(>commit.wait.lock);
@@ -521,6 +521,7 @@ static int atmel_hlcdc_dc_atomic_commit(struct drm_device 
*dev,
/* Swap the state, this is the point of no return. */
drm_atomic_helper_swap_state(state, true);

+   drm_atomic_state_get(state);
if (async)
queue_work(dc->wq, >work);
else
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 23739609427d..5dd70540219c 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -74,6 +74,8 @@ EXPORT_SYMBOL(drm_atomic_state_default_release);
 int
 drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
 {
+   kref_init(>ref);
+
/* TODO legacy paths should maybe do a better job about
 * setting this appropriately?
 */
@@ -215,22 +217,16 @@ void drm_atomic_state_clear(struct drm_atomic_state 
*state)
 EXPORT_SYMBOL(drm_atomic_state_clear);

 /**
- * drm_atomic_state_free - free all memory for an atomic state
- * @state: atomic state to deallocate
+ * __drm_atomic_state_free - free all memory for an atomic state
+ * @ref: This atomic state to deallocate
  *
  * This frees all memory associated with an atomic state, including all the
  * per-object state for planes, crtcs and connectors.
  */
-void drm_atomic_state_free(struct drm_atomic_state *state)
+void __drm_atomic_state_free(struct kref *ref)
 {
-   struct drm_device *dev;
-   struct drm_mode_config *config;
-
-   if (!state)
-   return;
-
-   dev = state->dev;
-   config = >mode_config;
+   struct drm_atomic_state *state = container_of(ref, typeof(*state), ref);
+   struct drm_mode_config *config = >dev->mode_config;

drm_atomic_state_clear(state);

@@ -243,7 +239,7 @@ void drm_atomic_state_free(struct drm_atomic_state *state)
kfree(state);
}
 }
-EXPORT_SYMBOL(drm_atomic_state_free);
+EXPORT_SYMBOL(__drm_atomic_state_free);

 /**
  * drm_atomic_get_crtc_state - get crtc state
@@ -1742,7 +1738,7 @@ retry:
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
/*
 * Unlike commit, check_only does not clean up state.
-* Below we call drm_atomic_state_free for it.
+* Below we call drm_atomic_state_put for it.
 */
ret = drm_atomic_check_only(state);
} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
@@ -1775,8 +1771,7 @@ out:
goto retry;
}

-   if (ret || arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
-   drm_atomic_state_free(state);
+   drm_atomic_state_put(state);

drm_modeset_drop_locks();

[Intel-gfx] [PATCH] drm: Print device information again in debugfs

[Jani Nikula]

On Thu, 13 Oct 2016, Daniel Vetter  wrote:
> I was a bit over-eager in my cleanup in
>
> commit 95c081c17f284de50eaca60d4d55643a64d39019
> Author: Daniel Vetter 
> Date:   Tue Jun 21 10:54:12 2016 +0200
>
> drm: Move master pointer from drm_minor to drm_device
>
> Noticed by Chris Wilson.
>
> Fixes: 95c081c17f28 ("drm: Move master pointer from drm_minor to drm_device")

Cc:  # v4.8+

> Cc: Chris Wilson 
> Cc: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Emil Velikov 
> Cc: Julia Lawall 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_info.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
> index 1df2d33d0b40..ffb2ab389d1d 100644
> --- a/drivers/gpu/drm/drm_info.c
> +++ b/drivers/gpu/drm/drm_info.c
> @@ -54,9 +54,6 @@ int drm_name_info(struct seq_file *m, void *data)
>  
>   mutex_lock(>master_mutex);
>   master = dev->master;
> - if (!master)
> - goto out_unlock;
> -
>   seq_printf(m, "%s", dev->driver->name);
>   if (dev->dev)
>   seq_printf(m, " dev=%s", dev_name(dev->dev));
> @@ -65,7 +62,6 @@ int drm_name_info(struct seq_file *m, void *data)
>   if (dev->unique)
>   seq_printf(m, " unique=%s", dev->unique);
>   seq_printf(m, "\n");
> -out_unlock:
>   mutex_unlock(>master_mutex);
>  
>   return 0;

-- 
Jani Nikula, Intel Open Source Technology Center

[RFC] drm: add atomic state printing

[Ville Syrjälä]

On Thu, Oct 13, 2016 at 04:14:04PM -0400, Rob Clark wrote:
> On Thu, Oct 13, 2016 at 2:24 PM, Ville Syrjälä
>  wrote:
> > On Thu, Oct 13, 2016 at 01:16:29PM -0400, Rob Clark wrote:
> >> Sometimes we just want to see the atomic state, without getting so many
> >> other debug traces.  So add a new drm.debug bit for that.
> >>
> >> Note: it would be nice to put the helpers for printing plane/crtc state
> >> next to plane/crtc state structs.. so someone adding new stuff to the
> >> state structs is more likely to remember to update the corresponding
> >> print_state() fxn.  But the header include order for that doesn't really
> >> work out.
> >>
> >> Also, just including the corresponding mdp bits as an example.  Dumps
> >> out something like:
> >>
> >> [drm] plane[24]: RGB0
> >> [drm] crtc=crtc-0
> >> [drm] fb=35
> >> [drm] crtc-pos=[0,0, 720x400]
> >> [drm] src-pos=[0,0, 720x400]
> >> [drm] rotation=0
> >> [drm] premultiplied=0
> >> [drm] zpos=1
> >> [drm] alpha=255
> >> [drm] stage=STAGE0
> >> [drm] mode_changed=1
> >> [drm] pending=0
> >> [drm] crtc[27]: crtc-0
> >> [drm] enable=1
> >> [drm] active=0
> >> [drm] planes_changed=1
> >> [drm] mode_changed=1
> >> [drm] active_changed=0
> >> [drm] connectors_changed=1
> >> [drm] color_mgmt_changed=0
> >> [drm] plane_mask=1
> >> [drm] connector_mask=1
> >> [drm] encoder_mask=1
> >> [drm] mode: 0:"1920x1080" 60 148500 1920 2008 2052 2200 1080 1084 
> >> 1089 1125 0x48 0x5
> >> ---
> >>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c  |  3 ++
> >>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   | 28 ++
> >>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |  4 ++-
> >>  include/drm/drmP.h| 47 
> >> ++-
> >>  4 files changed, 80 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c 
> >> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> >> index e42f62d..da84179 100644
> >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> >> @@ -435,6 +435,9 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc 
> >> *crtc,
> >>
> >>   DBG("%s: event: %p", mdp5_crtc->name, crtc->state->event);
> >>
> >> + DRM_DEBUG_STATE("crtc[%u]: %s\n", crtc->base.id, crtc->name);
> >> + drm_print_crtc_state(crtc->state);
> >> +
> >>   WARN_ON(mdp5_crtc->event);
> >>
> >>   spin_lock_irqsave(>event_lock, flags);
> >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h 
> >> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> >> index e4b3fb3..466acbc 100644
> >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> >> @@ -92,6 +92,22 @@ struct mdp5_plane_state {
> >>  #define to_mdp5_plane_state(x) \
> >>   container_of(x, struct mdp5_plane_state, base)
> >>
> >> +static inline const char *stage2name(enum mdp_mixer_stage_id stage);
> >> +
> >> +static inline void
> >> +mdp5_print_plane_state(const struct mdp5_plane_state *state)
> >> +{
> >> + const struct drm_plane *plane = state->base.plane;
> >> + DRM_DEBUG_STATE("plane[%u]: %s\n", plane->base.id, plane->name);
> >> + drm_print_plane_state(>base);
> >> + DRM_DEBUG_STATE("\tpremultiplied=%u\n", state->premultiplied);
> >> + DRM_DEBUG_STATE("\tzpos=%u\n", state->zpos);
> >> + DRM_DEBUG_STATE("\talpha=%u\n", state->alpha);
> >> + DRM_DEBUG_STATE("\tstage=%s\n", stage2name(state->stage));
> >> + DRM_DEBUG_STATE("\tmode_changed=%u\n", state->mode_changed);
> >> + DRM_DEBUG_STATE("\tpending=%u\n", state->pending);
> >> +}
> >> +
> >>  enum mdp5_intf_mode {
> >>   MDP5_INTF_MODE_NONE = 0,
> >>
> >> @@ -120,6 +136,18 @@ static inline u32 mdp5_read(struct mdp5_kms 
> >> *mdp5_kms, u32 reg)
> >>   return msm_readl(mdp5_kms->mmio + reg);
> >>  }
> >>
> >> +static inline const char *stage2name(enum mdp_mixer_stage_id stage)
> >> +{
> >> + static const char *names[] = {
> >> +#define NAME(n) [n] = #n
> >> + NAME(STAGE_UNUSED), NAME(STAGE_BASE),
> >> + NAME(STAGE0), NAME(STAGE1), NAME(STAGE2),
> >> + NAME(STAGE3), NAME(STAGE4), NAME(STAGE6),
> >> +#undef NAME
> >> + };
> >> + return names[stage];
> >> +}
> >> +
> >>  static inline const char *pipe2name(enum mdp5_pipe pipe)
> >>  {
> >>   static const char *names[] = {
> >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c 
> >> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> >> index 432c098..df301df 100644
> >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> >> @@ -356,6 +356,8 @@ static void mdp5_plane_atomic_update(struct drm_plane 
> >> *plane,
> >>
> >>   DBG("%s: update", mdp5_plane->name);
> >>
> >> + mdp5_print_plane_state(to_mdp5_plane_state(state));
> >> +
> 

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

[Zhu, Rex]

 Hi Alex,

Your new attached patch is Tested-by and Reviewed-by: Rex Zhu 

Just one question,

Do we need to set clock_gate for smu? 

Best Regards
Rex


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

 On Thu, Oct 13, 2016 at 3:45 AM, Zhu, Rex  wrote:
> Hi all,
>
> The attached patches were also for this issue.
> Disable dpm when rmmod amdgpu.
>
> Please help to review.

Patch 1:
+r = adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->hw_fini((void *)adev);
+if (r)
+DRM_DEBUG("hw_fini of IP block <%s> failed %d\n",
+adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->name, r);
+
+adev->ip_block_status[AMD_IP_BLOCK_TYPE_SMC].hw = false;

You can't use AMD_IP_BLOCK_TYPE_* as index.  Some chips may not have the IP 
block.  Maybe something like the attached patch.  That said, I think longer 
term it makes more sense to allows the SOCs to specify the order for init, 
fini, etc. otherwise we'll have lots of variable logic in the common code.

Patch 2:
+if (1 == PHM_READ_VFPF_INDIRECT_FIELD(hwmgr->device,
CGS_IND_REG__SMC, FEATURE_STATUS, AVS_ON)) {
+PP_ASSERT_WITH_CODE((0 == smum_send_msg_to_smc(hwmgr->smumgr,
PPSMC_MSG_DisableAvfs)),
+"Failed to disable AVFS!",
+return -1;);
+}

Use a proper error code there like -EINVAL or something.  With that fixed:
Reviewed-by: Alex Deucher 

Alex

>
> Best Regards
> Rex
>
> -Original Message-
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf 
> Of Zhu, Rex
> Sent: Wednesday, October 12, 2016 9:45 PM
> To: Alex Deucher; Grazvydas Ignotas
> Cc: amd-gfx list; Maling list - DRI developers
> Subject: RE: [PATCH] drm/amd/powerplay: don't give up if DPM is 
> already running
>
> 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:alexdeucher at 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
> 

[PATCH] drm/edid: Only print the bad edid when aborting

[Chris Wilson]

On Fri, Oct 14, 2016 at 01:46:37PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 13, 2016 at 08:43:55PM +0100, Chris Wilson wrote:
> > Currently, if drm.debug is enabled, we get a DRM_ERROR message on the
> > intermediate edid reads. This causes transient failures in CI which
> > flags up the sporadic EDID read failures, which are recovered by
> > rereading the EDID automatically. This patch combines the reporting done
> > by drm_do_get_edid() itself with the bad block printing from
> > get_edid_block(), into a single warning associated with the connector
> > once all attempts to retrieve the EDID fail.
> 
> One question is why have been getting more of these corrupt EDID reads
> recently. Due to your gmbus change, or the live status revert perhaps?

The recent reports are from a new Ironlake setup aiui.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[Bug 98016] [bisected] Fury fails to boot on drm-next-4.9

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

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

--- Comment #9 from Ernst Sjöstrand  ---
I know what may have been fooling me.
I run sudo make install on Ubuntu. It adds a .old entry in grub.
However that .old entry doesn't have a .old initrd, where the amdgpu kernel
module lives, it shares initrd.

-- 
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/20161014/3e61ed33/attachment-0001.html>

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

[Alex Deucher]

On Fri, Oct 14, 2016 at 10:32 AM, Dan Carpenter
 wrote:
> Hello Rex Zhu,
>
> 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 static checker warning:
>
> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c:2125 
> smu7_patch_limits_vddc()
> warn: passing casted pointer '>vddc' to 
> 'smu7_patch_ppt_v0_with_vdd_leakage()' 16 vs 32.
>
> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c
>   2119  static int smu7_patch_limits_vddc(struct pp_hwmgr *hwmgr,
>   2120   struct 
> phm_clock_and_voltage_limits *tab)
>   2121  {
>   2122  struct smu7_hwmgr *data = (struct smu7_hwmgr 
> *)(hwmgr->backend);
>   2123
>   2124  if (tab) {
>   2125  smu7_patch_ppt_v0_with_vdd_leakage(hwmgr, (uint32_t 
> *)>vddc,
>   2126  
> >vddc_leakage);
>
> This call corrupts vddci.
>
>   2127  smu7_patch_ppt_v0_with_vdd_leakage(hwmgr, (uint32_t 
> *)>vddci,
>   2128  
> >vddci_leakage);
>
> But that's fine since we immediately overwrite it here.  But
> unfortunately this call corrupt tab->vddgfx.

Thanks.  Should be fixed in the attached patch.

Alex

>
>   2129  }
>   2130
>   2131  return 0;
>   2132  }
>
> regards,
> dan carpenter
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- next part --
A non-text attachment was scrubbed...
Name: 0001-drm-amdgpu-powerplay-smu7-fix-static-checker-warning.patch
Type: text/x-patch
Size: 1557 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161014/c36f0df1/attachment.bin>

mm: fix cache mode tracking in vm_insert_mixed() breaks AMDGPU [was: Re: Latest testing with drm-next-4.9-wip and latest LLVM/mesa stack - Regression in PowerPlay/DPM on CIK?]

[Michel Dänzer]

[ Adding Dan Williams and dri-devel ]

On 14/10/16 03:28 AM, Shawn Starr wrote:
> Hello AMD folks,
> 
> I have discovered a problem in Linus master that affects AMDGPU, nobody would 
> notice this in drm-next-4.9-wip since its not in this repo.

[...]

> 87744ab3832b83ba71b931f86f9cfdb000d07da5 is the first bad commit
> commit 87744ab3832b83ba71b931f86f9cfdb000d07da5
> Author: Dan Williams 
> Date:   Fri Oct 7 17:00:18 2016 -0700
> 
> mm: fix cache mode tracking in vm_insert_mixed()
> 
> vm_insert_mixed() unlike vm_insert_pfn_prot() and vmf_insert_pfn_pmd(),
> fails to check the pgprot_t it uses for the mapping against the one
> recorded in the memtype tracking tree.  Add the missing call to
> track_pfn_insert() to preclude cases where incompatible aliased mappings
> are established for a given physical address range.
> 
> Link: http://lkml.kernel.org/r/
> 147328717909.35069.14256589123570653697.stgit at dwillia2-
> desk3.amr.corp.intel.com
> Signed-off-by: Dan Williams 
> Cc: David Airlie 
> Cc: Matthew Wilcox 
> Cc: Ross Zwisler 
> Signed-off-by: Andrew Morton 
> Signed-off-by: Linus Torvalds 
> 
> :04 04 7517c0019fe49c1830b5a1d81f1dc099c5aab98a 
> fd497a604a2af5995db2b8ed1e9c640bede6adf3 M  mm
> 
> 
> Removal of this patch stops graphics stalls.

Thanks for bisecting this Shawn.


> A friend of mine mentions,
> 
> "looks like a graphics thingy you depend on is requesting a mapping with a 
> not-allowed cache mode, and now you are (rightfully) getting errors?"

It would be nice to get some more specific pointers what amdgpu (or
maybe ttm, since that calls vm_insert_mixed in ttm_bo_vm_fault) might be
doing wrong.


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

[PATCH v3 1/9] drm/hisilicon/hibmc: Add hisilicon hibmc drm master driver

[Benjamin Gaignard]

[snip]

> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> @@ -0,0 +1,15 @@
> +config DRM_HISI_HIBMC
> +   tristate "DRM Support for Hisilicon Hibmc"
> +   depends on DRM && PCI
> +   select DRM_KMS_HELPER
> +   select DRM_KMS_FB_HELPER
> +   select DRM_GEM_CMA_HELPER
> +   select DRM_KMS_CMA_HELPER

since you use TTM I don't think that selecting DRM_GEM_CMA_HELPER and
DRM_KMS_CMA_HELPER
help you lot here.
You could add configuration flags step by step in following patches
that will make you needs more clear (that also true for #include)

> +   select FB_SYS_FILLRECT
> +   select FB_SYS_COPYAREA
> +   select FB_SYS_IMAGEBLIT
> +   select DRM_TTM
> +
> +   help
> + Choose this option if you have a Hisilicon Hibmc soc chipset.
> + If M is selected the module will be called hibmc-drm.
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile 
> b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> new file mode 100644
> index 000..97cf4a0
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> @@ -0,0 +1,5 @@
> +ccflags-y := -Iinclude/drm
> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_power.o
> +
> +obj-$(CONFIG_DRM_HISI_HIBMC)   +=hibmc-drm.o
> +#obj-y += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c 
> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> new file mode 100644
> index 000..52c9353
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -0,0 +1,288 @@
> +/* Hisilicon Hibmc SoC drm driver
> + *
> + * Based on the bochs drm driver.
> + *
> + * Copyright (c) 2016 Huawei Limited.
> + *
> + * Author:
> + * Rongrong Zou 

".com" is missing in you email address (same typo in all other files)

> + * Rongrong Zou 
> + * Jianhua Li 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

cma_helpers look useless since you use TTM, no ?

> +#include 
> +
> +#include "hibmc_drm_drv.h"
> +#include "hibmc_drm_regs.h"
> +#include "hibmc_drm_power.h"
> +

[Bug 97099] Regression in 9ef8537e6 "drm/radeon: don't use fractional dividers on RS[78]80 if SS is enabled" (RV620)

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

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

Christian König  changed:

   What|Removed |Added

 Status|RESOLVED|CLOSED

-- 
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/20161014/6f916818/attachment.html>

[PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Rob Clark]

On Fri, Oct 14, 2016 at 8:45 AM, Ville Syrjälä
 wrote:
> On Thu, Oct 13, 2016 at 12:48:46PM -0400, Rob Clark wrote:
>> If the bottom-most layer is not fullscreen, we need to use the BASE
>> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
>> blend_setup() code pretty much handled this already, we just had to
>> figure this out in _atomic_check() and assign the stages appropriately.
>>
>> Signed-off-by: Rob Clark 
>> ---
>> TODO mdp4 might need similar treatment?
>>
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44 
>> 
>>  1 file changed, 27 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c 
>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> index fa2be7c..e42f62d 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>>   plane_cnt++;
>>   }
>>
>> - /*
>> - * If there is no base layer, enable border color.
>> - * Although it's not possbile in current blend logic,
>> - * put it here as a reminder.
>> - */
>>   if (!pstates[STAGE_BASE] && plane_cnt) {
>>   ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>>   DBG("Border Color is enabled");
>> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>>   return pa->state->zpos - pb->state->zpos;
>>  }
>>
>> +/* is there a helper for this? */
>> +static bool is_fullscreen(struct drm_crtc_state *cstate,
>> + struct drm_plane_state *pstate)
>> +{
>> + return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
>> + (pstate->crtc_w == cstate->mode.hdisplay) &&
>> + (pstate->crtc_h == cstate->mode.vdisplay);
>> +}
>
> And what if the plane is larger than the screen size while covering it
> fully?

good question.. if things aren't clipped elsewhere we probably end up
programming hw badly..

fwiw, it doesn't seem to hurt anything to turn on the solid-fill layer
but we'd be burning an extra mixer stage.  I guess I can make this
cover that case easily enough, but probably doesn't mean that case
works

BR,
-R

>> +
>>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>>   struct drm_crtc_state *state)
>>  {
>> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc 
>> *crtc,
>>   struct plane_state pstates[STAGE_MAX + 1];
>>   const struct mdp5_cfg_hw *hw_cfg;
>>   const struct drm_plane_state *pstate;
>> - int cnt = 0, i;
>> + int cnt = 0, base = 0, i;
>>
>>   DBG("%s: check", mdp5_crtc->name);
>>
>> - /* verify that there are not too many planes attached to crtc
>> -  * and that we don't have conflicting mixer stages:
>> -  */
>> - hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>   drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
>> - if (cnt >= (hw_cfg->lm.nb_stages)) {
>> - dev_err(dev->dev, "too many planes!\n");
>> - return -EINVAL;
>> - }
>> -
>> -
>>   pstates[cnt].plane = plane;
>>   pstates[cnt].state = to_mdp5_plane_state(pstate);
>>
>> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>>   /* assign a stage based on sorted zpos property */
>>   sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>>
>> + /* if the bottom-most layer is not fullscreen, we need to use
>> +  * it for solid-color:
>> +  */
>> + if (!is_fullscreen(state, [0].state->base))
>> + base++;
>> +
>> + /* verify that there are not too many planes attached to crtc
>> +  * and that we don't have conflicting mixer stages:
>> +  */
>> + hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>> +
>> + if ((cnt + base) >= hw_cfg->lm.nb_stages) {
>> + dev_err(dev->dev, "too many planes!\n");
>> + return -EINVAL;
>> + }
>> +
>>   for (i = 0; i < cnt; i++) {
>> - pstates[i].state->stage = STAGE_BASE + i;
>> + pstates[i].state->stage = STAGE_BASE + i + base;
>>   DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>>   pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>>   pstates[i].state->stage);
>> --
>> 2.7.4
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC

[Bug 96449] Dying Light reports OpenGL version 3.0 with mesa-git

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

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

--- Comment #10 from Jan Ziak <0xe2.0x9a.0x9b at gmail.com> ---
Quote from
[https://www.phoronix.com/forums/forum/phoronix/latest-phoronix-articles/904490-another-fun-day-in-mesa-git-radv-nvc0-radeonsi?p=904568#post904568]:

The first 2 frames contain:

[code]
1063 @0 glMatrixMode(mode = GL_MODELVIEW)
1064 @0 glLoadIdentity()
1065 @0 glViewport(x = 0, y = 0, width = 512, height = 256)
1066 @0 glMatrixMode(mode = GL_PROJECTION)
1067 @0 glLoadIdentity()

1090 @0 glBegin(mode = GL_TRIANGLE_STRIP)
1091 @0 glTexCoord2f(s = 0, t = 0)
1092 @0 glVertex2f(x = 0, y = 0)
1093 @0 glTexCoord2f(s = 1, t = 0)
1094 @0 glVertex2f(x = 512, y = 0)
1095 @0 glTexCoord2f(s = 0, t = 1)
1096 @0 glVertex2f(x = 0, y = 256)
1097 @0 glTexCoord2f(s = 1, t = 1)
1098 @0 glVertex2f(x = 512, y = 256)
1099 @0 glEnd()
[/code]

For reference: https://www.opengl.org/wiki/Legacy_OpenGL

Although the 3rd and subsequent frames are missing from qapitrace, it can be
assumed that Dying Light is most likely using the compatibility mode.

-- 
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/20161014/a05b382d/attachment-0001.html>

[PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> > Multiple incompatible data link layers have been used over time to
>> > transmit image data to LVDS panels. This binding supports display panels
>> > compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> > specifications.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > 
>> > ---
>> >
>> >  .../bindings/display/panel/panel-lvds.txt  | 119 
>> >  1 file changed, 119 insertions(+)
>> >  create mode 100644
>> >  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
>> > diff --git
>> > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt new file
>> > mode 100644
>> > index ..250861f2673e
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> > @@ -0,0 +1,119 @@
>> > +Generic LVDS Panel
>> > +==
>> > +
>> > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> > Multiple
>> > +incompatible data link layers have been used over time to transmit image
>> > data
>> > +to LVDS panels. This bindings supports display panels compatible with the
>> > +following specifications.
>> > +
>> > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
>> > February
>> > +1999 (Version 1.0), Japan Electronic Industry Development Association
>> > (JEIDA)
>> > +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
>> > +Semiconductor
>> > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
>> > +Electronics Standards Association (VESA)
>> > +
>> > +Device compatible with those specifications have been marketed under the
>> > +FPD-Link and FlatLink brands.
>> > +
>> > +
>> > +Required properties:
>> > +- compatible: shall contain "panel-lvds"
>>
>> Maybe as a fallback, but on its own, no way.
>
> Which brings an interesting question: when designing generic DT bindings,
> what's the rule regarding

Call it "simple" so I can easily NAK it. :)

Define a generic structure, not a single binding trying to serve all.

>
>> > +- width-mm: panel display width in millimeters
>> > +- height-mm: panel display height in millimeters
>>
>> This is already documented for all panels IIRC.
>
> Note that this DT binding has nothing to do with the simple-panel binding. It
> is instead similar to the panel-dpi and panel-dsi-cm bindings (which currently
> don't but should specify the panel size in DT). The LVDS panel driver will
> *not* include any panel-specific information such as size or timings, these
> are specified in DT.

The panel bindings aren't really different. The biggest difference was
location in the tree, but we now generally allow panels to be either a
child of the LCD controller or connected with OF graph. We probably
need to work on restructuring the panel bindings a bit. We should have
an inheritance with a base panel binding of things like size, label,
graph, backlight, etc, then perhaps an interface specific bindings for
LVDS, DSI, and parallel, then a panel specific binding. With this the
panel specific binding is typically just a compatible string and which
inherited properties apply to it.


>> > +- data-mapping: the color signals mapping order, "jeida-18", "jeida-24"
>> > +  or "vesa-24"
>>
>> Maybe this should be part of the compatible.
>
> I've thought about it, but given that some panels support selecting between
> multiple modes (through a mode pin that is usually hardwired), I believe a
> separate DT property makes sense.

Okay.

> Furthermore, LVDS data organization is controlled by the combination of both
> data-mapping and data-mirror. It makes little sense from my point of view to
> handle one as part of the compatible string and the other one as a separate
> property.
>
>> > +Optional properties:
>> > +- label: a symbolic name for the panel
>>
>> Could be for any panel or display connector.
>
> Yes, but I'm not sure to understand how that's relevant :-)

Meaning it should be a common property.

>> > +- avdd-supply: reference to the regulator that powers the panel
>> analog supply
>> > +- dvdd-supply: reference to the regulator that powers the panel digital
>> > supply
>>
>> Which one has to be powered on first, what voltage, and with what time
>> in between? This is why "generic" or "simple" bindings don't work.
>
> The above-mentioned specifications also define connectors, pinouts and power
> supplies, but many LVDS panels compatible with the LVDS physical and data
> layers use a different connector with small differences in power supplies.
>
> I believe the voltage is irrelevant here, it doesn't need to be controlled by
> the operating system. 

[Bug 93649] [radeonsi] Graphics lockup while playing tf2

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

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

--- Comment #42 from Amarildo  ---
(In reply to hofmann.zachary from comment #41)
> (In reply to Amarildo from comment #40)
> > If disabling DPM fixed the issue, shouldn't developers study it's code a
> > little bit? I'm 99.99% positive the issue is in there somewhere, even for
> > AMDGPU (since RadeonSI and AMDGPU drivers share a lot of code).
> 
> Another user previously stated in the thread that they were experiencing the
> issues and had DPM disabled.
> 
> @Marek Olšák
> Please let me know if there's anything I can do to help hunt this bug down.

But that's one user's word against at least 5. Do we even know if the user
actually disabled DPM or has the capacity to do so? Because I'm sure me and
others (like Gentoo users) did in fact disable DPM and the hang didn't happen.
So I don't think our word is less valid just because *one* user claimed he/she
disabled DPM and the hang still happened.

-- 
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/20161014/bb040fd3/attachment.html>

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

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

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

--- Comment #32 from Michel Dänzer  ---
(In reply to Yves W. from comment #31)
> Second I upgradedm, as I posted, to the ubuntu kernel 4.8.0 from
> ubuntu ppa, which gives errors 

But that's still using the radeon driver, right? If so, it's possible to narrow
down the exact kernel version where the problem was introduced, or even the
exact Git commit using git bisect.


> Furthermore the amd beta driver amdgpu-pro gives in every kernel I tried
> (4.4 and 4.8) flickering problems with refreshrates above 60hz.

This report is about the radeon driver, let's not confuse things here by mixing
in amdgpu-pro issues.

-- 
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/20161014/e9fc226a/attachment-0001.html>