[Bug 102401] Radeon Displayport Audio Warping
https://bugzilla.kernel.org/show_bug.cgi?id=102401 --- Comment #3 from Maxqia --- Yes, I just double checked. With the commit, the displayport audio is warped. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 91600] WoW crashes in OpenGL mode with Mesa, but not with NVIDIA blob
https://bugs.freedesktop.org/show_bug.cgi?id=91600 Mike Lothian changed: What|Removed |Added CC||mike at fireburn.co.uk --- Comment #2 from Mike Lothian --- I think this was a regression brought in by patch 6.2 - OpenGL worked fine for me before the patch. Now I'm forced to use DX9 which crashes on exit or Alt+Tab. I'm not sure if that's related to DRI3 or Kwin on Plasma 5 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/09906912/attachment.html>
[Bug 102401] Radeon Displayport Audio Warping
https://bugzilla.kernel.org/show_bug.cgi?id=102401 Maxqia changed: What|Removed |Added Attachment #184611|0 |1 is obsolete|| -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v2 11/11] drm/exynos: remove struct exynos_drm_encoder layer
Hi Inki, 2015-08-07 Inki Dae : > Hi Gustavo, > > On 2015ë 08ì 06ì¼ 22:31, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > struct exynos_drm_encoder was justing wrapping struct drm_encoder, it had > > only a drm_encoder member and the internal exynos_drm_encoders ops that > > was directly mapped to the drm_encoder helper funcs. > > > > So now exynos DRM uses struct drm_encoder directly, this removes > > completely the struct exynos_drm_encoder. > > > > Trats2 board, which uses Exynos4412 Soc, doesn't work after this patch > is applied. Below is the booting logs, > [1.171318] console [ttySAC2] enabled > [1.175522] 1383.serial: ttySAC3 at MMIO 0x1383 (irq = 60, > base_baud = 0) is a S3C6400/10 > [1.185545] [drm] Initialized drm 1.1.0 20060810 > [1.194104] exynos-drm exynos-drm: bound 11c0.fimd (ops > fimd_component_ops) > [1.200352] exynos-drm exynos-drm: bound 11c8.dsi (ops > exynos_dsi_component_ops) > [1.207688] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > [1.214313] [drm] No driver support for vblank timestamp query. > [1.220218] [drm] Initialized exynos 1.0.0 20110530 on minor 0 > > Booting is locked up here. This patch looks good to me so I tried to > find why locked up and I found the booting is locked up as soon as > console_lock function is called. Can you and other guys look into this > issue? I've realized that I left a fix for patch 01 behind, it could be the cause of this issue. I've just resent this patch with the added v2 fix up. Gustavo
[PATCH 01/11] drm/exynos: split display's .dpms() into .enable() and .disable()
From: Gustavo Padovan The DRM Core doesn't have a dpms() operation anymore, everything now is enable() or disable(). Signed-off-by: Gustavo Padovan --- v2: set dp->dpms_mode after enable/disable --- drivers/gpu/drm/exynos/exynos_dp_core.c | 36 ++-- drivers/gpu/drm/exynos/exynos_drm_dpi.c | 36 drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 ++- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 44 ++- drivers/gpu/drm/exynos/exynos_drm_encoder.c | 8 ++-- drivers/gpu/drm/exynos/exynos_hdmi.c| 65 ++--- 6 files changed, 65 insertions(+), 130 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index 172b800..ef24952 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -1066,8 +1066,9 @@ static void exynos_dp_phy_exit(struct exynos_dp_device *dp) phy_power_off(dp->phy); } -static void exynos_dp_poweron(struct exynos_dp_device *dp) +static void exynos_dp_enable(struct exynos_drm_display *display) { + struct exynos_dp_device *dp = display_to_dp(display); struct exynos_drm_crtc *crtc = dp_to_crtc(dp); if (dp->dpms_mode == DRM_MODE_DPMS_ON) @@ -1088,10 +1089,13 @@ static void exynos_dp_poweron(struct exynos_dp_device *dp) exynos_dp_init_dp(dp); enable_irq(dp->irq); exynos_dp_commit(&dp->display); + + dp->dpms_mode = DRM_MODE_DPMS_ON; } -static void exynos_dp_poweroff(struct exynos_dp_device *dp) +static void exynos_dp_disable(struct exynos_drm_display *display) { + struct exynos_dp_device *dp = display_to_dp(display); struct exynos_drm_crtc *crtc = dp_to_crtc(dp); if (dp->dpms_mode != DRM_MODE_DPMS_ON) @@ -1116,30 +1120,14 @@ static void exynos_dp_poweroff(struct exynos_dp_device *dp) if (drm_panel_unprepare(dp->panel)) DRM_ERROR("failed to turnoff the panel\n"); } -} - -static void exynos_dp_dpms(struct exynos_drm_display *display, int mode) -{ - struct exynos_dp_device *dp = display_to_dp(display); - switch (mode) { - case DRM_MODE_DPMS_ON: - exynos_dp_poweron(dp); - break; - case DRM_MODE_DPMS_STANDBY: - case DRM_MODE_DPMS_SUSPEND: - case DRM_MODE_DPMS_OFF: - exynos_dp_poweroff(dp); - break; - default: - break; - } - dp->dpms_mode = mode; + dp->dpms_mode = DRM_MODE_DPMS_OFF; } static struct exynos_drm_display_ops exynos_dp_display_ops = { .create_connector = exynos_dp_create_connector, - .dpms = exynos_dp_dpms, + .enable = exynos_dp_enable, + .disable = exynos_dp_disable, .commit = exynos_dp_commit, }; @@ -1319,7 +1307,7 @@ static void exynos_dp_unbind(struct device *dev, struct device *master, { struct exynos_dp_device *dp = dev_get_drvdata(dev); - exynos_dp_dpms(&dp->display, DRM_MODE_DPMS_OFF); + exynos_dp_disable(&dp->display); } static const struct component_ops exynos_dp_ops = { @@ -1377,7 +1365,7 @@ static int exynos_dp_suspend(struct device *dev) { struct exynos_dp_device *dp = dev_get_drvdata(dev); - exynos_dp_dpms(&dp->display, DRM_MODE_DPMS_OFF); + exynos_dp_disable(&dp->display); return 0; } @@ -1385,7 +1373,7 @@ static int exynos_dp_resume(struct device *dev) { struct exynos_dp_device *dp = dev_get_drvdata(dev); - exynos_dp_dpms(&dp->display, DRM_MODE_DPMS_ON); + exynos_dp_enable(&dp->display); return 0; } #endif diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c index 7cb6595..e042670 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c @@ -32,7 +32,6 @@ struct exynos_dpi { struct drm_encoder *encoder; struct videomode *vm; - int dpms_mode; }; #define connector_to_dpi(c) container_of(c, struct exynos_dpi, connector) @@ -133,46 +132,30 @@ static int exynos_dpi_create_connector(struct exynos_drm_display *display, return 0; } -static void exynos_dpi_poweron(struct exynos_dpi *ctx) +static void exynos_dpi_enable(struct exynos_drm_display *display) { + struct exynos_dpi *ctx = display_to_dpi(display); + if (ctx->panel) { drm_panel_prepare(ctx->panel); drm_panel_enable(ctx->panel); } } -static void exynos_dpi_poweroff(struct exynos_dpi *ctx) +static void exynos_dpi_disable(struct exynos_drm_display *display) { + struct exynos_dpi *ctx = display_to_dpi(display); + if (ctx->panel) { drm_panel_disable(ctx->panel); drm_panel_unprepare(ctx->panel); } } -static void exynos_dpi_dpms(struct exynos_drm_display *display, int mode) -{ - struct exynos_dpi *ctx = display_to_dpi
[Bug 102401] Radeon Displayport Audio Warping
https://bugzilla.kernel.org/show_bug.cgi?id=102401 Alex Deucher changed: What|Removed |Added CC||alexdeucher at gmail.com --- Comment #2 from Alex Deucher --- Are you sure that's the right commit? That commit shouldn't have any affect on DP audio. All it does is move the check for monitor audio support from radeon_atom_encoder_mode_set() into radeon_audio_mode_set(). It doesn't change the hw programming at all. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Heiko, å¨ 2015/8/10 20:08, Heiko Stübner åé: > Hi Yakir, > > Am Samstag, 8. August 2015, 11:54:38 schrieb Yakir Yang: +static int rockchip_dp_init(struct rockchip_dp_device *dp) +{ + struct device *dev = dp->dev; + struct device_node *np = dev->of_node; + int ret; + + dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); + if (IS_ERR(dp->grf)) { + dev_err(dev, + "rk3288-dp needs rockchip,grf property\n"); + return PTR_ERR(dp->grf); + } + + dp->clk_dp = devm_clk_get(dev, "clk_dp"); >>> I've looked at the manual, but couldn't find an actual clock-name >>> used there. Is it really "clk_dp" or should it just be "dp"? >> This should be "clk_dp", not "dp". >> Cause analogix_dp_core would need a clock name with "dp", so I would >> rather to pasted my rockchip-dp node here before I add dt-bindings in >> next version ;) > The clock we name PCLK_EDP_CTRL in the clock controller is probably the clock > supplying the APB interface and named pclk already in the "Figure 3-2 > DP_TXclock domain" diagram on page 19 of the manual. So your "clk_dp" should > actually be "pclk". > > So you would have "dp", "dp_24m" and "pclk" for the 3 supplying clocks. Oh, yes, "pclk" is for APB interface, and "sclk_edp" for IP controller, and "sclk_edp_24m" for DP PHY, thanks for your explain. So for now, I would pass "sclk_edp" to "edp" in analogix_dp, and "sclk_edp_24m" to "dp-phy_24m" in phy-rockchip-dp.c, and "pclk_edp" to "pclk" in analogix_dp-rockchip.c. > >> edp: edp at ff97 { >> compatible = "rockchip,rk3288-dp"; >> reg = <0xff97 0x4000>; >> interrupts = ; >> >> clocks = <&cru SCLK_EDP>, <&cru SCLK_EDP_24M>, <&cru >> PCLK_EDP_CTRL>; >> clock-names = "clk_dp", "clk_dp_24m", "dp"; >> >> rockchip,grf = <&grf>; >> resets = <&cru 111>; >> reset-names = "dp"; >> power-domains = <&power RK3288_PD_VIO>; >> status = "disabled"; >> >> hsync-active-high = <0>; >> vsync-active-high = <0>; >> interlaced = <0>; >> samsung,color-space = <0>; >> samsung,dynamic-range = <0>; >> samsung,ycbcr-coeff = <0>; >> samsung,color-depth = <1>; >> samsung,link-rate = <0x0a>; >> samsung,lane-count = <1>; > Thierry already said, that these should probably be somehow auto-detected. > Properties needing to stay around should probably also be "analogix,..." with > a fallback to not break Samsung devicetrees, so > look for "analogix,foo!, if not found try "samsung,foo" Okay, it's better to rename to "analogxi...", done. > >> ports { >> edp_in: port { >> #address-cells = <1>; >> #size-cells = <0>; >> edp_in_vopb: endpoint at 0 { >> reg = <0>; >> remote-endpoint = <&vopb_out_edp>; >> }; >> }; >> }; > > + + dp->clk_24m = devm_clk_get(dev, "clk_dp_24m"); >>> Same here, maybe "dp_24m". >> Like my previous reply. And actually as those two clocks all have >> a common prefix "SCLK" in rk3288 clock tree, I thinkt we can name >> them to "sclk_dp" & "sclk_dp_24m", is it okay ? > As Thierry said, please don't add prefixes. Okay, so is it okay to rename them to "dp", "dp-phy-24m", "pclk" ? > + if (IS_ERR(dp->clk_24m)) { + dev_err(dev, "cannot get clk_dp_24m\n"); + return PTR_ERR(dp->clk_24m); + } >>> I think you're missing the pclk here (PCLK_EDP_CTRL) or is this part of >>> something else? >> Whops, as I refered in commit message I leave pclk_dp to >> analogix_dp_core driver ;-) >> >> The reason why I want to leave pclk is I thought this clock is more like >> analogix dp >> core driver want, like a IP controller clock (whatever analogix_dp do >> need a clock >> named with "dp"). > Hmm, I'd think what the core (and Samsung) driver use as "dp" clock is > probably the generic clock for the IP and not the pclk for the APB interface. > > So I think it still should be "dp" for the core and "dp_24m" + "pclk" for the > rockchip part? Yes, I think you are right, thanks ;) > + + dp->rst = devm_reset_control_get(dev, "dp"); + if (IS_ERR(dp->rst)) { + dev_err(dev, "failed to get reset\n"); + return PTR_ERR(dp->rst); + } + + ret = rockchip_dp_clk_enable(dp); + if (ret < 0) { + dev_err(dp->dev, "cannot enable dp clk %d\n", ret); + return ret; +
[Bug 91600] WoW crashes in OpenGL mode with Mesa, but not with NVIDIA blob
https://bugs.freedesktop.org/show_bug.cgi?id=91600 --- Comment #1 from Chris Rankin --- It is possible that the WoW client is trying to use an extension that the R600 driver doesn't support. E,g, it could be trying to use something > OpenGL 3.3 without bothering to check for its availability. (The NVIDIA blobs I have tested have all advertised OpenGL 4.x) Is there any way of asking Mesa to log what an application is trying to use? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/0def5d27/attachment.html>
[Bug 102401] Radeon Displayport Audio Warping
https://bugzilla.kernel.org/show_bug.cgi?id=102401 --- Comment #1 from Maxqia --- Created attachment 184611 --> https://bugzilla.kernel.org/attachment.cgi?id=184611&action=edit Patch Reverting Commit 7726e72b3d6879ee5fc743a230eb6f5afa12844b Seems to fix my issues. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Thierry, å¨ 2015/8/10 18:00, Thierry Reding åé: > On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote: > [...] >> edp: edp at ff97 { > [...] >> hsync-active-high = <0>; >> vsync-active-high = <0>; >> interlaced = <0>; > These look like they should come from the display mode definition (EDID) > rather than device tree. I do think so, those numbers can parse from struct drm_mode. But I haven't send those changes yet, cause I want to merge the split analogix_dp first, and then send some patches to improve it. If you think it's better to imptoved those now, I would like to do it , please let me know ;) >> samsung,color-space = <0>; >> samsung,dynamic-range = <0>; >> samsung,ycbcr-coeff = <0>; > I think these should also come from EDID, though I'm not sure if we > store this in internal data structures yet. Same to previous reply >> samsung,color-depth = <1>; > This is probably drm_display_info.bpc. Same to previous reply >> samsung,link-rate = <0x0a>; >> samsung,lane-count = <1>; > And these should really be derived from values in the DPCD and adjusted > (if necessary) during link training. > > Why would you ever want to hard-code the above? Yes, I do meet the problem that my eDP screen need lane-count to 4, but my DP TV need lane-count to 1. Just like previous reply, if you think I should improved them in this series, I would rather to do it. + dp->clk_24m = devm_clk_get(dev, "clk_dp_24m"); >>> Same here, maybe "dp_24m". >> Like my previous reply. And actually as those two clocks all have >> a common prefix "SCLK" in rk3288 clock tree, I thinkt we can name >> them to "sclk_dp" & "sclk_dp_24m", is it okay ? > I don't think there's a need for these common prefixes. The names here > are identifiers in the context of the IP block, so any SoC-specific > prefixes are irrelevant. Also they do appear, in DT and in code, in the > context of clocks already, so "sclk_" or "clk_" is completely redundant > in these names. The sclk_dp & sclk_dp_24m is not IP common ask, it's only exist in RK3288 SoC (Like exynos only got one "dp" clock), and actually I add this to rockchip platform dp driver not analogix dp driver. So I think it's okay to add some platform some common prefixes. And I got a better idea for those clock. "sclk_dp" & "sclk_dp_24m" is provided for the eDP phy, and I just take Heiko suggest that add an new phy-rockchip-dp.c driver, so it's better to move those clock to phy driver, and rename them to "dp-phy" && "dp-phy-24m". Thanks, - Yakir > Thierry
[PATCH 2/2] amdgpu: add flag to support 32bit VA address v2
The AMDGPU_VA_RANGE_32_BIT flag is added to request VA range in the 32bit address space for amdgpu_va_range_alloc. The 32bit address space is reserved at initialization time, and managed with a separate VAMGR as part of the global VAMGR. And if no enough VA space available in range above 4GB, this reserved range can be used as fallback. v2: add comment for AMDGPU_VA_RANGE_32_BIT, and add vamgr to va_range Signed-off-by: Jammy Zhou Reviewed-by: Christian König --- amdgpu/amdgpu.h | 5 + amdgpu/amdgpu_device.c | 22 ++ amdgpu/amdgpu_internal.h | 9 + amdgpu/amdgpu_vamgr.c| 35 --- 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index a90c1ac..1e633e3 100644 --- a/amdgpu/amdgpu.h +++ b/amdgpu/amdgpu.h @@ -1075,6 +1075,11 @@ int amdgpu_read_mm_registers(amdgpu_device_handle dev, unsigned dword_offset, uint32_t *values); /** + * Flag to request VA address range in the 32bit address space +*/ +#define AMDGPU_VA_RANGE_32_BIT 0x1 + +/** * Allocate virtual address range * * \param dev - [in] Device handle. See #amdgpu_device_initialize() diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c index ba75db0..067ef64 100644 --- a/amdgpu/amdgpu_device.c +++ b/amdgpu/amdgpu_device.c @@ -40,6 +40,7 @@ #include "amdgpu_drm.h" #include "amdgpu_internal.h" #include "util_hash_table.h" +#include "util_math.h" #define PTR_TO_UINT(x) ((unsigned)((intptr_t)(x))) #define UINT_TO_PTR(x) ((void *)((intptr_t)(x))) @@ -126,6 +127,7 @@ int amdgpu_device_initialize(int fd, int flag_auth = 0; int flag_authexist=0; uint32_t accel_working = 0; + uint64_t start, max; *device_handle = NULL; @@ -204,6 +206,19 @@ int amdgpu_device_initialize(int fd, dev->vamgr = amdgpu_vamgr_get_global(dev); + max = MIN2(dev->dev_info.virtual_address_max, 0x); + start = amdgpu_vamgr_find_va(dev->vamgr, +max - dev->dev_info.virtual_address_offset, +dev->dev_info.virtual_address_alignment, 0); + if (start > 0x) + goto free_va; /* shouldn't get here */ + + dev->vamgr_32 = calloc(1, sizeof(struct amdgpu_bo_va_mgr)); + if (dev->vamgr_32 == NULL) + goto free_va; + amdgpu_vamgr_init(dev->vamgr_32, start, max, + dev->dev_info.virtual_address_alignment); + *major_version = dev->major_version; *minor_version = dev->minor_version; *device_handle = dev; @@ -212,6 +227,11 @@ int amdgpu_device_initialize(int fd, return 0; +free_va: + r = -ENOMEM; + amdgpu_vamgr_free_va(dev->vamgr, start, +max - dev->dev_info.virtual_address_offset); + cleanup: if (dev->fd >= 0) close(dev->fd); @@ -222,6 +242,8 @@ cleanup: void amdgpu_device_free_internal(amdgpu_device_handle dev) { + amdgpu_vamgr_deinit(dev->vamgr_32); + free(dev->vamgr_32); amdgpu_vamgr_reference(&dev->vamgr, NULL); util_hash_table_destroy(dev->bo_flink_names); util_hash_table_destroy(dev->bo_handles); diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h index 526a93f..6e65642 100644 --- a/amdgpu/amdgpu_internal.h +++ b/amdgpu/amdgpu_internal.h @@ -63,6 +63,7 @@ struct amdgpu_va { uint64_t address; uint64_t size; enum amdgpu_gpu_va_range range; + struct amdgpu_bo_va_mgr *vamgr; }; struct amdgpu_device { @@ -80,7 +81,10 @@ struct amdgpu_device { pthread_mutex_t bo_table_mutex; struct drm_amdgpu_info_device dev_info; struct amdgpu_gpu_info info; + /** The global VA manager for the whole virtual address space */ struct amdgpu_bo_va_mgr *vamgr; + /** The VA manager for the 32bit address space */ + struct amdgpu_bo_va_mgr *vamgr_32; }; struct amdgpu_bo { @@ -124,6 +128,11 @@ struct amdgpu_bo_va_mgr* amdgpu_vamgr_get_global(struct amdgpu_device *dev); void amdgpu_vamgr_reference(struct amdgpu_bo_va_mgr **dst, struct amdgpu_bo_va_mgr *src); +void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, uint64_t start, + uint64_t max, uint64_t alignment); + +void amdgpu_vamgr_deinit(struct amdgpu_bo_va_mgr *mgr); + uint64_t amdgpu_vamgr_find_va(struct amdgpu_bo_va_mgr *mgr, uint64_t size, uint64_t alignment, uint64_t base_required); diff --git a/amdgpu/amdgpu_vamgr.c b/amdgpu/amdgpu_vamgr.c index e044dfa..04e09e2 100644 --- a/amdgpu/amdgpu_vamgr.c +++ b/amdgpu/amdgpu_vamgr.c @@ -42,7 +42,7 @@ int amdgpu_va_range_query(amdgpu_device_handle dev, return -EINVAL; } -static void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, uint64_t start, +void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, uint64_t start,
[PATCH 1/2] amdgpu: improve amdgpu_vamgr_init
Make it a generic function independent of the device info. Signed-off-by: Jammy Zhou Reviewed-by: Christian König --- amdgpu/amdgpu_vamgr.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/amdgpu/amdgpu_vamgr.c b/amdgpu/amdgpu_vamgr.c index ced4f4f..e044dfa 100644 --- a/amdgpu/amdgpu_vamgr.c +++ b/amdgpu/amdgpu_vamgr.c @@ -42,11 +42,12 @@ int amdgpu_va_range_query(amdgpu_device_handle dev, return -EINVAL; } -static void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, struct amdgpu_device *dev) +static void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, uint64_t start, + uint64_t max, uint64_t alignment) { - mgr->va_offset = dev->dev_info.virtual_address_offset; - mgr->va_max = dev->dev_info.virtual_address_max; - mgr->va_alignment = dev->dev_info.virtual_address_alignment; + mgr->va_offset = start; + mgr->va_max = max; + mgr->va_alignment = alignment; list_inithead(&mgr->va_holes); pthread_mutex_init(&mgr->bo_va_mutex, NULL); @@ -68,7 +69,9 @@ struct amdgpu_bo_va_mgr * amdgpu_vamgr_get_global(struct amdgpu_device *dev) ref = atomic_inc_return(&vamgr.refcount); if (ref == 1) - amdgpu_vamgr_init(&vamgr, dev); + amdgpu_vamgr_init(&vamgr, dev->dev_info.virtual_address_offset, + dev->dev_info.virtual_address_max, + dev->dev_info.virtual_address_alignment); return &vamgr; } -- 1.9.1
[PATCH RFC 1/5] drm/msm/hdmi: deprecate non standard gpio properties.
On Mon, Aug 10, 2015 at 6:27 PM, Bjorn Andersson wrote: > On Mon 10 Aug 15:07 PDT 2015, Rob Herring wrote: > > [..] >> >> -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin >> >> -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin >> >> -- qcom,hdmi-tx-hpd-gpio: hpd pin >> >> +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin >> >> +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin >> >> The driver doesn't appear to do anything other than set these gpios >> high. Presumably you would ultimately do a bit-bang i2c bus for these? >> At that point, aren't you going to want to use the i2c-gpio binding? >> >> I think this binding has bigger issues like why is it not using the >> hdmi-connector binding which has a standard "hpd-gpios" property >> already. I can't really single out this binding though. There's a lot >> of crap when it comes to DRM related bindings. >> > > Shouldn't these pins just be muxed to the hdmi block in pinctl? > > I know Rob had something wrt the detect pin (hdp), but the others should > never be gpios(?) Isn't this just a reminder of the codeaurora tree > gpio_requesting all pins "just to be safe"? yeah, other than hdp pin (since the debounce logic in hdmi block doesn't seem to be bulletproof), others we only request.. rest is echo's of codeaurora.. some of it is probably not needed upstream.. and I'm ok w/ just not documenting that in the bindings doc, or documenting as "unsupported and going to go away so don't use", or whatever makes the most sense.. but would like to keep the driver code alive for the time being, since it is easier for me to maintain a bit of cruft or dead code in the upstream driver, than dealing with it on the backport (and I'm inevitably the one who has to do both) (Good news is that srini mentioned he was working on audio for 8064/ifc6410 upstream, so it at least seems like there is an end in sight for backports to 3.4 device kernel.. 3.10 stuff, I think we'll have to live with a bit longer..) BR, -R >> >> +- qcom,hdmi-tx-hpd-gpios: hpd pin >> >> - core-vdda-supply: phandle to supply regulator >> >> - hdmi-mux-supply: phandle to mux regulator >> >> >> >> +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use >> >> + "qcom,hdmi-tx-ddc-clk-gpios" instead >> >> +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use >> >> + "qcom,hdmi-tx-ddc-data-gpios" instead >> >> +- qcom,hdmi-tx-hpd-gpio: (deprecated) use >> >> + "qcom,hdmi-tx-hpd-gpios" instead >> >> + >> >> Optional properties: >> >> -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin >> >> -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin >> >> +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin >> >> +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin >> >> + >> >> +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" >> >> + instead >> >> +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" >> >> + instead >> >> - pinctrl-names: the pin control state names; should contain "default" >> >> - pinctrl-0: the default pinctrl state (active) >> >> - pinctrl-1: the "sleep" pinctrl state >> > >> > I don't see much use in listing that these properties are deprecated. We >> > already have code to catch the deprecated names, so having them in the >> > binding will at best be distracting. >> > >> > Anyway, I don't know if there's been any advice on this from the device >> > tree bindings maintainers, so adding devicetree at vger.kernel.org for >> > visibility. >> >> If they need to be maintained, then marking them deprecated is >> perfectly fine IMO. Also, if the maintainers of platforms using this >> are okay with it, you can just switch it. Given there are no in tree >> dts files using this, it can be argued that there is no ABI. >> > > Part of some dev trees floating around no-one should have used these. So > I think we should just document the proper ones and have the drivers > support Rob's old properties until he's comfortable dropping them. > > That way we have a documented way forward and Rob can run his backports > of this code without forking it. > > Regards, > Bjorn
[Bug 91600] WoW crashes in OpenGL mode with Mesa, but not with NVIDIA blob
https://bugs.freedesktop.org/show_bug.cgi?id=91600 Chris Rankin changed: What|Removed |Added Attachment #117619|text/plain |application/octet-stream mime type|| -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/6850cb38/attachment.html>
[Bug 91600] WoW crashes in OpenGL mode with Mesa, but not with NVIDIA blob
https://bugs.freedesktop.org/show_bug.cgi?id=91600 Bug ID: 91600 Summary: WoW crashes in OpenGL mode with Mesa, but not with NVIDIA blob Product: Mesa Version: git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/r600 Assignee: dri-devel at lists.freedesktop.org Reporter: rankincj at googlemail.com QA Contact: dri-devel at lists.freedesktop.org Created attachment 117619 --> https://bugs.freedesktop.org/attachment.cgi?id=117619&action=edit WINEDEBUG trace, including R600 shaders Hardware tested: Linux x64, both HD4890 and HD6450 World of Warcraft (OpenGL mode) has been crashing ever since it upgraded itself to 6.2. I had assumed that it was a bug in the WoW client, until I noticed that WoW did not crash on a machine running with the NVIDIA binary blob. I have executed WoW using the following command: $ R600_DEBUG=fs,vs,gs,ps,cs WINEDEBUG=+opengl,+wgl,seh ./Wow-64.exe And it appears that WoW is throwing an exception in a glLoadMatrix() operation: trace:opengl:glCallList (1) trace:opengl:glLoadMatrixf (0x22f610) trace:seh:raise_exception code=c005 flags=0 addr=(nil) ip=0 tid=0027 trace:seh:raise_exception info[0]= trace:seh:raise_exception info[1]= trace:seh:raise_exception rax=06e9d2c0 rbx=06e9ffd0 rcx=9117 rdx= trace:seh:raise_exception rsi=0001413696b1 rdi=06e9d2c0 rbp=0022fa60 rsp=0022f898 trace:seh:raise_exception r8=000a r9=0022f610 r10=7f2857d369f6 r11=0022f7f0 trace:seh:raise_exception r12=001b6b06 r13= r14=7f7e8000 r15=7ffd34f83da0 trace:seh:RtlVirtualUnwind type 1 rip 1400de519 rsp 22f8a0 trace:seh:dump_unwind_info func de4d0-de5aa trace:seh:dump_unwind_info unwind info at 0x1412e21ac flags 0 prolog 0x6 bytes function 0x1400de4d0-0x1400de5aa trace:seh:dump_unwind_info 0x6: subq $0x20,%rsp trace:seh:dump_unwind_info 0x2: pushq %rbx trace:seh:RtlVirtualUnwind type 1 rip 1400d0ec9 rsp 22f8d0 trace:seh:dump_unwind_info func d0e70-d0ef8 I don't know how to debug this any further; the more flags I add to WINEDEBUG, the less likely WoW is to do *anything* (i.e. "run at all"). Raising this as a means of requesting more ideas. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/ef6fb8e5/attachment.html>
[PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver
On Mon, Aug 10, 2015 at 05:49:41PM +0100, Russell King - ARM Linux wrote: > I'm not sure what the right solution is here: modifying every audio > player out there to make HDMI work sanely is crazy. Having alsalib > automatically generate the correct AES channel status bytes for > linear audio formats seems to be sensible, but difficult given its > present structure with the defaults - the iec958 plugin has no idea > if the defaults are being used or not. > The advantage of having the horrid conversion in the kernel is that > we can choose to generate proper AES channel status data without > regard to userspace for standard linear PCM, and when the iec958 plugin > is being used with proper channel status (eg, in compressed audio > pass-through mode by VLC) then that works too. The other advantage of doing it in kernel is that it also fixes tinyalsa applications (which mainly means Android systems) by default for PCM data. -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: Digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/8d801c69/attachment.sig>
[PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver
On Mon, Aug 10, 2015 at 02:23:07PM +0200, Takashi Iwai wrote: > I admit that alsa-lib code is very horrible to follow -- but I guess > the change you'd need for iec958 plugin would be fairly small. We can > add a config option and let iec958 behaving slightly differently > depending on it. Yes, but there's other problems there as well. The IEC958 plugin does the job of adding the 4 AES bytes and formatting fairly well, but the problem when the 'default' bytes specified in the ALSA configuration files are used. Let's take the old chestnut of PulseAudio, or even aplay, or the miriad of other audio-only players out there. Most of them do not supply the AES bytes to be used, so we end up with the default. The default is... 0x04 0x82 0x00 0x02, which specifies a sample rate of 48kHz. However, the actual sample rate may not be 48kHz. At least the HDMI specifications say that the channel status data must be correct, and there are AV receivers out there which do make use of this, and if the channel status does not agree with the actual sample rate, they either refuse to recognise the audio stream (saying there's nothing there) or they intermittently mute the audio. Yamaha RX-V677 is one example which has this behaviour. The only compliant program that I've found so far is VLC in SPDIF pass-through mode, which is the only case where VLC passes the channel status information. Everything else seems broken in this regard, by falling back to the default. Obviously, aplay can be made to work by setting the AES bytes manually when specifying the device for it to use, but this is not really user-friendly or programmer friendly - especially as the current use model expects things to "just work" (the common case being PCM output on a PC which doesn't care about channel status.) I'm not sure what the right solution is here: modifying every audio player out there to make HDMI work sanely is crazy. Having alsalib automatically generate the correct AES channel status bytes for linear audio formats seems to be sensible, but difficult given its present structure with the defaults - the iec958 plugin has no idea if the defaults are being used or not. The advantage of having the horrid conversion in the kernel is that we can choose to generate proper AES channel status data without regard to userspace for standard linear PCM, and when the iec958 plugin is being used with proper channel status (eg, in compressed audio pass-through mode by VLC) then that works too. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[PATCH] drm/panel: auo novatek 1080p video mode panel
On Mon, Aug 10, 2015 at 3:54 PM, Bjorn Andersson wrote: > On Fri 07 Aug 09:11 PDT 2015, Rob Clark wrote: > >> On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding >> wrote: >> > On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote: > [..] >> >> +- compatible: should be "auo,novatek-1080p-vid" >> > >> > This looks a little generic for a compatible string. Can't we get at the >> > specific panel model number that's been used? What if AUO ever produced >> > some other Novatek panel with a 1080p resolution? >> >> Maybe Sony or someone else can chime in? That somewhat generic name >> was all I could get from downstream android kernel. I'm sure there is >> a better possible name, although I have no means to find that out >> myself. >> > > We're working on it. Cool, thx >> > Also, what's the -vid suffix for? >> >> the same panel seems to also work in cmd mode.. so idea was to have >> -vid and -cmd compat strings to choose which mode to operate in. >> > > An alternative would be to make it a bool property, to indicate video > mode - following how the framework is implemented. I was debating both approaches.. I think I ended up going with the compat name so that we could, if wanted, have the two split out into different drivers, depending on how much was in common. Not even really sure if that is worth worrying about or not. BR, -R > [..] >> >> diff --git a/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c >> >> b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c >> >> +static int auo_panel_init(struct auo_panel *auo) >> >> +{ >> >> + struct mipi_dsi_device *dsi = auo->dsi; >> >> + int ret; >> >> + >> >> + dsi->mode_flags |= MIPI_DSI_MODE_LPM; >> >> + >> >> + ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xb0, 0x04 }, 2); >> > >> > I find this notation hard to read. Have you considered moving this into >> > some sort of table that you can loop through? Or perhaps add some >> > helpers, say, mipi_dsi_generic_writeb() and mipi_dsi_dcs_writeb() to >> > help make this more readable? >> > >> >> Yeah, helper macro thing might be a reasonable idea. The table option >> makes it hard to use the helpers for things that are not non-standard, >> or when you need delays, etc.. >> > > I agree with you here, we don't want lists of data that the driver has > to interpret into writes (of various types) and delays. > >> >> >> + if (ret < 0) >> >> + return ret; >> >> + > > Regards, > Bjorn
[PATCH RFC 1/5] drm/msm/hdmi: deprecate non standard gpio properties.
On Mon, Aug 10, 2015 at 7:45 AM, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 12:59:22PM +0100, Srinivas Kandagatla wrote: >> This patch updates the bindings to discourage the usage of non standard >> gpio properites, this will help in projects focused on upstreaming. > > That last part is an odd comment to make in the commit message of a > patch submitted upstream... > >> These deprecated properties are still supported but will be remove over >> the time. > > You can't ever remove them because you can't ever be sure that people > won't be using an old DTB. It is only an ABI if anyone notices... > >> >> Signed-off-by: Srinivas Kandagatla >> --- >> Documentation/devicetree/bindings/drm/msm/hdmi.txt | 22 >> +- >> 1 file changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> index c43aa53..acba581 100644 >> --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> @@ -11,15 +11,27 @@ Required properties: >> - interrupts: The interrupt signal from the hdmi block. >> - clocks: device clocks >>See ../clocks/clock-bindings.txt for details. >> -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin >> -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin >> -- qcom,hdmi-tx-hpd-gpio: hpd pin >> +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin >> +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin The driver doesn't appear to do anything other than set these gpios high. Presumably you would ultimately do a bit-bang i2c bus for these? At that point, aren't you going to want to use the i2c-gpio binding? I think this binding has bigger issues like why is it not using the hdmi-connector binding which has a standard "hpd-gpios" property already. I can't really single out this binding though. There's a lot of crap when it comes to DRM related bindings. >> +- qcom,hdmi-tx-hpd-gpios: hpd pin >> - core-vdda-supply: phandle to supply regulator >> - hdmi-mux-supply: phandle to mux regulator >> >> +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use >> + "qcom,hdmi-tx-ddc-clk-gpios" instead >> +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use >> + "qcom,hdmi-tx-ddc-data-gpios" instead >> +- qcom,hdmi-tx-hpd-gpio: (deprecated) use >> + "qcom,hdmi-tx-hpd-gpios" instead >> + >> Optional properties: >> -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin >> -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin >> +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin >> +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin >> + >> +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" >> + instead >> +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" >> + instead >> - pinctrl-names: the pin control state names; should contain "default" >> - pinctrl-0: the default pinctrl state (active) >> - pinctrl-1: the "sleep" pinctrl state > > I don't see much use in listing that these properties are deprecated. We > already have code to catch the deprecated names, so having them in the > binding will at best be distracting. > > Anyway, I don't know if there's been any advice on this from the device > tree bindings maintainers, so adding devicetree at vger.kernel.org for > visibility. If they need to be maintained, then marking them deprecated is perfectly fine IMO. Also, if the maintainers of platforms using this are okay with it, you can just switch it. Given there are no in tree dts files using this, it can be argued that there is no ABI. Rob
[PATCH 9/9] drm: bridge/dw_hdmi-i2s-audio: add audio driver
On Sat, Aug 08, 2015 at 05:10:47PM +0100, Russell King wrote: > From: Yakir Yang > > Add ALSA based HDMI I2S audio driver for dw_hdmi. Sound card > driver could connect to this codec through the codec dai name > "dw-hdmi-i2s-audio". > > [Fixed IRQ name, MODULE_DESCRIPTION, MODULE_ALIAS in > dw-hdmi-i2s-audio.c, and platform device name in dw-hdmi.c --rmk] > > Signed-off-by: Yakir Yang > Signed-off-by: Russell King I'm dropping this patch after all as it no longer builds against modern kernels due to the reference to the removed snd_soc_jack_new(). Its replacement is at card level, and I don't think it's a simple case of replacing it here. > +static int snd_dw_hdmi_audio_probe(struct snd_soc_codec *codec) > +{ > + struct snd_dw_hdmi *dw = snd_soc_codec_get_drvdata(codec); > + int ret; > + > + ret = snd_soc_jack_new(codec, "dw Jack", SND_JACK_LINEOUT, > +&dw->jack); ... > +static const struct snd_soc_codec_driver dw_hdmi_audio = { > + .probe = snd_dw_hdmi_audio_probe, > + .dapm_widgets = snd_dw_hdmi_audio_widgets, > + .num_dapm_widgets = ARRAY_SIZE(snd_dw_hdmi_audio_widgets), > + .dapm_routes = snd_dw_hdmi_audio_routes, > + .num_dapm_routes = ARRAY_SIZE(snd_dw_hdmi_audio_routes), > +}; > + > +static int dw_hdmi_audio_probe(struct platform_device *pdev) > +{ > + struct dw_hdmi_audio_data *data = pdev->dev.platform_data; > + struct snd_dw_hdmi *dw; > + int ret; > + > + dw = devm_kzalloc(&pdev->dev, sizeof(*dw), GFP_KERNEL); > + if (!dw) > + return -ENOMEM; > + > + dw->data = *data; > + dw->dev = &pdev->dev; > + dw->is_jack_ready = false; > + platform_set_drvdata(pdev, dw); > + > + ret = request_irq(dw->data.irq, snd_dw_hdmi_irq, IRQF_SHARED, > + DRIVER_NAME, dw); > + if (ret) { > + dev_err(&pdev->dev, "request irq failed (%d)\n", ret); > + return -EINVAL; > + } > + > + ret = snd_soc_register_codec(&pdev->dev, &dw_hdmi_audio, > + &dw_hdmi_audio_dai, 1); -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[PATCH 1/2] amdgpu: improve amdgpu_vamgr_init
On Mon, Aug 10, 2015 at 8:26 AM, Jammy Zhou wrote: > Make it a generic function independent of the device info. > > Signed-off-by: Jammy Zhou > Reviewed-by: Christian König Series is: Reviewed-by: Alex Deucher If there are not additional comments, I'll push this in the next couple of days. Alex > --- > amdgpu/amdgpu_vamgr.c | 13 - > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/amdgpu/amdgpu_vamgr.c b/amdgpu/amdgpu_vamgr.c > index ced4f4f..e044dfa 100644 > --- a/amdgpu/amdgpu_vamgr.c > +++ b/amdgpu/amdgpu_vamgr.c > @@ -42,11 +42,12 @@ int amdgpu_va_range_query(amdgpu_device_handle dev, > return -EINVAL; > } > > -static void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, struct > amdgpu_device *dev) > +static void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr, uint64_t start, > + uint64_t max, uint64_t alignment) > { > - mgr->va_offset = dev->dev_info.virtual_address_offset; > - mgr->va_max = dev->dev_info.virtual_address_max; > - mgr->va_alignment = dev->dev_info.virtual_address_alignment; > + mgr->va_offset = start; > + mgr->va_max = max; > + mgr->va_alignment = alignment; > > list_inithead(&mgr->va_holes); > pthread_mutex_init(&mgr->bo_va_mutex, NULL); > @@ -68,7 +69,9 @@ struct amdgpu_bo_va_mgr * amdgpu_vamgr_get_global(struct > amdgpu_device *dev) > ref = atomic_inc_return(&vamgr.refcount); > > if (ref == 1) > - amdgpu_vamgr_init(&vamgr, dev); > + amdgpu_vamgr_init(&vamgr, > dev->dev_info.virtual_address_offset, > + dev->dev_info.virtual_address_max, > + dev->dev_info.virtual_address_alignment); > return &vamgr; > } > > -- > 1.9.1 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/doc: Update docs about device instance setup
On Mon, Aug 10, 2015 at 02:34:18PM +0200, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 02:07:49PM +0200, Daniel Vetter wrote: > > On Mon, Aug 10, 2015 at 01:59:07PM +0200, Thierry Reding wrote: > > > On Mon, Aug 10, 2015 at 11:55:38AM +0200, Daniel Vetter wrote: > > > > ->load is depracated, bus functionst are deprecated and everyone > > > > should use drm_dev_alloc®ister. > > > > > > Why would you want to deprecated ->load()? Even if you use > > > drm_dev_alloc() and drm_dev_register(), there's still use for ->load() > > > because it gives you the subsystem-level initialization entry point. > > > > ->load is called after the drm /dev node is registered and hence you can't > > really do any driver setup in there without risking races. We paper over > > that using drm_global_mutex, but that doesn't work for any other > > driver/userspace interface like sysfs/debugfs because of deadlocks. > > > > And we can't just reorder ->load to happen before the /dev nodes are > > registered because a lot of drivers would fall over if we do that. > > > > This is typical midlayer fail where the core calls into the driver instead > > of the other way round. > > Okay, but then if we're going to deprecate ->load(), I think we should > also come up with an upgrade plan. As it is, this just says that users > shouldn't do ->load(), but it doesn't tell them what to do instead. > > Would the proper procedure be to fix drivers so that ->load() can be > called between drm_dev_alloc() and drm_dev_register()? I suppose we > could add some sort of (temporary) flag for drivers to signal this and > then have drm_dev_register() call ->load() at the right time. If drivers > don't support it, we can keep what we have. > > That, of course, doesn't get rid of the midlayer, so perhaps a better > way forward would be to tell driver writers that they should be doing > subsystem-level setup between drm_dev_alloc() and drm_dev_register(). That's exactly what this patch tries to accomplish by updating the kerneldoc and docbook. New sequence should be device_probe_callback_or_whatever() { drm_dev_alloc(); ... driver init code ... drm_dev_register(); return 0; } Unfortunately the kerneldoc markdown isn't merged yet, otherwise I'd have added this code snippet to the docs too. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Caching of EDID for X server to decrease startup time of X server
On Mon, Aug 10, 2015 at 03:36:13PM +0200, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 02:13:43PM +0100, Chris Wilson wrote: > > On Mon, Aug 10, 2015 at 03:01:49PM +0200, Thierry Reding wrote: > > > Russell and Sascha were discussing this kind of caching in the i.MX > > > driver recently. Adding both for visibility. Also not trimming the quote > > > in case they don't have the original. > > > > > > It sounds like this could be useful to have in the core. As I understand > > > it, hotplug detection is pretty well specified for more modern display > > > interfaces (like HDMI and DisplayPort), so I think caching of this sort > > > could work for those. However, I think some older interfaces such as VGA > > > (or perhaps even DVI as well) don't have reliable hotplug detection and > > > hence would need to be able to force reading the EDID. > > > > > > Still, perhaps a connector flag could be introduced to enable caching on > > > a per-connector basis, and then we should be able to deal with this in > > > the DRM core, rather than have per-driver quirks. > > > > We've shied away from EDID caching precisely because HPD on Intel is > > notoriously unreliable. The last suggestion no one followed up with was > > a short term EDID cache. > > That's why I think a per-connector flag would be good. It would allow > this caching to be enabled only where it's been confirmed to be safe. > > I'd expect that HPD works well for HDMI and DisplayPort even on Intel. My stance is that the kernel should take care of this for userspace (it knows best what the hw can do, and the code will be shared with everyone). Userspace should simply ask for the cached copy using the trick Chris Wilson submitted a while ago. Of course it should allow some means for users to force an explicit probe on those systems where nothing really works. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 2/4] mm/compaction: enable mobile-page migration
On Fri, Jul 31, 2015 at 07:43:09PM +0900, Minchan Kim wrote: > Hi Gioh, > > On Mon, Jul 13, 2015 at 05:35:17PM +0900, Gioh Kim wrote: > > From: Gioh Kim > > > > Add framework to register callback functions and check page mobility. > > There are some modes for page isolation so that isolate interface > > has arguments of page address and isolation mode while putback > > interface has only page address as argument. > > > > Signed-off-by: Gioh Kim > > Acked-by: Rafael Aquini > > --- > > fs/proc/page.c | 3 ++ > > include/linux/compaction.h | 80 > > ++ > > include/linux/fs.h | 2 + > > include/linux/page-flags.h | 19 > > include/uapi/linux/kernel-page-flags.h | 1 + > > 5 files changed, 105 insertions(+) > > > > diff --git a/fs/proc/page.c b/fs/proc/page.c > > index 7eee2d8..a4f5a00 100644 > > --- a/fs/proc/page.c > > +++ b/fs/proc/page.c > > @@ -146,6 +146,9 @@ u64 stable_page_flags(struct page *page) > > if (PageBalloon(page)) > > u |= 1 << KPF_BALLOON; > > > > + if (PageMobile(page)) > > + u |= 1 << KPF_MOBILE; > > + > > Need a new page flag for non-LRU page migration? > I am worry that we don't have empty slot for page flag on 32-bit. > > Aha, see SetPageMobile below. You use _mapcount. > It seems to be work for non-LRU pages but unfortunately, zsmalloc > already have used that field as own purpose so there is no room > for it. Gioh, I just sent a patch which zsmalloc doesn't use page->mapping and _mapcount so I think zsmalloc could support compaction with your patchset. Although zsmalloc can support compaction with it, I still don't think it's a good that driver have to mark pages mobile via _mapcount. I hope you can find another way. Thanks. > > > > u |= kpf_copy_bit(k, KPF_LOCKED,PG_locked); > > > > u |= kpf_copy_bit(k, KPF_SLAB, PG_slab); > > diff --git a/include/linux/compaction.h b/include/linux/compaction.h > > index aa8f61c..f693072 100644 > > --- a/include/linux/compaction.h > > +++ b/include/linux/compaction.h > > @@ -1,6 +1,9 @@ > > #ifndef _LINUX_COMPACTION_H > > #define _LINUX_COMPACTION_H > > > > +#include > > +#include > > + > > /* Return values for compact_zone() and try_to_compact_pages() */ > > /* compaction didn't start as it was deferred due to past failures */ > > #define COMPACT_DEFERRED 0 > > @@ -51,6 +54,70 @@ extern void compaction_defer_reset(struct zone *zone, > > int order, > > bool alloc_success); > > extern bool compaction_restarting(struct zone *zone, int order); > > > > +static inline bool mobile_page(struct page *page) > > +{ > > + return page->mapping && (PageMobile(page) || PageBalloon(page)); > > > What's the locking rule to test mobile_page? > Why we need such many checking? > > Why we need PageBalloon check? > You are trying to make generic abstraction for non-LRU page to migrate > but PageBalloon check in here breaks your goal. > > > +} > > + > > +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t > > mode) > > +{ > > + bool ret = false; > > + > > + /* > > +* Avoid burning cycles with pages that are yet under __free_pages(), > > +* or just got freed under us. > > +* > > +* In case we 'win' a race for a mobile page being freed under us and > > +* raise its refcount preventing __free_pages() from doing its job > > +* the put_page() at the end of this block will take care of > > +* release this page, thus avoiding a nasty leakage. > > +*/ > > Good. > > > + if (unlikely(!get_page_unless_zero(page))) > > + goto out; > > + > > + /* > > +* As mobile pages are not isolated from LRU lists, concurrent > > +* compaction threads can race against page migration functions > > +* as well as race against the releasing a page. > > How can it race with releasing a page? > We already get refcount above. > > Do you mean page->mapping tearing race? > If so, zsmalloc should be chaned to hold a lock. > > > > +* > > +* In order to avoid having an already isolated mobile page > > +* being (wrongly) re-isolated while it is under migration, > > +* or to avoid attempting to isolate pages being released, > > +* lets be sure we have the page lock > > +* before proceeding with the mobile page isolation steps. > > +*/ > > + if (unlikely(!trylock_page(page))) > > + goto out_putpage; > > + > > + if (!(mobile_page(page) && page->mapping->a_ops->isolatepage)) > > + goto out_not_isolated; > > I cannot know how isolate_mobilepage is called by upper layer > because this patch doesn't include it. > Anyway, why we need such many checks to prove it's mobile page? > > mobile_page() { > page->mapping && (PageMobile(page) || PageBalloon(page)); > } > > Additionally, added page->mapping->a_ops->isolatepage check > > Is it possible
[Intel-gfx] [PATCH mesa v3] i965/gen8+: bo in state base address must be in 32-bit address range
On Mon, Aug 10, 2015 at 2:21 AM, Michel Thierry wrote: > Hi, > > Thanks for the comments, > > On 8/7/2015 11:46 PM, Kristian Høgsberg wrote: >> >> On Fri, Aug 7, 2015 at 2:45 AM, Michel Thierry >> wrote: >>> >>> Gen8+ supports 48-bit virtual addresses, but some objects must always be >>> allocated inside the 32-bit address range. >>> >>> In specific, any resource used with flat/heapless (0x-0xf000) >>> General State Heap or Intruction State Heap must be in a 32-bit range >>> (GSH / ISH), because the General State Offset and Instruction State >>> Offset >>> are limited to 32-bits. >> >> >> This doesn't look right. From the workaround text, it doesn't sound >> like this is a HW problem, it only affects GMM. In mesa, we don't use >> "heapless" (which I assume means setting the base to 0 and max range) > > > It is a hw problem, specifically in the state base address command. General > State Base Address and Instruction Base Address shouldn't be > 4GB. > >> for instructions, dynamic state or surface state. Surface state and >> dynamic state is always in our batch bo which is limited to 8k. >> Provided STATE_BASE_ADDRESS works correctly in the HW, the batch bo >> can be places anywhere in the aperture. Offsets to dynamic or surface >> state is relative to the beginning of the batch bo and will always be >> less than 4g. Instructions are in their own bo (brw->cache.bo), but >> again, we point instruction state base to it and all our shader stage >> setup code references the instructions relative to the beginning of >> the instruction bo. > > > I've seen the issue when the driver allocates mapped objects from bottom to > top and the other bo's from top to bottom (gpu hangs). So I say this wa is > needed. mesa does use heapless for the scratch buffers, and on BDW+ that's the only address type that's relative to general state base address. So we need the workaround there, whether or not there's a HW limitation/bug. I don't think there is a HW problem though. The driver I'm working on can use 48 bit ppgtt and Chris Wilson's softpin ioctl for userspace BO placement. I enabled that and made the userspace allocator place all BOs above 8g, and it all stilll worked. In particular, the instructions were above 8g and referenced relative to instruction base address. If the instruction base was limited to 4g, this would have hung the GPU. The fact that the HW limits the size of the heaps to 4g can be restriction when we move to full 48 ppgtt, but for how mesa uses STATE_BASE_ADDRESS, it only affects general state base address. Kristiain > -Michel > >> >> Kristian >> >>> Use drm_intel_bo_emit_reloc_48bit when the 4GB limit is not necessary, >>> and >>> the bo can be in the full address space. >>> >>> This commit introduces a dependency of libdrm 2.4.63, which introduces >>> the >>> drm_intel_bo_emit_reloc_48bit function. >>> >>> v2: s/48baddress/48b_address/, >>> Only use in OUT_RELOC64 cases, OUT_RELOC implies a 32-bit address >>> offset >>> is needed (Ben) >>> v3: Added OUT_RELOC64_INSIDE_4G, so it stands out when a 64-bit >>> relocation >>> needs the 32-bit workaround (Chris) >>> >>> References: >>> http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html >>> Cc: Ben Widawsky >>> Cc: Chris Wilson >>> Signed-off-by: Michel Thierry >>> --- >>> configure.ac | 2 +- >>> src/mesa/drivers/dri/i965/gen8_misc_state.c | 19 +++ >>> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 20 >>> src/mesa/drivers/dri/i965/intel_batchbuffer.h | 10 -- >>> 4 files changed, 36 insertions(+), 15 deletions(-) >>> >>> diff --git a/configure.ac b/configure.ac >>> index af61aa2..c92ca44 100644 >>> --- a/configure.ac >>> +++ b/configure.ac >>> @@ -68,7 +68,7 @@ AC_SUBST([OSMESA_VERSION]) >>> dnl Versions for external dependencies >>> LIBDRM_REQUIRED=2.4.38 >>> LIBDRM_RADEON_REQUIRED=2.4.56 >>> -LIBDRM_INTEL_REQUIRED=2.4.60 >>> +LIBDRM_INTEL_REQUIRED=2.4.63 >>> LIBDRM_NVVIEUX_REQUIRED=2.4.33 >>> LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41" >>> LIBDRM_FREEDRENO_REQUIRED=2.4.57 >>> diff --git a/src/mesa/drivers/dri/i965/gen8_misc_state.c >>> b/src/mesa/drivers/dri/i965/gen8_misc_state.c >>> index b20038e..73eba06 100644 >>> --- a/src/mesa/drivers/dri/i965/gen8_misc_state.c >>> +++ b/src/mesa/drivers/dri/i965/gen8_misc_state.c >>> @@ -28,6 +28,10 @@ >>> >>> /** >>>* Define the base addresses which some state is referenced from. >>> + * >>> + * Use OUT_RELOC64_INSIDE_4G instead of OUT_RELOC64, the General State >>> + * Offset and Instruction State Offset are limited to 32-bits by >>> hardware, >>> + * and must be located in the first 4GBs (32-bit offset). >>>*/ >>> void gen8_upload_state_base_address(struct brw_context *brw) >>> { >>> @@ -41,19 +45,18 @@ void gen8_upload_state_base_address(struct >>> brw_context *brw) >>> OUT_BATCH(0); >>> OUT_BATCH(mocs_wb << 16); >>> /* Su
Caching of EDID for X server to decrease startup time of X server
On Mon, Aug 10, 2015 at 03:01:49PM +0200, Thierry Reding wrote: > On Sat, Aug 08, 2015 at 10:18:57PM +0200, Paul Menzel wrote: > > Is it correct, that EDID is probed for both outputs? Is that necessary? > > I assume during startup the Linux kernel for example already collected > > this information and if no change of monitor/output is detected, the > > EDID should be the same, right? > > > > Is that already implement and I just need to apply the correct > > configuration? > > > > Any suggestions on how to decrease the startup time for the X server > > are much appreciated. > > Russell and Sascha were discussing this kind of caching in the i.MX > driver recently. Adding both for visibility. Also not trimming the quote > in case they don't have the original. > > It sounds like this could be useful to have in the core. Yes, as I mentioned in the discussion between Sascha and myself, I think this should be implemented in the DRM core rather than each and every driver. My reasoning is that the pretense for caching the EDID is basically one of performance: we don't want to keep on performing the time consuming I2C bus cycles to read it. However, drm_helper_probe_single_connector_modes_merge_bits() itself can be very expensive - it can call out to drm_load_edid_firmware(), which can involve requesting EDID firmware - which results in calling out to userspace. So, if we're concerned about performance, I think it would be better to solve all the performance problems in this path in a generic way which covers both issues. > As I understand > it, hotplug detection is pretty well specified for more modern display > interfaces (like HDMI and DisplayPort), so I think caching of this sort > could work for those. However, I think some older interfaces such as VGA > (or perhaps even DVI as well) don't have reliable hotplug detection and > hence would need to be able to force reading the EDID. Yes, I think I've seen code in DRM which does hotplug-detection-by-EDID. Those are normally doing so in their ->detect callback, and using the presence of EDID to report whether there's a device connected. I'd suggest these remain separate from this caching as it's serving a different purpose. > Still, perhaps a connector flag could be introduced to enable caching on > a per-connector basis, and then we should be able to deal with this in > the DRM core, rather than have per-driver quirks. I agree, and I think a lot of it can be handled entirely within DRM for most cases if we: - arrange for a new connector "modes_invalid" flag which is set initially - arrange for drm_helper_probe_single_connector_modes_merge_bits to test this flag to determine whether it needs to re-read the modes and/or load EDID. - have drm_helper_probe_single_connector_modes_merge_bits clear this flag if caching is enabled and we successfully read the modes from the connector and/or loaded EDID. - set this flag whenever we see the connector transition from a non-connected state to a connected state. I'm trying not to talk myself into writing another patch at the moment... my latency for getting development work into the kernel seems to be getting on for 2 years judging by the state of my dwhdmi audio and CEC patches, though I'm pretty sure that's just because they're fairly low-priority for me. :( -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[PATCH] drm/amdgpu: remove VI hw bug workaround v3
From: Christian König The workaround simply doesn't work because VM mappings are controlled by userspace not the kernel. Additional to that this is just a performance problem which happens if you have holes in your VM mapping. v2: adjust virtual addr alignment as well. v3: fix trivial warning Signed-off-by: Christian König Reviewed-by: Monk Liu (v1) Reviewed-by: Jammy Zhou (v2) --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 0479fd6..1d2a92f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -450,7 +450,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file dev_info.ids_flags |= AMDGPU_IDS_FLAGS_FUSION; dev_info.virtual_address_offset = AMDGPU_VA_RESERVED_SIZE; dev_info.virtual_address_max = (uint64_t)adev->vm_manager.max_pfn * AMDGPU_GPU_PAGE_SIZE; - dev_info.virtual_address_alignment = max(PAGE_SIZE, 0x1UL); + dev_info.virtual_address_alignment = max((int)PAGE_SIZE, AMDGPU_GPU_PAGE_SIZE); dev_info.pte_fragment_size = (1 << AMDGPU_LOG2_PAGES_PER_FRAG) * AMDGPU_GPU_PAGE_SIZE; dev_info.gart_page_size = AMDGPU_GPU_PAGE_SIZE; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 9d365cc..3b2c214 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -223,18 +223,6 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev, size_t acc_size; int r; - /* VI has a hw bug where VM PTEs have to be allocated in groups of 8. -* do this as a temporary workaround -*/ - if (!(domain & (AMDGPU_GEM_DOMAIN_GDS | AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA))) { - if ((adev->asic_type >= CHIP_TOPAZ) && (adev->asic_type != CHIP_FIJI)) { - if (byte_align & 0x7fff) - byte_align = ALIGN(byte_align, 0x8000); - if (size & 0x7fff) - size = ALIGN(size, 0x8000); - } - } - page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT; size = ALIGN(size, PAGE_SIZE); -- 1.9.1
[PATCH] radeon: add new OLAND pci id
Signed-off-by: Alex Deucher --- radeon/r600_pci_ids.h | 1 + 1 file changed, 1 insertion(+) diff --git a/radeon/r600_pci_ids.h b/radeon/r600_pci_ids.h index 9d12587..a3b2eac 100644 --- a/radeon/r600_pci_ids.h +++ b/radeon/r600_pci_ids.h @@ -391,6 +391,7 @@ CHIPSET(0x6608, OLAND_6608, OLAND) CHIPSET(0x6610, OLAND_6610, OLAND) CHIPSET(0x6611, OLAND_6611, OLAND) CHIPSET(0x6613, OLAND_6613, OLAND) +CHIPSET(0x6617, OLAND_6617, OLAND) CHIPSET(0x6620, OLAND_6620, OLAND) CHIPSET(0x6621, OLAND_6621, OLAND) CHIPSET(0x6623, OLAND_6623, OLAND) -- 1.8.3.1
Caching of EDID for X server to decrease startup time of X server
On Mon, Aug 10, 2015 at 02:13:43PM +0100, Chris Wilson wrote: > On Mon, Aug 10, 2015 at 03:01:49PM +0200, Thierry Reding wrote: > > Russell and Sascha were discussing this kind of caching in the i.MX > > driver recently. Adding both for visibility. Also not trimming the quote > > in case they don't have the original. > > > > It sounds like this could be useful to have in the core. As I understand > > it, hotplug detection is pretty well specified for more modern display > > interfaces (like HDMI and DisplayPort), so I think caching of this sort > > could work for those. However, I think some older interfaces such as VGA > > (or perhaps even DVI as well) don't have reliable hotplug detection and > > hence would need to be able to force reading the EDID. > > > > Still, perhaps a connector flag could be introduced to enable caching on > > a per-connector basis, and then we should be able to deal with this in > > the DRM core, rather than have per-driver quirks. > > We've shied away from EDID caching precisely because HPD on Intel is > notoriously unreliable. The last suggestion no one followed up with was > a short term EDID cache. That's why I think a per-connector flag would be good. It would allow this caching to be enabled only where it's been confirmed to be safe. I'd expect that HPD works well for HDMI and DisplayPort even on Intel. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/2d2d8195/attachment.sig>
[PATCH RFC 4/5] drm/msm/hdmi: deprecate non standard clock-names
On Mon, Aug 10, 2015 at 02:18:15PM +0100, Srinivas Kandagatla wrote: > > > On 10/08/15 13:49, Thierry Reding wrote: > >On Mon, Aug 10, 2015 at 12:59:49PM +0100, Srinivas Kandagatla wrote: > >>This patch updates the bindings to discourage the usage of non standard > >>clock names, this will help in projects focused on upstreaming. > >> > >>These deprecated properties are still supported but will be remove over > >>the time. > >> > >>Signed-off-by: Srinivas Kandagatla > >>--- > >> Documentation/devicetree/bindings/drm/msm/hdmi.txt | 16 > >> 1 file changed, 8 insertions(+), 8 deletions(-) > >> > >>diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt > >>b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > >>index 6dc202e..6fbfdd8 100644 > >>--- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt > >>+++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > >>@@ -12,16 +12,16 @@ Required properties: > >> - clocks: device clocks > >> - clock-names: Corresponding name for each entry in the clocks property. > >> for "qcom,hdmi-tx-8960" compatible names should be > >>- "core_clk" > >>- "master_iface_clk" > >>- "slave_iface_clk" > >>+ "core_clk" is deprecated, use "core" instead > >>+ "master_iface_clk" is deprecated, use "master_iface" instead > >>+ "slave_iface_clk" is deprecated, use "slave_iface" instead > >> > >> for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should > >> be > >>- "extp_clk" > >>- "alt_iface_clk" > >>- "iface_clk" > >>- "core_clk" > >>- "mdp_core_clk" > >>+ "extp_clk" is deprecated, use "extp" instead > >>+ "alt_iface_clk" is deprecated, use "alt_iface" intstead > >>+ "iface_clk" is deprecated, use "iface" instead > >>+ "core_clk" is deprecated, use "core" instead > >>+ "mdp_core_clk" is deprecated, use "mdp_core" instead > > > >Shouldn't there be a driver counterpart of this to accept the new names? > Driver changes are in this same series "[PATCH RFC 5/5] drm/msm/hdmi: remove > _clk suffix from clock names"(https://lkml.org/lkml/2015/8/10/453) I don't have that patch in my inbox. It looks to be doing things backwards (look up the deprecated name first). I think it should be: clk = devm_clk_get(dev, id); if (IS_ERR(clk)) { char clk_name[32]; snprintf(clk_name, sizeof(clk_name), "%s_clk", id); clk = devm_clk_get(dev, clk_name); if (IS_ERR(clk)) return clk; } Also note how I've dropped the ERR_CAST(), that's not useful here because you aren't actually casting, but simply returning clk. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/67f98496/attachment.sig>
[PATCH RFC 1/5] drm/msm/hdmi: deprecate non standard gpio properties.
On Mon 10 Aug 15:07 PDT 2015, Rob Herring wrote: [..] > >> -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin > >> -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin > >> -- qcom,hdmi-tx-hpd-gpio: hpd pin > >> +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin > >> +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin > > The driver doesn't appear to do anything other than set these gpios > high. Presumably you would ultimately do a bit-bang i2c bus for these? > At that point, aren't you going to want to use the i2c-gpio binding? > > I think this binding has bigger issues like why is it not using the > hdmi-connector binding which has a standard "hpd-gpios" property > already. I can't really single out this binding though. There's a lot > of crap when it comes to DRM related bindings. > Shouldn't these pins just be muxed to the hdmi block in pinctl? I know Rob had something wrt the detect pin (hdp), but the others should never be gpios(?) Isn't this just a reminder of the codeaurora tree gpio_requesting all pins "just to be safe"? > >> +- qcom,hdmi-tx-hpd-gpios: hpd pin > >> - core-vdda-supply: phandle to supply regulator > >> - hdmi-mux-supply: phandle to mux regulator > >> > >> +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use > >> + "qcom,hdmi-tx-ddc-clk-gpios" instead > >> +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use > >> + "qcom,hdmi-tx-ddc-data-gpios" instead > >> +- qcom,hdmi-tx-hpd-gpio: (deprecated) use > >> + "qcom,hdmi-tx-hpd-gpios" instead > >> + > >> Optional properties: > >> -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin > >> -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin > >> +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin > >> +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin > >> + > >> +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" > >> + instead > >> +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" > >> + instead > >> - pinctrl-names: the pin control state names; should contain "default" > >> - pinctrl-0: the default pinctrl state (active) > >> - pinctrl-1: the "sleep" pinctrl state > > > > I don't see much use in listing that these properties are deprecated. We > > already have code to catch the deprecated names, so having them in the > > binding will at best be distracting. > > > > Anyway, I don't know if there's been any advice on this from the device > > tree bindings maintainers, so adding devicetree at vger.kernel.org for > > visibility. > > If they need to be maintained, then marking them deprecated is > perfectly fine IMO. Also, if the maintainers of platforms using this > are okay with it, you can just switch it. Given there are no in tree > dts files using this, it can be argued that there is no ABI. > Part of some dev trees floating around no-one should have used these. So I think we should just document the proper ones and have the drivers support Rob's old properties until he's comfortable dropping them. That way we have a documented way forward and Rob can run his backports of this code without forking it. Regards, Bjorn
[PATCH RFC 2/5] drm/msm/hdmi: make use of standard gpio properties.
On Mon, Aug 10, 2015 at 02:15:18PM +0100, Srinivas Kandagatla wrote: > > > On 10/08/15 13:38, Thierry Reding wrote: > >On Mon, Aug 10, 2015 at 12:59:34PM +0100, Srinivas Kandagatla wrote: > >>This patch modifies the driver to support standard gpio properties along > >>with deprecated properties. This will help us upstream and cleanup the > >>non-standard properties over the time. > >> > >>Signed-off-by: Srinivas Kandagatla > >>--- > >> drivers/gpu/drm/msm/hdmi/hdmi.c | 35 +-- > >> 1 file changed, 25 insertions(+), 10 deletions(-) > >> > >>diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c > >>b/drivers/gpu/drm/msm/hdmi/hdmi.c > >>index 8145362..e918889 100644 > >>--- a/drivers/gpu/drm/msm/hdmi/hdmi.c > >>+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c > >>@@ -339,19 +339,34 @@ static const struct of_device_id dt_match[] = { > >> }; > >> > >> #ifdef CONFIG_OF > >>+/* This code will be removed once we move to gpiod based calls */ > > > >Why don't you do this now instead of duplicating what is essentially > >already implemented in gpiolib? > > > One of the thing that Rob asked in his comments > (http://www.spinics.net/lists/arm-kernel/msg437675.html) was to retain the > support for old devices, moving to gpiod ATM would break such devices as > they are still using legacy gpiolib and its naming. > > > If Rob is ok to drop gpio properties which does not have "-gpio" or "-gpios" > suffix then we can get rid of this function all together. If you make the switch to gpiod_*() APIs you'll get this for free. There's really no need for having a duplicate of what gpiod_get() already does for you. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/9f82e041/attachment.sig>
Caching of EDID for X server to decrease startup time of X server
We have already added a patch, in I915 driver, which caches the EDID, in connector, and releases it only after a hot unplug. The patch is under review, but may be the same logic can be reused here. The problem is, every time we have a connector->detect() call, most of the driver re-read the EDID, which is not an efficient design. connector->detect() gets called many times during bootup, even without a hotplug/unplug, which causes many EDID reads for HDMI/DP. Ideally we should read EDID only when there is a hotplug, and release the cached EDID when there is a hot-unplug. The method can go like this: 1. Cache the EDID in connector->edid, when there is really a hot plug-in interrupt (encoder->hot_plug() function can be used for the same) 2. Do not read the EDID again, in any connector->detect() functions, just used the cached EDID from connector->edid for all decision making. 3. Release/Free the EDID from connecotor->edid, when there is really a hot-unplug (again encoder->hot_plug() function can be used) If we can maintain this state machine, there won't be any extra EDID read at all. Hope this was not complete waste of your time :) Regards Shashank -Original Message- From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of Russell King - ARM Linux Sent: Monday, August 10, 2015 8:17 PM To: Thierry Reding Cc: Paul Menzel; dri-devel at lists.freedesktop.org Subject: Re: Caching of EDID for X server to decrease startup time of X server On Mon, Aug 10, 2015 at 03:01:49PM +0200, Thierry Reding wrote: > On Sat, Aug 08, 2015 at 10:18:57PM +0200, Paul Menzel wrote: > > Is it correct, that EDID is probed for both outputs? Is that necessary? > > I assume during startup the Linux kernel for example already > > collected this information and if no change of monitor/output is > > detected, the EDID should be the same, right? > > > > Is that already implement and I just need to apply the correct > > configuration? > > > > Any suggestions on how to decrease the startup time for the X server > > are much appreciated. > > Russell and Sascha were discussing this kind of caching in the i.MX > driver recently. Adding both for visibility. Also not trimming the > quote in case they don't have the original. > > It sounds like this could be useful to have in the core. Yes, as I mentioned in the discussion between Sascha and myself, I think this should be implemented in the DRM core rather than each and every driver. My reasoning is that the pretense for caching the EDID is basically one of performance: we don't want to keep on performing the time consuming I2C bus cycles to read it. However, drm_helper_probe_single_connector_modes_merge_bits() itself can be very expensive - it can call out to drm_load_edid_firmware(), which can involve requesting EDID firmware - which results in calling out to userspace. So, if we're concerned about performance, I think it would be better to solve all the performance problems in this path in a generic way which covers both issues. > As I understand > it, hotplug detection is pretty well specified for more modern display > interfaces (like HDMI and DisplayPort), so I think caching of this > sort could work for those. However, I think some older interfaces such > as VGA (or perhaps even DVI as well) don't have reliable hotplug > detection and hence would need to be able to force reading the EDID. Yes, I think I've seen code in DRM which does hotplug-detection-by-EDID. Those are normally doing so in their ->detect callback, and using the presence of EDID to report whether there's a device connected. I'd suggest these remain separate from this caching as it's serving a different purpose. > Still, perhaps a connector flag could be introduced to enable caching > on a per-connector basis, and then we should be able to deal with this > in the DRM core, rather than have per-driver quirks. I agree, and I think a lot of it can be handled entirely within DRM for most cases if we: - arrange for a new connector "modes_invalid" flag which is set initially - arrange for drm_helper_probe_single_connector_modes_merge_bits to test this flag to determine whether it needs to re-read the modes and/or load EDID. - have drm_helper_probe_single_connector_modes_merge_bits clear this flag if caching is enabled and we successfully read the modes from the connector and/or loaded EDID. - set this flag whenever we see the connector transition from a non-connected state to a connected state. I'm trying not to talk myself into writing another patch at the moment... my latency for getting development work into the kernel seems to be getting on for 2 years judging by the state of my dwhdmi audio and CEC patches, though I'm pretty sure that's just because they're fairly low-priority for me. :( -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ___
[PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
On Mon, Aug 10, 2015 at 08:59:44PM +0800, Yakir Yang wrote: > Hi Thierry, > > å¨ 2015/8/10 18:00, Thierry Reding åé: > >On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote: > >[...] > >> edp: edp at ff97 { > >[...] > >> hsync-active-high = <0>; > >> vsync-active-high = <0>; > >> interlaced = <0>; > >These look like they should come from the display mode definition (EDID) > >rather than device tree. > > I do think so, those numbers can parse from struct drm_mode. But I haven't > send those changes yet, cause I want to merge the split analogix_dp first, > and > then send some patches to improve it. If you think it's better to imptoved > those > now, I would like to do it , please let me know ;) > > >> samsung,color-space = <0>; > >> samsung,dynamic-range = <0>; > >> samsung,ycbcr-coeff = <0>; > >I think these should also come from EDID, though I'm not sure if we > >store this in internal data structures yet. > > Same to previous reply > > >> samsung,color-depth = <1>; > >This is probably drm_display_info.bpc. > > Same to previous reply > > >> samsung,link-rate = <0x0a>; > >> samsung,lane-count = <1>; > >And these should really be derived from values in the DPCD and adjusted > >(if necessary) during link training. > > > >Why would you ever want to hard-code the above? > > Yes, I do meet the problem that my eDP screen need lane-count to 4, but my > DP TV need lane-count to 1. Just like previous reply, if you think I should > improved > them in this series, I would rather to do it. The problem with these is that if you keep them in for your initial submission, you can never (or only under extreme pain) remove them. Anything in DTB needs to be effectively supported forever. Also since these don't make sense to hard-code, just improve the code and get rid of the need for these DT properties. Mind you that you still need to keep the code to parse them, because presumably Exynos relies on them. But depending on how you split up the driver you might be able to restrict these compatibility hacks to Exynos and not carry them forward into your new driver. > >>>>+ dp->clk_24m = devm_clk_get(dev, "clk_dp_24m"); > >>>Same here, maybe "dp_24m". > >>Like my previous reply. And actually as those two clocks all have > >>a common prefix "SCLK" in rk3288 clock tree, I thinkt we can name > >>them to "sclk_dp" & "sclk_dp_24m", is it okay ? > >I don't think there's a need for these common prefixes. The names here > >are identifiers in the context of the IP block, so any SoC-specific > >prefixes are irrelevant. Also they do appear, in DT and in code, in the > >context of clocks already, so "sclk_" or "clk_" is completely redundant > >in these names. > > The sclk_dp & sclk_dp_24m is not IP common ask, it's only exist in RK3288 > SoC (Like exynos > only got one "dp" clock), and actually I add this to rockchip platform dp > driver not analogix > dp driver. So I think it's okay to add some platform some common prefixes. > > And I got a better idea for those clock. "sclk_dp" & "sclk_dp_24m" is > provided for the eDP phy, > and I just take Heiko suggest that add an new phy-rockchip-dp.c driver, so > it's better to move > those clock to phy driver, and rename them to "dp-phy" && "dp-phy-24m". I agree that dealing with these in a PHY driver sounds like the better option. However, I still think that the dp-phy prefix is redundant. The names are in a per-driver scope, so "dp-phy" is implied by the device tree binding and driver already. You could simply use shorter names such as "phy" and "24m" for example. Also note that the clock provider will already have the proper names for these, so the clock tree will end up showing the provider names. The names in the binding are merely the "consumer" names. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/3d54c596/attachment-0001.sig>
Caching of EDID for X server to decrease startup time of X server
ing this device. > [ 231.682] (II) This device may have been added with another device > file. > [ 231.683] (II) config/udev: Adding input device Logitech Logitech USB > Keyboard (/dev/input/event1) > [ 231.683] (**) Logitech Logitech USB Keyboard: Applying InputClass > "evdev keyboard catchall" > [ 231.683] (II) Using input driver 'evdev' for 'Logitech Logitech USB > Keyboard' > [ 231.683] (**) Logitech Logitech USB Keyboard: always reports core > events > [ 231.683] (**) evdev: Logitech Logitech USB Keyboard: Device: > "/dev/input/event1" > [ 231.683] (--) evdev: Logitech Logitech USB Keyboard: Vendor 0x46d > Product 0xc30f > [ 231.683] (--) evdev: Logitech Logitech USB Keyboard: Found keys > [ 231.683] (II) evdev: Logitech Logitech USB Keyboard: Configuring as > keyboard > [ 231.683] (**) Option "config_info" > "udev:/sys/devices/pci:00/:00:12.0/usb3/3-2/3-2:1.0/0003:046D:C30F.0002/input/input3/event1" > [ 231.683] (II) XINPUT: Adding extended input device "Logitech Logitech > USB Keyboard" (type: KEYBOARD, id 9) > [ 231.683] (**) Option "xkb_rules" "evdev" > [ 231.683] (**) Option "xkb_model" "pc105" > [ 231.683] (**) Option "xkb_layout" "de" > [ 231.683] (II) config/udev: Adding input device Logitech Logitech USB > Keyboard (/dev/input/event2) > [ 231.683] (**) Logitech Logitech USB Keyboard: Applying InputClass > "evdev keyboard catchall" > [ 231.684] (II) Using input driver 'evdev' for 'Logitech Logitech USB > Keyboard' > [ 231.684] (**) Logitech Logitech USB Keyboard: always reports core > events > [ 231.684] (**) evdev: Logitech Logitech USB Keyboard: Device: > "/dev/input/event2" > [ 231.684] (--) evdev: Logitech Logitech USB Keyboard: Vendor 0x46d > Product 0xc30f > [ 231.684] (--) evdev: Logitech Logitech USB Keyboard: Found keys > [ 231.684] (II) evdev: Logitech Logitech USB Keyboard: Configuring as > keyboard > [ 231.684] (**) Option "config_info" > "udev:/sys/devices/pci:00/:00:12.0/usb3/3-2/3-2:1.1/0003:046D:C30F.0003/input/input4/event2" > [ 231.684] (II) XINPUT: Adding extended input device "Logitech Logitech > USB Keyboard" (type: KEYBOARD, id 10) > [ 231.684] (**) Option "xkb_rules" "evdev" > [ 231.684] (**) Option "xkb_model" "pc105" > [ 231.684] (**) Option "xkb_layout" "de" > [ 231.684] (II) config/udev: Adding input device HDA ATI SB Line > (/dev/input/event7) > [ 231.684] (II) No input driver specified, ignoring this device. > [ 231.684] (II) This device may have been added with another device > file. > [ 231.685] (II) config/udev: Adding input device HDA ATI SB Line Out > (/dev/input/event8) > [ 231.685] (II) No input driver specified, ignoring this device. > [ 231.685] (II) This device may have been added with another device > file. > [ 231.685] (II) config/udev: Adding input device HDA ATI SB Rear Mic > (/dev/input/event6) > [ 231.685] (II) No input driver specified, ignoring this device. > [ 231.685] (II) This device may have been added with another device > file. > [ 231.685] (II) config/udev: Adding input device PC Speaker > (/dev/input/event5) > [ 231.685] (II) No input driver specified, ignoring this device. > [ 231.685] (II) This device may have been added with another device > file. > [ 231.904] (II) evdev: Logitech Logitech USB Keyboard: Close > [ 231.905] (II) UnloadModule: "evdev" > [ 231.905] (II) evdev: Logitech Logitech USB Keyboard: Close > [ 231.905] (II) UnloadModule: "evdev" > [ 231.905] (II) evdev: HID 04d9:1133: Close > [ 231.905] (II) UnloadModule: "evdev" > [ 231.905] (II) evdev: Power Button: Close > [ 231.905] (II) UnloadModule: "evdev" > [ 231.905] (II) evdev: Power Button: Close > [ 231.905] (II) UnloadModule: "evdev" > [ 231.922] (EE) Server terminated successfully (0). Closing log file. > > Taking the line below as the end of the startup, > > [ 231.685] (II) This device may have been added with another device > file. > > starting up takes 450 ms. > > It seems to take 240 ms from loading the Radeon module to the message below. > > [ 231.481] (II) [KMS] Kernel modesetting enabled. > > Another 100 ms are used up, in the lines below. > > [ 231.481] (II) RADEON(0): SwapBuffers wait for vsync: enabled > [ 231.504] (II) RADEON(0): Output VGA-0 has no monitor section > [ 231.534] (II) RADEON(0): Output DVI-0 has no monitor section > [ 231.556] (II) RADEON(0): EDID for output VGA-0 > [ 231.586] (II) RADEON(0): EDID for output DVI-0 > [ 231.586] (II) RADEON(0): Manufacturer: SAM Model: 194 Serial#: > 1112092985 > > Is it correct, that EDID is probed for both outputs? Is that necessary? > I assume during startup the Linux kernel for example already collected > this information and if no change of monitor/output is detected, the > EDID should be the same, right? > > Is that already implement and I just need to apply the correct > configuration? > > Any suggestions on how to decrease the startup time for the X server > are much appreciated. Russell and Sascha were discussing this kind of caching in the i.MX driver recently. Adding both for visibility. Also not trimming the quote in case they don't have the original. It sounds like this could be useful to have in the core. As I understand it, hotplug detection is pretty well specified for more modern display interfaces (like HDMI and DisplayPort), so I think caching of this sort could work for those. However, I think some older interfaces such as VGA (or perhaps even DVI as well) don't have reliable hotplug detection and hence would need to be able to force reading the EDID. Still, perhaps a connector flag could be introduced to enable caching on a per-connector basis, and then we should be able to deal with this in the DRM core, rather than have per-driver quirks. Russell, Sascha, any comments? Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/eb67fffc/attachment-0001.sig>
[PATCH] drm: Remove two-level menu in Kconfig
On 10/08/15 14:47, Daniel Vetter wrote: > On Mon, Aug 10, 2015 at 01:32:08PM +0300, Tomi Valkeinen wrote: >> >> >> On 07/08/15 19:10, Thierry Reding wrote: >>> From: Thierry Reding >>> >>> The Direct Rendering Manager Kconfig option is already a separate menu, >>> so remove the extra level to make it easier to navigate. >>> >>> Signed-off-by: Thierry Reding >>> --- >>> drivers/video/Kconfig | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig >>> index 8bf495ffb020..e0606c01e8ac 100644 >>> --- a/drivers/video/Kconfig >>> +++ b/drivers/video/Kconfig >>> @@ -22,9 +22,7 @@ source "drivers/gpu/vga/Kconfig" >>> source "drivers/gpu/host1x/Kconfig" >>> source "drivers/gpu/ipu-v3/Kconfig" >>> >>> -menu "Direct Rendering Manager" >>> source "drivers/gpu/drm/Kconfig" >>> -endmenu >>> >>> menu "Frame buffer Devices" >>> source "drivers/video/fbdev/Kconfig" >>> >> >> Thanks, queued for 4.3. > > Oops failed to send out my usual mail, but I picked this already up in > topic/drm-misc. Ok, dropping this from fbdev tree. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/0a105b00/attachment-0001.sig>
[PATCH RFC 4/5] drm/msm/hdmi: deprecate non standard clock-names
On Mon, Aug 10, 2015 at 12:59:49PM +0100, Srinivas Kandagatla wrote: > This patch updates the bindings to discourage the usage of non standard > clock names, this will help in projects focused on upstreaming. > > These deprecated properties are still supported but will be remove over > the time. > > Signed-off-by: Srinivas Kandagatla > --- > Documentation/devicetree/bindings/drm/msm/hdmi.txt | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt > b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > index 6dc202e..6fbfdd8 100644 > --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt > +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > @@ -12,16 +12,16 @@ Required properties: > - clocks: device clocks > - clock-names: Corresponding name for each entry in the clocks property. > for "qcom,hdmi-tx-8960" compatible names should be > - "core_clk" > - "master_iface_clk" > - "slave_iface_clk" > + "core_clk" is deprecated, use "core" instead > + "master_iface_clk" is deprecated, use "master_iface" instead > + "slave_iface_clk" is deprecated, use "slave_iface" instead > > for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should be > - "extp_clk" > - "alt_iface_clk" > - "iface_clk" > - "core_clk" > - "mdp_core_clk" > + "extp_clk" is deprecated, use "extp" instead > + "alt_iface_clk" is deprecated, use "alt_iface" intstead > + "iface_clk" is deprecated, use "iface" instead > + "core_clk" is deprecated, use "core" instead > + "mdp_core_clk" is deprecated, use "mdp_core" instead Shouldn't there be a driver counterpart of this to accept the new names? Otherwise people could be switching the DTS to the new value, but the driver won't find the clocks it's looking for. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/b7afdf1c/attachment.sig>
[PATCH RFC 2/5] drm/msm/hdmi: make use of standard gpio properties.
On 10/08/15 14:26, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 02:15:18PM +0100, Srinivas Kandagatla wrote: >> >> >> On 10/08/15 13:38, Thierry Reding wrote: >>> On Mon, Aug 10, 2015 at 12:59:34PM +0100, Srinivas Kandagatla wrote: This patch modifies the driver to support standard gpio properties along with deprecated properties. This will help us upstream and cleanup the non-standard properties over the time. Signed-off-by: Srinivas Kandagatla --- drivers/gpu/drm/msm/hdmi/hdmi.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 8145362..e918889 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -339,19 +339,34 @@ static const struct of_device_id dt_match[] = { }; #ifdef CONFIG_OF +/* This code will be removed once we move to gpiod based calls */ >>> >>> Why don't you do this now instead of duplicating what is essentially >>> already implemented in gpiolib? >>> >> One of the thing that Rob asked in his comments >> (http://www.spinics.net/lists/arm-kernel/msg437675.html) was to retain the >> support for old devices, moving to gpiod ATM would break such devices as >> they are still using legacy gpiolib and its naming. >> >> >> If Rob is ok to drop gpio properties which does not have "-gpio" or "-gpios" >> suffix then we can get rid of this function all together. > > If you make the switch to gpiod_*() APIs you'll get this for free. > There's really no need for having a duplicate of what gpiod_get() > already does for you. > Yes, you are right, my memory was over written on friday by looking at of_find_gpio() for long :-) AFAIK, gpiod_get would also request the gpios so we might want to remove the additional gpio requests in the hdmi_connector.c too. I will give that a try once again.. --srini > Thierry >
[PATCH RFC 1/5] drm/msm/hdmi: deprecate non standard gpio properties.
On Mon, Aug 10, 2015 at 12:59:22PM +0100, Srinivas Kandagatla wrote: > This patch updates the bindings to discourage the usage of non standard > gpio properites, this will help in projects focused on upstreaming. That last part is an odd comment to make in the commit message of a patch submitted upstream... > These deprecated properties are still supported but will be remove over > the time. You can't ever remove them because you can't ever be sure that people won't be using an old DTB. > > Signed-off-by: Srinivas Kandagatla > --- > Documentation/devicetree/bindings/drm/msm/hdmi.txt | 22 > +- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt > b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > index c43aa53..acba581 100644 > --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt > +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > @@ -11,15 +11,27 @@ Required properties: > - interrupts: The interrupt signal from the hdmi block. > - clocks: device clocks >See ../clocks/clock-bindings.txt for details. > -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin > -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin > -- qcom,hdmi-tx-hpd-gpio: hpd pin > +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin > +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin > +- qcom,hdmi-tx-hpd-gpios: hpd pin > - core-vdda-supply: phandle to supply regulator > - hdmi-mux-supply: phandle to mux regulator > > +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use > + "qcom,hdmi-tx-ddc-clk-gpios" instead > +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use > + "qcom,hdmi-tx-ddc-data-gpios" instead > +- qcom,hdmi-tx-hpd-gpio: (deprecated) use > + "qcom,hdmi-tx-hpd-gpios" instead > + > Optional properties: > -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin > -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin > +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin > +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin > + > +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" > + instead > +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" > + instead > - pinctrl-names: the pin control state names; should contain "default" > - pinctrl-0: the default pinctrl state (active) > - pinctrl-1: the "sleep" pinctrl state I don't see much use in listing that these properties are deprecated. We already have code to catch the deprecated names, so having them in the binding will at best be distracting. Anyway, I don't know if there's been any advice on this from the device tree bindings maintainers, so adding devicetree at vger.kernel.org for visibility. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/2b5ab5b3/attachment-0001.sig>
[PATCH RFC 4/5] drm/msm/hdmi: deprecate non standard clock-names
On 10/08/15 14:33, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 02:18:15PM +0100, Srinivas Kandagatla wrote: >> >> >> On 10/08/15 13:49, Thierry Reding wrote: >>> On Mon, Aug 10, 2015 at 12:59:49PM +0100, Srinivas Kandagatla wrote: This patch updates the bindings to discourage the usage of non standard clock names, this will help in projects focused on upstreaming. These deprecated properties are still supported but will be remove over the time. Signed-off-by: Srinivas Kandagatla --- Documentation/devicetree/bindings/drm/msm/hdmi.txt | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt index 6dc202e..6fbfdd8 100644 --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt @@ -12,16 +12,16 @@ Required properties: - clocks: device clocks - clock-names: Corresponding name for each entry in the clocks property. for "qcom,hdmi-tx-8960" compatible names should be - "core_clk" - "master_iface_clk" - "slave_iface_clk" + "core_clk" is deprecated, use "core" instead + "master_iface_clk" is deprecated, use "master_iface" instead + "slave_iface_clk" is deprecated, use "slave_iface" instead for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should be - "extp_clk" - "alt_iface_clk" - "iface_clk" - "core_clk" - "mdp_core_clk" + "extp_clk" is deprecated, use "extp" instead + "alt_iface_clk" is deprecated, use "alt_iface" intstead + "iface_clk" is deprecated, use "iface" instead + "core_clk" is deprecated, use "core" instead + "mdp_core_clk" is deprecated, use "mdp_core" instead >>> >>> Shouldn't there be a driver counterpart of this to accept the new names? >> Driver changes are in this same series "[PATCH RFC 5/5] drm/msm/hdmi: remove >> _clk suffix from clock names"(https://lkml.org/lkml/2015/8/10/453) > > I don't have that patch in my inbox. It looks to be doing things > backwards (look up the deprecated name first). I think it should be: > > clk = devm_clk_get(dev, id); > if (IS_ERR(clk)) { If the clock controller is not ready yet, it would return EPROBE DEFER, which gets dropped here, as a result the driver would not be probed again. Probably both of the error codes needs be checked before returning. > char clk_name[32]; > > snprintf(clk_name, sizeof(clk_name), "%s_clk", id); > clk = devm_clk_get(dev, clk_name); > if (IS_ERR(clk)) > return clk; > } > > Also note how I've dropped the ERR_CAST(), that's not useful here > because you aren't actually casting, but simply returning clk. > > Thierry >
[PATCH RFC 2/5] drm/msm/hdmi: make use of standard gpio properties.
On Mon, Aug 10, 2015 at 12:59:34PM +0100, Srinivas Kandagatla wrote: > This patch modifies the driver to support standard gpio properties along > with deprecated properties. This will help us upstream and cleanup the > non-standard properties over the time. > > Signed-off-by: Srinivas Kandagatla > --- > drivers/gpu/drm/msm/hdmi/hdmi.c | 35 +-- > 1 file changed, 25 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c > index 8145362..e918889 100644 > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c > @@ -339,19 +339,34 @@ static const struct of_device_id dt_match[] = { > }; > > #ifdef CONFIG_OF > +/* This code will be removed once we move to gpiod based calls */ Why don't you do this now instead of duplicating what is essentially already implemented in gpiolib? Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/374fc867/attachment.sig>
[PATCH 2/2] drm/doc: Update docs about device instance setup
On Mon, Aug 10, 2015 at 02:07:49PM +0200, Daniel Vetter wrote: > On Mon, Aug 10, 2015 at 01:59:07PM +0200, Thierry Reding wrote: > > On Mon, Aug 10, 2015 at 11:55:38AM +0200, Daniel Vetter wrote: > > > ->load is depracated, bus functionst are deprecated and everyone > > > should use drm_dev_alloc®ister. > > > > Why would you want to deprecated ->load()? Even if you use > > drm_dev_alloc() and drm_dev_register(), there's still use for ->load() > > because it gives you the subsystem-level initialization entry point. > > ->load is called after the drm /dev node is registered and hence you can't > really do any driver setup in there without risking races. We paper over > that using drm_global_mutex, but that doesn't work for any other > driver/userspace interface like sysfs/debugfs because of deadlocks. > > And we can't just reorder ->load to happen before the /dev nodes are > registered because a lot of drivers would fall over if we do that. > > This is typical midlayer fail where the core calls into the driver instead > of the other way round. Okay, but then if we're going to deprecate ->load(), I think we should also come up with an upgrade plan. As it is, this just says that users shouldn't do ->load(), but it doesn't tell them what to do instead. Would the proper procedure be to fix drivers so that ->load() can be called between drm_dev_alloc() and drm_dev_register()? I suppose we could add some sort of (temporary) flag for drivers to signal this and then have drm_dev_register() call ->load() at the right time. If drivers don't support it, we can keep what we have. That, of course, doesn't get rid of the midlayer, so perhaps a better way forward would be to tell driver writers that they should be doing subsystem-level setup between drm_dev_alloc() and drm_dev_register(). Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/86959f42/attachment.sig>
[PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver
On Mon, 10 Aug 2015 12:39:21 +0200, Russell King - ARM Linux wrote: > > On Mon, Aug 10, 2015 at 12:05:07PM +0200, Takashi Iwai wrote: > > On Sat, 08 Aug 2015 18:10:06 +0200, > > Russell King wrote: > > > +static irqreturn_t snd_dw_hdmi_irq(int irq, void *data) > > > +{ > > > + struct snd_dw_hdmi *dw = data; > > > + struct snd_pcm_substream *substream; > > > + unsigned stat; > > > + > > > + stat = readb_relaxed(dw->data.base + HDMI_IH_AHBDMAAUD_STAT0); > > > + if (!stat) > > > + return IRQ_NONE; > > > + > > > + writeb_relaxed(stat, dw->data.base + HDMI_IH_AHBDMAAUD_STAT0); > > > + > > > + substream = dw->substream; > > > + if (stat & HDMI_IH_AHBDMAAUD_STAT0_DONE && substream) { > > > + snd_pcm_period_elapsed(substream); > > > + if (dw->substream) > > > + dw_hdmi_start_dma(dw); > > > + } > > > > Don't we need locking? > > Possibly. > > > In theory, the trigger can be issued while the irq is being handled. > > Well, we can't have a lock around the whole of the above, because that > results in deadlock (as snd_pcm_period_elapsed() can end up calling into > the trigger method.) Yes, and a usual workaround is to unlock temporarily at calling snd_pcm_period_elapsed(), then relock or call it at the end of handler. > I'm not happy to throw a spinlock around this > because of the in-built format conversion (something else I'm really not > happy about - which has to exist here because alsalib is soo painful > to add custom sample reformatting to - such modules have to be built > as part of alsalib itself rather than an add-on module.) I admit that alsa-lib code is very horrible to follow -- but I guess the change you'd need for iec958 plugin would be fairly small. We can add a config option and let iec958 behaving slightly differently depending on it. Meanwhile, having an in-kernel workaround makes it much easier to deploy, so I think it's OK to have this in driver for now. > > > +static int dw_hdmi_trigger(struct snd_pcm_substream *substream, int cmd) > > > +{ > > > + struct snd_dw_hdmi *dw = substream->private_data; > > > + int ret = 0; > > > + > > > + switch (cmd) { > > > + case SNDRV_PCM_TRIGGER_START: > > > + dw->buf_offset = 0; > > > + dw->substream = substream; > > > + dw_hdmi_start_dma(dw); > > > + dw_hdmi_audio_enable(dw->data.hdmi); > > > + substream->runtime->delay = substream->runtime->period_size; > > > + break; > > > + > > > + case SNDRV_PCM_TRIGGER_STOP: > > > + dw_hdmi_stop_dma(dw); > > > + dw_hdmi_audio_disable(dw->data.hdmi); > > > + break; > > > + > > > + default: > > > + ret = -EINVAL; > > > + break; > > > > SNDRV_PCM_TRIGGER_SUSPEND may be passed at suspend, too. > > I think rather than adding code which would be difficult for me to test, > I'd instead remove the suspend/resume callbacks, or at least disable them > until someone can test that feature, or is willing to implement it. That's fine. > > > +static snd_pcm_uframes_t dw_hdmi_pointer(struct snd_pcm_substream > > > *substream) > > > +{ > > > + struct snd_pcm_runtime *runtime = substream->runtime; > > > + struct snd_dw_hdmi *dw = substream->private_data; > > > + > > > + return bytes_to_frames(runtime, dw->buf_offset); > > > > So, this returns the offset that has been reformatted. Does the > > hardware support any better position reporting? We may give the delay > > from the driver if possible. > > Basically, no. Reading a 32-bit DMA position as separate bytes while > DMA is active is racy. > > This is the best we can do, and the way we report the position has been > arrived at after what's getting on for two years of testing with > pulseaudio, vlc direct access & spdif pass-through, aplay, etc: > > Author: Russell King > Date: Thu Nov 7 16:01:45 2013 + > > drm: bridge/dw_hdmi-ahb-audio: add audio driver OK, then this is a driver with the low update granularity. Hopefully we'll get some good API to indicate that in near future, as we've been discussing about it for a while. thanks, Takashi
[PATCH 00/12] dw-hdmi development
On Sat, Aug 08, 2015 at 05:02:51PM +0100, Russell King - ARM Linux wrote: > This sub-series is a mixture of development: > > * Removing the incorrect pixel repetition configuration code > * Preventing pixel-doubled modes from being used > * Adding interlaced video support > * Implementing the sink_is_hdmi/sink_has_audio flags I suggested a few > months ago > * Only enabling audio support if the sink indicates it has audio > * Avoiding double-enabling the HDMI interface > * Fixing the mis-leading name of "dw_hdmi_phy_enable_power" > * Adding connector mode forcing (important if your monitor bounces > RXSENSE and HPD signals while in low-power mode.) > * Improving the HDMI enable/disabling on sink status > > For review (and testing if people feel like it). Acks/tested-bys etc > welcome. It applies on top of my drm-dwhdmi-devel branch, which is > waiting for David Airlie to pull (see pull request on dri-devel, 15th > July.) Hi Russell, I have in the past merged patches for the bridge subdirectory via the drm/panel tree, though lately much of the dw-hdmi patches have gone in via Philipp or you directly. This seems to have worked fine so far, but this time around I carry a patch to clean up Kconfig and Makefile a little and bring more consistency to the subdirectory and I think it's going to conflict with your series here (and potentially any ongoing work you have). Would you be open to me picking up these patches into the drm/panel tree? It feeds into linux-next, so the code would get some exposure before Dave's return. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/867cb263/attachment.sig>
[PATCH RFC 4/5] drm/msm/hdmi: deprecate non standard clock-names
On 10/08/15 13:49, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 12:59:49PM +0100, Srinivas Kandagatla wrote: >> This patch updates the bindings to discourage the usage of non standard >> clock names, this will help in projects focused on upstreaming. >> >> These deprecated properties are still supported but will be remove over >> the time. >> >> Signed-off-by: Srinivas Kandagatla >> --- >> Documentation/devicetree/bindings/drm/msm/hdmi.txt | 16 >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> index 6dc202e..6fbfdd8 100644 >> --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> @@ -12,16 +12,16 @@ Required properties: >> - clocks: device clocks >> - clock-names: Corresponding name for each entry in the clocks property. >>for "qcom,hdmi-tx-8960" compatible names should be >> -"core_clk" >> -"master_iface_clk" >> -"slave_iface_clk" >> +"core_clk" is deprecated, use "core" instead >> +"master_iface_clk" is deprecated, use "master_iface" instead >> +"slave_iface_clk" is deprecated, use "slave_iface" instead >> >>for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should be >> -"extp_clk" >> -"alt_iface_clk" >> -"iface_clk" >> -"core_clk" >> -"mdp_core_clk" >> +"extp_clk" is deprecated, use "extp" instead >> +"alt_iface_clk" is deprecated, use "alt_iface" intstead >> +"iface_clk" is deprecated, use "iface" instead >> +"core_clk" is deprecated, use "core" instead >> +"mdp_core_clk" is deprecated, use "mdp_core" instead > > Shouldn't there be a driver counterpart of this to accept the new names? Driver changes are in this same series "[PATCH RFC 5/5] drm/msm/hdmi: remove _clk suffix from clock names"(https://lkml.org/lkml/2015/8/10/453) --srini > Otherwise people could be switching the DTS to the new value, but the > driver won't find the clocks it's looking for. > > Thierry >
[PATCH RFC 2/5] drm/msm/hdmi: make use of standard gpio properties.
On 10/08/15 13:38, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 12:59:34PM +0100, Srinivas Kandagatla wrote: >> This patch modifies the driver to support standard gpio properties along >> with deprecated properties. This will help us upstream and cleanup the >> non-standard properties over the time. >> >> Signed-off-by: Srinivas Kandagatla >> --- >> drivers/gpu/drm/msm/hdmi/hdmi.c | 35 +-- >> 1 file changed, 25 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c >> b/drivers/gpu/drm/msm/hdmi/hdmi.c >> index 8145362..e918889 100644 >> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c >> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c >> @@ -339,19 +339,34 @@ static const struct of_device_id dt_match[] = { >> }; >> >> #ifdef CONFIG_OF >> +/* This code will be removed once we move to gpiod based calls */ > > Why don't you do this now instead of duplicating what is essentially > already implemented in gpiolib? > One of the thing that Rob asked in his comments (http://www.spinics.net/lists/arm-kernel/msg437675.html) was to retain the support for old devices, moving to gpiod ATM would break such devices as they are still using legacy gpiolib and its naming. If Rob is ok to drop gpio properties which does not have "-gpio" or "-gpios" suffix then we can get rid of this function all together. --srini > Thierry >
Caching of EDID for X server to decrease startup time of X server
On Mon, Aug 10, 2015 at 03:01:49PM +0200, Thierry Reding wrote: > Russell and Sascha were discussing this kind of caching in the i.MX > driver recently. Adding both for visibility. Also not trimming the quote > in case they don't have the original. > > It sounds like this could be useful to have in the core. As I understand > it, hotplug detection is pretty well specified for more modern display > interfaces (like HDMI and DisplayPort), so I think caching of this sort > could work for those. However, I think some older interfaces such as VGA > (or perhaps even DVI as well) don't have reliable hotplug detection and > hence would need to be able to force reading the EDID. > > Still, perhaps a connector flag could be introduced to enable caching on > a per-connector basis, and then we should be able to deal with this in > the DRM core, rather than have per-driver quirks. We've shied away from EDID caching precisely because HPD on Intel is notoriously unreliable. The last suggestion no one followed up with was a short term EDID cache. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH] drm/atomic: Call ww_acquire_done after check phase is complete
On Thu, Aug 06, 2015 at 03:06:40PM +0200, Daniel Vetter wrote: > We want to make sure that no one tries to acquire more locks and > states, and ww mutexes provide debug facilities for that. So use them. > > v2: Only call acquire_done when ->atomic_check was successful to avoid > falling over an -EDEADLK (spotted by Maarten). > > Cc: Rob Clark > Cc: Maarten Lankhorst > Signed-off-by: Daniel Vetter Picked up to drm-misc with Maarten's irc r-b. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/atomic: Paper over locking WARN in default_state_clear
On Fri, Jul 31, 2015 at 03:41:15PM +0200, Daniel Vetter wrote: > On Fri, Jul 31, 2015 at 10:34:43AM +0200, Maarten Lankhorst wrote: > > Hey, > > > > Op 29-07-15 om 12:51 schreef Daniel Vetter: > > > In > > > > > > commit 6f75cea66c8dd043ced282016b21a639af176642 > > > Author: Daniel Vetter > > > Date: Wed Nov 19 18:38:07 2014 +0100 > > > > > > drm/atomic: Only destroy connector states with connection mutex held > > > > > > I tried to fix races of atomic commits against connector > > > hot-unplugging. The idea is to ensure lifetimes by holding the > > > connection_mutex long enough. That works for synchronous commits, but > > > not for async ones. > > > > > > For async atomic commit we really need to fix up connector lifetimes > > > for real. But that's a much bigger task, so just add more duct-tape: > > > For cleaning up connector states we currently don't need the connector > > > itself. So NULL it out and remove the locking check. Of course that > > > check was to protect the entire sequence, but the modeset itself > > > should be save since currently DP MST hot-removal does a dpms-off. And > > > that should synchronize with any outstanding async atomic commit. > > > > > > Or at least that's my hope, this is all a giant mess. > > > > > > Reported-by: Maarten Lankhorst > > > Cc: Maarten Lankhorst > > > Signed-off-by: Daniel Vetter > > > --- > > > drivers/gpu/drm/drm_atomic.c | 12 +--- > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > > index 3efd91c0c6cb..434915448ea0 100644 > > > --- a/drivers/gpu/drm/drm_atomic.c > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > @@ -153,9 +153,15 @@ void drm_atomic_state_default_clear(struct > > > drm_atomic_state *state) > > > if (!connector) > > > continue; > > > > > > - WARN_ON(!drm_modeset_is_locked(&config->connection_mutex)); > > > - > > > - connector->funcs->atomic_destroy_state(connector, > > > + /* > > > + * FIXME: Async commits can race with connector unplugging and > > > + * there's currently nothing that prevents cleanup up state for > > > + * deleted connectors. As long as the callback doesn't look at > > > + * the connector we'll be fine though, so make sure that's the > > > + * case by setting all connector pointers to NULL. > > > + */ > > > + state->connector_states[i]->connector = NULL; > > > + connector->funcs->atomic_destroy_state(NULL, > > > > > > state->connector_states[i]); > > > > > This wouldn't provide any additional guarantee during the async commit > > itself, so please don't do this. :-) > > Nope, it's really just a big reminder that we have a bug here. Ok, picked up to drm-misc with Maarten's irc r-b. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/i915: Use CONFIG_DRM_FBDEV_EMULATION
On Mon, Aug 10, 2015 at 01:48:53PM +0200, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 01:34:08PM +0200, Daniel Vetter wrote: > > Instead of our own duplicated one. This fixes a bug in the driver > > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > > try to unregister the nonexistent fbdev drm_framebuffer. > > > > Cc: Archit Taneja > > Cc: Maarten Lankhorst > > Reported-by: Maarten Lankhorst > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/i915/Kconfig | 15 --- > > drivers/gpu/drm/i915/Makefile| 2 +- > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > 7 files changed, 8 insertions(+), 23 deletions(-) > > Isn't this going to cause some pain to users because .config may not > have this symbol yet? Arguably this is somewhat mitigated by the fact > that both symbols are "default y", but technically somebody could have > DRM_I915_FBDEV=n in their .config and after this change fbdev emulation > will be switched on again. > > I'm not sure how to upgrade more sanely, though, so perhaps this is just > a bullet that needs biting. There are other drivers two with their private FBDEV option (like msm) so I don't think we can do any sensible upgrade logic that just works. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Yakir, Am Samstag, 8. August 2015, 11:54:38 schrieb Yakir Yang: > >> +static int rockchip_dp_init(struct rockchip_dp_device *dp) > >> +{ > >> + struct device *dev = dp->dev; > >> + struct device_node *np = dev->of_node; > >> + int ret; > >> + > >> + dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); > >> + if (IS_ERR(dp->grf)) { > >> + dev_err(dev, > >> + "rk3288-dp needs rockchip,grf property\n"); > >> + return PTR_ERR(dp->grf); > >> + } > >> + > >> + dp->clk_dp = devm_clk_get(dev, "clk_dp"); > > > > I've looked at the manual, but couldn't find an actual clock-name > > used there. Is it really "clk_dp" or should it just be "dp"? > > This should be "clk_dp", not "dp". > Cause analogix_dp_core would need a clock name with "dp", so I would > rather to pasted my rockchip-dp node here before I add dt-bindings in > next version ;) The clock we name PCLK_EDP_CTRL in the clock controller is probably the clock supplying the APB interface and named pclk already in the "Figure 3-2 DP_TXclock domain" diagram on page 19 of the manual. So your "clk_dp" should actually be "pclk". So you would have "dp", "dp_24m" and "pclk" for the 3 supplying clocks. > > edp: edp at ff97 { > compatible = "rockchip,rk3288-dp"; > reg = <0xff97 0x4000>; > interrupts = ; > > clocks = <&cru SCLK_EDP>, <&cru SCLK_EDP_24M>, <&cru > PCLK_EDP_CTRL>; > clock-names = "clk_dp", "clk_dp_24m", "dp"; > > rockchip,grf = <&grf>; > resets = <&cru 111>; > reset-names = "dp"; > power-domains = <&power RK3288_PD_VIO>; > status = "disabled"; > > hsync-active-high = <0>; > vsync-active-high = <0>; > interlaced = <0>; > samsung,color-space = <0>; > samsung,dynamic-range = <0>; > samsung,ycbcr-coeff = <0>; > samsung,color-depth = <1>; > samsung,link-rate = <0x0a>; > samsung,lane-count = <1>; Thierry already said, that these should probably be somehow auto-detected. Properties needing to stay around should probably also be "analogix,..." with a fallback to not break Samsung devicetrees, so look for "analogix,foo!, if not found try "samsung,foo" > ports { > edp_in: port { > #address-cells = <1>; > #size-cells = <0>; > edp_in_vopb: endpoint at 0 { > reg = <0>; > remote-endpoint = <&vopb_out_edp>; > }; > }; > }; > >> + > >> + dp->clk_24m = devm_clk_get(dev, "clk_dp_24m"); > > > > Same here, maybe "dp_24m". > > Like my previous reply. And actually as those two clocks all have > a common prefix "SCLK" in rk3288 clock tree, I thinkt we can name > them to "sclk_dp" & "sclk_dp_24m", is it okay ? As Thierry said, please don't add prefixes. > > >> + if (IS_ERR(dp->clk_24m)) { > >> + dev_err(dev, "cannot get clk_dp_24m\n"); > >> + return PTR_ERR(dp->clk_24m); > >> + } > > > > I think you're missing the pclk here (PCLK_EDP_CTRL) or is this part of > > something else? > > Whops, as I refered in commit message I leave pclk_dp to > analogix_dp_core driver ;-) > > The reason why I want to leave pclk is I thought this clock is more like > analogix dp > core driver want, like a IP controller clock (whatever analogix_dp do > need a clock > named with "dp"). Hmm, I'd think what the core (and Samsung) driver use as "dp" clock is probably the generic clock for the IP and not the pclk for the APB interface. So I think it still should be "dp" for the core and "dp_24m" + "pclk" for the rockchip part? > > >> + > >> + dp->rst = devm_reset_control_get(dev, "dp"); > >> + if (IS_ERR(dp->rst)) { > >> + dev_err(dev, "failed to get reset\n"); > >> + return PTR_ERR(dp->rst); > >> + } > >> + > >> + ret = rockchip_dp_clk_enable(dp); > >> + if (ret < 0) { > >> + dev_err(dp->dev, "cannot enable dp clk %d\n", ret); > >> + return ret; > >> + } > >> + > >> + ret = rockchip_dp_pre_init(dp); > >> + if (ret < 0) { > >> + dev_err(dp->dev, "failed to pre init %d\n", ret); > >> + return ret; > >> + } > >> + > >> + return 0; > >> +} > > > > [...] > > > >> +static int rockchip_dp_probe(struct platform_device *pdev) > >> +{ > >> + struct device *dev = &pdev->dev; > >> + struct device_node *panel_node; > >> + struct rockchip_dp_device *dp; > >> + struct drm_panel *panel; > >> + > >> + panel_node = of_parse_phandle(dev->of_node, "rockchip,panel", 0); > >> + if (!panel_node) { > >> + DRM_ERROR(
[PATCH 1/2] drm/edid: Use ARRAY_SIZE in drm_add_modes_noedid
On Mon, Aug 10, 2015 at 01:57:21PM +0200, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 11:55:37AM +0200, Daniel Vetter wrote: > > Spotted while reading code for random reasons. > > > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/drm_edid.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 4a403eb90ded..4780b1924bef 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -3810,7 +3810,7 @@ int drm_add_modes_noedid(struct drm_connector > > *connector, > > struct drm_display_mode *mode; > > struct drm_device *dev = connector->dev; > > > > - count = sizeof(drm_dmt_modes) / sizeof(struct drm_display_mode); > > + count = ARRAY_SIZE(drm_dmt_modes); > > if (hdisplay < 0) > > hdisplay = 0; > > if (vdisplay < 0) > > > Reviewed-by: Thierry Reding Thanks for the review, applied to drm-misc. -Danie -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 2/2] drm/doc: Update docs about device instance setup
On Mon, Aug 10, 2015 at 01:59:07PM +0200, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 11:55:38AM +0200, Daniel Vetter wrote: > > ->load is depracated, bus functionst are deprecated and everyone > > should use drm_dev_alloc®ister. > > Why would you want to deprecated ->load()? Even if you use > drm_dev_alloc() and drm_dev_register(), there's still use for ->load() > because it gives you the subsystem-level initialization entry point. ->load is called after the drm /dev node is registered and hence you can't really do any driver setup in there without risking races. We paper over that using drm_global_mutex, but that doesn't work for any other driver/userspace interface like sysfs/debugfs because of deadlocks. And we can't just reorder ->load to happen before the /dev nodes are registered because a lot of drivers would fall over if we do that. This is typical midlayer fail where the core calls into the driver instead of the other way round. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 00/18] dev->struct_mutex crusade
On Mon, Aug 10, 2015 at 01:35:58PM +0200, Thierry Reding wrote: > On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote: > > Hi all, > > > > I wanted to take another look at struct_mutex usage in modern (gem) drivers > > and > > noticed that for a fair lot we're very to be completely struct_mutex free. > > > > This pile here is the the simple part, which mostly just removes code and > > mutex_lock/unlock calls. All the patches here are independent and can be > > merged > > in any order whatsoever. My plan is to send out a pull request for all > > those not > > picked up by driver maintainers in 2-3 weeks or so, assuming no one > > complains. > > > > Of course review & comments still very much welcome. > > > > The more tricky 2nd part of this (and that one's not yet done) is to rework > > the > > gem mmap handler to use the same kref_get_unless_zero trick as ttm. With > > that > > there's no core requirement to hold struct_mutex over the final unref, which > > means we can make that one lockless. I plan to add a > > gem_object_free_unlocked > > for all the drivers which don't have any need for this lock. > > > > Also there's a few more drivers which can be made struct_mutex free easily, > > I'll > > propably stitch together poc patches for those. > > There's a concurrency bug in Tegra DRM currently because we don't lock > accesses to drm_mm (I guess this demonstrates how badly we need better > testing...) and it seems like this is typically protected by the very > same struct_mutex that you're on a crusade to remove. If your goal is > to get rid of it for good, should we simply add a separate lock just > for the drm_mm? We don't have another one that would fit. Yes, please just add your own driver-private lock. The next struct_mutex crusade series will actually do exactly that, at least for simple cases like armada. With that changes most drivers don't care about struct_mutex any more in their ->gem_free_object hook and I plan to add a new ->gem_free_object_unlocked variant for all the drivers which don't have any residual usage of struct_mutex. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 2/2] drm/doc: Update docs about device instance setup
On Mon, Aug 10, 2015 at 11:55:38AM +0200, Daniel Vetter wrote: > ->load is depracated, bus functionst are deprecated and everyone > should use drm_dev_alloc®ister. Why would you want to deprecated ->load()? Even if you use drm_dev_alloc() and drm_dev_register(), there's still use for ->load() because it gives you the subsystem-level initialization entry point. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/6fff2876/attachment.sig>
[PATCH] intel: Add new resource streamer interface
Signed-off-by: Abdiel Janulgue --- include/drm/i915_drm.h | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index ded43b1..73ed1bf 100644 --- a/include/drm/i915_drm.h +++ b/include/drm/i915_drm.h @@ -350,6 +350,7 @@ typedef struct drm_i915_irq_wait { #define I915_PARAM_REVISION 32 #define I915_PARAM_SUBSLICE_TOTAL 33 #define I915_PARAM_EU_TOTAL 34 +#define I915_PARAM_HAS_RESOURCE_STREAMER 36 typedef struct drm_i915_getparam { int param; @@ -760,7 +761,12 @@ struct drm_i915_gem_execbuffer2 { #define I915_EXEC_BSD_RING1(1<<13) #define I915_EXEC_BSD_RING2(2<<13) -#define __I915_EXEC_UNKNOWN_FLAGS -(1<<15) +/** Tell the kernel that the batchbuffer is processed by + * the resource streamer. + */ +#define I915_EXEC_RESOURCE_STREAMER (1<<15) + +#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER<<1) #define I915_EXEC_CONTEXT_ID_MASK (0x) #define i915_execbuffer2_set_context_id(eb2, context) \ -- 1.9.1
[PATCH 1/2] drm/edid: Use ARRAY_SIZE in drm_add_modes_noedid
On Mon, Aug 10, 2015 at 11:55:37AM +0200, Daniel Vetter wrote: > Spotted while reading code for random reasons. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_edid.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 4a403eb90ded..4780b1924bef 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -3810,7 +3810,7 @@ int drm_add_modes_noedid(struct drm_connector > *connector, > struct drm_display_mode *mode; > struct drm_device *dev = connector->dev; > > - count = sizeof(drm_dmt_modes) / sizeof(struct drm_display_mode); > + count = ARRAY_SIZE(drm_dmt_modes); > if (hdisplay < 0) > hdisplay = 0; > if (vdisplay < 0) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/8858ba09/attachment.sig>
[PATCH 00/18] dev->struct_mutex crusade
On Mon, Aug 10, 2015 at 12:53:23PM +0100, Chris Wilson wrote: > On Mon, Aug 10, 2015 at 01:35:58PM +0200, Thierry Reding wrote: > > On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote: > > > Hi all, > > > > > > I wanted to take another look at struct_mutex usage in modern (gem) > > > drivers and > > > noticed that for a fair lot we're very to be completely struct_mutex free. > > > > > > This pile here is the the simple part, which mostly just removes code and > > > mutex_lock/unlock calls. All the patches here are independent and can be > > > merged > > > in any order whatsoever. My plan is to send out a pull request for all > > > those not > > > picked up by driver maintainers in 2-3 weeks or so, assuming no one > > > complains. > > > > > > Of course review & comments still very much welcome. > > > > > > The more tricky 2nd part of this (and that one's not yet done) is to > > > rework the > > > gem mmap handler to use the same kref_get_unless_zero trick as ttm. With > > > that > > > there's no core requirement to hold struct_mutex over the final unref, > > > which > > > means we can make that one lockless. I plan to add a > > > gem_object_free_unlocked > > > for all the drivers which don't have any need for this lock. > > > > > > Also there's a few more drivers which can be made struct_mutex free > > > easily, I'll > > > propably stitch together poc patches for those. > > > > There's a concurrency bug in Tegra DRM currently because we don't lock > > accesses to drm_mm (I guess this demonstrates how badly we need better > > testing...) and it seems like this is typically protected by the very > > same struct_mutex that you're on a crusade to remove. If your goal is > > to get rid of it for good, should we simply add a separate lock just > > for the drm_mm? We don't have another one that would fit. > > Actually that is one of the first targets for more fine-grained locking. > I would not add a new lock to drm_mm as at least for i915, we want to use > a similar per-vm lock (of which the drm_mm is just one part). Sorry if I was being unclear. I wasn't suggesting adding the lock to struct drm_mm, but rather add a driver-specific one specifically to serialize accesses to the drm_mm. I agree that it's better to do this in a driver-specific way because other structures may need to be protected by the same lock. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/4a0b5d9e/attachment.sig>
[PATCH] drm/i915: Use CONFIG_DRM_FBDEV_EMULATION
On Mon, Aug 10, 2015 at 01:34:08PM +0200, Daniel Vetter wrote: > Instead of our own duplicated one. This fixes a bug in the driver > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > try to unregister the nonexistent fbdev drm_framebuffer. > > Cc: Archit Taneja > Cc: Maarten Lankhorst > Reported-by: Maarten Lankhorst > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/Kconfig | 15 --- > drivers/gpu/drm/i915/Makefile| 2 +- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > 7 files changed, 8 insertions(+), 23 deletions(-) Isn't this going to cause some pain to users because .config may not have this symbol yet? Arguably this is somewhat mitigated by the fact that both symbols are "default y", but technically somebody could have DRM_I915_FBDEV=n in their .config and after this change fbdev emulation will be switched on again. I'm not sure how to upgrade more sanely, though, so perhaps this is just a bullet that needs biting. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/7032657b/attachment.sig>
[PATCH] intel: Add new resource streamer interface
On Mon, Aug 10, 2015 at 01:58:37PM +0300, Abdiel Janulgue wrote: > Signed-off-by: Abdiel Janulgue This header should be generated from the kernel version using make headers_install, and hence should reference from which kernel version (drm-intel-next tag or release tag) this has been generated from. Also you should then update everything ofc. > --- > include/drm/i915_drm.h | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h > index ded43b1..73ed1bf 100644 > --- a/include/drm/i915_drm.h > +++ b/include/drm/i915_drm.h > @@ -350,6 +350,7 @@ typedef struct drm_i915_irq_wait { > #define I915_PARAM_REVISION 32 > #define I915_PARAM_SUBSLICE_TOTAL 33 > #define I915_PARAM_EU_TOTAL 34 Spotted that you're not following process since 35 here is missing. -Daniel > +#define I915_PARAM_HAS_RESOURCE_STREAMER 36 > > typedef struct drm_i915_getparam { > int param; > @@ -760,7 +761,12 @@ struct drm_i915_gem_execbuffer2 { > #define I915_EXEC_BSD_RING1 (1<<13) > #define I915_EXEC_BSD_RING2 (2<<13) > > -#define __I915_EXEC_UNKNOWN_FLAGS -(1<<15) > +/** Tell the kernel that the batchbuffer is processed by > + * the resource streamer. > + */ > +#define I915_EXEC_RESOURCE_STREAMER (1<<15) > + > +#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER<<1) > > #define I915_EXEC_CONTEXT_ID_MASK(0x) > #define i915_execbuffer2_set_context_id(eb2, context) \ > -- > 1.9.1 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Bug 73530] Asus U38N: Black screen with Radeon driver in Linux
https://bugs.freedesktop.org/show_bug.cgi?id=73530 --- Comment #114 from Christian AÃfalg --- Actually, for me on Arch it works with 4.1.4, too. I tried 4.2 RC5 as well and that works too. That's great! -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/5488eeb7/attachment-0001.html>
[PATCH] drm: Remove two-level menu in Kconfig
On Mon, Aug 10, 2015 at 01:32:08PM +0300, Tomi Valkeinen wrote: > > > On 07/08/15 19:10, Thierry Reding wrote: > > From: Thierry Reding > > > > The Direct Rendering Manager Kconfig option is already a separate menu, > > so remove the extra level to make it easier to navigate. > > > > Signed-off-by: Thierry Reding > > --- > > drivers/video/Kconfig | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig > > index 8bf495ffb020..e0606c01e8ac 100644 > > --- a/drivers/video/Kconfig > > +++ b/drivers/video/Kconfig > > @@ -22,9 +22,7 @@ source "drivers/gpu/vga/Kconfig" > > source "drivers/gpu/host1x/Kconfig" > > source "drivers/gpu/ipu-v3/Kconfig" > > > > -menu "Direct Rendering Manager" > > source "drivers/gpu/drm/Kconfig" > > -endmenu > > > > menu "Frame buffer Devices" > > source "drivers/video/fbdev/Kconfig" > > > > Thanks, queued for 4.3. Oops failed to send out my usual mail, but I picked this already up in topic/drm-misc. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 03/18] drm/gem: Be more friendly with locking checks
On Mon, Aug 10, 2015 at 12:42:30PM +0200, Thierry Reding wrote: > On Thu, Jul 09, 2015 at 11:32:35PM +0200, Daniel Vetter wrote: > > BUG_ON kills the driver, WARN_ON is much friendly. And usually nothing > > bad happens when the locking is lightly busted. > > s/much friendly/much friendlier/, s/lightly/slightly/? Yeah, changed. > Otherwise: > > Reviewed-by: Thierry Reding Thanks for your review, I've applied the patches to topic/drm-misc except armada (needs to be split), one nouveau patch (superseeded meanwhile by Archit's work), radeon/amdgpu (I'll ping Alex) and the two tegra ones. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 1/2] drm/tegra: don't take dev->struct_mutex in mmap offset ioctl
On Mon, Aug 10, 2015 at 01:31:42PM +0200, Daniel Vetter wrote: > On Mon, Aug 10, 2015 at 12:30:21PM +0200, Thierry Reding wrote: > > On Wed, Jul 15, 2015 at 03:38:51PM +0200, Daniel Vetter wrote: > > > Since David Herrmann's mmap vma manager rework we don't need to grab > > > dev->struct_mutex any more to prevent races when looking up the mmap > > > offset. Drop it and instead don't forget to use the unref_unlocked > > > variant (since the drm core still cares). > > > > > > While at it also fix a leak when this ioctl is called on an imported > > > buffer. > > > > I don't see where the leak's fixed, but other than that this looks good > > to me. Shall I pick this up into the drm/tegra tree? > > Copypaste in the commit message from armada, doesn't apply to tegra. Do > you also plan to pick up "drm/tegra: Use > drm_gem_object_reference_unlocked" directly? I don't mind much either way. I don't think they'll conflict with anything, but since you already said that they're all independent, I don't see a reason why I shouldn't pull them into drm/tegra. If you prefer to keep them together, that's fine with me too. Thanks, Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/3b04f067/attachment.sig>
[PATCH 00/18] dev->struct_mutex crusade
On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote: > Hi all, > > I wanted to take another look at struct_mutex usage in modern (gem) drivers > and > noticed that for a fair lot we're very to be completely struct_mutex free. > > This pile here is the the simple part, which mostly just removes code and > mutex_lock/unlock calls. All the patches here are independent and can be > merged > in any order whatsoever. My plan is to send out a pull request for all those > not > picked up by driver maintainers in 2-3 weeks or so, assuming no one complains. > > Of course review & comments still very much welcome. > > The more tricky 2nd part of this (and that one's not yet done) is to rework > the > gem mmap handler to use the same kref_get_unless_zero trick as ttm. With that > there's no core requirement to hold struct_mutex over the final unref, which > means we can make that one lockless. I plan to add a gem_object_free_unlocked > for all the drivers which don't have any need for this lock. > > Also there's a few more drivers which can be made struct_mutex free easily, > I'll > propably stitch together poc patches for those. There's a concurrency bug in Tegra DRM currently because we don't lock accesses to drm_mm (I guess this demonstrates how badly we need better testing...) and it seems like this is typically protected by the very same struct_mutex that you're on a crusade to remove. If your goal is to get rid of it for good, should we simply add a separate lock just for the drm_mm? We don't have another one that would fit. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/2dce169e/attachment-0001.sig>
[PATCH] drm/i915: Use CONFIG_DRM_FBDEV_EMULATION
Instead of our own duplicated one. This fixes a bug in the driver unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we try to unregister the nonexistent fbdev drm_framebuffer. Cc: Archit Taneja Cc: Maarten Lankhorst Reported-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/Kconfig | 15 --- drivers/gpu/drm/i915/Makefile| 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_display.c | 4 ++-- drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- drivers/gpu/drm/i915/intel_drv.h | 2 +- 7 files changed, 8 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index eb87e2538861..051eab33e4c7 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -36,21 +36,6 @@ config DRM_I915 i810 driver instead, and the Atom z5xx series has an entirely different implementation. -config DRM_I915_FBDEV - bool "Enable legacy fbdev support for the modesetting intel driver" - depends on DRM_I915 - select DRM_KMS_FB_HELPER - select FB_CFB_FILLRECT - select FB_CFB_COPYAREA - select FB_CFB_IMAGEBLIT - default y - help - Choose this option if you have a need for the legacy fbdev - support. Note that this support also provide the linux console - support on top of the intel modesetting driver. - - If in doubt, say "Y". - config DRM_I915_PRELIMINARY_HW_SUPPORT bool "Enable preliminary support for prerelease Intel hardware by default" depends on DRM_I915 diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 41fb8a9c5bef..998b4643109f 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -62,7 +62,7 @@ i915-y += intel_audio.o \ intel_sideband.o \ intel_sprite.o i915-$(CONFIG_ACPI)+= intel_acpi.o intel_opregion.o -i915-$(CONFIG_DRM_I915_FBDEV) += intel_fbdev.o +i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o # modesetting output/encoder code i915-y += dvo_ch7017.o \ diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 95e7b82f05d1..86734be84a65 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1868,7 +1868,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data) struct intel_framebuffer *fb; struct drm_framebuffer *drm_fb; -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION struct drm_i915_private *dev_priv = dev->dev_private; ifbdev = dev_priv->fbdev; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4932d298e3af..95c115f6a4c7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1855,7 +1855,7 @@ struct drm_i915_private { struct drm_i915_gem_object *vlv_pctx; -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION /* list of fbdev register on this device */ struct intel_fbdev *fbdev; struct work_struct fbdev_suspend_work; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 16c8052f7eab..9a2f229a1c3a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10131,7 +10131,7 @@ static struct drm_framebuffer * mode_fits_in_fbdev(struct drm_device *dev, struct drm_display_mode *mode) { -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; struct drm_framebuffer *fb; @@ -14313,7 +14313,7 @@ intel_user_framebuffer_create(struct drm_device *dev, return intel_framebuffer_create(dev, mode_cmd, obj); } -#ifndef CONFIG_DRM_I915_FBDEV +#ifndef CONFIG_DRM_FBDEV_EMULATION static inline void intel_fbdev_output_poll_changed(struct drm_device *dev) { } diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index f4fe1183bae6..369f8b6b804f 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -406,7 +406,7 @@ static bool intel_dp_mst_get_hw_state(struct intel_connector *connector) static void intel_connector_add_to_fbdev(struct intel_connector *connector) { -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION struct drm_i915_private *dev_priv = to_i915(connector->base.dev); drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper, &connector->base); #endif @@ -414,7 +414,7 @@ static void intel_connector_add_to_fbdev(struct intel_connector *connector) static void intel_connector_remove_from_fbdev(struct intel_connector *connector) { -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION struct drm_i
[PATCH 01/18] drm/gem: rip out drm vma accounting for gem mmaps
On Thu, Jul 09, 2015 at 11:32:33PM +0200, Daniel Vetter wrote: > Doesn't really add anything which can't be figured out through > proc files. And more clearly separates the new gem mmap handling > code from the old drm maps mmap handling code, which is surely a > good thing. > > Cc: Martin Peres > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_gem.c | 11 +-- > 1 file changed, 1 insertion(+), 10 deletions(-) Doesn't this mean that the "vma" debugfs file will now be empty for GEM drivers? Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/a46d55fc/attachment.sig>
[PATCH] drm: Remove two-level menu in Kconfig
On 07/08/15 19:10, Thierry Reding wrote: > From: Thierry Reding > > The Direct Rendering Manager Kconfig option is already a separate menu, > so remove the extra level to make it easier to navigate. > > Signed-off-by: Thierry Reding > --- > drivers/video/Kconfig | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig > index 8bf495ffb020..e0606c01e8ac 100644 > --- a/drivers/video/Kconfig > +++ b/drivers/video/Kconfig > @@ -22,9 +22,7 @@ source "drivers/gpu/vga/Kconfig" > source "drivers/gpu/host1x/Kconfig" > source "drivers/gpu/ipu-v3/Kconfig" > > -menu "Direct Rendering Manager" > source "drivers/gpu/drm/Kconfig" > -endmenu > > menu "Frame buffer Devices" > source "drivers/video/fbdev/Kconfig" > Thanks, queued for 4.3. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/b7507f89/attachment.sig>
[PATCH 1/2] drm/tegra: don't take dev->struct_mutex in mmap offset ioctl
On Mon, Aug 10, 2015 at 12:30:21PM +0200, Thierry Reding wrote: > On Wed, Jul 15, 2015 at 03:38:51PM +0200, Daniel Vetter wrote: > > Since David Herrmann's mmap vma manager rework we don't need to grab > > dev->struct_mutex any more to prevent races when looking up the mmap > > offset. Drop it and instead don't forget to use the unref_unlocked > > variant (since the drm core still cares). > > > > While at it also fix a leak when this ioctl is called on an imported > > buffer. > > I don't see where the leak's fixed, but other than that this looks good > to me. Shall I pick this up into the drm/tegra tree? Copypaste in the commit message from armada, doesn't apply to tegra. Do you also plan to pick up "drm/tegra: Use drm_gem_object_reference_unlocked" directly? And thanks for all the review. -Daniel > > Thierry > > > Cc: Thierry Reding > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/tegra/gem.c | 5 - > > 1 file changed, 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c > > index 01e16e146bfe..827838e64d6e 100644 > > --- a/drivers/gpu/drm/tegra/gem.c > > +++ b/drivers/gpu/drm/tegra/gem.c > > @@ -408,12 +408,9 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, > > struct drm_device *drm, > > struct drm_gem_object *gem; > > struct tegra_bo *bo; > > > > - mutex_lock(&drm->struct_mutex); > > - > > gem = drm_gem_object_lookup(drm, file, handle); > > if (!gem) { > > dev_err(drm->dev, "failed to lookup GEM object\n"); > > - mutex_unlock(&drm->struct_mutex); > > return -EINVAL; > > } > > > > @@ -423,8 +420,6 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, > > struct drm_device *drm, > > > > drm_gem_object_unreference(gem); > > > > - mutex_unlock(&drm->struct_mutex); > > - > > return 0; > > } > > > > -- > > 2.1.4 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 18/18] drm/amdgpu: don't grab dev->struct_mutex in pm functions
On Thu, Jul 09, 2015 at 11:32:50PM +0200, Daniel Vetter wrote: > Similar to radeon, except that amdgpu doesn't even use struct_mutex to > protect anything like the shared z buffer (sane gpu architecture, > yay!). And the code already grabs the globa adev->ring_lock, so this > code can't race with itself. Which makes struct_mutex completely > redundnant. Remove it. > > Cc: Alex Deucher > Cc: "Christian König" > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 2 -- > 1 file changed, 2 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/cb253f64/attachment-0001.sig>
[PATCH 16/18] drm/radeon: Don't take dev->struct_mutex in pm functions
On Thu, Jul 09, 2015 at 11:32:48PM +0200, Daniel Vetter wrote: > We already grab 2 device-global locks (write-sema rdev->pm.mclk_lock > and rdev->ring_lock), adding another global mutex won't serialize this > code more. And since there's really nothing interesting that gets > protected in radeon by dev->struct mutex (we only have the global z > buffer owners and it's still serializing gem bo destruction in the drm > core - which is irrelevant since radeon uses ttm anyway internally) > this doesn't add protection. Remove it. > > Cc: Alex Deucher > Cc: "Christian König" > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/radeon/radeon_pm.c | 5 - > 1 file changed, 5 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/329a5ebe/attachment.sig>
[PATCH 15/18] drm/radeon: Don't take dev->struct_mutex in bo_force_delete
On Thu, Jul 09, 2015 at 11:32:47PM +0200, Daniel Vetter wrote: > It really doesn't protect anything which doesn't have other locks > already. Also this is run from driver unload code so not much need for > locks anyway. > > Cc: Alex Deucher > Cc: "Christian König" > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/radeon/radeon_object.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/39991da6/attachment.sig>
[PATCH 14/18] drm/qxl: Don't take dev->struct_mutex in bo_force_delete
On Thu, Jul 09, 2015 at 11:32:46PM +0200, Daniel Vetter wrote: > It really doesn't protect anything which doesn't have other locks > already. It also doesn't seem to be wired up into the driver unload > code fwiw, but that's a different issue. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/qxl/qxl_object.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/cae20c23/attachment.sig>
[PATCH RFC 5/5] drm/msm/hdmi: remove _clk suffix from clock names.
This patch modifies the driver to support clock names without _clk suffix, usage of clk names with _clk suffix seems to be non-standard and picked up everytime in DT patch review. So lets fix this and make the other clk names deprecated till we decide to remove them forever. Signed-off-by: Srinivas Kandagatla --- drivers/gpu/drm/msm/hdmi/hdmi.c | 29 - 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index e918889..c454d32 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -69,6 +69,24 @@ static void hdmi_destroy(struct hdmi *hdmi) platform_set_drvdata(hdmi->pdev, NULL); } +static struct clk *hdmi_clk_get(struct device *dev, const char *id) +{ + char clk_name[32]; + struct clk *clk; + + snprintf(clk_name, sizeof(clk_name), "%s_clk", id); + clk = devm_clk_get(dev, clk_name); + if (IS_ERR(clk)) { + clk = devm_clk_get(dev, id); + if (IS_ERR(clk)) + return ERR_CAST(clk); + } else { + dev_warn(dev, "binding deprecated for %s\n", clk_name); + } + + return clk; +} + /* construct hdmi at bind/probe time, grab all the resources. If * we are to EPROBE_DEFER we want to do it here, rather than later * at modeset_init() time @@ -158,7 +176,7 @@ static struct hdmi *hdmi_init(struct platform_device *pdev) for (i = 0; i < config->hpd_clk_cnt; i++) { struct clk *clk; - clk = devm_clk_get(&pdev->dev, config->hpd_clk_names[i]); + clk = hdmi_clk_get(&pdev->dev, config->hpd_clk_names[i]); if (IS_ERR(clk)) { ret = PTR_ERR(clk); dev_err(&pdev->dev, "failed to get hpd clk: %s (%d)\n", @@ -175,10 +193,11 @@ static struct hdmi *hdmi_init(struct platform_device *pdev) ret = -ENOMEM; goto fail; } + for (i = 0; i < config->pwr_clk_cnt; i++) { struct clk *clk; - clk = devm_clk_get(&pdev->dev, config->pwr_clk_names[i]); + clk = hdmi_clk_get(&pdev->dev, config->pwr_clk_names[i]); if (IS_ERR(clk)) { ret = PTR_ERR(clk); dev_err(&pdev->dev, "failed to get pwr clk: %s (%d)\n", @@ -296,7 +315,7 @@ static struct hdmi_platform_config hdmi_tx_8660_config = { }; static const char *hpd_reg_names_8960[] = {"core-vdda", "hdmi-mux"}; -static const char *hpd_clk_names_8960[] = {"core_clk", "master_iface_clk", "slave_iface_clk"}; +static const char *hpd_clk_names_8960[] = {"core", "master_iface", "slave_iface"}; static struct hdmi_platform_config hdmi_tx_8960_config = { .phy_init = hdmi_phy_8960_init, @@ -306,8 +325,8 @@ static struct hdmi_platform_config hdmi_tx_8960_config = { static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"}; static const char *hpd_reg_names_8x74[] = {"hpd-gdsc", "hpd-5v"}; -static const char *pwr_clk_names_8x74[] = {"extp_clk", "alt_iface_clk"}; -static const char *hpd_clk_names_8x74[] = {"iface_clk", "core_clk", "mdp_core_clk"}; +static const char *pwr_clk_names_8x74[] = {"extp", "alt_iface"}; +static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core"}; static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0}; static struct hdmi_platform_config hdmi_tx_8074_config = { -- 1.9.1
[PATCH RFC 4/5] drm/msm/hdmi: deprecate non standard clock-names
This patch updates the bindings to discourage the usage of non standard clock names, this will help in projects focused on upstreaming. These deprecated properties are still supported but will be remove over the time. Signed-off-by: Srinivas Kandagatla --- Documentation/devicetree/bindings/drm/msm/hdmi.txt | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt index 6dc202e..6fbfdd8 100644 --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt @@ -12,16 +12,16 @@ Required properties: - clocks: device clocks - clock-names: Corresponding name for each entry in the clocks property. for "qcom,hdmi-tx-8960" compatible names should be - "core_clk" - "master_iface_clk" - "slave_iface_clk" + "core_clk" is deprecated, use "core" instead + "master_iface_clk" is deprecated, use "master_iface" instead + "slave_iface_clk" is deprecated, use "slave_iface" instead for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should be - "extp_clk" - "alt_iface_clk" - "iface_clk" - "core_clk" - "mdp_core_clk" + "extp_clk" is deprecated, use "extp" instead + "alt_iface_clk" is deprecated, use "alt_iface" intstead + "iface_clk" is deprecated, use "iface" instead + "core_clk" is deprecated, use "core" instead + "mdp_core_clk" is deprecated, use "mdp_core" instead See ../clocks/clock-bindings.txt for details. - qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin -- 1.9.1
[PATCH 13/18] drm/nouveau: Don't take dev->struct_mutex in ttm_fini
On Thu, Jul 09, 2015 at 11:32:45PM +0200, Daniel Vetter wrote: > This is only called in driver load/unload paths, no need to grab any > locks at all. Also, ttm takes care of itself anyway. > > Cc: Ben Skeggs > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 -- > 1 file changed, 2 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/2b0aa918/attachment.sig>
[PATCH RFC 3/5] drm/msm/hdmi: update bindings to include clock-names.
This patch updates the bindings to include the clock names used in the driver, this would make it easy to add deprecated warning once we move to use standard clock properties. Signed-off-by: Srinivas Kandagatla --- Documentation/devicetree/bindings/drm/msm/hdmi.txt | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt index acba581..6dc202e 100644 --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt @@ -10,6 +10,19 @@ Required properties: - reg-names: "core_physical" - interrupts: The interrupt signal from the hdmi block. - clocks: device clocks +- clock-names: Corresponding name for each entry in the clocks property. + for "qcom,hdmi-tx-8960" compatible names should be + "core_clk" + "master_iface_clk" + "slave_iface_clk" + + for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should be + "extp_clk" + "alt_iface_clk" + "iface_clk" + "core_clk" + "mdp_core_clk" + See ../clocks/clock-bindings.txt for details. - qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin - qcom,hdmi-tx-ddc-data-gpios: ddc data pin -- 1.9.1
[PATCH RFC 2/5] drm/msm/hdmi: make use of standard gpio properties.
This patch modifies the driver to support standard gpio properties along with deprecated properties. This will help us upstream and cleanup the non-standard properties over the time. Signed-off-by: Srinivas Kandagatla --- drivers/gpu/drm/msm/hdmi/hdmi.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 8145362..e918889 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -339,19 +339,34 @@ static const struct of_device_id dt_match[] = { }; #ifdef CONFIG_OF +/* This code will be removed once we move to gpiod based calls */ static int get_gpio(struct device *dev, struct device_node *of_node, const char *name) { + char name2[32]; int gpio = of_get_named_gpio(of_node, name, 0); - if (gpio < 0) { - char name2[32]; - snprintf(name2, sizeof(name2), "%s-gpio", name); - gpio = of_get_named_gpio(of_node, name2, 0); - if (gpio < 0) { - dev_err(dev, "failed to get gpio: %s (%d)\n", - name, gpio); - gpio = -1; - } - } + + if (gpio_is_valid(gpio)) + goto deprecated; + + snprintf(name2, sizeof(name2), "%s-gpio", name); + gpio = of_get_named_gpio(of_node, name2, 0); + + if (gpio_is_valid(gpio)) + goto deprecated; + + + snprintf(name2, sizeof(name2), "%s-gpios", name); + gpio = of_get_named_gpio(of_node, name2, 0); + + if (gpio_is_valid(gpio)) + return gpio; + + dev_err(dev, "failed to get gpio: %s (%d)\n", name, gpio); + + return -1; + +deprecated: + dev_warn(dev, "binding deprecated for %s\n", name); return gpio; } #endif -- 1.9.1
[PATCH 12/18] drm/nouveau: Don't take dev->struct_mutex in fbcon init
On Thu, Jul 09, 2015 at 11:32:44PM +0200, Daniel Vetter wrote: > It doesn't protect anything at all. fbdev helper state is all > protected by modeset locks, and nouveau bo state is taken care of by > ttm. There's also nothing else grabbing struct_mutex that might need > to coordinate with this code. Also this is driver load code, no one > can get at the device yet anyway so locking is fairly futile. > There's also no drm_gem_object_unreference that would now suddenly > need the _unlocked variant. > > Cc: Ben Skeggs > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/nouveau/nouveau_fbcon.c | 11 +++ > 1 file changed, 3 insertions(+), 8 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/dc441815/attachment.sig>
[PATCH RFC 1/5] drm/msm/hdmi: deprecate non standard gpio properties.
This patch updates the bindings to discourage the usage of non standard gpio properites, this will help in projects focused on upstreaming. These deprecated properties are still supported but will be remove over the time. Signed-off-by: Srinivas Kandagatla --- Documentation/devicetree/bindings/drm/msm/hdmi.txt | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt index c43aa53..acba581 100644 --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt @@ -11,15 +11,27 @@ Required properties: - interrupts: The interrupt signal from the hdmi block. - clocks: device clocks See ../clocks/clock-bindings.txt for details. -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin -- qcom,hdmi-tx-hpd-gpio: hpd pin +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin +- qcom,hdmi-tx-hpd-gpios: hpd pin - core-vdda-supply: phandle to supply regulator - hdmi-mux-supply: phandle to mux regulator +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use + "qcom,hdmi-tx-ddc-clk-gpios" instead +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use + "qcom,hdmi-tx-ddc-data-gpios" instead +- qcom,hdmi-tx-hpd-gpio: (deprecated) use + "qcom,hdmi-tx-hpd-gpios" instead + Optional properties: -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin + +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" + instead +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" + instead - pinctrl-names: the pin control state names; should contain "default" - pinctrl-0: the default pinctrl state (active) - pinctrl-1: the "sleep" pinctrl state -- 1.9.1
[PATCH 11/18] drm/armada: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:43PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > While at it also fix a leak when this ioctl is called on an imported > buffer. > > Cc: Russell King > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/armada/armada_gem.c | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/aa993589/attachment.sig>
[PATCH RFC 0/5] drm/msm/hdmi: DT bindings cleanup
Hi Rob, Almost every time when we submit the DT changes for hdmi, we are asked why are we using non-standard dt properties. This patchset marks the old style dt properties as deprecated, and let the driver support latest *-gpios and clk names properties. This should allow us to submit the DT patches with more standard dt properites Old style properties are still supported and will be removed once we see no users for it. I have tested these changes on IFC6410. Thanks, srini Srinivas Kandagatla (5): drm/msm/hdmi: deprecate non standard gpio properties. drm/msm/hdmi: make use of standard gpio properties. drm/msm/hdmi: update bindings to include clock-names. drm/msm/hdmi: deprecate non standard clock-names drm/msm/hdmi: remove _clk suffix from clock names. Documentation/devicetree/bindings/drm/msm/hdmi.txt | 35 ++-- drivers/gpu/drm/msm/hdmi/hdmi.c| 64 +- 2 files changed, 79 insertions(+), 20 deletions(-) -- 1.9.1
[PATCH 10/18] drm/rockchip: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:42PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Aside: I stumbled over the mmap handler which directly does a > dma_mmap_attrs. But totally fails to grab a reference on the > underlying object and hence looks like it happily just leaks the ptes > since there's no guarantee the mmap isn't still around when > gem_free_object is called. Which the kerneldoc of dma_mmap_attrs > explicitly forbids. Same is true for Exynos, which seems to be the source for copy/paste here. Anyway, for this change: Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/63d340dd/attachment-0001.sig>
[PATCH 01/12] drm/amdgpu: add amd_gnb_bus support
On Fri, Aug 07, 2015 at 04:03:08PM -0400, Felix Kuehling wrote: > 1. GPU driver gets initialized, detects a GPU with audio co-processor (ACP) > 2. GPU driver registers mfd_cell for the ACP device using > mfd_add_hotplug_devices > * It's not really hot-plug, but the mem_base, irq_base, irq_domain > parameters don't make sense for us All those should be optional... > 3. Platform_data in the MFD cell contains audio driver-specific data, > function pointers, etc. for the audio driver to use Note that a MFD knows that its parent is the core device so it can just look at the driver data of the parent unless things vary per child. > 4. Audio driver binds to platform device created by > mfd_add_hotplug_devices based on driver name > Or do we have to convert our GPU device to be an MFD cell itself, a peer > of the ACP cell? If they're all part of the same block of hardware that'd be more normal, but it all depends on what the code looks like and what the relevant maintainers think. -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: Digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/ccb8ab36/attachment-0001.sig>
[PATCH] drm/panel: auo novatek 1080p video mode panel
On Fri 07 Aug 09:11 PDT 2015, Rob Clark wrote: > On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding > wrote: > > On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote: [..] > >> +- compatible: should be "auo,novatek-1080p-vid" > > > > This looks a little generic for a compatible string. Can't we get at the > > specific panel model number that's been used? What if AUO ever produced > > some other Novatek panel with a 1080p resolution? > > Maybe Sony or someone else can chime in? That somewhat generic name > was all I could get from downstream android kernel. I'm sure there is > a better possible name, although I have no means to find that out > myself. > We're working on it. > > Also, what's the -vid suffix for? > > the same panel seems to also work in cmd mode.. so idea was to have > -vid and -cmd compat strings to choose which mode to operate in. > An alternative would be to make it a bool property, to indicate video mode - following how the framework is implemented. [..] > >> diff --git a/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c > >> b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c > >> +static int auo_panel_init(struct auo_panel *auo) > >> +{ > >> + struct mipi_dsi_device *dsi = auo->dsi; > >> + int ret; > >> + > >> + dsi->mode_flags |= MIPI_DSI_MODE_LPM; > >> + > >> + ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xb0, 0x04 }, 2); > > > > I find this notation hard to read. Have you considered moving this into > > some sort of table that you can loop through? Or perhaps add some > > helpers, say, mipi_dsi_generic_writeb() and mipi_dsi_dcs_writeb() to > > help make this more readable? > > > > Yeah, helper macro thing might be a reasonable idea. The table option > makes it hard to use the helpers for things that are not non-standard, > or when you need delays, etc.. > I agree with you here, we don't want lists of data that the driver has to interpret into writes (of various types) and delays. > > >> + if (ret < 0) > >> + return ret; > >> + Regards, Bjorn
[PATCH 09/18] drm/cma-helper: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:41PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Cc: Laurent Pinchart > Cc: Rob Clark > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_gem_cma_helper.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/16d2099b/attachment.sig>
[PATCH 08/18] drm/cirrus: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:40PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/cirrus/cirrus_main.c | 15 --- > 1 file changed, 4 insertions(+), 11 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/fcf7c466/attachment.sig>
[PATCH 00/18] dev->struct_mutex crusade
On Mon, Aug 10, 2015 at 01:35:58PM +0200, Thierry Reding wrote: > On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote: > > Hi all, > > > > I wanted to take another look at struct_mutex usage in modern (gem) drivers > > and > > noticed that for a fair lot we're very to be completely struct_mutex free. > > > > This pile here is the the simple part, which mostly just removes code and > > mutex_lock/unlock calls. All the patches here are independent and can be > > merged > > in any order whatsoever. My plan is to send out a pull request for all > > those not > > picked up by driver maintainers in 2-3 weeks or so, assuming no one > > complains. > > > > Of course review & comments still very much welcome. > > > > The more tricky 2nd part of this (and that one's not yet done) is to rework > > the > > gem mmap handler to use the same kref_get_unless_zero trick as ttm. With > > that > > there's no core requirement to hold struct_mutex over the final unref, which > > means we can make that one lockless. I plan to add a > > gem_object_free_unlocked > > for all the drivers which don't have any need for this lock. > > > > Also there's a few more drivers which can be made struct_mutex free easily, > > I'll > > propably stitch together poc patches for those. > > There's a concurrency bug in Tegra DRM currently because we don't lock > accesses to drm_mm (I guess this demonstrates how badly we need better > testing...) and it seems like this is typically protected by the very > same struct_mutex that you're on a crusade to remove. If your goal is > to get rid of it for good, should we simply add a separate lock just > for the drm_mm? We don't have another one that would fit. Actually that is one of the first targets for more fine-grained locking. I would not add a new lock to drm_mm as at least for i915, we want to use a similar per-vm lock (of which the drm_mm is just one part). -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH 17/18] drm/amdgpu: Don't take dev->struct_mutex in bo_force_delete
On Thu, Jul 09, 2015 at 11:32:49PM +0200, Daniel Vetter wrote: > It really doesn't protect anything which doesn't have other locks > already. Also this is run from driver unload code so not much need for > locks anyway. > > Same changes as for readone really. s/radeone/radeon/ Otherwise: Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/ba02b5e1/attachment.sig>
[PATCH 07/18] drm/mga200g: Hold a proper reference for cursor_set
On Thu, Jul 09, 2015 at 11:32:39PM +0200, Daniel Vetter wrote: > Looking up an obj, immediate dropping the acquired reference and then > continuing to use it isn't how this is supposed to work. Fix this by > holding a reference for the entire function. > > While at it stop grabbing dev->struct_mutex, it doesn't protect > anything here. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/mgag200/mgag200_cursor.c | 22 ++ > 1 file changed, 10 insertions(+), 12 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/5e07bfd0/attachment.sig>
[PATCH 06/18] drm/mga200g: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:38PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/mgag200/mgag200_main.c | 16 > 1 file changed, 4 insertions(+), 12 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/db571af9/attachment.sig>
[PATCH 04/18] drm/ast: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:36PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/ast/ast_main.c | 16 +--- > 1 file changed, 5 insertions(+), 11 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/611c239a/attachment.sig>
[PATCH 05/18] drm/bochs: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:37PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/bochs/bochs_mm.c | 16 > 1 file changed, 4 insertions(+), 12 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/85732c7e/attachment-0001.sig>
[PATCH 03/18] drm/gem: Be more friendly with locking checks
On Thu, Jul 09, 2015 at 11:32:35PM +0200, Daniel Vetter wrote: > BUG_ON kills the driver, WARN_ON is much friendly. And usually nothing > bad happens when the locking is lightly busted. s/much friendly/much friendlier/, s/lightly/slightly/? Otherwise: Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/1cc29dea/attachment.sig>
[PATCH 02/18] drm/cma-helper: Fix locking in drm_fb_cma_debugfs_show
On Thu, Jul 09, 2015 at 11:32:34PM +0200, Daniel Vetter wrote: > This function takes two locks, both of them the wrong ones. This > wasn't an oversight from my fb locking rework since both patches > landed in parallel. We really only need fb_lock when walking that > list, since everything we can reach from that is refcounted properly > already. > > Cc: Rob Clark > Cc: Laurent Pinchart > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_fb_cma_helper.c | 16 ++-- > drivers/gpu/drm/drm_gem_cma_helper.c | 2 -- > 2 files changed, 2 insertions(+), 16 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/98267768/attachment.sig>
[PATCH 1/2] drm/tegra: don't take dev->struct_mutex in mmap offset ioctl
On Wed, Jul 15, 2015 at 03:38:51PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > While at it also fix a leak when this ioctl is called on an imported > buffer. I don't see where the leak's fixed, but other than that this looks good to me. Shall I pick this up into the drm/tegra tree? Thierry > Cc: Thierry Reding > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/tegra/gem.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c > index 01e16e146bfe..827838e64d6e 100644 > --- a/drivers/gpu/drm/tegra/gem.c > +++ b/drivers/gpu/drm/tegra/gem.c > @@ -408,12 +408,9 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, > struct drm_device *drm, > struct drm_gem_object *gem; > struct tegra_bo *bo; > > - mutex_lock(&drm->struct_mutex); > - > gem = drm_gem_object_lookup(drm, file, handle); > if (!gem) { > dev_err(drm->dev, "failed to lookup GEM object\n"); > - mutex_unlock(&drm->struct_mutex); > return -EINVAL; > } > > @@ -423,8 +420,6 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, > struct drm_device *drm, > > drm_gem_object_unreference(gem); > > - mutex_unlock(&drm->struct_mutex); > - > return 0; > } > > -- > 2.1.4 > -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/9942a0e3/attachment.sig>
[PATCH 11/18] drm/armada: Don't grab dev->struct_mutex for in mmap offset ioctl
On Thu, Jul 09, 2015 at 11:32:43PM +0200, Daniel Vetter wrote: > Since David Herrmann's mmap vma manager rework we don't need to grab > dev->struct_mutex any more to prevent races when looking up the mmap > offset. Drop it and instead don't forget to use the unref_unlocked > variant (since the drm core still cares). > > While at it also fix a leak when this ioctl is called on an imported > buffer. Good catch, thanks Daniel. I'd prefer these to be two different commits though, so that the leak can be easily backported to stable kernels if needed. I suspect this should go through the same tree that David's work is, otherwise we end up with random dependencies between trees. With the bug fix separated out: Acked-by: Russell King -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver
On Sat, 08 Aug 2015 18:10:06 +0200, Russell King wrote: > > Add ALSA based HDMI AHB audio driver for dw_hdmi. The only buffer > format supported by the hardware is its own special IEC958 based format, > which is not compatible with any ALSA format. To avoid doing too much > data manipulation within the driver, we support only ALSAs IEC958 LE and > 24-bit PCM formats for 2 to 6 channels, which we convert to its hardware > format. > > A more desirable solution would be to have this conversion in userspace, > but ALSA does not appear to allow such transformations outside of > libasound itself. > > Signed-off-by: Russell King > --- > drivers/gpu/drm/bridge/Kconfig | 10 + > drivers/gpu/drm/bridge/Makefile| 1 + > drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c | 561 > + > drivers/gpu/drm/bridge/dw_hdmi-audio.h | 13 + > drivers/gpu/drm/bridge/dw_hdmi.c | 24 ++ > drivers/gpu/drm/bridge/dw_hdmi.h | 3 + > 6 files changed, 612 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c > create mode 100644 drivers/gpu/drm/bridge/dw_hdmi-audio.h > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index acef3223772c..56ed35fe0734 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -3,6 +3,16 @@ config DRM_DW_HDMI > depends on DRM > select DRM_KMS_HELPER > > +config DRM_DW_HDMI_AHB_AUDIO > + tristate "Synopsis Designware AHB Audio interface" > + depends on DRM_DW_HDMI && SND > + select SND_PCM > + select SND_PCM_IEC958 > + help > + Support the AHB Audio interface which is part of the Synopsis > + Designware HDMI block. This is used in conjunction with > + the i.MX6 HDMI driver. > + > config DRM_PTN3460 > tristate "PTN3460 DP/LVDS bridge" > depends on DRM > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 8dfebd984370..eb80dbbb8365 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -3,3 +3,4 @@ ccflags-y := -Iinclude/drm > obj-$(CONFIG_DRM_PS8622) += ps8622.o > obj-$(CONFIG_DRM_PTN3460) += ptn3460.o > obj-$(CONFIG_DRM_DW_HDMI) += dw_hdmi.o > +obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw_hdmi-ahb-audio.o > diff --git a/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c > b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c > new file mode 100644 > index ..22bbbc5c2393 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/dw_hdmi-ahb-audio.c > @@ -0,0 +1,561 @@ > +/* > + * DesignWare HDMI audio driver > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Written and tested against the Designware HDMI Tx found in iMX6. > + */ > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +#include "dw_hdmi-audio.h" > + > +#define DRIVER_NAME "dw-hdmi-ahb-audio" > + > +/* Provide some bits rather than bit offsets */ > +enum { > + HDMI_AHB_DMA_CONF0_SW_FIFO_RST = BIT(7), > + HDMI_AHB_DMA_CONF0_EN_HLOCK = BIT(3), > + HDMI_AHB_DMA_START_START = BIT(0), > + HDMI_AHB_DMA_STOP_STOP = BIT(0), > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_ERROR = BIT(5), > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_LOST = BIT(4), > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_RETRY = BIT(3), > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_DONE = BIT(2), > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFFULL = BIT(1), > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFEMPTY = BIT(0), > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_ALL = > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_ERROR | > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_LOST | > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_RETRY | > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_DONE | > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFFULL | > + HDMI_IH_MUTE_AHBDMAAUD_STAT0_BUFFEMPTY, > + HDMI_IH_AHBDMAAUD_STAT0_ERROR = BIT(5), > + HDMI_IH_AHBDMAAUD_STAT0_LOST = BIT(4), > + HDMI_IH_AHBDMAAUD_STAT0_RETRY = BIT(3), > + HDMI_IH_AHBDMAAUD_STAT0_DONE = BIT(2), > + HDMI_IH_AHBDMAAUD_STAT0_BUFFFULL = BIT(1), > + HDMI_IH_AHBDMAAUD_STAT0_BUFFEMPTY = BIT(0), > + HDMI_IH_AHBDMAAUD_STAT0_ALL = > + HDMI_IH_AHBDMAAUD_STAT0_ERROR | > + HDMI_IH_AHBDMAAUD_STAT0_LOST | > + HDMI_IH_AHBDMAAUD_STAT0_RETRY | > + HDMI_IH_AHBDMAAUD_STAT0_DONE | > + HDMI_IH_AHBDMAAUD_STAT0_BUFFFULL | > + HDMI_IH_AHBDMAAUD_STAT0_BUFFEMPTY, > + HDMI_AHB_DMA_CONF0_INCR16 = 2 << 1, > + HDMI_AHB_DMA_CONF0_INCR8 = 1 << 1, > + HDMI_AHB_DMA_CONF0_INCR4 = 0, > + HDMI_AHB_DMA_CONF0_BURST_MODE = BIT(0), > + HDMI_AHB_DMA_MASK_DONE = BIT(7), > + HDMI_REVISION_ID = 0x0001, > + HDMI_IH_AHBDMAAUD_STAT0 = 0x0109, > + H
[PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote: [...] > edp: edp at ff97 { [...] > hsync-active-high = <0>; > vsync-active-high = <0>; > interlaced = <0>; These look like they should come from the display mode definition (EDID) rather than device tree. > samsung,color-space = <0>; > samsung,dynamic-range = <0>; > samsung,ycbcr-coeff = <0>; I think these should also come from EDID, though I'm not sure if we store this in internal data structures yet. > samsung,color-depth = <1>; This is probably drm_display_info.bpc. > samsung,link-rate = <0x0a>; > samsung,lane-count = <1>; And these should really be derived from values in the DPCD and adjusted (if necessary) during link training. Why would you ever want to hard-code the above? > >>+ dp->clk_24m = devm_clk_get(dev, "clk_dp_24m"); > >Same here, maybe "dp_24m". > Like my previous reply. And actually as those two clocks all have > a common prefix "SCLK" in rk3288 clock tree, I thinkt we can name > them to "sclk_dp" & "sclk_dp_24m", is it okay ? I don't think there's a need for these common prefixes. The names here are identifiers in the context of the IP block, so any SoC-specific prefixes are irrelevant. Also they do appear, in DT and in code, in the context of clocks already, so "sclk_" or "clk_" is completely redundant in these names. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150810/80d87f4c/attachment.sig>
[PATCH 2/2] drm/doc: Update docs about device instance setup
->load is depracated, bus functionst are deprecated and everyone should use drm_dev_alloc®ister. So update the .tmpl (and pull a bunch of the overview docs into the sourcecode to increase chances that it'll stay in sync in the future) and add notes to functions which are deprecated. I didn't bother to clean up and document the unload sequence similarly since that one is still a bit a mess: drm_dev_unregister does way too much, drm_unplug_dev does what _unregister should be doing but then has the complication of promising something it doesn't actually do (it doesn't unplug existing open fds for instance, only prevents new ones). Motivated since I don't want to hunt every new driver for usage of drm_platform_init any more ;-) Signed-off-by: Daniel Vetter --- Documentation/DocBook/drm.tmpl | 99 -- drivers/gpu/drm/drm_drv.c | 55 +-- drivers/gpu/drm/drm_pci.c | 11 + drivers/gpu/drm/drm_platform.c | 3 ++ 4 files changed, 83 insertions(+), 85 deletions(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index ec34b9becebd..f1884038b90f 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -138,14 +138,10 @@ At the core of every DRM driver is a drm_driver structure. Drivers typically statically initialize a drm_driver structure, - and then pass it to one of the drm_*_init() functions - to register it with the DRM subsystem. - - - Newer drivers that no longer require a drm_bus - structure can alternatively use the low-level device initialization and - registration functions such as drm_dev_alloc() and - drm_dev_register() directly. + and then pass it to drm_dev_alloc() to allocate a + device instance. After the device instance is fully initialized it can be + registered (which makes it accessible from userspace) using + drm_dev_register(). The drm_driver structure contains static @@ -296,83 +292,12 @@ char *date; - Device Registration - -A number of functions are provided to help with device registration. -The functions deal with PCI and platform devices, respectively. - -!Edrivers/gpu/drm/drm_pci.c -!Edrivers/gpu/drm/drm_platform.c - -New drivers that no longer rely on the services provided by the -drm_bus structure can call the low-level -device registration functions directly. The -drm_dev_alloc() function can be used to allocate -and initialize a new drm_device structure. -Drivers will typically want to perform some additional setup on this -structure, such as allocating driver-specific data and storing a -pointer to it in the DRM device's dev_private -field. Drivers should also set the device's unique name using the -drm_dev_set_unique() function. After it has been -set up a device can be registered with the DRM subsystem by calling -drm_dev_register(). This will cause the device to -be exposed to userspace and will call the driver's -.load() implementation. When a device is -removed, the DRM device can safely be unregistered and freed by calling -drm_dev_unregister() followed by a call to -drm_dev_unref(). - + Device Instance and Driver Handling +!Pdrivers/gpu/drm/drm_drv.c driver instance overview !Edrivers/gpu/drm/drm_drv.c Driver Load - -The load method is the driver and device -initialization entry point. The method is responsible for allocating and - initializing driver private data, performing resource allocation and - mapping (e.g. acquiring -clocks, mapping registers or allocating command buffers), initializing -the memory manager (), installing -the IRQ handler (), setting up -vertical blanking handling (), mode - setting () and initial output - configuration (). - - -If compatibility is a concern (e.g. with drivers converted over from -User Mode Setting to Kernel Mode Setting), care must be taken to prevent -device initialization and control that is incompatible with currently -active userspace drivers. For instance, if user level mode setting -drivers are in use, it would be problematic to perform output discovery -& configuration at load time. Likewise, if user-level drivers -unaware of memory management are in use, memory management and command -buffer setup may need to be omitted. These requirements are -driver-specific, and care needs to be taken to keep both old and new -applications and libraries working. - - int (*load) (struct drm_device *, unsigned long flags); - -The method takes two arguments, a pointer to the newly created -