Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

[zhoucm1]

On 2017年02月14日 18:37, Nicolai Hähnle wrote:

From: Nicolai Hähnle 

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle 
---
  drivers/gpu/drm/ttm/ttm_bo.c | 62 +---
  include/drm/ttm/ttm_bo_api.h | 45 
  2 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
  }
  EXPORT_SYMBOL(ttm_bo_validate);
  
-int ttm_bo_init(struct ttm_bo_device *bdev,

-   struct ttm_buffer_object *bo,
-   unsigned long size,
-   enum ttm_bo_type type,
-   struct ttm_placement *placement,
-   uint32_t page_alignment,
-   bool interruptible,
-   struct file *persistent_swap_storage,
-   size_t acc_size,
-   struct sg_table *sg,
-   struct reservation_object *resv,
-   void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+   struct ttm_buffer_object *bo,
+   unsigned long size,
+   enum ttm_bo_type type,
+   uint32_t page_alignment,
+   struct file *persistent_swap_storage,
+   size_t acc_size,
+   struct sg_table *sg,
+   struct reservation_object *resv,
+   void (*destroy) (struct ttm_buffer_object *))
  {
int ret = 0;
unsigned long num_pages;
struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-   bool locked;
  
  	ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);

if (ret) {
pr_err("Out of kernel memory\n");
-   if (destroy)
-   (*destroy)(bo);
-   else
-   kfree(bo);
return -ENOMEM;
}
  
  	num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;

if (num_pages == 0) {
pr_err("Illegal buffer object size\n");
-   if (destroy)
-   (*destroy)(bo);
-   else
-   kfree(bo);
ttm_mem_global_free(mem_glob, acc_size);
return -EINVAL;
}
@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
ret = drm_vma_offset_add(>vma_manager, >vma_node,
 bo->mem.num_pages);
if (ret && !resv), we should call 
reservation_object_fini(>ttm_resv), right?


  
+	return ret;

+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+   struct ttm_buffer_object *bo,
+   unsigned long size,
+   enum ttm_bo_type type,
+   struct ttm_placement *placement,
+   uint32_t page_alignment,
+   bool interruptible,
+   struct file *persistent_swap_storage,
+   size_t acc_size,
+   struct sg_table *sg,
+   struct reservation_object *resv,
+   void (*destroy) (struct ttm_buffer_object *))
+{
+   bool locked;
+   int ret;
+
Can we lock resv anyway before ttm_bo_init_top like what you did in 
patch #3? if yes, seems we don't need patch#3 any more, right?



if (!resv) {
bool locked;

reservation_object_init(>tbo.ttm_resv);
locked = ww_mutex_trylock(>tbo.ttm_resv.lock);
WARN_ON(!locked);
}
r = ttm_bo_init_top(>mman.bdev, >tbo, size, type,
page_align, NULL,
acc_size, sg, resv ? resv : >tbo.ttm_resv,
_ttm_bo_destroy);


Regards,
David Zhou

+   ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+ persistent_swap_storage, acc_size, sg, resv,
+ destroy);
+   if (ret) {
+   if (destroy)
+   (*destroy)(bo);
+   else
+   kfree(bo);
+   return ret;
+   }
+
/* passed reservation objects should already be locked,
 * since otherwise lockdep will be angered in radeon.
 */
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index f195899..d44b8e4 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -453,6 +453,51 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev,
   unsigned struct_size);
  
  /**

+ 

Re: [PATCH] uapi: add missing install of dma-buf.h

[Sumit Semwal]

Hi Denys,

On 15 February 2017 at 01:30, Denys Dmytriyenko  wrote:
> From: Denys Dmytriyenko 
>
> As part of c11e391da2a8fe973c3c2398452000bed505851e "dma-buf: Add ioctls to
> allow userspace to flush" a new uapi header file dma-buf.h was added, but an
> entry was not added on Kbuild to install it. This patch resolves this omission
> so that "make headers_install" installs this header.
>
Thanks for the patch; please feel free to add my
Acked-by: Sumit Semwal 

> Signed-off-by: Denys Dmytriyenko 
> Reviewed-by: Tomi Valkeinen 
> Cc: Ville Syrjälä 
> Cc: David Herrmann 
> Cc: Sumit Semwal 
> Cc: Daniel Vetter 
> Cc: Tiago Vignatti 
> Cc: Daniel Vetter 
> ---
>  include/uapi/linux/Kbuild | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index f330ba4..900129c 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -109,6 +109,7 @@ header-y += dlm_netlink.h
>  header-y += dlm_plock.h
>  header-y += dm-ioctl.h
>  header-y += dm-log-userspace.h
> +header-y += dma-buf.h
>  header-y += dn.h
>  header-y += dqblk_xfs.h
>  header-y += edd.h
> --
> 2.7.4
>

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

[Bug 99323] White horizontal lines and graphics curruption in ATI HD 4570 when radeon.dpm=1

[bugzilla-daemon]

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

--- Comment #12 from kartik  ---
Forcing dpm to high performance mode resolves the problem for now.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v7 2/7] drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi

[Chris Zhong]

The vopb/vopl switch register of RK3399 mipi is different from RK3288,
the default setting for mipi dsi mode is different too, so add a
of_device_id structure to distinguish them, and make sure set the
correct mode before mipi phy init.

Signed-off-by: Chris Zhong 
Signed-off-by: Mark Yao 
Reviewed-by: Sean Paul 

---

Changes in v6:
- no need check phy_cfg_clk before enable/disable

Changes in v5:
- check the error of phy_cfg_clk in dw_mipi_dsi_bind

Changes in v4:
- remove the unrelated change

Changes in v3:
- base on John Keeping's patch series

 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 72 +-
 1 file changed, 62 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index cc58ada..4e74681 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -29,9 +29,17 @@
 
 #define DRIVER_NAME"dw-mipi-dsi"
 
-#define GRF_SOC_CON60x025c
-#define DSI0_SEL_VOP_LIT(1 << 6)
-#define DSI1_SEL_VOP_LIT(1 << 9)
+#define RK3288_GRF_SOC_CON60x025c
+#define RK3288_DSI0_SEL_VOP_LITBIT(6)
+#define RK3288_DSI1_SEL_VOP_LITBIT(9)
+
+#define RK3399_GRF_SOC_CON19   0x6250
+#define RK3399_DSI0_SEL_VOP_LITBIT(0)
+#define RK3399_DSI1_SEL_VOP_LITBIT(4)
+
+/* disable turnrequest, turndisable, forcetxstopmode, forcerxmode */
+#define RK3399_GRF_SOC_CON22   0x6258
+#define RK3399_GRF_DSI_MODE0x
 
 #define DSI_VERSION0x00
 #define DSI_PWR_UP 0x04
@@ -265,6 +273,11 @@ enum {
 };
 
 struct dw_mipi_dsi_plat_data {
+   u32 dsi0_en_bit;
+   u32 dsi1_en_bit;
+   u32 grf_switch_reg;
+   u32 grf_dsi0_mode;
+   u32 grf_dsi0_mode_reg;
unsigned int max_data_lanes;
enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
   struct drm_display_mode *mode);
@@ -281,6 +294,7 @@ struct dw_mipi_dsi {
 
struct clk *pllref_clk;
struct clk *pclk;
+   struct clk *phy_cfg_clk;
 
unsigned int lane_mbps; /* per lane */
u32 channel;
@@ -425,6 +439,12 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
 
+   ret = clk_prepare_enable(dsi->phy_cfg_clk);
+   if (ret) {
+   dev_err(dsi->dev, "Failed to enable phy_cfg_clk\n");
+   return ret;
+   }
+
dw_mipi_dsi_phy_write(dsi, 0x10, BYPASS_VCO_RANGE |
 VCO_RANGE_CON_SEL(vco) |
 VCO_IN_CAP_CON_LOW |
@@ -481,17 +501,18 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
 val, val & LOCK, 1000, PHY_STATUS_TIMEOUT_US);
if (ret < 0) {
dev_err(dsi->dev, "failed to wait for phy lock state\n");
-   return ret;
+   goto phy_init_end;
}
 
ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
 val, val & STOP_STATE_CLK_LANE, 1000,
 PHY_STATUS_TIMEOUT_US);
-   if (ret < 0) {
+   if (ret < 0)
dev_err(dsi->dev,
"failed to wait for phy clk lane stop state\n");
-   return ret;
-   }
+
+phy_init_end:
+   clk_disable_unprepare(dsi->phy_cfg_clk);
 
return ret;
 }
@@ -960,6 +981,7 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder 
*encoder)
 {
struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
struct drm_display_mode *mode = >crtc->state->adjusted_mode;
+   const struct dw_mipi_dsi_plat_data *pdata = dsi->pdata;
int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
u32 val;
int ret;
@@ -985,6 +1007,10 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder 
*encoder)
dw_mipi_dsi_dphy_interface_config(dsi);
dw_mipi_dsi_clear_err(dsi);
 
+   if (pdata->grf_dsi0_mode_reg)
+   regmap_write(dsi->grf_regmap, pdata->grf_dsi0_mode_reg,
+pdata->grf_dsi0_mode);
+
dw_mipi_dsi_phy_init(dsi);
dw_mipi_dsi_wait_for_two_frames(mode);
 
@@ -998,11 +1024,11 @@ static void dw_mipi_dsi_encoder_enable(struct 
drm_encoder *encoder)
clk_disable_unprepare(dsi->pclk);
 
if (mux)
-   val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16);
+   val = pdata->dsi0_en_bit | (pdata->dsi0_en_bit << 16);
else
-   val = DSI0_SEL_VOP_LIT << 16;
+   val = pdata->dsi0_en_bit << 16;
 
-   regmap_write(dsi->grf_regmap, GRF_SOC_CON6, val);
+  

[PATCH v7 1/7] dt-bindings: add rk3399 support for dw-mipi-rockchip

[Chris Zhong]

The dw-mipi-dsi of rk3399 is almost the same as rk3288, the rk3399 has
additional phy config clock.

Signed-off-by: Chris Zhong 
Acked-by: Rob Herring 
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None

 .../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt 
b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
index 1753f0c..0f82568 100644
--- 
a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
+++ 
b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
@@ -5,10 +5,12 @@ Required properties:
 - #address-cells: Should be <1>.
 - #size-cells: Should be <0>.
 - compatible: "rockchip,rk3288-mipi-dsi", "snps,dw-mipi-dsi".
+ "rockchip,rk3399-mipi-dsi", "snps,dw-mipi-dsi".
 - reg: Represent the physical address range of the controller.
 - interrupts: Represent the controller's interrupt to the CPU(s).
 - clocks, clock-names: Phandles to the controller's pll reference
-  clock(ref) and APB clock(pclk), as described in [1].
+  clock(ref) and APB clock(pclk). For RK3399, a phy config clock
+  (phy_cfg) is additional required. As described in [1].
 - rockchip,grf: this soc should set GRF regs to mux vopl/vopb.
 - ports: contain a port node with endpoint definitions as defined in [2].
   For vopb,set the reg = <0> and set the reg = <1> for vopl.
-- 
2.6.3

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

[PATCH v7 0/7] Rockchip dw-mipi-dsi driver

[Chris Zhong]

Hi all

This version does not change the existing v6 patches, just to add the
"bandwidth fix" patch back, since we really need it.

This patch serial is for RK3399 MIPI DSI. The MIPI DSI controller of
RK3399 is almost the same as RK3288, except a little bit of difference
in phy clock controlling and port id selection register. These patches
add RK3399 support and the power domain support.

And these patches base on John Keeping's v3 patches[0], it fixes many bugs,
they have been tested on rk3288 evb board.

[0]:
[01/24] https://patchwork.kernel.org/patch/9544089
[02/24] https://patchwork.kernel.org/patch/9544061
[03/24] https://patchwork.kernel.org/patch/9544065
[04/24] https://patchwork.kernel.org/patch/9544077
[05/24] https://patchwork.kernel.org/patch/9544033
[06/24] https://patchwork.kernel.org/patch/9544037
[07/24] https://patchwork.kernel.org/patch/9544029
[08/24] https://patchwork.kernel.org/patch/9544031
[09/24] https://patchwork.kernel.org/patch/9544083
[10/24] https://patchwork.kernel.org/patch/9544063
[11/24] https://patchwork.kernel.org/patch/9544085
[12/24] https://patchwork.kernel.org/patch/9544093
[13/24] https://patchwork.kernel.org/patch/9544081
[14/24] https://patchwork.kernel.org/patch/9544057
[15/24] https://patchwork.kernel.org/patch/9544079
[16/24] https://patchwork.kernel.org/patch/9544035
[17/24] https://patchwork.kernel.org/patch/9544105
[18/24] https://patchwork.kernel.org/patch/9544059
[21/24] https://patchwork.kernel.org/patch/9544009
[22/24] https://patchwork.kernel.org/patch/9544049
[23/24] https://patchwork.kernel.org/patch/9544055
[24/24] https://patchwork.kernel.org/patch/9544109


Changes in v6:
- no need check phy_cfg_clk before enable/disable

Changes in v5:
- check the error of phy_cfg_clk in dw_mipi_dsi_bind

Changes in v4:
- remove the unrelated change

Changes in v3:
- base on John Keeping's patch series

Chris Zhong (7):
  dt-bindings: add rk3399 support for dw-mipi-rockchip
  drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi
  drm/rockchip/dsi: dw-mipi: correct the coding style
  drm/rockchip/dsi: remove mode_valid function
  dt-bindings: add power domain node for dw-mipi-rockchip
  drm/rockchip/dsi: fix insufficient bandwidth of some panel
  drm/rockchip/dsi: add dw-mipi power domain support

 .../display/rockchip/dw_mipi_dsi_rockchip.txt  |   7 +-
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 160 -
 2 files changed, 100 insertions(+), 67 deletions(-)

-- 
2.6.3

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

Re: [PATCH v2 11/11] drm/rockchip/dsi: fix insufficient bandwidth of some panel

[Chris Zhong]

Hi John

On 01/17/2017 06:54 PM, John Keeping wrote:

On Tue, 17 Jan 2017 17:31:53 +0800, Chris Zhong wrote:


On 01/16/2017 08:44 PM, John Keeping wrote:

On Mon, 16 Jan 2017 18:08:31 +0800, Chris Zhong wrote:
  

Set the lanes bps to 1 / 0.9 times of pclk, the margin is not enough
for some panel, it will cause the screen display is not normal, so
increases the badnwidth to 1 / 0.8.

Signed-off-by: Chris Zhong 

---

   drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index 9dfa73d..5a973fe 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -501,8 +501,8 @@ static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi)
   
   	mpclk = DIV_ROUND_UP(dsi->mode->clock, MSEC_PER_SEC);

if (mpclk) {
-   /* take 1 / 0.9, since mbps must big than bandwidth of RGB */
-   tmp = mpclk * (bpp / dsi->lanes) * 10 / 9;
+   /* take 1 / 0.8, since mbps must big than bandwidth of RGB */
+   tmp = mpclk * (bpp / dsi->lanes) * 10 / 8;

This and patch 9 are just hacking around the underlying problem in order
to make particular panels work.  I'm pretty sure the actual issue is the
use of hardcoded values when configuring the PHY, since the PHY
parameters are specified in clock cycles but the MIPI spec requires
absolute time durations.

I posted a series addressing this a while ago, although I screwed up
sending it so some patches were included twice and since no one
expressed any interest I didn't post a cleaned up version.

The relevant patch is here:

https://patchwork.kernel.org/patch/9340193/

Thanks very much, your patches are very useful for me. It looks your
method is correct.
And I am very confused why Mark Yao and me did not receive your patches
before,
although we have subscribed the .

In addition, could you tell me which device ware you testing with these
mipi patches.
I going to test them these day.

I'm using RK3288 and I tested my patches with three different MIPI
displays, two of which require commands to be sent in order to set up
the panel.

Thanks for testing the patches.


John
I think we really need this patch, one mipi panel hit this problem 
again, with all your 24 patches

and my 6 MIPI DSI patches
So I will update my series to v7, and add this patch into it.







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

Re: [PATCH v3 23/24] drm/rockchip: dw-mipi-dsi: add reset control

[Chris Zhong]

Hi John

On 01/29/2017 09:24 PM, John Keeping wrote:

In order to fully reset the state of the MIPI controller we must assert
this reset.

This is slightly more complicated than it could be in order to maintain
compatibility with device trees that do not specify the reset property.

Signed-off-by: John Keeping 
Reviewed-by: Chris Zhong 
---
v3:
- Add Chris' Reviewed-by
Unchanged in v2

  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 30 ++
  1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index 58cb8ace2fe8..cf3ca6b0cbdb 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -13,6 +13,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -1124,6 +1125,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct 
device *master,
of_match_device(dw_mipi_dsi_dt_ids, dev);
const struct dw_mipi_dsi_plat_data *pdata = of_id->data;
struct platform_device *pdev = to_platform_device(dev);
+   struct reset_control *apb_rst;
struct drm_device *drm = data;
struct dw_mipi_dsi *dsi;
struct resource *res;
@@ -1162,6 +1164,34 @@ static int dw_mipi_dsi_bind(struct device *dev, struct 
device *master,
return ret;
}
  
+	/*

+* Note that the reset was not defined in the initial device tree, so
+* we have to be prepared for it not being found.
+*/
+   apb_rst = devm_reset_control_get(dev, "apb");
+   if (IS_ERR(apb_rst)) {
+   if (PTR_ERR(apb_rst) == -ENODEV) {

According to [0], I think it should be -ENOENT here.

[0]
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=3d81216fde465e76c5eae98f61d3666163634395

commit 3d81216fde465e76c5eae98f61d3666163634395
Author: Alban Bedel 
Date:   Tue Sep 1 17:28:31 2015 +0200

reset: Fix of_reset_control_get() for consistent return values

When of_reset_control_get() is called without connection ID it returns
-ENOENT when the 'resets' property doesn't exists or is an empty entry.
However when a connection ID is given it returns -EINVAL when the 
'resets'

property doesn't exists or the requested name can't be found. This is
because the error code returned by of_property_match_string() is just
passed down as an index to of_parse_phandle_with_args(), which then
returns -EINVAL.

To get a consistent return value with both code paths we must return
-ENOENT when of_property_match_string() fails.

Signed-off-by: Alban Bedel 
Signed-off-by: Philipp Zabel 



+   apb_rst = NULL;
+   } else {
+   dev_err(dev, "Unable to get reset control: %d\n", ret);
+   return PTR_ERR(apb_rst);
+   }
+   }
+
+   if (apb_rst) {
+   ret = clk_prepare_enable(dsi->pclk);
+   if (ret) {
+   dev_err(dev, "%s: Failed to enable pclk\n", __func__);
+   return ret;
+   }
+
+   reset_control_assert(apb_rst);
+   usleep_range(10, 20);
+   reset_control_deassert(apb_rst);
+
+   clk_disable_unprepare(dsi->pclk);
+   }
+
ret = clk_prepare_enable(dsi->pllref_clk);
if (ret) {
dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);



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

Re: [PATCH] uapi: add missing install of dma-buf.h

[Sumit Semwal]

Hi Denys,

On 15 February 2017 at 01:30, Denys Dmytriyenko  wrote:

> From: Denys Dmytriyenko 
>
> As part of c11e391da2a8fe973c3c2398452000bed505851e "dma-buf: Add ioctls
> to
> allow userspace to flush" a new uapi header file dma-buf.h was added, but
> an
> entry was not added on Kbuild to install it. This patch resolves this
> omission
> so that "make headers_install" installs this header.
>
> Thanks for the patch; please feel free to add my
Acked-by: Sumit Semwal 


> Signed-off-by: Denys Dmytriyenko 
> Reviewed-by: Tomi Valkeinen 
> Cc: Ville Syrjälä 
> Cc: David Herrmann 
> Cc: Sumit Semwal 
> Cc: Daniel Vetter 
> Cc: Tiago Vignatti 
> Cc: Daniel Vetter 
> ---
>  include/uapi/linux/Kbuild | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index f330ba4..900129c 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -109,6 +109,7 @@ header-y += dlm_netlink.h
>  header-y += dlm_plock.h
>  header-y += dm-ioctl.h
>  header-y += dm-log-userspace.h
> +header-y += dma-buf.h
>  header-y += dn.h
>  header-y += dqblk_xfs.h
>  header-y += edd.h
> --
> 2.7.4
>
>
Best,
Sumit.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[Rob Clark]

On Tue, Feb 14, 2017 at 4:49 PM, Daniel Vetter  wrote:
> On Tue, Feb 14, 2017 at 01:07:21PM -0800, John Stultz wrote:
>> On Tue, Feb 14, 2017 at 12:32 PM, Daniel Stone  wrote:
>> > Hi John,
>> >
>> > On 14 February 2017 at 19:25, John Stultz  wrote:
>> >> +static enum drm_mode_status
>> >> +drm_connector_check_crtc_modes(struct drm_connector *connector,
>> >> +  struct drm_display_mode *mode)
>> >> +{
>> >> +   struct drm_device *dev = connector->dev;
>> >> +   const struct drm_crtc_helper_funcs *crtc_funcs;
>> >> +   struct drm_crtc *c;
>> >> +
>> >> +   if (mode->status != MODE_OK)
>> >> +   return mode->status;
>> >> +
>> >> +   /* Check all the crtcs on a connector to make sure the mode is 
>> >> valid */
>> >> +   drm_for_each_crtc(c, dev) {
>> >> +   crtc_funcs = c->helper_private;
>> >> +   if (crtc_funcs && crtc_funcs->mode_valid)
>> >> +   mode->status = crtc_funcs->mode_valid(c, mode);
>> >> +   if (mode->status != MODE_OK)
>> >> +   break;
>> >> +   }
>> >> +   return mode->status;
>> >> +}
>> >
>> > Hm, that's unfortunate: it limits the mode list for every connector,
>> > to those which are supported by every single CRTC. So if you have one
>> > CRTC serving low-res LVDS, and another serving higher-res HDMI,
>> > suddenly you can't get bigger modes on HDMI. The idea seems sound
>> > enough, but a little more nuance might be good ...
>>
>> Yea. That is not my intent at all I'm just trying to get the drm_crtc
>> attached to the connector that we're getting the EDID mode lines from.
>> I had tried going connector->encoder->crtc, but at the time this is
>> called, the encoder is null. So Rob suggested the for_each_crtc(), and
>> I guess I mistook that for being each crtc on the connector.
>>
>> Thanks for pointing out this issue. From Daniel's feedback it looks
>> like I need to start over from scratch though, so little worry this
>> implementation will go much further.
>
> Well your idea was somewhat right, but logic inverted. In ->mode_valid we
> need to check whether any encoder/crtc combo could support the mode. Which
> means you need to reject it only when there's no encoder/crtc combo that
> could support the mode (you reject it if there's only one crtc which can't
> handle it).

sorry, I was probably not expressing my idea to John very well on IRC,
but yeah, the idea was for this to only reject modes that are
impossible for all CRTCs (so a bit different than the case that the
atomic_check callbacks would be validating)

and btw, yeah, this is specifically about fixing things for bridges or
situations where the connector is shared across multiple drivers.  It
isn't really something we can solve in-driver.  Maybe driver provided
callbacks to the bridge would do the trick, but that seemed a bit
weird.  The simple idea was to give the bridge a way to figure out
things that were completely unpossible and let the driver figure out
how to make the things that are possible work somehow.

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

[Bug 99387] Kernel 4.9: Kaveri + Hainan choked on boot using amdgpu

[bugzilla-daemon]

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

--- Comment #17 from Luya Tshimbalanga  ---
The fix is working. Hainan video card aka Jet Pro R5 M230 successfully
initialized along the Kaveri card

01:00.0 Display controller: Advanced Micro Devices, Inc. [AMD/ATI] Jet PRO
[Radeon R5 M230]
Subsystem: ASUSTeK Computer Inc. Device 130d
Physical Slot: 0
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- 
Capabilities: [50] Power Management version 3
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA
PME(D0-,D1+,D2+,D3hot+,D3cold-)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [58] Express (v2) Legacy Endpoint, MSI 00
DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s <4us, L1
unlimited
ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
MaxPayload 256 bytes, MaxReadReq 512 bytes
DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr-
TransPend-
LnkCap: Port #0, Speed 5GT/s, Width x8, ASPM L0s L1, Exit
Latency L0s <64ns, L1 <1us
ClockPM- Surprise- LLActRep- BwNot- ASPMOptComp+
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+
ExtSynch- ClockPM- AutWidDis+ BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x8, TrErr- Train- SlotClk+
DLActive- BWMgmt- ABWMgmt-
DevCap2: Completion Timeout: Not Supported, TimeoutDis-, LTR-,
OBFF Not Supported
DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-,
OBFF Disabled
LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-
 Transmit Margin: Normal Operating Range,
EnterModifiedCompliance- ComplianceSOS-
 Compliance De-emphasis: -6dB
LnkSta2: Current De-emphasis Level: -3.5dB,
EqualizationComplete-, EqualizationPhase1-
 EqualizationPhase2-, EqualizationPhase3-,
LinkEqualizationRequest+
Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
Address: fee0f00c  Data: 4163
Capabilities: [100 v1] Vendor Specific Information: ID=0001 Rev=1
Len=010 
Capabilities: [150 v2] Advanced Error Reporting
UESta:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt-
RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UEMsk:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt-
RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt-
RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta:  RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
CEMsk:  RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
AERCap: First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-
Capabilities: [270 v1] #19
Kernel driver in use: amdgpu
Kernel modules: radeon, amdgpu

Minor issue is optimal power management. Essentially, the Hainan card is
running fine. Hopefully Marco will have similar success.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/rockchip: add extcon dependency for DP

[Mark yao]

On 2017年02月15日 05:31, Arnd Bergmann wrote:

The newly added DP driver links against the extcon core, which fails when
extcon is a module and this driver is not:

drivers/gpu/drm/rockchip/cdn-dp-core.o: In function `cdn_dp_get_port_lanes':
cdn-dp-core.c:(.text.cdn_dp_get_port_lanes+0x24): undefined reference to 
`extcon_get_state'
cdn-dp-core.c:(.text.cdn_dp_get_port_lanes+0x44): undefined reference to 
`extcon_get_property'

Let's make Kconfig enforce correct behavior with a dependency.


Thanks for the fix.

Applied to my drm-next.


Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399")
Signed-off-by: Arnd Bergmann 
---
  drivers/gpu/drm/rockchip/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index ad31b3eb408f..0e4eb845cbb0 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -24,6 +24,7 @@ config ROCKCHIP_ANALOGIX_DP
  config ROCKCHIP_CDN_DP
  tristate "Rockchip cdn DP"
  depends on DRM_ROCKCHIP
+   depends on EXTCON
select SND_SOC_HDMI_CODEC if SND_SOC
  help
  This selects support for Rockchip SoC specific extensions



--
Ｍark Yao


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

[drm-intel:drm-intel-next-queued 11/59] drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: warning: missing braces around initializer

[kbuild test robot]

tree:   git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head:   d892e9398ecf6defc7972a62227b77dad6be20bd
commit: 953c7f82eb890085c60dbe22578e883d6837c674 [11/59] drm/i915: Provide a 
hook for selftests
config: x86_64-randconfig-s1-02150712 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
git checkout 953c7f82eb890085c60dbe22578e883d6837c674
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/selftests/i915_selftest.c:68:
   drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: unknown 
field 'mock' specified in initializer
>> drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: warning: missing 
>> braces around initializer
   drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: warning: (near 
initialization for 'mock_selftests[0].')
   In file included from drivers/gpu/drm/i915/selftests/i915_selftest.c:74:
   drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: unknown 
field 'live' specified in initializer
>> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: warning: missing 
>> braces around initializer
   drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: warning: (near 
initialization for 'live_selftests[0].')
>> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: warning: 
>> initialization from incompatible pointer type

vim +11 drivers/gpu/drm/i915/selftests/i915_mock_selftests.h

 1  /* List each unit test as selftest(name, function)
 2   *
 3   * The name is used as both an enum and expanded as subtest__name to 
create
 4   * a module parameter. It must be unique and legal for a C identifier.
 5   *
 6   * The function should be of type int function(void). It may be 
conditionally
 7   * compiled using #if IS_ENABLED(DRM_I915_SELFTEST).
 8   *
 9   * Tests are executed in order by igt/drv_selftest
10   */
  > 11  selftest(sanitycheck, i915_mock_sanitycheck) /* keep first (igt 
selfcheck) */

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

[Nicolai Hähnle]

On 14.02.2017 11:49, Christian König wrote:

Am 14.02.2017 um 11:37 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle 


Please squash that into your other patch. It fixes another bug, but I
don't think fixing one bug just to run into another is really a good idea.


I don't understand. I'm not aware that this patch fixes anything, it 
just enables the subsequent fix in amdgpu in patch #2. I don't think 
squashing those together is a good idea (one is in ttm, the other in 
amdgpu).




Additional to that one comment below.


---
  drivers/gpu/drm/ttm/ttm_bo.c | 62
+---
  include/drm/ttm/ttm_bo_api.h | 45 
  2 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
  }
  EXPORT_SYMBOL(ttm_bo_validate);
  -int ttm_bo_init(struct ttm_bo_device *bdev,
-struct ttm_buffer_object *bo,
-unsigned long size,
-enum ttm_bo_type type,
-struct ttm_placement *placement,
-uint32_t page_alignment,
-bool interruptible,
-struct file *persistent_swap_storage,
-size_t acc_size,
-struct sg_table *sg,
-struct reservation_object *resv,
-void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+uint32_t page_alignment,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
  {
  int ret = 0;
  unsigned long num_pages;
  struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-bool locked;
ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
  if (ret) {
  pr_err("Out of kernel memory\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  return -ENOMEM;
  }
num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
  if (num_pages == 0) {
  pr_err("Illegal buffer object size\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  ttm_mem_global_free(mem_glob, acc_size);
  return -EINVAL;
  }


I would move those checks after all the field initializations. This way
the structure has at least a valid content and we can safely use
ttm_bo_unref on it.


That feels odd to me, since the return value indicates that the buffer 
wasn't properly initialized, but I don't feel strongly about it.


Cheers,
Nicolai




Regards,
Christian.


@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
  ret = drm_vma_offset_add(>vma_manager, >vma_node,
   bo->mem.num_pages);
  +return ret;
+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+struct ttm_placement *placement,
+uint32_t page_alignment,
+bool interruptible,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
+{
+bool locked;
+int ret;
+
+ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+  persistent_swap_storage, acc_size, sg, resv,
+  destroy);
+if (ret) {
+if (destroy)
+(*destroy)(bo);
+else
+kfree(bo);
+return ret;
+}
+
  /* passed reservation objects should already be locked,
   * since otherwise lockdep will be angered in radeon.
   */
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index f195899..d44b8e4 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -453,6 +453,51 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device
*bdev,
 unsigned struct_size);
/**
+ * ttm_bo_init_top
+ *
+ * @bdev: Pointer to a ttm_bo_device struct.
+ * @bo: Pointer to a ttm_buffer_object to be initialized.
+ * @size: Requested size of buffer object.
+ * @type: Requested type of buffer object.
+ * @flags: Initial placement flags.
+ * @page_alignment: Data alignment in pages.
+ * @persistent_swap_storage: 

[PATCH] uapi: add missing install of dma-buf.h

[Denys Dmytriyenko]

From: Denys Dmytriyenko 

As part of c11e391da2a8fe973c3c2398452000bed505851e "dma-buf: Add ioctls to
allow userspace to flush" a new uapi header file dma-buf.h was added, but an
entry was not added on Kbuild to install it. This patch resolves this omission
so that "make headers_install" installs this header.

Signed-off-by: Denys Dmytriyenko 
Reviewed-by: Tomi Valkeinen 
Cc: Ville Syrjälä 
Cc: David Herrmann 
Cc: Sumit Semwal 
Cc: Daniel Vetter 
Cc: Tiago Vignatti 
Cc: Daniel Vetter 
---
 include/uapi/linux/Kbuild | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index f330ba4..900129c 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -109,6 +109,7 @@ header-y += dlm_netlink.h
 header-y += dlm_plock.h
 header-y += dm-ioctl.h
 header-y += dm-log-userspace.h
+header-y += dma-buf.h
 header-y += dn.h
 header-y += dqblk_xfs.h
 header-y += edd.h
-- 
2.7.4

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

Re: [PATCH] drm/rockchip: add extcon dependency for DP

[Guenter Roeck]

On Tue, Feb 14, 2017 at 1:31 PM, Arnd Bergmann  wrote:
> The newly added DP driver links against the extcon core, which fails when
> extcon is a module and this driver is not:
>
> drivers/gpu/drm/rockchip/cdn-dp-core.o: In function `cdn_dp_get_port_lanes':
> cdn-dp-core.c:(.text.cdn_dp_get_port_lanes+0x24): undefined reference to 
> `extcon_get_state'
> cdn-dp-core.c:(.text.cdn_dp_get_port_lanes+0x44): undefined reference to 
> `extcon_get_property'
>
> Let's make Kconfig enforce correct behavior with a dependency.
>
> Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399")
> Signed-off-by: Arnd Bergmann 

Reviewed-by: Guenter Roeck 

> ---
>  drivers/gpu/drm/rockchip/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/rockchip/Kconfig 
> b/drivers/gpu/drm/rockchip/Kconfig
> index ad31b3eb408f..0e4eb845cbb0 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -24,6 +24,7 @@ config ROCKCHIP_ANALOGIX_DP
>  config ROCKCHIP_CDN_DP
>  tristate "Rockchip cdn DP"
>  depends on DRM_ROCKCHIP
> +   depends on EXTCON
> select SND_SOC_HDMI_CODEC if SND_SOC
>  help
>   This selects support for Rockchip SoC specific extensions
> --
> 2.9.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 5/6] drm: convert drivers to use drm_of_find_panel_or_bridge

[Rob Herring]

On Mon, Feb 13, 2017 at 1:47 AM, Boris Brezillon
 wrote:
> On Thu,  9 Feb 2017 13:05:57 -0600
> Rob Herring  wrote:
>
>> Similar to the previous commit, convert drivers open coding OF graph
>> parsing to use drm_of_find_panel_or_bridge instead.
>>
>> This changes some error messages to debug messages (in the graph core).
>> Graph connections are often "no connects" depending on the particular
>> board, so we want to avoid spurious messages. Plus the kernel is not a
>> DT validator.
>>
>> Signed-off-by: Rob Herring 
>> ---
>> v2:
>> - fix wrong node ptr in imx-ldb
>> - build fixes in kirin and imx drivers
>>
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 64 -
>>  drivers/gpu/drm/bridge/nxp-ptn3460.c | 16 ++---
>>  drivers/gpu/drm/bridge/parade-ps8622.c   | 16 ++---
>>  drivers/gpu/drm/bridge/tc358767.c| 27 +--
>>  drivers/gpu/drm/exynos/exynos_dp.c   | 35 -
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c| 49 -
>>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 27 ++-
>>  drivers/gpu/drm/imx/imx-ldb.c| 27 ++-
>>  drivers/gpu/drm/imx/parallel-display.c   | 36 ++
>>  drivers/gpu/drm/mediatek/mtk_dsi.c   | 23 ++
>>  drivers/gpu/drm/mxsfb/mxsfb_out.c| 36 ++
>>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c  | 26 ++-
>>  drivers/gpu/drm/sun4i/sun4i_rgb.c| 13 ++--
>>  drivers/gpu/drm/sun4i/sun4i_tcon.c   | 90 
>> ++--
>>  14 files changed, 88 insertions(+), 397 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c 
>> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
>> index 6119b5085501..4614048a4935 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
>> @@ -22,7 +22,7 @@
>>  #include 
>>
>>  #include 
>> -#include 
>> +#include 
>>
>>  #include "atmel_hlcdc_dc.h"
>>
>> @@ -152,29 +152,11 @@ static const struct drm_connector_funcs 
>> atmel_hlcdc_panel_connector_funcs = {
>>   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>  };
>>
>> -static int atmel_hlcdc_check_endpoint(struct drm_device *dev,
>> -   const struct of_endpoint *ep)
>> -{
>> - struct device_node *np;
>> - void *obj;
>> -
>> - np = of_graph_get_remote_port_parent(ep->local_node);
>> -
>> - obj = of_drm_find_panel(np);
>> - if (!obj)
>> - obj = of_drm_find_bridge(np);
>> -
>> - of_node_put(np);
>> -
>> - return obj ? 0 : -EPROBE_DEFER;
>> -}
>> -
>>  static int atmel_hlcdc_attach_endpoint(struct drm_device *dev,
>> -const struct of_endpoint *ep)
>> +const struct device_node *np)
>>  {
>>   struct atmel_hlcdc_dc *dc = dev->dev_private;
>>   struct atmel_hlcdc_rgb_output *output;
>> - struct device_node *np;
>>   struct drm_panel *panel;
>>   struct drm_bridge *bridge;
>>   int ret;
>> @@ -195,13 +177,11 @@ static int atmel_hlcdc_attach_endpoint(struct 
>> drm_device *dev,
>>
>>   output->encoder.possible_crtcs = 0x1;
>>
>> - np = of_graph_get_remote_port_parent(ep->local_node);
>> -
>> - ret = -EPROBE_DEFER;
>> + ret = drm_of_find_panel_or_bridge(np, 0, 0, , );
>> + if (ret)
>> + return ret;
>>
>> - panel = of_drm_find_panel(np);
>>   if (panel) {
>> - of_node_put(np);
>>   output->connector.dpms = DRM_MODE_DPMS_OFF;
>>   output->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
>>   drm_connector_helper_add(>connector,
>> @@ -226,9 +206,6 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device 
>> *dev,
>>   return 0;
>>   }
>>
>> - bridge = of_drm_find_bridge(np);
>> - of_node_put(np);
>> -
>>   if (bridge) {
>>   output->encoder.bridge = bridge;
>>   bridge->encoder = >encoder;
>> @@ -245,31 +222,14 @@ static int atmel_hlcdc_attach_endpoint(struct 
>> drm_device *dev,
>>
>>  int atmel_hlcdc_create_outputs(struct drm_device *dev)
>>  {
>> - struct device_node *ep_np = NULL;
>> - struct of_endpoint ep;
>> + struct device_node *remote;
>>   int ret;
>>
>> - for_each_endpoint_of_node(dev->dev->of_node, ep_np) {
>> - ret = of_graph_parse_endpoint(ep_np, );
>> - if (!ret)
>> - ret = atmel_hlcdc_check_endpoint(dev, );
>> -
>> - if (ret) {
>> - of_node_put(ep_np);
>> - return ret;
>> - }
>> - }
>> -
>> - for_each_endpoint_of_node(dev->dev->of_node, ep_np) {
>> - ret = of_graph_parse_endpoint(ep_np, );
>> - if (!ret)
>> - ret = 

Re: [Intel-gfx] [PATCH v3 7/8] drm: Connector helper function to release resources

[Pandiyan, Dhinakaran]

On Tue, 2017-02-14 at 20:51 +0100, Daniel Vetter wrote:
> On Mon, Feb 13, 2017 at 10:26 PM, Pandiyan, Dhinakaran
>  wrote:
> > On Mon, 2017-02-13 at 09:05 +, Lankhorst, Maarten wrote:
> >> Pandiyan, Dhinakaran schreef op do 09-02-2017 om 18:55 [+]:
> >> > On Thu, 2017-02-09 at 09:01 +, Lankhorst, Maarten wrote:
> >> > >
> >> > > Dhinakaran Pandiyan schreef op wo 08-02-2017 om 22:38 [-0800]:
> >> > > >
> >> > > > Having a ->atomic_release callback is useful to release shared
> >> > > > resources
> >> > > > that get allocated in compute_config(). This function is expected
> >> > > > to
> >> > > > be
> >> > > > called in the atomic_check() phase before new resources are
> >> > > > acquired.
> >> > > >
> >> > > > v2: Moved the caller hunk to this patch (Daniel)
> >> > > >
> >> > > > Suggested-by: Daniel Vetter 
> >> > > > Signed-off-by: Dhinakaran Pandiyan  >> > > > >
> >> > > > ---
> >> > > >  drivers/gpu/drm/drm_atomic_helper.c  | 19
> >> > > > +++
> >> > > >  include/drm/drm_modeset_helper_vtables.h | 13 +
> >> > > >  2 files changed, 32 insertions(+)
> >> > > >
> >> > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> >> > > > b/drivers/gpu/drm/drm_atomic_helper.c
> >> > > > index 8795088..92bd741 100644
> >> > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> > > > @@ -576,6 +576,25 @@ drm_atomic_helper_check_modeset(struct
> >> > > > drm_device *dev,
> >> > > > }
> >> > > > }
> >> > > >
> >> > > > +   for_each_connector_in_state(state, connector,
> >> > > > connector_state, i) {
> >> > > > +   const struct drm_connector_helper_funcs
> >> > > > *conn_funcs;
> >> > > > +   struct drm_crtc_state *crtc_state;
> >> > > > +
> >> > > > +   conn_funcs = connector->helper_private;
> >> > > > +   if (!conn_funcs->atomic_release)
> >> > > > +   continue;
> >> > > > +
> >> > > > +   if (!connector->state->crtc)
> >> > > > +   continue;
> >> > > > +
> >> > > > +   crtc_state =
> >> > > > drm_atomic_get_existing_crtc_state(state, connector->state-
> >> > > > >crtc);
> >> > > > +
> >> > > > +   if (crtc_state->connectors_changed ||
> >> > > > +   crtc_state->mode_changed ||
> >> > > > +   (crtc_state->active_changed && !crtc_state-
> >> > > > >
> >> > > > > active))
> >> > > > +   conn_funcs->atomic_release(connector,
> >> > > > connector_state);
> >> > > > +   }
> >> > >
> >> > > Could we deal with the VCPI state separately in
> >> > > intel_modeset_checks,
> >> > > like we do with dpll?
> >> >
> >> > We'd want to release the VCPI slots before they are acquired in
> >> > ->compute_config(). intel_modeset_checks() will be too late to
> >> > release
> >> > them. Are you suggesting both acquiring and releasing slots should be
> >> > done in intel_modeset_checks()?
> >>
> >> That makes things a bit more nasty. Maybe add a
> >> conn_funcs->atomic_check that always gets called, something like I did
> >> below?
> >>
> >> I'd love to use it for some atomic connector properties too.
> >
> >
> > Adding and unconditionally calling conn_funcs->atomic_check() should be
> > doable. It also follows the pattern we have for encoders and CRTCs. But
> > I'll have to move the connector->state->crtc state checks inside the
> > function.
> 
> Adding ->atomic_check that's unconditionally called sounds troubling,
> because all the other ->atomic_check functions are _only_ called when
> enabling stuff. ->atomic_release sounds much better to me, and from a
> helper pov DK's patch above is the right place.
> 
> If that place doesn't work for i915.ko, then we need our own callback
> (like we already have with e.g. ->compute_config, we could do a
> ->release_config). But if it's just cosmetics, then I don't see the
> reason why we need to change this. On that issue: How exactly does our
> compute_config work if we haven't updated the routing (using the above
> helper) yet? That sounds like a pretty big misdesign on our side ...
> -Daniel
> 
Are you referring to the drm_atomic_helper_check_modeset() helper? We do
call it to update connector routing before compute_config .

-DK

> >
> > -DK
> >> > >
> >> > >
> >> > > Maybe implementing the relevant VCPI state could be done as an
> >> > > atomic
> >> > > helper function too, so other atomic drivers can just plug it in.
> >> > >
> >> > The idea was to reduce boilerplate in the drivers and use the
> >> > private_obj state for different object types.
> >> >
> >> >
> >> > >
> >> > > Not sure how doable this is, but if it's not too hard, then it's
> >> > > probably cleaner :)
> >> > > ___
> >> > > Intel-gfx mailing list
> >> > > intel-...@lists.freedesktop.org
> >> > > 

Re: [PATCH v2 3/6] drm: of: introduce drm_of_find_panel_or_bridge

[Rob Herring]

On Sun, Feb 12, 2017 at 10:47 PM, Archit Taneja  wrote:
>
>
> On 02/10/2017 12:35 AM, Rob Herring wrote:
>>
>> Many drivers have a common pattern of searching the OF graph for either an
>> attached panel or bridge and then finding the DRM struct for the panel
>> or bridge. Also, most drivers need to handle deferred probing when the
>> DRM device is not yet instantiated. Create a common function,
>> drm_of_find_panel_or_bridge, to find the connected node and the
>> associated DRM panel or bridge device.
>>
>> Signed-off-by: Rob Herring 
>> Acked-by: Philipp Zabel 
>> ---
>> v2:
>> - Reworked code flow
>> - Added note that at least one of panel or bridge must not be NULL.
>>
>>  drivers/gpu/drm/drm_of.c | 52
>> 
>>  include/drm/drm_of.h | 13 
>>  2 files changed, 65 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
>> index 47848ed8ca48..86b8b959587a 100644
>> --- a/drivers/gpu/drm/drm_of.c
>> +++ b/drivers/gpu/drm/drm_of.c
>> @@ -3,7 +3,9 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>> +#include 
>>  #include 
>>
>>  static void drm_release_of(struct device *dev, void *data)
>> @@ -207,3 +209,53 @@ int drm_of_encoder_active_endpoint(struct device_node
>> *node,
>> return -EINVAL;
>>  }
>>  EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint);
>> +
>> +/*
>> + * drm_of_find_panel_or_bridge - return connected panel or bridge device
>> + * @np: device tree node containing encoder input ports
>
>
> Should this be 'encoder output ports'? It's the encoder's output port(s)
> that
> contain the endpoint corresponding to the bridge/panel input port.

Yes, indeed.

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

Re: [PATCH 0/8] Refactor DC atomic commit and gamma

[Harry Wentland]

On 2017-02-14 04:30 PM, Daniel Vetter wrote:

On Fri, Feb 10, 2017 at 11:26:22AM -0500, Harry Wentland wrote:

Resending with CC to dri-devel as per Alex's suggestions. This
might be of interest to a wider audience.

These patches are first steps of addressing some of the problems
in DC's atomic implementation. Please take a look and provide
feedback if possible. Our hope is that we can start setting a
direction on fixing up DC to do atomic correctly and lay the
groundwork for moving past the midlayer.

THe biggest patch here is Andrey's work to bring atomic_commit
in line with the atomic helpers instead of rolling our own. We
got atomic_commmit_tail now and things appear to work correctly
with this change. It allowed us to clean up some of the commit
code, but there's still a lot left.

The second important patch is fixing up our gamma implementation
and correct the use of crtc_set_property and atomic_set_properties.

Beyond that there's some minor cleanup and support patches for
the above change.

The whole DC tree with these patches and rebased on drm-next a couple
days ago can be found at

https://cgit.freedesktop.org/~hwentland/linux/log/?h=dc-drm-next-atomic

Known issue:
  - corruption on one display in two-display setup


Props to amd for starting to submit core stuff and critical driver bits
for review, but since these are incremental patches a bit hard to
review ... Not sure what best to do, since I can't really justify
to my boss that I constantly look at the entire amdgpu-dal branch either.

Probably best if you folks ping me and others on irc with questions
directly, and then I try to sometimes take a look at the end result.
Probably best to wait until you've worked down the todo list for an area
though.
-Daniel



Makes sense. We'll bug you on IRC if we have any direct questions. 
Thanks for all the feedback to Andrey and steering some of the core work 
in a good direction, like the private atomic struct. I'm working on 
picking that one up next.


A lot of these changes are very much incremental. A lot of work here and 
we don't want to break things along the way.


Thanks,
Harry



Cheers,
Harry

Andrey Grodzovsky (3):
  drm/amdgpu: Add a few members to support DAL atomic refactor.
  drm/amd/display: Refactor atomic commit implementation.
  drm/amd/display: Refactor headless to use atomic commit.

Harry Wentland (5):
  drm/amdgpu: Expose mode_config functions for DM
  drm/amd/display: Use amdgpu mode funcs statically
  drm/amd/display: Use atomic helpers for gamma
  drm/amd/display: Remove unused define from amdgpu_dm_types
  Revert "drm/amdgpu: Refactor flip into prepare submit and submit.
(v3)"

 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c| 140 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h|  33 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h   |  19 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  70 ++-
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 548 +
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.h|  12 +-
 6 files changed, 341 insertions(+), 481 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h

--
2.9.3




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

Re: [Intel-gfx] [PATCH] drm: Reduce EDID warnings from DRM_ERROR to DRM_NOTE

[Daniel Vetter]

On Tue, Feb 14, 2017 at 09:43:45PM +, Chris Wilson wrote:
> On Tue, Feb 14, 2017 at 10:36:09PM +0100, Daniel Vetter wrote:
> > On Mon, Feb 13, 2017 at 12:17:27PM -0500, Sean Paul wrote:
> > > On Mon, Feb 13, 2017 at 3:59 AM, Chris Wilson  
> > > wrote:
> > > > On Mon, Feb 13, 2017 at 08:41:10AM +0100, Thierry Reding wrote:
> > > >> On Fri, Feb 10, 2017 at 07:59:13PM +, Chris Wilson wrote:
> > > >> > The warnings from parsing the EDID are not driver errors, but the
> > > >> > "normal but significant" conditions from the external device. As 
> > > >> > such,
> > > >> > they do not need the ferocity of an *ERROR*, but can use the less 
> > > >> > harsh
> > > >> > DRM_NOTE instead.
> > > >> >
> > > >> > Signed-off-by: Chris Wilson 
> > > >> > ---
> > > >> >  drivers/gpu/drm/drm_edid.c | 15 ---
> > > >> >  1 file changed, 8 insertions(+), 7 deletions(-)
> > > >>
> > > >> The below are all conditions that happen when the EDID is bad. I'm not
> > > >> sure that really qualifies as "normal".
> > > >
> > > > Often it is - a bad EDID on the monitor will always be bad. The
> > > > challenge is distinguishing that from silent data corruption during the
> > > > read - a reported read failure are trivial.
> > > >
> > > >> From a quick look through the code we don't always trigger an error 
> > > >> from
> > > >> the below failure paths at higher levels, so decreasing the level here
> > > >> has the potential to let this kind of exceptional condition go
> > > >> unnoticed.
> > > >
> > > > The messages are not gone, they are higher than the default loglevel,
> > > > but now below the level at which they are printed to a terminal. The
> > > > bad EDID is either expected or recoverable, and definitely not fatal
> > > > so I don't think an *ERROR* is justified.
> > > 
> > > I tend to agree.
> > > 
> > > The description for the KERN_NOTICE level is "normal but significant
> > > condition". I might argue that the presence of these EDID messages
> > > represents a normal *or* significant condition (depending on why the
> > > EDID is bad), but I don't think it's unreasonable to expect people to
> > > check their logs if the display/mode is not working properly.
> > 
> > So for cases where we know that there is shit hw out there (specifically
> > kvm switches that mangle the cea block without adjusting the edid) we
> > already tune down the error to debug level. So in principle totally agree
> > with tuning down anything that happens because it's outside of our control
> > to info or debug, but do we still need this patch after the cea one has
> > landed? Our CI at least seems happy ...
> 
> Yes. The one machine with a dodgy EDID also happens to have a dodgy
> BIOS. This reduces the number of consistent errors to 1, but since an
> unrelated error still remains, CI doesn't detect the improvement.
> https://intel-gfx-ci.01.org/CI/CI_DRM_2198/fi-skl-6700k/igt@drv_module_rel...@basic-reload.html

Ok, count my convinced, I pushed the patch to drm-misc-next.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 194579] AMDGPU: Possible size overflow detected by PaX in ttm_bo_handle_move_mem (drivers/gpu/drm/ttm/ttm_bo.c:388)

[bugzilla-daemon]

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

--- Comment #6 from PaX Team (pagee...@freemail.hu) ---
```
(In reply to Christian König from comment #4)
> Now back to topic, what code base is this? Could you post the branch
> you are using somewhere?

it's vanilla 4.9.9 + grsecurity (we don't change the amdgpu code except for
some function pointer correctness issues).

> In the upstream kernel the line at ttm_bo.c:388 is just an assignment
> and can't cause any overflow.

yet it does ;). here's the code in context, just to make sure we're talking
about the same thing:

   387 if (bo->mem.mm_node) {
   388 bo->offset = (bo->mem.start << PAGE_SHIFT) +
   389 bdev->man[bo->mem.mem_type].gpu_offset;
   390 bo->cur_placement = bo->mem.placement;

as you can see from the printk log, bo->mem.start was LONG_MAX (probably via
AMDGPU_BO_INVALID_OFFSET) and shifting it left triggered our size overflow
instrumentation. my guess is that AMDGPU_BO_INVALID_OFFSET isn't supposed to
be used in offset calculations and thus there's some higher level logic bug
here where the mem.start field should have been initialized by the time we
got here. some casual code browsing points at amdgpu_ttm_placement_init that
probably initializes the ttm_placement structures for amdgpu but for some of
them it doesn't set any of fpfn/lpfn/TTM_PL_FLAG_TOPDOWN which is what would
trigger the initialization of mem.start in amdgpu_gtt_mgr_new.
```

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99542] vdpau logging errors since gallium/radeon: adjust the rule for using the LINEAR_ALIGNED layout

[bugzilla-daemon]

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

Kai  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #9 from Kai  ---
Fix commited to master:

commit 0561b3c75af2e4bb216b686bf62ae9d89c584dc8
Author: Marek Olšák 
Date:   Sun Feb 12 15:48:48 2017 +0100

vdpau: skip vlVdpOutputSurfacePutBitsNative with a zero-area rectangle

This prevents errors:
"EE r600_texture.c:1571 r600_texture_transfer_map - failed to create
 temporary texture to hold untiled copy"

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

Tested-by: Kai Wasserbäch 
Reviewed-by: Kai Wasserbäch 
Reviewed-by: Christian König 

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 7/7] drm: Rename connector list iterator API

[Daniel Vetter]

On Fri, Feb 10, 2017 at 06:48:06PM +0100, Thierry Reding wrote:
> On Fri, Feb 10, 2017 at 06:14:14PM +0100, Thierry Reding wrote:
> [...]
> >  /**
> > - * drm_connector_list_iter_get - initialize a connector_list iterator
> > + * drm_connector_list_iter_begin - initialize a connector_list iterator
> >   * @dev: DRM device
> >   * @iter: connector_list iterator
> >   *
> >   * Sets @iter up to walk the _mode_config.connector_list of @dev. @iter
> > - * must always be cleaned up again by calling 
> > drm_connector_list_iter_put().
> > + * must always be cleaned up again by calling 
> > drm_connector_list_iter_end().
> >   * Iteration itself happens using drm_connector_list_iter_next() or
> >   * drm_for_each_connector_iter().
> >   */
> > -void drm_connector_list_iter_get(struct drm_device *dev,
> > -struct drm_connector_list_iter *iter)
> > +void drm_connector_list_iter_begin(struct drm_device *dev,
> > +  struct drm_connector_list_iter *iter)
> >  {
> > iter->dev = dev;
> > iter->conn = NULL;
> > lock_acquire_shared_recursive(_list_iter_dep_map, 0, 1, NULL, 
> > _RET_IP_);
> >  }
> > -EXPORT_SYMBOL(drm_connector_list_iter_get);
> > +EXPORT_SYMBOL(drm_connector_list_iter_end);
> 
> Erm... this should obviously have been drm_connector_list_iter_begin, no
> idea why the build tests didn't catch this on the first run.

With gcc properly appeased:

Reviewed-by: Daniel Vetter 

Since you have drm-misc commit rights, want to test-drive by pushing the
entire pile to drm-misc-next? Iirc you've tried it with 2 panel patches a
few months back, pls note that you need to update the dim script and
re-run dim setup, since things changed a bit with the split-out of the
drm-misc.git repo.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[Daniel Vetter]

On Tue, Feb 14, 2017 at 01:07:21PM -0800, John Stultz wrote:
> On Tue, Feb 14, 2017 at 12:32 PM, Daniel Stone  wrote:
> > Hi John,
> >
> > On 14 February 2017 at 19:25, John Stultz  wrote:
> >> +static enum drm_mode_status
> >> +drm_connector_check_crtc_modes(struct drm_connector *connector,
> >> +  struct drm_display_mode *mode)
> >> +{
> >> +   struct drm_device *dev = connector->dev;
> >> +   const struct drm_crtc_helper_funcs *crtc_funcs;
> >> +   struct drm_crtc *c;
> >> +
> >> +   if (mode->status != MODE_OK)
> >> +   return mode->status;
> >> +
> >> +   /* Check all the crtcs on a connector to make sure the mode is 
> >> valid */
> >> +   drm_for_each_crtc(c, dev) {
> >> +   crtc_funcs = c->helper_private;
> >> +   if (crtc_funcs && crtc_funcs->mode_valid)
> >> +   mode->status = crtc_funcs->mode_valid(c, mode);
> >> +   if (mode->status != MODE_OK)
> >> +   break;
> >> +   }
> >> +   return mode->status;
> >> +}
> >
> > Hm, that's unfortunate: it limits the mode list for every connector,
> > to those which are supported by every single CRTC. So if you have one
> > CRTC serving low-res LVDS, and another serving higher-res HDMI,
> > suddenly you can't get bigger modes on HDMI. The idea seems sound
> > enough, but a little more nuance might be good ...
> 
> Yea. That is not my intent at all I'm just trying to get the drm_crtc
> attached to the connector that we're getting the EDID mode lines from.
> I had tried going connector->encoder->crtc, but at the time this is
> called, the encoder is null. So Rob suggested the for_each_crtc(), and
> I guess I mistook that for being each crtc on the connector.
> 
> Thanks for pointing out this issue. From Daniel's feedback it looks
> like I need to start over from scratch though, so little worry this
> implementation will go much further.

Well your idea was somewhat right, but logic inverted. In ->mode_valid we
need to check whether any encoder/crtc combo could support the mode. Which
means you need to reject it only when there's no encoder/crtc combo that
could support the mode (you reject it if there's only one crtc which can't
handle it).

On the ->mode_fixup/atomic_check callbacks otoh we need to reject the mode
when it's not suitable for the current chain (as described in the atomic
states). That little difference is why this is not an entirely trivial
problem, and yes there's lots of hw out there where this matters.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH] drm: Reduce EDID warnings from DRM_ERROR to DRM_NOTE

[Chris Wilson]

On Tue, Feb 14, 2017 at 10:36:09PM +0100, Daniel Vetter wrote:
> On Mon, Feb 13, 2017 at 12:17:27PM -0500, Sean Paul wrote:
> > On Mon, Feb 13, 2017 at 3:59 AM, Chris Wilson  
> > wrote:
> > > On Mon, Feb 13, 2017 at 08:41:10AM +0100, Thierry Reding wrote:
> > >> On Fri, Feb 10, 2017 at 07:59:13PM +, Chris Wilson wrote:
> > >> > The warnings from parsing the EDID are not driver errors, but the
> > >> > "normal but significant" conditions from the external device. As such,
> > >> > they do not need the ferocity of an *ERROR*, but can use the less harsh
> > >> > DRM_NOTE instead.
> > >> >
> > >> > Signed-off-by: Chris Wilson 
> > >> > ---
> > >> >  drivers/gpu/drm/drm_edid.c | 15 ---
> > >> >  1 file changed, 8 insertions(+), 7 deletions(-)
> > >>
> > >> The below are all conditions that happen when the EDID is bad. I'm not
> > >> sure that really qualifies as "normal".
> > >
> > > Often it is - a bad EDID on the monitor will always be bad. The
> > > challenge is distinguishing that from silent data corruption during the
> > > read - a reported read failure are trivial.
> > >
> > >> From a quick look through the code we don't always trigger an error from
> > >> the below failure paths at higher levels, so decreasing the level here
> > >> has the potential to let this kind of exceptional condition go
> > >> unnoticed.
> > >
> > > The messages are not gone, they are higher than the default loglevel,
> > > but now below the level at which they are printed to a terminal. The
> > > bad EDID is either expected or recoverable, and definitely not fatal
> > > so I don't think an *ERROR* is justified.
> > 
> > I tend to agree.
> > 
> > The description for the KERN_NOTICE level is "normal but significant
> > condition". I might argue that the presence of these EDID messages
> > represents a normal *or* significant condition (depending on why the
> > EDID is bad), but I don't think it's unreasonable to expect people to
> > check their logs if the display/mode is not working properly.
> 
> So for cases where we know that there is shit hw out there (specifically
> kvm switches that mangle the cea block without adjusting the edid) we
> already tune down the error to debug level. So in principle totally agree
> with tuning down anything that happens because it's outside of our control
> to info or debug, but do we still need this patch after the cea one has
> landed? Our CI at least seems happy ...

Yes. The one machine with a dodgy EDID also happens to have a dodgy
BIOS. This reduces the number of consistent errors to 1, but since an
unrelated error still remains, CI doesn't detect the improvement.
https://intel-gfx-ci.01.org/CI/CI_DRM_2198/fi-skl-6700k/igt@drv_module_rel...@basic-reload.html
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[Daniel Vetter]

On Tue, Feb 14, 2017 at 01:03:27PM -0800, John Stultz wrote:
> On Tue, Feb 14, 2017 at 12:22 PM, Daniel Vetter  wrote:
> > On Tue, Feb 14, 2017 at 11:45:54AM -0800, John Stultz wrote:
> >>
> >> Not following here. What do you mean by "put it into drivers"?  Where?
> >
> > In your driver callback for ->mode_valid, call into a shared function to
> > validate CRTC limits. Which you also call from the crtc's ->mode_fixup
> > function.
> 
> So bascially have the adv7511 driver's mode_valid() have a special
> callback to the kirin (and freedreno, and whatever other) drm driver
> to check the modes? Or I guess the drm driver that uses that bridge
> should register something w/ the adv7511 code?
> 
> Part of my confusion here is that the main issue is that its not just
> one driver I'm dealing with, and they're all really just tied together
> via device tree, so I'm not sure how to special case it in just one of
> the drivers.

This sounds you want to fix this for bridges, but your patch only adds a
crtc callback?

> > In short my complain here is that this is only a partial solution of the
> > larger problem, specific for your driver. That kind of code is better put
> > into drivers, until we have a clear understanding to type up something
> > complete in the helpers. Shared code is imo overrated :-)
> 
> Yea, apologies for my not seeing the larger problem at first (its
> still a bit hazy, really), part of this submission is just trying to
> get something to throw darts at and figure out the right thing.
> 
> But I'll try to figure out another approach here.

Latest kerneldoc in drm-tip should explain this, not sure you didn't spot
it or looked at an old kernel version. For drm docs, _always_ look at
drm-tip, they're moving real fast :-)

https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#modeset-helper-reference-for-common-vtables

See e.g. drm_crtc_helpers_funcs->mode_fixup docs, it's there. There's
explanations sprinkled at various places (mode_valid, plus the different
helper funcs), probably good to first hunt these all down. Then read a
bunch of driver callbacks so you have a better idea of the patterns of
checks, otoh _lots_ of kms drivers get this wrong :(

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH RESEND] drm/dp/mst: fix kernel oops when turning off secondary monitor

[Daniel Vetter]

On Tue, Feb 14, 2017 at 02:49:21PM +0200, Jani Nikula wrote:
> From: Pierre-Louis Bossart 
> 
> 100% reproducible issue found on SKL SkullCanyon NUC with two external
> DP daisy-chained monitors in DP/MST mode. When turning off or changing
> the input of the second monitor the machine stops with a kernel
> oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
> 
> This issue is traced to an inconsistent control flow in
> drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the
> same time as 'req_payload.num_slots' is set to zero, but the pointer is
> dereferenced even when req_payload.num_slot is zero.
> 
> The problematic dereference was introduced in commit dfda0df34
> ("drm/mst: rework payload table allocation to conform better") and may
> impact all versions since v3.18
> 
> The fix suggested by Chris Wilson removes the kernel oops and was found to
> work well after 10mn of monkey-testing with the second monitor power and
> input buttons
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
> Fixes: dfda0df34264 ("drm/mst: rework payload table allocation to conform 
> better.")
> Cc: Dave Airlie 
> Cc: Chris Wilson 
> Cc: Nathan D Ciobanu 
> Cc: Dhinakaran Pandiyan 
> Cc: Sean Paul 
> Cc:  # v3.18+
> Tested-by: Nathan D Ciobanu 
> Reviewed-by: Dhinakaran Pandiyan 
> Signed-off-by: Pierre-Louis Bossart 
> Signed-off-by: Jani Nikula 

You haz drm-misc commit rights, pls use them :-)

Since it doesn't have deps, probably simplest to smash into drm-misc-fixes
and then send a pull req to Dave right away. If you want, you can roll
-fixes forward to -rc8 while at it.
-Daniel

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 122a1b04bebc..f2cc375907d0 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct 
> drm_dp_mst_topology_mgr *mgr)
>   mgr->payloads[i].vcpi = req_payload.vcpi;
>   } else if (mgr->payloads[i].num_slots) {
>   mgr->payloads[i].num_slots = 0;
> - drm_dp_destroy_payload_step1(mgr, port, 
> port->vcpi.vcpi, >payloads[i]);
> + drm_dp_destroy_payload_step1(mgr, port, 
> mgr->payloads[i].vcpi, >payloads[i]);
>   req_payload.payload_state = 
> mgr->payloads[i].payload_state;
>   mgr->payloads[i].start_slot = 0;
>   }
> -- 
> 2.1.4
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH] drm: Reduce EDID warnings from DRM_ERROR to DRM_NOTE

[Daniel Vetter]

On Mon, Feb 13, 2017 at 12:17:27PM -0500, Sean Paul wrote:
> On Mon, Feb 13, 2017 at 3:59 AM, Chris Wilson  
> wrote:
> > On Mon, Feb 13, 2017 at 08:41:10AM +0100, Thierry Reding wrote:
> >> On Fri, Feb 10, 2017 at 07:59:13PM +, Chris Wilson wrote:
> >> > The warnings from parsing the EDID are not driver errors, but the
> >> > "normal but significant" conditions from the external device. As such,
> >> > they do not need the ferocity of an *ERROR*, but can use the less harsh
> >> > DRM_NOTE instead.
> >> >
> >> > Signed-off-by: Chris Wilson 
> >> > ---
> >> >  drivers/gpu/drm/drm_edid.c | 15 ---
> >> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >>
> >> The below are all conditions that happen when the EDID is bad. I'm not
> >> sure that really qualifies as "normal".
> >
> > Often it is - a bad EDID on the monitor will always be bad. The
> > challenge is distinguishing that from silent data corruption during the
> > read - a reported read failure are trivial.
> >
> >> From a quick look through the code we don't always trigger an error from
> >> the below failure paths at higher levels, so decreasing the level here
> >> has the potential to let this kind of exceptional condition go
> >> unnoticed.
> >
> > The messages are not gone, they are higher than the default loglevel,
> > but now below the level at which they are printed to a terminal. The
> > bad EDID is either expected or recoverable, and definitely not fatal
> > so I don't think an *ERROR* is justified.
> 
> I tend to agree.
> 
> The description for the KERN_NOTICE level is "normal but significant
> condition". I might argue that the presence of these EDID messages
> represents a normal *or* significant condition (depending on why the
> EDID is bad), but I don't think it's unreasonable to expect people to
> check their logs if the display/mode is not working properly.

So for cases where we know that there is shit hw out there (specifically
kvm switches that mangle the cea block without adjusting the edid) we
already tune down the error to debug level. So in principle totally agree
with tuning down anything that happens because it's outside of our control
to info or debug, but do we still need this patch after the cea one has
landed? Our CI at least seems happy ...

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/atmel-hlcdc: Simplify the HLCDC layer logic

[Daniel Vetter]

On Fri, Feb 10, 2017 at 07:07:45PM +0100, Boris Brezillon wrote:
> An HLCDC layers in Atmel's nomenclature is either a DRM plane or a 'Post
> Processing Layer' which can be used to output the results of the HLCDC
> composition in a memory buffer.
> 
> atmel_hlcdc_layer.c was designed to be generic enough to be re-usable in
> both cases, but we're not exposing the post-processing layer yet, and
> even if we were, I'm not sure the code would provide the necessary tools
> to manipulate this kind of layer.
> 
> Moreover, the code in atmel_hlcdc_{plane,layer}.c was designed before the
> atomic modesetting API, and was trying solve the
> check-setting/commit-if-ok/rollback-otherwise problem, which is now
> entirely solved by the existing core infrastructure.
> 
> And finally, the code in atmel_hlcdc_layer.c in over-complicated compared
> to what we really need. This rework is a good excuse to simplify it. Note
> that this rework solves an existing resource leak (leading to a -EBUSY
> error) which I failed to clearly identify.
> 
> Signed-off-by: Boris Brezillon 
> ---
> Hi Daniel,
> 
> I intentionally dropped your ack, since inheriting from atmel_hlcdc_layer
> is implying a lot of changes.

Well I acked the idea, that still kinda holds. But if you want to
kickstart the drm-misc driver ack economy, Eric has 1-2 vc4 patches that
still need an ack, you could trade r-bs :-)

Cheers, Daniel

> 
> Regards,
> 
> Boris
> 
> Changes in v2:
> - make atmel_hlcdc_plane inherit from atmel_hlcdc_layer
> - provide read/write_reg/cfg() helpers to access layer regs
> - move all layer related definitions into atmel_hlcdc_dc.h and remove
>   atmel_hlcdc_layer.h
> - fix a bug in atmel_hlcdc_plane_atomic_duplicate_state()
> ---
>  drivers/gpu/drm/atmel-hlcdc/Makefile|   1 -
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  39 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c|  82 +--
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h| 364 +++--
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c | 666 
> 
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h | 399 --
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 637 +++---
>  7 files changed, 695 insertions(+), 1493 deletions(-)
>  delete mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c
>  delete mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/Makefile 
> b/drivers/gpu/drm/atmel-hlcdc/Makefile
> index 10ae426e60bd..bb5f8507a8ce 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/Makefile
> +++ b/drivers/gpu/drm/atmel-hlcdc/Makefile
> @@ -1,6 +1,5 @@
>  atmel-hlcdc-dc-y := atmel_hlcdc_crtc.o \
>   atmel_hlcdc_dc.o \
> - atmel_hlcdc_layer.o \
>   atmel_hlcdc_output.o \
>   atmel_hlcdc_plane.o
>  
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 9b17a66cf0e1..2fcec0a72567 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -445,8 +445,8 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs 
> = {
>  
>  int atmel_hlcdc_crtc_create(struct drm_device *dev)
>  {
> + struct atmel_hlcdc_plane *primary = NULL, *cursor = NULL;
>   struct atmel_hlcdc_dc *dc = dev->dev_private;
> - struct atmel_hlcdc_planes *planes = dc->planes;
>   struct atmel_hlcdc_crtc *crtc;
>   int ret;
>   int i;
> @@ -457,20 +457,41 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev)
>  
>   crtc->dc = dc;
>  
> - ret = drm_crtc_init_with_planes(dev, >base,
> - >primary->base,
> - planes->cursor ? >cursor->base : NULL,
> - _hlcdc_crtc_funcs, NULL);
> + for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
> + if (!dc->layers[i])
> + continue;
> +
> + switch (dc->layers[i]->desc->type) {
> + case ATMEL_HLCDC_BASE_LAYER:
> + primary = atmel_hlcdc_layer_to_plane(dc->layers[i]);
> + break;
> +
> + case ATMEL_HLCDC_CURSOR_LAYER:
> + cursor = atmel_hlcdc_layer_to_plane(dc->layers[i]);
> + break;
> +
> + default:
> + break;
> + }
> + }
> +
> + ret = drm_crtc_init_with_planes(dev, >base, >base,
> + >base, _hlcdc_crtc_funcs,
> + NULL);
>   if (ret < 0)
>   goto fail;
>  
>   crtc->id = drm_crtc_index(>base);
>  
> - if (planes->cursor)
> - planes->cursor->base.possible_crtcs = 1 << crtc->id;
> + for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
> + struct atmel_hlcdc_plane *overlay;
>  
> - for (i = 0; i < 

[PATCH] drm/rockchip: add extcon dependency for DP

[Arnd Bergmann]

The newly added DP driver links against the extcon core, which fails when
extcon is a module and this driver is not:

drivers/gpu/drm/rockchip/cdn-dp-core.o: In function `cdn_dp_get_port_lanes':
cdn-dp-core.c:(.text.cdn_dp_get_port_lanes+0x24): undefined reference to 
`extcon_get_state'
cdn-dp-core.c:(.text.cdn_dp_get_port_lanes+0x44): undefined reference to 
`extcon_get_property'

Let's make Kconfig enforce correct behavior with a dependency.

Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/rockchip/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index ad31b3eb408f..0e4eb845cbb0 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -24,6 +24,7 @@ config ROCKCHIP_ANALOGIX_DP
 config ROCKCHIP_CDN_DP
 tristate "Rockchip cdn DP"
 depends on DRM_ROCKCHIP
+   depends on EXTCON
select SND_SOC_HDMI_CODEC if SND_SOC
 help
  This selects support for Rockchip SoC specific extensions
-- 
2.9.0

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

Re: [PATCH 0/8] Refactor DC atomic commit and gamma

[Daniel Vetter]

On Fri, Feb 10, 2017 at 11:26:22AM -0500, Harry Wentland wrote:
> Resending with CC to dri-devel as per Alex's suggestions. This
> might be of interest to a wider audience.
> 
> These patches are first steps of addressing some of the problems
> in DC's atomic implementation. Please take a look and provide
> feedback if possible. Our hope is that we can start setting a
> direction on fixing up DC to do atomic correctly and lay the
> groundwork for moving past the midlayer.
> 
> THe biggest patch here is Andrey's work to bring atomic_commit
> in line with the atomic helpers instead of rolling our own. We
> got atomic_commmit_tail now and things appear to work correctly
> with this change. It allowed us to clean up some of the commit
> code, but there's still a lot left.
> 
> The second important patch is fixing up our gamma implementation
> and correct the use of crtc_set_property and atomic_set_properties.
> 
> Beyond that there's some minor cleanup and support patches for
> the above change.
> 
> The whole DC tree with these patches and rebased on drm-next a couple
> days ago can be found at 
> 
> https://cgit.freedesktop.org/~hwentland/linux/log/?h=dc-drm-next-atomic
> 
> Known issue:
>   - corruption on one display in two-display setup

Props to amd for starting to submit core stuff and critical driver bits
for review, but since these are incremental patches a bit hard to
review ... Not sure what best to do, since I can't really justify
to my boss that I constantly look at the entire amdgpu-dal branch either.

Probably best if you folks ping me and others on irc with questions
directly, and then I try to sometimes take a look at the end result.
Probably best to wait until you've worked down the todo list for an area
though.
-Daniel

> 
> Cheers,
> Harry
> 
> Andrey Grodzovsky (3):
>   drm/amdgpu: Add a few members to support DAL atomic refactor.
>   drm/amd/display: Refactor atomic commit implementation.
>   drm/amd/display: Refactor headless to use atomic commit.
> 
> Harry Wentland (5):
>   drm/amdgpu: Expose mode_config functions for DM
>   drm/amd/display: Use amdgpu mode funcs statically
>   drm/amd/display: Use atomic helpers for gamma
>   drm/amd/display: Remove unused define from amdgpu_dm_types
>   Revert "drm/amdgpu: Refactor flip into prepare submit and submit.
> (v3)"
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c| 140 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.h|  33 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h   |  19 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  70 ++-
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 548 
> +
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.h|  12 +-
>  6 files changed, 341 insertions(+), 481 deletions(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
> 
> -- 
> 2.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/2] drm/cma-helper: Add multi buffer support for cma fbdev

[Laurent Pinchart]

Hi Daniel,

On Tuesday 14 Feb 2017 21:09:51 Daniel Vetter wrote:
> On Mon, Feb 13, 2017 at 11:20:51AM +, Daniel Stone wrote:
> > On 13 February 2017 at 10:54, Maxime Ripard wrote:
> >> On Sun, Feb 12, 2017 at 02:28:11PM +0200, Laurent Pinchart wrote:
> >>> On Thursday 02 Feb 2017 11:31:56 Maxime Ripard wrote:
>  This patch add a config to support to create multi buffer for cma
>  fbdev. Such as double buffer and triple buffer.
>  
>  Cma fbdev is convient to add a legency fbdev. And still many Android
>  devices use fbdev now and at least double buffer is needed for these
>  Android devices, so that a buffer flip can be operated. It will need
>  some time for Android device vendors to abondon legency fbdev. So
>  multi buffer for fbdev is needed.
> >>> 
> >>> How exactly do we expect Android to move away from fbdev if we add
> >>> features to the fbdev compat layer ? I'd much rather make it clear to
> >>> them that fbdev is a thing from the past and that they'd better
> >>> migrate now.
> >> 
> >> If your point is that merging this patch will slow down the Android
> >> move away from fbdev, I disagree with that (obviously).
> >> 
> >> I don't care at all about Android on my platform of choice, but don't
> >> see how that merging this patch will change anything.
> >> 
> >> Let's be honest, Android trees typically have thousands of patches on
> >> top of mainline. Do you think a simple, 15 LoC, patch will make any
> >> difference to vendors? If they want to stay on fbdev and have that
> >> feature, they'll just merge this patch, done.
> > 
> > So, in that case, why not just let them do that? They'd already have
> > to add patches to use this, surely; we don't have anything in mainline
> > kernels which allows people to actually use this larger allocation.
> > Apart from software mmap() and using panning to do flips, but I'm
> > taking it as a given that people shipping Android on their devices
> > aren't using software rendering.
> 
> I think we need to make a distinction between fbdev the subsystem in the
> kernel, and fbdev the uabi:
> 
> - fbdev the subsystem is completely dead in upstream. I think we have full
>   agreement on that.
> - fbdev the uabi isn't, and if we can get more users from fbdev based
>   drivers to kms/atomic drivers by adding fairly simple stuff like this,
>   I'm all for it.

The real question, in my opinion, is how to get more users for the DRM/KMS 
userspace API, to help killing the fbdev API. What's the incentive for 
userspace to migrate if we tell them that we're going to support the fbdev API 
forever, and will even go through the trouble of extending the supported 
feature set ? I have a customer who wouldn't have migrated their userspace to 
DRM/KMS if these two patches had been merged a few years ago. I'd rather 
*reduce* the supported feature set over time until we can finally switch fbdev 
off.

> Which means: Yes, I fully plan to merge this, it makes sense. It even
> _helps_ by making fbdev-the-subsystem even deader. Making live hard for
> out-of-tree folks or folks with shit userspace doesn't make sense, at
> least if the only benefit for us is that we'll feel pure about our
> intentions :-)

-- 
Regards,

Laurent Pinchart

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

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[John Stultz]

On Tue, Feb 14, 2017 at 12:32 PM, Daniel Stone  wrote:
> Hi John,
>
> On 14 February 2017 at 19:25, John Stultz  wrote:
>> +static enum drm_mode_status
>> +drm_connector_check_crtc_modes(struct drm_connector *connector,
>> +  struct drm_display_mode *mode)
>> +{
>> +   struct drm_device *dev = connector->dev;
>> +   const struct drm_crtc_helper_funcs *crtc_funcs;
>> +   struct drm_crtc *c;
>> +
>> +   if (mode->status != MODE_OK)
>> +   return mode->status;
>> +
>> +   /* Check all the crtcs on a connector to make sure the mode is valid 
>> */
>> +   drm_for_each_crtc(c, dev) {
>> +   crtc_funcs = c->helper_private;
>> +   if (crtc_funcs && crtc_funcs->mode_valid)
>> +   mode->status = crtc_funcs->mode_valid(c, mode);
>> +   if (mode->status != MODE_OK)
>> +   break;
>> +   }
>> +   return mode->status;
>> +}
>
> Hm, that's unfortunate: it limits the mode list for every connector,
> to those which are supported by every single CRTC. So if you have one
> CRTC serving low-res LVDS, and another serving higher-res HDMI,
> suddenly you can't get bigger modes on HDMI. The idea seems sound
> enough, but a little more nuance might be good ...

Yea. That is not my intent at all I'm just trying to get the drm_crtc
attached to the connector that we're getting the EDID mode lines from.
I had tried going connector->encoder->crtc, but at the time this is
called, the encoder is null. So Rob suggested the for_each_crtc(), and
I guess I mistook that for being each crtc on the connector.

Thanks for pointing out this issue. From Daniel's feedback it looks
like I need to start over from scratch though, so little worry this
implementation will go much further.

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

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[John Stultz]

On Tue, Feb 14, 2017 at 12:22 PM, Daniel Vetter  wrote:
> On Tue, Feb 14, 2017 at 11:45:54AM -0800, John Stultz wrote:
>>
>> Not following here. What do you mean by "put it into drivers"?  Where?
>
> In your driver callback for ->mode_valid, call into a shared function to
> validate CRTC limits. Which you also call from the crtc's ->mode_fixup
> function.

So bascially have the adv7511 driver's mode_valid() have a special
callback to the kirin (and freedreno, and whatever other) drm driver
to check the modes? Or I guess the drm driver that uses that bridge
should register something w/ the adv7511 code?

Part of my confusion here is that the main issue is that its not just
one driver I'm dealing with, and they're all really just tied together
via device tree, so I'm not sure how to special case it in just one of
the drivers.

> In short my complain here is that this is only a partial solution of the
> larger problem, specific for your driver. That kind of code is better put
> into drivers, until we have a clear understanding to type up something
> complete in the helpers. Shared code is imo overrated :-)

Yea, apologies for my not seeing the larger problem at first (its
still a bit hazy, really), part of this submission is just trying to
get something to throw darts at and figure out the right thing.

But I'll try to figure out another approach here.

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

Re: [PATCH] drm/nouveau/core: recognise GP107 chipset

[Ilia Mirkin]

I believe what's missing at this point is a mmiotrace of the NVIDIA
driver to make sure that there's nothing different about this GPU. If
you can make one (see https://wiki.ubuntu.com/X/MMIOTracing for a
guide - should end up ~100MB uncompressed), please send a compressed
one to mmio.du...@gmail.com or make available some other way.

On Tue, Feb 14, 2017 at 2:34 PM, Daniel Drake  wrote:
> From: Chris Chiu 
>
> This new graphics card was failing to initialize with nouveau due to
> an "unknown chipset" error.
>
> Copy the GP106 configuration and rename for GP107/NV137. We don't
> know for certain that this is fully correct, but brief desktop testing
> suggests this is working fine.
>
> Signed-off-by: Chris Chiu 
> Signed-off-by: Daniel Drake 
> ---
>  drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 29 
> +++
>  1 file changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c 
> b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> index fea30d6..d242431 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> @@ -2237,6 +2237,34 @@ nv136_chipset = {
> .fifo = gp100_fifo_new,
>  };
>
> +static const struct nvkm_device_chip
> +nv137_chipset = {
> +   .name = "GP107",
> +   .bar = gf100_bar_new,
> +   .bios = nvkm_bios_new,
> +   .bus = gf100_bus_new,
> +   .devinit = gm200_devinit_new,
> +   .fb = gp104_fb_new,
> +   .fuse = gm107_fuse_new,
> +   .gpio = gk104_gpio_new,
> +   .i2c = gm200_i2c_new,
> +   .ibus = gm200_ibus_new,
> +   .imem = nv50_instmem_new,
> +   .ltc = gp100_ltc_new,
> +   .mc = gp100_mc_new,
> +   .mmu = gf100_mmu_new,
> +   .pci = gp100_pci_new,
> +   .timer = gk20a_timer_new,
> +   .top = gk104_top_new,
> +   .ce[0] = gp104_ce_new,
> +   .ce[1] = gp104_ce_new,
> +   .ce[2] = gp104_ce_new,
> +   .ce[3] = gp104_ce_new,
> +   .disp = gp104_disp_new,
> +   .dma = gf119_dma_new,
> +   .fifo = gp100_fifo_new,
> +};
> +
>  static int
>  nvkm_device_event_ctor(struct nvkm_object *object, void *data, u32 size,
>struct nvkm_notify *notify)
> @@ -2673,6 +2701,7 @@ nvkm_device_ctor(const struct nvkm_device_func *func,
> case 0x130: device->chip = _chipset; break;
> case 0x134: device->chip = _chipset; break;
> case 0x136: device->chip = _chipset; break;
> +   case 0x137: device->chip = _chipset; break;
> default:
> nvdev_error(device, "unknown chipset (%08x)\n", 
> boot0);
> goto done;
> --
> 2.9.3
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[Daniel Stone]

Hi John,

On 14 February 2017 at 19:25, John Stultz  wrote:
> +static enum drm_mode_status
> +drm_connector_check_crtc_modes(struct drm_connector *connector,
> +  struct drm_display_mode *mode)
> +{
> +   struct drm_device *dev = connector->dev;
> +   const struct drm_crtc_helper_funcs *crtc_funcs;
> +   struct drm_crtc *c;
> +
> +   if (mode->status != MODE_OK)
> +   return mode->status;
> +
> +   /* Check all the crtcs on a connector to make sure the mode is valid 
> */
> +   drm_for_each_crtc(c, dev) {
> +   crtc_funcs = c->helper_private;
> +   if (crtc_funcs && crtc_funcs->mode_valid)
> +   mode->status = crtc_funcs->mode_valid(c, mode);
> +   if (mode->status != MODE_OK)
> +   break;
> +   }
> +   return mode->status;
> +}

Hm, that's unfortunate: it limits the mode list for every connector,
to those which are supported by every single CRTC. So if you have one
CRTC serving low-res LVDS, and another serving higher-res HDMI,
suddenly you can't get bigger modes on HDMI. The idea seems sound
enough, but a little more nuance might be good ...

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

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[Daniel Vetter]

On Tue, Feb 14, 2017 at 11:45:54AM -0800, John Stultz wrote:
> On Tue, Feb 14, 2017 at 11:38 AM, Daniel Vetter  wrote:
> > On Tue, Feb 14, 2017 at 8:25 PM, John Stultz  wrote:
> >> Currently, on the hikey board, we have the adv7511 bridge wired
> >> up to the kirin ade drm driver. Unfortunately, the kirin ade
> >> core cannot generate accurate byteclocks for all pixel clock
> >> values.
> >>
> >> Thus if a mode clock is selected that we cannot calculate a
> >> matching byteclock, the device will boot with a blank screen.
> >>
> >> Unfortunately, currently the only place we can properly check
> >> potential modes for this issue in the connector mode_valid
> >> helper. Again, hikey uses the adv7511 bridge, which is shared
> >> between a number of different devices, so its improper to put
> >> restrictions caused by the kirin drm driver in the adv7511
> >> logic.
> >>
> >> So this patch tries to correct for that, by adding some
> >> infrastructure so that the drm_crtc_helper_funcs can optionally
> >> implement a mode_valid check, so that the probe helpers can
> >> check to make sure there are not any restrictions at the crtc
> >> level as well.
> >>
> >> Cc: Daniel Vetter 
> >> Cc: Jani Nikula 
> >> Cc: Sean Paul 
> >> Cc: David Airlie 
> >> Cc: Rob Clark 
> >> Cc: Xinliang Liu 
> >> Cc: Xinliang Liu 
> >> Cc: Rongrong Zou 
> >> Cc: Xinwei Kong 
> >> Cc: Chen Feng 
> >> Cc: Archit Taneja 
> >> Cc: dri-devel@lists.freedesktop.org
> >> Signed-off-by: John Stultz 
> >
> > So I'm going to be super-annoying here and ask for a complete
> > solution. This here is defacto what ever driver already does (or has
> > too), but it doesn't really solve the overall issue of having entirely
> > separate validation paths for probe and atomic_check paths. I think if
> > we wan to solve this, we need to solve this properly, with a generic
> > solution. That would mean:
> > - still in helpers, to make it all opt-int
> 
> Just to be clear, I believe what I proposed is opt-in, but I assume
> you want that in addition to the following, right?
> 
> > - covers crtc and encoders and bridges
> 
> So you'd like similar mode_valid() calls in the crtc/encoder/bridge
> helpers? right?
> 
> > - allows you to implement the current mode_valid in terms of the new
> > stuff (maybe as a default hook)
> 
> This bit I'm not sure I'm following. The current drm_connector's
> mode_valid in terms of a new mode_valid call that also looks at
> crtc/encoder/bridges? Or do you mean something else?
> 
> > - allows you to implement the current assortment of mode_fixup and/or
> > atomic_check in terms of the new stuff, or at least to not have to
> > duplicate logic in there
> 
> This is over my head, so I'll have to research to better understand.
> 
> > Docs for all this, especially updating all the warnings on how to use
> > the existing hooks correctly.
> 
> That's fair.
> 
> > I think just pushing this specific case into the helpers, without
> > rethinking the overall big picture, isn't gaining us all that much.
> > For just this I'd say just put it into drivers, until we have some
> 
> Not following here. What do you mean by "put it into drivers"?  Where?

In your driver callback for ->mode_valid, call into a shared function to
validate CRTC limits. Which you also call from the crtc's ->mode_fixup
function.

In short my complain here is that this is only a partial solution of the
larger problem, specific for your driver. That kind of code is better put
into drivers, until we have a clear understanding to type up something
complete in the helpers. Shared code is imo overrated :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: ensure atomic messages consistently include the name of the component

[Daniel Vetter]

On Mon, Feb 13, 2017 at 04:47:01PM +, Russell King - ARM Linux wrote:
> On Mon, Feb 13, 2017 at 02:37:31PM +0100, Maarten Lankhorst wrote:
> > All for it, looks sane. The last hunk fails to apply because it's based on
> > an older version of page_flip, but easy enough to fix.
> 
> Yes, it's based on v4.10-rc7.

Resolved conflict and merged for 4.12 into drm-misc-next.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99815] Power management problems & kernel hangs with Cap Verde

[bugzilla-daemon]

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

--- Comment #1 from Klaus Kusche  ---
I just checked: It seems that the problem does not depend on two active
outputs.
The gpu consumes roughly the same amount of power with just the laptop display.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH -next] drm/msm/dsi: fix error return code in msm_dsi_host_init()

[Daniel Vetter]

On Thu, Feb 09, 2017 at 03:19:07PM +, Wei Yongjun wrote:
> From: Wei Yongjun 
> 
> Fix to return error code -ENOMEM from the malloc error handling
> case instead of 0, as done elsewhere in this function.
> 
> Signed-off-by: Wei Yongjun 

Applied for 4.12 to drm-misc-next, thanks.
-Daniel

> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 1fc07ce..4f79b10 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1740,6 +1740,7 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
>  
>   msm_host->rx_buf = devm_kzalloc(>dev, SZ_4K, GFP_KERNEL);
>   if (!msm_host->rx_buf) {
> + ret = -ENOMEM;
>   pr_err("%s: alloc rx temp buf failed\n", __func__);
>   goto fail;
>   }
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}()

[Daniel Vetter]

On Thu, Feb 09, 2017 at 09:55:22PM +, Emil Velikov wrote:
> On 9 February 2017 at 20:41, Thierry Reding  wrote:
> > On Thu, Feb 09, 2017 at 06:08:10PM +0100, Daniel Vetter wrote:
> >> On Wed, Feb 08, 2017 at 02:28:14PM -0500, Sean Paul wrote:
> >> > On Wed, Feb 08, 2017 at 07:24:04PM +0100, Thierry Reding wrote:
> >> > > From: Thierry Reding 
> >> > >
> >> > > For consistency with other reference counting APIs in the kernel, add
> >> > > drm_mode_object_get() and drm_mode_object_put() to reference count DRM
> >> > > mode objects.
> >> > >
> >> > > Compatibility aliases are added to keep existing code working. To help
> >> > > speed up the transition, all the instances of the old functions in the
> >> > > DRM core are already replaced in this commit.
> >> > >
> >> >
> >> > drm code looks good and is
> >> >
> >> > Reviewed-by: Sean Paul 
> >> >
> >> > > A semantic patch is provided that can be used to convert all drivers to
> >> > > the new helpers.
> >> >
> >> > I'm not convinced we need to commit the cocci patch. I think including 
> >> > it in
> >> > your cover letter and then following up with a follow on series to 
> >> > actually make
> >> > the change is sufficient (See: ickle's s/fence/dma_fence/ series).
> >>
> >> Yeah, if you do a large-scale refactor anyway, I think it's best to just
> >> store the cocci in the commit message. I think storing the cocci is ok if
> >> you have thousands of hits among lots of subsystems, and it's clear it's
> >> going to take at least a few release cycles or maybe even years to clean
> >> it all up. drm is luckily not yet that big :-)
> >>
> >> I'll drop this while applying if no one minds ...
> >
> > I thought it was actually quite nice that this was part of the series.
> > That way it doesn't get lost and it is really easy to rerun. Also it can
> > trivially be removed once we've converted everyone to the new functions
> > and removed the old ones.
> >
> Hidden bonus:
> Some of the people who run those on semi-regular basis can update
> some/all the drivers ;-)

I agree that this is a nice bonus, but thus far we just mass-converted
everyone. That seems to be the plan here too, which is why I don't see
much value in recording the cocci patch (outside of the commit message).

But drm is growing, and in the future I guess we'll get more refactorings
that can't be done in one go, and then adding the spatch makes perfect
sense.

Anyway, no strong opinions from my side at all, whatever makes sense, just
wanted to explain my reasoning quickly.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/2] drm/cma-helper: Add multi buffer support for cma fbdev

[Daniel Vetter]

On Mon, Feb 13, 2017 at 11:20:51AM +, Daniel Stone wrote:
> Hi Maxime,
> 
> On 13 February 2017 at 10:54, Maxime Ripard
>  wrote:
> > On Sun, Feb 12, 2017 at 02:28:11PM +0200, Laurent Pinchart wrote:
> >> On Thursday 02 Feb 2017 11:31:56 Maxime Ripard wrote:
> >> > This patch add a config to support to create multi buffer for cma fbdev.
> >> > Such as double buffer and triple buffer.
> >> >
> >> > Cma fbdev is convient to add a legency fbdev. And still many Android
> >> > devices use fbdev now and at least double buffer is needed for these
> >> > Android devices, so that a buffer flip can be operated. It will need
> >> > some time for Android device vendors to abondon legency fbdev. So multi
> >> > buffer for fbdev is needed.
> >>
> >> How exactly do we expect Android to move away from fbdev if we add 
> >> features to
> >> the fbdev compat layer ? I'd much rather make it clear to them that fbdev 
> >> is a
> >> thing from the past and that they'd better migrate now.
> >
> > If your point is that merging this patch will slow down the Android
> > move away from fbdev, I disagree with that (obviously).
> >
> > I don't care at all about Android on my platform of choice, but don't
> > see how that merging this patch will change anything.
> >
> > Let's be honest, Android trees typically have thousands of patches on
> > top of mainline. Do you think a simple, 15 LoC, patch will make any
> > difference to vendors? If they want to stay on fbdev and have that
> > feature, they'll just merge this patch, done.
> 
> So, in that case, why not just let them do that? They'd already have
> to add patches to use this, surely; we don't have anything in mainline
> kernels which allows people to actually use this larger allocation.
> Apart from software mmap() and using panning to do flips, but I'm
> taking it as a given that people shipping Android on their devices
> aren't using software rendering.

I think we need to make a distinction between fbdev the subsystem in the
kernel, and fbdev the uabi:

- fbdev the subsystem is completely dead in upstream. I think we have full
  agreement on that.
- fbdev the uabi isn't, and if we can get more users from fbdev based
  drivers to kms/atomic drivers by adding fairly simple stuff like this,
  I'm all for it.

Which means: Yes, I fully plan to merge this, it makes sense. It even
_helps_ by making fbdev-the-subsystem even deader. Making live hard for
out-of-tree folks or folks with shit userspace doesn't make sense, at
least if the only benefit for us is that we'll feel pure about our
intentions :-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/7] drm/atomic: Use new atomic iterator macros.

[Laurent Pinchart]

Hi Daniel,

On Tuesday 14 Feb 2017 21:03:07 Daniel Vetter wrote:
> On Tue, Jan 17, 2017 at 01:55:50AM +0200, Laurent Pinchart wrote:
> > On Monday 16 Jan 2017 10:37:40 Maarten Lankhorst wrote:
> > 
> > Could we please get a description ? Apart from that,
> 
> Typed something small and merged the first 3 patches from this series.

Thanks. 4 more patches to go. Maarten ? :-)

> > Reviewed-by: Laurent Pinchart 
> > 
> > > Signed-off-by: Maarten Lankhorst 
> > > ---
> > > 
> > >  drivers/gpu/drm/drm_atomic.c | 16 
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 18cdf2c956c6..7b61e0da9ace 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -1562,7 +1562,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > > *state)
> > > 
> > >   DRM_DEBUG_ATOMIC("checking %p\n", state);
> > > 
> > > - for_each_plane_in_state(state, plane, plane_state, i) {
> > > + for_each_new_plane_in_state(state, plane, plane_state, i) {
> > > 
> > >   ret = drm_atomic_plane_check(plane, plane_state);
> > >   if (ret) {
> > >   
> > >   DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check
> > 
> > failed\n",
> > 
> > > @@ -1571,7 +1571,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > > *state) }
> > > 
> > >   }
> > > 
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > > 
> > >   ret = drm_atomic_crtc_check(crtc, crtc_state);
> > >   if (ret) {
> > >   
> > >   DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check
> > 
> > failed\n",
> > 
> > > @@ -1584,7 +1584,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > > *state) ret = config->funcs->atomic_check(state->dev, state);
> > > 
> > >   if (!state->allow_modeset) {
> > > 
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > > 
> > >   if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> > >   
> > >   DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full
> > 
> > modeset\n",
> > 
> > >crtc->base.id, crtc->name);
> > > 
> > > @@ -1668,13 +1668,13 @@ static void drm_atomic_print_state(const struct
> > > drm_atomic_state *state)
> > > 
> > >   DRM_DEBUG_ATOMIC("checking %p\n", state);
> > > 
> > > - for_each_plane_in_state(state, plane, plane_state, i)
> > > + for_each_new_plane_in_state(state, plane, plane_state, i)
> > > 
> > >   drm_atomic_plane_print_state(, plane_state);
> > > 
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i)
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> > > 
> > >   drm_atomic_crtc_print_state(, crtc_state);
> > > 
> > > - for_each_connector_in_state(state, connector, connector_state, i)
> > > + for_each_new_connector_in_state(state, connector, connector_state, i)
> > > 
> > >   drm_atomic_connector_print_state(, connector_state);
> > >  
> > >  }
> > > 
> > > @@ -1961,7 +1961,7 @@ static int prepare_crtc_signaling(struct
> > > drm_device
> > > *dev, if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
> > > 
> > >   return 0;
> > > 
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > > 
> > >   u64 __user *fence_ptr;
> > >   
> > >   fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
> > > 
> > > @@ -2041,7 +2041,7 @@ static void complete_crtc_signaling(struct
> > > drm_device
> > > *dev, return;
> > > 
> > >   }
> > > 
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > > 
> > >   /*
> > >   
> > >* TEST_ONLY and PAGE_FLIP_EVENT are mutually
> > >* exclusive, if they weren't, this code should be

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] drm: change connector disconnected debug message to an error

[Daniel Vetter]

On Fri, Feb 10, 2017 at 09:29:07AM -0700, Shuah Khan wrote:
> On 02/03/2017 01:06 AM, Daniel Vetter wrote:
> > On Thu, Feb 02, 2017 at 10:25:44AM -0700, Shuah Khan wrote:
> >> On 02/02/2017 01:32 AM, Jani Nikula wrote:
> >>> On Thu, 02 Feb 2017, Shuah Khan  wrote:
>  Change drm_helper_probe_single_connector_modes() to print an error to
>  report connector disconnected status instead of a debug message.
> 
>  When this condition occurs, application doesn't know the real error and
>  reports it as driver lacking support for mode setting. Change it to an
>  error to make it easier to debug.
> >>>
> >>> Please explain what makes this condition an error. Connectors get
> >>> connected and disconnected, business as usual, why should this be an
> >>> error?
> >>>
> >>> BR,
> >>> Jani.
> >>
> >> Disconnecting connector itself isn't an error. When user-space tries
> >> to access it, it would be useful to report the status that the connector
> >> is disconnected.
> >>
> >> I use embedded system(s) that don't like it when HDMI is hot added or
> >> removed. Also, because of return power, it is safer to disconnect HDMI
> >> and then apply power to the board. It chased a few libdrm and user-space
> >> dead ends before I enabled drm debug and was able to fix the real issue,
> >> which is a disconnected cable.
> >>
> >> User-space prints rather confusing messages as it doesn't really know
> >> the disconnected status as it isn't returned to it.
> >>
> >> I figured it might be a good idea to at least print a message and this can
> >> be a notice or info instead of an error. I do think its is worth while in
> >> some cases.
> > 
> > This sounds like a very specific use-case you have here, and it can easily
> > be supported by a small deamon in userspace (only on debug builds ofc)
> > that tell you that someone unplugged the screen when it shouldn't have
> > been.
> 
> drm_helper_probe_single_connector_modes() finds the condition and doesn't
> have a means to return it to the user-space.

Erhm, we send out the uevent for this, and userspace can react. If that's
not working, then we need to fix this bug, not add more uapi interfaces on
top ...
-Daniel

> 
> Instead of error or debug message, would it be useful to add a trace event
> to report status of connector to drm_helper_probe_single_connector_modes()
> Trace could be triggered as needed and turned off.
> 
> Please let me know what you think of this idea? If it sounds useful, I can
> add it.
> 
> > 
> > Because upstream runs also on non-embedded systems, where unplugging is
> > normal, and we definitely don't want to spam dmesg.
> > -Daniel
> > 
> 
> thanks,
> -- Shuah
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/7] drm/atomic: Use new atomic iterator macros.

[Daniel Vetter]

On Tue, Jan 17, 2017 at 01:55:50AM +0200, Laurent Pinchart wrote:
> Hi Maarten,
> 
> Thank you for the patch.
> 
> On Monday 16 Jan 2017 10:37:40 Maarten Lankhorst wrote:
> 
> Could we please get a description ? Apart from that,

Typed something small and merged the first 3 patches from this series.

Thanks, Daniel

> 
> Reviewed-by: Laurent Pinchart 
> 
> > Signed-off-by: Maarten Lankhorst 
> > ---
> >  drivers/gpu/drm/drm_atomic.c | 16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 18cdf2c956c6..7b61e0da9ace 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1562,7 +1562,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > *state)
> > 
> > DRM_DEBUG_ATOMIC("checking %p\n", state);
> > 
> > -   for_each_plane_in_state(state, plane, plane_state, i) {
> > +   for_each_new_plane_in_state(state, plane, plane_state, i) {
> > ret = drm_atomic_plane_check(plane, plane_state);
> > if (ret) {
> > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check 
> failed\n",
> > @@ -1571,7 +1571,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > *state) }
> > }
> > 
> > -   for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > +   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > ret = drm_atomic_crtc_check(crtc, crtc_state);
> > if (ret) {
> > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check 
> failed\n",
> > @@ -1584,7 +1584,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > *state) ret = config->funcs->atomic_check(state->dev, state);
> > 
> > if (!state->allow_modeset) {
> > -   for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > +   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full 
> modeset\n",
> >  crtc->base.id, crtc->name);
> > @@ -1668,13 +1668,13 @@ static void drm_atomic_print_state(const struct
> > drm_atomic_state *state)
> > 
> > DRM_DEBUG_ATOMIC("checking %p\n", state);
> > 
> > -   for_each_plane_in_state(state, plane, plane_state, i)
> > +   for_each_new_plane_in_state(state, plane, plane_state, i)
> > drm_atomic_plane_print_state(, plane_state);
> > 
> > -   for_each_crtc_in_state(state, crtc, crtc_state, i)
> > +   for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> > drm_atomic_crtc_print_state(, crtc_state);
> > 
> > -   for_each_connector_in_state(state, connector, connector_state, i)
> > +   for_each_new_connector_in_state(state, connector, connector_state, i)
> > drm_atomic_connector_print_state(, connector_state);
> >  }
> > 
> > @@ -1961,7 +1961,7 @@ static int prepare_crtc_signaling(struct drm_device
> > *dev, if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
> > return 0;
> > 
> > -   for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > +   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > u64 __user *fence_ptr;
> > 
> > fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
> > @@ -2041,7 +2041,7 @@ static void complete_crtc_signaling(struct drm_device
> > *dev, return;
> > }
> > 
> > -   for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > +   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > /*
> >  * TEST_ONLY and PAGE_FLIP_EVENT are mutually
> >  * exclusive, if they weren't, this code should be
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC simple allocator v2 1/2] Create Simple Allocator module

[Laurent Pinchart]

Hi Daniel,

On Tuesday 14 Feb 2017 20:44:44 Daniel Vetter wrote:
> On Tue, Feb 14, 2017 at 8:39 PM, Laurent Pinchart wrote:
> > On Tuesday 14 Feb 2017 20:33:58 Daniel Vetter wrote:
> >> On Mon, Feb 13, 2017 at 3:45 PM, Benjamin Gaignard wrote:
> >>> This is the core of simple allocator module.
> >>> It aim to offert one common ioctl to allocate specific memory.
> >>> 
> >>> version 2:
> >>> - rebased on 4.10-rc7
> >>> 
> >>> Signed-off-by: Benjamin Gaignard 
> >> 
> >> Why not ION? It's a bit a broken record question, but if there is a
> >> clear answer it should be in the patch ...
> > 
> > There's a bit of love & hate relationship between Linux developers and
> > ION. The API has shortcomings, and attempts to fix the issues went
> > nowhere. As Laura explained, starting from a blank slate (obviously
> > keeping in mind the lessons learnt so far with ION and other similar APIs)
> > and then adding a wrapper to expose ION on Android systems (at least as an
> > interim measure) was thought to be a better option. I still believe it is,
> > but we seem to lack traction. The problem has been around for so long that
> > I feel everybody has lost hope.
> > 
> > I don't think this is unsolvable, but we need to regain motivation. In my
> > opinion the first step would be to define the precise extent of the
> > problem we want to solve.
> 
> I'm not sure anyone really tried hard enough (in the same way no one
> tried hard enough to destage android syncpts, until last year). And
> anything new should at least very clearly explain why ION (even with
> the various todo items we collected at a few conferences) won't work,
> and how exactly the new allocator is different from ION. I don't think
> we need a full design doc (like you say, buffer allocation is hard,
> we'll get it wrong anyway), but at least a proper comparison with the
> existing thing. Plus explanation why we can't reuse the uabi.

I've explained several of my concerns (including open questions that need 
answers) in another reply to this patch, let's discuss them there to avoid 
splitting the discussion.

> Because ime when you rewrite something, you generally get one thing
> right (the one thing that pissed you off about the old solution), plus
> lots and lots of things that the old solution got right, wrong
> (because it's all lost in the history).

History, repeating mistakes, all that. History never repeats itself though. We 
might make similar or identical mistakes, but there's no fatality, unless we 
decide not to try before even starting :-)

> ADF was probably the best example in this. KMS also took a while until all
> the fbdev wheels have been properly reinvented (some are still the same old
> squeaky onces as fbdev had, e.g. fbcon).
> 
> And I don't think destaging ION is going to be hard, just a bit of
> work (could be a nice gsoc or whatever).

Oh, technically speaking, it would be pretty simple. The main issue is to 
decide whether we want to commit to the existing ION API. I don't :-)

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v4 1/7] drm: Add DRM support for tiny LCD displays

[Daniel Vetter]

On Sun, Feb 12, 2017 at 2:57 PM, Noralf Trønnes  wrote:
> tinydrm will be merged the way it is now, unless someone points to
> something that is broken. But I collect your comments for a later
> cleanup patchset. It takes a lot of effort for me as an amateur to
> keeps this code up-to-date out-of-tree for months. It's not even
> sure that I've hit the mark with this, so there will most likely be
> changes when I start converting fbtft drivers, and I'll implement the
> remaining bits and pieces as I make changes. The core of tinydrm won't
> change because of unforseen fbtft quirks, but other parts might.

+1, pls send pull request to Dave Airlie. DT-acks are all there, and
Thierry confirmed on irc that his reply on the last thread was indeed
meant as a full ack for going ahead and polishing later. I also
chatted with Dave directly, and he's perfectly happy to take this in
as-is. If you do this soon, should still easily get into 4.11 (since
the 4.10 release is delayed).

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[Ville Syrjälä]

On Tue, Feb 14, 2017 at 08:38:40PM +0100, Daniel Vetter wrote:
> On Tue, Feb 14, 2017 at 8:25 PM, John Stultz  wrote:
> > Currently, on the hikey board, we have the adv7511 bridge wired
> > up to the kirin ade drm driver. Unfortunately, the kirin ade
> > core cannot generate accurate byteclocks for all pixel clock
> > values.
> >
> > Thus if a mode clock is selected that we cannot calculate a
> > matching byteclock, the device will boot with a blank screen.
> >
> > Unfortunately, currently the only place we can properly check
> > potential modes for this issue in the connector mode_valid
> > helper. Again, hikey uses the adv7511 bridge, which is shared
> > between a number of different devices, so its improper to put
> > restrictions caused by the kirin drm driver in the adv7511
> > logic.
> >
> > So this patch tries to correct for that, by adding some
> > infrastructure so that the drm_crtc_helper_funcs can optionally
> > implement a mode_valid check, so that the probe helpers can
> > check to make sure there are not any restrictions at the crtc
> > level as well.
> >
> > Cc: Daniel Vetter 
> > Cc: Jani Nikula 
> > Cc: Sean Paul 
> > Cc: David Airlie 
> > Cc: Rob Clark 
> > Cc: Xinliang Liu 
> > Cc: Xinliang Liu 
> > Cc: Rongrong Zou 
> > Cc: Xinwei Kong 
> > Cc: Chen Feng 
> > Cc: Archit Taneja 
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: John Stultz 
> 
> So I'm going to be super-annoying here and ask for a complete
> solution. This here is defacto what ever driver already does (or has
> too), but it doesn't really solve the overall issue of having entirely
> separate validation paths for probe and atomic_check paths. I think if
> we wan to solve this, we need to solve this properly, with a generic
> solution. That would mean:
> - still in helpers, to make it all opt-int
> - covers crtc and encoders and bridges
> - allows you to implement the current mode_valid in terms of the new
> stuff (maybe as a default hook)
> - allows you to implement the current assortment of mode_fixup and/or
> atomic_check in terms of the new stuff, or at least to not have to
> duplicate logic in there

Long ago I quickly looked at doing this for i915. IIRC the main
complication was the normal vs. the crtc_ timings stored in the
mode. I suppose one option would be to populate the crtc_ timings
in .mode_valid() as well and otherwise just ignore the normal timings.

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

Re: [Intel-gfx] [PATCH v3 7/8] drm: Connector helper function to release resources

[Daniel Vetter]

On Mon, Feb 13, 2017 at 10:26 PM, Pandiyan, Dhinakaran
 wrote:
> On Mon, 2017-02-13 at 09:05 +, Lankhorst, Maarten wrote:
>> Pandiyan, Dhinakaran schreef op do 09-02-2017 om 18:55 [+]:
>> > On Thu, 2017-02-09 at 09:01 +, Lankhorst, Maarten wrote:
>> > >
>> > > Dhinakaran Pandiyan schreef op wo 08-02-2017 om 22:38 [-0800]:
>> > > >
>> > > > Having a ->atomic_release callback is useful to release shared
>> > > > resources
>> > > > that get allocated in compute_config(). This function is expected
>> > > > to
>> > > > be
>> > > > called in the atomic_check() phase before new resources are
>> > > > acquired.
>> > > >
>> > > > v2: Moved the caller hunk to this patch (Daniel)
>> > > >
>> > > > Suggested-by: Daniel Vetter 
>> > > > Signed-off-by: Dhinakaran Pandiyan > > > > >
>> > > > ---
>> > > >  drivers/gpu/drm/drm_atomic_helper.c  | 19
>> > > > +++
>> > > >  include/drm/drm_modeset_helper_vtables.h | 13 +
>> > > >  2 files changed, 32 insertions(+)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> > > > b/drivers/gpu/drm/drm_atomic_helper.c
>> > > > index 8795088..92bd741 100644
>> > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
>> > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> > > > @@ -576,6 +576,25 @@ drm_atomic_helper_check_modeset(struct
>> > > > drm_device *dev,
>> > > > }
>> > > > }
>> > > >
>> > > > +   for_each_connector_in_state(state, connector,
>> > > > connector_state, i) {
>> > > > +   const struct drm_connector_helper_funcs
>> > > > *conn_funcs;
>> > > > +   struct drm_crtc_state *crtc_state;
>> > > > +
>> > > > +   conn_funcs = connector->helper_private;
>> > > > +   if (!conn_funcs->atomic_release)
>> > > > +   continue;
>> > > > +
>> > > > +   if (!connector->state->crtc)
>> > > > +   continue;
>> > > > +
>> > > > +   crtc_state =
>> > > > drm_atomic_get_existing_crtc_state(state, connector->state-
>> > > > >crtc);
>> > > > +
>> > > > +   if (crtc_state->connectors_changed ||
>> > > > +   crtc_state->mode_changed ||
>> > > > +   (crtc_state->active_changed && !crtc_state-
>> > > > >
>> > > > > active))
>> > > > +   conn_funcs->atomic_release(connector,
>> > > > connector_state);
>> > > > +   }
>> > >
>> > > Could we deal with the VCPI state separately in
>> > > intel_modeset_checks,
>> > > like we do with dpll?
>> >
>> > We'd want to release the VCPI slots before they are acquired in
>> > ->compute_config(). intel_modeset_checks() will be too late to
>> > release
>> > them. Are you suggesting both acquiring and releasing slots should be
>> > done in intel_modeset_checks()?
>>
>> That makes things a bit more nasty. Maybe add a
>> conn_funcs->atomic_check that always gets called, something like I did
>> below?
>>
>> I'd love to use it for some atomic connector properties too.
>
>
> Adding and unconditionally calling conn_funcs->atomic_check() should be
> doable. It also follows the pattern we have for encoders and CRTCs. But
> I'll have to move the connector->state->crtc state checks inside the
> function.

Adding ->atomic_check that's unconditionally called sounds troubling,
because all the other ->atomic_check functions are _only_ called when
enabling stuff. ->atomic_release sounds much better to me, and from a
helper pov DK's patch above is the right place.

If that place doesn't work for i915.ko, then we need our own callback
(like we already have with e.g. ->compute_config, we could do a
->release_config). But if it's just cosmetics, then I don't see the
reason why we need to change this. On that issue: How exactly does our
compute_config work if we haven't updated the routing (using the above
helper) yet? That sounds like a pretty big misdesign on our side ...
-Daniel


>
> -DK
>> > >
>> > >
>> > > Maybe implementing the relevant VCPI state could be done as an
>> > > atomic
>> > > helper function too, so other atomic drivers can just plug it in.
>> > >
>> > The idea was to reduce boilerplate in the drivers and use the
>> > private_obj state for different object types.
>> >
>> >
>> > >
>> > > Not sure how doable this is, but if it's not too hard, then it's
>> > > probably cleaner :)
>> > > ___
>> > > Intel-gfx mailing list
>> > > intel-...@lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> ___
>> Intel-gfx mailing list
>> intel-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[John Stultz]

On Tue, Feb 14, 2017 at 11:38 AM, Daniel Vetter  wrote:
> On Tue, Feb 14, 2017 at 8:25 PM, John Stultz  wrote:
>> Currently, on the hikey board, we have the adv7511 bridge wired
>> up to the kirin ade drm driver. Unfortunately, the kirin ade
>> core cannot generate accurate byteclocks for all pixel clock
>> values.
>>
>> Thus if a mode clock is selected that we cannot calculate a
>> matching byteclock, the device will boot with a blank screen.
>>
>> Unfortunately, currently the only place we can properly check
>> potential modes for this issue in the connector mode_valid
>> helper. Again, hikey uses the adv7511 bridge, which is shared
>> between a number of different devices, so its improper to put
>> restrictions caused by the kirin drm driver in the adv7511
>> logic.
>>
>> So this patch tries to correct for that, by adding some
>> infrastructure so that the drm_crtc_helper_funcs can optionally
>> implement a mode_valid check, so that the probe helpers can
>> check to make sure there are not any restrictions at the crtc
>> level as well.
>>
>> Cc: Daniel Vetter 
>> Cc: Jani Nikula 
>> Cc: Sean Paul 
>> Cc: David Airlie 
>> Cc: Rob Clark 
>> Cc: Xinliang Liu 
>> Cc: Xinliang Liu 
>> Cc: Rongrong Zou 
>> Cc: Xinwei Kong 
>> Cc: Chen Feng 
>> Cc: Archit Taneja 
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: John Stultz 
>
> So I'm going to be super-annoying here and ask for a complete
> solution. This here is defacto what ever driver already does (or has
> too), but it doesn't really solve the overall issue of having entirely
> separate validation paths for probe and atomic_check paths. I think if
> we wan to solve this, we need to solve this properly, with a generic
> solution. That would mean:
> - still in helpers, to make it all opt-int

Just to be clear, I believe what I proposed is opt-in, but I assume
you want that in addition to the following, right?

> - covers crtc and encoders and bridges

So you'd like similar mode_valid() calls in the crtc/encoder/bridge
helpers? right?

> - allows you to implement the current mode_valid in terms of the new
> stuff (maybe as a default hook)

This bit I'm not sure I'm following. The current drm_connector's
mode_valid in terms of a new mode_valid call that also looks at
crtc/encoder/bridges? Or do you mean something else?

> - allows you to implement the current assortment of mode_fixup and/or
> atomic_check in terms of the new stuff, or at least to not have to
> duplicate logic in there

This is over my head, so I'll have to research to better understand.

> Docs for all this, especially updating all the warnings on how to use
> the existing hooks correctly.

That's fair.

> I think just pushing this specific case into the helpers, without
> rethinking the overall big picture, isn't gaining us all that much.
> For just this I'd say just put it into drivers, until we have some

Not following here. What do you mean by "put it into drivers"?  Where?

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

Re: [RFC simple allocator v2 1/2] Create Simple Allocator module

[Daniel Vetter]

On Tue, Feb 14, 2017 at 8:39 PM, Laurent Pinchart
 wrote:
> Hi Daniel,
>
> On Tuesday 14 Feb 2017 20:33:58 Daniel Vetter wrote:
>> On Mon, Feb 13, 2017 at 3:45 PM, Benjamin Gaignard wrote:
>> > This is the core of simple allocator module.
>> > It aim to offert one common ioctl to allocate specific memory.
>> >
>> > version 2:
>> > - rebased on 4.10-rc7
>> >
>> > Signed-off-by: Benjamin Gaignard 
>>
>> Why not ION? It's a bit a broken record question, but if there is a
>> clear answer it should be in the patch ...
>
> There's a bit of love & hate relationship between Linux developers and ION.
> The API has shortcomings, and attempts to fix the issues went nowhere. As
> Laura explained, starting from a blank slate (obviously keeping in mind the
> lessons learnt so far with ION and other similar APIs) and then adding a
> wrapper to expose ION on Android systems (at least as an interim measure) was
> thought to be a better option. I still believe it is, but we seem to lack
> traction. The problem has been around for so long that I feel everybody has
> lost hope.
>
> I don't think this is unsolvable, but we need to regain motivation. In my
> opinion the first step would be to define the precise extent of the problem we
> want to solve.

I'm not sure anyone really tried hard enough (in the same way no one
tried hard enough to destage android syncpts, until last year). And
anything new should at least very clearly explain why ION (even with
the various todo items we collected at a few conferences) won't work,
and how exactly the new allocator is different from ION. I don't think
we need a full design doc (like you say, buffer allocation is hard,
we'll get it wrong anyway), but at least a proper comparison with the
existing thing. Plus explanation why we can't reuse the uabi.

Because ime when you rewrite something, you generally get one thing
right (the one thing that pissed you off about the old solution), plus
lots and lots of things that the old solution got right, wrong
(because it's all lost in the history). ADF was probably the best
example in this. KMS also took a while until all the fbdev wheels have
been properly reinvented (some are still the same old squeaky onces as
fbdev had, e.g. fbcon).

And I don't think destaging ION is going to be hard, just a bit of
work (could be a nice gsoc or whatever).
-Daniel

>
>> > ---
>> >
>> >  Documentation/simple-allocator.txt  |  81 +++
>> >  drivers/Kconfig |   2 +
>> >  drivers/Makefile|   1 +
>> >  drivers/simpleallocator/Kconfig |  10 ++
>> >  drivers/simpleallocator/Makefile|   1 +
>> >  drivers/simpleallocator/simple-allocator-priv.h |  33 +
>> >  drivers/simpleallocator/simple-allocator.c  | 180 +++
>> >  include/uapi/linux/simple-allocator.h   |  35 +
>> >  8 files changed, 343 insertions(+)
>> >  create mode 100644 Documentation/simple-allocator.txt
>> >  create mode 100644 drivers/simpleallocator/Kconfig
>> >  create mode 100644 drivers/simpleallocator/Makefile
>> >  create mode 100644 drivers/simpleallocator/simple-allocator-priv.h
>> >  create mode 100644 drivers/simpleallocator/simple-allocator.c
>> >  create mode 100644 include/uapi/linux/simple-allocator.h
>> >
>> > diff --git a/Documentation/simple-allocator.txt
>> > b/Documentation/simple-allocator.txt new file mode 100644
>> > index 000..89ba883
>> > --- /dev/null
>> > +++ b/Documentation/simple-allocator.txt
>> > @@ -0,0 +1,81 @@
>> > +Simple Allocator Framework
>> > +
>> > +Simple Allocator offer a single ioctl SA_IOC_ALLOC to allocate buffers
>> > +on dedicated memory regions and export them as a dmabuf file descriptor.
>> > +Using dmabuf file descriptor allow to share this memory between processes
>> > +and/or import it into other frameworks like v4l2 or drm/kms (prime).
>> > +When userland wants to free the memory only a call to close() in needed
>> > +so it could done even without knowing that buffer has been allocated by
>> > +simple allocator ioctl.
>> > +
>> > +Each memory regions will be seen as a filein /dev/.
>> > +For example CMA regions will exposed has /dev/cmaX.
>> > +
>> > +Implementing a simple allocator
>> > +---
>> > +
>> > +Simple Allocator provide helpers functions to register/unregister an
>> > +allocator:
>> > +- simple_allocator_register(struct sa_device *sadev)
>> > +  Register simple_allocator_device using sa_device structure where name,
>> > +  owner and allocate fields must be set.
>> > +
>> > +- simple_allocator_unregister(struct sa_device *sadev)
>> > +  Unregister a simple allocator device.
>> > +
>> > +Using Simple Allocator /dev interface example
>> > +-
>> > +
>> > +This example of code allocate a buffer on the first CMA region
>> > (/dev/cma0)
>> > +before mmap and close it.

Re: [RFC simple allocator v2 1/2] Create Simple Allocator module

[Laurent Pinchart]

Hi Daniel,

On Tuesday 14 Feb 2017 20:33:58 Daniel Vetter wrote:
> On Mon, Feb 13, 2017 at 3:45 PM, Benjamin Gaignard wrote:
> > This is the core of simple allocator module.
> > It aim to offert one common ioctl to allocate specific memory.
> > 
> > version 2:
> > - rebased on 4.10-rc7
> > 
> > Signed-off-by: Benjamin Gaignard 
> 
> Why not ION? It's a bit a broken record question, but if there is a
> clear answer it should be in the patch ...

There's a bit of love & hate relationship between Linux developers and ION. 
The API has shortcomings, and attempts to fix the issues went nowhere. As 
Laura explained, starting from a blank slate (obviously keeping in mind the 
lessons learnt so far with ION and other similar APIs) and then adding a 
wrapper to expose ION on Android systems (at least as an interim measure) was 
thought to be a better option. I still believe it is, but we seem to lack 
traction. The problem has been around for so long that I feel everybody has 
lost hope.

I don't think this is unsolvable, but we need to regain motivation. In my 
opinion the first step would be to define the precise extent of the problem we 
want to solve.

> > ---
> > 
> >  Documentation/simple-allocator.txt  |  81 +++
> >  drivers/Kconfig |   2 +
> >  drivers/Makefile|   1 +
> >  drivers/simpleallocator/Kconfig |  10 ++
> >  drivers/simpleallocator/Makefile|   1 +
> >  drivers/simpleallocator/simple-allocator-priv.h |  33 +
> >  drivers/simpleallocator/simple-allocator.c  | 180 +++
> >  include/uapi/linux/simple-allocator.h   |  35 +
> >  8 files changed, 343 insertions(+)
> >  create mode 100644 Documentation/simple-allocator.txt
> >  create mode 100644 drivers/simpleallocator/Kconfig
> >  create mode 100644 drivers/simpleallocator/Makefile
> >  create mode 100644 drivers/simpleallocator/simple-allocator-priv.h
> >  create mode 100644 drivers/simpleallocator/simple-allocator.c
> >  create mode 100644 include/uapi/linux/simple-allocator.h
> > 
> > diff --git a/Documentation/simple-allocator.txt
> > b/Documentation/simple-allocator.txt new file mode 100644
> > index 000..89ba883
> > --- /dev/null
> > +++ b/Documentation/simple-allocator.txt
> > @@ -0,0 +1,81 @@
> > +Simple Allocator Framework
> > +
> > +Simple Allocator offer a single ioctl SA_IOC_ALLOC to allocate buffers
> > +on dedicated memory regions and export them as a dmabuf file descriptor.
> > +Using dmabuf file descriptor allow to share this memory between processes
> > +and/or import it into other frameworks like v4l2 or drm/kms (prime).
> > +When userland wants to free the memory only a call to close() in needed
> > +so it could done even without knowing that buffer has been allocated by
> > +simple allocator ioctl.
> > +
> > +Each memory regions will be seen as a filein /dev/.
> > +For example CMA regions will exposed has /dev/cmaX.
> > +
> > +Implementing a simple allocator
> > +---
> > +
> > +Simple Allocator provide helpers functions to register/unregister an
> > +allocator:
> > +- simple_allocator_register(struct sa_device *sadev)
> > +  Register simple_allocator_device using sa_device structure where name,
> > +  owner and allocate fields must be set.
> > +
> > +- simple_allocator_unregister(struct sa_device *sadev)
> > +  Unregister a simple allocator device.
> > +
> > +Using Simple Allocator /dev interface example
> > +-
> > +
> > +This example of code allocate a buffer on the first CMA region
> > (/dev/cma0)
> > +before mmap and close it.
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "simple-allocator.h"
> > +
> > +#define LENGTH 1024*16
> > +
> > +void main (void)
> > +{
> > +   struct simple_allocate_data data;
> > +   int fd = open("/dev/cma0", O_RDWR, 0);
> > +   int ret;
> > +   void *mem;
> > +
> > +   if (fd < 0) {
> > +   printf("Can't open /dev/cma0\n");
> > +   return;
> > +   }
> > +
> > +   memset(, 0, sizeof(data));
> > +
> > +   data.length = LENGTH;
> > +   data.flags = O_RDWR | O_CLOEXEC;
> > +
> > +   ret = ioctl(fd, SA_IOC_ALLOC, );
> > +   if (ret) {
> > +   printf("Buffer allocation failed\n");
> > +   goto end;
> > +   }
> > +
> > +   mem = mmap(0, LENGTH, PROT_READ | PROT_WRITE, MAP_SHARED, data.fd,
> > 0); +   if (mem == MAP_FAILED) {
> > +   printf("mmap failed\n");
> > +   }
> > +
> > +   memset(mem, 0xFF, LENGTH);
> > +   munmap(mem, LENGTH);
> > +
> > +   printf("test simple allocator CMA OK\n");
> > +end:
> > +   close(fd);
> > +}
> > diff --git a/drivers/Kconfig b/drivers/Kconfig
> > index 

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[Daniel Vetter]

On Tue, Feb 14, 2017 at 8:25 PM, John Stultz  wrote:
> Currently, on the hikey board, we have the adv7511 bridge wired
> up to the kirin ade drm driver. Unfortunately, the kirin ade
> core cannot generate accurate byteclocks for all pixel clock
> values.
>
> Thus if a mode clock is selected that we cannot calculate a
> matching byteclock, the device will boot with a blank screen.
>
> Unfortunately, currently the only place we can properly check
> potential modes for this issue in the connector mode_valid
> helper. Again, hikey uses the adv7511 bridge, which is shared
> between a number of different devices, so its improper to put
> restrictions caused by the kirin drm driver in the adv7511
> logic.
>
> So this patch tries to correct for that, by adding some
> infrastructure so that the drm_crtc_helper_funcs can optionally
> implement a mode_valid check, so that the probe helpers can
> check to make sure there are not any restrictions at the crtc
> level as well.
>
> Cc: Daniel Vetter 
> Cc: Jani Nikula 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: Rob Clark 
> Cc: Xinliang Liu 
> Cc: Xinliang Liu 
> Cc: Rongrong Zou 
> Cc: Xinwei Kong 
> Cc: Chen Feng 
> Cc: Archit Taneja 
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz 

So I'm going to be super-annoying here and ask for a complete
solution. This here is defacto what ever driver already does (or has
too), but it doesn't really solve the overall issue of having entirely
separate validation paths for probe and atomic_check paths. I think if
we wan to solve this, we need to solve this properly, with a generic
solution. That would mean:
- still in helpers, to make it all opt-int
- covers crtc and encoders and bridges
- allows you to implement the current mode_valid in terms of the new
stuff (maybe as a default hook)
- allows you to implement the current assortment of mode_fixup and/or
atomic_check in terms of the new stuff, or at least to not have to
duplicate logic in there

Docs for all this, especially updating all the warnings on how to use
the existing hooks correctly.

I think just pushing this specific case into the helpers, without
rethinking the overall big picture, isn't gaining us all that much.
For just this I'd say just put it into drivers, until we have some
good clue about how the helpers should look like (maybe this time is
the time? ...).

Cheers, Daniel
> ---
>  drivers/gpu/drm/drm_probe_helper.c   | 24 
>  include/drm/drm_modeset_helper_vtables.h | 26 ++
>  2 files changed, 50 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> b/drivers/gpu/drm/drm_probe_helper.c
> index cf8f012..a808348 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -170,6 +170,28 @@ drm_connector_detect(struct drm_connector *connector, 
> bool force)
> connector_status_connected;
>  }
>
> +static enum drm_mode_status
> +drm_connector_check_crtc_modes(struct drm_connector *connector,
> +  struct drm_display_mode *mode)
> +{
> +   struct drm_device *dev = connector->dev;
> +   const struct drm_crtc_helper_funcs *crtc_funcs;
> +   struct drm_crtc *c;
> +
> +   if (mode->status != MODE_OK)
> +   return mode->status;
> +
> +   /* Check all the crtcs on a connector to make sure the mode is valid 
> */
> +   drm_for_each_crtc(c, dev) {
> +   crtc_funcs = c->helper_private;
> +   if (crtc_funcs && crtc_funcs->mode_valid)
> +   mode->status = crtc_funcs->mode_valid(c, mode);
> +   if (mode->status != MODE_OK)
> +   break;
> +   }
> +   return mode->status;
> +}
> +
>  /**
>   * drm_helper_probe_single_connector_modes - get complete set of display 
> modes
>   * @connector: connector to probe
> @@ -338,6 +360,8 @@ int drm_helper_probe_single_connector_modes(struct 
> drm_connector *connector,
> if (mode->status == MODE_OK && connector_funcs->mode_valid)
> mode->status = connector_funcs->mode_valid(connector,
>mode);
> +
> +   mode->status = drm_connector_check_crtc_modes(connector, 
> mode);
> }
>
>  prune:
> diff --git a/include/drm/drm_modeset_helper_vtables.h 
> b/include/drm/drm_modeset_helper_vtables.h
> index 69c3974..53ca0e4 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -105,6 +105,32 @@ struct drm_crtc_helper_funcs {
> void (*commit)(struct drm_crtc *crtc);
>
>  

[PATCH] drm/nouveau/core: recognise GP107 chipset

[Daniel Drake]

From: Chris Chiu 

This new graphics card was failing to initialize with nouveau due to
an "unknown chipset" error.

Copy the GP106 configuration and rename for GP107/NV137. We don't
know for certain that this is fully correct, but brief desktop testing
suggests this is working fine.

Signed-off-by: Chris Chiu 
Signed-off-by: Daniel Drake 
---
 drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 29 +++
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
index fea30d6..d242431 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
@@ -2237,6 +2237,34 @@ nv136_chipset = {
.fifo = gp100_fifo_new,
 };
 
+static const struct nvkm_device_chip
+nv137_chipset = {
+   .name = "GP107",
+   .bar = gf100_bar_new,
+   .bios = nvkm_bios_new,
+   .bus = gf100_bus_new,
+   .devinit = gm200_devinit_new,
+   .fb = gp104_fb_new,
+   .fuse = gm107_fuse_new,
+   .gpio = gk104_gpio_new,
+   .i2c = gm200_i2c_new,
+   .ibus = gm200_ibus_new,
+   .imem = nv50_instmem_new,
+   .ltc = gp100_ltc_new,
+   .mc = gp100_mc_new,
+   .mmu = gf100_mmu_new,
+   .pci = gp100_pci_new,
+   .timer = gk20a_timer_new,
+   .top = gk104_top_new,
+   .ce[0] = gp104_ce_new,
+   .ce[1] = gp104_ce_new,
+   .ce[2] = gp104_ce_new,
+   .ce[3] = gp104_ce_new,
+   .disp = gp104_disp_new,
+   .dma = gf119_dma_new,
+   .fifo = gp100_fifo_new,
+};
+
 static int
 nvkm_device_event_ctor(struct nvkm_object *object, void *data, u32 size,
   struct nvkm_notify *notify)
@@ -2673,6 +2701,7 @@ nvkm_device_ctor(const struct nvkm_device_func *func,
case 0x130: device->chip = _chipset; break;
case 0x134: device->chip = _chipset; break;
case 0x136: device->chip = _chipset; break;
+   case 0x137: device->chip = _chipset; break;
default:
nvdev_error(device, "unknown chipset (%08x)\n", boot0);
goto done;
-- 
2.9.3

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

Re: [RFC simple allocator v2 1/2] Create Simple Allocator module

[Daniel Vetter]

On Mon, Feb 13, 2017 at 3:45 PM, Benjamin Gaignard
 wrote:
> This is the core of simple allocator module.
> It aim to offert one common ioctl to allocate specific memory.
>
> version 2:
> - rebased on 4.10-rc7
>
> Signed-off-by: Benjamin Gaignard 

Why not ION? It's a bit a broken record question, but if there is a
clear answer it should be in the patch ...
-Daniel

> ---
>  Documentation/simple-allocator.txt  |  81 +++
>  drivers/Kconfig |   2 +
>  drivers/Makefile|   1 +
>  drivers/simpleallocator/Kconfig |  10 ++
>  drivers/simpleallocator/Makefile|   1 +
>  drivers/simpleallocator/simple-allocator-priv.h |  33 +
>  drivers/simpleallocator/simple-allocator.c  | 180 
> 
>  include/uapi/linux/simple-allocator.h   |  35 +
>  8 files changed, 343 insertions(+)
>  create mode 100644 Documentation/simple-allocator.txt
>  create mode 100644 drivers/simpleallocator/Kconfig
>  create mode 100644 drivers/simpleallocator/Makefile
>  create mode 100644 drivers/simpleallocator/simple-allocator-priv.h
>  create mode 100644 drivers/simpleallocator/simple-allocator.c
>  create mode 100644 include/uapi/linux/simple-allocator.h
>
> diff --git a/Documentation/simple-allocator.txt 
> b/Documentation/simple-allocator.txt
> new file mode 100644
> index 000..89ba883
> --- /dev/null
> +++ b/Documentation/simple-allocator.txt
> @@ -0,0 +1,81 @@
> +Simple Allocator Framework
> +
> +Simple Allocator offer a single ioctl SA_IOC_ALLOC to allocate buffers
> +on dedicated memory regions and export them as a dmabuf file descriptor.
> +Using dmabuf file descriptor allow to share this memory between processes
> +and/or import it into other frameworks like v4l2 or drm/kms (prime).
> +When userland wants to free the memory only a call to close() in needed
> +so it could done even without knowing that buffer has been allocated by
> +simple allocator ioctl.
> +
> +Each memory regions will be seen as a filein /dev/.
> +For example CMA regions will exposed has /dev/cmaX.
> +
> +Implementing a simple allocator
> +---
> +
> +Simple Allocator provide helpers functions to register/unregister an
> +allocator:
> +- simple_allocator_register(struct sa_device *sadev)
> +  Register simple_allocator_device using sa_device structure where name,
> +  owner and allocate fields must be set.
> +
> +- simple_allocator_unregister(struct sa_device *sadev)
> +  Unregister a simple allocator device.
> +
> +Using Simple Allocator /dev interface example
> +-
> +
> +This example of code allocate a buffer on the first CMA region (/dev/cma0)
> +before mmap and close it.
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "simple-allocator.h"
> +
> +#define LENGTH 1024*16
> +
> +void main (void)
> +{
> +   struct simple_allocate_data data;
> +   int fd = open("/dev/cma0", O_RDWR, 0);
> +   int ret;
> +   void *mem;
> +
> +   if (fd < 0) {
> +   printf("Can't open /dev/cma0\n");
> +   return;
> +   }
> +
> +   memset(, 0, sizeof(data));
> +
> +   data.length = LENGTH;
> +   data.flags = O_RDWR | O_CLOEXEC;
> +
> +   ret = ioctl(fd, SA_IOC_ALLOC, );
> +   if (ret) {
> +   printf("Buffer allocation failed\n");
> +   goto end;
> +   }
> +
> +   mem = mmap(0, LENGTH, PROT_READ | PROT_WRITE, MAP_SHARED, data.fd, 0);
> +   if (mem == MAP_FAILED) {
> +   printf("mmap failed\n");
> +   }
> +
> +   memset(mem, 0xFF, LENGTH);
> +   munmap(mem, LENGTH);
> +
> +   printf("test simple allocator CMA OK\n");
> +end:
> +   close(fd);
> +}
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index e1e2066..a6d8828 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -202,4 +202,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
>
>  source "drivers/fpga/Kconfig"
>
> +source "drivers/simpleallocator/Kconfig"
> +
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 060026a..5081eb8 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -173,3 +173,4 @@ obj-$(CONFIG_STM)   += hwtracing/stm/
>  obj-$(CONFIG_ANDROID)  += android/
>  obj-$(CONFIG_NVMEM)+= nvmem/
>  obj-$(CONFIG_FPGA) += fpga/
> +obj-$(CONFIG_SIMPLE_ALLOCATOR) += simpleallocator/
> diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig
> new file mode 100644
> index 000..c6fc2e3
> --- /dev/null
> +++ b/drivers/simpleallocator/Kconfig
> @@ -0,0 +1,10 @@
> +menu "Simple Allocator"
> +
> +config SIMPLE_ALLOCATOR
> +   tristate "Simple Alllocator Framework"
> +   select DMA_SHARED_BUFFER
> +   

[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460

[bugzilla-daemon]

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

--- Comment #33 from Reimar Imhof  ---
(In reply to Michel Dänzer from comment #27)
> (In reply to Reimar Imhof from comment #26)
> > Together with comment #24 there is a render bug in kernel 4.8 that shows up
> > at 100% cpu load.
> > With kernel 4.9 this same bug shows up at 0% / idle cpu load.
> > 
> > With 
> > af79ad2b1f337a00aa150b993635b10bc68dc842
> > Merge branch 'sched-core-for-linus' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> > it changed from "bug shows at 100% load" to "bug shows at 0% load". And the
> > change is something about the scheduler.
> > To me this seems likely.
> 
> Not really. That commit is a pure merge commit, which makes it unlikely that
> it behaves any differently from either of its parent commits. So git bisect
> should have identified one of its ancestor commits instead. The fact that it
> identified a pure merge commit indicates that the result is incorrect, most
> likely because at least one commit along the way was incorrectly classified
> as good (or bad).

I forgot to mention:
I had a look at the first merge commits. I did _not_ do a "git bisect" but for
example a "git reset --hard 7af8a0f8088831428051976cb06cc1e450f8bab5" followed
by a "make rpm" to compile "Merge tag 'arm64-upstream' of
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux".
"e606d81d2d9596ab2b4fd0dc052eea0485b7e8c2
Merge branch 'ras-core-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip" was a good commit - no
problems at idle cpu as described in Comment #23.
That one was followed by "af79ad2b1f337a00aa150b993635b10bc68dc842
Merge branch 'sched-core-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip" which turned out to be
the first bad commit (glitches at 0 cpu load).
So all tested commits were pure merge commits.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC simple allocator v2 1/2] Create Simple Allocator module

[Laurent Pinchart]

Hi Benjamin,

Thank you for the patch. I've CC'ed the linux-api mailing list.

On Monday 13 Feb 2017 15:45:05 Benjamin Gaignard wrote:
> This is the core of simple allocator module.
> It aim to offert one common ioctl to allocate specific memory.
> 
> version 2:
> - rebased on 4.10-rc7
> 
> Signed-off-by: Benjamin Gaignard 
> ---
>  Documentation/simple-allocator.txt  |  81 +++
>  drivers/Kconfig |   2 +
>  drivers/Makefile|   1 +
>  drivers/simpleallocator/Kconfig |  10 ++
>  drivers/simpleallocator/Makefile|   1 +
>  drivers/simpleallocator/simple-allocator-priv.h |  33 +
>  drivers/simpleallocator/simple-allocator.c  | 180 +
>  include/uapi/linux/simple-allocator.h   |  35 +
>  8 files changed, 343 insertions(+)
>  create mode 100644 Documentation/simple-allocator.txt
>  create mode 100644 drivers/simpleallocator/Kconfig
>  create mode 100644 drivers/simpleallocator/Makefile
>  create mode 100644 drivers/simpleallocator/simple-allocator-priv.h
>  create mode 100644 drivers/simpleallocator/simple-allocator.c
>  create mode 100644 include/uapi/linux/simple-allocator.h
> 
> diff --git a/Documentation/simple-allocator.txt
> b/Documentation/simple-allocator.txt new file mode 100644
> index 000..89ba883
> --- /dev/null
> +++ b/Documentation/simple-allocator.txt
> @@ -0,0 +1,81 @@
> +Simple Allocator Framework

There's nothing simple in buffer allocation, otherwise we would have solved 
the problem a long time ago. Let's not use a misleading name.

> +Simple Allocator offer a single ioctl SA_IOC_ALLOC to allocate buffers
> +on dedicated memory regions and export them as a dmabuf file descriptor.
> +Using dmabuf file descriptor allow to share this memory between processes
> +and/or import it into other frameworks like v4l2 or drm/kms (prime).
> +When userland wants to free the memory only a call to close() in needed
> +so it could done even without knowing that buffer has been allocated by
> +simple allocator ioctl.
> +
> +Each memory regions will be seen as a filein /dev/.
> +For example CMA regions will exposed has /dev/cmaX.

Why do you need multiple devices ? Furthermore, given how central this API 
will become, I believe it needs very strict review, and would be a candidate 
for a syscall.

Without diving into details yet, there are a few particular points I'm 
concerned about.

- What is the scope of this API ? What problems do you want to solve, plan to 
solve in the future, and consider as out of scope ?

- How should we handle permissions and resource limits ? Is file-based 
permission on a device node a good model ? How do we prevent or at least 
mitigate memory-related DoS ?

- How should such a central allocator API interact with containers and 
virtualization in general ?

- What model do we want to expose to applications to select a memory type ? 
You still haven't convinced me that we should expose memory pools explicitly 
to application (à la ION), and I believe a usage/constraint model would be 
better.

> +Implementing a simple allocator
> +---
> +
> +Simple Allocator provide helpers functions to register/unregister an
> +allocator:
> +- simple_allocator_register(struct sa_device *sadev)
> +  Register simple_allocator_device using sa_device structure where name,
> +  owner and allocate fields must be set.
> +
> +- simple_allocator_unregister(struct sa_device *sadev)
> +  Unregister a simple allocator device.
> +
> +Using Simple Allocator /dev interface example
> +-
> +
> +This example of code allocate a buffer on the first CMA region (/dev/cma0)
> +before mmap and close it.
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "simple-allocator.h"
> +
> +#define LENGTH 1024*16
> +
> +void main (void)
> +{
> + struct simple_allocate_data data;
> + int fd = open("/dev/cma0", O_RDWR, 0);
> + int ret;
> + void *mem;
> +
> + if (fd < 0) {
> + printf("Can't open /dev/cma0\n");
> + return;
> + }
> +
> + memset(, 0, sizeof(data));
> +
> + data.length = LENGTH;
> + data.flags = O_RDWR | O_CLOEXEC;
> +
> + ret = ioctl(fd, SA_IOC_ALLOC, );
> + if (ret) {
> + printf("Buffer allocation failed\n");
> + goto end;
> + }
> +
> + mem = mmap(0, LENGTH, PROT_READ | PROT_WRITE, MAP_SHARED, data.fd, 0);
> + if (mem == MAP_FAILED) {
> + printf("mmap failed\n");
> + }
> +
> + memset(mem, 0xFF, LENGTH);
> + munmap(mem, LENGTH);
> +
> + printf("test simple allocator CMA OK\n");
> +end:
> + close(fd);
> +}
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index e1e2066..a6d8828 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> 

[RFC][PATCH 0/2] Add mode_valid drm_crtc_helper_funcs for HiKey

[John Stultz]

Currently, on the hikey board, we have the adv7511 bridge wired
up to the kirin ade drm driver. Unfortunately, the kirin ade
core cannot generate accurate byteclocks for all pixel clock
values.

Thus if a mode clock is selected that we cannot calculate a
matching byteclock, the device will boot with a blank screen.

Unfortunately, currently the only place we can properly check
potential modes for this issue in the connector mode_valid
helper. Again, hikey uses the adv7511 bridge, which is shared
between a number of different devices, so its improper to put
restrictions caused by the kirin drm driver in the adv7511
logic.

So this patch set tries to fix this by adding some
infrastructure and logic to the probe helpers so that drm_crtcs
can filter the probed modes. And then adds a whitelist of valid
modes for the kirin drm driver used on HiKey.

This is a first pass attempt here, implementing a suggestion from
Rob Clark on irc, so I'd really welcome any feedback or ideas for
how to best do this.

Thanks so much!
-john

Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Rob Clark 
Cc: Xinliang Liu 
Cc: Xinliang Liu 
Cc: Rongrong Zou 
Cc: Xinwei Kong 
Cc: Chen Feng 
Cc: Archit Taneja 
Cc: dri-devel@lists.freedesktop.org

John Stultz (2):
  drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
  drm: kirin: Restrict modes to known good mode clocks

 drivers/gpu/drm/drm_probe_helper.c  | 24 
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 38 +
 include/drm/drm_modeset_helper_vtables.h| 26 +
 3 files changed, 88 insertions(+)

-- 
2.7.4

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

[RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

[John Stultz]

Currently, on the hikey board, we have the adv7511 bridge wired
up to the kirin ade drm driver. Unfortunately, the kirin ade
core cannot generate accurate byteclocks for all pixel clock
values.

Thus if a mode clock is selected that we cannot calculate a
matching byteclock, the device will boot with a blank screen.

Unfortunately, currently the only place we can properly check
potential modes for this issue in the connector mode_valid
helper. Again, hikey uses the adv7511 bridge, which is shared
between a number of different devices, so its improper to put
restrictions caused by the kirin drm driver in the adv7511
logic.

So this patch tries to correct for that, by adding some
infrastructure so that the drm_crtc_helper_funcs can optionally
implement a mode_valid check, so that the probe helpers can
check to make sure there are not any restrictions at the crtc
level as well.

Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Rob Clark 
Cc: Xinliang Liu 
Cc: Xinliang Liu 
Cc: Rongrong Zou 
Cc: Xinwei Kong 
Cc: Chen Feng 
Cc: Archit Taneja 
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz 
---
 drivers/gpu/drm/drm_probe_helper.c   | 24 
 include/drm/drm_modeset_helper_vtables.h | 26 ++
 2 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index cf8f012..a808348 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -170,6 +170,28 @@ drm_connector_detect(struct drm_connector *connector, bool 
force)
connector_status_connected;
 }
 
+static enum drm_mode_status
+drm_connector_check_crtc_modes(struct drm_connector *connector,
+  struct drm_display_mode *mode)
+{
+   struct drm_device *dev = connector->dev;
+   const struct drm_crtc_helper_funcs *crtc_funcs;
+   struct drm_crtc *c;
+
+   if (mode->status != MODE_OK)
+   return mode->status;
+
+   /* Check all the crtcs on a connector to make sure the mode is valid */
+   drm_for_each_crtc(c, dev) {
+   crtc_funcs = c->helper_private;
+   if (crtc_funcs && crtc_funcs->mode_valid)
+   mode->status = crtc_funcs->mode_valid(c, mode);
+   if (mode->status != MODE_OK)
+   break;
+   }
+   return mode->status;
+}
+
 /**
  * drm_helper_probe_single_connector_modes - get complete set of display modes
  * @connector: connector to probe
@@ -338,6 +360,8 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,
if (mode->status == MODE_OK && connector_funcs->mode_valid)
mode->status = connector_funcs->mode_valid(connector,
   mode);
+
+   mode->status = drm_connector_check_crtc_modes(connector, mode);
}
 
 prune:
diff --git a/include/drm/drm_modeset_helper_vtables.h 
b/include/drm/drm_modeset_helper_vtables.h
index 69c3974..53ca0e4 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -105,6 +105,32 @@ struct drm_crtc_helper_funcs {
void (*commit)(struct drm_crtc *crtc);
 
/**
+* @mode_valid:
+*
+* Callback to validate a mode for a crtc, irrespective of the
+* specific display configuration.
+*
+* This callback is used by the probe helpers to filter the mode list
+* (which is usually derived from the EDID data block from the sink).
+* See e.g. drm_helper_probe_single_connector_modes().
+*
+* NOTE:
+*
+* This only filters the mode list supplied to userspace in the
+* GETCONNECOTR IOCTL. Userspace is free to create modes of its own and
+* ask the kernel to use them. It this case the atomic helpers or legacy
+* CRTC helpers will not call this function. Drivers therefore must
+* still fully validate any mode passed in in a modeset request.
+*
+* RETURNS:
+*
+* Either MODE_OK or one of the failure reasons in enum
+* _mode_status.
+*/
+   enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
+  struct drm_display_mode *mode);
+
+   /**
 * @mode_fixup:
 *
 * This callback is used to validate a mode. The parameter mode is the
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org

[RFC][PATCH 2/2] drm: kirin: Restrict modes to known good mode clocks

[John Stultz]

Currently the hikey dsi logic cannot generate accurate byte
clocks values for all pixel clock values. Thus if a mode clock
is selected that cannot match the calculated byte clock, the
device will boot with a blank screen.

This patch uses the new mode_valid callback to enforces known
good mode clocks for well known resolutions, which should allow
the display to work from given EDID options, and ensures for a
given resolution & refresh, the right mode clock is selected.

Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Rob Clark 
Cc: Xinliang Liu 
Cc: Xinliang Liu 
Cc: Rongrong Zou 
Cc: Xinwei Kong 
Cc: Chen Feng 
Cc: Archit Taneja 
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz 
---
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 38 +
 1 file changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
index afc2b5d..9161633 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
@@ -504,6 +504,43 @@ static void ade_crtc_disable(struct drm_crtc *crtc)
acrtc->enable = false;
 }
 
+static enum drm_mode_status ade_crtc_mode_valid(struct drm_crtc *crtc,
+   struct drm_display_mode *mode)
+{
+   /*
+* kirin_ade cannot generate all modes, so use the whitelist below
+*/
+   DRM_DEBUG("Checking mode %ix%i@%i clock: %i...",
+ mode->hdisplay, mode->vdisplay, drm_mode_vrefresh(mode), 
mode->clock);
+   if ((mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock == 
148500) ||
+   (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock == 
80192)  ||
+   (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock == 
74250)  ||
+   (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock == 
61855)  ||
+   (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock == 
147116) ||
+   (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock == 
146250) ||
+   (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock == 
144589) ||
+   (mode->hdisplay == 1600 && mode->vdisplay == 1200 && mode->clock == 
160961) ||
+   (mode->hdisplay == 1600 && mode->vdisplay == 900  && mode->clock == 
118963) ||
+   (mode->hdisplay == 1440 && mode->vdisplay == 900  && mode->clock == 
126991) ||
+   (mode->hdisplay == 1280 && mode->vdisplay == 1024 && mode->clock == 
128946) ||
+   (mode->hdisplay == 1280 && mode->vdisplay == 1024 && mode->clock == 
98619)  ||
+   (mode->hdisplay == 1280 && mode->vdisplay == 960  && mode->clock == 
102081) ||
+   (mode->hdisplay == 1280 && mode->vdisplay == 800  && mode->clock == 
83496)  ||
+   (mode->hdisplay == 1280 && mode->vdisplay == 720  && mode->clock == 
74440)  ||
+   (mode->hdisplay == 1280 && mode->vdisplay == 720  && mode->clock == 
74250)  ||
+   (mode->hdisplay == 1024 && mode->vdisplay == 768  && mode->clock == 
78800)  ||
+   (mode->hdisplay == 1024 && mode->vdisplay == 768  && mode->clock == 
75000)  ||
+   (mode->hdisplay == 1024 && mode->vdisplay == 768  && mode->clock == 
81833)  ||
+   (mode->hdisplay == 800  && mode->vdisplay == 600  && mode->clock == 
48907)  ||
+   (mode->hdisplay == 800  && mode->vdisplay == 600  && mode->clock == 
4)) {
+   mode->type |= DRM_MODE_TYPE_PREFERRED;
+   DRM_DEBUG("OK\n");
+   return MODE_OK;
+   }
+   DRM_DEBUG("BAD\n");
+   return MODE_BAD;
+}
+
 static void ade_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
struct ade_crtc *acrtc = to_ade_crtc(crtc);
@@ -557,6 +594,7 @@ static void ade_crtc_atomic_flush(struct drm_crtc *crtc,
 static const struct drm_crtc_helper_funcs ade_crtc_helper_funcs = {
.enable = ade_crtc_enable,
.disable= ade_crtc_disable,
+   .mode_valid = ade_crtc_mode_valid,
.mode_set_nofb  = ade_crtc_mode_set_nofb,
.atomic_begin   = ade_crtc_atomic_begin,
.atomic_flush   = ade_crtc_atomic_flush,
-- 
2.7.4

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

[Bug 98784] Talos Principle rendering flickering garbage on start instead of its logo and loading squares

[bugzilla-daemon]

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

--- Comment #16 from Torbjörn Andersson  ---
I can't reproduce the glitch any more. Perhaps today's update of the game fixed
it?

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 65968] Massive memory corruption in Planetary Annihilation Alpha

[bugzilla-daemon]

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

Andreas Ringlstetter  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |INVALID

--- Comment #12 from Andreas Ringlstetter  ---
It's a bug in PA itself, not in Mesa.

The root cause is a race condition on the shared buffer which is used to
transfer the rendered HTML UI from the Coherent host process back to PA.

There is a missing mutex inside PA when the buffer gets reallocated as a result
of a window resize event. Effectively, this results in a use-after-free by the
render thread of the PA process.

The faster the realloc, the lower the chance of this bug occurring.
It's also subject to possibly missing protections against use after free
conditions on previously shared buffers. And also to the memory allocation
strategy, as a reuse of the same memory region without a clear leads to the
most visible effect.

Unfortunately, various Mesa drivers so not wipe the video memory after a buffer
was returned to the global pool!

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amd/dc: hw_sequencer: fix semicolon.cocci warnings

[Harry Wentland]

Reviewed-by: Harry Wentland 

Harry

On 2017-02-14 01:19 AM, Julia Lawall wrote:

Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Harry Wentland 
Signed-off-by: Julia Lawall 
Signed-off-by: Fengguang Wu 
---

tree:   git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9
head:   79d2de1bcb650296adff1cb08bfbf1501a6e6e14
commit: bad4c165a6986a131cdd1455507ba3857baaa561 [201/657] drm/amd/dc: Add
dc display driver

  dce110_hw_sequencer.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_hw_sequencer.c
@@ -1643,7 +1643,7 @@ static void init_hw(struct core_dc *dc)
true);
}

-   dce_clock_gating_power_up(dc->hwseq, false);;
+   dce_clock_gating_power_up(dc->hwseq, false);
/***/

for (i = 0; i < dc->link_count; i++) {


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

[Bug 98869] Electronic Super Joy graphic artefacts (regression,bisected)

[bugzilla-daemon]

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

--- Comment #25 from cosiek...@o2.pl ---
I can do more testing if needed.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amd/dc: fix semicolon.cocci warnings

[Harry Wentland]

Reviewed-by: Harry Wentland 

Harry

On 2017-02-14 01:13 AM, Julia Lawall wrote:

Remove unneeded semicolons.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Harry Wentland 
Signed-off-by: Julia Lawall 
Signed-off-by: Fengguang Wu 
---

tree:   git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9
head:   79d2de1bcb650296adff1cb08bfbf1501a6e6e14
commit: bad4c165a6986a131cdd1455507ba3857baaa561 [201/657] drm/amd/dc: Add
dc display driver

  amdgpu_dm_types.c |   10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

--- a/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_types.c
+++ b/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_types.c
@@ -890,11 +890,11 @@ static void copy_crtc_timing_for_drm_dis
dst_mode->crtc_hsync_end = src_mode->crtc_hsync_end;
dst_mode->crtc_htotal = src_mode->crtc_htotal;
dst_mode->crtc_hskew = src_mode->crtc_hskew;
-   dst_mode->crtc_vblank_start = src_mode->crtc_vblank_start;;
-   dst_mode->crtc_vblank_end = src_mode->crtc_vblank_end;;
-   dst_mode->crtc_vsync_start = src_mode->crtc_vsync_start;;
-   dst_mode->crtc_vsync_end = src_mode->crtc_vsync_end;;
-   dst_mode->crtc_vtotal = src_mode->crtc_vtotal;;
+   dst_mode->crtc_vblank_start = src_mode->crtc_vblank_start;
+   dst_mode->crtc_vblank_end = src_mode->crtc_vblank_end;
+   dst_mode->crtc_vsync_start = src_mode->crtc_vsync_start;
+   dst_mode->crtc_vsync_end = src_mode->crtc_vsync_end;
+   dst_mode->crtc_vtotal = src_mode->crtc_vtotal;
  }

  static void decide_crtc_timing_for_drm_display_mode(


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

Re: [PATCH] drm/amd/dc: fix semicolon.cocci warnings

[Harry Wentland]

Reviewed-by: Harry Wentland 

Harry

On 2017-02-14 01:14 AM, Julia Lawall wrote:

Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Harry Wentland 
Signed-off-by: Julia Lawall 
Signed-off-by: Fengguang Wu 
---

tree:   git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9
head:   79d2de1bcb650296adff1cb08bfbf1501a6e6e14
commit: bad4c165a6986a131cdd1455507ba3857baaa561 [201/657] drm/amd/dc: Add
dc display driver

  dc_resource.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
@@ -135,7 +135,7 @@ static void update_num_audio(
break;
default:
DC_ERR("DC: unexpected audio fuse!\n");
-   };
+   }
  }

  bool resource_construct(


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

Re: [PATCH v2] drm/amd/dc: resource: fix semicolon.cocci warnings (fwd)

[Harry Wentland]

Thanks for these fixes. I'll merge them.

Reviewed-by: Harry Wentland 

Harry

On 2017-02-14 04:47 AM, Christian König wrote:

Am 14.02.2017 um 07:21 schrieb Julia Lawall:

Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Harry Wentland 
Signed-off-by: Julia Lawall 
Signed-off-by: Fengguang Wu 


Acked-by: Christian König  for this one as 
well as the other similar patches.


Thanks for letting us know about those typos.

Regards,
Christian.


---

v2: make subject line unique

tree:   git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9
head:   79d2de1bcb650296adff1cb08bfbf1501a6e6e14
commit: bad4c165a6986a131cdd1455507ba3857baaa561 [201/657] 
drm/amd/dc: Add

dc display driver

  dc_resource.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
@@ -135,7 +135,7 @@ static void update_num_audio(
  break;
  default:
  DC_ERR("DC: unexpected audio fuse!\n");
-};
+}
  }

  bool resource_construct(





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

[Bug 99801] Rx480 doesn't output properly onto z27q at 5120x2880

[bugzilla-daemon]

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

--- Comment #12 from Harry Wentland  ---
We're currently debugging an issue that looks very similar, if not the same.
It's also with a 5k display with two DP input but on a different platform. The
display pipe driving one half of the display is underflowing. Hope we'll find a
fix soon.

Not sure about the issue in 4k mode but there's a good chance the display pipe
is underflowing for the same reason as the problem in 5k mode.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99815] Power management problems & kernel hangs with Cap Verde

[bugzilla-daemon]

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

Bug ID: 99815
   Summary: Power management problems & kernel hangs with Cap
Verde
   Product: DRI
   Version: unspecified
  Hardware: Other
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: klaus.kus...@computerix.info

I recently switched from the radeon kernel driver to the amdgpu kernel driver.
As a consequence, battery lifetime of my laptop was more than halved
(before I could give *two* 90 minutes lectures in a row running on battery, 
now the battery is empty after around 70 minutes),
i.e. with amdgpu, the graphic card consumes a multiple of the power it needed
with radeon.

GPU is AMDGPU(0): Chipset: "VERDE" (ChipID = 0x6825)
Laptop is Dell Precision M6700
OS is linux 4.9, userland is running latest official releases (Gentoo)
Typical load is two screens (internal + VGA-Beamer, both 1024x768 mirrored)
with very light load (no 3D & no video at all, just static pdf slides).

Power settings in battery mode with the old radeon driver were
* no dpm 
(always was very unstable and consumed by far more power than "classic" pm)
* "classic" pm with power_method = profile and power_profile = low

Analysis of the problems with amdgpu showed:

1.) There is no "classic" power management: If dpm is turned off with amdgpu,
there is no power management at all, and the card permanently runs
with high power. Why was "classic" power management dropped completely?

Basically, I neither want nor need any dynamic adjustment:
When on battery, the gpu should permanently run on minimal power,
independent of its load.

2.) The default dpm mode is "balanced" and "auto".
"balanced" does not change to "battery" automatically when AC is removed,
and it is impossible to set the mode to "battery" manually:
Any attempt to set it via sysfs either kills the process doing so
or hangs it in a kill-9-immune state (which prevents system shutdown).

I then code-changed the default initialization in si_dpm.c from "balanced"
to "battery". This hangs the whole kernel hard during early boot:
No display, no reaction to Alt-Sysrq, ... only hard power off helps.

3.) It is possible to set the dpm level from "auto" to "low",
but that does not seem to result in any measurable effect or power savings?

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC simple allocator v2 0/2] Simple allocator

[Mark Brown]

On Mon, Feb 13, 2017 at 11:01:14AM -0800, Laura Abbott wrote:
> On 02/13/2017 10:18 AM, Mark Brown wrote:

> > The software defined networking people seemed to think they had a use
> > case for this as well.  They're not entirely upstream of course but
> > still...

> This is the first I've heard of anything like this. Do you have any more
> details/reading?

No, unfortunately it was in a meeting and I was asking for more details
on what specifically the hardware was doing myself.  My understanding is
that it's very similar to the GPU/video needs.


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

Re: [PATCH v4 5/7] dt-bindings: display/panel: Add common rotation property

[Rob Herring]

On Sat, Feb 11, 2017 at 12:48 PM, Noralf Trønnes  wrote:
> Display panels can be oriented many ways, especially in the embedded
> world. The rotation property is a way to describe this orientation.
> The counter clockwise direction is chosen because that's what fbdev
> and drm use.
>
> Signed-off-by: Noralf Trønnes 
> Acked-by: Thierry Reding 
> ---
>
> Changes since version 3:
> - Changed display/display.txt -> display/panel/panel.txt
>
>  Documentation/devicetree/bindings/display/panel/panel.txt | 4 
>  1 file changed, 4 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel.txt

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

[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460

[bugzilla-daemon]

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

--- Comment #32 from alvarex  ---
I'm sorry but I can't find a consistent way of reproducing the bug. I presumed
that with 4.9 the bug will still be there but right now, no matter how hard I
try I cannot reproduce with 4.9.4, 4.9.9 and 4.9rc1, I think that in my case is
a hardware problem and possibly unrelated to Reimar bug. It seems to me that
the artifacting varies under load, or under temperature, just that maybe some
kernel versions mitigate the problem and it goes unnoticed and some a aggravate
the problem (IMHO). 
I really don't know what the cause is in my case.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 194559] amdgpu problems loading 2 firmwares on multi-smp system

[bugzilla-daemon]

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

Christian König (deathsim...@vodafone.de) changed:

   What|Removed |Added

 CC||deathsim...@vodafone.de

--- Comment #8 from Christian König (deathsim...@vodafone.de) ---
(In reply to fin4478 from comment #7)
> You clearly want bad reputation for Amd gpus so I stop giving this info.

Well as an AMD employee I can only advise you to stop giving incorrect
informations.

Alex branches only contain additional features not upstream yet, so they are
way more unstable than the upstream kernel driver.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 194579] AMDGPU: Possible size overflow detected by PaX in ttm_bo_handle_move_mem (drivers/gpu/drm/ttm/ttm_bo.c:388)

[bugzilla-daemon]

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

--- Comment #5 from fin4...@hotmail.com ---
(In reply to Christian König from comment #3)
> Please ignore the incorrect comment by fin4...@hotmail.com.
> 
> The upstream kernel the official base amdgpu driver code. Only a few not
> parts which are not upstream yet are in private repositories waiting for
> cleanup.
> 
> Regards,
> Christian (co-maintainer of the amdgpu module).

Amd developers want bad bad reputation for amd gpus, so I stop giving info.
Here is an example how the wip kernel solved the problem:
https://bugzilla.kernel.org/show_bug.cgi?id=194559

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 194559] amdgpu problems loading 2 firmwares on multi-smp system

[bugzilla-daemon]

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

--- Comment #7 from fin4...@hotmail.com ---
(In reply to Michel Dänzer from comment #6)
> (In reply to fin4478 from comment #5)
> > This and many other amdgpu bug reports prove my point.
> 
> Your bug report comments like this one rather indicate that you don't
> understand how the kernel development process works.

You do not see how agd5f wip kernel solved this and many other problems.

Amd should warn not use stock kernels and tell how to use use  ~agd5f wip
kernel and latest mesa git. Here is the page for you, dear Amd:
http://support.amd.com/en-us/download/linux

You clearly want bad reputation for Amd gpus so I stop giving this info.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] dma-buf/fence: Avoid use of uninitialised timestamp

[Chris Wilson]

On Tue, Feb 14, 2017 at 12:22:02PM -0200, Gustavo Padovan wrote:
> 2017-02-14 Chris Wilson :
> 
> > On Tue, Feb 14, 2017 at 11:40:38AM -0200, Gustavo Padovan wrote:
> > > Hi Chris,
> > > 
> > > 2017-02-14 Chris Wilson :
> > One thing that occurs to me is whether we should be setting the
> > timestamp when we set an error. The above (sync_debug though) implies
> > that it expects the error to have the timestamp. sync_fence_info could
> > go either way.
> 
> We could do it. I don't see any reason against it.

It is just uncertain as to what timestamp means, and what the user
wants. In i915 we may end up flagging an error on a fence long (many
10s) before the fence is signaled. It still feels more appropriate to
set the timestamp on when the fence is complete - certainly for the case
where we replay the request (and we flag fence->error that we did so as
there may have been side-effects that we should inform the user about).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] dma-buf/fence: Avoid use of uninitialised timestamp

[Gustavo Padovan]

2017-02-14 Chris Wilson :

> On Tue, Feb 14, 2017 at 11:40:38AM -0200, Gustavo Padovan wrote:
> > Hi Chris,
> > 
> > 2017-02-14 Chris Wilson :
> > > diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
> > > index c769dc653b34..bfead12390f2 100644
> > > --- a/drivers/dma-buf/sync_debug.c
> > > +++ b/drivers/dma-buf/sync_debug.c
> > > @@ -84,7 +84,7 @@ static void sync_print_fence(struct seq_file *s,
> > >  show ? "_" : "",
> > >  sync_status_str(status));
> > >  
> > > - if (status) {
> > > + if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags)) {
> > >   struct timespec64 ts64 =
> > >   ktime_to_timespec64(fence->timestamp);
> > 
> > How about add this test_bit() to dma_fence_is_signaled_locked() so
> > we test both for DMA_FENCE_FLAG_SIGNALED_BIT and
> > DMA_FENCE_FLAG_TIMESTAMP_BIT there at the same time?
> 
> I was thinking of only using it as communication with the timestamp
> user. That avoids getting into the situation as to which bit truly means
> is-signaled and we still only synchronize on SIGNALED_BIT.
> 
> It would be possible, but I don't think it makes anything simpler.

Yes, it doesn't make anything better. We should keep it that way for
users that doesn't need timestamp.

> 
> One thing that occurs to me is whether we should be setting the
> timestamp when we set an error. The above (sync_debug though) implies
> that it expects the error to have the timestamp. sync_fence_info could
> go either way.

We could do it. I don't see any reason against it.

Gustavo

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

[drm-intel:drm-intel-next-queued 31/59] drivers/gpu/drm/i915/selftests/i915_gem_coherency.c:274: error: 'err' may be used uninitialized in this function

[kbuild test robot]

tree:   git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head:   d892e9398ecf6defc7972a62227b77dad6be20bd
commit: 170594502cf591fd0789d7e5239937b1a87af4c6 [31/59] drm/i915: Test 
coherency of and barriers between cache domains
config: x86_64-randconfig-s2-02141638 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
git checkout 170594502cf591fd0789d7e5239937b1a87af4c6
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All errors (new ones prefixed by >>):

   cc1: warnings being treated as errors
   In file included from drivers/gpu/drm/i915/i915_gem.c:5029:
   drivers/gpu/drm/i915/selftests/i915_gem_coherency.c: In function 
'igt_gem_coherency':
>> drivers/gpu/drm/i915/selftests/i915_gem_coherency.c:274: error: 'err' may be 
>> used uninitialized in this function
   drivers/gpu/drm/i915/i915_gem.c: In function 'i915_gem_object_map':
   drivers/gpu/drm/i915/i915_gem.c:2467: error: 'pgprot.pgprot' may be used 
uninitialized in this function

vim +/err +274 drivers/gpu/drm/i915/selftests/i915_gem_coherency.c

   268  I915_RND_STATE(prng);
   269  struct drm_i915_private *i915 = arg;
   270  const struct igt_coherency_mode *read, *write, *over;
   271  struct drm_i915_gem_object *obj;
   272  unsigned long count, n;
   273  u32 *offsets, *values;
 > 274  int err;
   275  
   276  /* We repeatedly write, overwrite and read from a sequence of
   277   * cachelines in order to try and detect incoherency (unflushed 
writes

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: make TTM_MAX_BO_PRIORITY unsigned

[Christian König]

Am 14.02.2017 um 13:25 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

Fix a warning about different types in min() macro in amdgpu:

In file included from ./include/linux/list.h:8:0,
  from drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:32:
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function 
‘amdgpu_bo_create_restricted’:
./include/linux/kernel.h:739:16: warning: comparison of distinct pointer types 
lacks a cast
   (void) ( == );   \
 ^
./include/linux/kernel.h:742:2: note: in expansion of macro ‘__min’
   __min(typeof(x), typeof(y),   \
   ^
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:434:21: note: in expansion of macro 
‘min’
   bo->tbo.priority = min(bo->tbo.priority, TTM_MAX_BO_PRIORITY - 1);
  ^~~

Signed-off-by: Nicolai Hähnle 


Reviewed-by: Christian König 


---
  include/drm/ttm/ttm_bo_driver.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4395db1..c3d74be 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -42,7 +42,7 @@
  #include 
  #include 
  
-#define TTM_MAX_BO_PRIORITY	16

+#define TTM_MAX_BO_PRIORITY16U
  
  struct ttm_backend_func {

/**



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

[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460

[bugzilla-daemon]

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

--- Comment #31 from alvarex  ---
I will try to git bisect but last time I tried bisecting the kernel I failed
miserably and it failed compiling several times.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] dma-buf/fence: Avoid use of uninitialised timestamp

[Chris Wilson]

On Tue, Feb 14, 2017 at 11:40:38AM -0200, Gustavo Padovan wrote:
> Hi Chris,
> 
> 2017-02-14 Chris Wilson :
> > diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
> > index c769dc653b34..bfead12390f2 100644
> > --- a/drivers/dma-buf/sync_debug.c
> > +++ b/drivers/dma-buf/sync_debug.c
> > @@ -84,7 +84,7 @@ static void sync_print_fence(struct seq_file *s,
> >show ? "_" : "",
> >sync_status_str(status));
> >  
> > -   if (status) {
> > +   if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags)) {
> > struct timespec64 ts64 =
> > ktime_to_timespec64(fence->timestamp);
> 
> How about add this test_bit() to dma_fence_is_signaled_locked() so
> we test both for DMA_FENCE_FLAG_SIGNALED_BIT and
> DMA_FENCE_FLAG_TIMESTAMP_BIT there at the same time?

I was thinking of only using it as communication with the timestamp
user. That avoids getting into the situation as to which bit truly means
is-signaled and we still only synchronize on SIGNALED_BIT.

It would be possible, but I don't think it makes anything simpler.

One thing that occurs to me is whether we should be setting the
timestamp when we set an error. The above (sync_debug though) implies
that it expects the error to have the timestamp. sync_fence_info could
go either way.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] dma-buf/fence: Avoid use of uninitialised timestamp

[Gustavo Padovan]

Hi Chris,

2017-02-14 Chris Wilson :

> [  236.821534] WARNING: kmemcheck: Caught 64-bit read from uninitialized 
> memory (8802538683d0)
> [  236.828642] 
> 42001e7f0008
> [  236.839543]  i i i i u u u u i i i i i i i i u u u u u u u u u u u u u u u 
> u
> [  236.850420]  ^
> [  236.854123] RIP: 0010:[]  [] 
> fence_signal+0x17/0xd0
> [  236.861313] RSP: 0018:88024acd7ba0  EFLAGS: 00010282
> [  236.865027] RAX: 812f6a90 RBX: 8802527ca800 RCX: 
> 880252cb30e0
> [  236.868801] RDX: 88024ac5d918 RSI: 880252f780e0 RDI: 
> 880253868380
> [  236.872579] RBP: 88024acd7bc0 R08: 88024acd7be0 R09: 
> 
> [  236.876407] R10:  R11:  R12: 
> 880253868380
> [  236.880185] R13: 8802538684d0 R14: 880253868380 R15: 
> 88024cd48e00
> [  236.883983] FS:  7f1646d1a740() GS:88025d00() 
> knlGS:
> [  236.890959] CS:  0010 DS:  ES:  CR0: 80050033
> [  236.894702] CR2: 880251360318 CR3: 00024ad21000 CR4: 
> 001406f0
> [  236.898481]  [] i915_gem_request_retire+0x1cd/0x230
> [  236.902439]  [] i915_gem_request_alloc+0xa3/0x2f0
> [  236.906435]  [] 
> i915_gem_do_execbuffer.isra.41+0xb6d/0x18b0
> [  236.910434]  [] i915_gem_execbuffer2+0x95/0x1e0
> [  236.914390]  [] drm_ioctl+0x1e5/0x460
> [  236.918275]  [] do_vfs_ioctl+0x8f/0x5c0
> [  236.922168]  [] SyS_ioctl+0x3c/0x70
> [  236.926090]  [] entry_SYSCALL_64_fastpath+0x17/0x93
> [  236.930045]  [] 0x
> 
> We only set the timestamp before we mark the fence as signaled. It is
> done before to avoid observers having a window in which they may see the
> fence as complete but no timestamp. Having it does incur a potential for
> the timestamp to be written twice, and even for it to be corrupted if
> the u64 write is not atomic. Instead use a new bit to record the
> presence of the timestamp, and teach the readers to wait until it is set
> if the fence is complete. There still remains a race where the timestamp
> for the signaled fence may be shown before the fence is reported as
> signaled, but that's a pre-existing error.
> 
> Signed-off-by: Chris Wilson 
> Cc: Sumit Semwal 
> Cc: Gustavo Padovan 
> Cc: Daniel Vetter 
> ---
>  drivers/dma-buf/dma-fence.c  | 17 ++---
>  drivers/dma-buf/sync_debug.c |  2 +-
>  drivers/dma-buf/sync_file.c  |  8 +++-
>  include/linux/dma-fence.h|  2 ++
>  4 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index d1f1f456f5c4..dd2d7b6d2831 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -74,11 +74,6 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>   if (WARN_ON(!fence))
>   return -EINVAL;
>  
> - if (!ktime_to_ns(fence->timestamp)) {
> - fence->timestamp = ktime_get();
> - smp_mb__before_atomic();
> - }
> -
>   if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) {
>   ret = -EINVAL;
>  
> @@ -86,8 +81,11 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>* we might have raced with the unlocked dma_fence_signal,
>* still run through all callbacks
>*/
> - } else
> + } else {
> + fence->timestamp = ktime_get();
> + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
>   trace_dma_fence_signaled(fence);
> + }
>  
>   list_for_each_entry_safe(cur, tmp, >cb_list, node) {
>   list_del_init(>node);
> @@ -114,14 +112,11 @@ int dma_fence_signal(struct dma_fence *fence)
>   if (!fence)
>   return -EINVAL;
>  
> - if (!ktime_to_ns(fence->timestamp)) {
> - fence->timestamp = ktime_get();
> - smp_mb__before_atomic();
> - }
> -
>   if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags))
>   return -EINVAL;
>  
> + fence->timestamp = ktime_get();
> + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
>   trace_dma_fence_signaled(fence);
>  
>   if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) {
> diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
> index c769dc653b34..bfead12390f2 100644
> --- a/drivers/dma-buf/sync_debug.c
> +++ b/drivers/dma-buf/sync_debug.c
> @@ -84,7 +84,7 @@ static void sync_print_fence(struct seq_file *s,
>  show ? "_" : "",
>  sync_status_str(status));
>  
> - if (status) {
> + if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags)) {
>   struct timespec64 ts64 =
>   ktime_to_timespec64(fence->timestamp);

How about add this test_bit() to 

Re: [PATCH 1/2] drm/ttm: never add BO that failed to validate to the LRU list

[Christian König]

Am 14.02.2017 um 12:37 schrieb Nicolai Hähnle:

On 14.02.2017 11:38, Christian König wrote:

Am 14.02.2017 um 10:18 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

Fixes a potential race condition in amdgpu that looks as follows:

Task 1: attempt ttm_bo_init, but ttm_bo_validate fails
Task 1: add BO to global list anyway
Task 2: grabs hold of the BO, waits on its reservation lock
Task 1: releases its reference of the BO; never gives up the
 reservation lock

The patch "drm/amdgpu: fix a potential deadlock in
amdgpu_bo_create_restricted()" attempts to fix that by releasing
the reservation lock in amdgpu code; unfortunately, it introduces
a use-after-free when this race _doesn't_ happen.

This patch should fix the race properly by never adding the BO
to the global list in the first place.

Cc: Samuel Pitoiset 
Cc: zhoucm1 
Signed-off-by: Nicolai Hähnle 


NAK, that is actually not correct either.

The previous implementation doesn't have an use after free, but actually
a memory leak.

I completely agree that adding the BO to the LRU in case of an error is
incorrect, but ttm_bo_add_to_lru() will add some references to the BO.


Yes, but those are bo->list_krefs, not bo->krefs. ttm_bo_unref will 
put the bo->kref, which should lead to ttm_bo_release. This will then 
add the buffer to the ddestroy list and eventually put all the 
list_krefs. (Or possibly, if the ttm_bo_wait in 
ttm_bo_cleanup_refs_or_queue fails, it will explicitly delete the 
buffer from the LRU, which should amount to the same, I think).


Or perhaps I'm still missing something, I'm not too familiar with the 
whole BO reference counting.


Ah, yes of course, never mind.





So the following ttm_bo_unref() will just drop the initial reference.

Let's clean up this mess by moving the freeing of the BO in case of an
error to the caller.


Yes, see my other patches.


In this case the set is Reviewed-by: Christian König 
.


Regards,
Christian.



Thanks,
Nicolai




Regards,
Christian.


---
  drivers/gpu/drm/ttm/ttm_bo.c | 12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
b/drivers/gpu/drm/ttm/ttm_bo.c

index 239a957..76bee42 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1215,18 +1215,20 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
  if (likely(!ret))
  ret = ttm_bo_validate(bo, placement, interruptible, false);
  -if (!resv) {
+if (!resv)
  ttm_bo_unreserve(bo);
  -} else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+if (unlikely(ret)) {
+ttm_bo_unref();
+return ret;
+}
+
+if (resv && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
  spin_lock(>glob->lru_lock);
  ttm_bo_add_to_lru(bo);
  spin_unlock(>glob->lru_lock);
  }
  -if (unlikely(ret))
-ttm_bo_unref();
-
  return ret;
  }
  EXPORT_SYMBOL(ttm_bo_init);







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

Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

[Christian König]

Am 14.02.2017 um 13:00 schrieb Nicolai Hähnle:

On 14.02.2017 11:49, Christian König wrote:

Am 14.02.2017 um 11:37 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle 


Please squash that into your other patch. It fixes another bug, but I
don't think fixing one bug just to run into another is really a good 
idea.


I don't understand. I'm not aware that this patch fixes anything, it 
just enables the subsequent fix in amdgpu in patch #2. I don't think 
squashing those together is a good idea (one is in ttm, the other in 
amdgpu).


Ok, forget it I've messed up the different reference count.

With at least initializing bo->kref and bo->destroy before returning the 
first error the patch is Reviewed-by: Christian König 
.


Regards,
Christian.





Additional to that one comment below.


---
  drivers/gpu/drm/ttm/ttm_bo.c | 62
+---
  include/drm/ttm/ttm_bo_api.h | 45 
  2 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
b/drivers/gpu/drm/ttm/ttm_bo.c

index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object 
*bo,

  }
  EXPORT_SYMBOL(ttm_bo_validate);
  -int ttm_bo_init(struct ttm_bo_device *bdev,
-struct ttm_buffer_object *bo,
-unsigned long size,
-enum ttm_bo_type type,
-struct ttm_placement *placement,
-uint32_t page_alignment,
-bool interruptible,
-struct file *persistent_swap_storage,
-size_t acc_size,
-struct sg_table *sg,
-struct reservation_object *resv,
-void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+uint32_t page_alignment,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
  {
  int ret = 0;
  unsigned long num_pages;
  struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-bool locked;
ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
  if (ret) {
  pr_err("Out of kernel memory\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  return -ENOMEM;
  }
num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
  if (num_pages == 0) {
  pr_err("Illegal buffer object size\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  ttm_mem_global_free(mem_glob, acc_size);
  return -EINVAL;
  }


I would move those checks after all the field initializations. This way
the structure has at least a valid content and we can safely use
ttm_bo_unref on it.


That feels odd to me, since the return value indicates that the buffer 
wasn't properly initialized, but I don't feel strongly about it.


Cheers,
Nicolai




Regards,
Christian.


@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
  ret = drm_vma_offset_add(>vma_manager, >vma_node,
   bo->mem.num_pages);
  +return ret;
+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+struct ttm_placement *placement,
+uint32_t page_alignment,
+bool interruptible,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
+{
+bool locked;
+int ret;
+
+ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+  persistent_swap_storage, acc_size, sg, resv,
+  destroy);
+if (ret) {
+if (destroy)
+(*destroy)(bo);
+else
+kfree(bo);
+return ret;
+}
+
  /* passed reservation objects should already be locked,
   * since otherwise lockdep will be angered in radeon.
   */
diff --git a/include/drm/ttm/ttm_bo_api.h 
b/include/drm/ttm/ttm_bo_api.h

index f195899..d44b8e4 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -453,6 +453,51 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device
*bdev,
 unsigned struct_size);
/**
+ * ttm_bo_init_top
+ *
+ * @bdev: 

[PATCH RESEND] drm/dp/mst: fix kernel oops when turning off secondary monitor

[Jani Nikula]

From: Pierre-Louis Bossart 

100% reproducible issue found on SKL SkullCanyon NUC with two external
DP daisy-chained monitors in DP/MST mode. When turning off or changing
the input of the second monitor the machine stops with a kernel
oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.

This issue is traced to an inconsistent control flow in
drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the
same time as 'req_payload.num_slots' is set to zero, but the pointer is
dereferenced even when req_payload.num_slot is zero.

The problematic dereference was introduced in commit dfda0df34
("drm/mst: rework payload table allocation to conform better") and may
impact all versions since v3.18

The fix suggested by Chris Wilson removes the kernel oops and was found to
work well after 10mn of monkey-testing with the second monitor power and
input buttons

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Fixes: dfda0df34264 ("drm/mst: rework payload table allocation to conform 
better.")
Cc: Dave Airlie 
Cc: Chris Wilson 
Cc: Nathan D Ciobanu 
Cc: Dhinakaran Pandiyan 
Cc: Sean Paul 
Cc:  # v3.18+
Tested-by: Nathan D Ciobanu 
Reviewed-by: Dhinakaran Pandiyan 
Signed-off-by: Pierre-Louis Bossart 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 122a1b04bebc..f2cc375907d0 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
mgr->payloads[i].vcpi = req_payload.vcpi;
} else if (mgr->payloads[i].num_slots) {
mgr->payloads[i].num_slots = 0;
-   drm_dp_destroy_payload_step1(mgr, port, 
port->vcpi.vcpi, >payloads[i]);
+   drm_dp_destroy_payload_step1(mgr, port, 
mgr->payloads[i].vcpi, >payloads[i]);
req_payload.payload_state = 
mgr->payloads[i].payload_state;
mgr->payloads[i].start_slot = 0;
}
-- 
2.1.4

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

[PATCH] dma-buf/fence: Avoid use of uninitialised timestamp

[Chris Wilson]

[  236.821534] WARNING: kmemcheck: Caught 64-bit read from uninitialized memory 
(8802538683d0)
[  236.828642] 42001e7f0008
[  236.839543]  i i i i u u u u i i i i i i i i u u u u u u u u u u u u u u u u
[  236.850420]  ^
[  236.854123] RIP: 0010:[]  [] 
fence_signal+0x17/0xd0
[  236.861313] RSP: 0018:88024acd7ba0  EFLAGS: 00010282
[  236.865027] RAX: 812f6a90 RBX: 8802527ca800 RCX: 880252cb30e0
[  236.868801] RDX: 88024ac5d918 RSI: 880252f780e0 RDI: 880253868380
[  236.872579] RBP: 88024acd7bc0 R08: 88024acd7be0 R09: 
[  236.876407] R10:  R11:  R12: 880253868380
[  236.880185] R13: 8802538684d0 R14: 880253868380 R15: 88024cd48e00
[  236.883983] FS:  7f1646d1a740() GS:88025d00() 
knlGS:
[  236.890959] CS:  0010 DS:  ES:  CR0: 80050033
[  236.894702] CR2: 880251360318 CR3: 00024ad21000 CR4: 001406f0
[  236.898481]  [] i915_gem_request_retire+0x1cd/0x230
[  236.902439]  [] i915_gem_request_alloc+0xa3/0x2f0
[  236.906435]  [] i915_gem_do_execbuffer.isra.41+0xb6d/0x18b0
[  236.910434]  [] i915_gem_execbuffer2+0x95/0x1e0
[  236.914390]  [] drm_ioctl+0x1e5/0x460
[  236.918275]  [] do_vfs_ioctl+0x8f/0x5c0
[  236.922168]  [] SyS_ioctl+0x3c/0x70
[  236.926090]  [] entry_SYSCALL_64_fastpath+0x17/0x93
[  236.930045]  [] 0x

We only set the timestamp before we mark the fence as signaled. It is
done before to avoid observers having a window in which they may see the
fence as complete but no timestamp. Having it does incur a potential for
the timestamp to be written twice, and even for it to be corrupted if
the u64 write is not atomic. Instead use a new bit to record the
presence of the timestamp, and teach the readers to wait until it is set
if the fence is complete. There still remains a race where the timestamp
for the signaled fence may be shown before the fence is reported as
signaled, but that's a pre-existing error.

Signed-off-by: Chris Wilson 
Cc: Sumit Semwal 
Cc: Gustavo Padovan 
Cc: Daniel Vetter 
---
 drivers/dma-buf/dma-fence.c  | 17 ++---
 drivers/dma-buf/sync_debug.c |  2 +-
 drivers/dma-buf/sync_file.c  |  8 +++-
 include/linux/dma-fence.h|  2 ++
 4 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index d1f1f456f5c4..dd2d7b6d2831 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -74,11 +74,6 @@ int dma_fence_signal_locked(struct dma_fence *fence)
if (WARN_ON(!fence))
return -EINVAL;
 
-   if (!ktime_to_ns(fence->timestamp)) {
-   fence->timestamp = ktime_get();
-   smp_mb__before_atomic();
-   }
-
if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) {
ret = -EINVAL;
 
@@ -86,8 +81,11 @@ int dma_fence_signal_locked(struct dma_fence *fence)
 * we might have raced with the unlocked dma_fence_signal,
 * still run through all callbacks
 */
-   } else
+   } else {
+   fence->timestamp = ktime_get();
+   set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
trace_dma_fence_signaled(fence);
+   }
 
list_for_each_entry_safe(cur, tmp, >cb_list, node) {
list_del_init(>node);
@@ -114,14 +112,11 @@ int dma_fence_signal(struct dma_fence *fence)
if (!fence)
return -EINVAL;
 
-   if (!ktime_to_ns(fence->timestamp)) {
-   fence->timestamp = ktime_get();
-   smp_mb__before_atomic();
-   }
-
if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags))
return -EINVAL;
 
+   fence->timestamp = ktime_get();
+   set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags);
trace_dma_fence_signaled(fence);
 
if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) {
diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
index c769dc653b34..bfead12390f2 100644
--- a/drivers/dma-buf/sync_debug.c
+++ b/drivers/dma-buf/sync_debug.c
@@ -84,7 +84,7 @@ static void sync_print_fence(struct seq_file *s,
   show ? "_" : "",
   sync_status_str(status));
 
-   if (status) {
+   if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags)) {
struct timespec64 ts64 =
ktime_to_timespec64(fence->timestamp);
 
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 2321035f6204..95f259b719fc 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -375,7 +375,13 @@ static void sync_fill_fence_info(struct dma_fence *fence,

[drm-intel:drm-intel-next-queued 23/59] drivers/gpu/drm/i915/selftests/i915_gem_request.c:280: error: 'request' may be used uninitialized in this function

[kbuild test robot]

tree:   git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head:   d892e9398ecf6defc7972a62227b77dad6be20bd
commit: b348090d6758cc391dc91f8a8233b18f0acc8458 [23/59] drm/i915: Simple 
selftest to exercise live requests
config: x86_64-randconfig-s2-02141638 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
git checkout b348090d6758cc391dc91f8a8233b18f0acc8458
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All errors (new ones prefixed by >>):

   cc1: warnings being treated as errors
   In file included from drivers/gpu/drm/i915/i915_gem_request.c:1204:
   drivers/gpu/drm/i915/selftests/i915_gem_request.c: In function 
'live_nop_request':
>> drivers/gpu/drm/i915/selftests/i915_gem_request.c:280: error: 'request' may 
>> be used uninitialized in this function

vim +/request +280 drivers/gpu/drm/i915/selftests/i915_gem_request.c

   274   */
   275  
   276  mutex_lock(>drm.struct_mutex);
   277  
   278  for_each_engine(engine, i915, id) {
   279  IGT_TIMEOUT(end_time);
 > 280  struct drm_i915_gem_request *request;
   281  unsigned long n, prime;
   282  ktime_t times[2] = {};
   283  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/ttm: make TTM_MAX_BO_PRIORITY unsigned

[Nicolai Hähnle]

From: Nicolai Hähnle 

Fix a warning about different types in min() macro in amdgpu:

In file included from ./include/linux/list.h:8:0,
 from drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:32:
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function 
‘amdgpu_bo_create_restricted’:
./include/linux/kernel.h:739:16: warning: comparison of distinct pointer types 
lacks a cast
  (void) ( == );   \
^
./include/linux/kernel.h:742:2: note: in expansion of macro ‘__min’
  __min(typeof(x), typeof(y),   \
  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:434:21: note: in expansion of macro 
‘min’
  bo->tbo.priority = min(bo->tbo.priority, TTM_MAX_BO_PRIORITY - 1);
 ^~~

Signed-off-by: Nicolai Hähnle 
---
 include/drm/ttm/ttm_bo_driver.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4395db1..c3d74be 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -42,7 +42,7 @@
 #include 
 #include 
 
-#define TTM_MAX_BO_PRIORITY16
+#define TTM_MAX_BO_PRIORITY16U
 
 struct ttm_backend_func {
/**
-- 
2.9.3

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

Re: [PATCH -next] drm/bridge/tfp410: Make symbol tfp410_platform_driver static

[Archit Taneja]

On 2/9/2017 8:55 PM, Wei Yongjun wrote:

From: Wei Yongjun 

Fixes the following sparse warning:

drivers/gpu/drm/bridge/ti-tfp410.c:223:24: warning:
 symbol 'tfp410_platform_driver' was not declared. Should it be static?



This was queued to drm-misc-next

Thanks,
Archit


Signed-off-by: Wei Yongjun 
---
 drivers/gpu/drm/bridge/ti-tfp410.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c 
b/drivers/gpu/drm/bridge/ti-tfp410.c
index b054ea3..b379d04 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -220,7 +220,7 @@ static const struct of_device_id tfp410_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tfp410_match);

-struct platform_driver tfp410_platform_driver = {
+static struct platform_driver tfp410_platform_driver = {
.probe  = tfp410_probe,
.remove = tfp410_remove,
.driver = {



--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99484] Crusader Kings 2 - Loading bars, siege bars, morale bars, etc. do not render correctly

[bugzilla-daemon]

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

--- Comment #6 from Andreas Boll  ---
(In reply to Timothy Arceri from comment #5)
> Running Mesa master with llvm 3.8 (instead of 5.0 from git) resolves the
> problem.

Be aware that llvm 3.8 degrades OpenGL support for radeonsi. Just check your
glxinfo output.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 65968] Massive memory corruption in Planetary Annihilation Alpha

[bugzilla-daemon]

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

--- Comment #11 from Timothy Arceri  ---
The game runs (mostly fine on) i965, and a trace from i965 seem to run without
issue on radeonsi.

However running the radeonsi trace on the nvidia blob results in the same
corruptions.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460

[bugzilla-daemon]

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

--- Comment #30 from alvarex  ---
it happens really fast for a fraction of a second and then it draws correctly,
I had to record the desktop and play the video in slow motion to take the
screenshot. 
And it's random sometimes it won't happen for a long period and sometimes
clicking every drop down menu will trigger it.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460

[bugzilla-daemon]

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

--- Comment #29 from alvarex  ---
Hi I think I ve run into the same issue. I'm not quite sure. Firefox and other
elements sometimes present artifacts, and as Reimar suggests with kernel 4.8 it
doesn't happen.
RX460 on Opensuse 42.2. 
It seams like some sort of memory corruption, my worst fear is that maybe this
is a defective hardware. What brand is your rx460? I have a rx460 Gigabyte
Windforce OC edition.

I attach a screenshot.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460

[bugzilla-daemon]

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

--- Comment #28 from alvarex  ---
Created attachment 129582
  --> https://bugs.freedesktop.org/attachment.cgi?id=129582=edit
artifacts on radeon rx460

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99484] Crusader Kings 2 - Loading bars, siege bars, morale bars, etc. do not render correctly

[bugzilla-daemon]

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

--- Comment #5 from Timothy Arceri  ---
Running Mesa master with llvm 3.8 (instead of 5.0 from git) resolves the
problem.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [drm-intel:drm-intel-next-queued 11/59] drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: unknown field 'mock' specified in initializer

[Chris Wilson]

On Tue, Feb 14, 2017 at 06:32:17PM +0800, kbuild test robot wrote:
> tree:   git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
> head:   d892e9398ecf6defc7972a62227b77dad6be20bd
> commit: 953c7f82eb890085c60dbe22578e883d6837c674 [11/59] drm/i915: Provide a 
> hook for selftests
> config: x86_64-randconfig-s2-02141638 (attached as .config)
> compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
> reproduce:
> git checkout 953c7f82eb890085c60dbe22578e883d6837c674
> # save the attached .config to linux build tree
> make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>In file included from drivers/gpu/drm/i915/selftests/i915_selftest.c:68:
> >> drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: unknown 
> >> field 'mock' specified in initializer
>cc1: warnings being treated as errors
> >> drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: missing 
> >> braces around initializer
>drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: (near 
> initialization for 'mock_selftests[0].')
>In file included from drivers/gpu/drm/i915/selftests/i915_selftest.c:74:
> >> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: unknown 
> >> field 'live' specified in initializer
> >> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: missing 
> >> braces around initializer
>drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: (near 
> initialization for 'live_selftests[0].')
> >> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: 
> >> initialization from incompatible pointer type
> 
> vim +/mock +11 drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> 
>  5 *
>  6 * The function should be of type int function(void). It may be 
> conditionally
>  7 * compiled using #if IS_ENABLED(DRM_I915_SELFTEST).
>  8 *
>  9 * Tests are executed in order by igt/drv_selftest
> 10 */
>   > 11selftest(sanitycheck, i915_mock_sanitycheck) /* keep first (igt 
> selfcheck) */

This is a named initializer for an anonymous union. It wasn't supported in
gcc until 4.6 :|
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

[Christian König]

Am 14.02.2017 um 11:37 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle 


Please squash that into your other patch. It fixes another bug, but I 
don't think fixing one bug just to run into another is really a good idea.


Additional to that one comment below.


---
  drivers/gpu/drm/ttm/ttm_bo.c | 62 +---
  include/drm/ttm/ttm_bo_api.h | 45 
  2 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
  }
  EXPORT_SYMBOL(ttm_bo_validate);
  
-int ttm_bo_init(struct ttm_bo_device *bdev,

-   struct ttm_buffer_object *bo,
-   unsigned long size,
-   enum ttm_bo_type type,
-   struct ttm_placement *placement,
-   uint32_t page_alignment,
-   bool interruptible,
-   struct file *persistent_swap_storage,
-   size_t acc_size,
-   struct sg_table *sg,
-   struct reservation_object *resv,
-   void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+   struct ttm_buffer_object *bo,
+   unsigned long size,
+   enum ttm_bo_type type,
+   uint32_t page_alignment,
+   struct file *persistent_swap_storage,
+   size_t acc_size,
+   struct sg_table *sg,
+   struct reservation_object *resv,
+   void (*destroy) (struct ttm_buffer_object *))
  {
int ret = 0;
unsigned long num_pages;
struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-   bool locked;
  
  	ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);

if (ret) {
pr_err("Out of kernel memory\n");
-   if (destroy)
-   (*destroy)(bo);
-   else
-   kfree(bo);
return -ENOMEM;
}
  
  	num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;

if (num_pages == 0) {
pr_err("Illegal buffer object size\n");
-   if (destroy)
-   (*destroy)(bo);
-   else
-   kfree(bo);
ttm_mem_global_free(mem_glob, acc_size);
return -EINVAL;
}


I would move those checks after all the field initializations. This way 
the structure has at least a valid content and we can safely use 
ttm_bo_unref on it.


Regards,
Christian.


@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
ret = drm_vma_offset_add(>vma_manager, >vma_node,
 bo->mem.num_pages);
  
+	return ret;

+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+   struct ttm_buffer_object *bo,
+   unsigned long size,
+   enum ttm_bo_type type,
+   struct ttm_placement *placement,
+   uint32_t page_alignment,
+   bool interruptible,
+   struct file *persistent_swap_storage,
+   size_t acc_size,
+   struct sg_table *sg,
+   struct reservation_object *resv,
+   void (*destroy) (struct ttm_buffer_object *))
+{
+   bool locked;
+   int ret;
+
+   ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+ persistent_swap_storage, acc_size, sg, resv,
+ destroy);
+   if (ret) {
+   if (destroy)
+   (*destroy)(bo);
+   else
+   kfree(bo);
+   return ret;
+   }
+
/* passed reservation objects should already be locked,
 * since otherwise lockdep will be angered in radeon.
 */
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index f195899..d44b8e4 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -453,6 +453,51 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev,
   unsigned struct_size);
  
  /**

+ * ttm_bo_init_top
+ *
+ * @bdev: Pointer to a ttm_bo_device struct.
+ * @bo: Pointer to a ttm_buffer_object to be initialized.
+ * @size: Requested size of buffer object.
+ * @type: Requested type of buffer object.
+ * @flags: Initial placement flags.
+ * @page_alignment: Data alignment in pages.

Re: [PATCH 1/2] drm/ttm: never add BO that failed to validate to the LRU list

[Nicolai Hähnle]

On 14.02.2017 11:38, Christian König wrote:

Am 14.02.2017 um 10:18 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

Fixes a potential race condition in amdgpu that looks as follows:

Task 1: attempt ttm_bo_init, but ttm_bo_validate fails
Task 1: add BO to global list anyway
Task 2: grabs hold of the BO, waits on its reservation lock
Task 1: releases its reference of the BO; never gives up the
 reservation lock

The patch "drm/amdgpu: fix a potential deadlock in
amdgpu_bo_create_restricted()" attempts to fix that by releasing
the reservation lock in amdgpu code; unfortunately, it introduces
a use-after-free when this race _doesn't_ happen.

This patch should fix the race properly by never adding the BO
to the global list in the first place.

Cc: Samuel Pitoiset 
Cc: zhoucm1 
Signed-off-by: Nicolai Hähnle 


NAK, that is actually not correct either.

The previous implementation doesn't have an use after free, but actually
a memory leak.

I completely agree that adding the BO to the LRU in case of an error is
incorrect, but ttm_bo_add_to_lru() will add some references to the BO.


Yes, but those are bo->list_krefs, not bo->krefs. ttm_bo_unref will put 
the bo->kref, which should lead to ttm_bo_release. This will then add 
the buffer to the ddestroy list and eventually put all the list_krefs. 
(Or possibly, if the ttm_bo_wait in ttm_bo_cleanup_refs_or_queue fails, 
it will explicitly delete the buffer from the LRU, which should amount 
to the same, I think).


Or perhaps I'm still missing something, I'm not too familiar with the 
whole BO reference counting.




So the following ttm_bo_unref() will just drop the initial reference.

Let's clean up this mess by moving the freeing of the BO in case of an
error to the caller.


Yes, see my other patches.

Thanks,
Nicolai




Regards,
Christian.


---
  drivers/gpu/drm/ttm/ttm_bo.c | 12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 239a957..76bee42 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1215,18 +1215,20 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
  if (likely(!ret))
  ret = ttm_bo_validate(bo, placement, interruptible, false);
  -if (!resv) {
+if (!resv)
  ttm_bo_unreserve(bo);
  -} else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+if (unlikely(ret)) {
+ttm_bo_unref();
+return ret;
+}
+
+if (resv && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
  spin_lock(>glob->lru_lock);
  ttm_bo_add_to_lru(bo);
  spin_unlock(>glob->lru_lock);
  }
  -if (unlikely(ret))
-ttm_bo_unref();
-
  return ret;
  }
  EXPORT_SYMBOL(ttm_bo_init);





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

Re: [PATCH 1/2] drm/ttm: never add BO that failed to validate to the LRU list

[Christian König]

Am 14.02.2017 um 10:18 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

Fixes a potential race condition in amdgpu that looks as follows:

Task 1: attempt ttm_bo_init, but ttm_bo_validate fails
Task 1: add BO to global list anyway
Task 2: grabs hold of the BO, waits on its reservation lock
Task 1: releases its reference of the BO; never gives up the
 reservation lock

The patch "drm/amdgpu: fix a potential deadlock in
amdgpu_bo_create_restricted()" attempts to fix that by releasing
the reservation lock in amdgpu code; unfortunately, it introduces
a use-after-free when this race _doesn't_ happen.

This patch should fix the race properly by never adding the BO
to the global list in the first place.

Cc: Samuel Pitoiset 
Cc: zhoucm1 
Signed-off-by: Nicolai Hähnle 


NAK, that is actually not correct either.

The previous implementation doesn't have an use after free, but actually 
a memory leak.


I completely agree that adding the BO to the LRU in case of an error is 
incorrect, but ttm_bo_add_to_lru() will add some references to the BO.


So the following ttm_bo_unref() will just drop the initial reference.

Let's clean up this mess by moving the freeing of the BO in case of an 
error to the caller.


Regards,
Christian.


---
  drivers/gpu/drm/ttm/ttm_bo.c | 12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 239a957..76bee42 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1215,18 +1215,20 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
if (likely(!ret))
ret = ttm_bo_validate(bo, placement, interruptible, false);
  
-	if (!resv) {

+   if (!resv)
ttm_bo_unreserve(bo);
  
-	} else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {

+   if (unlikely(ret)) {
+   ttm_bo_unref();
+   return ret;
+   }
+
+   if (resv && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
spin_lock(>glob->lru_lock);
ttm_bo_add_to_lru(bo);
spin_unlock(>glob->lru_lock);
}
  
-	if (unlikely(ret))

-   ttm_bo_unref();
-
return ret;
  }
  EXPORT_SYMBOL(ttm_bo_init);



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