Re: drm_hwcomposer moving to fd.o
2017-09-26 13:26 GMT+08:00 Daniel Vetter : > On Mon, Sep 25, 2017 at 09:06:55AM +0200, Tomeu Vizoso wrote: >> On 09/25/2017 04:44 AM, Chih-Wei Huang wrote: >> > Hey Robert, thank you for the reply. >> > >> > 2017-09-22 23:43 GMT+08:00 Robert Foss : >> >> Hey Chih-Wei, >> >> On Fri, 2017-09-22 at 10:40 +0800, Chih-Wei Huang wrote: >> > >> >> The android-ia project has supported using drm_hwcomposer and an >> >> alternative hwcomposer, so it would think there should be no issues. >> > >> > By x86 GPUs, I meant i915(i965)/nouveau/radeon/amdgpu/virgl/vmwgfx. >> > Among these only virgl works with the current drm_hwcomposer. >> > All the others don't have a ready atomic mode-setting API to use it. >> > That's the problem which should be addressed I meant. >> >> Most if not all of those drivers support the atomic updates API in >> mainline (and have for a few releases by now). > > amdgpu does not (but will once DC has landed, at least when you enable > DC). Radeon doesn't, and likely never will. Nouveau is atomic only for > nv50+. Thank you for the clarification. I have made test with kernel 4.13 + gbm_grallc + drm_hwcomposer + mesa 17.1 on Android-x86 7.1. Except virgl all others just failed as expected. i915 (i965) was tested in Intel Broxton. It's the best result. SurfaceFlinger runs and bootanimation is shown. However, after boot complete it only shown systemui + navbar with black background. No app icons or mouse cursor in the desktop. I guess the composition of 3+ layers has some problems. nouveau is tested on GTX 1060. drm_hwcomposer init failed so SurfaceFlinger crashed. Seems the driver is not atomic: 09-28 13:21:39.719 3096 3096 E hwc-drm-resources: Failed to set atomic cap -1 which comes from this line of drmresources.cpp: ret = drmSetClientCap(fd(), DRM_CLIENT_CAP_ATOMIC, 1); vmwgfx also has issues on drm_hwcomposer initialization. SurfaceFlinger didn't crashes but no bootanimation. It seems just blocked. 09-28 04:46:09.043 3389 3389 E hwcomposer-drm: Failed to set active config d=1 ret=-19 09-28 04:46:09.043 3389 3389 E hwcomposer-drm: Failed to set initial config for d=1 ret=-19 09-28 04:46:09.043 3389 3389 E hwcomposer-drm: Failed to initialize display 1 09-28 04:46:09.043 3389 3389 E hwcomposer-drm: Failed to enumerate displays: Unknown error -19 Full log: https://drive.google.com/open?id=0B3GICgSwrKXcci1lbDdaSlJBRE0 Shall I create bugs in the Bugzilla? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/amd/display: DC I2C review
On Wed, Sep 27, 2017 at 9:46 PM, Harry Wentland wrote: > While reviewing I2C in DC identified a few places. Added a couple to the > TODO list. > > 1) Connector info read > > See get_ext_display_connection_info > > On some boards the connector information has to be read through a > special I2C channel. This line is only used for this purpose and only on > driver init. > > 2) SCDC stuff > > This should all be reworked to go through DRM's SCDC code. When this is > done some unnecessary I2C code can be retired as well. > > 3) Max TMDS clock read > > See dal_ddc_service_i2c_query_dp_dual_mode_adaptor > > This should happen in DRM as well. I haven't checked if there's > currently functionality in DRM. If not we can propose something. > > 4) HDMI retimer programming > > Some boards have an HDMI retimer that we need to program to pass PHY > compliance. > > 1 & 3 might be a good exercise if someone is looking for things to do. > > Signed-off-by: Harry Wentland > --- > drivers/gpu/drm/amd/display/TODO | 26 -- > 1 file changed, 12 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/TODO > b/drivers/gpu/drm/amd/display/TODO > index eea645b102a1..981352bc95f0 100644 > --- a/drivers/gpu/drm/amd/display/TODO > +++ b/drivers/gpu/drm/amd/display/TODO > @@ -62,20 +62,10 @@ TODOs > ~ Daniel Vetter > > > -11. Remove existing i2c implementation from DC > - > -"Similar story for i2c, it uses the kernel's i2c code now, but there's > -still a full i2c implementation hidden beneath that in > -display/dc/i2caux. Kinda not cool, but imo ok if you fix that > -post-merging (perhaps by not including any of this in the linux DC > -code in the upstream kernel, but as an aux module in your internal > -codebase since there you probably need that, same applies to the edid > -parsing DC still does. For both cases I assume that the minimal shim > -you need on linux (bit banging and edid parsing isn't rocket since) is > -a lot less than the glue code to interface with the dc-provided > -abstraction." > -~ Daniel Vetter > - > +11. Remove dc/i2caux. This folder can be somewhat misleading. It's basically > an > +overy complicated HW programming function for sendind and receiving i2c/aux > +commands. We can greatly simplify that and move it into dc/dceXYZ like other > +HW blocks. Best case I think would be if you directly implement the i2c_adapter in there. It's a tiny abstraction/api, so should be trivial to reimplement for the windows side. Or at least align really closely. Even more so for the gpio bit-banging case, that should use the linux implementation I think. Might be good to clarify. Anyway, ack on this. > 12. drm_modeset_lock in MST should no longer be needed in recent kernels > * Adopt appropriate locking scheme > @@ -110,3 +100,11 @@ guilty. > stuff just isn't up to the challenges either. We need to figure out something > that integrates better with DRM and linux debug printing, while not being > useless with filtering output. dynamic debug printing might be an option. > + > +20. Move Max TMDS clock read to DRM. See > +dal_ddc_service_i2c_query_dp_dual_mode_adaptor. I haven't checked if there's > +currently functionality in DRM. If not we can propose something. We already have dual_mode helpers. It's one of the todo's I've added, merged this with point 15? > +21. Use kernel i2c device to program HDMI retimer. Some boards have an HDMI > +retimer that we need to program to pass PHY compliance. Currently that's > +bypassing the i2c device and goes directly to HW. This should be changed. I thought it eventually goes through the i2c stuff, after a few layers at least. Maybe I got derailed. Anyway, makes sense. With 20 merged into 15, ack on the patch from me. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd: DC pull request review
On Wed, Sep 27, 2017 at 9:25 PM, Harry Wentland wrote: > On 2017-09-27 02:12 PM, Harry Wentland wrote: >> On 2017-09-27 12:48 PM, Daniel Vetter wrote: >>> On Wed, Sep 27, 2017 at 6:38 PM, Sean Paul wrote: Any chance we can address the i2c/gpio [re-]implementations as well? >>> >>> It's already on the list. Part of this is code that's probably dead, >>> the other is a bit too much layer cake still left, and the final bits >>> are not fully using drm helpers for edid parsing and tons of other >>> stuff. But afaics all the i2c stuff does go through the i2c_adapter >>> abstraction in a proper fashion, which is at least the foundational >>> work. The other bits should be all captured in the todo already. >> >> There might still be some dead code around i2c stuff. I'll take a look >> and see what I can rip out. >> > > Looks like there currently are no easy pickings. What we have is: > > 1) Connector info read > > See get_ext_display_connection_info > > On some boards the connector information has to be read through a > special I2C channel. This line is only used for this purpose and only on > driver init. > > > 2) SCDC stuff > > This should all be reworked to go through DRM's SCDC code. When this is > done some unnecessary I2C code can be retired as well. > > > 3) Max TMDS clock read > > See dal_ddc_service_i2c_query_dp_dual_mode_adaptor > > This should happen in DRM as well. I haven't checked if there's > currently functionality in DRM. If not we can propose something. > > > 4) HDMI retimer programming > > Some boards have an HDMI retimer that we need to program to pass PHY > compliance. I spotted this too, but I'm not aware of any other driver/hw using this. If it's some standard we might indeed want to move it into the helpers, but not as a priority. That's why I didn't add it as a todo item. All the other bits should be in the TODO already (with my latest patch at least). You might want to extend/clarify those entries though. > It would take a bit of time to address all of them. I'll add them on the > TODO. > > 1 & 3 might be a good exercise if someone is looking for things to do. Yeah, some of this stuff looks like good gsoc/evoc/outreachy stuff. -Daniel > > Harry > > >> If there's any specific function, struct, etc. that's of concern let me >> know as well. >> >> Harry >> >>> -Daniel >>> >> ___ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/omap: Fix error handling path in 'omap_dmm_probe()'
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 24/09/17 09:01, Christophe JAILLET wrote: > If we don't find a matching device node, we must free the memory allocated > in 'omap_dmm' a few lines above. > > Fixes: 7cb0d6c17b96 ("drm/omap: fix TILER on OMAP5") > Signed-off-by: Christophe JAILLET > --- > drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c > b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c > index 1dd3dafc59af..c60a85e82c6d 100644 > --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c > +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c > @@ -638,7 +638,8 @@ static int omap_dmm_probe(struct platform_device *dev) > match = of_match_node(dmm_of_match, dev->dev.of_node); > if (!match) { > dev_err(&dev->dev, "failed to find matching device > node\n"); > - return -ENODEV; > + ret = -ENODEV; > + goto fail; > } > > omap_dmm->plat_data = match->data; > Thanks! I have picked this up. Tomi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] drm/tinydrm: Add devres versions of drm_of_find_backlight
Add devm_drm_of_find_backlight and the corresponding release function because some drivers such as tinydrm use devres versions of functions for requiring device resources. Signed-off-by: Meghana Madhyastha --- Changes in v2: -This was not present in the initial version. drivers/gpu/drm/drm_of.c | 47 +++ include/drm/drm_of.h | 2 ++ 2 files changed, 49 insertions(+) diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index d878d3a..26090e1 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -304,3 +304,50 @@ struct backlight_device *drm_of_find_backlight(struct device *dev) return backlight; } EXPORT_SYMBOL(drm_of_find_backlight); + +/** + * devm_drm_of_find_backlight - Find backlight device in device-tree + * devres version of the function + * @dev: Device + * + * This is the devres version of the function drm_of_find_backlight. + * Some drivers such as tinydrm use devres versions of functions for + * requiring device resources. + * + * Returns: + * NULL if there's no backlight property. + * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device + * is found. + * If the backlight device is found, a pointer to the structure is returned. + */ +struct backlight_device *devm_drm_of_find_backlight(struct device *dev) +{ + struct backlight_device *backlight; + int ret; + + backlight = drm_of_find_backlight(dev); + if (IS_ERR_OR_NULL(backlight)) + return backlight; + + ret = devm_add_action(dev, devm_drm_of_find_backlight_release, + backlight->dev); + if (ret) { + put_device(backlight->dev); + return ERR_PTR(ret); + } + + return backlight; +} +EXPORT_SYMBOL(devm_drm_of_find_backlight); + +/** + * devm_drm_of_find_backlight_release - Release backlight device + * + * This is the release function corresponding to the devm_drm_of_find_backlight. + * Each devres entry is associated with a release function. + */ +static void devm_drm_of_find_backlight_release(void *data) +{ + put_device(data); +} +EXPORT_SYMBOL(devm_drm_of_find_backlight_release) diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h index e8fba5b..89a37da 100644 --- a/include/drm/drm_of.h +++ b/include/drm/drm_of.h @@ -30,6 +30,8 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, struct drm_panel **panel, struct drm_bridge **bridge); struct backlight_device *drm_of_find_backlight(struct device *dev); +struct backlight_device *devm_drm_of_find_backlight(struct device *dev) +static void devm_drm_of_find_backlight_release(void *data) #else static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev, struct device_node *port) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] drm/tinydrm: Call drm_of_find_backlight instead of tinydrm_of_find_backlight
Call drm_of_find_backlight from of_drm.c as the function has been renamed and moved. Signed-off-by: Meghana Madhyastha --- Changes in v2: -Introduce this as a separate patch in a patchset instead of combining it with the previous changes. drivers/gpu/drm/tinydrm/mi0283qt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index 7e5bb7d..5e3d635 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -189,7 +190,7 @@ static int mi0283qt_probe(struct spi_device *spi) if (IS_ERR(mipi->regulator)) return PTR_ERR(mipi->regulator); - mipi->backlight = tinydrm_of_find_backlight(dev); + mipi->backlight = drm_of_find_backlight(dev); if (IS_ERR(mipi->backlight)) return PTR_ERR(mipi->backlight); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c
Rename tinydrm_of_find_backlight to drm_of_find_backlight and move it into drm_of.c from tinydrm-helpers.c. This is because other drivers in the drm subsystem might need to call this function. In that case and otherwise, it is better from an organizational point of view to move it into drm_of.c along with the other _of.c functions. Signed-off-by: Meghana Madhyastha --- Changes in v2: -Added note about it being the callers responsibility to call put_device. drivers/gpu/drm/drm_of.c | 44 ++ drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 --- include/drm/drm_of.h | 1 + include/drm/tinydrm/tinydrm-helpers.h | 1 - 4 files changed, 45 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index 8dafbdf..d878d3a 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -260,3 +261,46 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, return ret; } EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge); + +/** + * drm_of_find_backlight - Find backlight device in device-tree + * @dev: Device + * + * This function looks for a DT node pointed to by a property named 'backlight' + * and uses of_find_backlight_by_node() to get the backlight device. + * Additionally if the brightness property is zero, it is set to + * max_brightness. + * + * Note: It is the responsibility of the caller to call put_device() when + * releasing the resource. + * + * Returns: + * NULL if there's no backlight property. + * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device + * is found. + * If the backlight device is found, a pointer to the structure is returned. + */ +struct backlight_device *drm_of_find_backlight(struct device *dev) +{ + struct backlight_device *backlight; + struct device_node *np; + + np = of_parse_phandle(dev->of_node, "backlight", 0); + if (!np) + return NULL; + + backlight = of_find_backlight_by_node(np); + of_node_put(np); + + if (!backlight) + return ERR_PTR(-EPROBE_DEFER); + + if (!backlight->props.brightness) { + backlight->props.brightness = backlight->props.max_brightness; + DRM_DEBUG_KMS("Backlight brightness set to %d\n", + backlight->props.brightness); + } + + return backlight; +} +EXPORT_SYMBOL(drm_of_find_backlight); diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index bd6cce0..cd4c6a5 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -237,46 +237,6 @@ void tinydrm_xrgb_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, EXPORT_SYMBOL(tinydrm_xrgb_to_gray8); /** - * tinydrm_of_find_backlight - Find backlight device in device-tree - * @dev: Device - * - * This function looks for a DT node pointed to by a property named 'backlight' - * and uses of_find_backlight_by_node() to get the backlight device. - * Additionally if the brightness property is zero, it is set to - * max_brightness. - * - * Returns: - * NULL if there's no backlight property. - * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device - * is found. - * If the backlight device is found, a pointer to the structure is returned. - */ -struct backlight_device *tinydrm_of_find_backlight(struct device *dev) -{ - struct backlight_device *backlight; - struct device_node *np; - - np = of_parse_phandle(dev->of_node, "backlight", 0); - if (!np) - return NULL; - - backlight = of_find_backlight_by_node(np); - of_node_put(np); - - if (!backlight) - return ERR_PTR(-EPROBE_DEFER); - - if (!backlight->props.brightness) { - backlight->props.brightness = backlight->props.max_brightness; - DRM_DEBUG_KMS("Backlight brightness set to %d\n", - backlight->props.brightness); - } - - return backlight; -} -EXPORT_SYMBOL(tinydrm_of_find_backlight); - -/** * tinydrm_enable_backlight - Enable backlight helper * @backlight: Backlight device * diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h index 104dd51..e8fba5b 100644 --- a/include/drm/drm_of.h +++ b/include/drm/drm_of.h @@ -29,6 +29,7 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, int port, int endpoint, struct drm_panel **panel, struct drm_bridge **bridge); +struct backlight_device *drm_of_find_backlight(struct device *dev); #else static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
[PATCH v2 0/3] drm/tinydrm: drm_of_find_backlight helper
This patchset introduces some changes such as move tinydrm_of_find_backlight to drm_of.c and rename it to drm_of_find_backlight for better organizational structure. Changes in v2: -Broke the patch into a patchset of 3 patches -Added devres versions of drm_of_find_backlight -Added a note about caller having to call put_device Meghana Madhyastha (3): drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c drm/tinydrm: Call drm_of_find_backlight instead of tinydrm_of_find_backlight drm/tinydrm: Add devres versions of drm_of_find_backlight drivers/gpu/drm/drm_of.c | 44 ++ drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 --- drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +- include/drm/drm_of.h | 3 ++ include/drm/tinydrm/tinydrm-helpers.h | 1 - 5 files changed, 49 insertions(+), 42 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm:drm-next 3/3] drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:2: error: invalid initializer
tree: git://people.freedesktop.org/~airlied/linux.git drm-next head: 754270c7c56292e97d0eff924a5d5d83f92add07 commit: 754270c7c56292e97d0eff924a5d5d83f92add07 [3/3] Merge branch 'drm-next-4.15' of git://people.freedesktop.org/~agd5f/linux into drm-next config: x86_64-randconfig-it0-09281201 (attached as .config) compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4 reproduce: git checkout 754270c7c56292e97d0eff924a5d5d83f92add07 # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): >> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:2: error: invalid >> initializer {NULL, phm_thermal_l2h_irq}, ^ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:2: error: (near initialization for 'thermal_irq_src[0].') drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:2: error: invalid initializer {NULL, phm_thermal_h2l_irq}, ^ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:2: error: (near initialization for 'thermal_irq_src[1].') drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:129:2: error: invalid initializer {NULL, phm_ctf_irq} ^ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:129:2: error: (near initialization for 'thermal_irq_src[2].') vim +127 drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c 2a5b64c9 Eric Huang 2017-09-15 125 2a5b64c9 Eric Huang 2017-09-15 126 static const struct cgs_irq_src_funcs thermal_irq_src[3] = { 2a5b64c9 Eric Huang 2017-09-15 @127 {NULL, phm_thermal_l2h_irq}, 2a5b64c9 Eric Huang 2017-09-15 128 {NULL, phm_thermal_h2l_irq}, 2a5b64c9 Eric Huang 2017-09-15 129 {NULL, phm_ctf_irq} 2a5b64c9 Eric Huang 2017-09-15 130 }; 2a5b64c9 Eric Huang 2017-09-15 131 :: The code at line 127 was first introduced by commit :: 2a5b64c9fcd7adf6133e76966250ef3ab139f98b drm/amd/powerplay: add register thermal interrupt in hwmgr_hw_init :: TO: Eric Huang :: CC: Alex Deucher --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm:drm-next 3/3] include/linux/stddef.h:7:14: error: positional initialization of field in 'struct' declared with 'designated_init' attribute
tree: git://people.freedesktop.org/~airlied/linux.git drm-next head: 754270c7c56292e97d0eff924a5d5d83f92add07 commit: 754270c7c56292e97d0eff924a5d5d83f92add07 [3/3] Merge branch 'drm-next-4.15' of git://people.freedesktop.org/~agd5f/linux into drm-next config: x86_64-randconfig-ne0-09281127 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: git checkout 754270c7c56292e97d0eff924a5d5d83f92add07 # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): In file included from include/uapi/linux/posix_types.h:4:0, from include/uapi/linux/types.h:13, from include/linux/types.h:5, from drivers/gpu/drm/amd/amdgpu/../powerplay/inc/pp_debug.h:33, from drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:24: >> include/linux/stddef.h:7:14: error: positional initialization of field in >> 'struct' declared with 'designated_init' attribute [-Werror=designated-init] #define NULL ((void *)0) ^ >> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:3: note: in >> expansion of macro 'NULL' {NULL, phm_thermal_l2h_irq}, ^~~~ include/linux/stddef.h:7:14: note: (near initialization for 'thermal_irq_src[0]') #define NULL ((void *)0) ^ >> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:3: note: in >> expansion of macro 'NULL' {NULL, phm_thermal_l2h_irq}, ^~~~ >> include/linux/stddef.h:7:14: error: invalid initializer #define NULL ((void *)0) ^ >> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:3: note: in >> expansion of macro 'NULL' {NULL, phm_thermal_l2h_irq}, ^~~~ include/linux/stddef.h:7:14: note: (near initialization for 'thermal_irq_src[0].') #define NULL ((void *)0) ^ >> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:3: note: in >> expansion of macro 'NULL' {NULL, phm_thermal_l2h_irq}, ^~~~ >> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:9: error: >> positional initialization of field in 'struct' declared with >> 'designated_init' attribute [-Werror=designated-init] {NULL, phm_thermal_l2h_irq}, ^~~ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:9: note: (near initialization for 'thermal_irq_src[0]') drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:9: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:127:9: note: (near initialization for 'thermal_irq_src[0].set') In file included from include/uapi/linux/posix_types.h:4:0, from include/uapi/linux/types.h:13, from include/linux/types.h:5, from drivers/gpu/drm/amd/amdgpu/../powerplay/inc/pp_debug.h:33, from drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:24: >> include/linux/stddef.h:7:14: error: positional initialization of field in >> 'struct' declared with 'designated_init' attribute [-Werror=designated-init] #define NULL ((void *)0) ^ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:3: note: in expansion of macro 'NULL' {NULL, phm_thermal_h2l_irq}, ^~~~ include/linux/stddef.h:7:14: note: (near initialization for 'thermal_irq_src[1]') #define NULL ((void *)0) ^ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:3: note: in expansion of macro 'NULL' {NULL, phm_thermal_h2l_irq}, ^~~~ >> include/linux/stddef.h:7:14: error: invalid initializer #define NULL ((void *)0) ^ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:3: note: in expansion of macro 'NULL' {NULL, phm_thermal_h2l_irq}, ^~~~ include/linux/stddef.h:7:14: note: (near initialization for 'thermal_irq_src[1].') #define NULL ((void *)0) ^ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:3: note: in expansion of macro 'NULL' {NULL, phm_thermal_h2l_irq}, ^~~~ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:9: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init] {NULL, phm_thermal_h2l_irq}, ^~~ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:9: note: (near initialization for 'thermal_irq_src[1]') drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:9: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/hwmgr.c:128:9: note: (near initialization for 'thermal_irq_src[1].set') In file included from include/uapi/linux/posix_types.h:4:0, from include/uapi/linux/type
Re: [Mesa-dev] XDC 2017 feedback
On Thu, Sep 28, 2017 at 7:36 AM, Matt Turner wrote: > On Wed, Sep 27, 2017 at 10:07 PM, Rob Clark wrote: >> If you had known of the khr dates, and brought it up in Feb (or really >> somewhat earlier, given that XDC is roughly same time each year +/- >> few weeks), that *might* have been early enough to move things. > > That's unfair. It's part of the X.Org board's responsibilities to plan > conferences and that means being aware of potential conflicts. In > February, six of the eight members of the X.Org board worked for > companies with Khronos access (that's not including Keith who I > suspect has access as well). > > I replied to the 2017-03-02 minutes and noted the conflict, but as you > say that was too late. Unfortunately that was the first time a date > was publicly announced, so I'm not really sure what could have been > done from outside the X.Org board. I don't remember all the details anymore, but we have plumbers right before, and Linaro connect right afterwards, both conferences that also have considerable overlap with XDC (we have a lot more than x86 folks since 2-3 years now). Bunch of people decided not to do XDC this year even, because too much travelling in one row. Plus Google's limit in scheduling a room, plus Khr f2f. We'll try to do better next year, but sometimes perfect scheduling is just not an option. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] XDC 2017 feedback
On Wed, Sep 27, 2017 at 10:07 PM, Rob Clark wrote: > If you had known of the khr dates, and brought it up in Feb (or really > somewhat earlier, given that XDC is roughly same time each year +/- > few weeks), that *might* have been early enough to move things. That's unfair. It's part of the X.Org board's responsibilities to plan conferences and that means being aware of potential conflicts. In February, six of the eight members of the X.Org board worked for companies with Khronos access (that's not including Keith who I suspect has access as well). I replied to the 2017-03-02 minutes and noted the conflict, but as you say that was too late. Unfortunately that was the first time a date was publicly announced, so I'm not really sure what could have been done from outside the X.Org board. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] XDC 2017 feedback
On Wed, Sep 27, 2017 at 10:49 PM, Ian Romanick wrote: > On 09/27/2017 04:55 PM, Rob Clark wrote: >> On Wed, Sep 27, 2017 at 7:25 PM, Ian Romanick wrote: >>> On 09/26/2017 09:57 AM, Daniel Vetter wrote: Hi all, First again big thanks to Stéphane and Jennifer for organizing a great XDC. Like last year we'd like to hear feedback on how this year's XDC went, both the good (and what you'd like to see more of) and the not so good. Talk selection, organization, location, scheduling of talks, anything really. >>> >>> Not scheduling it to conflict with another industry event would be a >>> good start. This is the first XDC that I've missed in nearly a decade. >>> I know I'm not the only person that missed one or the other due to >>> scheduling fail. >> >> Sadly by the time we were aware of the dates for the khronos f2f it >> was not possible to change the dates for XDC :-( >> >> The XDC dates were set in Feb, and afaict the khronos dates were >> announced in July (?), so take this up with khronos ;-) > > Ok... so we're going to go there. > > Frankly, that's a giant steaming load of bull. Blocks of hotel rooms, > multiple conference rooms for 500+ people, and catering for the Khronos > meeting was booked in late 2016. We're already working on contracts for > the September 2018 meeting. Contracts of this scale are really hard to > change. There are 5x to 10x as many people at a Khronos face-to-face as > at XDC. Events of that scale have a massively deep pipeline. > > Google was just unwilling to find a different dates for space *at their > own campus*. That's really, really weak. This is especially > infuriating because there are numerous Googlers who attend the Khronos > meetings. Did the organizers poll any of them? The XDC organizers > clearly did not even exercise due diligence to detect a possible > conflict. If the organizers had cared to be aware of dates of > conflicting events, they would have known. I have no doubt there is a long lead time on organizing large conf's.. I wasn't calling that into question. The July date was based on a quick search of my khr emails. I couldn't find any earlier reference to dates, but I could have missed something. If you had known of the khr dates, and brought it up in Feb (or really somewhat earlier, given that XDC is roughly same time each year +/- few weeks), that *might* have been early enough to move things. But IIRC there wasn't much flexibility in booking such a large room from the google side either. Plus also trying to fit around LPC/etc.. Khronos isn't the only other conference to avoid. Once the XDC date is announced and people have begun booking travel, we can't really move things. Sorry, it sucks, I wasn't happy about it either, but it is what it is. As far as other conferences that XDC attendees are likely to go to, and given the turn-out (by far largest XDC in NA), I think the dates worked out reasonably well overall. BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: Fix memory leak in rockchip_drm_sys_resume()
Hi Sean, On 09/28/2017 04:27 AM, Sean Paul wrote: >@@ -299,6 +300,7 @@ static int rockchip_drm_sys_resume(struct device *dev) > >priv = drm->dev_private; >drm_atomic_helper_resume(drm, priv->state); >+ drm_atomic_state_put(priv->state); Won't this be freed for you eventually in commit_tail()? i think the drm_atomic_state_put in commit_tail is paired to the drm_atomic_state_get in drm_atomic_helper_commit. and the kmemleak shows(after a few suspend/resume): unreferenced object 0xffc0ce0fa400 (size 256): ... backtrace: [] __save_stack_trace+0x48/0x6c [] create_object+0x138/0x254 [] kmemleak_alloc+0x58/0x8c [] __kmalloc+0x1d4/0x2a0 [] usb_alloc_urb+0x30/0x60 [] alloc_ctrl_urb+0x38/0x120 [btusb] [] btusb_send_frame+0x64/0xf8 [btusb] checking the current code, i saw only i915/intel_display.c has this drm_atomic_state_put for the state allocated by drm_atomic_helper_suspend(), there're many drivers missing that(or maybe they free it in some other way?) Sean ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 03/10] drm/hisilicon/kirin: Use drm_gem_fb_create()
On 24 September 2017 at 20:26, Noralf Trønnes wrote: > drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now, > so use the function directly. > > Cc: Xinliang Liu > Cc: Rongrong Zou > Cc: Xinwei Kong > Cc: Chen Feng > Signed-off-by: Noralf Trønnes Thanks, Reviewed-by: Xinliang Liu Best, Xinliang > --- > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > index e27352c..c19ab4f 100644 > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -56,7 +57,7 @@ static void kirin_fbdev_output_poll_changed(struct > drm_device *dev) > } > > static const struct drm_mode_config_funcs kirin_drm_mode_config_funcs = { > - .fb_create = drm_fb_cma_create, > + .fb_create = drm_gem_fb_create, > .output_poll_changed = kirin_fbdev_output_poll_changed, > .atomic_check = drm_atomic_helper_check, > .atomic_commit = drm_atomic_helper_commit, > -- > 2.7.4 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/dp: Do not prune the last mode on the connector
Currently the drm_mode_prune_invalid() function will prune all the modes if it finds that the mode-status is not MODE_OK. But if it ends up pruning all modes then there are no modes left for that connector which will eventually result into a black screen as userspace sees no modes from the kernel. This can happen pretty quickly in case of eDP panel that has only mode that might get pruned. This patch fixes this problem by checking if the mode being pruned is the last mode(only mode) on that connector and if so doesnt prune it. v2: * Use correct macro from list.h (Keith Packard) Cc: dri-devel@lists.freedesktop.org Cc: Keith Packard Cc: Jani Nikula Cc: Ville Syrjala Cc: Daniel Vetter Signed-off-by: Manasi Navare --- drivers/gpu/drm/drm_modes.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 4a3f68a..9dc38a4 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1185,7 +1185,8 @@ void drm_mode_prune_invalid(struct drm_device *dev, struct drm_display_mode *mode, *t; list_for_each_entry_safe(mode, t, mode_list, head) { - if (mode->status != MODE_OK) { + if (mode->status != MODE_OK && + !list_is_singular(mode_list)) { list_del(&mode->head); if (verbose) { drm_mode_debug_printmodeline(mode); -- 2.1.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-fixes
Hi Dave, Here goes the drm/i915 fixes for 4.14-rc3 Couple fixes for stable: - Fix ELD connector types and consequently audio on DP (Jani). - Ignore HDMI on Port A and consequently fix an ops on i915 probe when VBT advertises HDMI on Port A (Jani). And a small fix: - That removes a reduntant hw_check on modeset. (Colin) This last one is really minor, but dim scripts got it and I decided to keep to keep consistency with our CI run. Also, unfortunately I didn't rebase on Linus 4.14-rc2 tag as I should, but it is based on a later merge he did. So pull request itself is not ideal but at least it is sane this time. But for next time I will make sure I rebase it on fixed tags as Jani was doing and also I'm already taking a look to improving our documents and maybe dim script to automate that rebase. Thanks, Rodrigo. The following changes since commit e365806ac289457263a133bd32df8df49897f612: Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs (2017-09-25 18:24:14 -0700) are available in the git repository at: git://anongit.freedesktop.org/git/drm-intel tags/drm-intel-fixes-2017-09-27 for you to fetch changes up to 2ba7d7e0437127314864238f8bfcb8369d81075c: drm/i915/bios: ignore HDMI on port A (2017-09-26 09:14:28 -0700) drm/i915 fixes for 4.14-rc3 Couple fixes for stable: - Fix ELD connector types and consequently audio on DP (Jani). - Ignore HDMI on Port A and consequently fix an ops on i915 probe when VBT advertises HDMI on Port A (Jani). And a small fix: - That removes a reduntant hw_check on modeset. (Colin) Colin Ian King (1): drm/i915: remove redundant variable hw_check Jani Nikula (2): drm/i915: always update ELD connector type after get modes drm/i915/bios: ignore HDMI on port A drivers/gpu/drm/i915/intel_audio.c | 5 - drivers/gpu/drm/i915/intel_bios.c| 7 +++ drivers/gpu/drm/i915/intel_display.c | 2 -- drivers/gpu/drm/i915/intel_modes.c | 17 + 4 files changed, 24 insertions(+), 7 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dp: Do not prune the last mode on the connector
On Wed, Sep 27, 2017 at 05:31:56PM -0700, Keith Packard wrote: > Manasi Navare writes: > > > This patch fixes this problem by checking if the mode being pruned > > is the last mode on that connector and if so doesnt prune it. > > I think you want to stop pruning when you've gotten to a single mode on > the list, not at the last mode in the list (which may well need to be > pruned). > > This is entirely untested, but perhaps > > + !(list_is_singular(mode_list))) { > > is what you want? > Thanks Keith. Yes thats correct, I got confused by the macro name. But its correct I should use the list_is_singular instead. Manasi > -- > -keith ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dp: Do not prune the last mode on the connector
Manasi Navare writes: > This patch fixes this problem by checking if the mode being pruned > is the last mode on that connector and if so doesnt prune it. I think you want to stop pruning when you've gotten to a single mode on the list, not at the last mode in the list (which may well need to be pruned). This is entirely untested, but perhaps + !(list_is_singular(mode_list))) { is what you want? -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dp: Do not prune the last mode on the connector
Currently the drm_mode_prune_invalid() function will prune all the modes if it finds that the mode-status is not MODE_OK. But if it ends up pruning all modes then there are no modes left for that connector which will eventually result into a black screen as userspace sees no modes from the kernel. This can happen pretty quickly in case of eDP panel that has only mode that might get pruned. This patch fixes this problem by checking if the mode being pruned is the last mode on that connector and if so doesnt prune it. Cc: dri-devel@lists.freedesktop.org Cc: Keith Packard Cc: Jani Nikula Cc: Ville Syrjala Cc: Daniel Vetter Signed-off-by: Manasi Navare --- drivers/gpu/drm/drm_modes.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 4a3f68a..a9369eb 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1185,7 +1185,8 @@ void drm_mode_prune_invalid(struct drm_device *dev, struct drm_display_mode *mode, *t; list_for_each_entry_safe(mode, t, mode_list, head) { - if (mode->status != MODE_OK) { + if (mode->status != MODE_OK && + !(list_is_last(&mode->head, mode_list))) { list_del(&mode->head); if (verbose) { drm_mode_debug_printmodeline(mode); -- 2.1.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] XDC 2017 feedback
On Wed, Sep 27, 2017 at 7:25 PM, Ian Romanick wrote: > On 09/26/2017 09:57 AM, Daniel Vetter wrote: >> Hi all, >> >> First again big thanks to Stéphane and Jennifer for organizing a great XDC. >> >> Like last year we'd like to hear feedback on how this year's XDC went, >> both the good (and what you'd like to see more of) and the not so >> good. Talk selection, organization, location, scheduling of talks, >> anything really. > > Not scheduling it to conflict with another industry event would be a > good start. This is the first XDC that I've missed in nearly a decade. > I know I'm not the only person that missed one or the other due to > scheduling fail. > Sadly by the time we were aware of the dates for the khronos f2f it was not possible to change the dates for XDC :-( The XDC dates were set in Feb, and afaict the khronos dates were announced in July (?), so take this up with khronos ;-) BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] DC pull request cleanup
On Wed, Sep 27, 2017 at 3:46 PM, Harry Wentland wrote: > Patches to make DC use kzalloc/krealloc/kfree directly. > > Also updating the TODO list after a closer look at I2C in DC. > > Harry Wentland (3): > drm/amd/display: Use kernel alloc/free > drm/amd/display: Remove alloc/free macros > drm/amd/display: DC I2C review patches 2, 3, and v2 of patch 1 are: Reviewed-by: Alex Deucher > > drivers/gpu/drm/amd/display/TODO | 26 + > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- > drivers/gpu/drm/amd/display/dc/basics/logger.c | 18 --- > drivers/gpu/drm/amd/display/dc/basics/vector.c | 19 +++ > drivers/gpu/drm/amd/display/dc/bios/bios_parser.c | 17 +++--- > drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 14 ++--- > drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 5 +- > drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c | 6 +-- > drivers/gpu/drm/amd/display/dc/core/dc.c | 42 --- > drivers/gpu/drm/amd/display/dc/core/dc_link.c | 6 +-- > drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c | 18 +++ > drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +- > drivers/gpu/drm/amd/display/dc/core/dc_sink.c | 11 ++-- > drivers/gpu/drm/amd/display/dc/core/dc_stream.c| 6 +-- > drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 15 +++--- > drivers/gpu/drm/amd/display/dc/dce/dce_abm.c | 4 +- > drivers/gpu/drm/amd/display/dc/dce/dce_audio.c | 4 +- > .../gpu/drm/amd/display/dc/dce/dce_clock_source.c | 12 +++-- > drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c| 10 ++-- > drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 6 +-- > drivers/gpu/drm/amd/display/dc/dce/dce_ipp.c | 2 +- > .../gpu/drm/amd/display/dc/dce/dce_link_encoder.c | 2 +- > drivers/gpu/drm/amd/display/dc/dce/dce_opp.c | 2 +- > .../drm/amd/display/dc/dce100/dce100_resource.c| 43 +++ > .../drm/amd/display/dc/dce110/dce110_compressor.c | 6 +-- > .../drm/amd/display/dc/dce110/dce110_resource.c| 61 > -- > .../drm/amd/display/dc/dce112/dce112_compressor.c | 6 +-- > .../drm/amd/display/dc/dce112/dce112_resource.c| 43 +++ > .../drm/amd/display/dc/dce120/dce120_resource.c| 43 +++ > .../drm/amd/display/dc/dce80/dce80_compressor.c| 6 +-- > .../gpu/drm/amd/display/dc/dce80/dce80_resource.c | 47 + > drivers/gpu/drm/amd/display/dc/dcn10/dcn10_ipp.c | 2 +- > drivers/gpu/drm/amd/display/dc/dcn10/dcn10_opp.c | 2 +- > .../gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 56 ++-- > drivers/gpu/drm/amd/display/dc/dm_services.h | 4 -- > drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c| 4 +- > drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 21 > drivers/gpu/drm/amd/display/dc/gpio/hw_ddc.c | 6 +-- > drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c | 2 +- > drivers/gpu/drm/amd/display/dc/gpio/hw_hpd.c | 6 +-- > .../amd/display/dc/i2caux/dce100/i2caux_dce100.c | 4 +- > .../display/dc/i2caux/dce110/aux_engine_dce110.c | 6 +-- > .../dc/i2caux/dce110/i2c_hw_engine_dce110.c| 7 +-- > .../dc/i2caux/dce110/i2c_sw_engine_dce110.c| 7 +-- > .../amd/display/dc/i2caux/dce110/i2caux_dce110.c | 6 +-- > .../amd/display/dc/i2caux/dce112/i2caux_dce112.c | 4 +- > .../amd/display/dc/i2caux/dce120/i2caux_dce120.c | 4 +- > .../display/dc/i2caux/dce80/i2c_hw_engine_dce80.c | 6 +-- > .../display/dc/i2caux/dce80/i2c_sw_engine_dce80.c | 6 +-- > .../drm/amd/display/dc/i2caux/dce80/i2caux_dce80.c | 6 +-- > .../drm/amd/display/dc/i2caux/dcn10/i2caux_dcn10.c | 4 +- > .../display/dc/i2caux/diagnostics/i2caux_diag.c| 7 +-- > .../gpu/drm/amd/display/dc/i2caux/i2c_sw_engine.c | 6 +-- > .../amd/display/dc/irq/dce110/irq_service_dce110.c | 5 +- > .../amd/display/dc/irq/dce120/irq_service_dce120.c | 5 +- > .../amd/display/dc/irq/dce80/irq_service_dce80.c | 5 +- > .../amd/display/dc/irq/dcn10/irq_service_dcn10.c | 5 +- > drivers/gpu/drm/amd/display/dc/irq/irq_service.c | 2 +- > .../amd/display/dc/virtual/virtual_link_encoder.c | 2 +- > .../display/dc/virtual/virtual_stream_encoder.c| 4 +- > .../drm/amd/display/modules/freesync/freesync.c| 14 ++--- > drivers/gpu/drm/amd/display/replace_alloc.cocci| 25 + > 62 files changed, 399 insertions(+), 348 deletions(-) > create mode 100644 drivers/gpu/drm/amd/display/replace_alloc.cocci > > -- > 2.11.0 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND RFC PATCH 4/7] dt-bindings: Document Allwinner DWC HDMI TX node
On Wed, Sep 20, 2017 at 10:01:21PM +0200, Jernej Skrabec wrote: > Add documentation about Allwinner DWC HDMI TX node, found in H3 SoC. > > Signed-off-by: Jernej Skrabec > --- > .../bindings/display/sunxi/sun4i-drm.txt | 158 > - > 1 file changed, 157 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > index 92512953943e..cb6aee5c486f 100644 > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > @@ -60,6 +60,40 @@ Required properties: > first port should be the input endpoint. The second should be the > output, usually to an HDMI connector. > > +DWC HDMI TX Encoder > +- > + > +The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP > +with Allwinner's own PHY IP. It supports audio and video outputs and CEC. > + > +These DT bindings follow the Synopsys DWC HDMI TX bindings defined in > +Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with the > +following device-specific properties. > + > +Required properties: > + > + - compatible: value must be one of: > +* "allwinner,sun8i-h3-dw-hdmi" > + - reg: two pairs of base address and size of memory-mapped region, first > +for controller and second for PHY > +registers. > + - reg-io-width: See dw_hdmi.txt. Shall be 1. > + - interrupts: HDMI interrupt number > + - clocks: phandles to the clocks feeding the HDMI encoder > +* iahb: the HDMI interface clock > +* isfr: the HDMI module clock > +* ddc: the HDMI ddc clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset controllers driving the encoder > +* hdmi: the reset line for the HDMI > +* ddc: the reset line for the DDC > + - reset-names: the reset names mentioned above > + > + - ports: A ports node with endpoint definitions as defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. The > +first port should be the input endpoint. The second should be the > +output, usually to an HDMI connector. > + > TV Encoder > -- > > @@ -255,7 +289,7 @@ Required properties: >- allwinner,pipelines: list of phandle to the display engine > frontends (DE 1.0) or mixers (DE 2.0) available. > > -Example: > +Example 1: > > panel: panel { > compatible = "olimex,lcd-olinuxino-43-ts"; > @@ -455,3 +489,125 @@ display-engine { > compatible = "allwinner,sun5i-a13-display-engine"; > allwinner,pipelines = <&fe0>; > }; > + > +Example 2: > + > +connector { > + compatible = "hdmi-connector"; > + type = "a"; > + > + port { > + hdmi_con_in: endpoint { > + remote-endpoint = <&hdmi_out_con>; > + }; > + }; > +}; > + > +de: display-engine { > + compatible = "allwinner,sun8i-h3-display-engine"; > + allwinner,pipelines = <&mixer0>; > +}; > + > +hdmi: hdmi@1ee { > + compatible = "allwinner,h3-dw-hdmi"; > + reg = <0x01ee 0x1>, > + <0x01ef 0x1>; > + reg-io-width = <1>; > + interrupts = ; > + clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>, > + <&ccu CLK_HDMI_DDC>; > + clock-names = "iahb", "isfr", "ddc"; > + resets = <&ccu RST_BUS_HDMI0>, <&ccu RST_BUS_HDMI1>; > + reset-names = "hdmi", "ddc"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + hdmi_in: port@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0>; > + > + hdmi_in_tcon0: endpoint@0 { > + reg = <0>; You don't need reg when there's only one. Otherwise, Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/amd/display: Use kernel alloc/free
Abstractions are frowned upon. cocci script: virtual context virtual patch virtual org virtual report @@ expression ptr; @@ - dm_alloc(ptr) + kzalloc(ptr, GFP_KERNEL) @@ expression ptr, size; @@ - dm_realloc(ptr, size) + krealloc(ptr, size, GFP_KERNEL) @@ expression ptr; @@ - dm_free(ptr) + kfree(ptr) v2: use GFP_KERNEL, not GFP_ATOMIC. add cocci script Signed-off-by: Harry Wentland --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- drivers/gpu/drm/amd/display/dc/basics/logger.c | 18 --- drivers/gpu/drm/amd/display/dc/basics/vector.c | 19 +++ drivers/gpu/drm/amd/display/dc/bios/bios_parser.c | 17 +++--- drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 14 ++--- drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 5 +- drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c | 6 +-- drivers/gpu/drm/amd/display/dc/core/dc.c | 42 --- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 6 +-- drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c | 18 +++ drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +- drivers/gpu/drm/amd/display/dc/core/dc_sink.c | 11 ++-- drivers/gpu/drm/amd/display/dc/core/dc_stream.c| 6 +-- drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 15 +++--- drivers/gpu/drm/amd/display/dc/dce/dce_abm.c | 4 +- drivers/gpu/drm/amd/display/dc/dce/dce_audio.c | 4 +- .../gpu/drm/amd/display/dc/dce/dce_clock_source.c | 12 +++-- drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c| 10 ++-- drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 6 +-- drivers/gpu/drm/amd/display/dc/dce/dce_ipp.c | 2 +- .../gpu/drm/amd/display/dc/dce/dce_link_encoder.c | 2 +- drivers/gpu/drm/amd/display/dc/dce/dce_opp.c | 2 +- .../drm/amd/display/dc/dce100/dce100_resource.c| 43 +++ .../drm/amd/display/dc/dce110/dce110_compressor.c | 6 +-- .../drm/amd/display/dc/dce110/dce110_resource.c| 61 -- .../drm/amd/display/dc/dce112/dce112_compressor.c | 6 +-- .../drm/amd/display/dc/dce112/dce112_resource.c| 43 +++ .../drm/amd/display/dc/dce120/dce120_resource.c| 43 +++ .../drm/amd/display/dc/dce80/dce80_compressor.c| 6 +-- .../gpu/drm/amd/display/dc/dce80/dce80_resource.c | 47 + drivers/gpu/drm/amd/display/dc/dcn10/dcn10_ipp.c | 2 +- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_opp.c | 2 +- .../gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 56 ++-- drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c| 4 +- drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 21 drivers/gpu/drm/amd/display/dc/gpio/hw_ddc.c | 6 +-- drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c | 2 +- drivers/gpu/drm/amd/display/dc/gpio/hw_hpd.c | 6 +-- .../amd/display/dc/i2caux/dce100/i2caux_dce100.c | 4 +- .../display/dc/i2caux/dce110/aux_engine_dce110.c | 6 +-- .../dc/i2caux/dce110/i2c_hw_engine_dce110.c| 7 +-- .../dc/i2caux/dce110/i2c_sw_engine_dce110.c| 7 +-- .../amd/display/dc/i2caux/dce110/i2caux_dce110.c | 6 +-- .../amd/display/dc/i2caux/dce112/i2caux_dce112.c | 4 +- .../amd/display/dc/i2caux/dce120/i2caux_dce120.c | 4 +- .../display/dc/i2caux/dce80/i2c_hw_engine_dce80.c | 6 +-- .../display/dc/i2caux/dce80/i2c_sw_engine_dce80.c | 6 +-- .../drm/amd/display/dc/i2caux/dce80/i2caux_dce80.c | 6 +-- .../drm/amd/display/dc/i2caux/dcn10/i2caux_dcn10.c | 4 +- .../display/dc/i2caux/diagnostics/i2caux_diag.c| 7 +-- .../gpu/drm/amd/display/dc/i2caux/i2c_sw_engine.c | 6 +-- .../amd/display/dc/irq/dce110/irq_service_dce110.c | 5 +- .../amd/display/dc/irq/dce120/irq_service_dce120.c | 5 +- .../amd/display/dc/irq/dce80/irq_service_dce80.c | 5 +- .../amd/display/dc/irq/dcn10/irq_service_dcn10.c | 5 +- drivers/gpu/drm/amd/display/dc/irq/irq_service.c | 2 +- .../amd/display/dc/virtual/virtual_link_encoder.c | 2 +- .../display/dc/virtual/virtual_stream_encoder.c| 4 +- .../drm/amd/display/modules/freesync/freesync.c| 14 ++--- 59 files changed, 362 insertions(+), 330 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 36635486b937..dba54c0a7b5f 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2429,7 +2429,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) if (WARN_ON(!crtc->state)) return NULL; - state = dm_alloc(sizeof(*state)); + state = kzalloc(sizeof(*state), GFP_KERNEL); __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base); diff --git a/drivers/gpu/drm/amd/display/dc/basics/logger.c b/drivers/gpu/drm/amd/display/dc/basics/logger.c index 5895dd3903a3..afb6d2d80e0c 100644 --- a/drivers/gpu/drm/amd/display/dc/basics/logger.c +++ b/drivers/gpu/drm/amd/display/dc/basics/logger.c @@ -70,8 +
Re: [PATCH hwc v1] drm_hwcomposer: Add CONTRIBUTING file
On Wed, Sep 27, 2017 at 1:55 PM, Robert Foss wrote: > Hey Emil, > > On Wed, 2017-09-27 at 15:42 +0100, Emil Velikov wrote: >> Hi Rob, >> >> Glad to see this. There's a couple of suggestions that I hope you'll >> find worth while. >> >> On 22 September 2017 at 01:37, Robert Foss > > wrote: >> > Some basic guidelines for contributions could come in handy. >> > >> > These are copied from IGT and modified to be suitable. >> > >> > Signed-off-by: Robert Foss >> > --- >> > CONTRIBUTING | 31 +++ >> > 1 file changed, 31 insertions(+) >> > create mode 100644 CONTRIBUTING >> > >> > diff --git a/CONTRIBUTING b/CONTRIBUTING >> > new file mode 100644 >> > index 000..f1b4775 >> > --- /dev/null >> > +++ b/CONTRIBUTING >> > @@ -0,0 +1,31 @@ >> > +Patches to drm_hwcomposer are very much welcome, we really want >> > this to be the >> > +universal HW composer implementation for Android and similar >> > platforms >> > +So please bring on porting patches, bugfixes, improvements for >> > documentation >> > +and new features. >> > + >> > +A short list of contribution guidelines: >> > + >> > +- Please submit patches formatted with git send-email/git format- >> > patch or >> > + equivalent to >> > + >> > +dri-devel >> > + >> > + Please use --subject-prefix="PATCH hwc" so that drm_hwcomposer >> > patches are easily >> > + identified in the massive amount mails on dri-devel. To ensure >> > this is always >> > + done, run: >> > + >> > +git config format.subjectprefix "PATCH hwc" >> > + >> >> One can add this into the autogen.sh or whatever bootstrap file the >> project uses. >> For example see https://cgit.freedesktop.org/mesa/drm/tree/autogen.sh > > Adding an autogen.sh sounds like a good idea to me. Except autogen.sh is a standard step as part of an autotools workflow and would not be obvious to run in an Android env. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: Fix memory leak in rockchip_drm_sys_resume()
On Wed, Sep 27, 2017 at 08:26:42PM +0800, Jeffy Chen wrote: > Free the drm_atomic_state allocated by drm_atomic_helper_suspend(). > > Fixes: 5a5873830972 ("drm/rockchip: Use atomic PM helpers") > Signed-off-by: Jeffy Chen > --- > > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > index 76d63de5921d..80235b672deb 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -15,6 +15,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -299,6 +300,7 @@ static int rockchip_drm_sys_resume(struct device *dev) > > priv = drm->dev_private; > drm_atomic_helper_resume(drm, priv->state); > + drm_atomic_state_put(priv->state); Won't this be freed for you eventually in commit_tail()? Sean > rockchip_drm_fb_resume(drm); > drm_kms_helper_poll_enable(drm); > > -- > 2.11.0 > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: Rely on the default best_encoder() behavior
On Wed, Sep 27, 2017 at 12:23:17PM -0600, Haneen Mohammed wrote: > Since the output has 1:1 relationship between connectors and encoders, > and the driver is relying on the atomic helpers, remove the custom > best_encoder() and let the core call drm_atomic_helper_best_encoder(). > > Signed-off-by: Haneen Mohammed Thanks for the patch, I've applied it to drm-misc-next. I noticed a few instances of .best_encoder = drm_atomic_helper_best_encoder hanging around. Any interest in removing those as well? Sean > --- > drivers/gpu/drm/rockchip/cdn-dp-core.c | 9 - > 1 file changed, 9 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c > b/drivers/gpu/drm/rockchip/cdn-dp-core.c > index a57da05..275844d 100644 > --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c > +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c > @@ -287,14 +287,6 @@ static int cdn_dp_connector_get_modes(struct > drm_connector *connector) > return ret; > } > > -static struct drm_encoder * > -cdn_dp_connector_best_encoder(struct drm_connector *connector) > -{ > - struct cdn_dp_device *dp = connector_to_dp(connector); > - > - return &dp->encoder; > -} > - > static int cdn_dp_connector_mode_valid(struct drm_connector *connector, > struct drm_display_mode *mode) > { > @@ -346,7 +338,6 @@ static int cdn_dp_connector_mode_valid(struct > drm_connector *connector, > > static struct drm_connector_helper_funcs cdn_dp_connector_helper_funcs = { > .get_modes = cdn_dp_connector_get_modes, > - .best_encoder = cdn_dp_connector_best_encoder, > .mode_valid = cdn_dp_connector_mode_valid, > }; > > -- > 2.7.4 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd: DC pull request review
On 2017-09-27 02:12 PM, Harry Wentland wrote: > On 2017-09-27 12:48 PM, Daniel Vetter wrote: >> On Wed, Sep 27, 2017 at 6:38 PM, Sean Paul wrote: >>> Any chance we can address the i2c/gpio [re-]implementations as well? >> >> It's already on the list. Part of this is code that's probably dead, >> the other is a bit too much layer cake still left, and the final bits >> are not fully using drm helpers for edid parsing and tons of other >> stuff. But afaics all the i2c stuff does go through the i2c_adapter >> abstraction in a proper fashion, which is at least the foundational >> work. The other bits should be all captured in the todo already. > > There might still be some dead code around i2c stuff. I'll take a look > and see what I can rip out. > Looks like there currently are no easy pickings. What we have is: 1) Connector info read See get_ext_display_connection_info On some boards the connector information has to be read through a special I2C channel. This line is only used for this purpose and only on driver init. 2) SCDC stuff This should all be reworked to go through DRM's SCDC code. When this is done some unnecessary I2C code can be retired as well. 3) Max TMDS clock read See dal_ddc_service_i2c_query_dp_dual_mode_adaptor This should happen in DRM as well. I haven't checked if there's currently functionality in DRM. If not we can propose something. 4) HDMI retimer programming Some boards have an HDMI retimer that we need to program to pass PHY compliance. It would take a bit of time to address all of them. I'll add them on the TODO. 1 & 3 might be a good exercise if someone is looking for things to do. Harry > If there's any specific function, struct, etc. that's of concern let me > know as well. > > Harry > >> -Daniel >> > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/8] drm/rockchip/dsi: correct Feedback divider setting
On Tue, Sep 26, 2017 at 03:55:16PM +0800, Nickey Yang wrote: > This patch correct Feedback divider setting: > 1、Set Feedback divider [8:5] when HIGH_PROGRAM_EN > 2、Due to the use of a "by 2 pre-scaler," the range of the > feedback multiplication Feedback divider is limited to even > division numbers, and Feedback divider must be greater than > 12, less than 1000. > 3、Make the previously configured Feedback divider(LSB) > factors effective > 4、Add the definition of the MIPI PHY register. > > Signed-off-by: Nickey Yang > --- Can you please add a changelog to your patches so it's easy to see what's changed? Thanks! Sean > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 219 > ++--- > 1 file changed, 146 insertions(+), 73 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > index 9a20b9d..c933a3a 100644 > -- > 1.9.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/amd/display: DC I2C review
While reviewing I2C in DC identified a few places. Added a couple to the TODO list. 1) Connector info read See get_ext_display_connection_info On some boards the connector information has to be read through a special I2C channel. This line is only used for this purpose and only on driver init. 2) SCDC stuff This should all be reworked to go through DRM's SCDC code. When this is done some unnecessary I2C code can be retired as well. 3) Max TMDS clock read See dal_ddc_service_i2c_query_dp_dual_mode_adaptor This should happen in DRM as well. I haven't checked if there's currently functionality in DRM. If not we can propose something. 4) HDMI retimer programming Some boards have an HDMI retimer that we need to program to pass PHY compliance. 1 & 3 might be a good exercise if someone is looking for things to do. Signed-off-by: Harry Wentland --- drivers/gpu/drm/amd/display/TODO | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/display/TODO b/drivers/gpu/drm/amd/display/TODO index eea645b102a1..981352bc95f0 100644 --- a/drivers/gpu/drm/amd/display/TODO +++ b/drivers/gpu/drm/amd/display/TODO @@ -62,20 +62,10 @@ TODOs ~ Daniel Vetter -11. Remove existing i2c implementation from DC - -"Similar story for i2c, it uses the kernel's i2c code now, but there's -still a full i2c implementation hidden beneath that in -display/dc/i2caux. Kinda not cool, but imo ok if you fix that -post-merging (perhaps by not including any of this in the linux DC -code in the upstream kernel, but as an aux module in your internal -codebase since there you probably need that, same applies to the edid -parsing DC still does. For both cases I assume that the minimal shim -you need on linux (bit banging and edid parsing isn't rocket since) is -a lot less than the glue code to interface with the dc-provided -abstraction." -~ Daniel Vetter - +11. Remove dc/i2caux. This folder can be somewhat misleading. It's basically an +overy complicated HW programming function for sendind and receiving i2c/aux +commands. We can greatly simplify that and move it into dc/dceXYZ like other +HW blocks. 12. drm_modeset_lock in MST should no longer be needed in recent kernels * Adopt appropriate locking scheme @@ -110,3 +100,11 @@ guilty. stuff just isn't up to the challenges either. We need to figure out something that integrates better with DRM and linux debug printing, while not being useless with filtering output. dynamic debug printing might be an option. + +20. Move Max TMDS clock read to DRM. See +dal_ddc_service_i2c_query_dp_dual_mode_adaptor. I haven't checked if there's +currently functionality in DRM. If not we can propose something. + +21. Use kernel i2c device to program HDMI retimer. Some boards have an HDMI +retimer that we need to program to pass PHY compliance. Currently that's +bypassing the i2c device and goes directly to HW. This should be changed. -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/amd/display: Remove alloc/free macros
Now that we don't abstract kernel alloc interfaces we don't need those anymore. Signed-off-by: Harry Wentland --- drivers/gpu/drm/amd/display/dc/dm_services.h | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dm_services.h b/drivers/gpu/drm/amd/display/dc/dm_services.h index 8ab0af6f4c6b..7260e772725d 100644 --- a/drivers/gpu/drm/amd/display/dc/dm_services.h +++ b/drivers/gpu/drm/amd/display/dc/dm_services.h @@ -79,10 +79,6 @@ #include #endif -#define dm_alloc(size) kzalloc(size, GFP_KERNEL) -#define dm_realloc(ptr, size) krealloc(ptr, size, GFP_KERNEL) -#define dm_free(ptr) kfree(ptr) - irq_handler_idx dm_register_interrupt( struct dc_context *ctx, struct dc_interrupt_params *int_params, -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/amd/display: Use kernel alloc/free
Abstractions are frowned upon. Signed-off-by: Harry Wentland --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- drivers/gpu/drm/amd/display/dc/basics/logger.c | 18 --- drivers/gpu/drm/amd/display/dc/basics/vector.c | 19 +++ drivers/gpu/drm/amd/display/dc/bios/bios_parser.c | 17 +++--- drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 14 ++--- drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 5 +- drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c | 6 +-- drivers/gpu/drm/amd/display/dc/core/dc.c | 42 --- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 6 +-- drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c | 18 +++ drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +- drivers/gpu/drm/amd/display/dc/core/dc_sink.c | 11 ++-- drivers/gpu/drm/amd/display/dc/core/dc_stream.c| 6 +-- drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 15 +++--- drivers/gpu/drm/amd/display/dc/dce/dce_abm.c | 4 +- drivers/gpu/drm/amd/display/dc/dce/dce_audio.c | 4 +- .../gpu/drm/amd/display/dc/dce/dce_clock_source.c | 12 +++-- drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c| 10 ++-- drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 6 +-- drivers/gpu/drm/amd/display/dc/dce/dce_ipp.c | 2 +- .../gpu/drm/amd/display/dc/dce/dce_link_encoder.c | 2 +- drivers/gpu/drm/amd/display/dc/dce/dce_opp.c | 2 +- .../drm/amd/display/dc/dce100/dce100_resource.c| 43 +++ .../drm/amd/display/dc/dce110/dce110_compressor.c | 6 +-- .../drm/amd/display/dc/dce110/dce110_resource.c| 61 -- .../drm/amd/display/dc/dce112/dce112_compressor.c | 6 +-- .../drm/amd/display/dc/dce112/dce112_resource.c| 43 +++ .../drm/amd/display/dc/dce120/dce120_resource.c| 43 +++ .../drm/amd/display/dc/dce80/dce80_compressor.c| 6 +-- .../gpu/drm/amd/display/dc/dce80/dce80_resource.c | 47 + drivers/gpu/drm/amd/display/dc/dcn10/dcn10_ipp.c | 2 +- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_opp.c | 2 +- .../gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 56 ++-- drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c| 4 +- drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 21 drivers/gpu/drm/amd/display/dc/gpio/hw_ddc.c | 6 +-- drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c | 2 +- drivers/gpu/drm/amd/display/dc/gpio/hw_hpd.c | 6 +-- .../amd/display/dc/i2caux/dce100/i2caux_dce100.c | 4 +- .../display/dc/i2caux/dce110/aux_engine_dce110.c | 6 +-- .../dc/i2caux/dce110/i2c_hw_engine_dce110.c| 7 +-- .../dc/i2caux/dce110/i2c_sw_engine_dce110.c| 7 +-- .../amd/display/dc/i2caux/dce110/i2caux_dce110.c | 6 +-- .../amd/display/dc/i2caux/dce112/i2caux_dce112.c | 4 +- .../amd/display/dc/i2caux/dce120/i2caux_dce120.c | 4 +- .../display/dc/i2caux/dce80/i2c_hw_engine_dce80.c | 6 +-- .../display/dc/i2caux/dce80/i2c_sw_engine_dce80.c | 6 +-- .../drm/amd/display/dc/i2caux/dce80/i2caux_dce80.c | 6 +-- .../drm/amd/display/dc/i2caux/dcn10/i2caux_dcn10.c | 4 +- .../display/dc/i2caux/diagnostics/i2caux_diag.c| 7 +-- .../gpu/drm/amd/display/dc/i2caux/i2c_sw_engine.c | 6 +-- .../amd/display/dc/irq/dce110/irq_service_dce110.c | 5 +- .../amd/display/dc/irq/dce120/irq_service_dce120.c | 5 +- .../amd/display/dc/irq/dce80/irq_service_dce80.c | 5 +- .../amd/display/dc/irq/dcn10/irq_service_dcn10.c | 5 +- drivers/gpu/drm/amd/display/dc/irq/irq_service.c | 2 +- .../amd/display/dc/virtual/virtual_link_encoder.c | 2 +- .../display/dc/virtual/virtual_stream_encoder.c| 4 +- .../drm/amd/display/modules/freesync/freesync.c| 14 ++--- drivers/gpu/drm/amd/display/replace_alloc.cocci| 25 + 60 files changed, 387 insertions(+), 330 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/replace_alloc.cocci diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 36635486b937..2afa99c0e95e 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2429,7 +2429,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) if (WARN_ON(!crtc->state)) return NULL; - state = dm_alloc(sizeof(*state)); + state = kzalloc(sizeof(*state), GFP_ATOMIC); __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base); diff --git a/drivers/gpu/drm/amd/display/dc/basics/logger.c b/drivers/gpu/drm/amd/display/dc/basics/logger.c index 5895dd3903a3..9180050424b9 100644 --- a/drivers/gpu/drm/amd/display/dc/basics/logger.c +++ b/drivers/gpu/drm/amd/display/dc/basics/logger.c @@ -70,8 +70,8 @@ static bool construct(struct dc_context *ctx, struct dal_logger *logger, { /* malloc buffer and init offsets */ logger->log_buffer_size = DAL_LOGGER_BUFFER_MAX_SIZE; -
[PATCH 0/3] DC pull request cleanup
Patches to make DC use kzalloc/krealloc/kfree directly. Also updating the TODO list after a closer look at I2C in DC. Harry Wentland (3): drm/amd/display: Use kernel alloc/free drm/amd/display: Remove alloc/free macros drm/amd/display: DC I2C review drivers/gpu/drm/amd/display/TODO | 26 + drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- drivers/gpu/drm/amd/display/dc/basics/logger.c | 18 --- drivers/gpu/drm/amd/display/dc/basics/vector.c | 19 +++ drivers/gpu/drm/amd/display/dc/bios/bios_parser.c | 17 +++--- drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 14 ++--- drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 5 +- drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c | 6 +-- drivers/gpu/drm/amd/display/dc/core/dc.c | 42 --- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 6 +-- drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c | 18 +++ drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +- drivers/gpu/drm/amd/display/dc/core/dc_sink.c | 11 ++-- drivers/gpu/drm/amd/display/dc/core/dc_stream.c| 6 +-- drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 15 +++--- drivers/gpu/drm/amd/display/dc/dce/dce_abm.c | 4 +- drivers/gpu/drm/amd/display/dc/dce/dce_audio.c | 4 +- .../gpu/drm/amd/display/dc/dce/dce_clock_source.c | 12 +++-- drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c| 10 ++-- drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 6 +-- drivers/gpu/drm/amd/display/dc/dce/dce_ipp.c | 2 +- .../gpu/drm/amd/display/dc/dce/dce_link_encoder.c | 2 +- drivers/gpu/drm/amd/display/dc/dce/dce_opp.c | 2 +- .../drm/amd/display/dc/dce100/dce100_resource.c| 43 +++ .../drm/amd/display/dc/dce110/dce110_compressor.c | 6 +-- .../drm/amd/display/dc/dce110/dce110_resource.c| 61 -- .../drm/amd/display/dc/dce112/dce112_compressor.c | 6 +-- .../drm/amd/display/dc/dce112/dce112_resource.c| 43 +++ .../drm/amd/display/dc/dce120/dce120_resource.c| 43 +++ .../drm/amd/display/dc/dce80/dce80_compressor.c| 6 +-- .../gpu/drm/amd/display/dc/dce80/dce80_resource.c | 47 + drivers/gpu/drm/amd/display/dc/dcn10/dcn10_ipp.c | 2 +- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_opp.c | 2 +- .../gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 56 ++-- drivers/gpu/drm/amd/display/dc/dm_services.h | 4 -- drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c| 4 +- drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 21 drivers/gpu/drm/amd/display/dc/gpio/hw_ddc.c | 6 +-- drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c | 2 +- drivers/gpu/drm/amd/display/dc/gpio/hw_hpd.c | 6 +-- .../amd/display/dc/i2caux/dce100/i2caux_dce100.c | 4 +- .../display/dc/i2caux/dce110/aux_engine_dce110.c | 6 +-- .../dc/i2caux/dce110/i2c_hw_engine_dce110.c| 7 +-- .../dc/i2caux/dce110/i2c_sw_engine_dce110.c| 7 +-- .../amd/display/dc/i2caux/dce110/i2caux_dce110.c | 6 +-- .../amd/display/dc/i2caux/dce112/i2caux_dce112.c | 4 +- .../amd/display/dc/i2caux/dce120/i2caux_dce120.c | 4 +- .../display/dc/i2caux/dce80/i2c_hw_engine_dce80.c | 6 +-- .../display/dc/i2caux/dce80/i2c_sw_engine_dce80.c | 6 +-- .../drm/amd/display/dc/i2caux/dce80/i2caux_dce80.c | 6 +-- .../drm/amd/display/dc/i2caux/dcn10/i2caux_dcn10.c | 4 +- .../display/dc/i2caux/diagnostics/i2caux_diag.c| 7 +-- .../gpu/drm/amd/display/dc/i2caux/i2c_sw_engine.c | 6 +-- .../amd/display/dc/irq/dce110/irq_service_dce110.c | 5 +- .../amd/display/dc/irq/dce120/irq_service_dce120.c | 5 +- .../amd/display/dc/irq/dce80/irq_service_dce80.c | 5 +- .../amd/display/dc/irq/dcn10/irq_service_dcn10.c | 5 +- drivers/gpu/drm/amd/display/dc/irq/irq_service.c | 2 +- .../amd/display/dc/virtual/virtual_link_encoder.c | 2 +- .../display/dc/virtual/virtual_stream_encoder.c| 4 +- .../drm/amd/display/modules/freesync/freesync.c| 14 ++--- drivers/gpu/drm/amd/display/replace_alloc.cocci| 25 + 62 files changed, 399 insertions(+), 348 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/replace_alloc.cocci -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 3/3] drm/panel: Add support for the Raspberry Pi 7" Touchscreen.
This driver communicates with the Atmel microcontroller for sequencing the poweron of the TC358762 DSI-DPI bridge and controlling the backlight PWM. v2: Set the same default orientation as the closed source firmware used, which is the best for viewing angle. v3: Rewrite as an i2c client driver after bridge driver rejection. v4: Finish probe without the DSI host, using the new delayed registration, and attach to the host during mipi_dsi_driver probe. v5: Rework to drop the "probe without DSI host" mode again, now that vc4 will create the host early on. Signed-off-by: Eric Anholt --- drivers/gpu/drm/panel/Kconfig | 8 + drivers/gpu/drm/panel/Makefile | 1 + .../gpu/drm/panel/panel-raspberrypi-touchscreen.c | 517 + 3 files changed, 526 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 718d8ce15b1f..726f3fb3312d 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -82,6 +82,14 @@ config DRM_PANEL_PANASONIC_VVX10F034N00 WUXGA (1920x1200) Novatek NT1397-based DSI panel as found in some Xperia Z2 tablets +config DRM_PANEL_RASPBERRYPI_TOUCHSCREEN + tristate "Raspberry Pi 7-inch touchscreen panel" + depends on DRM_MIPI_DSI + help + Say Y here if you want to enable support for the Raspberry + Pi 7" Touchscreen. To compile this driver as a module, + choose M here. + config DRM_PANEL_SAMSUNG_S6E3HA2 tristate "Samsung S6E3HA2 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index c8483fdd5b9b..77ede3467324 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o +obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c new file mode 100644 index ..4237a0cbcdcc --- /dev/null +++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c @@ -0,0 +1,517 @@ +/* + * Copyright © 2016-2017 Broadcom + * + * 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. + * + * Portions of this file (derived from panel-simple.c) are: + * + * Copyright (C) 2013, NVIDIA Corporation. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sub license, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the + * next paragraph) shall be included in all copies or substantial portions + * of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +/** + * Raspberry Pi 7" touchscreen panel driver. + * + * The 7" touchscreen consists of a DPI LCD panel, a Toshiba + * TC358762XBG DSI-DPI bridge, and an I2C-connected Atmel ATTINY88-MUR + * controlling power management, the LCD PWM, and initial register + * setup of the Tohsiba. + * + * This driver controls the TC358762 and ATTINY88, presenting a DSI + * device with a drm_panel. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#define RPI_DSI_DRIVER_NAME "rpi-ts-dsi" + +/* I2C registers of the Atmel microcontroller. */ +enum REG_ADDR { + REG_ID = 0x80, + REG_PORTA, /* BIT(2) for horizontal flip,
[PATCH v6 2/3] dt-bindings: Document the Raspberry Pi Touchscreen nodes.
This doesn't yet cover input, but the driver does get the display working when the firmware is disabled from talking to our I2C lines. Signed-off-by: Eric Anholt Acked-by: Rob Herring --- .../panel/raspberrypi,7inch-touchscreen.txt| 49 ++ 1 file changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.txt diff --git a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.txt b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.txt new file mode 100644 index ..e9e19c059260 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.txt @@ -0,0 +1,49 @@ +This binding covers the official 7" (800x480) Raspberry Pi touchscreen +panel. + +This DSI panel contains: + +- TC358762 DSI->DPI bridge +- Atmel microcontroller on I2C for power sequencing the DSI bridge and + controlling backlight +- Touchscreen controller on I2C for touch input + +and this binding covers the DSI display parts but not its touch input. + +Required properties: +- compatible: Must be "raspberrypi,7inch-touchscreen-panel" +- reg: Must be "45" +- port:See panel-common.txt + +Example: + +dsi1: dsi@7e70 { + #address-cells = <1>; + #size-cells = <0>; + <...> + + port { + dsi_out_port: endpoint { + remote-endpoint = <&panel_dsi_port>; + }; + }; +}; + +i2c_dsi: i2c { + compatible = "i2c-gpio"; + #address-cells = <1>; + #size-cells = <0>; + gpios = <&gpio 28 0 +&gpio 29 0>; + + lcd@45 { + compatible = "raspberrypi,7inch-touchscreen-panel"; + reg = <0x45>; + + port { + panel_dsi_port: endpoint { + remote-endpoint = <&dsi_out_port>; + }; + }; + }; +}; -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 0/3] RPi touchscreen panel resend.
I pushed a couple of the patches from the series, so here's what's left. I figured it was time for a resend at this point. Eric Anholt (3): drm/vc4: Move the DSI clock divider workaround closer to the clock call. dt-bindings: Document the Raspberry Pi Touchscreen nodes. drm/panel: Add support for the Raspberry Pi 7" Touchscreen. .../panel/raspberrypi,7inch-touchscreen.txt| 49 ++ drivers/gpu/drm/panel/Kconfig | 8 + drivers/gpu/drm/panel/Makefile | 1 + .../gpu/drm/panel/panel-raspberrypi-touchscreen.c | 517 + drivers/gpu/drm/vc4/vc4_dsi.c | 12 +- 5 files changed, 581 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.txt create mode 100644 drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 1/3] drm/vc4: Move the DSI clock divider workaround closer to the clock call.
We want the adjusted_mode->clock to be the actual clock we're expecting to program, so that consumers see the right values for clock and vrefresh. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_dsi.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 925c726ac694..554605af344e 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -859,11 +859,7 @@ static bool vc4_dsi_encoder_mode_fixup(struct drm_encoder *encoder, pll_clock = parent_rate / divider; pixel_clock_hz = pll_clock / dsi->divider; - /* Round up the clk_set_rate() request slightly, since -* PLLD_DSI1 is an integer divider and its rate selection will -* never round up. -*/ - adjusted_mode->clock = pixel_clock_hz / 1000 + 1; + adjusted_mode->clock = pixel_clock_hz / 1000; /* Given the new pixel clock, adjust HFP to keep vrefresh the same. */ adjusted_mode->htotal = adjusted_mode->clock * mode->htotal / @@ -901,7 +897,11 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) vc4_dsi_dump_regs(dsi); } - phy_clock = pixel_clock_hz * dsi->divider; + /* Round up the clk_set_rate() request slightly, since +* PLLD_DSI1 is an integer divider and its rate selection will +* never round up. +*/ + phy_clock = (pixel_clock_hz + 1000) * dsi->divider; ret = clk_set_rate(dsi->pll_phy_clock, phy_clock); if (ret) { dev_err(&dsi->pdev->dev, -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vc4: Fix pitch setup for T-format scanout.
The documentation said to use src_w here, and I didn't consider that we actually needed to be using pitch somewhere in our setup. Fixes scanout on my DSI panel when X11 does initial setup with 1920x1080 HDMI and 800x480 DSI both at 0,0 of the same framebuffer. Signed-off-by: Eric Anholt Fixes: 98830d91da08 ("drm/vc4: Add T-format scanout support.") --- drivers/gpu/drm/vc4/vc4_plane.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 2968b3ebb895..4ad0b9fcae99 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -547,14 +547,24 @@ static int vc4_plane_mode_set(struct drm_plane *plane, tiling = SCALER_CTL0_TILING_LINEAR; pitch0 = VC4_SET_FIELD(fb->pitches[0], SCALER_SRC_PITCH); break; - case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED: + + case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED: { + /* For T-tiled, the FB pitch is "how many bytes from +* one row to the next, such that pitch * tile_h == +* tile_size * tiles_per_row." +*/ + u32 tile_size_shift = 12; + u32 tile_h_shift = 5; + u32 tiles_w = fb->pitches[0] >> (tile_size_shift - tile_h_shift); + tiling = SCALER_CTL0_TILING_256B_OR_T; - pitch0 = (VC4_SET_FIELD(0, SCALER_PITCH0_TILE_Y_OFFSET), - VC4_SET_FIELD(0, SCALER_PITCH0_TILE_WIDTH_L), - VC4_SET_FIELD((vc4_state->src_w[0] + 31) >> 5, - SCALER_PITCH0_TILE_WIDTH_R)); + pitch0 = (VC4_SET_FIELD(0, SCALER_PITCH0_TILE_Y_OFFSET) | + VC4_SET_FIELD(0, SCALER_PITCH0_TILE_WIDTH_L) | + VC4_SET_FIELD(tiles_w, SCALER_PITCH0_TILE_WIDTH_R)); break; + } + default: DRM_DEBUG_KMS("Unsupported FB tiling flag 0x%16llx", (long long)fb->modifier); -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 4/6] drm_hwcomposer: Add FENCE_OUT_PTR property to DrmCrtc
On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss wrote: > Add support for handling the FENCE_OUT_PTR property to DrmCrtc > > Signed-off-by: Robert Foss Reviewed-by: Sean Paul > --- > drmcrtc.cpp | 10 ++ > drmcrtc.h | 2 ++ > 2 files changed, 12 insertions(+) > > diff --git a/drmcrtc.cpp b/drmcrtc.cpp > index 1fbdc12..c139869 100644 > --- a/drmcrtc.cpp > +++ b/drmcrtc.cpp > @@ -51,6 +51,12 @@ int DrmCrtc::Init() { > ALOGE("Failed to get MODE_ID property"); > return ret; >} > + > + ret = drm_->GetCrtcProperty(*this, "OUT_FENCE_PTR", > &out_fence_ptr_property_); > + if (ret) { > +ALOGE("Failed to get OUT_FENCE_PTR property"); > +return ret; > + } >return 0; > } > > @@ -81,4 +87,8 @@ const DrmProperty &DrmCrtc::active_property() const { > const DrmProperty &DrmCrtc::mode_property() const { >return mode_property_; > } > + > +const DrmProperty &DrmCrtc::out_fence_ptr_property() const { > + return out_fence_ptr_property_; > +} > } > diff --git a/drmcrtc.h b/drmcrtc.h > index ad95352..2e8c811 100644 > --- a/drmcrtc.h > +++ b/drmcrtc.h > @@ -45,6 +45,7 @@ class DrmCrtc { > >const DrmProperty &active_property() const; >const DrmProperty &mode_property() const; > + const DrmProperty &out_fence_ptr_property() const; > > private: >DrmResources *drm_; > @@ -63,6 +64,7 @@ class DrmCrtc { > >DrmProperty active_property_; >DrmProperty mode_property_; > + DrmProperty out_fence_ptr_property_; > }; > } > > -- > 2.11.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 2/6] drm_hwcomposer: Add support for IN_FENCE_FD property to DrmPlane
On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss wrote: > Add support for the IN_FENCE_FD property to DrmPlane. > > Signed-off-by: Robert Foss Reviewed-by: Sean Paul > --- > drmplane.cpp | 8 > drmplane.h | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/drmplane.cpp b/drmplane.cpp > index c4ea722..1f739ae 100644 > --- a/drmplane.cpp > +++ b/drmplane.cpp > @@ -126,6 +126,10 @@ int DrmPlane::Init() { >if (ret) > ALOGI("Could not get alpha property"); > > + ret = drm_->GetPlaneProperty(*this, "IN_FENCE_FD", &in_fence_fd_property_); > + if (ret) > +ALOGI("Could not get IN_FENCE_FD property"); > + >return 0; > } > > @@ -188,4 +192,8 @@ const DrmProperty &DrmPlane::rotation_property() const { > const DrmProperty &DrmPlane::alpha_property() const { >return alpha_property_; > } > + > +const DrmProperty &DrmPlane::in_fence_fd_property() const { > + return in_fence_fd_property_; > +} > } > diff --git a/drmplane.h b/drmplane.h > index 2e06986..5b73b08 100644 > --- a/drmplane.h > +++ b/drmplane.h > @@ -54,6 +54,7 @@ class DrmPlane { >const DrmProperty &src_h_property() const; >const DrmProperty &rotation_property() const; >const DrmProperty &alpha_property() const; > + const DrmProperty &in_fence_fd_property() const; > > private: >DrmResources *drm_; > @@ -75,6 +76,7 @@ class DrmPlane { >DrmProperty src_h_property_; >DrmProperty rotation_property_; >DrmProperty alpha_property_; > + DrmProperty in_fence_fd_property_; > }; > } > > -- > 2.11.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading
On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss wrote: > From: Sean Paul > > Since HWC2 doesn't require the use of threads to implement correct > synchronization, remove some of these threads. > My SoB seems to have been dropped (or perhaps I just forgot to add it in the original thread). At any rate, here you go: Signed-off-by: Sean Paul > Signed-off-by: Robert Foss > --- > Android.mk| 3 - > drmcomposition.cpp| 166 > drmcomposition.h | 79 --- > drmcompositor.cpp | 106 -- > drmcompositor.h | 56 -- > drmcompositorworker.h | 41 -- > drmdisplaycomposition.cpp | 1 + > drmdisplaycomposition.h | 10 +++ > drmdisplaycompositor.cpp | 189 > -- > drmdisplaycompositor.h| 36 + > drmeventlistener.cpp | 3 + > drmhwctwo.cpp | 6 +- > drmresources.cpp | 54 + > drmresources.h| 5 -- > glworker.cpp | 52 +++-- > glworker.h| 10 +++ > 16 files changed, 93 insertions(+), 724 deletions(-) > delete mode 100644 drmcomposition.cpp > delete mode 100644 drmcomposition.h > delete mode 100644 drmcompositor.cpp > delete mode 100644 drmcompositor.h > delete mode 100644 drmcompositorworker.h > > diff --git a/Android.mk b/Android.mk > index aa95b44..5d16c2f 100644 > --- a/Android.mk > +++ b/Android.mk > @@ -57,9 +57,6 @@ LOCAL_C_INCLUDES := \ > LOCAL_SRC_FILES := \ > autolock.cpp \ > drmresources.cpp \ > - drmcomposition.cpp \ > - drmcompositor.cpp \ > - drmcompositorworker.cpp \ > drmconnector.cpp \ > drmcrtc.cpp \ > drmdisplaycomposition.cpp \ > diff --git a/drmcomposition.cpp b/drmcomposition.cpp > deleted file mode 100644 > index 1aaf920..000 > --- a/drmcomposition.cpp > +++ /dev/null > @@ -1,166 +0,0 @@ > -/* > - * Copyright (C) 2015 The Android Open Source Project > - * > - * Licensed under the Apache License, Version 2.0 (the "License"); > - * you may not use this file except in compliance with the License. > - * You may obtain a copy of the License at > - * > - * http://www.apache.org/licenses/LICENSE-2.0 > - * > - * Unless required by applicable law or agreed to in writing, software > - * distributed under the License is distributed on an "AS IS" BASIS, > - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > - * See the License for the specific language governing permissions and > - * limitations under the License. > - */ > - > -#define LOG_TAG "hwc-drm-composition" > - > -#include "drmcomposition.h" > -#include "drmcrtc.h" > -#include "drmplane.h" > -#include "drmresources.h" > -#include "platform.h" > - > -#include > - > -#include > -#include > -#include > -#include > - > -namespace android { > - > -DrmComposition::DrmComposition(DrmResources *drm, Importer *importer, > - Planner *planner) > -: drm_(drm), importer_(importer), planner_(planner) { > - char use_overlay_planes_prop[PROPERTY_VALUE_MAX]; > - property_get("hwc.drm.use_overlay_planes", use_overlay_planes_prop, "1"); > - bool use_overlay_planes = atoi(use_overlay_planes_prop); > - > - for (auto &plane : drm->planes()) { > -if (plane->type() == DRM_PLANE_TYPE_PRIMARY) > - primary_planes_.push_back(plane.get()); > -else if (use_overlay_planes && plane->type() == DRM_PLANE_TYPE_OVERLAY) > - overlay_planes_.push_back(plane.get()); > - } > -} > - > -int DrmComposition::Init(uint64_t frame_no) { > - for (auto &conn : drm_->connectors()) { > -int display = conn->display(); > -composition_map_[display].reset(new DrmDisplayComposition()); > -if (!composition_map_[display]) { > - ALOGE("Failed to allocate new display composition\n"); > - return -ENOMEM; > -} > - > -// If the display hasn't been modeset yet, this will be NULL > -DrmCrtc *crtc = drm_->GetCrtcForDisplay(display); > - > -int ret = composition_map_[display]->Init(drm_, crtc, importer_, > planner_, > - frame_no); > -if (ret) { > - ALOGE("Failed to init display composition for %d", display); > - return ret; > -} > - } > - return 0; > -} > - > -int DrmComposition::SetLayers(size_t num_displays, > - DrmCompositionDisplayLayersMap *maps) { > - int ret = 0; > - for (size_t display_index = 0; display_index < num_displays; > - display_index++) { > -DrmCompositionDisplayLayersMap &map = maps[display_index]; > -int display = map.display; > - > -if (!drm_->GetConnectorForDisplay(display)) { > - ALOGE("Invalid display given to SetLayers %d", display); > - continue; > -} > - > -ret = composition_map_[display]->SetLayers( > -map.layers.data(), map.layers.size(), map.
Re: [PATCH hwc v2 5/6] drm_hwcomposer: Add GetCrtcCount function
On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss wrote: > This GetCrtrcCount helper functions enables convenient > fetching of the number of Crtcs from DrmResources. > > Signed-off-by: Robert Foss > --- > drmresources.cpp | 4 > drmresources.h | 1 + > 2 files changed, 5 insertions(+) > > diff --git a/drmresources.cpp b/drmresources.cpp > index 762f5ef..0578cc6 100644 > --- a/drmresources.cpp > +++ b/drmresources.cpp > @@ -241,6 +241,10 @@ DrmPlane *DrmResources::GetPlane(uint32_t id) const { >return NULL; > } > > +uint32_t DrmResources::GetCrtcCount() const { > + return (uint32_t) crtcs_.size(); > +} The "blessed" way of doing this would be to add a new function const std::vector> &crtcs() const { return crtcs_; } and then use crtcs()->size() wherever needed. > + > uint32_t DrmResources::next_mode_id() { >return ++mode_id_; > } > diff --git a/drmresources.h b/drmresources.h > index a2d8d16..0cc2456 100644 > --- a/drmresources.h > +++ b/drmresources.h > @@ -66,6 +66,7 @@ class DrmResources { >int GetConnectorProperty(const DrmConnector &connector, const char > *prop_name, > DrmProperty *property); > > + uint32_t GetCrtcCount() const; >uint32_t next_mode_id(); > >int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id); > -- > 2.11.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 6/6] drm_hwcomposer: Add out-fence support
On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss wrote: > Add support for out-fences through the OUT_FENCE_PTR property. > Out-fences signal when their associated buffer may be read by a device. > > Signed-off-by: Robert Foss > --- > > Changes since v1: > Sergi Granell > - Set atomic property to be out_fences[crtc->pipe()] not out_fences[0] > > drmdisplaycomposition.h | 9 + > drmdisplaycompositor.cpp | 16 > drmhwctwo.cpp| 9 ++--- > 3 files changed, 27 insertions(+), 7 deletions(-) > > diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h > index b165adc..0586d58 100644 > --- a/drmdisplaycomposition.h > +++ b/drmdisplaycomposition.h > @@ -189,6 +189,14 @@ class DrmDisplayComposition { > return planner_; >} > > + int take_out_fence() { > +return out_fence_.Release(); > + } > + > + void set_out_fence(int out_fence) { > +out_fence_.Set(dup(out_fence)); Why dup if you're just going to close the original? I think the helper functions actually hurt you here. It would be easier to understand what was going on if you just manipulated out_fence_ directly in CommitFrame (then you wouldn't need the dup/close). > + } > + >void Dump(std::ostringstream *out) const; > > private: > @@ -215,6 +223,7 @@ class DrmDisplayComposition { >int timeline_current_ = 0; >int timeline_squash_done_ = 0; >int timeline_pre_comp_done_ = 0; > + UniqueFd out_fence_ = -1; > >bool geometry_changed_; >std::vector layers_; > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp > index 71c0451..a1427d3 100644 > --- a/drmdisplaycompositor.cpp > +++ b/drmdisplaycompositor.cpp > @@ -492,6 +492,7 @@ int > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, >display_comp->composition_planes(); >std::vector &pre_comp_regions = >display_comp->pre_comp_regions(); > + uint64_t out_fences[drm_->GetCrtcCount()]; Huh. I didn't know you could do this. > >DrmConnector *connector = drm_->GetConnectorForDisplay(display_); >if (!connector) { > @@ -510,6 +511,16 @@ int > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, > return -ENOMEM; >} > > + if (crtc->out_fence_ptr_property().id() != 0) { > +ret = drmModeAtomicAddProperty(pset, crtc->id(), > crtc->out_fence_ptr_property().id(), > + (uint64_t) &out_fences[crtc->pipe()]); > +if (ret < 0) { > + ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret); > + drmModeAtomicFree(pset); > + return ret; > +} > + } > + >if (mode_.needs_modeset) { > ret = drmModeAtomicAddProperty(pset, crtc->id(), > crtc->mode_property().id(), > mode_.blob_id) < 0 || > @@ -708,6 +719,11 @@ out: > mode_.needs_modeset = false; >} > > + if (crtc->out_fence_ptr_property().id()) { > +display_comp->set_out_fence((int) out_fences[crtc->pipe()]); > +close((int) out_fences[crtc->pipe()]); > + } > + >return ret; > } > > diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp > index 762ee8c..89399bf 100644 > --- a/drmhwctwo.cpp > +++ b/drmhwctwo.cpp > @@ -557,19 +557,14 @@ HWC2::Error > DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { > i = overlay_planes.erase(i); >} > > + AddFenceToRetireFence(composition->take_out_fence()); > + >ret = compositor_.ApplyComposition(std::move(composition)); >if (ret) { > ALOGE("Failed to apply the frame composition ret=%d", ret); > return HWC2::Error::BadParameter; >} > > - // Now that the release fences have been generated by the compositor, make > - // sure they're managed properly > - for (std::pair &l : z_map) { > -l.second->manage_release_fence(); > -AddFenceToRetireFence(l.second->release_fence()); > - } > - >// The retire fence returned here is for the last frame, so return it and >// promote the next retire fence >*retire_fence = retire_fence_.Release(); > -- > 2.11.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 3/6] drm_hwcomposer: Submit in-fence to DRM
Hey Sean, On Wed, 2017-09-27 at 14:55 -0400, Sean Paul wrote: > On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss om> wrote: > > Add support for in-fences through the IN_FENCE_FD property. In- > > fences signal > > when their associated buffer may be read by DRM/KMS. > > > > Signed-off-by: Robert Foss > > --- > > drmdisplaycompositor.cpp | 35 --- > > 1 file changed, 16 insertions(+), 19 deletions(-) > > > > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp > > index bd670cf..71c0451 100644 > > --- a/drmdisplaycompositor.cpp > > +++ b/drmdisplaycompositor.cpp > > @@ -529,6 +529,7 @@ int > > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition > > *display_comp, > > std::vector &source_layers = > > comp_plane.source_layers(); > > > > int fb_id = -1; > > +int fence_fd = -1; > > DrmHwcRect display_frame; > > DrmHwcRect source_crop; > > uint64_t rotation = 0; > > @@ -547,30 +548,12 @@ int > > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition > > *display_comp, > > break; > > } > > DrmHwcLayer &layer = layers[source_layers.front()]; > > - if (!test_only && layer.acquire_fence.get() >= 0) { > > -int acquire_fence = layer.acquire_fence.get(); > > -int total_fence_timeout = 0; > > -for (int i = 0; i < kAcquireWaitTries; ++i) { > > - int fence_timeout = kAcquireWaitTimeoutMs * (1 << i); > > - total_fence_timeout += fence_timeout; > > - ret = sync_wait(acquire_fence, fence_timeout); > > - if (ret) > > -ALOGW("Acquire fence %d wait %d failed (%d). Total > > time %d", > > - acquire_fence, i, ret, total_fence_timeout); > > - else > > -break; > > -} > > -if (ret) { > > - ALOGE("Failed to wait for acquire %d/%d", acquire_fence, > > ret); > > - break; > > -} > > -layer.acquire_fence.Close(); > > - } > > if (!layer.buffer) { > > ALOGE("Expected a valid framebuffer for pset"); > > break; > > } > > fb_id = layer.buffer->fb_id; > > + fence_fd = layer.acquire_fence.get(); > > display_frame = layer.display_frame; > > source_crop = layer.source_crop; > > if (layer.blending == DrmHwcBlending::kPreMult) > > @@ -587,7 +570,21 @@ int > > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition > > *display_comp, > > rotation |= 1 << DRM_ROTATE_180; > > else if (layer.transform & DrmHwcTransform::kRotate270) > > rotation |= 1 << DRM_ROTATE_270; > > + > > + if (fence_fd != -1) { > > nit: if (fence_fd < 0) > > With that fixed: > > Reviewed-by: Sean Paul Ack. Submitting a fix in v3 tomorrow. > > > +int prop_id = plane->in_fence_fd_property().id(); > > +if (prop_id == 0) { > > +ALOGE("Failed to get IN_FENCE_FD property id"); > > +break; > > +} > > +ret = drmModeAtomicAddProperty(pset, plane->id(), prop_id, > > fence_fd); > > +if (ret < 0) { > > + ALOGE("Failed to add IN_FENCE_FD property to pset: %d", > > ret); > > + break; > > +} > > + } > > } > > + > > // Disable the plane if there's no framebuffer > > if (fb_id < 0) { > > ret = drmModeAtomicAddProperty(pset, plane->id(), > > -- > > 2.11.0 > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm: rcar-du: Reject planes located fully off-screen
Hi Laurent, Thankyou for the patch, This looks good, and passes the tests. On 16/08/17 00:03, Laurent Pinchart wrote: > There is no point in accepting fully off-screen planes as they won't be > displayed. Reject them in the atomic check. > > Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham > --- > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 16 > 1 file changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c > b/drivers/gpu/drm/rcar-du/rcar_du_plane.c > index 4f076c364f25..714e02960635 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c > @@ -569,6 +569,8 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane, >struct drm_plane_state *state, >const struct rcar_du_format_info **format) > { > + const struct drm_display_mode *mode; > + struct drm_crtc_state *crtc_state; > struct drm_device *dev = plane->dev; > > if (!state->fb || !state->crtc) { > @@ -582,6 +584,20 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane, > return -EINVAL; > } > > + crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc); > + if (IS_ERR(crtc_state)) > + return PTR_ERR(crtc_state); > + > + mode = &crtc_state->adjusted_mode; > + if (state->crtc_x >= mode->hdisplay || > + state->crtc_y >= mode->vdisplay || > + state->crtc_x + (int)state->crtc_w <= 0 || > + state->crtc_y + (int)state->crtc_h <= 0) { > + dev_dbg(dev->dev, "%s: plane can't be fully off-screen\n", > + __func__); > + return -EINVAL; > + } > + > *format = rcar_du_format_info(state->fb->format->format); > if (*format == NULL) { > dev_dbg(dev->dev, "%s: unsupported format %08x\n", __func__, > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm: rcar-du: Clip planes to screen boundaries
Hi Laurent, Thanks for the patch, On 16/08/17 00:03, Laurent Pinchart wrote: > Unlike the KMS API, the hardware doesn't support planes exceeding the > screen boundaries. Clip plane coordinates to support the use case. > > Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham > --- > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 66 > + > drivers/gpu/drm/rcar-du/rcar_du_plane.h | 4 ++ > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 21 +++ > 3 files changed, 67 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c > b/drivers/gpu/drm/rcar-du/rcar_du_plane.c > index 714e02960635..7944790bac25 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > > #include "rcar_du_drv.h" > #include "rcar_du_group.h" > @@ -329,11 +330,39 @@ static void rcar_du_plane_write(struct rcar_du_group > *rgrp, > data); > } > > +void rcar_du_plane_clip(const struct drm_plane_state *state, > + struct drm_rect *src, struct drm_rect *dst) > +{ > + const struct drm_display_mode *mode; > + > + /* > + * The hardware requires all planes to be fully contained in the output > + * rectangle. Crop the destination rectangle to fit in the CRTC. > + */ > + mode = &state->crtc->state->adjusted_mode; > + > + dst->x1 = max(0, state->crtc_x); > + dst->y1 = max(0, state->crtc_y); > + dst->x2 = min_t(unsigned int, mode->hdisplay, > + state->crtc_x + state->crtc_w); > + dst->y2 = min_t(unsigned int, mode->vdisplay, > + state->crtc_y + state->crtc_h); > + > + /* > + * Offset the left and top source coordinates by the same displacement > + * we have applied to the destination rectangle. Copy the width and > + * height as the hardware can't scale. > + */ > + src->x1 = (state->src_x >> 16) + (dst->x1 - state->crtc_x); > + src->y1 = (state->src_y >> 16) + (dst->y1 - state->crtc_y); I had to look up why we do a >> 16 here. I guess this is for sub-pixel positioning. That might be obvious to DRM developers, but was new to me. Probably not worth a comment though - as I know what it is now :) Do we have any macros to make this 16.16 -> int conversion more explicit? (I think this comes under the same category as using BIT() to which we differ on opinion anyway though) > + src->x2 = src->x1 + (dst->x2 - dst->x1); > + src->y2 = src->y1 + (dst->y2 - dst->y1); > +} > + > static void rcar_du_plane_setup_scanout(struct rcar_du_group *rgrp, > - const struct rcar_du_plane_state *state) > + const struct rcar_du_plane_state *state, > + const struct drm_rect *src) > { > - unsigned int src_x = state->state.src_x >> 16; > - unsigned int src_y = state->state.src_y >> 16; > unsigned int index = state->hwindex; > unsigned int pitch; > bool interlaced; > @@ -357,7 +386,7 @@ static void rcar_du_plane_setup_scanout(struct > rcar_du_group *rgrp, > dma[i] = gem->paddr + fb->offsets[i]; > } > } else { > - pitch = state->state.src_w >> 16; > + pitch = drm_rect_width(&state->state.src); > dma[0] = 0; > dma[1] = 0; > } > @@ -383,8 +412,8 @@ static void rcar_du_plane_setup_scanout(struct > rcar_du_group *rgrp, >* require a halved Y position value, in both progressive and interlaced >* modes. >*/ > - rcar_du_plane_write(rgrp, index, PnSPXR, src_x); > - rcar_du_plane_write(rgrp, index, PnSPYR, src_y * > + rcar_du_plane_write(rgrp, index, PnSPXR, src->x1); > + rcar_du_plane_write(rgrp, index, PnSPYR, src->y1 * > (!interlaced && state->format->bpp == 32 ? 2 : 1)); > > rcar_du_plane_write(rgrp, index, PnDSA0R, dma[0]); > @@ -394,8 +423,8 @@ static void rcar_du_plane_setup_scanout(struct > rcar_du_group *rgrp, > > rcar_du_plane_write(rgrp, index, PnMWR, pitch); > > - rcar_du_plane_write(rgrp, index, PnSPXR, src_x); > - rcar_du_plane_write(rgrp, index, PnSPYR, src_y * > + rcar_du_plane_write(rgrp, index, PnSPXR, src->x1); > + rcar_du_plane_write(rgrp, index, PnSPYR, src->y1 * > (state->format->bpp == 16 ? 2 : 1) / 2); > > rcar_du_plane_write(rgrp, index, PnDSA0R, dma[1]); > @@ -518,7 +547,8 @@ static void rcar_du_plane_setup_format_gen3(struct > rcar_du_group *rgrp, > > static void rcar_du_plane_setup_format(struct rcar_du_group *rgrp, > unsigned int index, > -const struct rcar_du_plane_state *state) > +
Re: [PATCH hwc v1] drm_hwcomposer: Add CONTRIBUTING file
Hey Emil, On Wed, 2017-09-27 at 15:42 +0100, Emil Velikov wrote: > Hi Rob, > > Glad to see this. There's a couple of suggestions that I hope you'll > find worth while. > > On 22 September 2017 at 01:37, Robert Foss > wrote: > > Some basic guidelines for contributions could come in handy. > > > > These are copied from IGT and modified to be suitable. > > > > Signed-off-by: Robert Foss > > --- > > CONTRIBUTING | 31 +++ > > 1 file changed, 31 insertions(+) > > create mode 100644 CONTRIBUTING > > > > diff --git a/CONTRIBUTING b/CONTRIBUTING > > new file mode 100644 > > index 000..f1b4775 > > --- /dev/null > > +++ b/CONTRIBUTING > > @@ -0,0 +1,31 @@ > > +Patches to drm_hwcomposer are very much welcome, we really want > > this to be the > > +universal HW composer implementation for Android and similar > > platforms > > +So please bring on porting patches, bugfixes, improvements for > > documentation > > +and new features. > > + > > +A short list of contribution guidelines: > > + > > +- Please submit patches formatted with git send-email/git format- > > patch or > > + equivalent to > > + > > +dri-devel > > + > > + Please use --subject-prefix="PATCH hwc" so that drm_hwcomposer > > patches are easily > > + identified in the massive amount mails on dri-devel. To ensure > > this is always > > + done, run: > > + > > +git config format.subjectprefix "PATCH hwc" > > + > > One can add this into the autogen.sh or whatever bootstrap file the > project uses. > For example see https://cgit.freedesktop.org/mesa/drm/tree/autogen.sh Adding an autogen.sh sounds like a good idea to me. > > > > +- When submitting new code please follow the naming conventions > > documented > > + in the generated documentation. Also please make full use of all > > the helpers and > > + convenience macros provided by drm_hwcomposer. The below command > > can help you > > + with formatting of your patches: > > + git diff | clang-format-diff-3.5 -p 1 -style=file > > + > > One could wire that in the pre-merge hook on the server side. > It should be quite lightweight, although I'm not sure about the > integration with FDO's Debian(?) setup ;-) Yeah, me neither. Maybe asking Daniel Stone about how this should be done is the next step. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 3/6] drm_hwcomposer: Submit in-fence to DRM
On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss wrote: > Add support for in-fences through the IN_FENCE_FD property. In-fences signal > when their associated buffer may be read by DRM/KMS. > > Signed-off-by: Robert Foss > --- > drmdisplaycompositor.cpp | 35 --- > 1 file changed, 16 insertions(+), 19 deletions(-) > > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp > index bd670cf..71c0451 100644 > --- a/drmdisplaycompositor.cpp > +++ b/drmdisplaycompositor.cpp > @@ -529,6 +529,7 @@ int > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, > std::vector &source_layers = comp_plane.source_layers(); > > int fb_id = -1; > +int fence_fd = -1; > DrmHwcRect display_frame; > DrmHwcRect source_crop; > uint64_t rotation = 0; > @@ -547,30 +548,12 @@ int > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, > break; >} >DrmHwcLayer &layer = layers[source_layers.front()]; > - if (!test_only && layer.acquire_fence.get() >= 0) { > -int acquire_fence = layer.acquire_fence.get(); > -int total_fence_timeout = 0; > -for (int i = 0; i < kAcquireWaitTries; ++i) { > - int fence_timeout = kAcquireWaitTimeoutMs * (1 << i); > - total_fence_timeout += fence_timeout; > - ret = sync_wait(acquire_fence, fence_timeout); > - if (ret) > -ALOGW("Acquire fence %d wait %d failed (%d). Total time %d", > - acquire_fence, i, ret, total_fence_timeout); > - else > -break; > -} > -if (ret) { > - ALOGE("Failed to wait for acquire %d/%d", acquire_fence, ret); > - break; > -} > -layer.acquire_fence.Close(); > - } >if (!layer.buffer) { > ALOGE("Expected a valid framebuffer for pset"); > break; >} >fb_id = layer.buffer->fb_id; > + fence_fd = layer.acquire_fence.get(); >display_frame = layer.display_frame; >source_crop = layer.source_crop; >if (layer.blending == DrmHwcBlending::kPreMult) > @@ -587,7 +570,21 @@ int > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, > rotation |= 1 << DRM_ROTATE_180; >else if (layer.transform & DrmHwcTransform::kRotate270) > rotation |= 1 << DRM_ROTATE_270; > + > + if (fence_fd != -1) { nit: if (fence_fd < 0) With that fixed: Reviewed-by: Sean Paul > +int prop_id = plane->in_fence_fd_property().id(); > +if (prop_id == 0) { > +ALOGE("Failed to get IN_FENCE_FD property id"); > +break; > +} > +ret = drmModeAtomicAddProperty(pset, plane->id(), prop_id, fence_fd); > +if (ret < 0) { > + ALOGE("Failed to add IN_FENCE_FD property to pset: %d", ret); > + break; > +} > + } > } > + > // Disable the plane if there's no framebuffer > if (fb_id < 0) { >ret = drmModeAtomicAddProperty(pset, plane->id(), > -- > 2.11.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading
Hey Emil, On Wed, 2017-09-27 at 14:34 +0100, Emil Velikov wrote: > On 27 September 2017 at 12:58, Robert Foss > wrote: > > > 16 files changed, 93 insertions(+), 724 deletions(-) > > Holly smokes, that's some amazing stat. > Please sir can I have some more ;-) > > Question - this patch removes the threading implementation, while the > actual substitute lands with patches 2-6. > Did I get this right? > > If so, ideally this patch will be the final in the series. > Guess it doesn't matter too much though. Ack. I'll submit v3 with this change tomorrow. Thanks! > > -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: Add the DRM_IOCTL_VC4_GEM_MADVISE ioctl
Boris Brezillon writes: > This ioctl will allow us to purge inactive userspace buffers when the > system is running out of contiguous memory. > > For now, the purge logic is rather dumb in that it does not try to > release only the amount of BO needed to meet the last CMA alloc request > but instead purges all objects placed in the purgeable pool as soon as > we experience a CMA allocation failure. > > Note that the in-kernel BO cache is always purged before the purgeable > cache because those objects are known to be unused while objects marked > as purgeable by a userspace application/library might have to be > restored when they are marked back as unpurgeable, which can be > expensive. > > Signed-off-by: Boris Brezillon > --- > Hello, > > Updates to libdrm, mesa and igt making use of this kernel feature can > be found on my github repos [1][2][3]. > > There's currently no debugfs hook to manually force a purge, but this > is being discussed and will probably be added in v2. > > Regards, > diff --git a/include/uapi/drm/vc4_drm.h b/include/uapi/drm/vc4_drm.h > index afae87004963..c01b93d453db 100644 > --- a/include/uapi/drm/vc4_drm.h > +++ b/include/uapi/drm/vc4_drm.h > @@ -41,6 +41,7 @@ extern "C" { > #define DRM_VC4_SET_TILING0x08 > #define DRM_VC4_GET_TILING0x09 > #define DRM_VC4_LABEL_BO 0x0a > +#define DRM_VC4_GEM_MADVISE 0x0b > > #define DRM_IOCTL_VC4_SUBMIT_CL DRM_IOWR(DRM_COMMAND_BASE + > DRM_VC4_SUBMIT_CL, struct drm_vc4_submit_cl) > #define DRM_IOCTL_VC4_WAIT_SEQNO DRM_IOWR(DRM_COMMAND_BASE + > DRM_VC4_WAIT_SEQNO, struct drm_vc4_wait_seqno) > @@ -53,6 +54,7 @@ extern "C" { > #define DRM_IOCTL_VC4_SET_TILING DRM_IOWR(DRM_COMMAND_BASE + > DRM_VC4_SET_TILING, struct drm_vc4_set_tiling) > #define DRM_IOCTL_VC4_GET_TILING DRM_IOWR(DRM_COMMAND_BASE + > DRM_VC4_GET_TILING, struct drm_vc4_get_tiling) > #define DRM_IOCTL_VC4_LABEL_BODRM_IOWR(DRM_COMMAND_BASE + > DRM_VC4_LABEL_BO, struct drm_vc4_label_bo) > +#define DRM_IOCTL_VC4_GEM_MADVISE DRM_IOWR(DRM_COMMAND_BASE + > DRM_VC4_GEM_MADVISE, struct drm_vc4_gem_madvise) > > struct drm_vc4_submit_rcl_surface { > __u32 hindex; /* Handle index, or ~0 if not present. */ > @@ -333,6 +335,16 @@ struct drm_vc4_label_bo { > __u64 name; > }; > > +#define VC4_MADV_WILLNEED0 > +#define VC4_MADV_DONTNEED1 > +#define __VC4_MADV_PURGED2 > + > +struct drm_vc4_gem_madvise { > + __u32 handle; > + __u32 madv; > + __u32 retained; > +}; danvet had a note in http://blog.ffwll.ch/2013/11/botching-up-ioctls.html: Pad the entire struct to a multiple of 64bits - the structure size will otherwise differ on 32bit versus 64bit. Which hurts when passing arrays of structures to the kernel. Or with the ioctl structure size checking that e.g. the drm core does. I'm surprised that i915's ioctl didn't do this or have compat code to handle it. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd: DC pull request review
On 2017-09-27 12:48 PM, Daniel Vetter wrote: > On Wed, Sep 27, 2017 at 6:38 PM, Sean Paul wrote: >> Any chance we can address the i2c/gpio [re-]implementations as well? > > It's already on the list. Part of this is code that's probably dead, > the other is a bit too much layer cake still left, and the final bits > are not fully using drm helpers for edid parsing and tons of other > stuff. But afaics all the i2c stuff does go through the i2c_adapter > abstraction in a proper fashion, which is at least the foundational > work. The other bits should be all captured in the todo already. There might still be some dead code around i2c stuff. I'll take a look and see what I can rip out. If there's any specific function, struct, etc. that's of concern let me know as well. Harry > -Daniel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/8] drm/rockchip/dsi: correct phy parameter setting
Hi Nickey, El Tue, Sep 26, 2017 at 03:55:19PM +0800 Nickey Yang ha dit: > As MIPI PHY document show, icpctrl<3..0> and lpfctrl<5..0> > should depend on frequency,so fix it. > > Signed-off-by: Nickey Yang > --- > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 98 > -- > 1 file changed, 70 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > index 191037c..20d3f36 100644 > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > @@ -267,10 +267,21 @@ > #define VCO_IN_CAP_CON_HIGH (0x2 << 1) > #define REF_BIAS_CUR_SEL BIT(0) > > -#define CP_CURRENT_3MA BIT(3) > +#define CP_CURRENT_1_5UA 0x1 > +#define CP_CURRENT_4_5UA 0x2 > +#define CP_CURRENT_7_5UA 0x6 > +#define CP_CURRENT_6UA 0x9 > +#define CP_CURRENT_12UA 0xb > +#define CP_CURRENT_SEL(val) ((val) & 0xf) > #define CP_PROGRAM_ENBIT(7) > + > +#define LPF_RESISTORS_15_5KOHM 0x1 > +#define LPF_RESISTORS_13KOHM 0x2 > +#define LPF_RESISTORS_11_5KOHM 0x4 > +#define LPF_RESISTORS_10_5KOHM 0x8 > +#define LPF_RESISTORS_8KOHM 0x10 > #define LPF_PROGRAM_EN BIT(6) > -#define LPF_RESISTORS_20_KOHM0 > +#define LPF_RESISTORS_SEL(val) ((val) & 0x3f) > > #define HSFREQRANGE_SEL(val) (((val) & 0x3f) << 1) > > @@ -400,32 +411,63 @@ enum dw_mipi_dsi_mode { > DW_MIPI_DSI_VID_MODE, > }; > > -struct dphy_pll_testdin_map { > +struct dphy_pll_parameter_map { > unsigned int max_mbps; > - u8 testdin; > + u8 hsfreqrange; > + u8 icpctrl; > + u8 lpfctrl; > }; > > /* The table is based on 27MHz DPHY pll reference clock. */ > -static const struct dphy_pll_testdin_map dptdin_map[] = { > - { 90, 0x00}, { 100, 0x10}, { 110, 0x20}, { 130, 0x01}, > - { 140, 0x11}, { 150, 0x21}, { 170, 0x02}, { 180, 0x12}, > - { 200, 0x22}, { 220, 0x03}, { 240, 0x13}, { 250, 0x23}, > - { 270, 0x04}, { 300, 0x14}, { 330, 0x05}, { 360, 0x15}, > - { 400, 0x25}, { 450, 0x06}, { 500, 0x16}, { 550, 0x07}, > - { 600, 0x17}, { 650, 0x08}, { 700, 0x18}, { 750, 0x09}, > - { 800, 0x19}, { 850, 0x29}, { 900, 0x39}, { 950, 0x0a}, > - {1000, 0x1a}, {1050, 0x2a}, {1100, 0x3a}, {1150, 0x0b}, > - {1200, 0x1b}, {1250, 0x2b}, {1300, 0x3b}, {1350, 0x0c}, > - {1400, 0x1c}, {1450, 0x2c}, {1500, 0x3c} > +static const struct dphy_pll_parameter_map dppa_map[] = { > + { 90, 0x00, CP_CURRENT_1_5UA, LPF_RESISTORS_13KOHM}, max_mbps in this table is off by one. According to the databook the ranges for the date rate are: 80-89 90-99 ... 1450-1500 I think most people would interpret 'max_mbps' as the highest value of the range, not the first value outside of the range on the upper side. The code below 'fixes' this by using '>' instead of '>=' when looking up the configuration for a data rate: if (dppa_map[i].max_mbps > max_mbps) return i; Both the table and this check are confusing, just use the actual max value of the range and '>='. Also the current code wouldn't work with a max rate of 1500 Mbps, since (1500 > 1500) is false. > + { 100, 0x10, CP_CURRENT_1_5UA, LPF_RESISTORS_13KOHM}, > + { 110, 0x20, CP_CURRENT_1_5UA, LPF_RESISTORS_13KOHM}, > + { 130, 0x01, CP_CURRENT_1_5UA, LPF_RESISTORS_15_5KOHM}, > + { 140, 0x11, CP_CURRENT_1_5UA, LPF_RESISTORS_15_5KOHM}, > + { 150, 0x21, CP_CURRENT_1_5UA, LPF_RESISTORS_15_5KOHM}, > + { 170, 0x02, CP_CURRENT_6UA, LPF_RESISTORS_13KOHM}, > + { 180, 0x12, CP_CURRENT_6UA, LPF_RESISTORS_13KOHM}, > + { 200, 0x22, CP_CURRENT_6UA, LPF_RESISTORS_13KOHM}, > + { 220, 0x03, CP_CURRENT_4_5UA, LPF_RESISTORS_13KOHM}, > + { 240, 0x13, CP_CURRENT_4_5UA, LPF_RESISTORS_13KOHM}, > + { 250, 0x23, CP_CURRENT_4_5UA, LPF_RESISTORS_13KOHM}, > + { 270, 0x04, CP_CURRENT_6UA, LPF_RESISTORS_11_5KOHM}, > + { 300, 0x14, CP_CURRENT_6UA, LPF_RESISTORS_11_5KOHM}, > + { 330, 0x05, CP_CURRENT_1_5UA, LPF_RESISTORS_15_5KOHM}, > + { 360, 0x15, CP_CURRENT_1_5UA, LPF_RESISTORS_15_5KOHM}, > + { 400, 0x25, CP_CURRENT_1_5UA, LPF_RESISTORS_15_5KOHM}, > + { 450, 0x06, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 500, 0x16, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 550, 0x07, CP_CURRENT_7_5UA, LPF_RESISTORS_10_5KOHM}, > + { 600, 0x17, CP_CURRENT_7_5UA, LPF_RESISTORS_10_5KOHM}, > + { 650, 0x08, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 700, 0x18, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 750, 0x09, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 800, 0x19, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 850, 0x29, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 900, 0x39, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 950, 0x0a, CP_CURRENT_12UA, LPF_RESISTORS_8KOHM}, > + {1000, 0x1a, CP_CURRENT_12UA, LPF_RESISTORS_8KOHM}, > + {1050, 0x2a, CP_CURRENT_1
Re: [PATCH] drm/amd: DC pull request review
On Wed, Sep 27, 2017 at 6:38 PM, Sean Paul wrote: > Any chance we can address the i2c/gpio [re-]implementations as well? It's already on the list. Part of this is code that's probably dead, the other is a bit too much layer cake still left, and the final bits are not fully using drm helpers for edid parsing and tons of other stuff. But afaics all the i2c stuff does go through the i2c_adapter abstraction in a proper fashion, which is at least the foundational work. The other bits should be all captured in the todo already. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd: DC pull request review
On Wed, Sep 27, 2017 at 6:15 AM, Daniel Vetter wrote: > Ok, here's one more attempt at scrolling through 130k diff. > > Overall verdict from me is that DC is big project, and like any big > project it's never done. So at least for me the goal isn't to make > things perfect, becaue if that's the hoop to jump through we wouldn't > have any gpu drivers at all. More important is whether merging a new > driver base will benefit the overall subsystem, and here this > primarily means whether the DC team understands how upstream works and > is designed, and whether the code is largely aligned with upstream > (especially the atomic modeset) architecture. > > Looking back over the last two years I think that's the case now, so > > Acked-by: Daniel Vetter > > for merging this pull. I'll echo the above. Perfect is a hard target to hit, even more so if the target is moving. AFAICS, the biggest issue DC faces is not a quality issue, but rather a volume issue. The driver's origin being from Windows means that they have a lot of things built from first principles instead of leveraging existing infrastructure. While this is a real issue, I've seen (and worked on) drm drivers in much worse shape than this. Given the amount of effort AMD have already put into upstreaming and community involvement, I have no doubt they'll continue to trim things down after pull. I hope the community will also kick in to help them out. So, Acked-by: Sean Paul > > While scrolling through the pull I spotted a bunch more things that > should be refactored, but most of these will be a real pain with DC > is out of tree, and much easier in tree since in many of these areas > the in-tree helpers aren't up to snuff yet for what DC needs. That > kind of work is best done when there's one tree with everything > integrated. > > That's also why I think we should merge DC into drm-next directly, so > we can get started on the integration polish right away. That has a > bit higher risk of Linus having a spazz, so here's my recommendation > for merging: > > - There's a few additions to drm_dp_helper.h sprinkled all over the > pull. I think those should be put into a patch of it's own, and > merged first. No need to rebase DC, git merge will dtrt and not end > up with duplicates. > > - dm_alloc/realloc/free is something Dave Airlie noticed, and I agree > it's an easy red flag that might upset Linus. cocci can fix this > easy, so no real problem I think to patch up in one big patch (I > thought we've had a "remove malloc wrappers" todo item in the very > first review, apparently there was more than one such wrapper). > > - The history is huge, but AMD folks want to keep it if possible, and > I see the value in that. Would be good to get an ack from Linus for > that (but shouldn't be an issue, not the first time we've merged the > full history of out-of-tree work). Any chance we can address the i2c/gpio [re-]implementations as well? Sean > > Short&longer term TODO items are still tracked, might be a good idea > to integrate those the overall drm todo in our gpu documentation, for > more visibility. > > So in a way this is kinda like staging, except not with the horribly > broken process of having an entirely separate tree for staging drivers > which just makes refactoring needlessly painful (which defeats the > point of staging really). So staging-within-the-subsystem. We've had > that before, with early nouveau. > > And yes some of the files are utterly horrible to read and not > anything close to kernel coding style standards. But that's the point, > they're essentially gospel from hw engineers that happens to be > parseable by gcc. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/amd/display/TODO | 31 +++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/TODO > b/drivers/gpu/drm/amd/display/TODO > index 2737873db12d..eea645b102a1 100644 > --- a/drivers/gpu/drm/amd/display/TODO > +++ b/drivers/gpu/drm/amd/display/TODO > @@ -79,3 +79,34 @@ TODOs > > 12. drm_modeset_lock in MST should no longer be needed in recent kernels > * Adopt appropriate locking scheme > + > +13. get_modes and best_encoder callbacks look a bit funny. Can probably rip > out > +a few indirections, and consider removing entirely and using the > +drm_atomic_helper_best_encoder default behaviour. > + > +14. core/dc_debug.c, consider switching to the atomic state debug helpers and > +moving all your driver state printing into the various atomic_print_state > +callbacks. There's also plans to expose this stuff in a standard way across > all > +drivers, to make debugging userspace compositors easier across different hw. > + > +15. Move DP/HDMI dual mode adaptors to drm_dp_dual_mode_helper.c. > + > +16. Move to core SCDC helpers (I think those are new since initial DC > review). > + > +17. There's still a pretty massive layer cake around dp aux and DPCD > handling, > +with like 3 leve
[Bug 101213] Kernel 4.11: amdgpu regression causes artifacts with OLAND amd gpu gcn 1.0
https://bugs.freedesktop.org/show_bug.cgi?id=101213 --- Comment #7 from Francisco Lloret --- Same problem with kernel 4.12.12 i Gentoo (gentoo-sources-4.12.12) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] dim: add drm-amd.git
On Wed, Sep 27, 2017 at 09:48:50AM +, Daniel Vetter wrote: > Unfortunately there's a little bit of hardcoded stuff still. > > Cc: Alex Deucher > Cc: Harry Wentland > Signed-off-by: Daniel Vetter Acked-by: Rodrigo Vivi > --- > dim | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/dim b/dim > index 6ebbad2b6dfd..63ffebe14781 100755 > --- a/dim > +++ b/dim > @@ -761,6 +761,7 @@ function dim_push_branch > > update_linux_next $branch drm-intel-next-queued drm-intel-next-fixes > drm-intel-fixes > update_linux_next $branch drm-misc-next drm-misc-next-fixes > drm-misc-fixes > + update_linux_next $branch drm-amd-next drm-amd-next-fixes drm-amd-fixes > > dim_rebuild_tip > } > -- > 2.14.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/3] dim: auto-add remotes
On Wed, Sep 27, 2017 at 09:48:49AM +, Daniel Vetter wrote: > Well, bother to at least prompt. Prep work for adding drm-amd.git. > > Signed-off-by: Daniel Vetter > --- > dim | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/dim b/dim > index 4d75c7a7fb0e..6ebbad2b6dfd 100755 > --- a/dim > +++ b/dim > @@ -274,7 +274,14 @@ function url_to_remote # url > echoerr "Please set it up using:" > echoerr "$ git remote add $url" I think these 2 messages above doesn't make sense anymore, right? > echoerr "with a name of your choice." > - return 1 \o/ I like to see more automation on the setup... > + remote=${url%.git} > + remote=${remote##*/} > + read -r -i "$remote" -e -p "Enter a name to auto-add > this remote, leave blank to abort: " || true I'd be more in favor of forcing a standard and be flexible on .dimrc to any later change if people really need that. Anyways not blocking. With messages above checked: Acked-by: Rodrigo Vivi > + if [[ "$REPLY" == "" ]] ; then > + exit 1 > + fi > + > + git remote add $remote $url > fi > fi > > -- > 2.14.1 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] dim: redo the author sob checks
On Wed, Sep 27, 2017 at 09:48:48AM +, Daniel Vetter wrote: > This reverts commit 41dddc0287bb9ef14be8de3c3185ed6aaa809d98 and then > tries a different approach because from the commit that originally > introduced this: > > commit 3dd25f235c73f7855dc570585eb2551961a1911a > Author: Benjamin Gaignard > Date: Wed Jul 26 14:07:49 2017 +0200 > > dim: add checks for author and committer sign-off-by > > we have > > "Use real names for people with many different email addresses." > > to work around outlook we need to convert the "Last, First" firm into > what git expects instead, which also should solve the problem Rodrigo > has. > > Cc: Rodrigo Vivi > Cc: Jani Nikula > Signed-off-by: Daniel Vetter Acked-by: Rodrigo Vivi > --- > dim | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/dim b/dim > index 5e8c4d5b212d..4d75c7a7fb0e 100755 > --- a/dim > +++ b/dim > @@ -698,11 +698,13 @@ function checkpatch_commit_push > sha1=$1 > > # use real names for people with many different email addresses > - author=$(git show -s $sha1 --format="format:%ae") > + author=$(git show -s $sha1 --format="format:%an") > committer=$(git show -s $sha1 --format="format:%cn") > + # outlook mangles mails into "Last, First" > + author_outlook=$(git show -s $sha1 --format="format:%an" | sed -e > 's/\([^ ]*\) \(.*\)/\2, \1/') > > # check for author sign-off > - if ! git show -s $sha1 | grep -qi "S.*-by:.*$author" ; then > + if ! git show -s $sha1 | grep -qi > "S.*-by:.*\\($author\\|$author_outlook\\)" ; then > warn_or_fail "$sha1 is lacking author of sign-off" > fi > > -- > 2.14.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99950] Add cl_khr_int64_base_atomics to Clover and make it work on radeonsi
https://bugs.freedesktop.org/show_bug.cgi?id=99950 Jan Vesely changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #2 from Jan Vesely --- you'll need latest mesa (17.3) and libclc >=r313811 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99553] Tracker bug for runnning OpenCL applications on Clover
https://bugs.freedesktop.org/show_bug.cgi?id=99553 Bug 99553 depends on bug 99950, which changed state. Bug 99950 Summary: Add cl_khr_int64_base_atomics to Clover and make it work on radeonsi https://bugs.freedesktop.org/show_bug.cgi?id=99950 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Call sysfs_notify after changing drm_connector::dpms
On Wed, 27 Sep 2017, Karsten Wiese wrote: > 2017-09-27 14:58 GMT+02:00 Jani Nikula : >> It still feels wrong to add sysfs notify support for just one of our >> properties, for just this one use case. In particular if the reason is >> to workaround bugs in another interface. >> > > Would you accept a patch supporting all relevant attributes? It's not my call alone, but personally I'd be hesitant about taking on the burden of maintaining that ABI. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c
Den 27.09.2017 15.54, skrev Meghana Madhyastha: Rename tinydrm_of_find_backlight to drm_of_find_backlight and move it into drm_of.c from tinydrm-helpers.c. This is because other drivers in the drm subsystem might need to call this function. In that case and otherwise, it is better from an organizational point of view to move it into drm_of.c along with the other _of.c functions. Signed-off-by: Meghana Madhyastha --- I suggest you split this patch in 2, one to add the function and one to use it in tinydrm. drivers/gpu/drm/drm_of.c | 41 ++ drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 - drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +- include/drm/drm_of.h | 1 + include/drm/tinydrm/tinydrm-helpers.h | 1 - 5 files changed, 44 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index 8dafbdf..d8cded3 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -260,3 +261,43 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, return ret; } EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge); + +/** + * drm_of_find_backlight - Find backlight device in device-tree + * @dev: Device + * + * This function looks for a DT node pointed to by a property named 'backlight' + * and uses of_find_backlight_by_node() to get the backlight device. + * Additionally if the brightness property is zero, it is set to + * max_brightness. Please add a note here about the callers responsibility to call put_device() when releasing the resource. See the of_find_backlight_by_node() docs. + * + * Returns: + * NULL if there's no backlight property. + * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device + * is found. + * If the backlight device is found, a pointer to the structure is returned. + */ +struct backlight_device *drm_of_find_backlight(struct device *dev) +{ + struct backlight_device *backlight; + struct device_node *np; + + np = of_parse_phandle(dev->of_node, "backlight", 0); + if (!np) + return NULL; + + backlight = of_find_backlight_by_node(np); + of_node_put(np); + + if (!backlight) + return ERR_PTR(-EPROBE_DEFER); + + if (!backlight->props.brightness) { + backlight->props.brightness = backlight->props.max_brightness; + DRM_DEBUG_KMS("Backlight brightness set to %d\n", + backlight->props.brightness); + } + + return backlight; +} +EXPORT_SYMBOL(drm_of_find_backlight); Can you also please add a devm_ version of this function. tinydrm uses devres[1] versions of functions for requiring device resources, so it would be nice to do this for backlight as well. tinydrm is currently broken in this respect, it doesn't put the backlight device. I had a devm_of_find_backlight() version lying around that I've adjusted to give you an idea of what I'm talking about: static void devm_drm_of_find_backlight_release(void *data) { put_device(data); } struct backlight_device *devm_drm_of_find_backlight(struct device *dev) { struct backlight_device *backlight; int ret; backlight = drm_of_find_backlight(dev); if (IS_ERR_OR_NULL(backlight)) return backlight; ret = devm_add_action(dev, devm_drm_of_find_backlight_release, backlight->dev); if (ret) { put_device(backlight->dev); return ERR_PTR(ret); } return backlight; } [1] https://www.kernel.org/doc/Documentation/driver-model/devres.txt Noralf. diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index bd6cce0..cd4c6a5 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -237,46 +237,6 @@ void tinydrm_xrgb_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, EXPORT_SYMBOL(tinydrm_xrgb_to_gray8); /** - * tinydrm_of_find_backlight - Find backlight device in device-tree - * @dev: Device - * - * This function looks for a DT node pointed to by a property named 'backlight' - * and uses of_find_backlight_by_node() to get the backlight device. - * Additionally if the brightness property is zero, it is set to - * max_brightness. - * - * Returns: - * NULL if there's no backlight property. - * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device - * is found. - * If the backlight device is found, a pointer to the structure is returned. - */ -struct backlight_device *tinydrm_of_find_backlight(struct device *dev) -{ - struct backlight_device *backlight; - struct device_node *np; - - np = of_parse_phandle(dev->of_node, "backlight", 0); - if (!np) -
Re: [PATCH] drm: Call sysfs_notify after changing drm_connector::dpms
2017-09-27 14:58 GMT+02:00 Jani Nikula : > On Wed, 27 Sep 2017, Daniel Vetter wrote: > > On Wed, Sep 27, 2017 at 12:20:21PM +0200, Karsten Wiese wrote: > >> 2017-09-27 9:18 GMT+02:00 Jani Nikula : > >> > >> > On Wed, 27 Sep 2017, Karsten Wiese wrote: > >> > > 2017-09-25 15:48 GMT+02:00 Jani Nikula >: > >> > > > >> > >> On Tue, 19 Sep 2017, Karsten Wiese wrote: > >> > >> > This makes poll work for the > >> > >> > /sys/class/drm/cardX/connectorY/dpms attributes. > >> > >> > >> > >> I guess the question is, what are you trying to achieve? What is > the > >> > >> problem that this solves? > >> > >> > >> > > > >> > > The patch enables cpu cycle less waiting for dpms flag changes. > >> > > > >> > > Here it lets a screen brightness setting daemon know which monitor > to > >> > > handle. One of the attached screens gets confused if it is set just > >> > > after being switched on, hence the daemon leaves it untouched until > it > >> > > has been active for some seconds. > >> > > >> > Screen brightness settings daemon? > >> > > >> Running on a laptop lacking an ambient light sensor employing it's > webcam > >> instead to measure ambient light and adjust monitors' brightnesses > >> accordingly. > >> > >> > >> What exactly do you mean by "if it is > >> > set"? What interface are you using to change brightness? > >> > >> The external monitor's brightness is adjusted by DDC/CI via the > /dev/i2c-x > >> the monitor is attached to. > >> I there a saner interface to use? > > > > There's supposed to be a backlight property on the panel, exposed by the > X > > driver. If there's not, then that needs to be fixed/adjusted. > > For eDP/LVDS in Intel driver yes, for external displays not. For > external displays DDC/CI is currently probably the best bet. > > However, DDC/CI is overall not very robust. I think we have prior bug > reports about it, but it's never really been very high on our list of > priorities. > > > There's also plans to directly link that up on the kernel side, but > that's > > a bit more involved due to how screwed up the backlight driver design on > > linux is right now. > > It's going to take forever and a half before DDC/CI for external > displays get that treatment, if ever. > > It still feels wrong to add sysfs notify support for just one of our > properties, for just this one use case. In particular if the reason is > to workaround bugs in another interface. > Would you accept a patch supporting all relevant attributes? BR, Karsten > > BR, > Jani. > > > > -Daniel > > > >> > >> What happens > >> > when the display "gets confused"? > >> > > >> The monitor wrongly toggles sharpness if it receives the brightness > >> adjusting > >> DDC/CI soon after resuming from power saving state. > >> > >> > > >> > My first instinct is that you're proposing a new kernel ABI to solve > an > >> > issue you shouldn't be solving in userspace to begin with. > >> > >> Calculating ambient light from pictures acquired via > >> v4l2 in kernel seamed wrong to me. > >> > >> Or that > >> > perhaps the userspace should be doing this in cooperation with the drm > >> > master, not standalone. > >> > > >> Is there a way to call into the drm-master (xscreensaver/xserver here > >> ) with the call only returning if a monitor's power state changed? > >> > >> There is DPMSInfo, but it returns immediately rendering the daemon less > >> efficient. > >> > >> > >> BR, > >> > >> Karsten > >> > >> > > >> > BR, > >> > Jani. > >> > > >> > > > >> > > Without the patch the daemon would have to actively read the dpms > flag > >> > > every once in a while. > >> > > > >> > > > >> > >> > >> > >> We have zero sysfs_notify in all of drm AFAICT. > >> > > > >> > > > >> > > Yes I noticed too and looked for some dbus signal to listen to but > >> > didn't > >> > > find any. > >> > > > >> > > BR, > >> > > Karsten > >> > > > >> > >> > >> > >> BR, > >> > >> Jani. > >> > >> > >> > >> > >> > >> > > >> > >> > Tested with i915 suspended by XScreenServer and > >> > >> > suspend to RAM. > >> > >> > > >> > >> > Signed-off-by: Karsten Wiese > >> > >> > --- > >> > >> > drivers/gpu/drm/drm_atomic.c| 4 > >> > >> > drivers/gpu/drm/drm_atomic_helper.c | 6 +- > >> > >> > 2 files changed, 9 insertions(+), 1 deletion(-) > >> > >> > > >> > >> > diff --git a/drivers/gpu/drm/drm_atomic.c > >> > b/drivers/gpu/drm/drm_atomic.c > >> > >> > index 2fd383d..b6fa87b 100644 > >> > >> > --- a/drivers/gpu/drm/drm_atomic.c > >> > >> > +++ b/drivers/gpu/drm/drm_atomic.c > >> > >> > @@ -1880,6 +1880,10 @@ int drm_atomic_connector_commit_ > dpms(struct > >> > >> drm_atomic_state *state, > >> > >> > out: > >> > >> > if (ret != 0) > >> > >> > connector->dpms = old_mode; > >> > >> > + else > >> > >> > + if (connector->dpms != old_mode) > >> > >> > + sysfs_notify(&connector->kdev->kobj, NULL, > >> > >> "dpms"); > >> > >> > + > >> > >> > return ret; > >> > >> > } > >> > >> > > >> > >> > diff
Re: [PATCH hwc v1] drm_hwcomposer: Add CONTRIBUTING file
Hi Rob, Glad to see this. There's a couple of suggestions that I hope you'll find worth while. On 22 September 2017 at 01:37, Robert Foss wrote: > Some basic guidelines for contributions could come in handy. > > These are copied from IGT and modified to be suitable. > > Signed-off-by: Robert Foss > --- > CONTRIBUTING | 31 +++ > 1 file changed, 31 insertions(+) > create mode 100644 CONTRIBUTING > > diff --git a/CONTRIBUTING b/CONTRIBUTING > new file mode 100644 > index 000..f1b4775 > --- /dev/null > +++ b/CONTRIBUTING > @@ -0,0 +1,31 @@ > +Patches to drm_hwcomposer are very much welcome, we really want this to be > the > +universal HW composer implementation for Android and similar platforms > +So please bring on porting patches, bugfixes, improvements for documentation > +and new features. > + > +A short list of contribution guidelines: > + > +- Please submit patches formatted with git send-email/git format-patch or > + equivalent to > + > +dri-devel > + > + Please use --subject-prefix="PATCH hwc" so that drm_hwcomposer patches are > easily > + identified in the massive amount mails on dri-devel. To ensure this is > always > + done, run: > + > +git config format.subjectprefix "PATCH hwc" > + One can add this into the autogen.sh or whatever bootstrap file the project uses. For example see https://cgit.freedesktop.org/mesa/drm/tree/autogen.sh > +- When submitting new code please follow the naming conventions documented > + in the generated documentation. Also please make full use of all the > helpers and > + convenience macros provided by drm_hwcomposer. The below command can help > you > + with formatting of your patches: > + git diff | clang-format-diff-3.5 -p 1 -style=file > + One could wire that in the pre-merge hook on the server side. It should be quite lightweight, although I'm not sure about the integration with FDO's Debian(?) setup ;-) -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vc4: Add the DRM_IOCTL_VC4_GEM_MADVISE ioctl
This ioctl will allow us to purge inactive userspace buffers when the system is running out of contiguous memory. For now, the purge logic is rather dumb in that it does not try to release only the amount of BO needed to meet the last CMA alloc request but instead purges all objects placed in the purgeable pool as soon as we experience a CMA allocation failure. Note that the in-kernel BO cache is always purged before the purgeable cache because those objects are known to be unused while objects marked as purgeable by a userspace application/library might have to be restored when they are marked back as unpurgeable, which can be expensive. Signed-off-by: Boris Brezillon --- Hello, Updates to libdrm, mesa and igt making use of this kernel feature can be found on my github repos [1][2][3]. There's currently no debugfs hook to manually force a purge, but this is being discussed and will probably be added in v2. Regards, Boris [1]https://github.com/bbrezillon/drm/tree/vc4/purgeable [2]https://github.com/bbrezillon/mesa/tree/vc4/purgeable [3]https://github.com/bbrezillon/igt/tree/vc4/purgeable --- drivers/gpu/drm/vc4/vc4_bo.c| 188 +-- drivers/gpu/drm/vc4/vc4_drv.c | 9 +- drivers/gpu/drm/vc4/vc4_drv.h | 26 + drivers/gpu/drm/vc4/vc4_gem.c | 213 +++- drivers/gpu/drm/vc4/vc4_plane.c | 20 drivers/gpu/drm/vc4/vc4_render_cl.c | 3 + include/uapi/drm/vc4_drm.h | 12 ++ 7 files changed, 455 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c index 3afdbf4bc10b..f53aa508cb6f 100644 --- a/drivers/gpu/drm/vc4/vc4_bo.c +++ b/drivers/gpu/drm/vc4/vc4_bo.c @@ -233,7 +233,7 @@ static struct list_head *vc4_get_cache_list_for_size(struct drm_device *dev, return &vc4->bo_cache.size_list[page_index]; } -static void vc4_bo_cache_purge(struct drm_device *dev) +void vc4_bo_cache_purge(struct drm_device *dev) { struct vc4_dev *vc4 = to_vc4_dev(dev); @@ -293,6 +293,8 @@ struct drm_gem_object *vc4_create_object(struct drm_device *dev, size_t size) if (!bo) return ERR_PTR(-ENOMEM); + bo->madv = VC4_MADV_WILLNEED; + mutex_init(&bo->madv_lock); mutex_lock(&vc4->bo_lock); bo->label = VC4_BO_TYPE_KERNEL; vc4->bo_labels[VC4_BO_TYPE_KERNEL].num_allocated++; @@ -330,13 +332,29 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size, * CMA allocations we've got laying around and try again. */ vc4_bo_cache_purge(dev); + cma_obj = drm_gem_cma_create(dev, size); + } + if (IS_ERR(cma_obj)) { + /* +* Still not enough CMA memory, purge the userspace BO +* cache and retry. +* This is sub-optimal since we purge the whole userspace +* BO cache which forces user that want to re-use the BO to +* restore its initial content. +* Ideally, we should purge entries one by one and retry +* after each to see if CMA allocation succeeds. Or even +* better, try to find an entry with at least the same +* size. +*/ + vc4_userspace_bo_cache_purge(dev); cma_obj = drm_gem_cma_create(dev, size); - if (IS_ERR(cma_obj)) { - DRM_ERROR("Failed to allocate from CMA:\n"); - vc4_bo_stats_dump(vc4); - return ERR_PTR(-ENOMEM); - } + } + + if (IS_ERR(cma_obj)) { + DRM_ERROR("Failed to allocate from CMA:\n"); + vc4_bo_stats_dump(vc4); + return ERR_PTR(-ENOMEM); } bo = to_vc4_bo(&cma_obj->base); @@ -403,6 +421,15 @@ void vc4_free_object(struct drm_gem_object *gem_bo) struct vc4_bo *bo = to_vc4_bo(gem_bo); struct list_head *cache_list; + /* Remove the BO from the purgeable list. */ + mutex_lock(&bo->madv_lock); + if (bo->madv == VC4_MADV_DONTNEED && !refcount_read(&bo->usecnt)) { + mutex_lock(&vc4->purgeable.lock); + list_del(&bo->size_head); + mutex_unlock(&vc4->purgeable.lock); + } + mutex_unlock(&bo->madv_lock); + mutex_lock(&vc4->bo_lock); /* If the object references someone else's memory, we can't cache it. */ @@ -418,7 +445,8 @@ void vc4_free_object(struct drm_gem_object *gem_bo) } /* If this object was partially constructed but CMA allocation -* had failed, just free it. +* had failed, just free it. Can also happen when the BO has been +* purged. */ if (!bo->base.vaddr) { vc4_bo_destroy(bo); @@ -437,6 +465,9 @@ void vc4_free_object(struct drm_gem_object *gem_bo)
Re: [PATCH] drm/amd: DC pull request review
On 2017-09-27 06:15 AM, Daniel Vetter wrote: > Ok, here's one more attempt at scrolling through 130k diff. > > Overall verdict from me is that DC is big project, and like any big > project it's never done. So at least for me the goal isn't to make > things perfect, becaue if that's the hoop to jump through we wouldn't > have any gpu drivers at all. More important is whether merging a new > driver base will benefit the overall subsystem, and here this > primarily means whether the DC team understands how upstream works and > is designed, and whether the code is largely aligned with upstream > (especially the atomic modeset) architecture. > > Looking back over the last two years I think that's the case now, so > > Acked-by: Daniel Vetter > > for merging this pull. > > While scrolling through the pull I spotted a bunch more things that > should be refactored, but most of these will be a real pain with DC > is out of tree, and much easier in tree since in many of these areas > the in-tree helpers aren't up to snuff yet for what DC needs. That > kind of work is best done when there's one tree with everything > integrated. > > That's also why I think we should merge DC into drm-next directly, so > we can get started on the integration polish right away. That has a > bit higher risk of Linus having a spazz, so here's my recommendation > for merging: > > - There's a few additions to drm_dp_helper.h sprinkled all over the > pull. I think those should be put into a patch of it's own, and > merged first. No need to rebase DC, git merge will dtrt and not end > up with duplicates. > > - dm_alloc/realloc/free is something Dave Airlie noticed, and I agree > it's an easy red flag that might upset Linus. cocci can fix this > easy, so no real problem I think to patch up in one big patch (I > thought we've had a "remove malloc wrappers" todo item in the very > first review, apparently there was more than one such wrapper). > > - The history is huge, but AMD folks want to keep it if possible, and > I see the value in that. Would be good to get an ack from Linus for > that (but shouldn't be an issue, not the first time we've merged the > full history of out-of-tree work). > > Short&longer term TODO items are still tracked, might be a good idea > to integrate those the overall drm todo in our gpu documentation, for > more visibility. > > So in a way this is kinda like staging, except not with the horribly > broken process of having an entirely separate tree for staging drivers > which just makes refactoring needlessly painful (which defeats the > point of staging really). So staging-within-the-subsystem. We've had > that before, with early nouveau. > > And yes some of the files are utterly horrible to read and not > anything close to kernel coding style standards. But that's the point, > they're essentially gospel from hw engineers that happens to be > parseable by gcc. > > Signed-off-by: Daniel Vetter Thanks for the feedback, ack and help along the way. This patch is Reviewed-by: Harry Wentland > --- > drivers/gpu/drm/amd/display/TODO | 31 +++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/TODO > b/drivers/gpu/drm/amd/display/TODO > index 2737873db12d..eea645b102a1 100644 > --- a/drivers/gpu/drm/amd/display/TODO > +++ b/drivers/gpu/drm/amd/display/TODO > @@ -79,3 +79,34 @@ TODOs > > 12. drm_modeset_lock in MST should no longer be needed in recent kernels > * Adopt appropriate locking scheme > + > +13. get_modes and best_encoder callbacks look a bit funny. Can probably rip > out > +a few indirections, and consider removing entirely and using the > +drm_atomic_helper_best_encoder default behaviour. > + > +14. core/dc_debug.c, consider switching to the atomic state debug helpers and > +moving all your driver state printing into the various atomic_print_state > +callbacks. There's also plans to expose this stuff in a standard way across > all > +drivers, to make debugging userspace compositors easier across different hw. > + > +15. Move DP/HDMI dual mode adaptors to drm_dp_dual_mode_helper.c. > + > +16. Move to core SCDC helpers (I think those are new since initial DC > review). > + > +17. There's still a pretty massive layer cake around dp aux and DPCD > handling, > +with like 3 levels of abstraction and using your own structures instead of > the > +stuff in drm_dp_helper.h. drm_dp_helper.h isn't really great and already has > 2 > +incompatible styles, just means more reasons not to add a third (or well > third > +one gets to do the cleanup refactor). > + > +18. There's a pile of sink handling code, both for DP and HDMI where I didn't > +immediately recognize the standard. I think long term it'd be best for the > drm > +subsystem if we try to move as much of that into helpers/core as possible, > and > +share it with drivers. But that's a very long term goal, and by far not just > an > +issue with DC - other dr
[PATCH] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c
Rename tinydrm_of_find_backlight to drm_of_find_backlight and move it into drm_of.c from tinydrm-helpers.c. This is because other drivers in the drm subsystem might need to call this function. In that case and otherwise, it is better from an organizational point of view to move it into drm_of.c along with the other _of.c functions. Signed-off-by: Meghana Madhyastha --- drivers/gpu/drm/drm_of.c | 41 ++ drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 - drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +- include/drm/drm_of.h | 1 + include/drm/tinydrm/tinydrm-helpers.h | 1 - 5 files changed, 44 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index 8dafbdf..d8cded3 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -260,3 +261,43 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, return ret; } EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge); + +/** + * drm_of_find_backlight - Find backlight device in device-tree + * @dev: Device + * + * This function looks for a DT node pointed to by a property named 'backlight' + * and uses of_find_backlight_by_node() to get the backlight device. + * Additionally if the brightness property is zero, it is set to + * max_brightness. + * + * Returns: + * NULL if there's no backlight property. + * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device + * is found. + * If the backlight device is found, a pointer to the structure is returned. + */ +struct backlight_device *drm_of_find_backlight(struct device *dev) +{ + struct backlight_device *backlight; + struct device_node *np; + + np = of_parse_phandle(dev->of_node, "backlight", 0); + if (!np) + return NULL; + + backlight = of_find_backlight_by_node(np); + of_node_put(np); + + if (!backlight) + return ERR_PTR(-EPROBE_DEFER); + + if (!backlight->props.brightness) { + backlight->props.brightness = backlight->props.max_brightness; + DRM_DEBUG_KMS("Backlight brightness set to %d\n", + backlight->props.brightness); + } + + return backlight; +} +EXPORT_SYMBOL(drm_of_find_backlight); diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index bd6cce0..cd4c6a5 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -237,46 +237,6 @@ void tinydrm_xrgb_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, EXPORT_SYMBOL(tinydrm_xrgb_to_gray8); /** - * tinydrm_of_find_backlight - Find backlight device in device-tree - * @dev: Device - * - * This function looks for a DT node pointed to by a property named 'backlight' - * and uses of_find_backlight_by_node() to get the backlight device. - * Additionally if the brightness property is zero, it is set to - * max_brightness. - * - * Returns: - * NULL if there's no backlight property. - * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device - * is found. - * If the backlight device is found, a pointer to the structure is returned. - */ -struct backlight_device *tinydrm_of_find_backlight(struct device *dev) -{ - struct backlight_device *backlight; - struct device_node *np; - - np = of_parse_phandle(dev->of_node, "backlight", 0); - if (!np) - return NULL; - - backlight = of_find_backlight_by_node(np); - of_node_put(np); - - if (!backlight) - return ERR_PTR(-EPROBE_DEFER); - - if (!backlight->props.brightness) { - backlight->props.brightness = backlight->props.max_brightness; - DRM_DEBUG_KMS("Backlight brightness set to %d\n", - backlight->props.brightness); - } - - return backlight; -} -EXPORT_SYMBOL(tinydrm_of_find_backlight); - -/** * tinydrm_enable_backlight - Enable backlight helper * @backlight: Backlight device * diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index 7e5bb7d..5e3d635 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -189,7 +190,7 @@ static int mi0283qt_probe(struct spi_device *spi) if (IS_ERR(mipi->regulator)) return PTR_ERR(mipi->regulator); - mipi->backlight = tinydrm_of_find_backlight(dev); + mipi->backlight = drm_of_find_backlight(dev); if (IS_ERR(mipi->backlight)) return PTR_ERR(mipi->backlight); diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h index 104dd51..
Re: [PATCH] drm/amd: DC pull request review
On Wed, Sep 27, 2017 at 3:47 PM, Harry Wentland wrote: > On 2017-09-27 06:15 AM, Daniel Vetter wrote: >> Ok, here's one more attempt at scrolling through 130k diff. >> +19. The DC logger is still a rather sore thing, but I know that the >> DRM_DEBUG >> +stuff just isn't up to the challenges either. We need to figure out >> something >> +that integrates better with DRM and linux debug printing, while not being >> +useless with filtering output. dynamic debug printing might be an option. >> > > Have there been any thoughts on improving log filtering in DRM? I think > our KFD guys (which hopefully go upstream soon as well) are using > dynamic debug prints for it but not sure if that's entirely the right > approach. Plenty of complaints for sure, but I don't think anyone has looked into it seriously. The big trouble is backwards compat with existing drm.debug (too many people know about that one), and with dynamic printk specifically, that non-debug builds don't build it (so useless as-is for debugging issues users have). That was at least the conclusion from the last few discussions I remember. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading
On 27 September 2017 at 12:58, Robert Foss wrote: > 16 files changed, 93 insertions(+), 724 deletions(-) Holly smokes, that's some amazing stat. Please sir can I have some more ;-) Question - this patch removes the threading implementation, while the actual substitute lands with patches 2-6. Did I get this right? If so, ideally this patch will be the final in the series. Guess it doesn't matter too much though. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 1/2] staging: ion: simplify ioctl args checking function
Make arguments checking more easy to read. Signed-off-by: Benjamin Gaignard --- drivers/staging/android/ion/ion-ioctl.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c index d9f8b14..e26b786 100644 --- a/drivers/staging/android/ion/ion-ioctl.c +++ b/drivers/staging/android/ion/ion-ioctl.c @@ -27,19 +27,18 @@ union ion_ioctl_arg { static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg) { - int ret = 0; - switch (cmd) { case ION_IOC_HEAP_QUERY: - ret = arg->query.reserved0 != 0; - ret |= arg->query.reserved1 != 0; - ret |= arg->query.reserved2 != 0; + if (arg->query.reserved0 || + arg->query.reserved1 || + arg->query.reserved2) + return -EINVAL; break; default: break; } - return ret ? -EINVAL : 0; + return 0; } /* fix up the cases where the ioctl direction bits are incorrect */ -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/2] staging: ion: create one device entry per heap
Instead a getting only one common device "/dev/ion" for all the heaps this patch allow to create one device entry ("/dev/ionX") per heap. Getting an entry per heap could allow to set security rules per heap and global ones for all heaps. Allocation requests will be only allowed if the mask_id match with device minor. Query request could be done on any of the devices. Signed-off-by: Benjamin Gaignard --- version 5: - create a configuration flag to keep legacy Ion misc device version 4: - add a configuration flag to switch between legacy Ion misc device and one device per heap version. version 3: - change ion_device_add_heap prototype to return a possible error. version 2: - simplify ioctl check like propose by Dan - make sure that we don't register more than ION_DEV_MAX heaps. drivers/staging/android/TODO| 1 - drivers/staging/android/ion/Kconfig | 7 +++ drivers/staging/android/ion/ion-ioctl.c | 18 -- drivers/staging/android/ion/ion.c | 31 ++- drivers/staging/android/ion/ion.h | 15 +-- 5 files changed, 66 insertions(+), 6 deletions(-) diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO index 5f14247..d770ffa 100644 --- a/drivers/staging/android/TODO +++ b/drivers/staging/android/TODO @@ -9,7 +9,6 @@ TODO: ion/ - Add dt-bindings for remaining heaps (chunk and carveout heaps). This would involve putting appropriate bindings in a memory node for Ion to find. - - Split /dev/ion up into multiple nodes (e.g. /dev/ion/heap0) - Better test framework (integration with VGEM was suggested) Please send patches to Greg Kroah-Hartman and Cc: diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig index a517b2d..cb4666e 100644 --- a/drivers/staging/android/ion/Kconfig +++ b/drivers/staging/android/ion/Kconfig @@ -10,6 +10,13 @@ menuconfig ION If you're not using Android its probably safe to say N here. +config ION_LEGACY_DEVICE_API + bool "Keep using Ion legacy misc device API" + depends on ION + help + Choose this option to keep using Ion legacy misc device API + i.e. /dev/ion + config ION_SYSTEM_HEAP bool "Ion system heap" depends on ION diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c index e26b786..bb5c77b 100644 --- a/drivers/staging/android/ion/ion-ioctl.c +++ b/drivers/staging/android/ion/ion-ioctl.c @@ -25,7 +25,8 @@ union ion_ioctl_arg { struct ion_heap_query query; }; -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg) +static int validate_ioctl_arg(struct file *filp, + unsigned int cmd, union ion_ioctl_arg *arg) { switch (cmd) { case ION_IOC_HEAP_QUERY: @@ -34,6 +35,19 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg) arg->query.reserved2 ) return -EINVAL; break; + + case ION_IOC_ALLOC: + { + int mask = 1 << iminor(filp->f_inode); + +#ifdef CONFIG_ION_LEGACY_DEVICE_API + if (imajor(filp->f_inode) == MISC_MAJOR) + return 0; +#endif + if (!(arg->allocation.heap_id_mask & mask)) + return -EINVAL; + break; + } default: break; } @@ -69,7 +83,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd))) return -EFAULT; - ret = validate_ioctl_arg(cmd, &data); + ret = validate_ioctl_arg(filp, cmd, &data); if (WARN_ON_ONCE(ret)) return ret; diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 93e2c90..092b24c 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -40,6 +40,8 @@ #include "ion.h" +#define ION_DEV_MAX 32 + static struct ion_device *internal_dev; static int heap_id; @@ -537,15 +539,28 @@ static int debug_shrink_get(void *data, u64 *val) DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get, debug_shrink_set, "%llu\n"); -void ion_device_add_heap(struct ion_heap *heap) +int ion_device_add_heap(struct ion_heap *heap) { struct dentry *debug_file; struct ion_device *dev = internal_dev; + int ret = 0; if (!heap->ops->allocate || !heap->ops->free) pr_err("%s: can not add heap with invalid ops struct.\n", __func__); + if (heap_id >= ION_DEV_MAX) + return -EBUSY; + + heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id); + dev_set_name(&heap->ddev, "ion%d", heap_id); + device_initialize(&heap->ddev); + cdev_init(&heap->chrdev, &ion_fops); + heap->chr
[PATCH v5 0/2] staging: ion: get one device per heap
version 5: - create a configuration flag to keep legacy Ion misc device version 4: - add a configuration flag to switch between legacy Ion misc device and one device per heap version. This change has been suggested after Laura talks at XDC 2017. version 3: - change ion_device_add_heap prototype to return a possible error. version 2: - simplify ioctl check like propose by Dan - make sure that we don't register more than ION_DEV_MAX heaps. Until now all ion heaps are addressing using the same device "/dev/ion". This way of working doesn't allow to give access rights (for example with SElinux rules) per heap. This series propose to have one device "/dev/ionX" per heap. Query heaps informations will be possible on each device node but allocation request will only be possible if heap_mask_id match with device minor number. Using legacy Ion misc device is still by setting ION_LEGACY_DEVICE_API configuration flag. Benjamin Gaignard (2): staging: ion: simplify ioctl args checking function staging: ion: create one device entry per heap drivers/staging/android/TODO| 1 - drivers/staging/android/ion/Kconfig | 7 +++ drivers/staging/android/ion/ion-ioctl.c | 29 + drivers/staging/android/ion/ion.c | 31 ++- drivers/staging/android/ion/ion.h | 15 +-- 5 files changed, 71 insertions(+), 12 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Call sysfs_notify after changing drm_connector::dpms
On Wed, 27 Sep 2017, Daniel Vetter wrote: > On Wed, Sep 27, 2017 at 12:20:21PM +0200, Karsten Wiese wrote: >> 2017-09-27 9:18 GMT+02:00 Jani Nikula : >> >> > On Wed, 27 Sep 2017, Karsten Wiese wrote: >> > > 2017-09-25 15:48 GMT+02:00 Jani Nikula : >> > > >> > >> On Tue, 19 Sep 2017, Karsten Wiese wrote: >> > >> > This makes poll work for the >> > >> > /sys/class/drm/cardX/connectorY/dpms attributes. >> > >> >> > >> I guess the question is, what are you trying to achieve? What is the >> > >> problem that this solves? >> > >> >> > > >> > > The patch enables cpu cycle less waiting for dpms flag changes. >> > > >> > > Here it lets a screen brightness setting daemon know which monitor to >> > > handle. One of the attached screens gets confused if it is set just >> > > after being switched on, hence the daemon leaves it untouched until it >> > > has been active for some seconds. >> > >> > Screen brightness settings daemon? >> > >> Running on a laptop lacking an ambient light sensor employing it's webcam >> instead to measure ambient light and adjust monitors' brightnesses >> accordingly. >> >> >> What exactly do you mean by "if it is >> > set"? What interface are you using to change brightness? >> >> The external monitor's brightness is adjusted by DDC/CI via the /dev/i2c-x >> the monitor is attached to. >> I there a saner interface to use? > > There's supposed to be a backlight property on the panel, exposed by the X > driver. If there's not, then that needs to be fixed/adjusted. For eDP/LVDS in Intel driver yes, for external displays not. For external displays DDC/CI is currently probably the best bet. However, DDC/CI is overall not very robust. I think we have prior bug reports about it, but it's never really been very high on our list of priorities. > There's also plans to directly link that up on the kernel side, but that's > a bit more involved due to how screwed up the backlight driver design on > linux is right now. It's going to take forever and a half before DDC/CI for external displays get that treatment, if ever. It still feels wrong to add sysfs notify support for just one of our properties, for just this one use case. In particular if the reason is to workaround bugs in another interface. BR, Jani. > -Daniel > >> >> What happens >> > when the display "gets confused"? >> > >> The monitor wrongly toggles sharpness if it receives the brightness >> adjusting >> DDC/CI soon after resuming from power saving state. >> >> > >> > My first instinct is that you're proposing a new kernel ABI to solve an >> > issue you shouldn't be solving in userspace to begin with. >> >> Calculating ambient light from pictures acquired via >> v4l2 in kernel seamed wrong to me. >> >> Or that >> > perhaps the userspace should be doing this in cooperation with the drm >> > master, not standalone. >> > >> Is there a way to call into the drm-master (xscreensaver/xserver here >> ) with the call only returning if a monitor's power state changed? >> >> There is DPMSInfo, but it returns immediately rendering the daemon less >> efficient. >> >> >> BR, >> >> Karsten >> >> > >> > BR, >> > Jani. >> > >> > > >> > > Without the patch the daemon would have to actively read the dpms flag >> > > every once in a while. >> > > >> > > >> > >> >> > >> We have zero sysfs_notify in all of drm AFAICT. >> > > >> > > >> > > Yes I noticed too and looked for some dbus signal to listen to but >> > didn't >> > > find any. >> > > >> > > BR, >> > > Karsten >> > > >> > >> >> > >> BR, >> > >> Jani. >> > >> >> > >> >> > >> > >> > >> > Tested with i915 suspended by XScreenServer and >> > >> > suspend to RAM. >> > >> > >> > >> > Signed-off-by: Karsten Wiese >> > >> > --- >> > >> > drivers/gpu/drm/drm_atomic.c| 4 >> > >> > drivers/gpu/drm/drm_atomic_helper.c | 6 +- >> > >> > 2 files changed, 9 insertions(+), 1 deletion(-) >> > >> > >> > >> > diff --git a/drivers/gpu/drm/drm_atomic.c >> > b/drivers/gpu/drm/drm_atomic.c >> > >> > index 2fd383d..b6fa87b 100644 >> > >> > --- a/drivers/gpu/drm/drm_atomic.c >> > >> > +++ b/drivers/gpu/drm/drm_atomic.c >> > >> > @@ -1880,6 +1880,10 @@ int drm_atomic_connector_commit_dpms(struct >> > >> drm_atomic_state *state, >> > >> > out: >> > >> > if (ret != 0) >> > >> > connector->dpms = old_mode; >> > >> > + else >> > >> > + if (connector->dpms != old_mode) >> > >> > + sysfs_notify(&connector->kdev->kobj, NULL, >> > >> "dpms"); >> > >> > + >> > >> > return ret; >> > >> > } >> > >> > >> > >> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c >> > >> b/drivers/gpu/drm/drm_atomic_helper.c >> > >> > index 4e53aae..6198772 100644 >> > >> > --- a/drivers/gpu/drm/drm_atomic_helper.c >> > >> > +++ b/drivers/gpu/drm/drm_atomic_helper.c >> > >> > @@ -921,12 +921,16 @@ drm_atomic_helper_update_ >> > legacy_modeset_state(struct >> > >> drm_device *dev, >> > >> > crtc = new_conn_state->crtc; >> > >> >
Re: [PATCH] drm/Documentation: Refine TODO for backlight helpers in tinydrm
On Wed, Sep 27, 2017 at 02:32:16PM +0200, Daniel Vetter wrote: > On Wed, Sep 27, 2017 at 04:21:23PM +0530, Meghana Madhyastha wrote: > > Add a summary which resulted from discussions on what should > > be done to refactor backlight helpers in tinydrm so that > > they can be used in other drivers as well. > > > > Signed-off-by: Meghana Madhyastha > > Applied, thanks. > > One thing that just crossed my mind is that we could still move > tinydrm_of_find_backlight into drm_of.c (and rename to > drm_of_find_backlight). Plus check out all other drivers for similar code. > > That should be a much more well defined task, suitable for the application > period. Signed up? Sure ! I will work on this. Thanks and regards, Meghana > Thanks, Daniel > > > --- > > Documentation/gpu/todo.rst | 11 ++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 22af55d..1acf84fe 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -353,7 +353,16 @@ those drivers as simple as possible, so lots of room > > for refactoring: > > - backlight helpers, probably best to put them into a new drm_backlight.c. > >This is because drivers/video is de-facto unmaintained. We could also > >move drivers/video/backlight to drivers/gpu/backlight and take it all > > - over within drm-misc, but that's more work. > > + over within drm-misc, but that's more work. Backlight helpers require a > > fair > > + bit of reworking and refactoring. A simple example is the enabling of a > > backlight. > > + Tinydrm has helpers for this. It would be good if other drivers can also > > use the > > + helper. However, there are various cases we need to consider i.e > > different > > + drivers seem to have different ways of enabling/disabling a backlight. > > + We also need to consider the backlight drivers (like gpio_backlight). > > The situation > > + is further complicated by the fact that the backlight is tied to fbdev > > + via fb_notifier_callback() which has complicated logic. For further > > details, refer > > + to the following discussion thread: > > + https://groups.google.com/forum/#!topic/outreachy-kernel/8rBe30lwtdA > > > > - spi helpers, probably best put into spi core/helper code. Thierry said > >the spi maintainer is fast&reactive, so shouldn't be a big issue. > > -- > > 2.7.4 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/Documentation: Refine TODO for backlight helpers in tinydrm
On Wed, Sep 27, 2017 at 04:21:23PM +0530, Meghana Madhyastha wrote: > Add a summary which resulted from discussions on what should > be done to refactor backlight helpers in tinydrm so that > they can be used in other drivers as well. > > Signed-off-by: Meghana Madhyastha Applied, thanks. One thing that just crossed my mind is that we could still move tinydrm_of_find_backlight into drm_of.c (and rename to drm_of_find_backlight). Plus check out all other drivers for similar code. That should be a much more well defined task, suitable for the application period. Signed up? Thanks, Daniel > --- > Documentation/gpu/todo.rst | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 22af55d..1acf84fe 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -353,7 +353,16 @@ those drivers as simple as possible, so lots of room for > refactoring: > - backlight helpers, probably best to put them into a new drm_backlight.c. >This is because drivers/video is de-facto unmaintained. We could also >move drivers/video/backlight to drivers/gpu/backlight and take it all > - over within drm-misc, but that's more work. > + over within drm-misc, but that's more work. Backlight helpers require a > fair > + bit of reworking and refactoring. A simple example is the enabling of a > backlight. > + Tinydrm has helpers for this. It would be good if other drivers can also > use the > + helper. However, there are various cases we need to consider i.e different > + drivers seem to have different ways of enabling/disabling a backlight. > + We also need to consider the backlight drivers (like gpio_backlight). The > situation > + is further complicated by the fact that the backlight is tied to fbdev > + via fb_notifier_callback() which has complicated logic. For further > details, refer > + to the following discussion thread: > + https://groups.google.com/forum/#!topic/outreachy-kernel/8rBe30lwtdA > > - spi helpers, probably best put into spi core/helper code. Thierry said >the spi maintainer is fast&reactive, so shouldn't be a big issue. > -- > 2.7.4 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/rockchip: Fix memory leak in rockchip_drm_sys_resume()
Free the drm_atomic_state allocated by drm_atomic_helper_suspend(). Fixes: 5a5873830972 ("drm/rockchip: Use atomic PM helpers") Signed-off-by: Jeffy Chen --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 76d63de5921d..80235b672deb 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -15,6 +15,7 @@ */ #include +#include #include #include #include @@ -299,6 +300,7 @@ static int rockchip_drm_sys_resume(struct device *dev) priv = drm->dev_private; drm_atomic_helper_resume(drm, priv->state); + drm_atomic_state_put(priv->state); rockchip_drm_fb_resume(drm); drm_kms_helper_poll_enable(drm); -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 0/6] Implement fencing
This series removes the old thread-based synchronization utilities and replaces them with fences. It has been tested on various platforms, including etnaviv/freedreno/virgl. Robert Foss (5): drm_hwcomposer: Add support for IN_FENCE_FD property to DrmPlane drm_hwcomposer: Submit in-fence to DRM drm_hwcomposer: Add FENCE_OUT_PTR property to DrmCrtc drm_hwcomposer: Add GetCrtcCount function drm_hwcomposer: Add out-fence support Sean Paul (1): drm_hwcomposer: Remove threading Android.mk| 3 - drmcomposition.cpp| 166 drmcomposition.h | 79 --- drmcompositor.cpp | 106 drmcompositor.h | 56 --- drmcompositorworker.h | 41 drmcrtc.cpp | 10 ++ drmcrtc.h | 2 + drmdisplaycomposition.cpp | 1 + drmdisplaycomposition.h | 19 drmdisplaycompositor.cpp | 240 +- drmdisplaycompositor.h| 36 +-- drmeventlistener.cpp | 3 + drmhwctwo.cpp | 15 +-- drmplane.cpp | 8 ++ drmplane.h| 2 + drmresources.cpp | 58 +-- drmresources.h| 6 +- glworker.cpp | 52 +- glworker.h| 10 ++ 20 files changed, 163 insertions(+), 750 deletions(-) delete mode 100644 drmcomposition.cpp delete mode 100644 drmcomposition.h delete mode 100644 drmcompositor.cpp delete mode 100644 drmcompositor.h delete mode 100644 drmcompositorworker.h -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 1/6] drm_hwcomposer: Remove threading
From: Sean Paul Since HWC2 doesn't require the use of threads to implement correct synchronization, remove some of these threads. Signed-off-by: Robert Foss --- Android.mk| 3 - drmcomposition.cpp| 166 drmcomposition.h | 79 --- drmcompositor.cpp | 106 -- drmcompositor.h | 56 -- drmcompositorworker.h | 41 -- drmdisplaycomposition.cpp | 1 + drmdisplaycomposition.h | 10 +++ drmdisplaycompositor.cpp | 189 -- drmdisplaycompositor.h| 36 + drmeventlistener.cpp | 3 + drmhwctwo.cpp | 6 +- drmresources.cpp | 54 + drmresources.h| 5 -- glworker.cpp | 52 +++-- glworker.h| 10 +++ 16 files changed, 93 insertions(+), 724 deletions(-) delete mode 100644 drmcomposition.cpp delete mode 100644 drmcomposition.h delete mode 100644 drmcompositor.cpp delete mode 100644 drmcompositor.h delete mode 100644 drmcompositorworker.h diff --git a/Android.mk b/Android.mk index aa95b44..5d16c2f 100644 --- a/Android.mk +++ b/Android.mk @@ -57,9 +57,6 @@ LOCAL_C_INCLUDES := \ LOCAL_SRC_FILES := \ autolock.cpp \ drmresources.cpp \ - drmcomposition.cpp \ - drmcompositor.cpp \ - drmcompositorworker.cpp \ drmconnector.cpp \ drmcrtc.cpp \ drmdisplaycomposition.cpp \ diff --git a/drmcomposition.cpp b/drmcomposition.cpp deleted file mode 100644 index 1aaf920..000 --- a/drmcomposition.cpp +++ /dev/null @@ -1,166 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "hwc-drm-composition" - -#include "drmcomposition.h" -#include "drmcrtc.h" -#include "drmplane.h" -#include "drmresources.h" -#include "platform.h" - -#include - -#include -#include -#include -#include - -namespace android { - -DrmComposition::DrmComposition(DrmResources *drm, Importer *importer, - Planner *planner) -: drm_(drm), importer_(importer), planner_(planner) { - char use_overlay_planes_prop[PROPERTY_VALUE_MAX]; - property_get("hwc.drm.use_overlay_planes", use_overlay_planes_prop, "1"); - bool use_overlay_planes = atoi(use_overlay_planes_prop); - - for (auto &plane : drm->planes()) { -if (plane->type() == DRM_PLANE_TYPE_PRIMARY) - primary_planes_.push_back(plane.get()); -else if (use_overlay_planes && plane->type() == DRM_PLANE_TYPE_OVERLAY) - overlay_planes_.push_back(plane.get()); - } -} - -int DrmComposition::Init(uint64_t frame_no) { - for (auto &conn : drm_->connectors()) { -int display = conn->display(); -composition_map_[display].reset(new DrmDisplayComposition()); -if (!composition_map_[display]) { - ALOGE("Failed to allocate new display composition\n"); - return -ENOMEM; -} - -// If the display hasn't been modeset yet, this will be NULL -DrmCrtc *crtc = drm_->GetCrtcForDisplay(display); - -int ret = composition_map_[display]->Init(drm_, crtc, importer_, planner_, - frame_no); -if (ret) { - ALOGE("Failed to init display composition for %d", display); - return ret; -} - } - return 0; -} - -int DrmComposition::SetLayers(size_t num_displays, - DrmCompositionDisplayLayersMap *maps) { - int ret = 0; - for (size_t display_index = 0; display_index < num_displays; - display_index++) { -DrmCompositionDisplayLayersMap &map = maps[display_index]; -int display = map.display; - -if (!drm_->GetConnectorForDisplay(display)) { - ALOGE("Invalid display given to SetLayers %d", display); - continue; -} - -ret = composition_map_[display]->SetLayers( -map.layers.data(), map.layers.size(), map.geometry_changed); -if (ret) - return ret; - } - - return 0; -} - -int DrmComposition::SetDpmsMode(int display, uint32_t dpms_mode) { - return composition_map_[display]->SetDpmsMode(dpms_mode); -} - -int DrmComposition::SetDisplayMode(int display, const DrmMode &display_mode) { - return composition_map_[display]->SetDisplayMode(display_mode); -} - -std::unique_ptr DrmComposition::TakeDisplayComposition( -int display) { - return std::move(composition_map_
[PATCH hwc v2 2/6] drm_hwcomposer: Add support for IN_FENCE_FD property to DrmPlane
Add support for the IN_FENCE_FD property to DrmPlane. Signed-off-by: Robert Foss --- drmplane.cpp | 8 drmplane.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/drmplane.cpp b/drmplane.cpp index c4ea722..1f739ae 100644 --- a/drmplane.cpp +++ b/drmplane.cpp @@ -126,6 +126,10 @@ int DrmPlane::Init() { if (ret) ALOGI("Could not get alpha property"); + ret = drm_->GetPlaneProperty(*this, "IN_FENCE_FD", &in_fence_fd_property_); + if (ret) +ALOGI("Could not get IN_FENCE_FD property"); + return 0; } @@ -188,4 +192,8 @@ const DrmProperty &DrmPlane::rotation_property() const { const DrmProperty &DrmPlane::alpha_property() const { return alpha_property_; } + +const DrmProperty &DrmPlane::in_fence_fd_property() const { + return in_fence_fd_property_; +} } diff --git a/drmplane.h b/drmplane.h index 2e06986..5b73b08 100644 --- a/drmplane.h +++ b/drmplane.h @@ -54,6 +54,7 @@ class DrmPlane { const DrmProperty &src_h_property() const; const DrmProperty &rotation_property() const; const DrmProperty &alpha_property() const; + const DrmProperty &in_fence_fd_property() const; private: DrmResources *drm_; @@ -75,6 +76,7 @@ class DrmPlane { DrmProperty src_h_property_; DrmProperty rotation_property_; DrmProperty alpha_property_; + DrmProperty in_fence_fd_property_; }; } -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 3/6] drm_hwcomposer: Submit in-fence to DRM
Add support for in-fences through the IN_FENCE_FD property. In-fences signal when their associated buffer may be read by DRM/KMS. Signed-off-by: Robert Foss --- drmdisplaycompositor.cpp | 35 --- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index bd670cf..71c0451 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -529,6 +529,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, std::vector &source_layers = comp_plane.source_layers(); int fb_id = -1; +int fence_fd = -1; DrmHwcRect display_frame; DrmHwcRect source_crop; uint64_t rotation = 0; @@ -547,30 +548,12 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, break; } DrmHwcLayer &layer = layers[source_layers.front()]; - if (!test_only && layer.acquire_fence.get() >= 0) { -int acquire_fence = layer.acquire_fence.get(); -int total_fence_timeout = 0; -for (int i = 0; i < kAcquireWaitTries; ++i) { - int fence_timeout = kAcquireWaitTimeoutMs * (1 << i); - total_fence_timeout += fence_timeout; - ret = sync_wait(acquire_fence, fence_timeout); - if (ret) -ALOGW("Acquire fence %d wait %d failed (%d). Total time %d", - acquire_fence, i, ret, total_fence_timeout); - else -break; -} -if (ret) { - ALOGE("Failed to wait for acquire %d/%d", acquire_fence, ret); - break; -} -layer.acquire_fence.Close(); - } if (!layer.buffer) { ALOGE("Expected a valid framebuffer for pset"); break; } fb_id = layer.buffer->fb_id; + fence_fd = layer.acquire_fence.get(); display_frame = layer.display_frame; source_crop = layer.source_crop; if (layer.blending == DrmHwcBlending::kPreMult) @@ -587,7 +570,21 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, rotation |= 1 << DRM_ROTATE_180; else if (layer.transform & DrmHwcTransform::kRotate270) rotation |= 1 << DRM_ROTATE_270; + + if (fence_fd != -1) { +int prop_id = plane->in_fence_fd_property().id(); +if (prop_id == 0) { +ALOGE("Failed to get IN_FENCE_FD property id"); +break; +} +ret = drmModeAtomicAddProperty(pset, plane->id(), prop_id, fence_fd); +if (ret < 0) { + ALOGE("Failed to add IN_FENCE_FD property to pset: %d", ret); + break; +} + } } + // Disable the plane if there's no framebuffer if (fb_id < 0) { ret = drmModeAtomicAddProperty(pset, plane->id(), -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 5/6] drm_hwcomposer: Add GetCrtcCount function
This GetCrtrcCount helper functions enables convenient fetching of the number of Crtcs from DrmResources. Signed-off-by: Robert Foss --- drmresources.cpp | 4 drmresources.h | 1 + 2 files changed, 5 insertions(+) diff --git a/drmresources.cpp b/drmresources.cpp index 762f5ef..0578cc6 100644 --- a/drmresources.cpp +++ b/drmresources.cpp @@ -241,6 +241,10 @@ DrmPlane *DrmResources::GetPlane(uint32_t id) const { return NULL; } +uint32_t DrmResources::GetCrtcCount() const { + return (uint32_t) crtcs_.size(); +} + uint32_t DrmResources::next_mode_id() { return ++mode_id_; } diff --git a/drmresources.h b/drmresources.h index a2d8d16..0cc2456 100644 --- a/drmresources.h +++ b/drmresources.h @@ -66,6 +66,7 @@ class DrmResources { int GetConnectorProperty(const DrmConnector &connector, const char *prop_name, DrmProperty *property); + uint32_t GetCrtcCount() const; uint32_t next_mode_id(); int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id); -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 4/6] drm_hwcomposer: Add FENCE_OUT_PTR property to DrmCrtc
Add support for handling the FENCE_OUT_PTR property to DrmCrtc Signed-off-by: Robert Foss --- drmcrtc.cpp | 10 ++ drmcrtc.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/drmcrtc.cpp b/drmcrtc.cpp index 1fbdc12..c139869 100644 --- a/drmcrtc.cpp +++ b/drmcrtc.cpp @@ -51,6 +51,12 @@ int DrmCrtc::Init() { ALOGE("Failed to get MODE_ID property"); return ret; } + + ret = drm_->GetCrtcProperty(*this, "OUT_FENCE_PTR", &out_fence_ptr_property_); + if (ret) { +ALOGE("Failed to get OUT_FENCE_PTR property"); +return ret; + } return 0; } @@ -81,4 +87,8 @@ const DrmProperty &DrmCrtc::active_property() const { const DrmProperty &DrmCrtc::mode_property() const { return mode_property_; } + +const DrmProperty &DrmCrtc::out_fence_ptr_property() const { + return out_fence_ptr_property_; +} } diff --git a/drmcrtc.h b/drmcrtc.h index ad95352..2e8c811 100644 --- a/drmcrtc.h +++ b/drmcrtc.h @@ -45,6 +45,7 @@ class DrmCrtc { const DrmProperty &active_property() const; const DrmProperty &mode_property() const; + const DrmProperty &out_fence_ptr_property() const; private: DrmResources *drm_; @@ -63,6 +64,7 @@ class DrmCrtc { DrmProperty active_property_; DrmProperty mode_property_; + DrmProperty out_fence_ptr_property_; }; } -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 6/6] drm_hwcomposer: Add out-fence support
Add support for out-fences through the OUT_FENCE_PTR property. Out-fences signal when their associated buffer may be read by a device. Signed-off-by: Robert Foss --- Changes since v1: Sergi Granell - Set atomic property to be out_fences[crtc->pipe()] not out_fences[0] drmdisplaycomposition.h | 9 + drmdisplaycompositor.cpp | 16 drmhwctwo.cpp| 9 ++--- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h index b165adc..0586d58 100644 --- a/drmdisplaycomposition.h +++ b/drmdisplaycomposition.h @@ -189,6 +189,14 @@ class DrmDisplayComposition { return planner_; } + int take_out_fence() { +return out_fence_.Release(); + } + + void set_out_fence(int out_fence) { +out_fence_.Set(dup(out_fence)); + } + void Dump(std::ostringstream *out) const; private: @@ -215,6 +223,7 @@ class DrmDisplayComposition { int timeline_current_ = 0; int timeline_squash_done_ = 0; int timeline_pre_comp_done_ = 0; + UniqueFd out_fence_ = -1; bool geometry_changed_; std::vector layers_; diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index 71c0451..a1427d3 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -492,6 +492,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, display_comp->composition_planes(); std::vector &pre_comp_regions = display_comp->pre_comp_regions(); + uint64_t out_fences[drm_->GetCrtcCount()]; DrmConnector *connector = drm_->GetConnectorForDisplay(display_); if (!connector) { @@ -510,6 +511,16 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, return -ENOMEM; } + if (crtc->out_fence_ptr_property().id() != 0) { +ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->out_fence_ptr_property().id(), + (uint64_t) &out_fences[crtc->pipe()]); +if (ret < 0) { + ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret); + drmModeAtomicFree(pset); + return ret; +} + } + if (mode_.needs_modeset) { ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->mode_property().id(), mode_.blob_id) < 0 || @@ -708,6 +719,11 @@ out: mode_.needs_modeset = false; } + if (crtc->out_fence_ptr_property().id()) { +display_comp->set_out_fence((int) out_fences[crtc->pipe()]); +close((int) out_fences[crtc->pipe()]); + } + return ret; } diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp index 762ee8c..89399bf 100644 --- a/drmhwctwo.cpp +++ b/drmhwctwo.cpp @@ -557,19 +557,14 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { i = overlay_planes.erase(i); } + AddFenceToRetireFence(composition->take_out_fence()); + ret = compositor_.ApplyComposition(std::move(composition)); if (ret) { ALOGE("Failed to apply the frame composition ret=%d", ret); return HWC2::Error::BadParameter; } - // Now that the release fences have been generated by the compositor, make - // sure they're managed properly - for (std::pair &l : z_map) { -l.second->manage_release_fence(); -AddFenceToRetireFence(l.second->release_fence()); - } - // The retire fence returned here is for the last frame, so return it and // promote the next retire fence *retire_fence = retire_fence_.Release(); -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Call sysfs_notify after changing drm_connector::dpms
On Wed, Sep 27, 2017 at 12:20:21PM +0200, Karsten Wiese wrote: > 2017-09-27 9:18 GMT+02:00 Jani Nikula : > > > On Wed, 27 Sep 2017, Karsten Wiese wrote: > > > 2017-09-25 15:48 GMT+02:00 Jani Nikula : > > > > > >> On Tue, 19 Sep 2017, Karsten Wiese wrote: > > >> > This makes poll work for the > > >> > /sys/class/drm/cardX/connectorY/dpms attributes. > > >> > > >> I guess the question is, what are you trying to achieve? What is the > > >> problem that this solves? > > >> > > > > > > The patch enables cpu cycle less waiting for dpms flag changes. > > > > > > Here it lets a screen brightness setting daemon know which monitor to > > > handle. One of the attached screens gets confused if it is set just > > > after being switched on, hence the daemon leaves it untouched until it > > > has been active for some seconds. > > > > Screen brightness settings daemon? > > > Running on a laptop lacking an ambient light sensor employing it's webcam > instead to measure ambient light and adjust monitors' brightnesses > accordingly. > > > What exactly do you mean by "if it is > > set"? What interface are you using to change brightness? > > The external monitor's brightness is adjusted by DDC/CI via the /dev/i2c-x > the monitor is attached to. > I there a saner interface to use? There's supposed to be a backlight property on the panel, exposed by the X driver. If there's not, then that needs to be fixed/adjusted. There's also plans to directly link that up on the kernel side, but that's a bit more involved due to how screwed up the backlight driver design on linux is right now. -Daniel > > What happens > > when the display "gets confused"? > > > The monitor wrongly toggles sharpness if it receives the brightness > adjusting > DDC/CI soon after resuming from power saving state. > > > > > My first instinct is that you're proposing a new kernel ABI to solve an > > issue you shouldn't be solving in userspace to begin with. > > Calculating ambient light from pictures acquired via > v4l2 in kernel seamed wrong to me. > > Or that > > perhaps the userspace should be doing this in cooperation with the drm > > master, not standalone. > > > Is there a way to call into the drm-master (xscreensaver/xserver here > ) with the call only returning if a monitor's power state changed? > > There is DPMSInfo, but it returns immediately rendering the daemon less > efficient. > > > BR, > > Karsten > > > > > BR, > > Jani. > > > > > > > > Without the patch the daemon would have to actively read the dpms flag > > > every once in a while. > > > > > > > > >> > > >> We have zero sysfs_notify in all of drm AFAICT. > > > > > > > > > Yes I noticed too and looked for some dbus signal to listen to but > > didn't > > > find any. > > > > > > BR, > > > Karsten > > > > > >> > > >> BR, > > >> Jani. > > >> > > >> > > >> > > > >> > Tested with i915 suspended by XScreenServer and > > >> > suspend to RAM. > > >> > > > >> > Signed-off-by: Karsten Wiese > > >> > --- > > >> > drivers/gpu/drm/drm_atomic.c| 4 > > >> > drivers/gpu/drm/drm_atomic_helper.c | 6 +- > > >> > 2 files changed, 9 insertions(+), 1 deletion(-) > > >> > > > >> > diff --git a/drivers/gpu/drm/drm_atomic.c > > b/drivers/gpu/drm/drm_atomic.c > > >> > index 2fd383d..b6fa87b 100644 > > >> > --- a/drivers/gpu/drm/drm_atomic.c > > >> > +++ b/drivers/gpu/drm/drm_atomic.c > > >> > @@ -1880,6 +1880,10 @@ int drm_atomic_connector_commit_dpms(struct > > >> drm_atomic_state *state, > > >> > out: > > >> > if (ret != 0) > > >> > connector->dpms = old_mode; > > >> > + else > > >> > + if (connector->dpms != old_mode) > > >> > + sysfs_notify(&connector->kdev->kobj, NULL, > > >> "dpms"); > > >> > + > > >> > return ret; > > >> > } > > >> > > > >> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > >> b/drivers/gpu/drm/drm_atomic_helper.c > > >> > index 4e53aae..6198772 100644 > > >> > --- a/drivers/gpu/drm/drm_atomic_helper.c > > >> > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > >> > @@ -921,12 +921,16 @@ drm_atomic_helper_update_ > > legacy_modeset_state(struct > > >> drm_device *dev, > > >> > crtc = new_conn_state->crtc; > > >> > if ((!crtc && old_conn_state->crtc) || > > >> > (crtc && drm_atomic_crtc_needs_modeset( > > crtc->state))) > > >> { > > >> > - int mode = DRM_MODE_DPMS_OFF; > > >> > + int old_mode, mode = DRM_MODE_DPMS_OFF; > > >> > > > >> > if (crtc && crtc->state->active) > > >> > mode = DRM_MODE_DPMS_ON; > > >> > > > >> > + old_mode = connector->dpms; > > >> > connector->dpms = mode; > > >> > + if (old_mode != mode) > > >> > + sysfs_notify(&connector->kdev->kobj, > > >> > + NULL, "dpms"); > > >> > } > >
Re: [PATCH 8/8] etnaviv: remove IOMMU dependency
On Fri, Sep 15, 2017 at 07:04:39PM +0200, Lucas Stach wrote: > Using the IOMMU API to manage the internal GPU MMU has been an > historical accident and it keeps getting in the way, as well as > entangling the driver with the inner workings of the IOMMU > subsystem. > > Clean this up by removing the usage of iommu_domain, which is the > last piece linking etnaviv to the IOMMU subsystem. > > Signed-off-by: Lucas Stach Reviewed-by: Wladimir J. van der Laan > --- > drivers/gpu/drm/etnaviv/Kconfig| 2 - > drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 - > drivers/gpu/drm/etnaviv/etnaviv_iommu.c| 71 > +++--- > drivers/gpu/drm/etnaviv/etnaviv_iommu.h| 6 ++- > drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c | 55 +++ > drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 38 +++- > drivers/gpu/drm/etnaviv/etnaviv_mmu.h | 20 ++--- > 7 files changed, 93 insertions(+), 100 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/8] etnaviv: mmu: mark local functions static
On Fri, Sep 15, 2017 at 07:04:38PM +0200, Lucas Stach wrote: > And clean up the header file a bit. > > Signed-off-by: Lucas Stach Reviewed-by: Wladimir J. van der Laan > --- > drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 8 > drivers/gpu/drm/etnaviv/etnaviv_mmu.h | 8 +--- > 2 files changed, 5 insertions(+), 11 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/8] etnaviv: mmu: stop using iommu map/unmap functions
On Fri, Sep 15, 2017 at 07:04:37PM +0200, Lucas Stach wrote: > This is a preparation to remove the etnaviv dependency on the IOMMU > subsystem by importing the relevant parts of the iommu map/unamp > functions into the driver. > > Signed-off-by: Lucas Stach Reviewed-by: Wladimir J. van der Laan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/8] etnaviv: iommuv1: fold pgtable_write into callers
On Fri, Sep 15, 2017 at 07:04:36PM +0200, Lucas Stach wrote: > A function doing a single assignment is not really helping the > code flow. > > Signed-off-by: Lucas Stach agreed. Reviewed-by: Wladimir J. van der Laan > --- > drivers/gpu/drm/etnaviv/etnaviv_iommu.c | 16 > 1 file changed, 4 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > index 78a7c0f3064a..43a0508bdbd7 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > @@ -49,15 +49,6 @@ static struct etnaviv_iommu_domain > *to_etnaviv_domain(struct iommu_domain *domai > return container_of(domain, struct etnaviv_iommu_domain, domain); > } > > -static void pgtable_write(struct etnaviv_iommu_domain_pgtable *pgtable, > - unsigned long iova, phys_addr_t paddr) > -{ > - /* calcuate index into page table */ > - unsigned int index = (iova - GPU_MEM_START) / SZ_4K; > - > - pgtable->pgtable[index] = paddr; > -} > - > static int __etnaviv_iommu_init(struct etnaviv_iommu_domain *etnaviv_domain) > { > u32 *p; > @@ -111,11 +102,12 @@ static int etnaviv_iommuv1_map(struct iommu_domain > *domain, unsigned long iova, > phys_addr_t paddr, size_t size, int prot) > { > struct etnaviv_iommu_domain *etnaviv_domain = to_etnaviv_domain(domain); > + unsigned int index = (iova - GPU_MEM_START) / SZ_4K; > > if (size != SZ_4K) > return -EINVAL; > > - pgtable_write(&etnaviv_domain->pgtable, iova, paddr); > + etnaviv_domain->pgtable.pgtable[index] = paddr; > > return 0; > } > @@ -124,12 +116,12 @@ static size_t etnaviv_iommuv1_unmap(struct iommu_domain > *domain, > unsigned long iova, size_t size) > { > struct etnaviv_iommu_domain *etnaviv_domain = to_etnaviv_domain(domain); > + unsigned int index = (iova - GPU_MEM_START) / SZ_4K; > > if (size != SZ_4K) > return -EINVAL; > > - pgtable_write(&etnaviv_domain->pgtable, iova, > - etnaviv_domain->bad_page_dma); > + etnaviv_domain->pgtable.pgtable[index] = etnaviv_domain->bad_page_dma; > > return SZ_4K; > } > -- > 2.11.0 > > ___ > etnaviv mailing list > etna...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/etnaviv ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/8] etnaviv: remove iova_to_phys iommu ops
On Fri, Sep 15, 2017 at 07:04:33PM +0200, Lucas Stach wrote: > They are not used in any way, so can go away. > > Signed-off-by: Lucas Stach Reviewed-by: Wladimir J. van der Laan > --- > drivers/gpu/drm/etnaviv/etnaviv_iommu.c| 21 - > drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c | 14 -- > 2 files changed, 35 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > index 7a7c97f599d7..f804c0aaa7a2 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > @@ -66,18 +66,6 @@ static void pgtable_free(struct > etnaviv_iommu_domain_pgtable *pgtable, > dma_free_coherent(NULL, size, pgtable->pgtable, pgtable->paddr); > } > > -static u32 pgtable_read(struct etnaviv_iommu_domain_pgtable *pgtable, > -unsigned long iova) > -{ > - /* calcuate index into page table */ > - unsigned int index = (iova - GPU_MEM_START) / SZ_4K; > - phys_addr_t paddr; > - > - paddr = pgtable->pgtable[index]; > - > - return paddr; > -} > - > static void pgtable_write(struct etnaviv_iommu_domain_pgtable *pgtable, > unsigned long iova, phys_addr_t paddr) > { > @@ -164,14 +152,6 @@ static size_t etnaviv_iommuv1_unmap(struct iommu_domain > *domain, > return SZ_4K; > } > > -static phys_addr_t etnaviv_iommu_iova_to_phys(struct iommu_domain *domain, > - dma_addr_t iova) > -{ > - struct etnaviv_iommu_domain *etnaviv_domain = to_etnaviv_domain(domain); > - > - return pgtable_read(&etnaviv_domain->pgtable, iova); > -} > - > static size_t etnaviv_iommuv1_dump_size(struct iommu_domain *domain) > { > return PT_SIZE; > @@ -189,7 +169,6 @@ static const struct etnaviv_iommu_ops etnaviv_iommu_ops = > { > .domain_free = etnaviv_domain_free, > .map = etnaviv_iommuv1_map, > .unmap = etnaviv_iommuv1_unmap, > - .iova_to_phys = etnaviv_iommu_iova_to_phys, > .pgsize_bitmap = SZ_4K, > }, > .dump_size = etnaviv_iommuv1_dump_size, > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c > b/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c > index cbe447ac5974..d794e8c0dd7e 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c > @@ -97,19 +97,6 @@ static size_t etnaviv_iommuv2_unmap(struct iommu_domain > *domain, > return SZ_4K; > } > > -static phys_addr_t etnaviv_iommuv2_iova_to_phys(struct iommu_domain *domain, > - dma_addr_t iova) > -{ > - struct etnaviv_iommuv2_domain *etnaviv_domain = > - to_etnaviv_domain(domain); > - int mtlb_entry, stlb_entry; > - > - mtlb_entry = (iova & MMUv2_MTLB_MASK) >> MMUv2_MTLB_SHIFT; > - stlb_entry = (iova & MMUv2_STLB_MASK) >> MMUv2_STLB_SHIFT; > - > - return etnaviv_domain->stlb_cpu[mtlb_entry][stlb_entry] & ~(SZ_4K - 1); > -} > - > static int etnaviv_iommuv2_init(struct etnaviv_iommuv2_domain > *etnaviv_domain) > { > u32 *p; > @@ -235,7 +222,6 @@ static const struct etnaviv_iommu_ops etnaviv_iommu_ops = > { > .domain_free = etnaviv_iommuv2_domain_free, > .map = etnaviv_iommuv2_map, > .unmap = etnaviv_iommuv2_unmap, > - .iova_to_phys = etnaviv_iommuv2_iova_to_phys, > .pgsize_bitmap = SZ_4K, > }, > .dump_size = etnaviv_iommuv2_dump_size, > -- > 2.11.0 > > ___ > etnaviv mailing list > etna...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/etnaviv ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/8] etnaviv: iommuv1: remove map_lock
On Fri, Sep 15, 2017 at 07:04:35PM +0200, Lucas Stach wrote: > It wasn't protecting anything, as the single word writes used to > set up or tear down a translation are already inherently atomic, > so the spinlock is pure overhead. > > Signed-off-by: Lucas Stach Reviewed-by: Wladimir J. van der Laan > --- > drivers/gpu/drm/etnaviv/etnaviv_iommu.c | 7 --- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > index aaa8c4136f53..78a7c0f3064a 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > @@ -42,7 +42,6 @@ struct etnaviv_iommu_domain { > void *bad_page_cpu; > dma_addr_t bad_page_dma; > struct etnaviv_iommu_domain_pgtable pgtable; > - spinlock_t map_lock; > }; > > static struct etnaviv_iommu_domain *to_etnaviv_domain(struct iommu_domain > *domain) > @@ -90,8 +89,6 @@ static int __etnaviv_iommu_init(struct etnaviv_iommu_domain > *etnaviv_domain) > etnaviv_domain->pgtable.pgtable[i] = > etnaviv_domain->bad_page_dma; > > - spin_lock_init(&etnaviv_domain->map_lock); > - > return 0; > } > > @@ -118,9 +115,7 @@ static int etnaviv_iommuv1_map(struct iommu_domain > *domain, unsigned long iova, > if (size != SZ_4K) > return -EINVAL; > > - spin_lock(&etnaviv_domain->map_lock); > pgtable_write(&etnaviv_domain->pgtable, iova, paddr); > - spin_unlock(&etnaviv_domain->map_lock); > > return 0; > } > @@ -133,10 +128,8 @@ static size_t etnaviv_iommuv1_unmap(struct iommu_domain > *domain, > if (size != SZ_4K) > return -EINVAL; > > - spin_lock(&etnaviv_domain->map_lock); > pgtable_write(&etnaviv_domain->pgtable, iova, > etnaviv_domain->bad_page_dma); > - spin_unlock(&etnaviv_domain->map_lock); > > return SZ_4K; > } > -- > 2.11.0 > > ___ > etnaviv mailing list > etna...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/etnaviv ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/8] etnaviv: iommuv1: fold pagetable alloc and free into caller
On Fri, Sep 15, 2017 at 07:04:34PM +0200, Lucas Stach wrote: > Those functions are simple enough to fold them into the calling > function. This also fixes a correctness issue, as the alloc/free > functions didn't specifiy the device the memory was allocated for. > > Signed-off-by: Lucas Stach Reviewed-by: Wladimir J. van der Laan > --- > drivers/gpu/drm/etnaviv/etnaviv_iommu.c | 27 --- > 1 file changed, 8 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > index f804c0aaa7a2..aaa8c4136f53 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c > @@ -50,22 +50,6 @@ static struct etnaviv_iommu_domain > *to_etnaviv_domain(struct iommu_domain *domai > return container_of(domain, struct etnaviv_iommu_domain, domain); > } > > -static int pgtable_alloc(struct etnaviv_iommu_domain_pgtable *pgtable, > - size_t size) > -{ > - pgtable->pgtable = dma_alloc_coherent(NULL, size, &pgtable->paddr, > GFP_KERNEL); > - if (!pgtable->pgtable) > - return -ENOMEM; > - > - return 0; > -} > - > -static void pgtable_free(struct etnaviv_iommu_domain_pgtable *pgtable, > - size_t size) > -{ > - dma_free_coherent(NULL, size, pgtable->pgtable, pgtable->paddr); > -} > - > static void pgtable_write(struct etnaviv_iommu_domain_pgtable *pgtable, > unsigned long iova, phys_addr_t paddr) > { > @@ -91,8 +75,11 @@ static int __etnaviv_iommu_init(struct > etnaviv_iommu_domain *etnaviv_domain) > for (i = 0; i < SZ_4K / 4; i++) > *p++ = 0xdead55aa; > > - ret = pgtable_alloc(&etnaviv_domain->pgtable, PT_SIZE); > - if (ret < 0) { > + etnaviv_domain->pgtable.pgtable = > + dma_alloc_coherent(etnaviv_domain->dev, PT_SIZE, > +&etnaviv_domain->pgtable.paddr, > +GFP_KERNEL); > + if (!etnaviv_domain->pgtable.pgtable) { > dma_free_coherent(etnaviv_domain->dev, SZ_4K, > etnaviv_domain->bad_page_cpu, > etnaviv_domain->bad_page_dma); > @@ -112,7 +99,9 @@ static void etnaviv_domain_free(struct iommu_domain > *domain) > { > struct etnaviv_iommu_domain *etnaviv_domain = to_etnaviv_domain(domain); > > - pgtable_free(&etnaviv_domain->pgtable, PT_SIZE); > + dma_free_coherent(etnaviv_domain->dev, PT_SIZE, > + etnaviv_domain->pgtable.pgtable, > + etnaviv_domain->pgtable.paddr); > > dma_free_coherent(etnaviv_domain->dev, SZ_4K, > etnaviv_domain->bad_page_cpu, > -- > 2.11.0 > > ___ > etnaviv mailing list > etna...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/etnaviv ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/8] etnaviv: remove iommu fault handler
On Fri, Sep 15, 2017 at 07:04:32PM +0200, Lucas Stach wrote: > The handler has never been used, so it's really just dead code. > > Signed-off-by: Lucas Stach Reviewed-by: Wladimir J. van der Laan > --- > drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 9 - > 1 file changed, 9 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > index f103e787de94..f3ed07db9b2d 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > @@ -22,13 +22,6 @@ > #include "etnaviv_iommu.h" > #include "etnaviv_mmu.h" > > -static int etnaviv_fault_handler(struct iommu_domain *iommu, struct device > *dev, > - unsigned long iova, int flags, void *arg) > -{ > - DBG("*** fault: iova=%08lx, flags=%d", iova, flags); > - return 0; > -} > - > int etnaviv_iommu_map(struct etnaviv_iommu *iommu, u32 iova, > struct sg_table *sgt, unsigned len, int prot) > { > @@ -307,8 +300,6 @@ struct etnaviv_iommu *etnaviv_iommu_new(struct > etnaviv_gpu *gpu) > mmu->domain->geometry.aperture_end - > mmu->domain->geometry.aperture_start + 1); > > - iommu_set_fault_handler(mmu->domain, etnaviv_fault_handler, gpu->dev); > - > return mmu; > } > > -- > 2.11.0 > > ___ > etnaviv mailing list > etna...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/etnaviv ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/tinydrm: Move backlight helpers to a separate file
On Wed, Sep 27, 2017 at 10:49:15AM +0200, Daniel Vetter wrote: > On Tue, Sep 26, 2017 at 03:53:33PM +0200, Noralf Trønnes wrote: > > > > Den 26.09.2017 15.06, skrev Noralf Trønnes: > > > > > > Den 26.09.2017 13.32, skrev Daniel Vetter: > > > > On Tue, Sep 26, 2017 at 04:46:53PM +0530, Meghana Madhyastha wrote: > > > > > On Mon, Sep 25, 2017 at 06:31:58PM +0200, Noralf Trønnes wrote: > > > > > > Den 25.09.2017 16.56, skrev Noralf Trønnes: > > > > > > > Hi Meghana, > > > > > > > > > > > > > > > > > > > > > Den 22.09.2017 17.09, skrev Meghana Madhyastha: > > > > > > > > Move backlight helpers from tinydrm-helpers.c to > > > > > > > > tinydrm-backlight.c. This is because it is organizationally > > > > > > > > simpler to understand and advantageous to group functions > > > > > > > > performing a similar function to a separate file as opposed to > > > > > > > > having one helper file with heteregenous helper functions. > > > > > > > > > > > > > > > > Signed-off-by: Meghana Madhyastha > > > > > > > > --- > > > > > > > I don't think there is much gain in just moving the code like > > > > > > > this. > > > > > > > > > > > > > > The idea is to add a drm_backlight helper that can be useful for > > > > > > > all > > > > > > > DRM drivers that use the backlight subsystem. > > > > > Yes I agree. That definitely makes more sense. > > > > > > The full path to that helper would be: > > > > > > drivers/gpu/drm/drm_backlight.c > > > > > > > > > > > > > This is what the TODO says: > > > > > > > https://dri.freedesktop.org/docs/drm/gpu/todo.html#tinydrm > > > > > > > > > > > > > > - backlight helpers, probably best to put them into a > > > > > > > new drm_backlight.c. > > > > > > > This is because drivers/video is de-facto > > > > > > > unmaintained. We could also > > > > > > > move drivers/video/backlight to drivers/gpu/backlight > > > > > > > and take it all > > > > > > > over within drm-misc, but that’s more work. > > > > > > > > > > > > > > There is also this discussion to take into account: > > > > > > > KMS backlight ABI proposition > > > > > > > https://lists.freedesktop.org/archives/dri-devel/2017-February/133206.html > > > > > > > > > > > > > > > > > > > > > > > > > > > > I don't remember what came out of that discussion. > > > > > > > > > > > > > > Noralf. > > > > > After having discussed this with Daniel on the #dri-devel irc channel, > > > > > here are some of the points suggested. > > > > > > > > > > Daniel suggested that I first look into the usage of shared backlight > > > > > helpers in drm (specifically backlight_update_status to begin > > > > > with). The idea > > > > > was to see whether there is any pattern in usage and/or code > > > > > dupication. > > > > > If there is, then the next step would be to write helper > > > > > functions which > > > > > can be used by other drivers (and not just tinydrm). > > > > > > > > > > To start with, I went through the instances of backlight_update_status > > > > > in the drm code, and made the following observations(most of them are > > > > > very simple/naive observations). > > > > > > > > > > - backlight_update_status is usually called in backlight init (and > > > > > sometimes exit) functions of the drivers just after the > > > > > device is registered. > > > > > So backlight_update_status is called with the registered > > > > > device as the > > > > > parameter. > > > > > > > > > > Here are the following cases of properties changed/set before > > > > > backlight_update_status is called. > > > > > > > > > > - CASE 1: Brightness is changed (either a macro > > > > > BRIGHTNESS_MAX_LEVEL 100 > > > > > is defined or it is manually set) This happens in the > > > > > following files: > > > > > > > > > > gma500/cdv_device.c, gma500/mdfld_device.c, > > > > > gma500/oaktrail_device.c, > > > > > gma500/psb_device.c, noveau/noveau_backlight.c(here > > > > > brightness is determined by fuction > > > > > static int nv50_get_intensity) > > > > > > > > > > - CASE 2: Power property is set (to FB_BLANK_UNBLANK mostly) > > > > > This happens in the following files: > > > > > > > > > > omapdrm/displays/panel-dpi.c, panel/panel-innolux-p079zca.c, > > > > > panel/panel-jdi-lt070me05000.c, panel/panel-sharp-lq101r1sx01.c, > > > > > panel/panel-sharp-ls043t1le01.c, tilcdc/tilcdc_panel.c > > > > > - CASE 3: State is set > > > > > This happens in the following files: > > > > > > > > > > tinydrm/tinydrm-helpers.c > > > > > > > > > > - CASE 4: Power and brightness properties are set > > > > > This happens in the following files: > > > > > > > > > > atombios_encoders.c, radeon/radeon_legacy_encoders.c, > > > > > shmobile/shmob_drm_backlight.c > > > > > > > > > > - CASE 5: Power and the state properties are set > > > > > This happens in the following files: > > > > > > > > > > panel/panel-lvds.c, panel/panel-panasonic-vvx10f034n00.c, > > > > > panel/panel-simple.c, panel/panel-sitronix-st7789v.c > >
[PATCH] drm/Documentation: Refine TODO for backlight helpers in tinydrm
Add a summary which resulted from discussions on what should be done to refactor backlight helpers in tinydrm so that they can be used in other drivers as well. Signed-off-by: Meghana Madhyastha --- Documentation/gpu/todo.rst | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 22af55d..1acf84fe 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -353,7 +353,16 @@ those drivers as simple as possible, so lots of room for refactoring: - backlight helpers, probably best to put them into a new drm_backlight.c. This is because drivers/video is de-facto unmaintained. We could also move drivers/video/backlight to drivers/gpu/backlight and take it all - over within drm-misc, but that's more work. + over within drm-misc, but that's more work. Backlight helpers require a fair + bit of reworking and refactoring. A simple example is the enabling of a backlight. + Tinydrm has helpers for this. It would be good if other drivers can also use the + helper. However, there are various cases we need to consider i.e different + drivers seem to have different ways of enabling/disabling a backlight. + We also need to consider the backlight drivers (like gpio_backlight). The situation + is further complicated by the fact that the backlight is tied to fbdev + via fb_notifier_callback() which has complicated logic. For further details, refer + to the following discussion thread: + https://groups.google.com/forum/#!topic/outreachy-kernel/8rBe30lwtdA - spi helpers, probably best put into spi core/helper code. Thierry said the spi maintainer is fast&reactive, so shouldn't be a big issue. -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Call sysfs_notify after changing drm_connector::dpms
2017-09-27 10:45 GMT+02:00 Daniel Vetter : > On Wed, Sep 27, 2017 at 12:29:46AM +0200, Karsten Wiese wrote: > > 2017-09-25 15:48 GMT+02:00 Jani Nikula : > > > > > On Tue, 19 Sep 2017, Karsten Wiese wrote: > > > > This makes poll work for the > > > > /sys/class/drm/cardX/connectorY/dpms attributes. > > > > > > I guess the question is, what are you trying to achieve? What is the > > > problem that this solves? > > > > > > > The patch enables cpu cycle less waiting for dpms flag changes. > > > > Here it lets a screen brightness setting daemon know which monitor to > > handle. > > One of the attached screens gets confused if it is set just after being > > switched on, > > hence the daemon leaves it untouched until it has been active for some > > seconds. > > Without knowing more details, but a brightness deamon in userspace sounds > rather wrong. The driver is supposed to take care of this and shut down > the backlight when we disable the output for a panel. > > > Without the patch the daemon would have to actively read the dpms flag > > every once in a while. > > On top of that, any uabi changes need open source userspace. > Will put publishing userspace on my list, if proposed uabi change is agreed upon to be accepted once userspace is open sourced. Or if I find another way of knowing monitors' power states using cpu cycles only if said states have changed. Anybody? BR, Karsten -Daniel > > > > > > > > > > > We have zero sysfs_notify in all of drm AFAICT. > > > > > > Yes I noticed too and looked for some dbus signal to listen to but > didn't > > find any. > > > > BR, > > Karsten > > > > > > > > BR, > > > Jani. > > > > > > > > > > > > > > Tested with i915 suspended by XScreenServer and > > > > suspend to RAM. > > > > > > > > Signed-off-by: Karsten Wiese > > > > --- > > > > drivers/gpu/drm/drm_atomic.c| 4 > > > > drivers/gpu/drm/drm_atomic_helper.c | 6 +- > > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c > b/drivers/gpu/drm/drm_atomic.c > > > > index 2fd383d..b6fa87b 100644 > > > > --- a/drivers/gpu/drm/drm_atomic.c > > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > > @@ -1880,6 +1880,10 @@ int drm_atomic_connector_commit_dpms(struct > > > drm_atomic_state *state, > > > > out: > > > > if (ret != 0) > > > > connector->dpms = old_mode; > > > > + else > > > > + if (connector->dpms != old_mode) > > > > + sysfs_notify(&connector->kdev->kobj, NULL, > > > "dpms"); > > > > + > > > > return ret; > > > > } > > > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > > b/drivers/gpu/drm/drm_atomic_helper.c > > > > index 4e53aae..6198772 100644 > > > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > > @@ -921,12 +921,16 @@ drm_atomic_helper_update_ > legacy_modeset_state(struct > > > drm_device *dev, > > > > crtc = new_conn_state->crtc; > > > > if ((!crtc && old_conn_state->crtc) || > > > > (crtc && drm_atomic_crtc_needs_modeset( > crtc->state))) > > > { > > > > - int mode = DRM_MODE_DPMS_OFF; > > > > + int old_mode, mode = DRM_MODE_DPMS_OFF; > > > > > > > > if (crtc && crtc->state->active) > > > > mode = DRM_MODE_DPMS_ON; > > > > > > > > + old_mode = connector->dpms; > > > > connector->dpms = mode; > > > > + if (old_mode != mode) > > > > + sysfs_notify(&connector->kdev->kobj, > > > > + NULL, "dpms"); > > > > } > > > > } > > > > > > -- > > > Jani Nikula, Intel Open Source Technology Center > > > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 8/8] phy: rockchip: add inno hdmi phy to makefile
Hi Algea, [auto build test ERROR on rockchip/for-next] [also build test ERROR on v4.14-rc2 next-20170927] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Algea-Cao/drm-rockchip-dw_hdmi-update-dw_hdmi_rockchip_dt_ids/20170927-114356 base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): >> make[4]: *** No rule to make target >> 'drivers/phy/rockchip/phy-rockchip-inno-hdmi-phy.c', needed by >> 'drivers/phy/rockchip/phy-rockchip-inno-hdmi-phy.o'. make[4]: Target '__build' not remade because of errors. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd: DC pull request review
Ok, here's one more attempt at scrolling through 130k diff. Overall verdict from me is that DC is big project, and like any big project it's never done. So at least for me the goal isn't to make things perfect, becaue if that's the hoop to jump through we wouldn't have any gpu drivers at all. More important is whether merging a new driver base will benefit the overall subsystem, and here this primarily means whether the DC team understands how upstream works and is designed, and whether the code is largely aligned with upstream (especially the atomic modeset) architecture. Looking back over the last two years I think that's the case now, so Acked-by: Daniel Vetter for merging this pull. While scrolling through the pull I spotted a bunch more things that should be refactored, but most of these will be a real pain with DC is out of tree, and much easier in tree since in many of these areas the in-tree helpers aren't up to snuff yet for what DC needs. That kind of work is best done when there's one tree with everything integrated. That's also why I think we should merge DC into drm-next directly, so we can get started on the integration polish right away. That has a bit higher risk of Linus having a spazz, so here's my recommendation for merging: - There's a few additions to drm_dp_helper.h sprinkled all over the pull. I think those should be put into a patch of it's own, and merged first. No need to rebase DC, git merge will dtrt and not end up with duplicates. - dm_alloc/realloc/free is something Dave Airlie noticed, and I agree it's an easy red flag that might upset Linus. cocci can fix this easy, so no real problem I think to patch up in one big patch (I thought we've had a "remove malloc wrappers" todo item in the very first review, apparently there was more than one such wrapper). - The history is huge, but AMD folks want to keep it if possible, and I see the value in that. Would be good to get an ack from Linus for that (but shouldn't be an issue, not the first time we've merged the full history of out-of-tree work). Short&longer term TODO items are still tracked, might be a good idea to integrate those the overall drm todo in our gpu documentation, for more visibility. So in a way this is kinda like staging, except not with the horribly broken process of having an entirely separate tree for staging drivers which just makes refactoring needlessly painful (which defeats the point of staging really). So staging-within-the-subsystem. We've had that before, with early nouveau. And yes some of the files are utterly horrible to read and not anything close to kernel coding style standards. But that's the point, they're essentially gospel from hw engineers that happens to be parseable by gcc. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/amd/display/TODO | 31 +++ 1 file changed, 31 insertions(+) diff --git a/drivers/gpu/drm/amd/display/TODO b/drivers/gpu/drm/amd/display/TODO index 2737873db12d..eea645b102a1 100644 --- a/drivers/gpu/drm/amd/display/TODO +++ b/drivers/gpu/drm/amd/display/TODO @@ -79,3 +79,34 @@ TODOs 12. drm_modeset_lock in MST should no longer be needed in recent kernels * Adopt appropriate locking scheme + +13. get_modes and best_encoder callbacks look a bit funny. Can probably rip out +a few indirections, and consider removing entirely and using the +drm_atomic_helper_best_encoder default behaviour. + +14. core/dc_debug.c, consider switching to the atomic state debug helpers and +moving all your driver state printing into the various atomic_print_state +callbacks. There's also plans to expose this stuff in a standard way across all +drivers, to make debugging userspace compositors easier across different hw. + +15. Move DP/HDMI dual mode adaptors to drm_dp_dual_mode_helper.c. + +16. Move to core SCDC helpers (I think those are new since initial DC review). + +17. There's still a pretty massive layer cake around dp aux and DPCD handling, +with like 3 levels of abstraction and using your own structures instead of the +stuff in drm_dp_helper.h. drm_dp_helper.h isn't really great and already has 2 +incompatible styles, just means more reasons not to add a third (or well third +one gets to do the cleanup refactor). + +18. There's a pile of sink handling code, both for DP and HDMI where I didn't +immediately recognize the standard. I think long term it'd be best for the drm +subsystem if we try to move as much of that into helpers/core as possible, and +share it with drivers. But that's a very long term goal, and by far not just an +issue with DC - other drivers, especially around DP sink handling, are equally +guilty. + +19. The DC logger is still a rather sore thing, but I know that the DRM_DEBUG +stuff just isn't up to the challenges either. We need to figure out something +that integrates better with DRM and linux debug printing, while not being +useless with filtering output. dynamic debug printing might be an option.
Re: [PATCH] drm: Call sysfs_notify after changing drm_connector::dpms
2017-09-27 9:18 GMT+02:00 Jani Nikula : > On Wed, 27 Sep 2017, Karsten Wiese wrote: > > 2017-09-25 15:48 GMT+02:00 Jani Nikula : > > > >> On Tue, 19 Sep 2017, Karsten Wiese wrote: > >> > This makes poll work for the > >> > /sys/class/drm/cardX/connectorY/dpms attributes. > >> > >> I guess the question is, what are you trying to achieve? What is the > >> problem that this solves? > >> > > > > The patch enables cpu cycle less waiting for dpms flag changes. > > > > Here it lets a screen brightness setting daemon know which monitor to > > handle. One of the attached screens gets confused if it is set just > > after being switched on, hence the daemon leaves it untouched until it > > has been active for some seconds. > > Screen brightness settings daemon? > Running on a laptop lacking an ambient light sensor employing it's webcam instead to measure ambient light and adjust monitors' brightnesses accordingly. What exactly do you mean by "if it is > set"? What interface are you using to change brightness? The external monitor's brightness is adjusted by DDC/CI via the /dev/i2c-x the monitor is attached to. I there a saner interface to use? What happens > when the display "gets confused"? > The monitor wrongly toggles sharpness if it receives the brightness adjusting DDC/CI soon after resuming from power saving state. > > My first instinct is that you're proposing a new kernel ABI to solve an > issue you shouldn't be solving in userspace to begin with. Calculating ambient light from pictures acquired via v4l2 in kernel seamed wrong to me. Or that > perhaps the userspace should be doing this in cooperation with the drm > master, not standalone. > Is there a way to call into the drm-master (xscreensaver/xserver here ) with the call only returning if a monitor's power state changed? There is DPMSInfo, but it returns immediately rendering the daemon less efficient. BR, Karsten > > BR, > Jani. > > > > > Without the patch the daemon would have to actively read the dpms flag > > every once in a while. > > > > > >> > >> We have zero sysfs_notify in all of drm AFAICT. > > > > > > Yes I noticed too and looked for some dbus signal to listen to but > didn't > > find any. > > > > BR, > > Karsten > > > >> > >> BR, > >> Jani. > >> > >> > >> > > >> > Tested with i915 suspended by XScreenServer and > >> > suspend to RAM. > >> > > >> > Signed-off-by: Karsten Wiese > >> > --- > >> > drivers/gpu/drm/drm_atomic.c| 4 > >> > drivers/gpu/drm/drm_atomic_helper.c | 6 +- > >> > 2 files changed, 9 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/gpu/drm/drm_atomic.c > b/drivers/gpu/drm/drm_atomic.c > >> > index 2fd383d..b6fa87b 100644 > >> > --- a/drivers/gpu/drm/drm_atomic.c > >> > +++ b/drivers/gpu/drm/drm_atomic.c > >> > @@ -1880,6 +1880,10 @@ int drm_atomic_connector_commit_dpms(struct > >> drm_atomic_state *state, > >> > out: > >> > if (ret != 0) > >> > connector->dpms = old_mode; > >> > + else > >> > + if (connector->dpms != old_mode) > >> > + sysfs_notify(&connector->kdev->kobj, NULL, > >> "dpms"); > >> > + > >> > return ret; > >> > } > >> > > >> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > >> b/drivers/gpu/drm/drm_atomic_helper.c > >> > index 4e53aae..6198772 100644 > >> > --- a/drivers/gpu/drm/drm_atomic_helper.c > >> > +++ b/drivers/gpu/drm/drm_atomic_helper.c > >> > @@ -921,12 +921,16 @@ drm_atomic_helper_update_ > legacy_modeset_state(struct > >> drm_device *dev, > >> > crtc = new_conn_state->crtc; > >> > if ((!crtc && old_conn_state->crtc) || > >> > (crtc && drm_atomic_crtc_needs_modeset( > crtc->state))) > >> { > >> > - int mode = DRM_MODE_DPMS_OFF; > >> > + int old_mode, mode = DRM_MODE_DPMS_OFF; > >> > > >> > if (crtc && crtc->state->active) > >> > mode = DRM_MODE_DPMS_ON; > >> > > >> > + old_mode = connector->dpms; > >> > connector->dpms = mode; > >> > + if (old_mode != mode) > >> > + sysfs_notify(&connector->kdev->kobj, > >> > + NULL, "dpms"); > >> > } > >> > } > >> > >> -- > >> Jani Nikula, Intel Open Source Technology Center > >> > > -- > Jani Nikula, Intel Open Source Technology Center > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/atomic: Make atomic iterators less surprising
On Wed, Sep 27, 2017 at 10:35:32AM +0200, Maarten Lankhorst wrote: > Commit 669c9215afea ("drm/atomic: Make async plane update checks work as > intended, v2.") assumed incorrectly that if only 1 plane is matched in > the loop, the variables will be set to that plane. In reality we reset > them to NULL every time a new plane was iterated. This behavior is > surprising, so fix this by making the for loops only assign the > variables on a match. Maybe explain a bit more how that's confusing, namely when we have not added all the planes/crtc/connector to the state, and there's a few NULL ones after the last one we iterated. Which then breaks the assumption that the pointers will hold the values from the last loop iteration, which holds true for all other for_each macros we're using (well except the iterator pointer itself, but that one really is entirely internal). With that bit of blabla added to the commit message: Reviewed-by: Daniel Vetter > > Cc: Dmitry Osipenko > Fixes: 669c9215afea ("drm/atomic: Make async plane update checks work as > intended, v2.") > Signed-off-by: Maarten Lankhorst > --- > include/drm/drm_atomic.h | 85 > > 1 file changed, 42 insertions(+), 43 deletions(-) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 6fae95f28e10..5afd6e364fb6 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -585,12 +585,12 @@ void drm_state_dump(struct drm_device *dev, struct > drm_printer *p); > */ > #define for_each_oldnew_connector_in_state(__state, connector, > old_connector_state, new_connector_state, __i) \ > for ((__i) = 0; > \ > - (__i) < (__state)->num_connector && > \ > - ((connector) = (__state)->connectors[__i].ptr, > \ > - (old_connector_state) = (__state)->connectors[__i].old_state, > \ > - (new_connector_state) = (__state)->connectors[__i].new_state, 1); > \ > - (__i)++) \ > - for_each_if (connector) > + (__i) < (__state)->num_connector; > \ > + (__i)++) > \ > + for_each_if ((__state)->connectors[__i].ptr && > \ > + ((connector) = (__state)->connectors[__i].ptr, > \ > + (old_connector_state) = > (__state)->connectors[__i].old_state, \ > + (new_connector_state) = > (__state)->connectors[__i].new_state, 1)) > > /** > * for_each_old_connector_in_state - iterate over all connectors in an > atomic update > @@ -606,11 +606,11 @@ void drm_state_dump(struct drm_device *dev, struct > drm_printer *p); > */ > #define for_each_old_connector_in_state(__state, connector, > old_connector_state, __i) \ > for ((__i) = 0; > \ > - (__i) < (__state)->num_connector && > \ > - ((connector) = (__state)->connectors[__i].ptr, > \ > - (old_connector_state) = (__state)->connectors[__i].old_state, 1); > \ > - (__i)++) \ > - for_each_if (connector) > + (__i) < (__state)->num_connector; > \ > + (__i)++) > \ > + for_each_if ((__state)->connectors[__i].ptr && > \ > + ((connector) = (__state)->connectors[__i].ptr, > \ > + (old_connector_state) = > (__state)->connectors[__i].old_state, 1)) > > /** > * for_each_new_connector_in_state - iterate over all connectors in an > atomic update > @@ -626,11 +626,11 @@ void drm_state_dump(struct drm_device *dev, struct > drm_printer *p); > */ > #define for_each_new_connector_in_state(__state, connector, > new_connector_state, __i) \ > for ((__i) = 0; > \ > - (__i) < (__state)->num_connector && > \ > - ((connector) = (__state)->connectors[__i].ptr, > \ > - (new_connector_state) = (__state)->connectors[__i].new_state, 1); > \ > - (__i)++) \ > - for_each_if (connector) > + (__i) < (__state)->num_connector; > \ > + (__i)++) > \ > + for_each_if ((__state)->connectors[__i].ptr && > \ > + ((connector) = (__state)->connectors[_
Re: [PATCH 1/2] drm/atomic: Remove unneeded null check for private objects
On Wed, Sep 27, 2017 at 10:35:31AM +0200, Maarten Lankhorst wrote: > It can be seen in drm_atomic_get_private_obj_state() that > ptr will never be NULL, so skip the check for that case. > > Signed-off-by: Maarten Lankhorst Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic.c | 3 --- > include/drm/drm_atomic.h | 9 +++-- > 2 files changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 8bd5a1c4c7e8..29525bffdad9 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -182,9 +182,6 @@ void drm_atomic_state_default_clear(struct > drm_atomic_state *state) > for (i = 0; i < state->num_private_objs; i++) { > struct drm_private_obj *obj = state->private_objs[i].ptr; > > - if (!obj) > - continue; > - > obj->funcs->atomic_destroy_state(obj, >state->private_objs[i].state); > state->private_objs[i].ptr = NULL; > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 5834580d75bc..6fae95f28e10 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -768,8 +768,7 @@ void drm_state_dump(struct drm_device *dev, struct > drm_printer *p); >((obj) = (__state)->private_objs[__i].ptr, \ > (old_obj_state) = (__state)->private_objs[__i].old_state, > \ > (new_obj_state) = (__state)->private_objs[__i].new_state, > 1); \ > - (__i)++) \ > - for_each_if (obj) > + (__i)++) > > /** > * for_each_old_private_obj_in_state - iterate over all private objects in > an atomic update > @@ -787,8 +786,7 @@ void drm_state_dump(struct drm_device *dev, struct > drm_printer *p); >(__i) < (__state)->num_private_objs && \ >((obj) = (__state)->private_objs[__i].ptr, \ > (old_obj_state) = (__state)->private_objs[__i].old_state, > 1); \ > - (__i)++) \ > - for_each_if (obj) > + (__i)++) > > /** > * for_each_new_private_obj_in_state - iterate over all private objects in > an atomic update > @@ -806,8 +804,7 @@ void drm_state_dump(struct drm_device *dev, struct > drm_printer *p); >(__i) < (__state)->num_private_objs && \ >((obj) = (__state)->private_objs[__i].ptr, \ > (new_obj_state) = (__state)->private_objs[__i].new_state, > 1); \ > - (__i)++) \ > - for_each_if (obj) > + (__i)++) > > /** > * drm_atomic_crtc_needs_modeset - compute combined modeset need > -- > 2.14.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel