Re: [PATCH 2/6] drm: rcar-du: lvds: Don't fail probe if output is not connected on D3/E3
Hello! On 17.01.2019 4:49, Laurent Pinchart wrote: On the D3 and E3 SoCs the LVDS encoder has an extended internal PLL and supplies a clock to the DU. That clock is used not only for the LVDS outputs but also for the DPAD output. The LVDS encoder thus needs to be available to the DU even when its output is disabled. Don't fail probe in that cose on D3 and E3. Case? Signed-off-by: Laurent Pinchart [...] MBR, Sergei ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows
https://bugs.freedesktop.org/show_bug.cgi?id=106175 --- Comment #104 from Michel Dänzer --- (In reply to Hans D from comment #103) > With the latest xf86 DDX driver 3d works better in amdgpu.dc=0, but still > lags, even though a built-in counter is showing high fps. Does amdgpu.dc=1 > improve 3d performance that much? It shouldn't affect 3D performance directly at at all. I'm not sure what exactly you mean by "lags", so it's hard to guess what's going on. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/crtc-helper: Add store the property value
There is a problem in crtc_helper that property value is not updated
when dpms is turned on or off. So modify the property value when dpms
is on.
Signed-off-by: Hoegeun Kwon
---
drivers/gpu/drm/drm_crtc_helper.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index a3c81850e755..57d359f0725c 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -735,6 +735,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS
on\n", set->connectors[i]->base.id,
set->connectors[i]->name);
set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
+
+
drm_object_property_set_value(&set->connectors[i]->base,
+
set->connectors[i]->dev->mode_config.dpms_property,
+ DRM_MODE_DPMS_ON);
}
}
__drm_helper_disable_unused_functions(dev);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: HDMI/DVI spurious failure
On Wed, Jan 16, 2019 at 08:24:42PM +0100, Maxime Ripard wrote: > Hi Priit, > > On Wed, Jan 16, 2019 at 07:58:54AM +, Priit Laes wrote: > > > On Mon, Jan 14, 2019 at 01:29:34PM +, Priit Laes wrote: > > > > I have a somewhat curious case with one HDMI/DVI screen that fails > > > > to initialize properly every 6-7 boots. The display itself is also > > > > somewhat flawed (missing HPD pin and the VSYNC/HSYNC pulse width > > > > is set to 0 in EDID), but I suspect there could be some issues > > > > regarding timing in A20 HDMI driver in Linux. > > > > > > ... > > > > > It doesn't look related to the clock rate itself, since it doesn't > > > change between the two cases. However, in one case the DDC clock is > > > enabled and in the other it's disabled. > > > > > > Was it taken at the same time? Maybe you can try with that patch? > > > http://code.bulix.org/z7jmkm-555344?raw > > > > Thanks, after doing ~50+ boots I haven't seen a single failure. > > > > Previously I had following failure cases which are now both fixed: > > > > a) Linux without u-boot HDMI, where one in every 6-7 boots failed. > > b) u--boot with hdmi enabled switching to simplefb and then switching > > to kms, where previously all boots ended up with garbled screen. > > So it's not really a fix, but it really looks like the clock is not > enabled when it should. > > Can you describe your test scenario a bit more? What are you doing > exactly, just booting? When do you start using the display? When did > you capture the debugfs output that you pasted? Display is already connected via HDMI to the board. I don't really remove it, I just boot the device and let it start Xorg. Meanwhile I just ssh into the device and capture debugfs output. See my 3 testing scenarios below. Kernel also includes one extra patch to fall back to DDC, in case HPD fails. Mostly the same I already submitted last November [1]. For u-boot I have also some extra patches, to detect HPD-less HDMI displays [2] + relax some EDID timing checks [3] so u-boot can actually initialize my screen. So first configuration with 100% failures: 1) u-boot initializes HDMI ( A20-OLinuXino-Lime2-eMMC_defconfig ) 2) Linux switches to simplefb ... somewhere around here blinking cursor is replaced with garbage on screen 3) Linux switches to kms 4) Xorg starts Second scenario with failure every 6-7 boots: 1) Disabled HDMI in u-boot for my board 2) Linux sets up kms (sometimes fails here) 3) Xorg starts 4) ssh to machine and take the clock dump Third scenario with no failures (but not suitable in long term..) 1) Disabled sun4i HDMI driver (CONFIG_DRM_SUN4I_HDMI=n) in Linux 2) u-boot sets up HDMI 3) Linux continues with simplefb 4) Xorg (with fbdev) starts [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-November/613168.html [2] https://lists.denx.de/pipermail/u-boot/2018-December/352541.html [3] https://lists.denx.de/pipermail/u-boot/2018-December/352540.html > > Thanks! > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/11] Add support for DRM bridge and additional pixel formats
This patchset improves the use of eLCDIF block on iMX 8 SoCs (like 8MQ, 8MM and 8QXP): 1. Add support for drm_bridge On 8MQ and 8MM, the LCDIF block is not directly connected to a parallel display connector, where an LCD panel can be attached, but instead it is connected to DSI controller. Since this DSI stands between the display controller (eLCDIF) and the physical connector, the DSI can be implemented as a DRM bridge. So, in order to be able to connect the mxsfb driver to the DSI driver, the support for a drm_bridge was needed in mxsfb DRM driver (the actual driver for the eLCDIF block). 2. Add support for additional pixel formats Some of the pixel formats needed by Android were not implemented in this driver, but they were actually supported. So, add support for them. 3. Few minor features and bug-fixing The addition of max-res DT property was actually needed in order to limit the bandwidth usage of the eLCDIF block. This is need on systems where multiple display controllers are presend and the memory bandwidth is not enough to handle all of them at maximum capacity (like it is the case on 8MQ, where there are two display controllers: DCSS and eLCDIF). The rest of the patches are bug-fixes. Changes since v1: * Replaced for_each_crtc_in_state (removed since deprecated) with for_each_new_crtc_in_state * Removed unused MXSFB_FLAG_* flags * Split the patch for LCD reset and moved the pm_runtime_enable fix into a separate patch Mirela Rabulea (2): drm/mxsfb: Add support for new pixel formats in eLCDIF drm/mxsfb: Signal mode changed when bpp changed Robert Chiras (9): drm/mxsfb: Update mxsfb to support a bridge dt-bindings: display: Add max-res property for mxsfb drm/mxsfb: Add max-res property for MXSFB drm/mxsfb: Update mxsfb with additional pixel formats drm/mxsfb: Fix the vblank events drm/mxsfb: Update mxsfb to support LCD reset drm/mxsfb: Move pm_runtime_enable at the end of probe drm/mxsfb: Improve the axi clock usage drm/mxsfb: Clear OUTSTANDING_REQS bits .../devicetree/bindings/display/mxsfb.txt | 6 + drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 215 ++--- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 183 ++ drivers/gpu/drm/mxsfb/mxsfb_drv.h | 10 +- drivers/gpu/drm/mxsfb/mxsfb_out.c | 26 +-- drivers/gpu/drm/mxsfb/mxsfb_regs.h | 112 +++ 6 files changed, 429 insertions(+), 123 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 6/9] drm/bridge: cdns: Separate DSI and D-PHY configuration
Hi Maxime,
On 09/01/19 3:03 PM, Maxime Ripard wrote:
> The current configuration of the DSI bridge and its associated D-PHY is
> intertwined. In order to ease the future conversion to the phy framework
> for the D-PHY part, let's split the configuration in two.
>
> Signed-off-by: Maxime Ripard
Can we get Ack from DRM MAINTAINERS?
Thanks
Kishon
> ---
> drivers/gpu/drm/bridge/cdns-dsi.c | 96 ++--
> 1 file changed, 68 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
> b/drivers/gpu/drm/bridge/cdns-dsi.c
> index ce9496d13986..3ac6dd524b6d 100644
> --- a/drivers/gpu/drm/bridge/cdns-dsi.c
> +++ b/drivers/gpu/drm/bridge/cdns-dsi.c
> @@ -545,6 +545,11 @@ bridge_to_cdns_dsi_input(struct drm_bridge *bridge)
> return container_of(bridge, struct cdns_dsi_input, bridge);
> }
>
> +static unsigned int mode_to_dpi_hfp(const struct drm_display_mode *mode)
> +{
> + return mode->crtc_hsync_start - mode->crtc_hdisplay;
> +}
> +
> static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
>struct cdns_dphy_cfg *cfg,
>unsigned int dpi_htotal,
> @@ -731,14 +736,12 @@ static unsigned int dpi_to_dsi_timing(unsigned int
> dpi_timing,
> static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
>const struct drm_display_mode *mode,
>struct cdns_dsi_cfg *dsi_cfg,
> - struct cdns_dphy_cfg *dphy_cfg,
>bool mode_valid_check)
> {
> - unsigned long dsi_htotal = 0, dsi_hss_hsa_hse_hbp = 0;
> struct cdns_dsi_output *output = &dsi->output;
> - unsigned int dsi_hfp_ext = 0, dpi_hfp, tmp;
> + unsigned int tmp;
> bool sync_pulse = false;
> - int bpp, nlanes, ret;
> + int bpp, nlanes;
>
> memset(dsi_cfg, 0, sizeof(*dsi_cfg));
>
> @@ -757,8 +760,6 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
> mode->crtc_hsync_end : mode->crtc_hsync_start);
>
> dsi_cfg->hbp = dpi_to_dsi_timing(tmp, bpp, DSI_HBP_FRAME_OVERHEAD);
> - dsi_htotal += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
> - dsi_hss_hsa_hse_hbp += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
>
> if (sync_pulse) {
> if (mode_valid_check)
> @@ -768,49 +769,90 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
>
> dsi_cfg->hsa = dpi_to_dsi_timing(tmp, bpp,
>DSI_HSA_FRAME_OVERHEAD);
> - dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
> - dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
> }
>
> dsi_cfg->hact = dpi_to_dsi_timing(mode_valid_check ?
> mode->hdisplay : mode->crtc_hdisplay,
> bpp, 0);
> - dsi_htotal += dsi_cfg->hact;
> + dsi_cfg->hfp = dpi_to_dsi_timing(mode_to_dpi_hfp(mode), bpp,
> + DSI_HFP_FRAME_OVERHEAD);
>
> - if (mode_valid_check)
> - dpi_hfp = mode->hsync_start - mode->hdisplay;
> - else
> - dpi_hfp = mode->crtc_hsync_start - mode->crtc_hdisplay;
> + return 0;
> +}
> +
> +static int cdns_dphy_validate(struct cdns_dsi *dsi,
> + struct cdns_dsi_cfg *dsi_cfg,
> + struct cdns_dphy_cfg *dphy_cfg,
> + const struct drm_display_mode *mode,
> + bool mode_valid_check)
> +{
> + struct cdns_dsi_output *output = &dsi->output;
> + unsigned long dsi_htotal;
> + unsigned int dsi_hfp_ext = 0;
> +
> + int ret;
>
> - dsi_cfg->hfp = dpi_to_dsi_timing(dpi_hfp, bpp, DSI_HFP_FRAME_OVERHEAD);
> + dsi_htotal = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
> + if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
> + dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
> +
> + dsi_htotal += dsi_cfg->hact;
> dsi_htotal += dsi_cfg->hfp + DSI_HFP_FRAME_OVERHEAD;
>
> if (mode_valid_check)
> ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
> - mode->htotal, bpp,
> + mode->htotal,
> mode->clock * 1000,
> - dsi_htotal, nlanes,
> +
> mipi_dsi_pixel_format_to_bpp(output->dev->format),
> + dsi_htotal,
> + output->dev->lanes,
> &dsi_hfp_ext);
> else
> ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
> - mode->crtc_htotal, bpp,
> + mode->crtc_ht
[PATCH 05/11] drm/mxsfb: Fix the vblank events
Currently, the vblank support is not correctly implemented in MXSFB_DRM
driver. The call to drm_vblank_init is made with mode_config.num_crtc
which at that time is 0. Because of this, vblank is not activated, so
there won't be any vblank event submitted.
Signed-off-by: Robert Chiras
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index bc1b750..d292192 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -43,6 +43,9 @@
#include "mxsfb_drv.h"
#include "mxsfb_regs.h"
+/* The eLCDIF max possible CRTCs */
+#define MAX_CRTCS 1
+
enum mxsfb_devtype {
MXSFB_V3,
MXSFB_V4,
@@ -138,6 +141,8 @@ static void mxsfb_pipe_enable(struct
drm_simple_display_pipe *pipe,
mxsfb->connector = &mxsfb->panel_connector;
}
+ drm_crtc_vblank_on(&mxsfb->pipe.crtc);
+
pm_runtime_get_sync(drm->dev);
drm_panel_prepare(mxsfb->panel);
mxsfb_crtc_enable(mxsfb);
@@ -247,7 +252,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
flags)
pm_runtime_enable(drm->dev);
- ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
+ ret = drm_vblank_init(drm, MAX_CRTCS);
if (ret < 0) {
dev_err(drm->dev, "Failed to initialise vblank\n");
goto err_vblank;
@@ -270,6 +275,8 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
flags)
goto err_vblank;
}
+ drm_crtc_vblank_off(&mxsfb->pipe.crtc);
+
/*
* Attach panel only if there is one.
* If there is no panel attach, it must be a bridge. In this case, we
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v3 02/28] clk: sunxi-ng: Adjust MP clock parent rate when allowed
Dne sreda, 16. januar 2019 ob 13:09:58 CET je Priit Laes napisal(a): > On Thu, Jan 10, 2019 at 06:10:59PM +0100, Jernej Škrabec wrote: > > Dne četrtek, 10. januar 2019 ob 10:15:48 CET je Priit Laes napisal(a): > > > On Sun, Nov 04, 2018 at 07:26:39PM +0100, Jernej Skrabec wrote: > > > > Currently MP clocks don't consider adjusting parent rate even if they > > > > are allowed to do so. Such behaviour considerably lowers amount of > > > > possible rates, which is very inconvenient when such clock is used for > > > > pixel clock, for example. > > > > > > > > In order to improve the situation, adjusting parent rate is considered > > > > when allowed. > > > > > > > > This code is inspired by clk_divider_bestdiv() function, which does > > > > basically the same thing for different clock type. > > > > > > This patch seems to break the eMMC support on Olinuxino-Lime2-eMMC > > > boards: > > > > > > EXT4-fs (mmcblk1p4): INFO: recovery required on readonly filesystem > > > EXT4-fs (mmcblk1p4): write access will be enabled during recovery > > > sunxi-mmc 1c11000.mmc: data error, sending stop command > > > sunxi-mmc 1c11000.mmc: send stop command failed > > > > I'm not familiar with A20. What is interesting is that emmc clocks don't > > have CLK_SET_RATE_PARENT flag set, so you shouldn't see any difference. > > > > Can you post content of clk_summary with and without this patch? > > In both cases I booted from FEL with rootfs on sdcard and tried to mount > partition from eMMC to /mnt. With your patch, last step it fails. > > pre-patch working: > pll-ddr-other[768MHz] -> mmc2[512MHz]. (For some reason ahb-mmc2 is off?) > > post-patch not working: > pll-periph[600MHz] -> mmc2[500Mhz], (ahb-mmc2 is enabled) > > Also, attached the logs. Thanks. Just one more request. Can you enable debug messages in mmc driver? I'm interested in output of this line: dev_dbg(mmc_dev(mmc), "setting clk to %d, rounded %ld\n", clock, rate); Just wondering what it should be. Best regards, Jernej > > > Best regards, > > Jernej > > > > > $ git bisect log > > > git bisect start > > > # good: [3df407b2a5346db1c48809706ece7a8616c79e0b] mmc: > > > dw_mmc-bluefield: > > > simplify the probe() function git bisect good > > > 3df407b2a5346db1c48809706ece7a8616c79e0b > > > # bad: [00d59fde8532b2d42e80909d2e58678755e04da9] Merge tag 'mmc-v4.21' > > > of > > > git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc git bisect bad > > > 00d59fde8532b2d42e80909d2e58678755e04da9 > > > # good: [01e421feec0817bb3141eaae4c517410d193d440] Merge branch 'fixes' > > > into next git bisect good 01e421feec0817bb3141eaae4c517410d193d440 > > > # bad: [1eefdec18eded41833401cfd64749643ff72e7da] Merge branch > > > 'locking-core-for-linus' of > > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect bad > > > 1eefdec18eded41833401cfd64749643ff72e7da > > > # good: [eaa76499711535fd64d747cc4ef0d78ab0fd41c6] Merge tag > > > 'mtd/for-4.21' > > > of git://git.infradead.org/linux-mtd git bisect good > > > eaa76499711535fd64d747cc4ef0d78ab0fd41c6 > > > # good: [4e4390ad067a61ce4e7607bd0df31f19a4caa36a] Merge tag > > > 'leds-for-4.21-rc1' of > > > git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds > > > git > > > bisect good 4e4390ad067a61ce4e7607bd0df31f19a4caa36a > > > # bad: [c2f1f3e0e17d94ab0c66d83e669492cb9e9a3698] Merge > > > git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc-next git > > > bisect > > > bad c2f1f3e0e17d94ab0c66d83e669492cb9e9a3698 > > > # bad: [e4b99d415c3908581d4703203e1e805f043a3e71] Merge branch > > > 'irq-core-for-linus' of > > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect bad > > > e4b99d415c3908581d4703203e1e805f043a3e71 > > > # bad: [ffe05540d18013db62c43627836a3638e9a2c7aa] Merge branches > > > 'clk-renesas', 'clk-allwinner', 'clk-tegra', 'clk-meson' and > > > 'clk-rockchip' > > > into clk-next git bisect bad ffe05540d18013db62c43627836a3638e9a2c7aa > > > # good: [1a501c8defe950571316d5ddd917bf44f5ed7bd4] Merge branches > > > 'clk-managed-registration', 'clk-spdx', 'clk-remove-basic' and > > > 'clk-ops-const' into clk-next git bisect good > > > 1a501c8defe950571316d5ddd917bf44f5ed7bd4 > > > # good: [e74581b79ddd9b49b8c61e2791fc4dffc0245afb] Merge tag > > > 'meson-clk-4.21-2' of https://github.com/BayLibre/clk-meson into > > > clk-meson > > > git bisect good e74581b79ddd9b49b8c61e2791fc4dffc0245afb > > > # good: [60baf75e3f5b76043c25328ac0c5320aaef5ea41] Merge tag > > > 'clk-renesas-for-v4.21-tag2' of > > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers into > > > clk-renesas git bisect good 60baf75e3f5b76043c25328ac0c5320aaef5ea41 > > > # bad: [a41f85b6017ee20952a60e4330bcae2527fe2c2a] Merge tag > > > 'sunxi-clk-for-4.21' of > > > https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux into > > > clk-allwinner git bisect bad a41f85b6017ee20952a60e4330bcae2527fe2c2a > > > # bad: [ee678706e46d0d185c27cc214ad97828e0643159] clk: sunxi-ng:
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On Wed, 16 Jan 2019, Andrew F. Davis wrote:
> On 1/15/19 1:05 PM, Laura Abbott wrote:
> > On 1/15/19 10:38 AM, Andrew F. Davis wrote:
> >> On 1/15/19 11:45 AM, Liam Mark wrote:
> >>> On Tue, 15 Jan 2019, Andrew F. Davis wrote:
> >>>
> On 1/14/19 11:13 AM, Liam Mark wrote:
> > On Fri, 11 Jan 2019, Andrew F. Davis wrote:
> >
> >> Buffers may not be mapped from the CPU so skip cache maintenance
> >> here.
> >> Accesses from the CPU to a cached heap should be bracketed with
> >> {begin,end}_cpu_access calls so maintenance should not be needed
> >> anyway.
> >>
> >> Signed-off-by: Andrew F. Davis
> >> ---
> >> drivers/staging/android/ion/ion.c | 7 ---
> >> 1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/staging/android/ion/ion.c
> >> b/drivers/staging/android/ion/ion.c
> >> index 14e48f6eb734..09cb5a8e2b09 100644
> >> --- a/drivers/staging/android/ion/ion.c
> >> +++ b/drivers/staging/android/ion/ion.c
> >> @@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
> >> dma_buf_attachment *attachment,
> >> table = a->table;
> >> - if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
> >> - direction))
> >> + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
> >> + direction, DMA_ATTR_SKIP_CPU_SYNC))
> >
> > Unfortunately I don't think you can do this for a couple reasons.
> > You can't rely on {begin,end}_cpu_access calls to do cache
> > maintenance.
> > If the calls to {begin,end}_cpu_access were made before the call to
> > dma_buf_attach then there won't have been a device attached so the
> > calls
> > to {begin,end}_cpu_access won't have done any cache maintenance.
> >
>
> That should be okay though, if you have no attachments (or all
> attachments are IO-coherent) then there is no need for cache
> maintenance. Unless you mean a sequence where a non-io-coherent device
> is attached later after data has already been written. Does that
> sequence need supporting?
> >>>
> >>> Yes, but also I think there are cases where CPU access can happen before
> >>> in Android, but I will focus on later for now.
> >>>
> DMA-BUF doesn't have to allocate the backing
> memory until map_dma_buf() time, and that should only happen after all
> the devices have attached so it can know where to put the buffer. So we
> shouldn't expect any CPU access to buffers before all the devices are
> attached and mapped, right?
>
> >>>
> >>> Here is an example where CPU access can happen later in Android.
> >>>
> >>> Camera device records video -> software post processing -> video device
> >>> (who does compression of raw data) and writes to a file
> >>>
> >>> In this example assume the buffer is cached and the devices are not
> >>> IO-coherent (quite common).
> >>>
> >>
> >> This is the start of the problem, having cached mappings of memory that
> >> is also being accessed non-coherently is going to cause issues one way
> >> or another. On top of the speculative cache fills that have to be
> >> constantly fought back against with CMOs like below; some coherent
> >> interconnects behave badly when you mix coherent and non-coherent access
> >> (snoop filters get messed up).
> >>
> >> The solution is to either always have the addresses marked non-coherent
> >> (like device memory, no-map carveouts), or if you really want to use
> >> regular system memory allocated at runtime, then all cached mappings of
> >> it need to be dropped, even the kernel logical address (area as painful
> >> as that would be).
> >>
> >
> > I agree it's broken, hence my desire to remove it :)
> >
> > The other problem is that uncached buffers are being used for
> > performance reason so anything that would involve getting
> > rid of the logical address would probably negate any performance
> > benefit.
> >
>
> I wouldn't go as far as to remove them just yet.. Liam seems pretty
> adamant that they have valid uses. I'm just not sure performance is one
> of them, maybe in the case of software locks between devices or
> something where there needs to be a lot of back and forth interleaved
> access on small amounts of data?
>
I wasn't aware that ARM considered this not supported, I thought it was
supported but they advised against it because of the potential performance
impact.
This is after all supported in the DMA APIs and up until now devices have
been successfully commercializing with this configurations, and I think
they will continue to commercialize with these configurations for quite a
while.
It would be really unfortunate if support was removed as I think that
would drive clients away from using upstream ION.
> >>> ION buffer is allocated.
> >>>
> >>> //Camera device records video
> >>> dma_buf_attach
> >>> dma_map_attachment (buffer needs to b
[PATCH v2] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
Use the SIMPLE_DEV_PM_OPS() macro to declare the driver's pm_ops.
As a side effect, removing #ifdef CONFIG_PM_SLEEP will improve
compilation coverage.
Signed-off-by: Alexander Shiyan
---
drivers/video/backlight/pwm_bl.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index feb9076..eaefdb0 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -673,8 +673,7 @@ static void pwm_backlight_shutdown(struct platform_device
*pdev)
pwm_backlight_power_off(pb);
}
-#ifdef CONFIG_PM_SLEEP
-static int pwm_backlight_suspend(struct device *dev)
+static int __maybe_unused pwm_backlight_suspend(struct device *dev)
{
struct backlight_device *bl = dev_get_drvdata(dev);
struct pwm_bl_data *pb = bl_get_data(bl);
@@ -690,7 +689,7 @@ static int pwm_backlight_suspend(struct device *dev)
return 0;
}
-static int pwm_backlight_resume(struct device *dev)
+static int __maybe_unused pwm_backlight_resume(struct device *dev)
{
struct backlight_device *bl = dev_get_drvdata(dev);
@@ -698,16 +697,9 @@ static int pwm_backlight_resume(struct device *dev)
return 0;
}
-#endif
-static const struct dev_pm_ops pwm_backlight_pm_ops = {
-#ifdef CONFIG_PM_SLEEP
- .suspend = pwm_backlight_suspend,
- .resume = pwm_backlight_resume,
- .poweroff = pwm_backlight_suspend,
- .restore = pwm_backlight_resume,
-#endif
-};
+static SIMPLE_DEV_PM_OPS(pwm_backlight_pm_ops,
+pwm_backlight_suspend, pwm_backlight_resume);
static struct platform_driver pwm_backlight_driver = {
.driver = {
--
2.10.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v3 02/28] clk: sunxi-ng: Adjust MP clock parent rate when allowed
On Thu, Jan 10, 2019 at 06:10:59PM +0100, Jernej Škrabec wrote: > Dne četrtek, 10. januar 2019 ob 10:15:48 CET je Priit Laes napisal(a): > > On Sun, Nov 04, 2018 at 07:26:39PM +0100, Jernej Skrabec wrote: > > > Currently MP clocks don't consider adjusting parent rate even if they > > > are allowed to do so. Such behaviour considerably lowers amount of > > > possible rates, which is very inconvenient when such clock is used for > > > pixel clock, for example. > > > > > > In order to improve the situation, adjusting parent rate is considered > > > when allowed. > > > > > > This code is inspired by clk_divider_bestdiv() function, which does > > > basically the same thing for different clock type. > > > > This patch seems to break the eMMC support on Olinuxino-Lime2-eMMC boards: > > > > EXT4-fs (mmcblk1p4): INFO: recovery required on readonly filesystem > > EXT4-fs (mmcblk1p4): write access will be enabled during recovery > > sunxi-mmc 1c11000.mmc: data error, sending stop command > > sunxi-mmc 1c11000.mmc: send stop command failed > > > > I'm not familiar with A20. What is interesting is that emmc clocks don't have > CLK_SET_RATE_PARENT flag set, so you shouldn't see any difference. > > Can you post content of clk_summary with and without this patch? In both cases I booted from FEL with rootfs on sdcard and tried to mount partition from eMMC to /mnt. With your patch, last step it fails. pre-patch working: pll-ddr-other[768MHz] -> mmc2[512MHz]. (For some reason ahb-mmc2 is off?) post-patch not working: pll-periph[600MHz] -> mmc2[500Mhz], (ahb-mmc2 is enabled) Also, attached the logs. > > Best regards, > Jernej > > > > > $ git bisect log > > git bisect start > > # good: [3df407b2a5346db1c48809706ece7a8616c79e0b] mmc: dw_mmc-bluefield: > > simplify the probe() function git bisect good > > 3df407b2a5346db1c48809706ece7a8616c79e0b > > # bad: [00d59fde8532b2d42e80909d2e58678755e04da9] Merge tag 'mmc-v4.21' of > > git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc git bisect bad > > 00d59fde8532b2d42e80909d2e58678755e04da9 > > # good: [01e421feec0817bb3141eaae4c517410d193d440] Merge branch 'fixes' into > > next git bisect good 01e421feec0817bb3141eaae4c517410d193d440 > > # bad: [1eefdec18eded41833401cfd64749643ff72e7da] Merge branch > > 'locking-core-for-linus' of > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect bad > > 1eefdec18eded41833401cfd64749643ff72e7da > > # good: [eaa76499711535fd64d747cc4ef0d78ab0fd41c6] Merge tag 'mtd/for-4.21' > > of git://git.infradead.org/linux-mtd git bisect good > > eaa76499711535fd64d747cc4ef0d78ab0fd41c6 > > # good: [4e4390ad067a61ce4e7607bd0df31f19a4caa36a] Merge tag > > 'leds-for-4.21-rc1' of > > git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds git > > bisect good 4e4390ad067a61ce4e7607bd0df31f19a4caa36a > > # bad: [c2f1f3e0e17d94ab0c66d83e669492cb9e9a3698] Merge > > git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc-next git bisect > > bad c2f1f3e0e17d94ab0c66d83e669492cb9e9a3698 > > # bad: [e4b99d415c3908581d4703203e1e805f043a3e71] Merge branch > > 'irq-core-for-linus' of > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect bad > > e4b99d415c3908581d4703203e1e805f043a3e71 > > # bad: [ffe05540d18013db62c43627836a3638e9a2c7aa] Merge branches > > 'clk-renesas', 'clk-allwinner', 'clk-tegra', 'clk-meson' and 'clk-rockchip' > > into clk-next git bisect bad ffe05540d18013db62c43627836a3638e9a2c7aa > > # good: [1a501c8defe950571316d5ddd917bf44f5ed7bd4] Merge branches > > 'clk-managed-registration', 'clk-spdx', 'clk-remove-basic' and > > 'clk-ops-const' into clk-next git bisect good > > 1a501c8defe950571316d5ddd917bf44f5ed7bd4 > > # good: [e74581b79ddd9b49b8c61e2791fc4dffc0245afb] Merge tag > > 'meson-clk-4.21-2' of https://github.com/BayLibre/clk-meson into clk-meson > > git bisect good e74581b79ddd9b49b8c61e2791fc4dffc0245afb > > # good: [60baf75e3f5b76043c25328ac0c5320aaef5ea41] Merge tag > > 'clk-renesas-for-v4.21-tag2' of > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers into > > clk-renesas git bisect good 60baf75e3f5b76043c25328ac0c5320aaef5ea41 > > # bad: [a41f85b6017ee20952a60e4330bcae2527fe2c2a] Merge tag > > 'sunxi-clk-for-4.21' of > > https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux into > > clk-allwinner git bisect bad a41f85b6017ee20952a60e4330bcae2527fe2c2a > > # bad: [ee678706e46d0d185c27cc214ad97828e0643159] clk: sunxi-ng: a64: Fix > > gate bit of DSI DPHY git bisect bad > > ee678706e46d0d185c27cc214ad97828e0643159 > > # bad: [65b6657672388b72822e0367f06d41c1e3ffb5bb] clk: sunxi-ng: Use u64 for > > calculation of NM rate git bisect bad > > 65b6657672388b72822e0367f06d41c1e3ffb5bb > > # good: [db7548934603d9eda12649dff97ea5c29884405d] clk: sunxi-ng: sun50i: > > h6: Fix MMC clock mux width git bisect good > > db7548934603d9eda12649dff97ea5c29884405d > > # bad: [3f790433c3cb27ecaf2ca0e07ac25964e4fd9f15] clk: sunxi-ng: Adjust MP > > clock parent rate when al
[PATCH 03/11] drm/mxsfb: Add max-res property for MXSFB
Because of stability issues, we may want to limit the maximum resolution
supported by the MXSFB (eLCDIF) driver.
This patch add support for a new property which we can use to impose such
limitation.
Signed-off-by: Robert Chiras
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 9a73564..c0b6198 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -201,6 +201,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
flags)
struct platform_device *pdev = to_platform_device(drm->dev);
struct mxsfb_drm_private *mxsfb;
struct resource *res;
+ u32 max_res[2] = {0, 0};
int ret;
mxsfb = devm_kzalloc(&pdev->dev, sizeof(*mxsfb), GFP_KERNEL);
@@ -279,10 +280,17 @@ static int mxsfb_load(struct drm_device *drm, unsigned
long flags)
}
}
+ of_property_read_u32_array(drm->dev->of_node, "max-res",
+ &max_res[0], 2);
+ if (!max_res[0])
+ max_res[0] = MXSFB_MAX_XRES;
+ if (!max_res[1])
+ max_res[1] = MXSFB_MAX_YRES;
+
drm->mode_config.min_width = MXSFB_MIN_XRES;
drm->mode_config.min_height = MXSFB_MIN_YRES;
- drm->mode_config.max_width = MXSFB_MAX_XRES;
- drm->mode_config.max_height = MXSFB_MAX_YRES;
+ drm->mode_config.max_width = max_res[0];
+ drm->mode_config.max_height = max_res[1];
drm->mode_config.funcs = &mxsfb_mode_config_funcs;
drm->mode_config.helper_private = &mxsfb_mode_config_helpers;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/11] dt-bindings: display: Add max-res property for mxsfb
Add new optional property 'max-res', to limit the maximum supported
resolution by the MXSFB_DRM driver.
Signed-off-by: Robert Chiras
---
Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt
b/Documentation/devicetree/bindings/display/mxsfb.txt
index 472e1ea..55e22ed 100644
--- a/Documentation/devicetree/bindings/display/mxsfb.txt
+++ b/Documentation/devicetree/bindings/display/mxsfb.txt
@@ -17,6 +17,12 @@ Required properties:
Required sub-nodes:
- port: The connection to an encoder chip.
+Optional properties:
+- max-res: an array with a maximum of two integers, representing the
+ maximum supported resolution, in the form of
+ , ; if one of the item is <0>, the default
+ driver-defined maximum resolution for that axis is used
+
Example:
lcdif1: display-controller@222 {
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On Wed, 16 Jan 2019, Andrew F. Davis wrote:
> On 1/16/19 9:19 AM, Brian Starkey wrote:
> > Hi :-)
> >
> > On Tue, Jan 15, 2019 at 12:40:16PM -0600, Andrew F. Davis wrote:
> >> On 1/15/19 12:38 PM, Andrew F. Davis wrote:
> >>> On 1/15/19 11:45 AM, Liam Mark wrote:
> On Tue, 15 Jan 2019, Andrew F. Davis wrote:
>
> > On 1/14/19 11:13 AM, Liam Mark wrote:
> >> On Fri, 11 Jan 2019, Andrew F. Davis wrote:
> >>
> >>> Buffers may not be mapped from the CPU so skip cache maintenance here.
> >>> Accesses from the CPU to a cached heap should be bracketed with
> >>> {begin,end}_cpu_access calls so maintenance should not be needed
> >>> anyway.
> >>>
> >>> Signed-off-by: Andrew F. Davis
> >>> ---
> >>> drivers/staging/android/ion/ion.c | 7 ---
> >>> 1 file changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/staging/android/ion/ion.c
> >>> b/drivers/staging/android/ion/ion.c
> >>> index 14e48f6eb734..09cb5a8e2b09 100644
> >>> --- a/drivers/staging/android/ion/ion.c
> >>> +++ b/drivers/staging/android/ion/ion.c
> >>> @@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
> >>> dma_buf_attachment *attachment,
> >>>
> >>> table = a->table;
> >>>
> >>> - if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
> >>> - direction))
> >>> + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
> >>> + direction, DMA_ATTR_SKIP_CPU_SYNC))
> >>
> >> Unfortunately I don't think you can do this for a couple reasons.
> >> You can't rely on {begin,end}_cpu_access calls to do cache maintenance.
> >> If the calls to {begin,end}_cpu_access were made before the call to
> >> dma_buf_attach then there won't have been a device attached so the
> >> calls
> >> to {begin,end}_cpu_access won't have done any cache maintenance.
> >>
> >
> > That should be okay though, if you have no attachments (or all
> > attachments are IO-coherent) then there is no need for cache
> > maintenance. Unless you mean a sequence where a non-io-coherent device
> > is attached later after data has already been written. Does that
> > sequence need supporting?
>
> Yes, but also I think there are cases where CPU access can happen before
> in Android, but I will focus on later for now.
>
> > DMA-BUF doesn't have to allocate the backing
> > memory until map_dma_buf() time, and that should only happen after all
> > the devices have attached so it can know where to put the buffer. So we
> > shouldn't expect any CPU access to buffers before all the devices are
> > attached and mapped, right?
> >
>
> Here is an example where CPU access can happen later in Android.
>
> Camera device records video -> software post processing -> video device
> (who does compression of raw data) and writes to a file
>
> In this example assume the buffer is cached and the devices are not
> IO-coherent (quite common).
>
> >>>
> >>> This is the start of the problem, having cached mappings of memory that
> >>> is also being accessed non-coherently is going to cause issues one way
> >>> or another. On top of the speculative cache fills that have to be
> >>> constantly fought back against with CMOs like below; some coherent
> >>> interconnects behave badly when you mix coherent and non-coherent access
> >>> (snoop filters get messed up).
> >>>
> >>> The solution is to either always have the addresses marked non-coherent
> >>> (like device memory, no-map carveouts), or if you really want to use
> >>> regular system memory allocated at runtime, then all cached mappings of
> >>> it need to be dropped, even the kernel logical address (area as painful
> >>> as that would be).
> >
> > Ouch :-( I wasn't aware about these potential interconnect issues. How
> > "real" is that? It seems that we aren't really hitting that today on
> > real devices.
> >
>
> Sadly there is at least one real device like this now (TI AM654). We
> spent some time working with the ARM interconnect spec designers to see
> if this was allowed behavior, final conclusion was mixing coherent and
> non-coherent accesses is never a good idea.. So we have been working to
> try to minimize any cases of mixed attributes [0], if a region is
> coherent then everyone in the system needs to treat it as such and
> vice-versa, even clever CMO that work on other systems wont save you
> here. :(
>
> [0] https://github.com/ARM-software/arm-trusted-firmware/pull/1553
>
>
> >>>
> ION buffer is allocated.
>
> //Camera device records video
> dma_buf_attach
> dma_map_attachment (buffer needs to be cleaned)
> >>>
> >>> Why does the buffer need to be cleaned here? I just got through reading
> >>> the thread linked by Laura in the other reply. I d
Re: [RFC PATCH] drm/ttm: force cached mappings for system RAM on ARM
On Thu, 17 Jan 2019 at 07:07, Benjamin Herrenschmidt wrote: > > On Wed, 2019-01-16 at 08:47 +0100, Ard Biesheuvel wrote: > > > As far as I know on x86 it doesn't, so when you have an un-cached page > > > you can still access it with a snooping DMA read/write operation and > > > don't cause trouble. > > > > > > > I think it is the other way around. The question is, on an otherwise > > cache coherent device, whether the NoSnoop attribute set by the GPU > > propagates all the way to the bus so that it bypasses the caches. > > On powerpc it's ignored, all DMA accesses will be snooped. But that's > fine regardless of whether the memory was mapped cachable or not, the > snooper will simply not find anything if not. I *think* we only do > cache inject if the line already exists in one of the caches. > Others should correct me if I am wrong, but arm64 SoCs often have L3 system caches, and I would expect inbound transactions with writeback write-allocate (WBWA) attributes to allocate there. > > On x86, we can tolerate if this is not the case, since uncached memory > > accesses by the CPU snoop the caches as well. > > > > On other architectures, uncached accesses go straight to main memory, > > so if the device wrote anything to the caches we won't see it. > > Well, on all powerpc implementations that I am aware of at least (dunno > about ARM), they do, but we don't have a problem because I don't think > the devices can/will write to the caches directly unless a > corresponding line already exists (but I might be wrong, we need to > double check all implementations which is tricky). > > I am not aware of any powerpc chip implementing NoSnoop. > Do you have any history on why this optimization is disabled for power unless CONFIG_NOT_CACHE_COHERENT is set? That also begs the question how any of this is supposed to work with non-cache coherent DMA. The code appears to always assume cache coherent, and provide non-cache coherent as an optimization if dma_arch_can_wc_memory() returns true. So I wonder if that helper should take a struct device pointer instead, and return true for non-cache coherent devices. > > So to use this optimization, you have to either be 100% sure that > > NoSnoop is implemented correctly, or have a x86 CPU. > > > > > > The old hack of using non-cached mapping to avoid snoop cost in AGP and > > > > others is just that ... an ugly and horrible hacks that should have > > > > never eventuated, when the search for performance pushes HW people into > > > > utter insanity :) > > > > > > Well I agree that un-cached system memory makes things much more > > > complicated for a questionable gain. > > > > > > But fact is we now have to deal with the mess, so no point in > > > complaining about it to much :) > > > > > > > Indeed. I wonder if we should just disable it altogether unless CONFIG_X86=y > > The question is whether DMA from a device can instanciate cache lines > in your system. This a system specific rather than architecture > specific question I suspect... > The ARM architecture permits it, afaict, and write-allocate is a hint so the implementation is free to ignore it, whether it is set or cleared. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v3 02/28] clk: sunxi-ng: Adjust MP clock parent rate when allowed
On Wed, Jan 16, 2019 at 06:00:32PM +0100, Jernej Škrabec wrote: > Dne sreda, 16. januar 2019 ob 13:09:58 CET je Priit Laes napisal(a): > > On Thu, Jan 10, 2019 at 06:10:59PM +0100, Jernej Škrabec wrote: > > > Dne četrtek, 10. januar 2019 ob 10:15:48 CET je Priit Laes napisal(a): > > > > On Sun, Nov 04, 2018 at 07:26:39PM +0100, Jernej Skrabec wrote: > > > > > Currently MP clocks don't consider adjusting parent rate even if they > > > > > are allowed to do so. Such behaviour considerably lowers amount of > > > > > possible rates, which is very inconvenient when such clock is used for > > > > > pixel clock, for example. > > > > > > > > > > In order to improve the situation, adjusting parent rate is considered > > > > > when allowed. > > > > > > > > > > This code is inspired by clk_divider_bestdiv() function, which does > > > > > basically the same thing for different clock type. > > > > > > > > This patch seems to break the eMMC support on Olinuxino-Lime2-eMMC > > > > boards: > > > > > > > > EXT4-fs (mmcblk1p4): INFO: recovery required on readonly filesystem > > > > EXT4-fs (mmcblk1p4): write access will be enabled during recovery > > > > sunxi-mmc 1c11000.mmc: data error, sending stop command > > > > sunxi-mmc 1c11000.mmc: send stop command failed > > > > > > I'm not familiar with A20. What is interesting is that emmc clocks don't > > > have CLK_SET_RATE_PARENT flag set, so you shouldn't see any difference. > > > > > > Can you post content of clk_summary with and without this patch? > > > > In both cases I booted from FEL with rootfs on sdcard and tried to mount > > partition from eMMC to /mnt. With your patch, last step it fails. > > > > pre-patch working: > > pll-ddr-other[768MHz] -> mmc2[512MHz]. (For some reason ahb-mmc2 is off?) > > > > post-patch not working: > > pll-periph[600MHz] -> mmc2[500Mhz], (ahb-mmc2 is enabled) > > > > Also, attached the logs. > > Thanks. Just one more request. Can you enable debug messages in mmc driver? > I'm interested in output of this line: > > dev_dbg(mmc_dev(mmc), "setting clk to %d, rounded %ld\n", > clock, rate); 1c11000 is eMMC: [snip] [1.961644] sunxi-mmc 1c11000.mmc: setting clk to 40, rounded 40 [2.004091] sunxi-mmc 1c11000.mmc: setting clk to 40, rounded 40 [2.020296] sunxi-mmc 1c11000.mmc: setting clk to 40, rounded 40 [2.039917] sunxi-mmc 1c11000.mmc: setting clk to 40, rounded 40 [2.047847] sunxi-mmc 1c11000.mmc: setting clk to 40, rounded 40 [2.055053] sunxi-mmc 1c11000.mmc: setting clk to 40, rounded 40 [2.065256] sunxi-mmc 1c11000.mmc: setting clk to 40, rounded 40 [2.092351] sunxi-mmc 1c11000.mmc: setting clk to 40, rounded 40 [2.168725] sunxi-mmc 1c11000.mmc: setting clk to 40, rounded 40 [2.189403] sunxi-mmc 1c11000.mmc: setting clk to 5200, rounded 5200 [2.203340] sunxi-mmc 1c11000.mmc: setting clk to 5200, rounded 5200 [2.211412] sunxi-mmc 1c11000.mmc: setting clk to 5200, rounded 5200 [4.967865] sunxi-mmc 1c11000.mmc: setting clk to 5200, rounded 5200 [8.755345] sunxi-mmc 1c11000.mmc: setting clk to 5200, rounded 5200 [9.082510] sunxi-mmc 1c11000.mmc: setting clk to 5200, rounded 5200 Here I tried to mount partition from eMMC... [ 72.167311] sunxi-mmc 1c11000.mmc: setting clk to 5200, rounded 5200 [ 72.269629] sunxi-mmc 1c11000.mmc: data error, sending stop command [ 73.268999] sunxi-mmc 1c11000.mmc: send stop command failed [/snip] And clock tree: [snip] pll-periph-base330 12 0 0 5 pll-periph 660 6 0 0 5 mmc2 3305000 0 0 5 mmc2_sample 1105000 0 120 5 mmc2_output 1105000 060 5 ahb 18 180 3 0 0 5 ahb-mmc2 110 3 0 0 5 [/snip] And without patch: [snip] [2.003341] sunxi-mmc 1c11000.mmc: XXX: setting clk to 40, rounded 40 [2.019479] sunxi-mmc 1c11000.mmc: XXX: setting clk to 40, rounded 40 [2.039144] sunxi-mmc 1c11000.mmc: XXX: setting clk to 40, rounded 40 [2.047129] sunxi-mmc 1c11000.mmc: XXX: setting clk to 40, rounded 40 [2.054324] sunxi-mmc 1c11000.mmc: XXX: setting clk to 40, rounded 40 [2.064481] sunxi-mmc 1c11000.mmc: XXX: setting clk to 40, rounded 40 [2.091624] sunxi-mmc 1c11000.mmc: XXX: setting clk to 40, rounded 40 [2.168067] sunxi-mmc 1c11000.mmc: XXX: setting clk to 40, rounded 40 [2.188239] sunxi-mmc 1c11000.mmc: XXX: setting clk to 5200, rounded 5120 [2.202779]
[PATCH 10/11] drm/mxsfb: Improve the axi clock usage
Currently, the enable of the axi clock return status is ignored, causing
issues when the enable fails then we try to disable it. Therefore, it is
better to check the return status and disable it only when enable
succeeded.
Also, remove the helper functions around clk_axi, since we can directly
use the clk API function for enable/disable the clock. Those functions
are already checking for NULL clk and returning 0 if that's the case.
Signed-off-by: Robert Chiras
Acked-by: Leonard Crestez
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 8
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 32 +---
drivers/gpu/drm/mxsfb/mxsfb_drv.h | 3 ---
3 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 35d6f10..cbdbd47 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -411,7 +411,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
{
dma_addr_t paddr;
- mxsfb_enable_axi_clk(mxsfb);
+ clk_prepare_enable(mxsfb->clk_axi);
writel(0, mxsfb->base + LCDC_CTRL);
mxsfb_crtc_mode_set_nofb(mxsfb);
@@ -428,7 +428,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
{
mxsfb_disable_controller(mxsfb);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
}
void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
@@ -456,9 +456,9 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private
*mxsfb,
paddr = mxsfb_get_fb_paddr(mxsfb);
if (paddr) {
- mxsfb_enable_axi_clk(mxsfb);
+ clk_prepare_enable(mxsfb->clk_axi);
writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
}
if (!fb || !old_fb)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 9247c76..34f3de1 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -101,18 +101,6 @@ drm_pipe_to_mxsfb_drm_private(struct
drm_simple_display_pipe *pipe)
return container_of(pipe, struct mxsfb_drm_private, pipe);
}
-void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
-{
- if (mxsfb->clk_axi)
- clk_prepare_enable(mxsfb->clk_axi);
-}
-
-void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
-{
- if (mxsfb->clk_axi)
- clk_disable_unprepare(mxsfb->clk_axi);
-}
-
/**
* mxsfb_atomic_helper_check - validate state object
* @dev: DRM device
@@ -235,25 +223,31 @@ static void mxsfb_pipe_update(struct
drm_simple_display_pipe *pipe,
static int mxsfb_pipe_enable_vblank(struct drm_simple_display_pipe *pipe)
{
struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
+ int ret = 0;
+
+ ret = clk_prepare_enable(mxsfb->clk_axi);
+ if (ret)
+ return ret;
/* Clear and enable VBLANK IRQ */
- mxsfb_enable_axi_clk(mxsfb);
writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_SET);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
- return 0;
+ return ret;
}
static void mxsfb_pipe_disable_vblank(struct drm_simple_display_pipe *pipe)
{
struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
+ if (clk_prepare_enable(mxsfb->clk_axi))
+ return;
+
/* Disable and clear VBLANK IRQ */
- mxsfb_enable_axi_clk(mxsfb);
writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_CLR);
writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
}
static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
@@ -438,7 +432,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
struct mxsfb_drm_private *mxsfb = drm->dev_private;
u32 reg;
- mxsfb_enable_axi_clk(mxsfb);
+ clk_prepare_enable(mxsfb->clk_axi);
reg = readl(mxsfb->base + LCDC_CTRL1);
@@ -447,7 +441,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
return IRQ_HANDLED;
}
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index 8b6f332..e75f319 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -46,9 +46,6 @@ struct mxsfb_drm_private {
int mxsfb_setup_crtc(struct drm_device *dev);
int mxsfb_create_output(struct drm_device *dev);
-void mxsfb
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Wed, Jan 16, 2019 at 05:11:34PM +0100, h...@lst.de wrote: > On Tue, Jan 15, 2019 at 02:25:01PM -0700, Jason Gunthorpe wrote: > > RDMA needs something similar as well, in this case drivers take a > > struct page * from get_user_pages() and need to have the DMA map fail > > if the platform can't DMA map in a way that does not require any > > additional DMA API calls to ensure coherence. (think Userspace RDMA > > MR's) > > Any time you dma map pages you need to do further DMA API calls to > ensure coherent, that is the way it is implemented. These calls > just happen to be no-ops sometimes. > > > Today we just do the normal DMA map and when it randomly doesn't work > > and corrupts data tell those people their platforms don't support RDMA > > - it would be nice to have a safer API base solution.. > > Now that all these drivers are consolidated in rdma-core you can fix > the code to actually do the right thing. It isn't that userspace DMA > coherent is any harder than in-kernel DMA coherenence. It just is > that no one bothered to do it properly. If I recall we actually can't.. libverbs presents an API to the user that does not consider this possibility. ie consider post_recv - the driver has no idea what user buffers received data and can't possibly flush them transparently. The user would have to call some special DMA syncing API, which we don't have. It is the same reason the kernel API makes the ULP handle dma sync, not the driver. The fact is there is 0 industry interest in using RDMA on platforms that can't do HW DMA cache coherency - the kernel syscalls required to do the cache flushing on the IO path would just destroy performance to the point of making RDMA pointless. Better to use netdev on those platforms. VFIO is in a similar boat. Their user API can't handle cache syncing either, so they would use the same API too. .. and the GPU-compute systems (ie OpenCL/CUDA) are like verbs, they were never designed with incoherent DMA in mind, and don't have the API design to support it. The reality is that *all* the subsytems doing DMA kernel bypass are ignoring the DMA mapping rules, I think we should support this better, and just accept that user space DMA will not be using syncing. Block access in cases when this is required, otherwise let it work as is today. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] pwm: Add MediaTek MT8183 display PWM driver support
On 16/01/2019 08:52, Jitao Shi wrote:
> Use the mtk_pwm_data struction to define different registers
> and add MT8183 specific register operations, such as MT8183
> have commit register, needs to enable double buffer
has_commit is set to false, so I suppose you mean that MT8183 does not have a
commit register.
Regards,
Matthias
> before writing register, and needs to select commit mode
> and use PWM_PERIOD/PWM_HIGH_WIDTH.
>
> Signed-off-by: Jitao Shi
> ---
> drivers/pwm/pwm-mtk-disp.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> index 893940d45f0d..15803c71fe80 100644
> --- a/drivers/pwm/pwm-mtk-disp.c
> +++ b/drivers/pwm/pwm-mtk-disp.c
> @@ -277,10 +277,21 @@ static const struct mtk_pwm_data mt8173_pwm_data = {
> .commit_mask = 0x1,
> };
>
> +static const struct mtk_pwm_data mt8183_pwm_data = {
> + .enable_mask = BIT(0),
> + .con0 = 0x18,
> + .con0_sel = 0x0,
> + .con1 = 0x1c,
> + .has_commit = false,
> + .bls_debug = 0x80,
> + .bls_debug_mask = 0x3,
> +};
> +
> static const struct of_device_id mtk_disp_pwm_of_match[] = {
> { .compatible = "mediatek,mt2701-disp-pwm", .data = &mt2701_pwm_data},
> { .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
> { .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
> + { .compatible = "mediatek,mt8183-disp-pwm", .data = &mt8183_pwm_data},
> { }
> };
> MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/11] drm/mxsfb: Move pm_runtime_enable at the end of probe
Move the pm_runtime_enable call at the end of probbe, since it was made too
early, causing issues to DRM enable routines.
Signed-off-by: Robert Chiras
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 3032007..9247c76 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -301,12 +301,10 @@ static int mxsfb_load(struct drm_device *drm, unsigned
long flags)
if (ret)
return ret;
- pm_runtime_enable(drm->dev);
-
ret = drm_vblank_init(drm, MAX_CRTCS);
if (ret < 0) {
dev_err(drm->dev, "Failed to initialise vblank\n");
- goto err_vblank;
+ return ret;
}
/* Modeset init */
@@ -315,7 +313,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
flags)
ret = mxsfb_create_output(drm);
if (ret < 0) {
dev_err(drm->dev, "Failed to create outputs\n");
- goto err_vblank;
+ return ret;
}
ret = drm_simple_display_pipe_init(drm, &mxsfb->pipe, &mxsfb_funcs,
@@ -323,7 +321,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
flags)
mxsfb->connector);
if (ret < 0) {
dev_err(drm->dev, "Cannot setup simple display pipe\n");
- goto err_vblank;
+ return ret;
}
drm_crtc_vblank_off(&mxsfb->pipe.crtc);
@@ -340,14 +338,14 @@ static int mxsfb_load(struct drm_device *drm, unsigned
long flags)
ret = drm_panel_attach(mxsfb->panel, mxsfb->connector);
if (ret) {
dev_err(drm->dev, "Cannot connect panel\n");
- goto err_vblank;
+ return ret;
}
} else if (mxsfb->bridge) {
ret = drm_simple_display_pipe_attach_bridge(&mxsfb->pipe,
mxsfb->bridge);
if (ret) {
dev_err(drm->dev, "Cannot connect bridge\n");
- goto err_vblank;
+ return ret;
}
}
@@ -367,9 +365,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
flags)
drm_mode_config_reset(drm);
- pm_runtime_get_sync(drm->dev);
ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
- pm_runtime_put_sync(drm->dev);
if (ret < 0) {
dev_err(drm->dev, "Failed to install IRQ handler\n");
@@ -391,14 +387,14 @@ static int mxsfb_load(struct drm_device *drm, unsigned
long flags)
drm_helper_hpd_irq_event(drm);
+ pm_runtime_enable(drm->dev);
+
return 0;
err_cma:
drm_irq_uninstall(drm);
err_irq:
drm_panel_detach(mxsfb->panel);
-err_vblank:
- pm_runtime_disable(drm->dev);
return ret;
}
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/11] drm/mxsfb: Update mxsfb with additional pixel formats
Since version 4 of eLCDIF, there are some registers that can do
transformations on the input data, like re-arranging the pixel
components. By doing that, we can support more pixel formats.
This patch adds support for X/ABGR and RGBX/A. Although, the local alpha
is not supported by eLCDIF, the alpha pixel formats were added to the
supported pixel formats but it will be ignored. This was necessary since
there are systems (like Android) that requires such pixel formats.
Signed-off-by: Robert Chiras
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 108 -
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 25 ++---
drivers/gpu/drm/mxsfb/mxsfb_drv.h | 3 +-
drivers/gpu/drm/mxsfb/mxsfb_regs.h | 100 --
4 files changed, 166 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 3210be9..2fdb268 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -48,15 +48,35 @@ static u32 set_hsync_pulse_width(struct mxsfb_drm_private
*mxsfb, u32 val)
mxsfb->devdata->hs_wdth_shift;
}
+/* Print Four-character-code (FOURCC) */
+static char *fourcc_to_str(u32 fmt)
+{
+ /* Use 10 chars so we can simultaneously print two codes */
+ static char code[10], *c = &code[0];
+
+ if (c == &code[10])
+ c = &code[0];
+
+ *(c++) = (unsigned char)(fmt & 0xff);
+ *(c++) = (unsigned char)((fmt >> 8) & 0xff);
+ *(c++) = (unsigned char)((fmt >> 16) & 0xff);
+ *(c++) = (unsigned char)((fmt >> 24) & 0xff);
+ *(c++) = '\0';
+
+ return (c - 5);
+}
+
/* Setup the MXSFB registers for decoding the pixels out of the framebuffer */
-static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb)
+static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb, bool update)
{
struct drm_crtc *crtc = &mxsfb->pipe.crtc;
struct drm_device *drm = crtc->dev;
const u32 format = crtc->primary->state->fb->format->format;
- u32 ctrl, ctrl1;
+ u32 ctrl = 0, ctrl1 = 0;
+ bool bgr_format = true;
- ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
+ if (!update)
+ ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
/*
* WARNING: The bus width, CTRL_SET_BUS_WIDTH(), is configured to
@@ -65,31 +85,69 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private
*mxsfb)
* to arbitrary value. This limitation should not pose an issue.
*/
- /* CTRL1 contains IRQ config and status bits, preserve those. */
- ctrl1 = readl(mxsfb->base + LCDC_CTRL1);
- ctrl1 &= CTRL1_CUR_FRAME_DONE_IRQ_EN | CTRL1_CUR_FRAME_DONE_IRQ;
+ if (!update) {
+ /* CTRL1 contains IRQ config and status bits, preserve those. */
+ ctrl1 = readl(mxsfb->base + LCDC_CTRL1);
+ ctrl1 &= CTRL1_CUR_FRAME_DONE_IRQ_EN | CTRL1_CUR_FRAME_DONE_IRQ;
+ }
+
+ DRM_DEV_DEBUG_DRIVER(drm->dev, "Setting up %s mode\n",
+ fourcc_to_str(format));
+
+ /* Do some clean-up that we might have from a previous mode */
+ ctrl &= ~CTRL_SHIFT_DIR(1);
+ ctrl &= ~CTRL_SHIFT_NUM(0x3f);
+ if (mxsfb->devdata->ipversion >= 4)
+ writel(CTRL2_ODD_LINE_PATTERN(0x7) |
+ CTRL2_EVEN_LINE_PATTERN(0x7),
+ mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
switch (format) {
case DRM_FORMAT_RGB565:
- dev_dbg(drm->dev, "Setting up RGB565 mode\n");
ctrl |= CTRL_SET_WORD_LENGTH(0);
ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
break;
+ case DRM_FORMAT_RGBX:
+ case DRM_FORMAT_RGBA:
+ /* RGBX - > 0RGB */
+ ctrl |= CTRL_SHIFT_DIR(1);
+ ctrl |= CTRL_SHIFT_NUM(8);
+ bgr_format = false;
+ /* Fall through */
+ case DRM_FORMAT_XBGR:
+ case DRM_FORMAT_ABGR:
+ if (bgr_format) {
+ if (mxsfb->devdata->ipversion < 4)
+ goto err;
+ writel(CTRL2_ODD_LINE_PATTERN(0x5) |
+ CTRL2_EVEN_LINE_PATTERN(0x5),
+ mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+ }
+ /* Fall through */
case DRM_FORMAT_XRGB:
- dev_dbg(drm->dev, "Setting up XRGB mode\n");
+ case DRM_FORMAT_ARGB:
ctrl |= CTRL_SET_WORD_LENGTH(3);
/* Do not use packed pixels = one pixel per word instead. */
ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7);
break;
default:
- dev_err(drm->dev, "Unhandled pixel format %08x\n", format);
- return -EINVAL;
+ goto err;
}
- writel(ctrl1, mxsfb->base + LCDC_CTRL1);
- writel(ctrl, mxsfb->b
[PATCH 08/11] drm/mxsfb: Update mxsfb to support LCD reset
The eLCDIF controller has control pin for the external LCD reset pin.
Add support for it and assert this pin in enable and de-assert it in
disable.
Signed-off-by: Robert Chiras
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 10 --
drivers/gpu/drm/mxsfb/mxsfb_regs.h | 1 +
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index b1d0267..35d6f10 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -230,9 +230,12 @@ static void mxsfb_enable_controller(struct
mxsfb_drm_private *mxsfb)
clk_prepare_enable(mxsfb->clk_disp_axi);
clk_prepare_enable(mxsfb->clk);
- if (mxsfb->devdata->ipversion >= 4)
+ if (mxsfb->devdata->ipversion >= 4) {
writel(CTRL2_OUTSTANDING_REQS(REQ_16),
mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+ /* Assert LCD Reset bit */
+ writel(CTRL2_LCD_RESET, mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+ }
/* If it was disabled, re-enable the mode again */
writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
@@ -250,9 +253,12 @@ static void mxsfb_disable_controller(struct
mxsfb_drm_private *mxsfb)
{
u32 reg;
- if (mxsfb->devdata->ipversion >= 4)
+ if (mxsfb->devdata->ipversion >= 4) {
writel(CTRL2_OUTSTANDING_REQS(0x7),
mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
+ /* De-assert LCD Reset bit */
+ writel(CTRL2_LCD_RESET, mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
+ }
writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_CLR);
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index 4904fdd..1d85750 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -95,6 +95,7 @@
#define CTRL2_OUTSTANDING_REQS(x) REG_PUT((x), 23, 21)
#define CTRL2_ODD_LINE_PATTERN(x) REG_PUT((x), 18, 16)
#define CTRL2_EVEN_LINE_PATTERN(x) REG_PUT((x), 14, 12)
+#define CTRL2_LCD_RESETBIT(0)
#define TRANSFER_COUNT_SET_VCOUNT(x) (((x) & 0x) << 16)
#define TRANSFER_COUNT_GET_VCOUNT(x) (((x) >> 16) & 0x)
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] Staging: fbtft: Switch to the gpio descriptor interface
This switches the fbtft driver to use GPIO descriptors
rather than numerical gpios:
Utilize the GPIO library's intrinsic handling of OF GPIOs
and polarity. If the line is flagged active low, gpiolib
will deal with this.
Remove gpios from platform device structure. Neither assign
statically numbers to gpios in platform device nor allow
gpios to be parsed as module parameters.
Signed-off-by: Nishad Kamdar
---
Changes in v3:
- Correct the changelog position.
Changes in v2:
- Merge all patches in a single patch. This is because the
first patch changes par->gpio from an int to a pointer
so all the checks have to be updated in the same patch.
Otherwie it breaks 'git bisect'.
---
drivers/staging/fbtft/fb_agm1264k-fl.c | 52 ++--
drivers/staging/fbtft/fb_bd663474.c| 6 +-
drivers/staging/fbtft/fb_ili9163.c | 6 +-
drivers/staging/fbtft/fb_ili9320.c | 2 +-
drivers/staging/fbtft/fb_ili9325.c | 6 +-
drivers/staging/fbtft/fb_ili9340.c | 2 +-
drivers/staging/fbtft/fb_pcd8544.c | 4 +-
drivers/staging/fbtft/fb_ra8875.c | 4 +-
drivers/staging/fbtft/fb_s6d1121.c | 6 +-
drivers/staging/fbtft/fb_sh1106.c | 2 +-
drivers/staging/fbtft/fb_ssd1289.c | 6 +-
drivers/staging/fbtft/fb_ssd1305.c | 4 +-
drivers/staging/fbtft/fb_ssd1306.c | 4 +-
drivers/staging/fbtft/fb_ssd1325.c | 6 +-
drivers/staging/fbtft/fb_ssd1331.c | 10 +-
drivers/staging/fbtft/fb_ssd1351.c | 2 +-
drivers/staging/fbtft/fb_tls8204.c | 6 +-
drivers/staging/fbtft/fb_uc1611.c | 4 +-
drivers/staging/fbtft/fb_uc1701.c | 6 +-
drivers/staging/fbtft/fb_upd161704.c | 6 +-
drivers/staging/fbtft/fb_watterott.c | 4 +-
drivers/staging/fbtft/fbtft-bus.c | 6 +-
drivers/staging/fbtft/fbtft-core.c | 173 +++--
drivers/staging/fbtft/fbtft-io.c | 26 +-
drivers/staging/fbtft/fbtft.h | 21 +-
drivers/staging/fbtft/fbtft_device.c | 344 +
drivers/staging/fbtft/flexfb.c | 12 +-
27 files changed, 143 insertions(+), 587 deletions(-)
diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c
b/drivers/staging/fbtft/fb_agm1264k-fl.c
index f6f30f5bf15a..8f27bd8da17d 100644
--- a/drivers/staging/fbtft/fb_agm1264k-fl.c
+++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
@@ -8,7 +8,7 @@
#include
#include
#include
-#include
+#include
#include
#include
@@ -79,14 +79,14 @@ static int init_display(struct fbtft_par *par)
static void reset(struct fbtft_par *par)
{
- if (par->gpio.reset == -1)
+ if (!par->gpio.reset)
return;
dev_dbg(par->info->device, "%s()\n", __func__);
- gpio_set_value(par->gpio.reset, 0);
+ gpiod_set_value(par->gpio.reset, 0);
udelay(20);
- gpio_set_value(par->gpio.reset, 1);
+ gpiod_set_value(par->gpio.reset, 1);
mdelay(120);
}
@@ -98,30 +98,30 @@ static int verify_gpios(struct fbtft_par *par)
dev_dbg(par->info->device,
"%s()\n", __func__);
- if (par->EPIN < 0) {
+ if (!par->EPIN) {
dev_err(par->info->device,
"Missing info about 'wr' (aka E) gpio. Aborting.\n");
return -EINVAL;
}
for (i = 0; i < 8; ++i) {
- if (par->gpio.db[i] < 0) {
+ if (!par->gpio.db[i]) {
dev_err(par->info->device,
"Missing info about 'db[%i]' gpio. Aborting.\n",
i);
return -EINVAL;
}
}
- if (par->CS0 < 0) {
+ if (!par->CS0) {
dev_err(par->info->device,
"Missing info about 'cs0' gpio. Aborting.\n");
return -EINVAL;
}
- if (par->CS1 < 0) {
+ if (!par->CS1) {
dev_err(par->info->device,
"Missing info about 'cs1' gpio. Aborting.\n");
return -EINVAL;
}
- if (par->RW < 0) {
+ if (!par->RW) {
dev_err(par->info->device,
"Missing info about 'rw' gpio. Aborting.\n");
return -EINVAL;
@@ -139,22 +139,22 @@ request_gpios_match(struct fbtft_par *par, const struct
fbtft_gpio *gpio)
if (strcasecmp(gpio->name, "wr") == 0) {
/* left ks0108 E pin */
par->EPIN = gpio->gpio;
- return GPIOF_OUT_INIT_LOW;
+ return GPIOD_OUT_LOW;
} else if (strcasecmp(gpio->name, "cs0") == 0) {
/* left ks0108 controller pin */
par->CS0 = gpio->gpio;
- return GPIOF_OUT_INIT_HIGH;
+ return GPIOD_OUT_HIGH;
} else if (strcasecmp(gpio->name, "cs1") == 0) {
/* right ks0108 controller pin */
par->CS1 = gpio->gpio;
- return GPIOF_OUT_INIT_HIGH;
+ return GPIOD_O
[PATCH 01/11] drm/mxsfb: Update mxsfb to support a bridge
Currently, the MXSFB DRM driver only supports a panel. But, its output
display signal can also be redirected to another encoder, like a DSI
controller. In this case, that DSI controller may act like a drm_bridge.
In order support this use-case too, this patch adds support for
drm_bridge in mxsfb.
Signed-off-by: Robert Chiras
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 46 +++---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 46 +-
drivers/gpu/drm/mxsfb/mxsfb_drv.h | 4 +++-
drivers/gpu/drm/mxsfb/mxsfb_out.c | 26 +++--
drivers/gpu/drm/mxsfb/mxsfb_regs.h | 15 +
5 files changed, 116 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 24b1f0c..3210be9 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -101,8 +101,11 @@ static void mxsfb_set_bus_fmt(struct mxsfb_drm_private
*mxsfb)
reg = readl(mxsfb->base + LCDC_CTRL);
- if (mxsfb->connector.display_info.num_bus_formats)
- bus_format = mxsfb->connector.display_info.bus_formats[0];
+ if (mxsfb->connector->display_info.num_bus_formats)
+ bus_format = mxsfb->connector->display_info.bus_formats[0];
+
+ DRM_DEV_DEBUG_DRIVER(drm->dev,
+ "Using bus_format: 0x%08X\n", bus_format);
reg &= ~CTRL_BUS_WIDTH_MASK;
switch (bus_format) {
@@ -130,6 +133,9 @@ static void mxsfb_enable_controller(struct
mxsfb_drm_private *mxsfb)
clk_prepare_enable(mxsfb->clk_disp_axi);
clk_prepare_enable(mxsfb->clk);
+ writel(CTRL2_OUTSTANDING_REQS__REQ_16,
+ mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+
/* If it was disabled, re-enable the mode again */
writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
@@ -139,12 +145,15 @@ static void mxsfb_enable_controller(struct
mxsfb_drm_private *mxsfb)
writel(reg, mxsfb->base + LCDC_VDCTRL4);
writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
+ writel(CTRL1_RECOVERY_ON_UNDERFLOW, mxsfb->base + LCDC_CTRL1 + REG_SET);
}
static void mxsfb_disable_controller(struct mxsfb_drm_private *mxsfb)
{
u32 reg;
+ writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_CLR);
+
/*
* Even if we disable the controller here, it will still continue
* until its FIFOs are running out of data
@@ -210,8 +219,9 @@ static dma_addr_t mxsfb_get_fb_paddr(struct
mxsfb_drm_private *mxsfb)
static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)
{
+ struct drm_device *drm = mxsfb->pipe.crtc.dev;
struct drm_display_mode *m = &mxsfb->pipe.crtc.state->adjusted_mode;
- const u32 bus_flags = mxsfb->connector.display_info.bus_flags;
+ const u32 bus_flags = mxsfb->connector->display_info.bus_flags;
u32 vdctrl0, vsync_pulse_len, hsync_pulse_len;
int err;
@@ -235,6 +245,13 @@ static void mxsfb_crtc_mode_set_nofb(struct
mxsfb_drm_private *mxsfb)
clk_set_rate(mxsfb->clk, m->crtc_clock * 1000);
+ DRM_DEV_DEBUG_DRIVER(drm->dev, "Pixel clock: %dkHz (actual: %dkHz)\n",
+ m->crtc_clock, (int)(clk_get_rate(mxsfb->clk) / 1000));
+ DRM_DEV_DEBUG_DRIVER(drm->dev,
+ "Connector bus_flags: 0x%08X\n", bus_flags);
+ DRM_DEV_DEBUG_DRIVER(drm->dev,
+ "Mode flags: 0x%08X\n", m->flags);
+
writel(TRANSFER_COUNT_SET_VCOUNT(m->crtc_vdisplay) |
TRANSFER_COUNT_SET_HCOUNT(m->crtc_hdisplay),
mxsfb->base + mxsfb->devdata->transfer_count);
@@ -287,6 +304,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
dma_addr_t paddr;
mxsfb_enable_axi_clk(mxsfb);
+ writel(0, mxsfb->base + LCDC_CTRL);
mxsfb_crtc_mode_set_nofb(mxsfb);
/* Write cur_buf as well to avoid an initial corrupt frame */
@@ -310,6 +328,8 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private
*mxsfb,
{
struct drm_simple_display_pipe *pipe = &mxsfb->pipe;
struct drm_crtc *crtc = &pipe->crtc;
+ struct drm_framebuffer *fb = pipe->plane.state->fb;
+ struct drm_framebuffer *old_fb = old_state->fb;
struct drm_pending_vblank_event *event;
dma_addr_t paddr;
@@ -332,4 +352,24 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private
*mxsfb,
writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
mxsfb_disable_axi_clk(mxsfb);
}
+
+ if (!fb || !old_fb)
+ return;
+
+ /*
+* TODO: Currently, we only support pixel format change, but we need
+* also to care about size changes too
+*/
+ if (old_fb->format->format != fb->format->format) {
+ struct drm_format_name_buf old_fmt_buf;
+ struct drm_format_name_buf new_fmt_buf;
+
+ DRM_DEV_DEBUG_DRIVER(crtc-
[PATCH 07/11] drm/mxsfb: Signal mode changed when bpp changed
From: Mirela Rabulea
Add mxsfb_atomic_helper_check to signal mode changed when bpp changed.
This will trigger the execution of disable/enable on
a modeset with different bpp than the current one.
Signed-off-by: Mirela Rabulea
Signed-off-by: Robert Chiras
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 48 ++-
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index eb01484..3032007 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -113,9 +113,55 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
clk_disable_unprepare(mxsfb->clk_axi);
}
+/**
+ * mxsfb_atomic_helper_check - validate state object
+ * @dev: DRM device
+ * @state: the driver state object
+ *
+ * On top of the drm imlementation drm_atomic_helper_check,
+ * check if the bpp is changed, if so, signal mode_changed,
+ * this will trigger disable/enable
+ *
+ * RETURNS:
+ * Zero for success or -errno
+ */
+static int mxsfb_atomic_helper_check(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *new_state;
+ int i, ret;
+
+ ret = drm_atomic_helper_check(dev, state);
+ if (ret)
+ return ret;
+
+ for_each_new_crtc_in_state(state, crtc, new_state, i) {
+ struct drm_plane_state *primary_state;
+ int old_bpp = 0;
+ int new_bpp = 0;
+
+ if (!crtc->primary || !crtc->primary->old_fb)
+ continue;
+ primary_state =
+ drm_atomic_get_plane_state(state, crtc->primary);
+ if (!primary_state || !primary_state->fb)
+ continue;
+ old_bpp = crtc->primary->old_fb->format->depth;
+ new_bpp = primary_state->fb->format->depth;
+ if (old_bpp != new_bpp) {
+ new_state->mode_changed = true;
+ DRM_DEBUG_ATOMIC(
+ "[CRTC:%d:%s] mode changed, bpp %d->%d\n",
+ crtc->base.id, crtc->name, old_bpp, new_bpp);
+ }
+ }
+ return ret;
+}
+
static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
.fb_create = drm_gem_fb_create,
- .atomic_check = drm_atomic_helper_check,
+ .atomic_check = mxsfb_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
[PATCH 06/11] drm/mxsfb: Add support for new pixel formats in eLCDIF
From: Mirela Rabulea
Add support for the following pixel formats:
16 bpp: RG16 ,BG16, XR15, XB15, AR15, AB15
Set the bus format based on input from the user and panel capabilities.
Save the bus format in crtc->mode.private_flags, the DSI will use it.
Use drm_get_format_name instead of locally defined fourcc_to_str.
Signed-off-by: Mirela Rabulea
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 100 ++---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 5 ++
2 files changed, 76 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 2fdb268..b1d0267 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -48,24 +48,6 @@ static u32 set_hsync_pulse_width(struct mxsfb_drm_private
*mxsfb, u32 val)
mxsfb->devdata->hs_wdth_shift;
}
-/* Print Four-character-code (FOURCC) */
-static char *fourcc_to_str(u32 fmt)
-{
- /* Use 10 chars so we can simultaneously print two codes */
- static char code[10], *c = &code[0];
-
- if (c == &code[10])
- c = &code[0];
-
- *(c++) = (unsigned char)(fmt & 0xff);
- *(c++) = (unsigned char)((fmt >> 8) & 0xff);
- *(c++) = (unsigned char)((fmt >> 16) & 0xff);
- *(c++) = (unsigned char)((fmt >> 24) & 0xff);
- *(c++) = '\0';
-
- return (c - 5);
-}
-
/* Setup the MXSFB registers for decoding the pixels out of the framebuffer */
static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb, bool update)
{
@@ -74,6 +56,7 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private
*mxsfb, bool update)
const u32 format = crtc->primary->state->fb->format->format;
u32 ctrl = 0, ctrl1 = 0;
bool bgr_format = true;
+ struct drm_format_name_buf format_name_buf;
if (!update)
ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
@@ -92,7 +75,7 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private
*mxsfb, bool update)
}
DRM_DEV_DEBUG_DRIVER(drm->dev, "Setting up %s mode\n",
- fourcc_to_str(format));
+ drm_get_format_name(format, &format_name_buf));
/* Do some clean-up that we might have from a previous mode */
ctrl &= ~CTRL_SHIFT_DIR(1);
@@ -103,19 +86,41 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private
*mxsfb, bool update)
mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
switch (format) {
- case DRM_FORMAT_RGB565:
+ case DRM_FORMAT_BGR565: /* BG16 */
+ if (mxsfb->devdata->ipversion < 4)
+ goto err;
+ writel(CTRL2_ODD_LINE_PATTERN(0x5) |
+ CTRL2_EVEN_LINE_PATTERN(0x5),
+ mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+ /* Fall through */
+ case DRM_FORMAT_RGB565: /* RG16 */
ctrl |= CTRL_SET_WORD_LENGTH(0);
+ ctrl &= ~CTRL_DF16;
ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
break;
- case DRM_FORMAT_RGBX:
- case DRM_FORMAT_RGBA:
+ case DRM_FORMAT_XBGR1555: /* XB15 */
+ case DRM_FORMAT_ABGR1555: /* AB15 */
+ if (mxsfb->devdata->ipversion < 4)
+ goto err;
+ writel(CTRL2_ODD_LINE_PATTERN(0x5) |
+ CTRL2_EVEN_LINE_PATTERN(0x5),
+ mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+ /* Fall through */
+ case DRM_FORMAT_XRGB1555: /* XR15 */
+ case DRM_FORMAT_ARGB1555: /* AR15 */
+ ctrl |= CTRL_SET_WORD_LENGTH(0);
+ ctrl |= CTRL_DF16;
+ ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
+ break;
+ case DRM_FORMAT_RGBX: /* RX24 */
+ case DRM_FORMAT_RGBA: /* RA24 */
/* RGBX - > 0RGB */
ctrl |= CTRL_SHIFT_DIR(1);
ctrl |= CTRL_SHIFT_NUM(8);
bgr_format = false;
/* Fall through */
- case DRM_FORMAT_XBGR:
- case DRM_FORMAT_ABGR:
+ case DRM_FORMAT_XBGR: /* XB24 */
+ case DRM_FORMAT_ABGR: /* AB24 */
if (bgr_format) {
if (mxsfb->devdata->ipversion < 4)
goto err;
@@ -124,8 +129,8 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private
*mxsfb, bool update)
mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
}
/* Fall through */
- case DRM_FORMAT_XRGB:
- case DRM_FORMAT_ARGB:
+ case DRM_FORMAT_XRGB: /* XR24 */
+ case DRM_FORMAT_ARGB: /* AR24 */
ctrl |= CTRL_SET_WORD_LENGTH(3);
/* Do not use packed pixels = one pixel per word instead. */
ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7);
@@ -146,19 +151,56 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private
Re: [PATCH v4 9/9] drm/bridge: cdns: Convert to phy framework
Hi,
On 09/01/19 3:03 PM, Maxime Ripard wrote:
> Now that we have everything we need in the phy framework to allow to tune
> the phy parameters, let's convert the Cadence DSI bridge to that API
> instead of creating a ad-hoc driver for its phy.
>
> Signed-off-by: Maxime Ripard
For this too, need ACKs from DRM MAINTAINERS.
Thanks
Kishon
> ---
> drivers/gpu/drm/bridge/Kconfig| 1 +-
> drivers/gpu/drm/bridge/cdns-dsi.c | 485 +++
> drivers/phy/cadence/cdns-dphy.c | 2 +-
> 3 files changed, 61 insertions(+), 427 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 2fee47b0d50b..8840f396a7b6 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -30,6 +30,7 @@ config DRM_CDNS_DSI
> select DRM_KMS_HELPER
> select DRM_MIPI_DSI
> select DRM_PANEL_BRIDGE
> + select GENERIC_PHY_MIPI_DPHY
> depends on OF
> help
> Support Cadence DPI to DSI bridge. This is an internal
> diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
> b/drivers/gpu/drm/bridge/cdns-dsi.c
> index 3ac6dd524b6d..7b432257ffbe 100644
> --- a/drivers/gpu/drm/bridge/cdns-dsi.c
> +++ b/drivers/gpu/drm/bridge/cdns-dsi.c
> @@ -21,6 +21,9 @@
> #include
> #include
>
> +#include
> +#include
> +
> #define IP_CONF 0x0
> #define SP_HS_FIFO_DEPTH(x) (((x) & GENMASK(30, 26)) >> 26)
> #define SP_LP_FIFO_DEPTH(x) (((x) & GENMASK(25, 21)) >> 21)
> @@ -419,44 +422,11 @@
> #define DSI_NULL_FRAME_OVERHEAD 6
> #define DSI_EOT_PKT_SIZE 4
>
> -#define REG_WAKEUP_TIME_NS 800
> -#define DPHY_PLL_RATE_HZ 10800
> -
> -/* DPHY registers */
> -#define DPHY_PMA_CMN(reg)(reg)
> -#define DPHY_PMA_LCLK(reg) (0x100 + (reg))
> -#define DPHY_PMA_LDATA(lane, reg)(0x200 + ((lane) * 0x100) + (reg))
> -#define DPHY_PMA_RCLK(reg) (0x600 + (reg))
> -#define DPHY_PMA_RDATA(lane, reg)(0x700 + ((lane) * 0x100) + (reg))
> -#define DPHY_PCS(reg)(0xb00 + (reg))
> -
> -#define DPHY_CMN_SSM DPHY_PMA_CMN(0x20)
> -#define DPHY_CMN_SSM_EN BIT(0)
> -#define DPHY_CMN_TX_MODE_EN BIT(9)
> -
> -#define DPHY_CMN_PWM DPHY_PMA_CMN(0x40)
> -#define DPHY_CMN_PWM_DIV(x) ((x) << 20)
> -#define DPHY_CMN_PWM_LOW(x) ((x) << 10)
> -#define DPHY_CMN_PWM_HIGH(x) (x)
> -
> -#define DPHY_CMN_FBDIV DPHY_PMA_CMN(0x4c)
> -#define DPHY_CMN_FBDIV_VAL(low, high)(((high) << 11) | ((low) << 22))
> -#define DPHY_CMN_FBDIV_FROM_REG (BIT(10) | BIT(21))
> -
> -#define DPHY_CMN_OPIPDIV DPHY_PMA_CMN(0x50)
> -#define DPHY_CMN_IPDIV_FROM_REG BIT(0)
> -#define DPHY_CMN_IPDIV(x)((x) << 1)
> -#define DPHY_CMN_OPDIV_FROM_REG BIT(6)
> -#define DPHY_CMN_OPDIV(x)((x) << 7)
> -
> -#define DPHY_PSM_CFG DPHY_PCS(0x4)
> -#define DPHY_PSM_CFG_FROM_REGBIT(0)
> -#define DPHY_PSM_CLK_DIV(x) ((x) << 1)
> -
> struct cdns_dsi_output {
> struct mipi_dsi_device *dev;
> struct drm_panel *panel;
> struct drm_bridge *bridge;
> + union phy_configure_opts phy_opts;
> };
>
> enum cdns_dsi_input_id {
> @@ -465,14 +435,6 @@ enum cdns_dsi_input_id {
> CDNS_DSC_INPUT,
> };
>
> -struct cdns_dphy_cfg {
> - u8 pll_ipdiv;
> - u8 pll_opdiv;
> - u16 pll_fbdiv;
> - unsigned long lane_bps;
> - unsigned int nlanes;
> -};
> -
> struct cdns_dsi_cfg {
> unsigned int hfp;
> unsigned int hsa;
> @@ -481,34 +443,6 @@ struct cdns_dsi_cfg {
> unsigned int htotal;
> };
>
> -struct cdns_dphy;
> -
> -enum cdns_dphy_clk_lane_cfg {
> - DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
> - DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
> - DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
> - DPHY_CLK_CFG_RIGHT_DRIVES_ALL = 3,
> -};
> -
> -struct cdns_dphy_ops {
> - int (*probe)(struct cdns_dphy *dphy);
> - void (*remove)(struct cdns_dphy *dphy);
> - void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
> - void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
> - enum cdns_dphy_clk_lane_cfg cfg);
> - void (*set_pll_cfg)(struct cdns_dphy *dphy,
> - const struct cdns_dphy_cfg *cfg);
> - unsigned long (*get_wakeup_time_ns)(struct cdns_dphy *dphy);
> -};
> -
> -struct cdns_dphy {
> - struct cdns_dphy_cfg cfg;
> - void __iomem *regs;
> - struct clk *psm_clk;
> - struct clk *pll_ref_clk;
> - const struct cdns_dphy_ops *ops;
> -};
> -
> struct cdns_dsi_input {
> enum cdns_dsi_input_id id;
> struct drm_bridge bridge;
> @@ -526,7 +460,7 @@ struct cdns_dsi {
> struct reset_control *dsi_p_rst;
> struct clk *dsi_sys_clk;
> bool link_initialized;
> - struct cdns_
Re: [PATCH v2] Staging: fbtft: Switch to the gpio descriptor interface
On Tue, Jan 15, 2019 at 04:02:31PM +0100, Greg Kroah-Hartman wrote: > On Tue, Jan 15, 2019 at 10:17:09AM +0530, Nishad Kamdar wrote: > > This switches the fbtft driver to use GPIO descriptors > > rather than numerical gpios: > > > > Utilize the GPIO library's intrinsic handling of OF GPIOs > > and polarity. If the line is flagged active low, gpiolib > > will deal with this. > > > > Remove gpios from platform device structure. Neither assign > > statically numbers to gpios in platform device nor allow > > gpios to be parsed as module parameters. > > > > Signed-off-by: Nishad Kamdar > > Changes in v2: > > - Merge all patches in a single patch. This is because the > >first patch changes par->gpio from an int to a pointer > >so all the checks have to be updated in the same patch. > >Otherwie it breaks 'git bisect'. > > The "v2" information goes below the --- line please. > > Also, I got 2 copies of this, both different, so I have no idea which to > apply. Please fix up and resend a v3. > Sorry for the confusion. I'll fix and resend. > thanks, > > greg k-h Thanks for the review. Regards, Nishad ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 11/11] drm/mxsfb: Clear OUTSTANDING_REQS bits
Bit 21 can alter the CTRL2_OUTSTANDING_REQS value right after the eLCDIF
is enabled, since it comes up with default value of 1 (this behaviour
has been seen on some imx8 platforms).
In order to fix this, clear CTRL2_OUTSTANDING_REQS bits before setting
its value.
Signed-off-by: Robert Chiras
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index cbdbd47..41b3648 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -231,6 +231,13 @@ static void mxsfb_enable_controller(struct
mxsfb_drm_private *mxsfb)
clk_prepare_enable(mxsfb->clk);
if (mxsfb->devdata->ipversion >= 4) {
+ /*
+* On some platforms, bit 21 is defaulted to 1, which may alter
+* the below setting. So, to make sure we have the right setting
+* clear all the bits for CTRL2_OUTSTANDING_REQS.
+*/
+ writel(CTRL2_OUTSTANDING_REQS(0x7),
+ mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
writel(CTRL2_OUTSTANDING_REQS(REQ_16),
mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
/* Assert LCD Reset bit */
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: Fix kerneldoc comment for struct dma_fence_array
On Wed, Jan 16, 2019 at 03:34:36PM -0700, Jonathan Corbet wrote:
> The kerneldoc comment for struct dma_fence_array lacks a description
> of the "work" member, leading to this docs-build warning:
>
> ./include/linux/dma-fence-array.h:54: warning: Function parameter or member
> 'work' not described in 'dma_fence_array'
>
> Add a description and make the warning go away.
>
> Signed-off-by: Jonathan Corbet
Applied for 5.1, thanks for your patch.
-Daniel
> ---
> Hopefully I've sent this to the right place; perhaps this file needs to be
> added to the MAINTAINERS entry?
>
> include/linux/dma-fence-array.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
> index bc8940ca280d..c0ff417b4770 100644
> --- a/include/linux/dma-fence-array.h
> +++ b/include/linux/dma-fence-array.h
> @@ -40,6 +40,7 @@ struct dma_fence_array_cb {
> * @num_fences: number of fences in the array
> * @num_pending: fences in the array still pending
> * @fences: array of the fences
> + * @work: internal irq_work function
> */
> struct dma_fence_array {
> struct dma_fence base;
> --
> 2.20.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
Re: [PATCH] drm/crtc-helper: Add store the property value
On Thu, Jan 17, 2019 at 05:50:44PM +0900, Hoegeun Kwon wrote:
> There is a problem in crtc_helper that property value is not updated
> when dpms is turned on or off. So modify the property value when dpms
> is on.
>
> Signed-off-by: Hoegeun Kwon
This is fixed with atomic, and exynos is atomic. Why do you care about
this?
-Daniel
> ---
> drivers/gpu/drm/drm_crtc_helper.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c
> index a3c81850e755..57d359f0725c 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -735,6 +735,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
> DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS
> on\n", set->connectors[i]->base.id,
> set->connectors[i]->name);
>
> set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
> +
> +
> drm_object_property_set_value(&set->connectors[i]->base,
> +
> set->connectors[i]->dev->mode_config.dpms_property,
> + DRM_MODE_DPMS_ON);
> }
> }
> __drm_helper_disable_unused_functions(dev);
> --
> 2.17.1
>
--
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: Expose more EDID fields to userspace
On Wed, Jan 16, 2019 at 12:32:58PM -0800, Keith Packard wrote: > Adam Jackson writes: > > > If the kernel wanted to expose its quirks db somehow the library could > > certainly be taught to use it. However there are likely to be quirks > > relevant only to userspace (see below), making the kernel carry that > > doesn't make a ton of sense. > > We do expose some of the quirks to user space, but not as a database, > and not consistently. Some of the quirks just match EDID data to drive > some decision (like non-desktop). Other quirks override some EDID data > that is 'wrong'. For these latter instances, I wonder if we shouldn't be > re-writing the EDID data that user space gets? Or at least making sure > the quirked values are available to user space? > > > Part of the problem with the idea is that EDID parsing is not > > unambiguous. There are several fields for "physical output size" with > > slightly different semantics, which one do you want? There are both > > structured and free-form ways to encode monitor name and serial > > number. > > Hrm. For places where the kernel *does* parse the data, it would sure be > nice to have the kernel version to at least make that > consistent. Model/serial data used to select quirks seems like something > we should be exposing? > > I'm getting the feeling that either extreme approach (parse all of the > EDID data, or parse none of it) is not going to work and that our > current technique of picking some EDID-derived data to expose as > separate parsed values, and some to leave for user space to discover for > itself is where we'll be stuck for at least the near term. > > If we come to agreement that this approach is what we'll be doing, then > I'd like to have a couple of suggestions: > > 1) Document what we've got today Connector properties are documented here: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#standard-connector-properties I think we've caught up and have them all documented now, at least all the ones that have been standardized. There's not much, from a quick look we expose the following immutable props for userspace's benefit: EDID, PATH, TILE (this has been used for other composite screens, not just MST), non_desktop, "panel orientation". Plus vrr_capable, which is documented in a separate chapter. All the other properties are meant to be set, or not about parsing sink information. For all the other bits I think it'd be good to do a better job, but most likely we'll just muddle on. Exhibit A: The super consistent naming scheme for properties we have, see above :-) But in case we're going to do better, fully agree on documenting all this. -Daniel > 2) Document basic guidelines of when to expose parsed EDID-derived data > and when to just leave it in the EDID block for user space > > 3) Plan on exposing all values which the kernel *does* use internally > as parsed-out properties. Especially values which get quirked, > possibly exposing both the "raw" value and the "quirk" value. > > 4) Decide if we want to let user-space in on the quirking fun. I can > imagine a user-space helper that gets run at hot-plug time, reads > the EDID data and then pokes quirk values into the kernel. > > 5) Decide, as a matter of policy, whether the kernel will ever edit > EDID data that gets exposed to user space. I can think of good > reasons on both sides, but I think we need to hash out what the > kernel will do so that user space knows what's going on. > > I think what I want is for kernel and user space to at least be > operating on the same data, even if we end up re-implementing a pile of > code up in user space. > > -- > -keith > ___ > 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 1/6] include/drm: color_mgmt: Add enum labels
Hi,
18. 12. 14. 오후 9:10에 Christoph Manszewski 이(가) 쓴 글:
> Range setting makes sense for YCbCr and RGB buffers. Current
> drm_color_range enum labels suggest use with YCbCr buffers.
> Create enum labels without colorspace specification.
>
> Signed-off-by: Christoph Manszewski
> ---
> include/drm/drm_color_mgmt.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 90ef9996d9a4..52f6d5221a0d 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -62,6 +62,8 @@ enum drm_color_range {
> DRM_COLOR_YCBCR_LIMITED_RANGE,
> DRM_COLOR_YCBCR_FULL_RANGE,
> DRM_COLOR_RANGE_MAX,
> + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE,
> + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE,
I don't see why above two types are needed. Is there any case that the range
checking behavior should be different according to the color space?
And even if there is that case, I think you have to put DRM_COLOR_RANG_MAX
under DRM_COLOR_FULL_RANGE.
Thanks,
Inki Dae
> };
>
> int drm_plane_create_color_properties(struct drm_plane *plane,
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/crtc-helper: Add store the property value
On 1/17/19 6:20 PM, Daniel Vetter wrote:
> On Thu, Jan 17, 2019 at 05:50:44PM +0900, Hoegeun Kwon wrote:
>> There is a problem in crtc_helper that property value is not updated
>> when dpms is turned on or off. So modify the property value when dpms
>> is on.
>>
>> Signed-off-by: Hoegeun Kwon
> This is fixed with atomic, and exynos is atomic. Why do you care about
> this?
> -Daniel
Thank you Daniel.
That's right, there is no problem with exynos because it uses atomic.
But I think it could be a problem with other connectors that do not use
atoms.
Best regards,
Hoegeun
>
>> ---
>> drivers/gpu/drm/drm_crtc_helper.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
>> b/drivers/gpu/drm/drm_crtc_helper.c
>> index a3c81850e755..57d359f0725c 100644
>> --- a/drivers/gpu/drm/drm_crtc_helper.c
>> +++ b/drivers/gpu/drm/drm_crtc_helper.c
>> @@ -735,6 +735,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>> DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS
>> on\n", set->connectors[i]->base.id,
>>set->connectors[i]->name);
>>
>> set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
>> +
>> +
>> drm_object_property_set_value(&set->connectors[i]->base,
>> +
>> set->connectors[i]->dev->mode_config.dpms_property,
>> +DRM_MODE_DPMS_ON);
>> }
>> }
>> __drm_helper_disable_unused_functions(dev);
>> --
>> 2.17.1
>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/cma-helper: Remove unused fbdev code
Den 14.01.2019 13.10, skrev Noralf Trønnes: > CMA helper drivers have been converted to drm_fbdev_generic_setup() > so the fbdev code can be removed. > > v3: Remove CMA specific conditional in the generic fbdev client > > v2: Clean up the includes some more (Laurent) > > Cc: Laurent Pinchart > Signed-off-by: Noralf Trønnes > Acked-by: Sam Ravnborg > Reviewed-by: Laurent Pinchart > --- Applied to drm-misc-next. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/5] drm/tinydrm: Use damage helper for dirtyfb
Den 15.01.2019 05.36, skrev Noralf Trønnes: > David discovered a bug which gave memory corruption because the tx_buf > was written past its end for st7586 which has a smaller tx_buf. The > problem was that mipi_dbi_enable_flush() now calls directly into > mipi_dbi_fb_dirty() instead of going via the tindyrm_device dirtyfb > callback. > > Changes: > - Include vblank header (Sam) > - ili9225 and st7586 can't use mipi_dbi_enable_flush() (David) > > Noralf. > > Noralf Trønnes (5): > drm/gem-fb-helper: Add drm_gem_fb_create_with_dirty() > drm/damage-helper: Add drm_atomic_helper_damage_merged() > drm/tinydrm: Use struct drm_rect > drm/tinydrm: Use damage helper for dirtyfb > drm/todo: Tick off some tinydrm entries > Thanks for reviews and testing, applied to drm-misc-next. Noralf. > Documentation/gpu/todo.rst| 35 > drivers/gpu/drm/drm_damage_helper.c | 41 + > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 50 +- > drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 21 +-- > .../gpu/drm/tinydrm/core/tinydrm-helpers.c| 100 +--- > drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 31 > drivers/gpu/drm/tinydrm/hx8357d.c | 2 +- > drivers/gpu/drm/tinydrm/ili9225.c | 149 +++--- > drivers/gpu/drm/tinydrm/ili9341.c | 2 +- > drivers/gpu/drm/tinydrm/mi0283qt.c| 2 +- > drivers/gpu/drm/tinydrm/mipi-dbi.c| 94 +++ > drivers/gpu/drm/tinydrm/repaper.c | 43 ++--- > drivers/gpu/drm/tinydrm/st7586.c | 86 ++ > drivers/gpu/drm/tinydrm/st7735r.c | 2 +- > include/drm/drm_damage_helper.h | 3 + > include/drm/drm_gem_framebuffer_helper.h | 3 + > include/drm/tinydrm/mipi-dbi.h| 5 +- > include/drm/tinydrm/tinydrm-helpers.h | 20 +-- > include/drm/tinydrm/tinydrm.h | 26 --- > 19 files changed, 309 insertions(+), 406 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/6] arm64: dts: renesas: r8a77990: ebisu: Enable LVDS1 encoder
Hi Laurent,
On Thu, Jan 17, 2019 at 3:07 AM Laurent Pinchart
wrote:
> The LVDS1 encoder must supply a pixel clock to the DU for the DPAD
> output when the LVDS0 encoder is used. Enable it despite its output not
> being connected.
>
> Signed-off-by: Laurent Pinchart
Thanks for your patch!
> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> @@ -443,6 +443,8 @@
> };
>
> &lvds1 {
Would it make sense to add a comment
/* Must be enabled to supply a pixel clock to the DU for the DPAD
output when lvds0 is used */
also for D3?
> + status = "okay";
> +
> clocks = <&cpg CPG_MOD 727>,
> <&x13_clk>,
> <&extal_clk>;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-fixes
Hi Dave and Daniel - Just gvt this week, quoting Zhenyu: > This contains one cmd parser failure fix to allow cmd access for one > register, and fix region cleanup properly in vGPU destroy, and another > fix for critical mmap size check mistake. I didn't wait for CI results after pushing, because I don't think we have much in terms of gvt testing in igt anyway. drm-intel-fixes-2019-01-17: drm/i915/gvt fixes for v5.0-rc3 BR, Jani. The following changes since commit 1c7fc5cbc33980acd13d668f1c8f0313d6ae9fd8: Linux 5.0-rc2 (2019-01-14 10:41:12 +1200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2019-01-17 for you to fetch changes up to 15c05196ff84ab07d9b1d05289216de2d5bedcd7: Merge tag 'gvt-fixes-2018-01-17' of https://github.com/intel/gvt-linux into drm-intel-fixes (2019-01-17 11:49:51 +0200) drm/i915/gvt fixes for v5.0-rc3 Colin Xu (1): drm/i915/gvt: Allow F_CMD_ACCESS on mmio 0x21f0 Hang Yuan (1): drm/i915/gvt: free VFIO region space in vgpu detach Jani Nikula (1): Merge tag 'gvt-fixes-2018-01-17' of https://github.com/intel/gvt-linux into drm-intel-fixes Zhenyu Wang (1): drm/i915/gvt: Fix mmap range check drivers/gpu/drm/i915/gvt/handlers.c | 1 + drivers/gpu/drm/i915/gvt/hypercall.h | 2 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 30 ++ drivers/gpu/drm/i915/gvt/mpt.h | 2 +- 4 files changed, 29 insertions(+), 6 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Thu, 2019-01-17 at 10:30 +0100, h...@lst.de wrote: > On Wed, Jan 16, 2019 at 10:24:36AM -0700, Jason Gunthorpe wrote: > > The fact is there is 0 industry interest in using RDMA on platforms > > that can't do HW DMA cache coherency - the kernel syscalls required > > to > > do the cache flushing on the IO path would just destroy performance > > to > > the point of making RDMA pointless. Better to use netdev on those > > platforms. > > In general there is no syscall required for doing cache flushing, you > just issue the proper instructions directly from userspace. But what if there are other coherence issues? Like bounce-buffers? I'd like to +1 on what Jason says about industry interest: FWIW, vmwgfx is probably one of the graphics drivers that would lend itself best to do a fully-dma-interface compliant graphics stack experiment. But being a paravirtual driver, all platforms we can ever run on are fully coherent unless someone introduces a fake incoherency by forcing swiotlb. Putting many man-months of effort into supporting systems on which we would never run on and can never test on can never make more than academic sense. > > > > The reality is that *all* the subsytems doing DMA kernel bypass are > > ignoring the DMA mapping rules, I think we should support this > > better, > > and just accept that user space DMA will not be using syncing. > > Block > > access in cases when this is required, otherwise let it work as is > > today. > > In that case we just need to block userspace DMA access entirely. > Which given the amount of problems it creates sounds like a pretty > good idea anyway. I'm not sure I'm following you here. Are you suggesting scratching support for all (GP)GPU- and RDMA drivers? Thanks, Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm: arm/komeda: Remove IRQ parsing from initial series
On Tue, Jan 15, 2019 at 10:15:44AM +, Liviu Dudau wrote:
> The initial series is only introducing the basic components and not
> implementing IRQ handling. Remove the left over code that touches
> IRQs until the proper implementation is introduced in a later series.
>
> Signed-off-by: Liviu Dudau
Reviewed-by: James Qian Wang (Arm Technology China)
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 5 -
> drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 3 ---
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 +---
> 3 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index 84fdf707f2107..16f4e72abc1a3 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -59,11 +59,6 @@ static int komeda_parse_dt(struct device *dev, struct
> komeda_dev *mdev)
> return PTR_ERR(clk);
>
> mdev->mclk = clk;
> -mdev->irq = platform_get_irq(pdev, 0);
> -if (mdev->irq < 0) {
> -DRM_ERROR("could not get IRQ number.\n");
> -return mdev->irq;
> -}
>
> for_each_available_child_of_node(np, child) {
> if (of_node_cmp(child->name, "pipeline") == 0) {
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index a0bf7050037a0..0f77dead6a237 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -81,9 +81,6 @@ struct komeda_dev {
> /** @mck: HW main engine clk */
> struct clk *mclk;
>
> -/** @irq: irq number */
> -u32 irq;
> -
> int n_pipelines;
> struct komeda_pipeline *pipelines[KOMEDA_MAX_PIPELINES];
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index f41b20235130b..3fc096d3883e8 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -142,12 +142,10 @@ struct komeda_kms_dev *komeda_kms_attach(struct
> komeda_dev *mdev)
>
> err = drm_dev_register(drm, 0);
> if (err)
> -goto uninstall_irq;
> +goto cleanup_mode_config;
>
> return kms;
>
> -uninstall_irq:
> -drm_irq_uninstall(drm);
> cleanup_mode_config:
> drm_mode_config_cleanup(drm);
> free_kms:
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop
On Tue, Dec 18, 2018 at 04:37:26PM +0100, Lubomir Rintel wrote: > here are patches that make the Armada DRM work on an OLPC XO-1.75. > They build on patches previously submitted by Russel King (included here for > completeness as well). Hi, Would it be possible for you to re-post patches 1 through 6 to include the DT maintainers so that they can review those patches (as is required for their acceptance) - once they have done that, I can pick them up and resubmit some of the patches I have queued. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory
On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote: > Compared to the RFC[1] no changes to the patch itself, but igt moved > forward a lot: > > - gitlab CI builds with: reduced configs/libraries, arm cross build > and a sysroot build (should address all the build/cross platform > concerns raised in the RFC discussions). > > - tests reorganized into subdirectories so that the i915-gem tests > don't clog the main/shared tests directory anymore > > - quite a few more non-intel people contributing/reviewing/committing > igt tests patches. > > I think this addresses all the concerns raised in the RFC discussions, > and assuming there's enough Acks and no new issues that pop up, we can > go ahead with this. > > 1: https://patchwork.kernel.org/patch/10648851/ > Cc: Petri Latvala > Cc: Arkadiusz Hiler > Cc: Liviu Dudau > Cc: Sean Paul > Cc: Eric Anholt > Cc: Alex Deucher > Cc: Dave Airlie > Signed-off-by: Daniel Vetter Thanks for the trust! I hope we will serve the subsystem well. I am happy about the influx of people from outside of i915 that we have got in 2018 and I hope it will only get better this year. There are still some cleanup tasks left when it comes driver-specific test compartmentalization, but we are getting there :-) Acked-by: Arkadiusz Hiler -- Cheers, Arek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm: arm/komeda: Remove IRQ parsing from initial series
On Tue, Jan 15, 2019 at 10:15:44AM +, Liviu Dudau wrote:
> The initial series is only introducing the basic components and not
> implementing IRQ handling. Remove the left over code that touches
> IRQs until the proper implementation is introduced in a later series.
>
> Signed-off-by: Liviu Dudau
Reviewed-by: James Qian Wang (Arm Technology China)
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 5 -
> drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 3 ---
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 +---
> 3 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index 84fdf707f2107..16f4e72abc1a3 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -59,11 +59,6 @@ static int komeda_parse_dt(struct device *dev, struct
> komeda_dev *mdev)
> return PTR_ERR(clk);
>
> mdev->mclk = clk;
> -mdev->irq = platform_get_irq(pdev, 0);
> -if (mdev->irq < 0) {
> -DRM_ERROR("could not get IRQ number.\n");
> -return mdev->irq;
> -}
>
> for_each_available_child_of_node(np, child) {
> if (of_node_cmp(child->name, "pipeline") == 0) {
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index a0bf7050037a0..0f77dead6a237 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -81,9 +81,6 @@ struct komeda_dev {
> /** @mck: HW main engine clk */
> struct clk *mclk;
>
> -/** @irq: irq number */
> -u32 irq;
> -
> int n_pipelines;
> struct komeda_pipeline *pipelines[KOMEDA_MAX_PIPELINES];
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index f41b20235130b..3fc096d3883e8 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -142,12 +142,10 @@ struct komeda_kms_dev *komeda_kms_attach(struct
> komeda_dev *mdev)
>
> err = drm_dev_register(drm, 0);
> if (err)
> -goto uninstall_irq;
> +goto cleanup_mode_config;
>
> return kms;
>
> -uninstall_irq:
> -drm_irq_uninstall(drm);
> cleanup_mode_config:
> drm_mode_config_cleanup(drm);
> free_kms:
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-fixes
drm-misc-fixes-2019-01-17: drm-misc-fixes for v5.0-rc3: - Add missing calls to of_node_put to sun4i, meson, and rockchip. - Drop unimplemented prime callbacks in virtio and qxl, so support for prime is not advertised on those drivers. - Fix mode switching regression in meson. The following changes since commit 4089e272ac61603931beb024d4d640de2cb390e0: gpu/drm: Fix lock held when returning to user space. (2019-01-10 11:31:58 +0100) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2019-01-17 for you to fetch changes up to 4bb0e6d7258213d4893c2c876712fbba40e712fe: drm/sun4i: backend: add missing of_node_puts (2019-01-15 21:46:10 +0100) drm-misc-fixes for v5.0-rc3: - Add missing calls to of_node_put to sun4i, meson, and rockchip. - Drop unimplemented prime callbacks in virtio and qxl, so support for prime is not advertised on those drivers. - Fix mode switching regression in meson. Gerd Hoffmann (2): drm/qxl: drop prime import/export callbacks drm/virtio: drop prime import/export callbacks Julia Lawall (3): drm/rockchip: add missing of_node_put drm/meson: add missing of_node_put drm/sun4i: backend: add missing of_node_puts Neil Armstrong (1): drm/meson: Fix atomic mode switching regression drivers/gpu/drm/meson/meson_crtc.c | 23 ++- drivers/gpu/drm/meson/meson_drv.c | 14 -- drivers/gpu/drm/qxl/qxl_drv.c | 4 drivers/gpu/drm/qxl/qxl_prime.c | 14 -- drivers/gpu/drm/rockchip/rockchip_rgb.c | 4 +++- drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +++-- drivers/gpu/drm/virtio/virtgpu_drv.c| 4 drivers/gpu/drm/virtio/virtgpu_drv.h| 4 drivers/gpu/drm/virtio/virtgpu_prime.c | 14 -- 9 files changed, 20 insertions(+), 66 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory
On Thu, Jan 17, 2019 at 01:01:00PM +0200, Arkadiusz Hiler wrote: > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote: > > Compared to the RFC[1] no changes to the patch itself, but igt moved > > forward a lot: > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build > > and a sysroot build (should address all the build/cross platform > > concerns raised in the RFC discussions). > > > > - tests reorganized into subdirectories so that the i915-gem tests > > don't clog the main/shared tests directory anymore > > > > - quite a few more non-intel people contributing/reviewing/committing > > igt tests patches. > > > > I think this addresses all the concerns raised in the RFC discussions, > > and assuming there's enough Acks and no new issues that pop up, we can > > go ahead with this. > > > > 1: https://patchwork.kernel.org/patch/10648851/ > > Cc: Petri Latvala > > Cc: Arkadiusz Hiler > > Cc: Liviu Dudau > > Cc: Sean Paul > > Cc: Eric Anholt > > Cc: Alex Deucher > > Cc: Dave Airlie > > Signed-off-by: Daniel Vetter > > Thanks for the trust! I hope we will serve the subsystem well. > > I am happy about the influx of people from outside of i915 that we have > got in 2018 and I hope it will only get better this year. > > There are still some cleanup tasks left when it comes driver-specific > test compartmentalization, but we are getting there :-) > > Acked-by: Arkadiusz Hiler +1 Acked-by: Petri Latvala ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 14/16] drm/armada: optionally enable the AXI clock
On Tue, Dec 18, 2018 at 04:37:40PM +0100, Lubomir Rintel wrote:
> It needs to be enabled (at least on MMP2) in order for the register
> writes to LCDC to work.
Dove also has an AXI clock input to the LCDC, but it isn't clear
whether that is necessary for register access or not. From a quick
search of the documentation, there doesn't appear to be an enable bit
for the AXI clock. Unfortunately, the documentation of clocks on Dove
is not very good.
However, Dove LCDCs can have four clock inputs for the pixel clock -
AXI, PLL and two external clock inputs. It isn't clear what this AXI
clock is, and whether it's the same clock used for register access.
Can you check whether the AXI clock used for the pixel clock is the
same as the AXI bus clock for MMP2 and document that please?
Thanks.
>
> Signed-off-by: Lubomir Rintel
> ---
> drivers/gpu/drm/armada/armada_crtc.c | 7 +++
> drivers/gpu/drm/armada/armada_crtc.h | 1 +
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c
> b/drivers/gpu/drm/armada/armada_crtc.c
> index 5400fb925bcd..973c377975a1 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.c
> +++ b/drivers/gpu/drm/armada/armada_crtc.c
> @@ -679,6 +679,7 @@ static void armada_drm_crtc_destroy(struct drm_crtc *crtc)
>
> of_node_put(dcrtc->crtc.port);
>
> + clk_disable_unprepare(dcrtc->axiclk);
> kfree(dcrtc);
> }
>
> @@ -748,6 +749,11 @@ static int armada_drm_crtc_create(struct drm_device
> *drm, struct device *dev,
> dcrtc->clk = ERR_PTR(-EINVAL);
> dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
>
> + dcrtc->axiclk = devm_clk_get(dev, "axiclk");
> + if (IS_ERR(dcrtc->axiclk))
> + dcrtc->axiclk = NULL;
> + WARN_ON(clk_prepare_enable(dcrtc->axiclk));
> +
> endpoint = of_get_next_child(port, NULL);
> of_property_read_u32(endpoint, "bus-width", &bus_width);
> of_node_put(endpoint);
> @@ -829,6 +835,7 @@ static int armada_drm_crtc_create(struct drm_device *drm,
> struct device *dev,
> err_crtc_init:
> primary->funcs->destroy(primary);
> err_crtc:
> + clk_disable_unprepare(dcrtc->axiclk);
> kfree(dcrtc);
>
> return ret;
> diff --git a/drivers/gpu/drm/armada/armada_crtc.h
> b/drivers/gpu/drm/armada/armada_crtc.h
> index 7ebd337b60af..b07faea7257d 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.h
> +++ b/drivers/gpu/drm/armada/armada_crtc.h
> @@ -39,6 +39,7 @@ struct armada_crtc {
> const struct armada_variant *variant;
> unsignednum;
> void __iomem*base;
> + struct clk *axiclk;
> struct clk *clk;
> struct clk *extclk[2];
> struct {
> --
> 2.19.1
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm: arm/komeda: Remove IRQ parsing from initial series
On Tue, Jan 15, 2019 at 10:15:44AM +, Liviu Dudau wrote:
> The initial series is only introducing the basic components and not
> implementing IRQ handling. Remove the left over code that touches
> IRQs until the proper implementation is introduced in a later series.
>
> Signed-off-by: Liviu Dudau
Reviewed-by: James Qian Wang (Arm Technology China)
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 5 -
> drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 3 ---
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 +---
> 3 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index 84fdf707f2107..16f4e72abc1a3 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -59,11 +59,6 @@ static int komeda_parse_dt(struct device *dev, struct
> komeda_dev *mdev)
> return PTR_ERR(clk);
>
> mdev->mclk = clk;
> - mdev->irq = platform_get_irq(pdev, 0);
> - if (mdev->irq < 0) {
> - DRM_ERROR("could not get IRQ number.\n");
> - return mdev->irq;
> - }
>
> for_each_available_child_of_node(np, child) {
> if (of_node_cmp(child->name, "pipeline") == 0) {
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index a0bf7050037a0..0f77dead6a237 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -81,9 +81,6 @@ struct komeda_dev {
> /** @mck: HW main engine clk */
> struct clk *mclk;
>
> - /** @irq: irq number */
> - u32 irq;
> -
> int n_pipelines;
> struct komeda_pipeline *pipelines[KOMEDA_MAX_PIPELINES];
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index f41b20235130b..3fc096d3883e8 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -142,12 +142,10 @@ struct komeda_kms_dev *komeda_kms_attach(struct
> komeda_dev *mdev)
>
> err = drm_dev_register(drm, 0);
> if (err)
> - goto uninstall_irq;
> + goto cleanup_mode_config;
>
> return kms;
>
> -uninstall_irq:
> - drm_irq_uninstall(drm);
> cleanup_mode_config:
> drm_mode_config_cleanup(drm);
> free_kms:
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: support gpu aliases defined in DT data
The DRM device minor numbers are allocated according to the registration
order. This causes confusion in cases where the registration order can
change, or when, say, a modesetting capable device is preferred to be
card0, and a rendering device is preferred to be card1.
This patch adds similar functionality that is used in some other
subsystems, where device minor numbers can be defined in DT bindings'
aliases node.
For example, this sets the DRM device minor number to 1 for the 'dss'
device.
aliases {
gpu1 = &dss;
};
The logic on how to pick the minor number is:
- if there's a DT gpu alias for the device, use that
- else, if there are any gpu aliases, pick a minor number that is higher
than any of the aliases.
- else, use the full range of possible numbers
Signed-off-by: Tomi Valkeinen
---
drivers/gpu/drm/drm_drv.c | 31 +--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index a5fe91b8c3c9..f536a2760293 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -110,6 +110,8 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned
int type)
struct drm_minor *minor;
unsigned long flags;
int r;
+ int min_id, max_id;
+ bool of_id_found = false;
minor = kzalloc(sizeof(*minor), GFP_KERNEL);
if (!minor)
@@ -118,12 +120,37 @@ static int drm_minor_alloc(struct drm_device *dev,
unsigned int type)
minor->type = type;
minor->dev = dev;
+ min_id = 64 * type;
+ max_id = 64 * (type + 1);
+
+ if (dev->dev && dev->dev->of_node) {
+ int id;
+
+ id = of_alias_get_id(dev->dev->of_node, "gpu");
+
+ if (id >= 0) {
+ min_id = 64 * type + id;
+ max_id = 64 * type + id + 1;
+
+ of_id_found = true;
+ }
+ }
+
+ if (!of_id_found) {
+ int id;
+
+ id = of_alias_get_highest_id("gpu");
+
+ if (id >= 0)
+ min_id = id + 1;
+ }
+
idr_preload(GFP_KERNEL);
spin_lock_irqsave(&drm_minor_lock, flags);
r = idr_alloc(&drm_minors_idr,
NULL,
- 64 * type,
- 64 * (type + 1),
+ min_id,
+ max_id,
GFP_NOWAIT);
spin_unlock_irqrestore(&drm_minor_lock, flags);
idr_preload_end();
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory
On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote: > Compared to the RFC[1] no changes to the patch itself, but igt moved > forward a lot: > > - gitlab CI builds with: reduced configs/libraries, arm cross build > and a sysroot build (should address all the build/cross platform > concerns raised in the RFC discussions). > > - tests reorganized into subdirectories so that the i915-gem tests > don't clog the main/shared tests directory anymore > > - quite a few more non-intel people contributing/reviewing/committing > igt tests patches. > > I think this addresses all the concerns raised in the RFC discussions, > and assuming there's enough Acks and no new issues that pop up, we can > go ahead with this. > > 1: https://patchwork.kernel.org/patch/10648851/ > Cc: Petri Latvala > Cc: Arkadiusz Hiler > Cc: Liviu Dudau > Cc: Sean Paul > Cc: Eric Anholt > Cc: Alex Deucher > Cc: Dave Airlie > Signed-off-by: Daniel Vetter > --- > Documentation/gpu/drm-uapi.rst | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index a752aa561ea4..413915d6b7d2 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the slightly > unintuitive meaning of > Testing and validation > == > > +Testing Requirements for userspace API > +-- > + > +New cross-driver userspace interface extensions, like new IOCTL, new KMS > +properties, new files in sysfs or anything else that constitutes an API > change > +need to have driver-agnostic testcases in IGT for that feature. From an aspirational point of view I am fine with this and you can have my Acked-by: Liviu Dudau . From a practical point of view I would like to see a matrix of KMS APIs that are being validated and the drivers that have been tested. Otherwise, the next person that comes and tries to add a new IOCTL, KMS property or new file in sysfs is going to discover that he has subscribed to a much bigger task of getting enough KMS drivers testable in the first place. Best regards, Liviu > + > Validating changes with IGT > --- > > -- > 2.20.1 > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory
On Wed, Jan 16, 2019 at 11:41 PM Eric Anholt wrote: > > Daniel Vetter writes: > > > Compared to the RFC[1] no changes to the patch itself, but igt moved > > forward a lot: > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build > > and a sysroot build (should address all the build/cross platform > > concerns raised in the RFC discussions). > > > > - tests reorganized into subdirectories so that the i915-gem tests > > don't clog the main/shared tests directory anymore > > > > - quite a few more non-intel people contributing/reviewing/committing > > igt tests patches. > > > > I think this addresses all the concerns raised in the RFC discussions, > > and assuming there's enough Acks and no new issues that pop up, we can > > go ahead with this. > > > > 1: https://patchwork.kernel.org/patch/10648851/ > > Cc: Petri Latvala > > Cc: Arkadiusz Hiler > > Cc: Liviu Dudau > > Cc: Sean Paul > > Cc: Eric Anholt > > Cc: Alex Deucher > > Cc: Dave Airlie > > Signed-off-by: Daniel Vetter > > igt is a bit awkward to work in (the mailing list is very noisy with the > Intel CI being email-based instead of gitlab-based and most of the > traffic being Intel), but it's the right place to be putting shared > tests and hopefully that pain point goes away eventually using gitlab > MRs. I have a filter to mark all CI mails as read, to avoid the spam a bit. And then check it only when I merge a series. But yeah, it's pain. > I think there are going to be some interesting questions on how to deal > with things like KMS properties that aren't amenable to > chamelium/writeback-based testing. However, we should default to > requiring tests and only skip that when we agree collectively that > something isn't testable currently. Should I add a sentence clarifying this? We don't want to block features on a new unproven/unwritten testing approach (e.g. "write an entire vkms to test-drive that feature"), that doesn't make sense. -Daniel > > Reviewed-by: Eric Anholt -- 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/doc: Make igts for cross-driver stuff mandatory
On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau wrote: > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote: > > Compared to the RFC[1] no changes to the patch itself, but igt moved > > forward a lot: > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build > > and a sysroot build (should address all the build/cross platform > > concerns raised in the RFC discussions). > > > > - tests reorganized into subdirectories so that the i915-gem tests > > don't clog the main/shared tests directory anymore > > > > - quite a few more non-intel people contributing/reviewing/committing > > igt tests patches. > > > > I think this addresses all the concerns raised in the RFC discussions, > > and assuming there's enough Acks and no new issues that pop up, we can > > go ahead with this. > > > > 1: https://patchwork.kernel.org/patch/10648851/ > > Cc: Petri Latvala > > Cc: Arkadiusz Hiler > > Cc: Liviu Dudau > > Cc: Sean Paul > > Cc: Eric Anholt > > Cc: Alex Deucher > > Cc: Dave Airlie > > Signed-off-by: Daniel Vetter > > --- > > Documentation/gpu/drm-uapi.rst | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > > index a752aa561ea4..413915d6b7d2 100644 > > --- a/Documentation/gpu/drm-uapi.rst > > +++ b/Documentation/gpu/drm-uapi.rst > > @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the > > slightly unintuitive meaning of > > Testing and validation > > == > > > > +Testing Requirements for userspace API > > +-- > > + > > +New cross-driver userspace interface extensions, like new IOCTL, new KMS > > +properties, new files in sysfs or anything else that constitutes an API > > change > > +need to have driver-agnostic testcases in IGT for that feature. > > From an aspirational point of view I am fine with this and you can have > my Acked-by: Liviu Dudau . > > From a practical point of view I would like to see a matrix of KMS APIs > that are being validated and the drivers that have been tested. Otherwise, > the next person that comes and tries to add a new IOCTL, KMS property or new > file in sysfs is going to discover that he has subscribed to a much bigger > task of getting enough KMS drivers testable in the first place. This is what the _new_ features is about, no expectation to write tests for all the existing stuff. Although I think there's not really any big gaps in igt anymore, we do have at least some (rather rough and coarse in some case) test coverage for everything I think. Should this be clarified further? -Daniel > > Best regards, > Liviu > > > > + > > Validating changes with IGT > > --- > > > > -- > > 2.20.1 > > > > -- > > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --- > ¯\_(ツ)_/¯ > ___ > 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/crtc-helper: Add store the property value
On Thu, Jan 17, 2019 at 10:57 AM Hoegeun Kwon wrote:
>
>
> On 1/17/19 6:20 PM, Daniel Vetter wrote:
> > On Thu, Jan 17, 2019 at 05:50:44PM +0900, Hoegeun Kwon wrote:
> >> There is a problem in crtc_helper that property value is not updated
> >> when dpms is turned on or off. So modify the property value when dpms
> >> is on.
> >>
> >> Signed-off-by: Hoegeun Kwon
> > This is fixed with atomic, and exynos is atomic. Why do you care about
> > this?
> > -Daniel
>
>
> Thank you Daniel.
>
> That's right, there is no problem with exynos because it uses atomic.
>
> But I think it could be a problem with other connectors that do not use
> atoms.
Yeah, but not sure we care about those drivers all that much. If
someone does, probably better to convert them to atomic (which is
still happening). We did have the equivalent of your patch in the i915
legacy modeset code, but it was quite tricky to get right. Much easier
with atomic, where properties have the right value by design.
-Daniel
>
> Best regards,
>
> Hoegeun
>
>
> >
> >> ---
> >> drivers/gpu/drm/drm_crtc_helper.c | 4
> >> 1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> >> b/drivers/gpu/drm/drm_crtc_helper.c
> >> index a3c81850e755..57d359f0725c 100644
> >> --- a/drivers/gpu/drm/drm_crtc_helper.c
> >> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> >> @@ -735,6 +735,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set
> >> *set,
> >> DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS
> >> on\n", set->connectors[i]->base.id,
> >>set->connectors[i]->name);
> >>
> >> set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
> >> +
> >> +
> >> drm_object_property_set_value(&set->connectors[i]->base,
> >> +
> >> set->connectors[i]->dev->mode_config.dpms_property,
> >> +DRM_MODE_DPMS_ON);
> >> }
> >> }
> >> __drm_helper_disable_unused_functions(dev);
> >> --
> >> 2.17.1
> >>
--
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
[Bug 108940] QHD bug? drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:1613 core_link_enable_stream+0xc14/0x1040
https://bugs.freedesktop.org/show_bug.cgi?id=108940 H.Habighorst changed: What|Removed |Added Attachment #143140|0 |1 is obsolete|| --- Comment #15 from H.Habighorst --- Created attachment 143145 --> https://bugs.freedesktop.org/attachment.cgi?id=143145&action=edit dmesg 4.20.2 - drm.debug=0x06 I've forgotten to increase log size and add drm.debug=0x06 to the kernel line. -- 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: Armada DRM: bridge with componentized devices
On Wed, Jan 16, 2019 at 11:44 PM Rafael J. Wysocki wrote: > > On Wednesday, January 16, 2019 7:42:45 PM CET Daniel Vetter wrote: > > On Tue, Jan 15, 2019 at 11:47 PM Rafael J. Wysocki > > wrote: > > > > > > [CC Greg] > > > > > > On Tuesday, January 15, 2019 1:04:02 AM CET Rafael J. Wysocki wrote: > > > > [CC Lukas and linux-pm] > > > > > > > > On Mon, Jan 14, 2019 at 1:32 PM Rafael J. Wysocki > > > > wrote: > > > > > > > > > > On Fri, Jan 11, 2019 at 3:49 PM Russell King - ARM Linux > > > > > wrote: > > > > > > > > [cut] > > > > > > > > > > > > > > > > This thread is discussing how to deal with Armada DRM, its use of > > > > > > the > > > > > > component helper, TDA998x's hybrid use of the component helper and > > > > > > DRM bridges. > > > > > > > > > > > > Currently, DRM bridges register into the DRM system and are added to > > > > > > a global list of bridges. When a DRM driver binds, it looks up the > > > > > > DRM bridge and attaches to it. There is no way in generic DRM code > > > > > > for the DRM driver to know if the DRM bridge is unbound from DRM, > > > > > > consequently a DRM driver may continue trying to call functions in > > > > > > the DRM bridge driver using memory that has already been freed. > > > > > > > > > > > > We had similar issues with imx-drm, and a number of years ago at a > > > > > > kernel summit, this was discussed, and I proposed a system which is > > > > > > now known as the component helper. This handles the problem of a > > > > > > multi-component system. > > > > > > > > > > > > However, DRM bridge was already established, and there appears to be > > > > > > no desire to convert DRM bridges and DRM drivers to use the > > > > > > component > > > > > > helper. > > > > > > > > > > > > We are presently in the situation where Armada DRM is incompatible > > > > > > with the DRM bridges way of doing things, and making it compatible > > > > > > essentially means introducing potential use-after-free bugs into the > > > > > > code. > > > > > > > > > > > > Device links in their stateful form appear to provide an alternative > > > > > > to the component helper, in that a stateful device link will remove > > > > > > consumers of a resource when the supplier is going away - which is > > > > > > exactly the problem which the component helper is solving. The > > > > > > difference is that device links look like being a cleaner solution. > > > > > > > > > > > > Just like the component helper, a stateful link would unbind the > > > > > > consumer of a resource when the supplier goes away - which is > > > > > > exactly > > > > > > the behaviour we're wanting. > > > > > > > > > > > > The problem is that the connection between various drivers is only > > > > > > really known when the drivers obtain their resources, and the > > > > > > following can happen: > > > > > > > > > > > > supplierconsumer > > > > > > > > > > > > probe() > > > > > > alloc > > > > > > probe() > > > > > > publish > > > > > > obtain supplier's resource > > > > > > return > > > > > > > > > > > > Where things go wrong is if a stateful link is created when the > > > > > > consumer obtains the suppliers resource before the supplier has > > > > > > finished probing - which from what's been said is illegal. > > > > > > > > > > It just doesn't work (which means "invalid" rather than "illegal" I > > > > > guess, but whatever :-)). > > > > > > > > > > Admittedly, the original design didn't take this particular case into > > > > > account and I'm not actually sure how it can be taken into account > > > > > either. > > > > > > > > > > If the link is created by the consumer in the scenario above, its > > > > > status will be updated twice in a row after the consumer probe returns > > > > > and after the supplier probe returns. It looks like this update would > > > > > need to work regardless of the order it which this happened which > > > > > sounds somewhat challenging. I would need to think about that. > > > > > > > > So if I'm not mistaken it can be made work with the help of the > > > > (completely untested) attached patch (of course, the documentation > > > > would need to be updated too). > > > > > > > > The key observation here is that it should be fine to create a link > > > > from the consumer driver's probe while the supplier is still probing > > > > if the consumer has some way to find out that the supplier is > > > > functional at that point (like when it has published itself already in > > > > your example above). The initial state of the link can be "consumer > > > > probe" in that case and I don't see a reason why that might not work. > > > > > > > > > > Below is a more complete patch that should take all of the corner cases > > > into account unless I have missed anything. Testing it would be > > > appreciated. :-) > > > > > > --- > > > From: Rafael J. Wysocki > > > Subject: [PATCH] driver core: Fix adding device links to probing suppli
[Bug 108940] QHD bug? drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:1613 core_link_enable_stream+0xc14/0x1040
https://bugs.freedesktop.org/show_bug.cgi?id=108940 --- Comment #16 from H.Habighorst --- Created attachment 143146 --> https://bugs.freedesktop.org/attachment.cgi?id=143146&action=edit dmesg from ~agd5f/linux, branch drm-next-5.1-wip, drm.debug=0x06, commit e7498c5ed98802940cb21e4fb18c9c440b646e6a It still happens in latest GIT. I've add drm.debug = 0x06 and attached the dmesg. If you need anything, please comment. -- 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/doc: Make igts for cross-driver stuff mandatory
On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote: > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau wrote: > > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote: > > > Compared to the RFC[1] no changes to the patch itself, but igt moved > > > forward a lot: > > > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build > > > and a sysroot build (should address all the build/cross platform > > > concerns raised in the RFC discussions). > > > > > > - tests reorganized into subdirectories so that the i915-gem tests > > > don't clog the main/shared tests directory anymore > > > > > > - quite a few more non-intel people contributing/reviewing/committing > > > igt tests patches. > > > > > > I think this addresses all the concerns raised in the RFC discussions, > > > and assuming there's enough Acks and no new issues that pop up, we can > > > go ahead with this. > > > > > > 1: https://patchwork.kernel.org/patch/10648851/ > > > Cc: Petri Latvala > > > Cc: Arkadiusz Hiler > > > Cc: Liviu Dudau > > > Cc: Sean Paul > > > Cc: Eric Anholt > > > Cc: Alex Deucher > > > Cc: Dave Airlie > > > Signed-off-by: Daniel Vetter > > > --- > > > Documentation/gpu/drm-uapi.rst | 7 +++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/Documentation/gpu/drm-uapi.rst > > > b/Documentation/gpu/drm-uapi.rst > > > index a752aa561ea4..413915d6b7d2 100644 > > > --- a/Documentation/gpu/drm-uapi.rst > > > +++ b/Documentation/gpu/drm-uapi.rst > > > @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the > > > slightly unintuitive meaning of > > > Testing and validation > > > == > > > > > > +Testing Requirements for userspace API > > > +-- > > > + > > > +New cross-driver userspace interface extensions, like new IOCTL, new KMS > > > +properties, new files in sysfs or anything else that constitutes an API > > > change > > > +need to have driver-agnostic testcases in IGT for that feature. > > > > From an aspirational point of view I am fine with this and you can have > > my Acked-by: Liviu Dudau . > > > > From a practical point of view I would like to see a matrix of KMS APIs > > that are being validated and the drivers that have been tested. Otherwise, > > the next person that comes and tries to add a new IOCTL, KMS property or new > > file in sysfs is going to discover that he has subscribed to a much bigger > > task of getting enough KMS drivers testable in the first place. > > This is what the _new_ features is about, no expectation to write > tests for all the existing stuff. Yeah, but if the "tests for all the existing stuff" doesn't exist, your _new_ feature tests are not going to mean much, doesn't it? > Although I think there's not really > any big gaps in igt anymore, we do have at least some (rather rough > and coarse in some case) test coverage for everything I think. I would like to see some proof of that in the form of > Should this be clarified further? an URL that points to a page similar to Mesa's GL supported features would be nice to add here, from my point of view. Best regards, Liviu > -Daniel > > > > > Best regards, > > Liviu > > > > > > > + > > > Validating changes with IGT > > > --- > > > > > > -- > > > 2.20.1 > > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory
On Thu, Jan 17, 2019 at 1:26 PM Liviu Dudau wrote: > > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote: > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau wrote: > > > > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote: > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved > > > > forward a lot: > > > > > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build > > > > and a sysroot build (should address all the build/cross platform > > > > concerns raised in the RFC discussions). > > > > > > > > - tests reorganized into subdirectories so that the i915-gem tests > > > > don't clog the main/shared tests directory anymore > > > > > > > > - quite a few more non-intel people contributing/reviewing/committing > > > > igt tests patches. > > > > > > > > I think this addresses all the concerns raised in the RFC discussions, > > > > and assuming there's enough Acks and no new issues that pop up, we can > > > > go ahead with this. > > > > > > > > 1: https://patchwork.kernel.org/patch/10648851/ > > > > Cc: Petri Latvala > > > > Cc: Arkadiusz Hiler > > > > Cc: Liviu Dudau > > > > Cc: Sean Paul > > > > Cc: Eric Anholt > > > > Cc: Alex Deucher > > > > Cc: Dave Airlie > > > > Signed-off-by: Daniel Vetter > > > > --- > > > > Documentation/gpu/drm-uapi.rst | 7 +++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/Documentation/gpu/drm-uapi.rst > > > > b/Documentation/gpu/drm-uapi.rst > > > > index a752aa561ea4..413915d6b7d2 100644 > > > > --- a/Documentation/gpu/drm-uapi.rst > > > > +++ b/Documentation/gpu/drm-uapi.rst > > > > @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the > > > > slightly unintuitive meaning of > > > > Testing and validation > > > > == > > > > > > > > +Testing Requirements for userspace API > > > > +-- > > > > + > > > > +New cross-driver userspace interface extensions, like new IOCTL, new > > > > KMS > > > > +properties, new files in sysfs or anything else that constitutes an > > > > API change > > > > +need to have driver-agnostic testcases in IGT for that feature. > > > > > > From an aspirational point of view I am fine with this and you can have > > > my Acked-by: Liviu Dudau . > > > > > > From a practical point of view I would like to see a matrix of KMS APIs > > > that are being validated and the drivers that have been tested. Otherwise, > > > the next person that comes and tries to add a new IOCTL, KMS property or > > > new > > > file in sysfs is going to discover that he has subscribed to a much bigger > > > task of getting enough KMS drivers testable in the first place. > > > > This is what the _new_ features is about, no expectation to write > > tests for all the existing stuff. > > Yeah, but if the "tests for all the existing stuff" doesn't exist, your > _new_ feature tests are not going to mean much, doesn't it? Why? There's still good test coverage for the new stuff at least. And eventually someone gets bored and fills in the gaps. This is how we've created all the current igt testcases, so it works - mandatory igt for new features is the rule for anything intel does since well over 5 years. > > Although I think there's not really > > any big gaps in igt anymore, we do have at least some (rather rough > > and coarse in some case) test coverage for everything I think. > > I would like to see some proof of that in the form of > > > > Should this be clarified further? > > an URL that points to a page similar to Mesa's GL supported features > would be nice to add here, from my point of view. There isn't even a list of upstream drm features ... how exactly should that look like? Only thing I can tell you is that we had huge amounts of todo for missing igt tests, and afaik we've worked them all down. We = intel here. It's not perfect, but good enough that we've essentially stopped doing any validation except by running igt. Of course you can always write more tests, same way you can always bikeshed a patch a few more times. Eventually there's dimishing returns. -Daniel > Best regards, > Liviu > > > -Daniel > > > > > > > > Best regards, > > > Liviu > > > > > > > > > > + > > > > Validating changes with IGT > > > > --- > > > > > > > > -- > > > > 2.20.1 > > > > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > -- > > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --- > ¯\_(ツ)_/¯ -- 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: support gpu aliases defined in DT data
On Thu, Jan 17, 2019 at 01:19:18PM +0200, Tomi Valkeinen wrote:
> The DRM device minor numbers are allocated according to the registration
> order. This causes confusion in cases where the registration order can
> change, or when, say, a modesetting capable device is preferred to be
> card0, and a rendering device is preferred to be card1.
>
> This patch adds similar functionality that is used in some other
> subsystems, where device minor numbers can be defined in DT bindings'
> aliases node.
What other subsystem? I thought that minor numbers shouldn't be made uapi,
and that udev or similar is supposed to give you stable names ... Is that
not the case on SoC?
If we go with this I think we need a few acks from soc-tree people that
this is a reasonable idea which should be copied around.
-Daniel
> For example, this sets the DRM device minor number to 1 for the 'dss'
> device.
>
> aliases {
> gpu1 = &dss;
> };
>
> The logic on how to pick the minor number is:
>
> - if there's a DT gpu alias for the device, use that
> - else, if there are any gpu aliases, pick a minor number that is higher
> than any of the aliases.
> - else, use the full range of possible numbers
>
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/drm_drv.c | 31 +--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index a5fe91b8c3c9..f536a2760293 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -110,6 +110,8 @@ static int drm_minor_alloc(struct drm_device *dev,
> unsigned int type)
> struct drm_minor *minor;
> unsigned long flags;
> int r;
> + int min_id, max_id;
> + bool of_id_found = false;
>
> minor = kzalloc(sizeof(*minor), GFP_KERNEL);
> if (!minor)
> @@ -118,12 +120,37 @@ static int drm_minor_alloc(struct drm_device *dev,
> unsigned int type)
> minor->type = type;
> minor->dev = dev;
>
> + min_id = 64 * type;
> + max_id = 64 * (type + 1);
> +
> + if (dev->dev && dev->dev->of_node) {
> + int id;
> +
> + id = of_alias_get_id(dev->dev->of_node, "gpu");
> +
> + if (id >= 0) {
> + min_id = 64 * type + id;
> + max_id = 64 * type + id + 1;
> +
> + of_id_found = true;
> + }
> + }
> +
> + if (!of_id_found) {
> + int id;
> +
> + id = of_alias_get_highest_id("gpu");
> +
> + if (id >= 0)
> + min_id = id + 1;
> + }
> +
> idr_preload(GFP_KERNEL);
> spin_lock_irqsave(&drm_minor_lock, flags);
> r = idr_alloc(&drm_minors_idr,
> NULL,
> - 64 * type,
> - 64 * (type + 1),
> + min_id,
> + max_id,
> GFP_NOWAIT);
> spin_unlock_irqrestore(&drm_minor_lock, flags);
> idr_preload_end();
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
--
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 v4 2/3] locking: Implement an algorithm choice for Wound-Wait mutexes
On Wed, Jan 16, 2019 at 11:49 AM Thomas Hellstrom wrote: > > Hi, > > On Wed, 2019-01-16 at 09:24 -0500, Rob Clark wrote: > > So, I guess this is to do w/ the magic of merge commits, but it looks > > like the hunk changing the crtc_ww_class got lost: > > So what happened here is that this commit changed it to > DEFINE_WD_CLASS > and the following commit changed it back again to > DEFINE_WW_CLASS > so that the algorithm change and only that came with the last commit. > Apparently the history of that line got lost when merging since the > merge didn't change it; git blame doesn't show it changed, but when > looking at the commit history in gitk it's all clear. I guess that's a > feature of git. > > The modeset locks now use true wound-wait rather than wait-die. ahh, ok, that makes sense. Thanks for clearing up my confusion. BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] pwm: Add MediaTek MT8183 display PWM driver support
On Wed, Jan 16, 2019 at 03:52:52PM +0800, Jitao Shi wrote: > Use the mtk_pwm_data struction to define different registers > and add MT8183 specific register operations, such as MT8183 > have commit register, needs to enable double buffer > before writing register, and needs to select commit mode > and use PWM_PERIOD/PWM_HIGH_WIDTH. You forgot to add linux-pwm to the recipents. Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: support gpu aliases defined in DT data
On 17/01/19 14:33, Daniel Vetter wrote: > On Thu, Jan 17, 2019 at 01:19:18PM +0200, Tomi Valkeinen wrote: >> The DRM device minor numbers are allocated according to the registration >> order. This causes confusion in cases where the registration order can >> change, or when, say, a modesetting capable device is preferred to be >> card0, and a rendering device is preferred to be card1. >> >> This patch adds similar functionality that is used in some other >> subsystems, where device minor numbers can be defined in DT bindings' >> aliases node. > > What other subsystem? I thought that minor numbers shouldn't be made uapi, > and that udev or similar is supposed to give you stable names ... Is that > not the case on SoC? I think at least i2c, spi and uart use DT aliases. I also have my doubts about this, but thought to post this to get some comments, as it does make life quite a bit easier =). Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: support gpu aliases defined in DT data
On Thu, Jan 17, 2019 at 2:04 PM Tomi Valkeinen wrote: > > On 17/01/19 14:33, Daniel Vetter wrote: > > On Thu, Jan 17, 2019 at 01:19:18PM +0200, Tomi Valkeinen wrote: > >> The DRM device minor numbers are allocated according to the registration > >> order. This causes confusion in cases where the registration order can > >> change, or when, say, a modesetting capable device is preferred to be > >> card0, and a rendering device is preferred to be card1. > >> > >> This patch adds similar functionality that is used in some other > >> subsystems, where device minor numbers can be defined in DT bindings' > >> aliases node. > > > > What other subsystem? I thought that minor numbers shouldn't be made uapi, > > and that udev or similar is supposed to give you stable names ... Is that > > not the case on SoC? > > I think at least i2c, spi and uart use DT aliases. Commits/code implementing would be best. > I also have my doubts about this, but thought to post this to get some > comments, as it does make life quite a bit easier =). Yeah I think "soc without udev" seems reasonable assumption, I just think this is something the overall soc community should agree on as a good thing to do. I guess since the of stuff you're using is generic that's all happened already, so should amount to gathering a pile of acks and then merging it. -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 v4 6/9] drm/bridge: cdns: Separate DSI and D-PHY configuration
On Wed, Jan 09, 2019 at 10:33:23AM +0100, Maxime Ripard wrote:
> The current configuration of the DSI bridge and its associated D-PHY is
> intertwined. In order to ease the future conversion to the phy framework
> for the D-PHY part, let's split the configuration in two.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/bridge/cdns-dsi.c | 96 ++--
> 1 file changed, 68 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
> b/drivers/gpu/drm/bridge/cdns-dsi.c
> index ce9496d13986..3ac6dd524b6d 100644
> --- a/drivers/gpu/drm/bridge/cdns-dsi.c
> +++ b/drivers/gpu/drm/bridge/cdns-dsi.c
> @@ -545,6 +545,11 @@ bridge_to_cdns_dsi_input(struct drm_bridge *bridge)
> return container_of(bridge, struct cdns_dsi_input, bridge);
> }
>
> +static unsigned int mode_to_dpi_hfp(const struct drm_display_mode *mode)
> +{
> + return mode->crtc_hsync_start - mode->crtc_hdisplay;
> +}
> +
> static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
>struct cdns_dphy_cfg *cfg,
>unsigned int dpi_htotal,
> @@ -731,14 +736,12 @@ static unsigned int dpi_to_dsi_timing(unsigned int
> dpi_timing,
> static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
>const struct drm_display_mode *mode,
>struct cdns_dsi_cfg *dsi_cfg,
> - struct cdns_dphy_cfg *dphy_cfg,
>bool mode_valid_check)
> {
> - unsigned long dsi_htotal = 0, dsi_hss_hsa_hse_hbp = 0;
> struct cdns_dsi_output *output = &dsi->output;
> - unsigned int dsi_hfp_ext = 0, dpi_hfp, tmp;
> + unsigned int tmp;
> bool sync_pulse = false;
> - int bpp, nlanes, ret;
> + int bpp, nlanes;
>
> memset(dsi_cfg, 0, sizeof(*dsi_cfg));
>
> @@ -757,8 +760,6 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
> mode->crtc_hsync_end : mode->crtc_hsync_start);
>
> dsi_cfg->hbp = dpi_to_dsi_timing(tmp, bpp, DSI_HBP_FRAME_OVERHEAD);
> - dsi_htotal += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
> - dsi_hss_hsa_hse_hbp += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
>
> if (sync_pulse) {
> if (mode_valid_check)
> @@ -768,49 +769,90 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
>
> dsi_cfg->hsa = dpi_to_dsi_timing(tmp, bpp,
>DSI_HSA_FRAME_OVERHEAD);
> - dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
> - dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
> }
>
> dsi_cfg->hact = dpi_to_dsi_timing(mode_valid_check ?
> mode->hdisplay : mode->crtc_hdisplay,
> bpp, 0);
> - dsi_htotal += dsi_cfg->hact;
> + dsi_cfg->hfp = dpi_to_dsi_timing(mode_to_dpi_hfp(mode), bpp,
> + DSI_HFP_FRAME_OVERHEAD);
We're throwing away the mode_valid_check switch here to flip between crtc_h*
value and h* value. Is that intentional? We're using it above for hdisplay, so
it's a bit confusing.
>
> - if (mode_valid_check)
> - dpi_hfp = mode->hsync_start - mode->hdisplay;
> - else
> - dpi_hfp = mode->crtc_hsync_start - mode->crtc_hdisplay;
> + return 0;
> +}
> +
> +static int cdns_dphy_validate(struct cdns_dsi *dsi,
> + struct cdns_dsi_cfg *dsi_cfg,
> + struct cdns_dphy_cfg *dphy_cfg,
> + const struct drm_display_mode *mode,
> + bool mode_valid_check)
> +{
> + struct cdns_dsi_output *output = &dsi->output;
> + unsigned long dsi_htotal;
> + unsigned int dsi_hfp_ext = 0;
> +
> + int ret;
>
> - dsi_cfg->hfp = dpi_to_dsi_timing(dpi_hfp, bpp, DSI_HFP_FRAME_OVERHEAD);
> + dsi_htotal = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
> + if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
> + dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
> +
> + dsi_htotal += dsi_cfg->hact;
> dsi_htotal += dsi_cfg->hfp + DSI_HFP_FRAME_OVERHEAD;
>
> if (mode_valid_check)
> ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
> - mode->htotal, bpp,
> + mode->htotal,
> mode->clock * 1000,
> - dsi_htotal, nlanes,
> +
> mipi_dsi_pixel_format_to_bpp(output->dev->format),
> + dsi_htotal,
> + output->dev->lanes,
> &dsi_hfp_ext);
> else
> ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cf
Re: [PATCH v4 8/9] phy: Add Cadence D-PHY support
On Wed, Jan 09, 2019 at 10:33:25AM +0100, Maxime Ripard wrote:
> Cadence has designed a D-PHY that can be used by the, currently in tree,
> DSI bridge (DRM), CSI Transceiver and CSI Receiver (v4l2) drivers.
>
> Only the DSI driver has an ad-hoc driver for that phy at the moment, while
> the v4l2 drivers are completely missing any phy support. In order to make
> that phy support available to all these drivers, without having to
> duplicate that code three times, let's create a generic phy framework
> driver.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/phy/cadence/Kconfig | 13 +-
> drivers/phy/cadence/Makefile| 1 +-
> drivers/phy/cadence/cdns-dphy.c | 389 +-
> 3 files changed, 402 insertions(+), 1 deletion(-)
> create mode 100644 drivers/phy/cadence/cdns-dphy.c
>
> diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
> index 2b8c0851ff33..31f18b67dd7c 100644
> --- a/drivers/phy/cadence/Kconfig
> +++ b/drivers/phy/cadence/Kconfig
> @@ -1,6 +1,7 @@
> #
> # Phy drivers for Cadence PHYs
> #
> +
> config PHY_CADENCE_DP
> tristate "Cadence MHDP DisplayPort PHY driver"
> depends on OF
> @@ -9,9 +10,19 @@ config PHY_CADENCE_DP
> help
> Support for Cadence MHDP DisplayPort PHY.
>
> +config PHY_CADENCE_DPHY
> + tristate "Cadence D-PHY Support"
> + depends on HAS_IOMEM && OF
> + select GENERIC_PHY
> + select GENERIC_PHY_MIPI_DPHY
> + help
> + Choose this option if you have a Cadence D-PHY in your
> + system. If M is selected, the module will be called
> + cdns-dphy.
> +
> config PHY_CADENCE_SIERRA
> tristate "Cadence Sierra PHY Driver"
> depends on OF && HAS_IOMEM && RESET_CONTROLLER
> select GENERIC_PHY
> help
> - Enable this to support the Cadence Sierra PHY driver
> \ No newline at end of file
> + Enable this to support the Cadence Sierra PHY driver
> diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
> index 412349af0492..2f9e3457b954 100644
> --- a/drivers/phy/cadence/Makefile
> +++ b/drivers/phy/cadence/Makefile
> @@ -1,2 +1,3 @@
> obj-$(CONFIG_PHY_CADENCE_DP) += phy-cadence-dp.o
> +obj-$(CONFIG_PHY_CADENCE_DPHY) += cdns-dphy.o
> obj-$(CONFIG_PHY_CADENCE_SIERRA) += phy-cadence-sierra.o
> diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> new file mode 100644
> index ..1d0abba03f37
> --- /dev/null
> +++ b/drivers/phy/cadence/cdns-dphy.c
> @@ -0,0 +1,389 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright: 2017-2018 Cadence Design Systems, Inc.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#define REG_WAKEUP_TIME_NS 800
> +#define DPHY_PLL_RATE_HZ 10800
> +
> +/* DPHY registers */
> +#define DPHY_PMA_CMN(reg)(reg)
> +#define DPHY_PMA_LCLK(reg) (0x100 + (reg))
> +#define DPHY_PMA_LDATA(lane, reg)(0x200 + ((lane) * 0x100) + (reg))
> +#define DPHY_PMA_RCLK(reg) (0x600 + (reg))
> +#define DPHY_PMA_RDATA(lane, reg)(0x700 + ((lane) * 0x100) + (reg))
> +#define DPHY_PCS(reg)(0xb00 + (reg))
> +
> +#define DPHY_CMN_SSM DPHY_PMA_CMN(0x20)
> +#define DPHY_CMN_SSM_EN BIT(0)
> +#define DPHY_CMN_TX_MODE_EN BIT(9)
> +
> +#define DPHY_CMN_PWM DPHY_PMA_CMN(0x40)
> +#define DPHY_CMN_PWM_DIV(x) ((x) << 20)
> +#define DPHY_CMN_PWM_LOW(x) ((x) << 10)
> +#define DPHY_CMN_PWM_HIGH(x) (x)
> +
> +#define DPHY_CMN_FBDIV DPHY_PMA_CMN(0x4c)
> +#define DPHY_CMN_FBDIV_VAL(low, high)(((high) << 11) | ((low) << 22))
> +#define DPHY_CMN_FBDIV_FROM_REG (BIT(10) | BIT(21))
> +
> +#define DPHY_CMN_OPIPDIV DPHY_PMA_CMN(0x50)
> +#define DPHY_CMN_IPDIV_FROM_REG BIT(0)
> +#define DPHY_CMN_IPDIV(x)((x) << 1)
> +#define DPHY_CMN_OPDIV_FROM_REG BIT(6)
> +#define DPHY_CMN_OPDIV(x)((x) << 7)
> +
> +#define DPHY_PSM_CFG DPHY_PCS(0x4)
> +#define DPHY_PSM_CFG_FROM_REGBIT(0)
> +#define DPHY_PSM_CLK_DIV(x) ((x) << 1)
> +
> +#define DSI_HBP_FRAME_OVERHEAD 12
> +#define DSI_HSA_FRAME_OVERHEAD 14
> +#define DSI_HFP_FRAME_OVERHEAD 6
> +#define DSI_HSS_VSS_VSE_FRAME_OVERHEAD 4
> +#define DSI_BLANKING_FRAME_OVERHEAD 6
> +#define DSI_NULL_FRAME_OVERHEAD 6
> +#define DSI_EOT_PKT_SIZE 4
> +
> +struct cdns_dphy_cfg {
> + u8 pll_ipdiv;
> + u8 pll_opdiv;
> + u16 pll_fbdiv;
> + unsigned int nlanes;
> +};
> +
> +enum cdns_dphy_clk_lane_cfg {
> + DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
> + DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
> + DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
> + DPHY_CLK_CFG_RIGHT_DRIVES_
Re: [PATCH v4 9/9] drm/bridge: cdns: Convert to phy framework
On Wed, Jan 09, 2019 at 10:33:26AM +0100, Maxime Ripard wrote:
> Now that we have everything we need in the phy framework to allow to tune
> the phy parameters, let's convert the Cadence DSI bridge to that API
> instead of creating a ad-hoc driver for its phy.
>
> Signed-off-by: Maxime Ripard
Aside from the wakeup change mentioned in patch 8,
Acked-by: Sean Paul
> ---
> drivers/gpu/drm/bridge/Kconfig| 1 +-
> drivers/gpu/drm/bridge/cdns-dsi.c | 485 +++
> drivers/phy/cadence/cdns-dphy.c | 2 +-
> 3 files changed, 61 insertions(+), 427 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 2fee47b0d50b..8840f396a7b6 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -30,6 +30,7 @@ config DRM_CDNS_DSI
> select DRM_KMS_HELPER
> select DRM_MIPI_DSI
> select DRM_PANEL_BRIDGE
> + select GENERIC_PHY_MIPI_DPHY
> depends on OF
> help
> Support Cadence DPI to DSI bridge. This is an internal
> diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
> b/drivers/gpu/drm/bridge/cdns-dsi.c
> index 3ac6dd524b6d..7b432257ffbe 100644
> --- a/drivers/gpu/drm/bridge/cdns-dsi.c
> +++ b/drivers/gpu/drm/bridge/cdns-dsi.c
> @@ -21,6 +21,9 @@
> #include
> #include
>
> +#include
> +#include
> +
> #define IP_CONF 0x0
> #define SP_HS_FIFO_DEPTH(x) (((x) & GENMASK(30, 26)) >> 26)
> #define SP_LP_FIFO_DEPTH(x) (((x) & GENMASK(25, 21)) >> 21)
> @@ -419,44 +422,11 @@
> #define DSI_NULL_FRAME_OVERHEAD 6
> #define DSI_EOT_PKT_SIZE 4
>
> -#define REG_WAKEUP_TIME_NS 800
> -#define DPHY_PLL_RATE_HZ 10800
> -
> -/* DPHY registers */
> -#define DPHY_PMA_CMN(reg)(reg)
> -#define DPHY_PMA_LCLK(reg) (0x100 + (reg))
> -#define DPHY_PMA_LDATA(lane, reg)(0x200 + ((lane) * 0x100) + (reg))
> -#define DPHY_PMA_RCLK(reg) (0x600 + (reg))
> -#define DPHY_PMA_RDATA(lane, reg)(0x700 + ((lane) * 0x100) + (reg))
> -#define DPHY_PCS(reg)(0xb00 + (reg))
> -
> -#define DPHY_CMN_SSM DPHY_PMA_CMN(0x20)
> -#define DPHY_CMN_SSM_EN BIT(0)
> -#define DPHY_CMN_TX_MODE_EN BIT(9)
> -
> -#define DPHY_CMN_PWM DPHY_PMA_CMN(0x40)
> -#define DPHY_CMN_PWM_DIV(x) ((x) << 20)
> -#define DPHY_CMN_PWM_LOW(x) ((x) << 10)
> -#define DPHY_CMN_PWM_HIGH(x) (x)
> -
> -#define DPHY_CMN_FBDIV DPHY_PMA_CMN(0x4c)
> -#define DPHY_CMN_FBDIV_VAL(low, high)(((high) << 11) | ((low) << 22))
> -#define DPHY_CMN_FBDIV_FROM_REG (BIT(10) | BIT(21))
> -
> -#define DPHY_CMN_OPIPDIV DPHY_PMA_CMN(0x50)
> -#define DPHY_CMN_IPDIV_FROM_REG BIT(0)
> -#define DPHY_CMN_IPDIV(x)((x) << 1)
> -#define DPHY_CMN_OPDIV_FROM_REG BIT(6)
> -#define DPHY_CMN_OPDIV(x)((x) << 7)
> -
> -#define DPHY_PSM_CFG DPHY_PCS(0x4)
> -#define DPHY_PSM_CFG_FROM_REGBIT(0)
> -#define DPHY_PSM_CLK_DIV(x) ((x) << 1)
> -
> struct cdns_dsi_output {
> struct mipi_dsi_device *dev;
> struct drm_panel *panel;
> struct drm_bridge *bridge;
> + union phy_configure_opts phy_opts;
> };
>
> enum cdns_dsi_input_id {
> @@ -465,14 +435,6 @@ enum cdns_dsi_input_id {
> CDNS_DSC_INPUT,
> };
>
> -struct cdns_dphy_cfg {
> - u8 pll_ipdiv;
> - u8 pll_opdiv;
> - u16 pll_fbdiv;
> - unsigned long lane_bps;
> - unsigned int nlanes;
> -};
> -
> struct cdns_dsi_cfg {
> unsigned int hfp;
> unsigned int hsa;
> @@ -481,34 +443,6 @@ struct cdns_dsi_cfg {
> unsigned int htotal;
> };
>
> -struct cdns_dphy;
> -
> -enum cdns_dphy_clk_lane_cfg {
> - DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
> - DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
> - DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
> - DPHY_CLK_CFG_RIGHT_DRIVES_ALL = 3,
> -};
> -
> -struct cdns_dphy_ops {
> - int (*probe)(struct cdns_dphy *dphy);
> - void (*remove)(struct cdns_dphy *dphy);
> - void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
> - void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
> - enum cdns_dphy_clk_lane_cfg cfg);
> - void (*set_pll_cfg)(struct cdns_dphy *dphy,
> - const struct cdns_dphy_cfg *cfg);
> - unsigned long (*get_wakeup_time_ns)(struct cdns_dphy *dphy);
> -};
> -
> -struct cdns_dphy {
> - struct cdns_dphy_cfg cfg;
> - void __iomem *regs;
> - struct clk *psm_clk;
> - struct clk *pll_ref_clk;
> - const struct cdns_dphy_ops *ops;
> -};
> -
> struct cdns_dsi_input {
> enum cdns_dsi_input_id id;
> struct drm_bridge bridge;
> @@ -526,7 +460,7 @@ struct cdns_dsi {
> struct reset_control *dsi_p_rst;
> struct clk *dsi_sys_clk;
> bool link_ini
Re: [PATCH] drm: support gpu aliases defined in DT data
On 17/01/19 15:26, Daniel Vetter wrote:
> On Thu, Jan 17, 2019 at 2:04 PM Tomi Valkeinen wrote:
>>
>> On 17/01/19 14:33, Daniel Vetter wrote:
>>> On Thu, Jan 17, 2019 at 01:19:18PM +0200, Tomi Valkeinen wrote:
The DRM device minor numbers are allocated according to the registration
order. This causes confusion in cases where the registration order can
change, or when, say, a modesetting capable device is preferred to be
card0, and a rendering device is preferred to be card1.
This patch adds similar functionality that is used in some other
subsystems, where device minor numbers can be defined in DT bindings'
aliases node.
>>>
>>> What other subsystem? I thought that minor numbers shouldn't be made uapi,
>>> and that udev or similar is supposed to give you stable names ... Is that
>>> not the case on SoC?
>>
>> I think at least i2c, spi and uart use DT aliases.
>
> Commits/code implementing would be best.
For i2c:
ee5c27440cc24d62ec463cce4c000bb32c5692c7
("i2c: core: Pick i2c bus number from dt alias if present")
03bde7c31a360f814ca42101d60563b1b803dca1
("i2c: busses with dynamic ids should start after fixed ids for DT")
>> I also have my doubts about this, but thought to post this to get some
>> comments, as it does make life quite a bit easier =).
>
> Yeah I think "soc without udev" seems reasonable assumption, I just
I'm not that familiar with udev rules. I wonder how it would work... The
rule would need to keep all dynamic cards out from the group of reserved
card names, and also handle render nodes. All this is probably possible,
with a quick study I could find out how to implement something like that.
> think this is something the overall soc community should agree on as a
> good thing to do. I guess since the of stuff you're using is generic
> that's all happened already, so should amount to gathering a pile of
> acks and then merging it.
Ok. I'm not sure who the "SoC people" would be, but I added some more OF
people here.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/imx: imx-ldb: add missing of_node_puts
Hi Julia, On Sun, 2019-01-13 at 09:47 +0100, Julia Lawall wrote: > The device node iterators perform an of_node_get on each > iteration, so a jump out of the loop requires an of_node_put. > > Move the initialization channel->child = child; down to just > before the call to imx_ldb_register so that intervening failures > don't need to clear it. Add a label at the end of the function to > do all the of_node_puts. Thank you, I've applied the patch to the imx-drm/fixes branch. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109366] NULL pointer at pcie_capability_read_dword with Radeon SI vfio passthrough
https://bugs.freedesktop.org/show_bug.cgi?id=109366 --- Comment #3 from Ryan Bair --- Thank you both for the responses. I can confirm using the Q35 machine type does not see this issue. I'm rebuilding a kernel today to test the patch and will report back. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd, line option
On 01/17/2019 02:40 PM, Peter Rosin wrote: > On 2019-01-16 17:45, Bartlomiej Zolnierkiewicz wrote: >> On 01/07/2019 11:35 AM, Peter Rosin wrote: >>> Right. So, here's an update... >>> >>> Again, it would probably be best if this went in before 5.0 is released. >>> >>> Changes since v1: >>> - rename from fbcon=center-logo option to fbcon=logo-pos:center (and adjust >>> the help text accordingly). >>> >>> Cheers, >>> Peter >>> >>> From ebfb2feb7ea4298ac9c222e6ea6f9c9a92452084 Mon Sep 17 00:00:00 2001 >>> From: Peter Rosin >>> Cc: Bartlomiej Zolnierkiewicz >>> Cc: Jonathan Corbet >>> Cc: Peter Rosin >>> Cc: dri-devel@lists.freedesktop.org >>> Cc: linux-fb...@vger.kernel.org >>> Cc: linux-...@vger.kernel.org >>> To: linux-ker...@vger.kernel.org >>> Date: Mon, 7 Jan 2019 08:35:26 +0100 >>> Subject: [PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd >>> line option >>> >>> A command line option is much more flexible than a config option and >>> the supporting code is small. Gets rid of #ifdefs in the code too... >>> >>> Suggested-by: Geert Uytterhoeven >>> Signed-off-by: Peter Rosin >> >> Patch queued for 5.0, thanks. > > Hmm, I see the patch on the fbdev-for-next branch, but that sounds > like stuff destined for 5.1? Maybe you simply don't have any > fbdev-for-current or fbdev-fixes branch or something like that? Yes. the single public fbdev branch has been sufficient for now (there is no active fbdev development and patches for -rcX kernels are even more rare). OTOH there is no problem with adding -fixes branch once needed.. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109366] NULL pointer at pcie_capability_read_dword with Radeon SI vfio passthrough
https://bugs.freedesktop.org/show_bug.cgi?id=109366 --- Comment #4 from Ryan Bair --- I can confirm the attached patch does fix the issue for i440FX. -- 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/doc: Make igts for cross-driver stuff mandatory
On Thu, Jan 17, 2019 at 01:32:15PM +0100, Daniel Vetter wrote: > On Thu, Jan 17, 2019 at 1:26 PM Liviu Dudau wrote: > > > > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote: > > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau wrote: > > > > > > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote: > > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved > > > > > forward a lot: > > > > > > > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build > > > > > and a sysroot build (should address all the build/cross platform > > > > > concerns raised in the RFC discussions). > > > > > > > > > > - tests reorganized into subdirectories so that the i915-gem tests > > > > > don't clog the main/shared tests directory anymore > > > > > > > > > > - quite a few more non-intel people contributing/reviewing/committing > > > > > igt tests patches. > > > > > > > > > > I think this addresses all the concerns raised in the RFC discussions, > > > > > and assuming there's enough Acks and no new issues that pop up, we can > > > > > go ahead with this. > > > > > > > > > > 1: https://patchwork.kernel.org/patch/10648851/ > > > > > Cc: Petri Latvala > > > > > Cc: Arkadiusz Hiler > > > > > Cc: Liviu Dudau > > > > > Cc: Sean Paul > > > > > Cc: Eric Anholt > > > > > Cc: Alex Deucher > > > > > Cc: Dave Airlie > > > > > Signed-off-by: Daniel Vetter > > > > > --- > > > > > Documentation/gpu/drm-uapi.rst | 7 +++ > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > diff --git a/Documentation/gpu/drm-uapi.rst > > > > > b/Documentation/gpu/drm-uapi.rst > > > > > index a752aa561ea4..413915d6b7d2 100644 > > > > > --- a/Documentation/gpu/drm-uapi.rst > > > > > +++ b/Documentation/gpu/drm-uapi.rst > > > > > @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the > > > > > slightly unintuitive meaning of > > > > > Testing and validation > > > > > == > > > > > > > > > > +Testing Requirements for userspace API > > > > > +-- > > > > > + > > > > > +New cross-driver userspace interface extensions, like new IOCTL, new > > > > > KMS > > > > > +properties, new files in sysfs or anything else that constitutes an > > > > > API change > > > > > +need to have driver-agnostic testcases in IGT for that feature. > > > > > > > > From an aspirational point of view I am fine with this and you can have > > > > my Acked-by: Liviu Dudau . > > > > > > > > From a practical point of view I would like to see a matrix of KMS APIs > > > > that are being validated and the drivers that have been tested. > > > > Otherwise, > > > > the next person that comes and tries to add a new IOCTL, KMS property > > > > or new > > > > file in sysfs is going to discover that he has subscribed to a much > > > > bigger > > > > task of getting enough KMS drivers testable in the first place. > > > > > > This is what the _new_ features is about, no expectation to write > > > tests for all the existing stuff. > > > > Yeah, but if the "tests for all the existing stuff" doesn't exist, your > > _new_ feature tests are not going to mean much, doesn't it? > > Why? There's still good test coverage for the new stuff at least. And > eventually someone gets bored and fills in the gaps. This is how we've > created all the current igt testcases, so it works - mandatory igt for > new features is the rule for anything intel does since well over 5 > years. Because if you can't test basic stuff with igt how did you test your new feature? What I'm saying is: if developer X is having to use igt for a new feature then he is either: a) expecting that basic testing works for his driver (or he has bootstrapping isssues) or that b) if the feature doesn't get tested on driver A because that driver doesn't even have basic tests passing for it then the feature is acceptable. And it will be helpful to publish the list of igt tests that pass for each driver in KMS. Alternatively, you need to explain better what "driver-agnostic testcase" is (my reading: a testcase that can be run on any driver). Or point to some igt testcases that are considered driver-agnostic and known to run on different (read: non-x86) platforms. > > > > Although I think there's not really > > > any big gaps in igt anymore, we do have at least some (rather rough > > > and coarse in some case) test coverage for everything I think. > > > > I would like to see some proof of that in the form of > > > > > > > Should this be clarified further? > > > > an URL that points to a page similar to Mesa's GL supported features > > would be nice to add here, from my point of view. > > There isn't even a list of upstream drm features ... Yeah, I know that! :) > how exactly should that look like? What about a list of igt tests then? OR we could put in the igt TODO file the kind of tests we would like to have at some moment? Best regards, Liviu > Only thing I can tell you is that
Re: [PATCH v5 1/2] drm/sched: Refactor ring mirror list handling.
On 01/17/2019 02:45 AM, Christian König wrote: Am 16.01.19 um 18:17 schrieb Grodzovsky, Andrey: On 01/16/2019 11:02 AM, Koenig, Christian wrote: Am 16.01.19 um 16:45 schrieb Grodzovsky, Andrey: On 01/16/2019 02:46 AM, Christian König wrote: Am 15.01.19 um 23:01 schrieb Grodzovsky, Andrey: On 01/11/2019 05:03 PM, Andrey Grodzovsky wrote: On 01/11/2019 02:11 PM, Koenig, Christian wrote: Am 11.01.19 um 16:37 schrieb Grodzovsky, Andrey: On 01/11/2019 04:42 AM, Koenig, Christian wrote: Am 10.01.19 um 16:56 schrieb Grodzovsky, Andrey: [SNIP] But we will not be adding the cb back in drm_sched_stop anymore, now we are only going to add back the cb in drm_sched_startr after rerunning those jobs in drm_sched_resubmit_jobs and assign them a new parent there anyway. Yeah, but when we find that we don't need to reset anything anymore then adding the callbacks again won't be possible any more. Christian. I am not sure I understand it, can u point me to example of how this will happen ? I am attaching my latest patches with waiting only for the last job's fence here just so we are on same page regarding the code. Well the whole idea is to prepare all schedulers, then check once more if the offending job hasn't completed in the meantime. If the job completed we need to be able to rollback everything and continue as if nothing had happened. Christian. Oh, but this piece of functionality - skipping HW ASIC reset in case the guilty job done is totally missing form this patch series and so needs to be added. So what you say actually is that for the case were we skip HW asic reset because the guilty job did complete we also need to skip resubmitting the jobs in drm_sched_resubmit_jobs and hence preserve the old parent fence pointer for reuse ? If so I would like to add all this functionality as a third patch since the first 2 patches are more about resolving race condition with jobs in flight while doing reset - what do you think ? Yeah, sounds perfectly fine to me. Christian. I realized there is another complication now for XGMI hive use case, we currently skip gpu recover for adev in case another gpu recover for different adev in same hive is running, under the assumption that we are going to reset all devices in hive anyway because that should cover our own dev too. But if we chose to skip now HW asic reset if our guilty job did finish we will aslo not HW reset any other devices in the hive even if one of them might actually had a bad job, wanted to do gpu recover but skipped it because our recover was in progress in that time. My general idea on that is to keep a list of guilty jobs per hive, when you start gpu recover you first add you guilty job to the hive and trylock hive->reset_lock. Any owner of hive->reset_lock (gpu recovery in progress) once he finished his recovery and released hive->reset_lock should go over hive->bad_jobs_list and if at least one of them is still not signaled (not done) trigger another gpu recovery and so on. If you do manage to trylock you also go over the list, clean it and perform recovery. This list probably needs to be protected with per hive lock. I also think we can for now at least finish reviewing the first 2 patches and submit them since as I said before they are not dealing with this topic and fixing existing race conditions. If you are OK with that I can send for review the last iteration of the first 2 patches where I wait for the last fence in ring mirror list. Andrey I implemented HW reset avoidance including XGMI use case according to the plan i specified. Patch is attached but I can't test it yet due to XGMI regression in PSP which is supposed to be fixed soon. Please take a look. Looks a bit too complicated on first glance. In general we should probably get away from handling a hive in any special way. Yes, I guess i can do it the same way as the generic handling in amdgpu_device_gpu_recover - there is a list of devices to process which is of size 1 for non xgmi use case or more then 1 for XGMI. Multiple timeout jobs in a hive are identical to multiple timeout jobs on different engines on a single device. How about the following handling: 1. Add the timeout job to a list. 2. Try to grab a lock to handle the reset, if that doesn't work because there is already a reset underway return immediately. 3. Stop all schedulers on all affected devices including stopping the timeouts and detaching all callbacks. 4. Double check the list of timed out jobs, if all hw fences of all jobs are completed now we actually don't need to do anything. What if all the jobs on the timed out list did complete but other job (or jobs) for which we removed the time out timer became hanged ? Wouldn't we miss a required reset in this case and wouldn't even have any indication of their hang ? If the timeout triggers before we disable it we will have the job on the list of jobs which are hanging. If we found that we don't reset and re-enable the timeout
Re: [PATCH v5 1/2] drm/sched: Refactor ring mirror list handling.
Am 17.01.19 um 16:22 schrieb Grodzovsky, Andrey: On 01/17/2019 02:45 AM, Christian König wrote: Am 16.01.19 um 18:17 schrieb Grodzovsky, Andrey: On 01/16/2019 11:02 AM, Koenig, Christian wrote: Am 16.01.19 um 16:45 schrieb Grodzovsky, Andrey: On 01/16/2019 02:46 AM, Christian König wrote: Am 15.01.19 um 23:01 schrieb Grodzovsky, Andrey: On 01/11/2019 05:03 PM, Andrey Grodzovsky wrote: On 01/11/2019 02:11 PM, Koenig, Christian wrote: Am 11.01.19 um 16:37 schrieb Grodzovsky, Andrey: On 01/11/2019 04:42 AM, Koenig, Christian wrote: Am 10.01.19 um 16:56 schrieb Grodzovsky, Andrey: [SNIP] But we will not be adding the cb back in drm_sched_stop anymore, now we are only going to add back the cb in drm_sched_startr after rerunning those jobs in drm_sched_resubmit_jobs and assign them a new parent there anyway. Yeah, but when we find that we don't need to reset anything anymore then adding the callbacks again won't be possible any more. Christian. I am not sure I understand it, can u point me to example of how this will happen ? I am attaching my latest patches with waiting only for the last job's fence here just so we are on same page regarding the code. Well the whole idea is to prepare all schedulers, then check once more if the offending job hasn't completed in the meantime. If the job completed we need to be able to rollback everything and continue as if nothing had happened. Christian. Oh, but this piece of functionality - skipping HW ASIC reset in case the guilty job done is totally missing form this patch series and so needs to be added. So what you say actually is that for the case were we skip HW asic reset because the guilty job did complete we also need to skip resubmitting the jobs in drm_sched_resubmit_jobs and hence preserve the old parent fence pointer for reuse ? If so I would like to add all this functionality as a third patch since the first 2 patches are more about resolving race condition with jobs in flight while doing reset - what do you think ? Yeah, sounds perfectly fine to me. Christian. I realized there is another complication now for XGMI hive use case, we currently skip gpu recover for adev in case another gpu recover for different adev in same hive is running, under the assumption that we are going to reset all devices in hive anyway because that should cover our own dev too. But if we chose to skip now HW asic reset if our guilty job did finish we will aslo not HW reset any other devices in the hive even if one of them might actually had a bad job, wanted to do gpu recover but skipped it because our recover was in progress in that time. My general idea on that is to keep a list of guilty jobs per hive, when you start gpu recover you first add you guilty job to the hive and trylock hive->reset_lock. Any owner of hive->reset_lock (gpu recovery in progress) once he finished his recovery and released hive->reset_lock should go over hive->bad_jobs_list and if at least one of them is still not signaled (not done) trigger another gpu recovery and so on. If you do manage to trylock you also go over the list, clean it and perform recovery. This list probably needs to be protected with per hive lock. I also think we can for now at least finish reviewing the first 2 patches and submit them since as I said before they are not dealing with this topic and fixing existing race conditions. If you are OK with that I can send for review the last iteration of the first 2 patches where I wait for the last fence in ring mirror list. Andrey I implemented HW reset avoidance including XGMI use case according to the plan i specified. Patch is attached but I can't test it yet due to XGMI regression in PSP which is supposed to be fixed soon. Please take a look. Looks a bit too complicated on first glance. In general we should probably get away from handling a hive in any special way. Yes, I guess i can do it the same way as the generic handling in amdgpu_device_gpu_recover - there is a list of devices to process which is of size 1 for non xgmi use case or more then 1 for XGMI. Multiple timeout jobs in a hive are identical to multiple timeout jobs on different engines on a single device. How about the following handling: 1. Add the timeout job to a list. 2. Try to grab a lock to handle the reset, if that doesn't work because there is already a reset underway return immediately. 3. Stop all schedulers on all affected devices including stopping the timeouts and detaching all callbacks. 4. Double check the list of timed out jobs, if all hw fences of all jobs are completed now we actually don't need to do anything. What if all the jobs on the timed out list did complete but other job (or jobs) for which we removed the time out timer became hanged ? Wouldn't we miss a required reset in this case and wouldn't even have any indication of their hang ? If the timeout triggers before we disable it we will have the job on the list of jobs which are hanging. If we fo
Re: [RFC 00/16] Armada DRM support for OLPC XO-1.75 laptop
On Thu, Jan 17, 2019 at 04:03:45PM +0100, Lubomir Rintel wrote: > On Thu, 2019-01-17 at 10:55 +, Russell King - ARM Linux admin > wrote: > > On Tue, Dec 18, 2018 at 04:37:26PM +0100, Lubomir Rintel wrote: > > > here are patches that make the Armada DRM work on an OLPC XO-1.75. > > > They build on patches previously submitted by Russel King (included here > > > for > > > completeness as well). > > > > Hi, > > > > Would it be possible for you to re-post patches 1 through 6 to include > > the DT maintainers so that they can review those patches (as is > > required for their acceptance) - once they have done that, I can pick > > them up and resubmit some of the patches I have queued. > > Yes. Before I do so, I'd like to know your opinion about the compatible > strings of the framebuffer and display-subsystem nodes. > > In the "[RFC 05/16] dt-bindings: display: armada: Add mmp2 compatible > strings" message [1] I wrote: > > Note that perhaps "marvell,armada-display-subsystem" and > > "marvell,armada-framebuffer" would be a good idea in addition of > > dove/mmp2 specific strings since (at least for now) the driver code > > is the same. > > [1] https://lists.freedesktop.org/archives/dri-devel/2018-December/201016.html > > I essentially documented what you implemented, but I'd prefer if you > used the "marvell,armada-display-subsystem" and > "marvell,armada-framebuffer" compatible strings in the driver code > instead. Things in DT tend to be named by the first user who comes along - so we often get DT compatibles that refer to the first device. However, we do currently have the opportunity to change it, so please do as you suggest. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory
On Thu, Jan 17, 2019 at 3:54 PM Liviu Dudau wrote: > > On Thu, Jan 17, 2019 at 01:32:15PM +0100, Daniel Vetter wrote: > > On Thu, Jan 17, 2019 at 1:26 PM Liviu Dudau wrote: > > > > > > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote: > > > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau > > > > wrote: > > > > > > > > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote: > > > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved > > > > > > forward a lot: > > > > > > > > > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build > > > > > > and a sysroot build (should address all the build/cross platform > > > > > > concerns raised in the RFC discussions). > > > > > > > > > > > > - tests reorganized into subdirectories so that the i915-gem tests > > > > > > don't clog the main/shared tests directory anymore > > > > > > > > > > > > - quite a few more non-intel people > > > > > > contributing/reviewing/committing > > > > > > igt tests patches. > > > > > > > > > > > > I think this addresses all the concerns raised in the RFC > > > > > > discussions, > > > > > > and assuming there's enough Acks and no new issues that pop up, we > > > > > > can > > > > > > go ahead with this. > > > > > > > > > > > > 1: https://patchwork.kernel.org/patch/10648851/ > > > > > > Cc: Petri Latvala > > > > > > Cc: Arkadiusz Hiler > > > > > > Cc: Liviu Dudau > > > > > > Cc: Sean Paul > > > > > > Cc: Eric Anholt > > > > > > Cc: Alex Deucher > > > > > > Cc: Dave Airlie > > > > > > Signed-off-by: Daniel Vetter > > > > > > --- > > > > > > Documentation/gpu/drm-uapi.rst | 7 +++ > > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > > > diff --git a/Documentation/gpu/drm-uapi.rst > > > > > > b/Documentation/gpu/drm-uapi.rst > > > > > > index a752aa561ea4..413915d6b7d2 100644 > > > > > > --- a/Documentation/gpu/drm-uapi.rst > > > > > > +++ b/Documentation/gpu/drm-uapi.rst > > > > > > @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has > > > > > > the slightly unintuitive meaning of > > > > > > Testing and validation > > > > > > == > > > > > > > > > > > > +Testing Requirements for userspace API > > > > > > +-- > > > > > > + > > > > > > +New cross-driver userspace interface extensions, like new IOCTL, > > > > > > new KMS > > > > > > +properties, new files in sysfs or anything else that constitutes > > > > > > an API change > > > > > > +need to have driver-agnostic testcases in IGT for that feature. > > > > > > > > > > From an aspirational point of view I am fine with this and you can > > > > > have > > > > > my Acked-by: Liviu Dudau . > > > > > > > > > > From a practical point of view I would like to see a matrix of KMS > > > > > APIs > > > > > that are being validated and the drivers that have been tested. > > > > > Otherwise, > > > > > the next person that comes and tries to add a new IOCTL, KMS property > > > > > or new > > > > > file in sysfs is going to discover that he has subscribed to a much > > > > > bigger > > > > > task of getting enough KMS drivers testable in the first place. > > > > > > > > This is what the _new_ features is about, no expectation to write > > > > tests for all the existing stuff. > > > > > > Yeah, but if the "tests for all the existing stuff" doesn't exist, your > > > _new_ feature tests are not going to mean much, doesn't it? > > > > Why? There's still good test coverage for the new stuff at least. And > > eventually someone gets bored and fills in the gaps. This is how we've > > created all the current igt testcases, so it works - mandatory igt for > > new features is the rule for anything intel does since well over 5 > > years. > > Because if you can't test basic stuff with igt how did you test your new > feature? What I'm saying is: if developer X is having to use igt for > a new feature then he is either: a) expecting that basic testing works for > his driver (or he has bootstrapping isssues) or that b) if the feature > doesn't get tested on driver A because that driver doesn't even have basic > tests passing for it then the feature is acceptable. And it will be helpful > to publish the list of igt tests that pass for each driver in KMS. I think this is an entirely different discussion, something along the lines of "new drivers/new features implemented on existing drivers" must come with igt test results to prove it's working. Not what I'm proposing here at all. It would be a good discussion to have, but a different one I think. The proposal here is only for new uapi: Thus far the requirement is "driver + userspace", not it would be "driver + userspace + igt, if generic feature". > Alternatively, you need to explain better what "driver-agnostic testcase" is > (my reading: a testcase that can be run on any driver). Or point to some igt > testcases that are considered driver-agnostic and known to run on different > (read: non-x8
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On 1/16/19 4:48 PM, Liam Mark wrote:
> On Wed, 16 Jan 2019, Andrew F. Davis wrote:
>
>> On 1/15/19 1:05 PM, Laura Abbott wrote:
>>> On 1/15/19 10:38 AM, Andrew F. Davis wrote:
On 1/15/19 11:45 AM, Liam Mark wrote:
> On Tue, 15 Jan 2019, Andrew F. Davis wrote:
>
>> On 1/14/19 11:13 AM, Liam Mark wrote:
>>> On Fri, 11 Jan 2019, Andrew F. Davis wrote:
>>>
Buffers may not be mapped from the CPU so skip cache maintenance
here.
Accesses from the CPU to a cached heap should be bracketed with
{begin,end}_cpu_access calls so maintenance should not be needed
anyway.
Signed-off-by: Andrew F. Davis
---
drivers/staging/android/ion/ion.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 14e48f6eb734..09cb5a8e2b09 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
dma_buf_attachment *attachment,
table = a->table;
- if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
- direction))
+ if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
+ direction, DMA_ATTR_SKIP_CPU_SYNC))
>>>
>>> Unfortunately I don't think you can do this for a couple reasons.
>>> You can't rely on {begin,end}_cpu_access calls to do cache
>>> maintenance.
>>> If the calls to {begin,end}_cpu_access were made before the call to
>>> dma_buf_attach then there won't have been a device attached so the
>>> calls
>>> to {begin,end}_cpu_access won't have done any cache maintenance.
>>>
>>
>> That should be okay though, if you have no attachments (or all
>> attachments are IO-coherent) then there is no need for cache
>> maintenance. Unless you mean a sequence where a non-io-coherent device
>> is attached later after data has already been written. Does that
>> sequence need supporting?
>
> Yes, but also I think there are cases where CPU access can happen before
> in Android, but I will focus on later for now.
>
>> DMA-BUF doesn't have to allocate the backing
>> memory until map_dma_buf() time, and that should only happen after all
>> the devices have attached so it can know where to put the buffer. So we
>> shouldn't expect any CPU access to buffers before all the devices are
>> attached and mapped, right?
>>
>
> Here is an example where CPU access can happen later in Android.
>
> Camera device records video -> software post processing -> video device
> (who does compression of raw data) and writes to a file
>
> In this example assume the buffer is cached and the devices are not
> IO-coherent (quite common).
>
This is the start of the problem, having cached mappings of memory that
is also being accessed non-coherently is going to cause issues one way
or another. On top of the speculative cache fills that have to be
constantly fought back against with CMOs like below; some coherent
interconnects behave badly when you mix coherent and non-coherent access
(snoop filters get messed up).
The solution is to either always have the addresses marked non-coherent
(like device memory, no-map carveouts), or if you really want to use
regular system memory allocated at runtime, then all cached mappings of
it need to be dropped, even the kernel logical address (area as painful
as that would be).
>>>
>>> I agree it's broken, hence my desire to remove it :)
>>>
>>> The other problem is that uncached buffers are being used for
>>> performance reason so anything that would involve getting
>>> rid of the logical address would probably negate any performance
>>> benefit.
>>>
>>
>> I wouldn't go as far as to remove them just yet.. Liam seems pretty
>> adamant that they have valid uses. I'm just not sure performance is one
>> of them, maybe in the case of software locks between devices or
>> something where there needs to be a lot of back and forth interleaved
>> access on small amounts of data?
>>
>
> I wasn't aware that ARM considered this not supported, I thought it was
> supported but they advised against it because of the potential performance
> impact.
>
Not sure what you mean by "this" being not supported, do you mean mixed
attribute mappings? If so, it will certainly cause problems, and the
problems will change from platform to platform, avoid at all costs is my
understanding of ARM's position.
> This is after all supported in the DMA APIs and up until now devices have
> been successfully commercializing with this configurations, and I think
> they will continue to commerciali
Re: [PATCH v2] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
On Wed, Jan 16, 2019 at 04:38:17PM +0300, Alexander Shiyan wrote:
> Use the SIMPLE_DEV_PM_OPS() macro to declare the driver's pm_ops.
> As a side effect, removing #ifdef CONFIG_PM_SLEEP will improve
> compilation coverage.
Adopting SIMPLE_DEV_PM_OPS() results in the suspend/resume functions
being called during a freeze/thaw. Is turning off the backlight during a
freeze really necessary?
>
> Signed-off-by: Alexander Shiyan
> ---
Summary of changes since v1 would be useful here. For single patches you
can use git annotate and the corresponding argument to format-patch to
handle the change log.
> drivers/video/backlight/pwm_bl.c | 16
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/backlight/pwm_bl.c
> b/drivers/video/backlight/pwm_bl.c
> index feb9076..eaefdb0 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -673,8 +673,7 @@ static void pwm_backlight_shutdown(struct platform_device
> *pdev)
> pwm_backlight_power_off(pb);
> }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int pwm_backlight_suspend(struct device *dev)
> +static int __maybe_unused pwm_backlight_suspend(struct device *dev)
> {
> struct backlight_device *bl = dev_get_drvdata(dev);
> struct pwm_bl_data *pb = bl_get_data(bl);
> @@ -690,7 +689,7 @@ static int pwm_backlight_suspend(struct device *dev)
> return 0;
> }
>
> -static int pwm_backlight_resume(struct device *dev)
> +static int __maybe_unused pwm_backlight_resume(struct device *dev)
> {
> struct backlight_device *bl = dev_get_drvdata(dev);
>
> @@ -698,16 +697,9 @@ static int pwm_backlight_resume(struct device *dev)
>
> return 0;
> }
> -#endif
>
> -static const struct dev_pm_ops pwm_backlight_pm_ops = {
> -#ifdef CONFIG_PM_SLEEP
> - .suspend = pwm_backlight_suspend,
> - .resume = pwm_backlight_resume,
> - .poweroff = pwm_backlight_suspend,
> - .restore = pwm_backlight_resume,
> -#endif
> -};
> +static SIMPLE_DEV_PM_OPS(pwm_backlight_pm_ops,
> + pwm_backlight_suspend, pwm_backlight_resume);
>
> static struct platform_driver pwm_backlight_driver = {
> .driver = {
> --
> 2.10.2
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] video: lcd: Remove useless BACKLIGHT_CLASS_DEVICE dependencies
On Thu, Jan 17, 2019 at 04:33:36PM +0300, Alexander Shiyan wrote: > This patch removes dependencies on BACKLIGHT_CLASS_DEVICE for items > that are already placed under #if BACKLIGHT_CLASS_DEVICE. Why the # before the if (in Kconfig its just "if" right?). > > Signed-off-by: Alexander Shiyan Any didn't I ack this already? Daniel. > --- > drivers/video/backlight/Kconfig | 25 - > 1 file changed, 12 insertions(+), 13 deletions(-) > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > index 3fdc18e..3ed1d90 100644 > --- a/drivers/video/backlight/Kconfig > +++ b/drivers/video/backlight/Kconfig > @@ -193,7 +193,6 @@ config BACKLIGHT_IPAQ_MICRO > > config BACKLIGHT_LM3533 > tristate "Backlight Driver for LM3533" > - depends on BACKLIGHT_CLASS_DEVICE > depends on MFD_LM3533 > help > Say Y to enable the backlight driver for National Semiconductor / TI > @@ -317,7 +316,7 @@ config BACKLIGHT_ADP5520 > > config BACKLIGHT_ADP8860 > tristate "Backlight Driver for ADP8860/ADP8861/ADP8863 using WLED" > - depends on BACKLIGHT_CLASS_DEVICE && I2C > + depends on I2C > select NEW_LEDS > select LEDS_CLASS > help > @@ -329,7 +328,7 @@ config BACKLIGHT_ADP8860 > > config BACKLIGHT_ADP8870 > tristate "Backlight Driver for ADP8870 using WLED" > - depends on BACKLIGHT_CLASS_DEVICE && I2C > + depends on I2C > select NEW_LEDS > select LEDS_CLASS > help > @@ -347,28 +346,28 @@ config BACKLIGHT_88PM860X > > config BACKLIGHT_PCF50633 > tristate "Backlight driver for NXP PCF50633 MFD" > - depends on BACKLIGHT_CLASS_DEVICE && MFD_PCF50633 > + depends on MFD_PCF50633 > help > If you have a backlight driven by a NXP PCF50633 MFD, say Y here to > enable its driver. > > config BACKLIGHT_AAT2870 > tristate "AnalogicTech AAT2870 Backlight" > - depends on BACKLIGHT_CLASS_DEVICE && MFD_AAT2870_CORE > + depends on MFD_AAT2870_CORE > help > If you have a AnalogicTech AAT2870 say Y to enable the > backlight driver. > > config BACKLIGHT_LM3630A > tristate "Backlight Driver for LM3630A" > - depends on BACKLIGHT_CLASS_DEVICE && I2C && PWM > + depends on I2C && PWM > select REGMAP_I2C > help > This supports TI LM3630A Backlight Driver > > config BACKLIGHT_LM3639 > tristate "Backlight Driver for LM3639" > - depends on BACKLIGHT_CLASS_DEVICE && I2C > + depends on I2C > select REGMAP_I2C > select NEW_LEDS > select LEDS_CLASS > @@ -377,20 +376,20 @@ config BACKLIGHT_LM3639 > > config BACKLIGHT_LP855X > tristate "Backlight driver for TI LP855X" > - depends on BACKLIGHT_CLASS_DEVICE && I2C && PWM > + depends on I2C && PWM > help > This supports TI LP8550, LP8551, LP8552, LP8553, LP8555, LP8556 and > LP8557 backlight driver. > > config BACKLIGHT_LP8788 > tristate "Backlight driver for TI LP8788 MFD" > - depends on BACKLIGHT_CLASS_DEVICE && MFD_LP8788 && PWM > + depends on MFD_LP8788 && PWM > help > This supports TI LP8788 backlight driver. > > config BACKLIGHT_OT200 > tristate "Backlight driver for ot200 visualisation device" > - depends on BACKLIGHT_CLASS_DEVICE && CS5535_MFGPT && GPIO_CS5535 > + depends on CS5535_MFGPT && GPIO_CS5535 > help > To compile this driver as a module, choose M here: the module will be > called ot200_bl. > @@ -404,7 +403,7 @@ config BACKLIGHT_PANDORA > > config BACKLIGHT_SKY81452 > tristate "Backlight driver for SKY81452" > - depends on BACKLIGHT_CLASS_DEVICE && MFD_SKY81452 > + depends on MFD_SKY81452 > help > If you have a Skyworks SKY81452, say Y to enable the > backlight driver. > @@ -414,14 +413,14 @@ config BACKLIGHT_SKY81452 > > config BACKLIGHT_TPS65217 > tristate "TPS65217 Backlight" > - depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217 > + depends on MFD_TPS65217 > help > If you have a Texas Instruments TPS65217 say Y to enable the > backlight driver. > > config BACKLIGHT_AS3711 > tristate "AS3711 Backlight" > - depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711 > + depends on MFD_AS3711 > help > If you have an Austrian Microsystems AS3711 say Y to enable the > backlight driver. > -- > 2.10.2 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] video: lcd: Remove useless BACKLIGHT_LCD_SUPPORT kernel symbol
On Thu, Jan 17, 2019 at 04:33:35PM +0300, Alexander Shiyan wrote: > We have two *_CLASS_DEVICE kernel config options (LCD_CLASS_DEVICE > and BACKLIGHT_LCD_DEVICE) that do the same job. > The patch removes useless BACKLIGHT_LCD_SUPPORT option > and converts LCD_CLASS_DEVICE into a menu. > > Signed-off-by: Alexander Shiyan A cover letter with the v1 -> v2 changelog would be nice... but nevertheless: Acked-by: Daniel Thompson > --- > arch/unicore32/Kconfig| 1 - > drivers/gpu/drm/Kconfig | 2 -- > drivers/gpu/drm/bridge/Kconfig| 1 - > drivers/gpu/drm/fsl-dcu/Kconfig | 1 - > drivers/gpu/drm/i915/Kconfig | 1 - > drivers/gpu/drm/nouveau/Kconfig | 2 -- > drivers/gpu/drm/shmobile/Kconfig | 1 - > drivers/gpu/drm/tilcdc/Kconfig| 1 - > drivers/staging/olpc_dcon/Kconfig | 1 - > drivers/usb/misc/Kconfig | 1 - > drivers/video/backlight/Kconfig | 10 ++ > drivers/video/fbdev/Kconfig | 5 - > 12 files changed, 2 insertions(+), 25 deletions(-) > > diff --git a/arch/unicore32/Kconfig b/arch/unicore32/Kconfig > index c3a41bf..1138334 100644 > --- a/arch/unicore32/Kconfig > +++ b/arch/unicore32/Kconfig > @@ -195,7 +195,6 @@ config I2C_EEPROM_AT24 > > config LCD_BACKLIGHT > tristate "LCD Backlight support" > - select BACKLIGHT_LCD_SUPPORT > select BACKLIGHT_PWM > > endmenu > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 4385f00..ef442a7 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -198,7 +198,6 @@ config DRM_RADEON > select POWER_SUPPLY > select HWMON > select BACKLIGHT_CLASS_DEVICE > - select BACKLIGHT_LCD_SUPPORT > select INTERVAL_TREE > help > Choose this option if you have an ATI Radeon graphics card. There > @@ -219,7 +218,6 @@ config DRM_AMDGPU > select POWER_SUPPLY > select HWMON > select BACKLIGHT_CLASS_DEVICE > - select BACKLIGHT_LCD_SUPPORT > select INTERVAL_TREE > select CHASH > help > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 2fee47b..5b5a8e5 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -75,7 +75,6 @@ config DRM_PARADE_PS8622 > depends on OF > select DRM_PANEL > select DRM_KMS_HELPER > - select BACKLIGHT_LCD_SUPPORT > select BACKLIGHT_CLASS_DEVICE > ---help--- > Parade eDP-LVDS bridge chip driver. > diff --git a/drivers/gpu/drm/fsl-dcu/Kconfig b/drivers/gpu/drm/fsl-dcu/Kconfig > index 14a72c4..dc82588 100644 > --- a/drivers/gpu/drm/fsl-dcu/Kconfig > +++ b/drivers/gpu/drm/fsl-dcu/Kconfig > @@ -2,7 +2,6 @@ config DRM_FSL_DCU > tristate "DRM Support for Freescale DCU" > depends on DRM && OF && ARM && COMMON_CLK > select BACKLIGHT_CLASS_DEVICE > - select BACKLIGHT_LCD_SUPPORT > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > select DRM_PANEL > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > index 148be8e..3d5f1cb 100644 > --- a/drivers/gpu/drm/i915/Kconfig > +++ b/drivers/gpu/drm/i915/Kconfig > @@ -15,7 +15,6 @@ config DRM_I915 > select IRQ_WORK > # i915 depends on ACPI_VIDEO when ACPI is enabled > # but for select to work, need to select ACPI_VIDEO's dependencies, ick > - select BACKLIGHT_LCD_SUPPORT if ACPI > select BACKLIGHT_CLASS_DEVICE if ACPI > select INPUT if ACPI > select ACPI_VIDEO if ACPI > diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig > index 432c440..fd7b869 100644 > --- a/drivers/gpu/drm/nouveau/Kconfig > +++ b/drivers/gpu/drm/nouveau/Kconfig > @@ -5,14 +5,12 @@ config DRM_NOUVEAU > select DRM_KMS_HELPER > select DRM_TTM > select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT > - select BACKLIGHT_LCD_SUPPORT if DRM_NOUVEAU_BACKLIGHT > select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && INPUT > select X86_PLATFORM_DEVICES if ACPI && X86 > select ACPI_WMI if ACPI && X86 > select MXM_WMI if ACPI && X86 > select POWER_SUPPLY > # Similar to i915, we need to select ACPI_VIDEO and it's dependencies > - select BACKLIGHT_LCD_SUPPORT if ACPI && X86 > select BACKLIGHT_CLASS_DEVICE if ACPI && X86 > select INPUT if ACPI && X86 > select THERMAL if ACPI && X86 > diff --git a/drivers/gpu/drm/shmobile/Kconfig > b/drivers/gpu/drm/shmobile/Kconfig > index 61bbe8e..e2a6c82 100644 > --- a/drivers/gpu/drm/shmobile/Kconfig > +++ b/drivers/gpu/drm/shmobile/Kconfig > @@ -4,7 +4,6 @@ config DRM_SHMOBILE > depends on DRM && ARM > depends on ARCH_SHMOBILE || COMPILE_TEST > select BACKLIGHT_CLASS_DEVICE > - select BACKLIGHT_LCD_SUPPORT > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > select DRM_GEM_CMA_HELPER > diff --git a/drivers/gpu/drm/tilcdc/Kconfig b/drivers/gpu/drm/tilcdc/K
Re: [PATCH v5 1/2] drm/sched: Refactor ring mirror list handling.
On 01/17/2019 10:29 AM, Koenig, Christian wrote: Am 17.01.19 um 16:22 schrieb Grodzovsky, Andrey: On 01/17/2019 02:45 AM, Christian König wrote: Am 16.01.19 um 18:17 schrieb Grodzovsky, Andrey: On 01/16/2019 11:02 AM, Koenig, Christian wrote: Am 16.01.19 um 16:45 schrieb Grodzovsky, Andrey: On 01/16/2019 02:46 AM, Christian König wrote: Am 15.01.19 um 23:01 schrieb Grodzovsky, Andrey: On 01/11/2019 05:03 PM, Andrey Grodzovsky wrote: On 01/11/2019 02:11 PM, Koenig, Christian wrote: Am 11.01.19 um 16:37 schrieb Grodzovsky, Andrey: On 01/11/2019 04:42 AM, Koenig, Christian wrote: Am 10.01.19 um 16:56 schrieb Grodzovsky, Andrey: [SNIP] But we will not be adding the cb back in drm_sched_stop anymore, now we are only going to add back the cb in drm_sched_startr after rerunning those jobs in drm_sched_resubmit_jobs and assign them a new parent there anyway. Yeah, but when we find that we don't need to reset anything anymore then adding the callbacks again won't be possible any more. Christian. I am not sure I understand it, can u point me to example of how this will happen ? I am attaching my latest patches with waiting only for the last job's fence here just so we are on same page regarding the code. Well the whole idea is to prepare all schedulers, then check once more if the offending job hasn't completed in the meantime. If the job completed we need to be able to rollback everything and continue as if nothing had happened. Christian. Oh, but this piece of functionality - skipping HW ASIC reset in case the guilty job done is totally missing form this patch series and so needs to be added. So what you say actually is that for the case were we skip HW asic reset because the guilty job did complete we also need to skip resubmitting the jobs in drm_sched_resubmit_jobs and hence preserve the old parent fence pointer for reuse ? If so I would like to add all this functionality as a third patch since the first 2 patches are more about resolving race condition with jobs in flight while doing reset - what do you think ? Yeah, sounds perfectly fine to me. Christian. I realized there is another complication now for XGMI hive use case, we currently skip gpu recover for adev in case another gpu recover for different adev in same hive is running, under the assumption that we are going to reset all devices in hive anyway because that should cover our own dev too. But if we chose to skip now HW asic reset if our guilty job did finish we will aslo not HW reset any other devices in the hive even if one of them might actually had a bad job, wanted to do gpu recover but skipped it because our recover was in progress in that time. My general idea on that is to keep a list of guilty jobs per hive, when you start gpu recover you first add you guilty job to the hive and trylock hive->reset_lock. Any owner of hive->reset_lock (gpu recovery in progress) once he finished his recovery and released hive->reset_lock should go over hive->bad_jobs_list and if at least one of them is still not signaled (not done) trigger another gpu recovery and so on. If you do manage to trylock you also go over the list, clean it and perform recovery. This list probably needs to be protected with per hive lock. I also think we can for now at least finish reviewing the first 2 patches and submit them since as I said before they are not dealing with this topic and fixing existing race conditions. If you are OK with that I can send for review the last iteration of the first 2 patches where I wait for the last fence in ring mirror list. Andrey I implemented HW reset avoidance including XGMI use case according to the plan i specified. Patch is attached but I can't test it yet due to XGMI regression in PSP which is supposed to be fixed soon. Please take a look. Looks a bit too complicated on first glance. In general we should probably get away from handling a hive in any special way. Yes, I guess i can do it the same way as the generic handling in amdgpu_device_gpu_recover - there is a list of devices to process which is of size 1 for non xgmi use case or more then 1 for XGMI. Multiple timeout jobs in a hive are identical to multiple timeout jobs on different engines on a single device. How about the following handling: 1. Add the timeout job to a list. 2. Try to grab a lock to handle the reset, if that doesn't work because there is already a reset underway return immediately. 3. Stop all schedulers on all affected devices including stopping the timeouts and detaching all callbacks. 4. Double check the list of timed out jobs, if all hw fences of all jobs are completed now we actually don't need to do anything. What if all the jobs on the timed out list did complete but other job (or jobs) for which we removed the time out timer became hanged ? Wouldn't we miss a required reset in this case and wouldn't even have any indication of their hang ? If the timeout triggers before we disable it we will have the jo
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On 1/16/19 4:54 PM, Liam Mark wrote:
> On Wed, 16 Jan 2019, Andrew F. Davis wrote:
>
>> On 1/16/19 9:19 AM, Brian Starkey wrote:
>>> Hi :-)
>>>
>>> On Tue, Jan 15, 2019 at 12:40:16PM -0600, Andrew F. Davis wrote:
On 1/15/19 12:38 PM, Andrew F. Davis wrote:
> On 1/15/19 11:45 AM, Liam Mark wrote:
>> On Tue, 15 Jan 2019, Andrew F. Davis wrote:
>>
>>> On 1/14/19 11:13 AM, Liam Mark wrote:
On Fri, 11 Jan 2019, Andrew F. Davis wrote:
> Buffers may not be mapped from the CPU so skip cache maintenance here.
> Accesses from the CPU to a cached heap should be bracketed with
> {begin,end}_cpu_access calls so maintenance should not be needed
> anyway.
>
> Signed-off-by: Andrew F. Davis
> ---
> drivers/staging/android/ion/ion.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index 14e48f6eb734..09cb5a8e2b09 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
> dma_buf_attachment *attachment,
>
> table = a->table;
>
> - if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
> - direction))
> + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
> + direction, DMA_ATTR_SKIP_CPU_SYNC))
Unfortunately I don't think you can do this for a couple reasons.
You can't rely on {begin,end}_cpu_access calls to do cache maintenance.
If the calls to {begin,end}_cpu_access were made before the call to
dma_buf_attach then there won't have been a device attached so the
calls
to {begin,end}_cpu_access won't have done any cache maintenance.
>>>
>>> That should be okay though, if you have no attachments (or all
>>> attachments are IO-coherent) then there is no need for cache
>>> maintenance. Unless you mean a sequence where a non-io-coherent device
>>> is attached later after data has already been written. Does that
>>> sequence need supporting?
>>
>> Yes, but also I think there are cases where CPU access can happen before
>> in Android, but I will focus on later for now.
>>
>>> DMA-BUF doesn't have to allocate the backing
>>> memory until map_dma_buf() time, and that should only happen after all
>>> the devices have attached so it can know where to put the buffer. So we
>>> shouldn't expect any CPU access to buffers before all the devices are
>>> attached and mapped, right?
>>>
>>
>> Here is an example where CPU access can happen later in Android.
>>
>> Camera device records video -> software post processing -> video device
>> (who does compression of raw data) and writes to a file
>>
>> In this example assume the buffer is cached and the devices are not
>> IO-coherent (quite common).
>>
>
> This is the start of the problem, having cached mappings of memory that
> is also being accessed non-coherently is going to cause issues one way
> or another. On top of the speculative cache fills that have to be
> constantly fought back against with CMOs like below; some coherent
> interconnects behave badly when you mix coherent and non-coherent access
> (snoop filters get messed up).
>
> The solution is to either always have the addresses marked non-coherent
> (like device memory, no-map carveouts), or if you really want to use
> regular system memory allocated at runtime, then all cached mappings of
> it need to be dropped, even the kernel logical address (area as painful
> as that would be).
>>>
>>> Ouch :-( I wasn't aware about these potential interconnect issues. How
>>> "real" is that? It seems that we aren't really hitting that today on
>>> real devices.
>>>
>>
>> Sadly there is at least one real device like this now (TI AM654). We
>> spent some time working with the ARM interconnect spec designers to see
>> if this was allowed behavior, final conclusion was mixing coherent and
>> non-coherent accesses is never a good idea.. So we have been working to
>> try to minimize any cases of mixed attributes [0], if a region is
>> coherent then everyone in the system needs to treat it as such and
>> vice-versa, even clever CMO that work on other systems wont save you
>> here. :(
>>
>> [0] https://github.com/ARM-software/arm-trusted-firmware/pull/1553
>>
>>
>
>> ION buffer is allocated.
>>
>> //Camera device records video
>> dma_buf_attach
>> dma_map_attachment (buffer needs to be cleaned)
>
> Why does the buffer need to be cleaned here? I just got through reading
> th
Re: [PATCH] drm: Split out drm_probe_helper.h
On Wed, Jan 16, 2019 at 07:10:18PM +0100, Sam Ravnborg wrote: > Hi Daniel. > > > v5: Actually try to sort them, and while at it, sort all the ones I > > touch. > > Applied this variant on top of drm-misc and did a build test. > Looked good for ia64, x86 and alpha. > > Took a closer look at the changes to atmel_hlcd - and they looked OK. > > But I noticed that atmel_hlcdc uses only drm_kms_helper_poll_init() and > drm_kms_helper_poll_fini(). > But there are no hits on DRM_CONNECTOR_POLL - so I think we maybe > have a driver here where we have plugged the drm_poll infrastructure, > but it is not in use. > > > include/drm/drm_crtc_helper.h | 16 --- > > The list of include files in this file could be dropped and replaced by: > struct drm_connector; > struct drm_device; > struct drm_display_mode; > struct drm_encoder; > struct drm_framebuffer; > struct drm_mode_set; > struct drm_modeset_acquire_ctx; > > I tried to do so on top of your patch. > But there were too many build errros and I somehow lost the motivation. Yeah the drm_crtc_helper.h header is a bit the miniature drmP.h for legacy kms drivers. Just removing it from all the atomic drivers caused lots of fallout, I expect even more if you entirely remove the includes it has. Maybe a todo, care to pls create that patch since it's your idea? -Daniel > > > > include/drm/drm_probe_helper.h| 27 +++ > This on the other hand is fine - as expected as this is a new file. > > But the above is just some random comments so: > > Acked-by: Sam Ravnborg -- 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: Armada DRM: bridge with componentized devices
On Tue, Jan 15, 2019 at 11:47:00PM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > Subject: [PATCH] driver core: Fix adding device links to probing suppliers > > Currently, it is not valid to add a device link from a consumer > driver ->probe() callback to a supplier that is still probing too, but > generally this is a valid use case. For example, if the consumer has > just acquired a resource that can only be available when the supplier > is functional, adding a device link to that supplier right away > should be safe (and even desirable arguably), but device_link_add() > doesn't handle that case correctly and the initial state of the link > created by it is wrong then. > > To address this problem, change the initial state of device links > added between a probing supplier and a probing consumer to > DL_STATE_CONSUMER_PROBE and update device_links_driver_bound() to > skip such links on the supplier side. > > With this change, if the supplier probe completes first, > device_links_driver_bound() called for it will skip the link state > update and when it is called for the consumer, the link state will > be updated to "active". In turn, if the consumer probe completes > first, device_links_driver_bound() called for it will change the > state of the link to "active" and when it is called for the > supplier, the link status update will be skipped. > > However, in principle the supplier or consumer probe may still fail > after the link has been added, so modify device_links_no_driver() to > change device links in the "active" or "consumer probe" state to > "dormant" on the supplier side and update __device_links_no_driver() > to change the link state to "available" only if it is "consumer > probe" or "active". > > Then, if the supplier probe fails first, the leftover link to the > probing consumer will become "dormant" and device_links_no_driver() > called for the consumer (when its probe fails) will clean it up. > In turn, if the consumer probe fails first, it will either drop the > link, or change its state to "available" and, in the latter case, > when device_links_no_driver() is called for the supplier, it will > update the link state to "dormant". [If the supplier probe fails, > but the consumer probe succeeds, which should not happen as long as > the consumer driver is correct, the link still will be around, but > it will be "dormant" until the supplier is probed again.] > > Signed-off-by: Rafael J. Wysocki Hi Rafael, I've tried this with Armada DRM without using the component helper for bridges, and it seems to work up to a point (I have a vga bridge here to allow further testing): [2.323441] armada-drm display-subsystem: assigned reserved memory node framebuffer [2.332001] armada-drm display-subsystem: bound f182.lcd-controller (ops armada_lcd_ops) [2.340790] armada-drm display-subsystem: bound f181.lcd-controller (ops armada_lcd_ops) [2.349345] armada-drm display-subsystem: node=/i2c-mux/i2c@0/hdmi-encoder@70 [2.356719] armada-drm display-subsystem: panel=fdfb bridge= (null) ret=0 [2.364038] armada-drm display-subsystem: Linked as a consumer to 1-0070 [2.370818] armada-drm display-subsystem: bridge=ee8cda2c ret=0 [2.376894] armada-drm display-subsystem: node=/vga-bridge [2.382453] armada-drm display-subsystem: panel=fdfb bridge=(null) ret=0 [2.389762] armada-drm display-subsystem: Linked as a consumer to vga-bridge [2.396883] armada-drm display-subsystem: bridge=ef3bec40 ret=0 When I remove the HDMI encoder: root@cubox:/sys/bus/i2c/drivers/tda998x# echo 1-0070 > unbind then I get: [ 1013.824860] Console: switching to colour dummy device 80x30 [ 1013.866785] armada-drm display-subsystem: Dropping the link to vga-bridge [ 1013.867126] armada-drm display-subsystem: Dropping the link to 1-0070 which looks like it did what was expected - the DRM device is indeed unbound, the nodes in /dev/dri are gone. When rebinding the HDMI encoder: [ 1015.864703] tda998x 1-0070: found TDA19988 [ 1015.898078] tda9950 1-0034: TDA9950 CEC interface, hardware version 3.3 [ 1015.941374] Registered IR keymap rc-cec [ 1015.941684] rc rc0: tda9950 as /devices/platform/mbus/mbus:internal-regs/f1011000.i2c/i2c-0/i2c-1/1-0070/rc/rc0 [ 1015.942439] input: tda9950 as /devices/platform/mbus/mbus:internal-regs/f1011000.i2c/i2c-0/i2c-1/1-0070/rc/rc0/input2 [ 1016.982921] alloc_contig_range: [10390, 10490) PFNs busy [ 1016.987400] kirkwood-spdif-audio audio-subsystem: snd-soc-dummy-dai <-> kirkwood-fe mapping ok [ 1016.987591] kirkwood-spdif-audio audio-subsystem: multicodec <-> kirkwood-spdif mapping ok but the DRM stuff doesn't come back - this is what Daniel was talking about further down this thread. I guess the kernel can't know that it should come back because the device links were dropped at the unbind stage, which means the kernel has lost the information necessary to know that the display subsystem is dependent on the presence
Re: [PATCH] drm: Split out drm_probe_helper.h
On Thu, Jan 17, 2019 at 05:45:41PM +0100, Daniel Vetter wrote: > On Wed, Jan 16, 2019 at 07:10:18PM +0100, Sam Ravnborg wrote: > > Hi Daniel. > > > > > v5: Actually try to sort them, and while at it, sort all the ones I > > > touch. > > > > Applied this variant on top of drm-misc and did a build test. > > Looked good for ia64, x86 and alpha. > > > > Took a closer look at the changes to atmel_hlcd - and they looked OK. > > > > But I noticed that atmel_hlcdc uses only drm_kms_helper_poll_init() and > > drm_kms_helper_poll_fini(). > > But there are no hits on DRM_CONNECTOR_POLL - so I think we maybe > > have a driver here where we have plugged the drm_poll infrastructure, > > but it is not in use. > > > > > include/drm/drm_crtc_helper.h | 16 --- > > > > The list of include files in this file could be dropped and replaced by: > > struct drm_connector; > > struct drm_device; > > struct drm_display_mode; > > struct drm_encoder; > > struct drm_framebuffer; > > struct drm_mode_set; > > struct drm_modeset_acquire_ctx; > > > > I tried to do so on top of your patch. > > But there were too many build errros and I somehow lost the motivation. > > Yeah the drm_crtc_helper.h header is a bit the miniature drmP.h for legacy > kms drivers. Just removing it from all the atomic drivers caused lots of > fallout, I expect even more if you entirely remove the includes it has. > Maybe a todo, care to pls create that patch since it's your idea? The main reason I bailed out initially was that this would create small changes to several otherwise seldomly touched files. And then we would later come and remove drmP.h - so lots of small but incremental changes to the same otherwise seldomly edited files. And the job was only partially done. I will try to experiment with an approach where I clean up the include/drm/*.h files a little (like suggested above, +delete drmP.h and maybe a bit more). Then to try on a driver by driver basis to make it build with a cleaned set of include files. I hope that the cleaned up driver can still build without the cleaned header files so the changes can be submitted piecemal. Will do so with an eye on the lesser maintained drivers to try it out to avoid creating too much chrunch for others. And if it works out I expect the active drivers to follow the example. todo.rst item will wait until I run out of energy. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: HDMI/DVI spurious failure
On Wed, Jan 16, 2019 at 08:35:16PM +, Priit Laes wrote: > On Wed, Jan 16, 2019 at 08:24:42PM +0100, Maxime Ripard wrote: > > Hi Priit, > > > > On Wed, Jan 16, 2019 at 07:58:54AM +, Priit Laes wrote: > > > > On Mon, Jan 14, 2019 at 01:29:34PM +, Priit Laes wrote: > > > > > I have a somewhat curious case with one HDMI/DVI screen that fails > > > > > to initialize properly every 6-7 boots. The display itself is also > > > > > somewhat flawed (missing HPD pin and the VSYNC/HSYNC pulse width > > > > > is set to 0 in EDID), but I suspect there could be some issues > > > > > regarding timing in A20 HDMI driver in Linux. > > > > > > > > ... > > > > > > > It doesn't look related to the clock rate itself, since it doesn't > > > > change between the two cases. However, in one case the DDC clock is > > > > enabled and in the other it's disabled. > > > > > > > > Was it taken at the same time? Maybe you can try with that patch? > > > > http://code.bulix.org/z7jmkm-555344?raw > > > > > > Thanks, after doing ~50+ boots I haven't seen a single failure. > > > > > > Previously I had following failure cases which are now both fixed: > > > > > > a) Linux without u-boot HDMI, where one in every 6-7 boots failed. > > > b) u--boot with hdmi enabled switching to simplefb and then switching > > > to kms, where previously all boots ended up with garbled screen. > > > > So it's not really a fix, but it really looks like the clock is not > > enabled when it should. > > > > Can you describe your test scenario a bit more? What are you doing > > exactly, just booting? When do you start using the display? When did > > you capture the debugfs output that you pasted? > > Display is already connected via HDMI to the board. I don't really > remove it, I just boot the device and let it start Xorg. > Meanwhile I just ssh into the device and capture debugfs output. > See my 3 testing scenarios below. > > Kernel also includes one extra patch to fall back to DDC, in case HPD > fails. Mostly the same I already submitted last November [1]. Do you have the same issue without that patch? > For u-boot I have also some extra patches, to detect HPD-less HDMI > displays [2] + relax some EDID timing checks [3] so u-boot can actually > initialize my screen. Do you have the same issues without those patches? > So first configuration with 100% failures: > 1) u-boot initializes HDMI ( A20-OLinuXino-Lime2-eMMC_defconfig ) > 2) Linux switches to simplefb > ... somewhere around here blinking cursor is replaced with garbage > on screen > 3) Linux switches to kms > 4) Xorg starts > > Second scenario with failure every 6-7 boots: > 1) Disabled HDMI in u-boot for my board > 2) Linux sets up kms (sometimes fails here) > 3) Xorg starts > 4) ssh to machine and take the clock dump Do you have the DRM fbdev emulation enabled in that case or is Xorg the first to setup the kms driver? Do you have some logs and a configuration? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com 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: support gpu aliases defined in DT data
On Thu, Jan 17, 2019 at 01:19:18PM +0200, Tomi Valkeinen wrote:
> The DRM device minor numbers are allocated according to the registration
> order. This causes confusion in cases where the registration order can
> change, or when, say, a modesetting capable device is preferred to be
> card0, and a rendering device is preferred to be card1.
>
> This patch adds similar functionality that is used in some other
> subsystems, where device minor numbers can be defined in DT bindings'
> aliases node.
>
> For example, this sets the DRM device minor number to 1 for the 'dss'
> device.
>
> aliases {
> gpu1 = &dss;
> };
>
> The logic on how to pick the minor number is:
>
> - if there's a DT gpu alias for the device, use that
> - else, if there are any gpu aliases, pick a minor number that is higher
> than any of the aliases.
> - else, use the full range of possible numbers
>
> Signed-off-by: Tomi Valkeinen
I guess we'd need a binding document for this? IIRC, Rob was against
introducing new aliases. Rob?
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Expose more EDID fields to userspace
Daniel Vetter writes: > Connector properties are documented here: > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#standard-connector-properties Cool. Seems like we might want a bit more organization here to make it clear which of these are derived from the connected monitor. It would be good to read through the source code to find other EDID-ish data being used within the kernel. Things like the vendor and model info might be useful as properties, especially if there might be quirks to 'fix' the edid values and drive other kernel behavior. -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] include/drm: color_mgmt: Add enum labels
On Fri, Dec 14, 2018 at 01:10:16PM +0100, Christoph Manszewski wrote:
> Range setting makes sense for YCbCr and RGB buffers. Current
> drm_color_range enum labels suggest use with YCbCr buffers.
> Create enum labels without colorspace specification.
>
> Signed-off-by: Christoph Manszewski
> ---
> include/drm/drm_color_mgmt.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 90ef9996d9a4..52f6d5221a0d 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -62,6 +62,8 @@ enum drm_color_range {
> DRM_COLOR_YCBCR_LIMITED_RANGE,
> DRM_COLOR_YCBCR_FULL_RANGE,
> DRM_COLOR_RANGE_MAX,
> + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE,
> + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE,
These enum values don't mean anything really. The strings are what
matter for the uapi.
The default for YCbCr needs to be limited range, the default for RGB
needs to be full range. So I think this would really require a separate
prop for each.
But is there an actual usecase for this stuff?
> };
>
> int drm_plane_create_color_properties(struct drm_plane *plane,
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Expose more EDID fields to userspace
On Wed, Jan 16, 2019 at 1:35 PM Pekka Paalanen wrote: > > On Mon, 7 Jan 2019 17:07:09 + > Brian Starkey wrote: > > > On Mon, Jan 07, 2019 at 07:57:54AM -0800, Keith Packard wrote: > > > Daniel Vetter writes: > > > > > > > Best to pull in some other compositor people and get them to agree. > > > > From a > > > > kernel pov I'm fine with whatever userspace preferes. > > > > > > Hrm. It would be good to have everyone using the same interpretation of > > > EDID data; in particular, where the kernel has quirks that change the > > > interpretation, user space should be consistent with that. > > > > > > Unless we expose all of the EDID data, then user space may still have to > > > parse EDID. If the kernel has EDID quirks, it might be good to to make > > > those affect the "raw" EDID data visible to use space so that values the > > > kernel supplies separately are consistent with values extracted from the > > > "raw" EDID data. > > > > If the quirks can be re-encoded back into an EDID representation, then > > this sounds like a fairly good approach to me. > > > > > > > > Doing this in the kernel does make it harder to quickly supply fixes for > > > a specific user space application. This will probably lead to > > > kludge-arounds in user space that could depend on kernel > > > version. Perhaps these EDID capabilities in the kernel should be > > > versioned separately? > > > > > > I see good benefits from having user space able to see how the kernel is > > > interpreting EDID so that it can adapt as appropriate, but we should be > > > cautious about moving functionality into the kernel that would be more > > > easily maintained up in user space. > > > > > > > I agree. It seems likely that whatever happens (some) userspace is > > still going to implement (some) EDID parsing functionality, so it's > > hard to reason about what belongs where. Shared code in userspace > > (libdrm?) may well be better than exposing it from the kernel. > > > > If it is exposed by the kernel, then it's still non-obvious to me > > how the kernel exposes that information/interpretation. Adding > > a property for every potentially-useful field really doesn't scale > > well, and what fields are useful isn't obvious - e.g. serial string vs > > serial no., as mentioned by Simon. > > > > Uma's recent series: "Add HDR Metadata Parsing and handling in DRM > > layer"[1] is a good example of more stuff which userspace would want to > > parse out of the EDID (supported display colorimetry and transfer > > functions). > > > > It would be nice to avoid duplicating all the CEA extension parsing > > code, but the EDID/CEA data structure is extensible by design. So the > > kernel API would need to be similarly extensible, or we'll just > > balloon loads of properties... and then the kernel API would likely > > just end up just looking similar to the CEA block anyway. > > > > Cheers, > > -Brian > > > > [1] > > https://lists.freedesktop.org/archives/dri-devel/2018-December/200154.html > > I would agree with an effort to establish a userspace EDID parsing > library in any case. As mentioned above, there will probably be too > much to expose via kernel UABI, or it will become just another > encoded format that again should have a shared parser library in > userspace. > > Would it be possible to architect the library so that it would be > shared with the kernel? Maybe the quirks database could be shared > with the kernel as well? That way both kernel and userspace would > more or less agree on the parsing details. Maybe make it part of libdrm and import the shared files periodically like we do for driver ioctl interfaces? Alex > > I've been dreaming of a "liboutput" that would e.g. parse EDID, > generate CVT video mode timings, and whatnot that all display > servers more or less will duplicate. Once upon a time Ajax started > minitru IIRC... > > Another thing for "liboutput" was a device description database, > whose first use would have been the "non-desktop" property. Because > we don't expose monitors through udev to have udev rules tag them > with interesting bits. > > Imagine this: monitors exposed as devices via udev, tagged with > helpers as regular monitor, HMD, TV, projector, ... and all EDID > fields decoded as well. And quirks in hwdb. But I suppose that > won't happen, because a "monitor device node" would have no other > use. Except... programmatical monitor controls? Like backlight, > brightness, contrast, and so on. > > Ah, nevermind. :-) > > > Thanks, > pq > ___ > 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
[Bug 109239] Polaris10: Periodic random black screens for 1-2 seconds
https://bugs.freedesktop.org/show_bug.cgi?id=109239 --- Comment #9 from Raman Gupta --- (In reply to Harry Wentland from comment #7) > If you didn't say you tried swapping monitors and cables I'd say it was a > cable issue. > > Are those high refresh rate displays (120Hz+)? If so you might want to give > what's suggested in this comment a try: > https://bugs.freedesktop.org/show_bug.cgi?id=102646#c41 It would help us > diagnose the issue. While I believe this was directed at Samuel Pitoiset (I don't have high refresh rate displays), I tried it anyway. Same problem occurs. -- 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: Expose more EDID fields to userspace
On Thu, Jan 17, 2019 at 8:59 PM Alex Deucher wrote: > > On Wed, Jan 16, 2019 at 1:35 PM Pekka Paalanen wrote: > > > > On Mon, 7 Jan 2019 17:07:09 + > > Brian Starkey wrote: > > > > > On Mon, Jan 07, 2019 at 07:57:54AM -0800, Keith Packard wrote: > > > > Daniel Vetter writes: > > > > > > > > > Best to pull in some other compositor people and get them to agree. > > > > > From a > > > > > kernel pov I'm fine with whatever userspace preferes. > > > > > > > > Hrm. It would be good to have everyone using the same interpretation of > > > > EDID data; in particular, where the kernel has quirks that change the > > > > interpretation, user space should be consistent with that. > > > > > > > > Unless we expose all of the EDID data, then user space may still have to > > > > parse EDID. If the kernel has EDID quirks, it might be good to to make > > > > those affect the "raw" EDID data visible to use space so that values the > > > > kernel supplies separately are consistent with values extracted from the > > > > "raw" EDID data. > > > > > > If the quirks can be re-encoded back into an EDID representation, then > > > this sounds like a fairly good approach to me. > > > > > > > > > > > Doing this in the kernel does make it harder to quickly supply fixes for > > > > a specific user space application. This will probably lead to > > > > kludge-arounds in user space that could depend on kernel > > > > version. Perhaps these EDID capabilities in the kernel should be > > > > versioned separately? > > > > > > > > I see good benefits from having user space able to see how the kernel is > > > > interpreting EDID so that it can adapt as appropriate, but we should be > > > > cautious about moving functionality into the kernel that would be more > > > > easily maintained up in user space. > > > > > > > > > > I agree. It seems likely that whatever happens (some) userspace is > > > still going to implement (some) EDID parsing functionality, so it's > > > hard to reason about what belongs where. Shared code in userspace > > > (libdrm?) may well be better than exposing it from the kernel. > > > > > > If it is exposed by the kernel, then it's still non-obvious to me > > > how the kernel exposes that information/interpretation. Adding > > > a property for every potentially-useful field really doesn't scale > > > well, and what fields are useful isn't obvious - e.g. serial string vs > > > serial no., as mentioned by Simon. > > > > > > Uma's recent series: "Add HDR Metadata Parsing and handling in DRM > > > layer"[1] is a good example of more stuff which userspace would want to > > > parse out of the EDID (supported display colorimetry and transfer > > > functions). > > > > > > It would be nice to avoid duplicating all the CEA extension parsing > > > code, but the EDID/CEA data structure is extensible by design. So the > > > kernel API would need to be similarly extensible, or we'll just > > > balloon loads of properties... and then the kernel API would likely > > > just end up just looking similar to the CEA block anyway. > > > > > > Cheers, > > > -Brian > > > > > > [1] > > > https://lists.freedesktop.org/archives/dri-devel/2018-December/200154.html > > > > I would agree with an effort to establish a userspace EDID parsing > > library in any case. As mentioned above, there will probably be too > > much to expose via kernel UABI, or it will become just another > > encoded format that again should have a shared parser library in > > userspace. > > > > Would it be possible to architect the library so that it would be > > shared with the kernel? Maybe the quirks database could be shared > > with the kernel as well? That way both kernel and userspace would > > more or less agree on the parsing details. > > Maybe make it part of libdrm and import the shared files periodically > like we do for driver ioctl interfaces? We could also merge libdrm into the kernel ... Just as a wild idea to consider at least. I think it would also help in other areas, keeping the headers in sync, documenting the uapi properly, and all that. Would lockstep libdrm release with kernel releases, but most drivers+igt fixed that by just having copies of the headers in their tree. -Daniel > > Alex > > > > > I've been dreaming of a "liboutput" that would e.g. parse EDID, > > generate CVT video mode timings, and whatnot that all display > > servers more or less will duplicate. Once upon a time Ajax started > > minitru IIRC... > > > > Another thing for "liboutput" was a device description database, > > whose first use would have been the "non-desktop" property. Because > > we don't expose monitors through udev to have udev rules tag them > > with interesting bits. > > > > Imagine this: monitors exposed as devices via udev, tagged with > > helpers as regular monitor, HMD, TV, projector, ... and all EDID > > fields decoded as well. And quirks in hwdb. But I suppose that > > won't happen, because a "monitor device node" would have no other > > use. Except... programmat
Re: Expose more EDID fields to userspace
On Mon, Jan 7, 2019, 09:07 Brian Starkey On Mon, Jan 07, 2019 at 07:57:54AM -0800, Keith Packard wrote: > > Daniel Vetter writes: > > > > > Best to pull in some other compositor people and get them to agree. > From a > > > kernel pov I'm fine with whatever userspace preferes. > > > > Hrm. It would be good to have everyone using the same interpretation of > > EDID data; in particular, where the kernel has quirks that change the > > interpretation, user space should be consistent with that. > > > > Unless we expose all of the EDID data, then user space may still have to > > parse EDID. If the kernel has EDID quirks, it might be good to to make > > those affect the "raw" EDID data visible to use space so that values the > > kernel supplies separately are consistent with values extracted from the > > "raw" EDID data. > > If the quirks can be re-encoded back into an EDID representation, then > this sounds like a fairly good approach to me. > I don't have strong feelings for against this approach, but if we do this, I think we should ensure we keep providing the original EDID to user space. The contents of EDIDs have so many implications that even the kernel isn't always in the best position to decide if a rewrite is a good idea. For a simple example, we can look at the max pixel clock which is reported in the EDID. If the EDID gets rewritten with a lower pixel clock that matches what the link can do, user space loses the ability to pop up a UI dialog telling the user that if they were using a better cable, they could get higher resolutions. Something similar already happens today with some display dongles which decide to rewrite EDIDs based on their own limitations. It prevents user space from showing a dialog recommending a better dongle. Of course one could argue the dongle is protecting itself here :) > > > > Doing this in the kernel does make it harder to quickly supply fixes for > > a specific user space application. This will probably lead to > > kludge-arounds in user space that could depend on kernel > > version. Perhaps these EDID capabilities in the kernel should be > > versioned separately? > > > > I see good benefits from having user space able to see how the kernel is > > interpreting EDID so that it can adapt as appropriate, but we should be > > cautious about moving functionality into the kernel that would be more > > easily maintained up in user space. > > > > I agree. It seems likely that whatever happens (some) userspace is > still going to implement (some) EDID parsing functionality, so it's > hard to reason about what belongs where. Shared code in userspace > (libdrm?) may well be better than exposing it from the kernel. > > If it is exposed by the kernel, then it's still non-obvious to me > how the kernel exposes that information/interpretation. Adding > a property for every potentially-useful field really doesn't scale > well, and what fields are useful isn't obvious - e.g. serial string vs > serial no., as mentioned by Simon. > > Uma's recent series: "Add HDR Metadata Parsing and handling in DRM > layer"[1] is a good example of more stuff which userspace would want to > parse out of the EDID (supported display colorimetry and transfer > functions). > FWIW for Chrome OS we do parse the color space in user space, since as you mention this isn't available through the DRM properties. Tangentially related, the content of these color points is often very... "buggy". We have to do some sanity checking before deciding to use it or not. That's why I think that even with all the information parsed by the kernel, you still need another layer... > > It would be nice to avoid duplicating all the CEA extension parsing > code, but the EDID/CEA data structure is extensible by design. So the > kernel API would need to be similarly extensible, or we'll just > balloon loads of properties... and then the kernel API would likely > just end up just looking similar to the CEA block anyway. > Yes I like the idea of parsing in user space, since it doesn't require new kernel changes at all, and typically updating a user space library is simpler than changing kernel versions. Frankly it feels to me that the kernel doesn't really have a business here except passing through the raw EDID contents to a component which knows better. Stéphane > > Cheers, > -Brian > > [1] > https://lists.freedesktop.org/archives/dri-devel/2018-December/200154.html > > > -- > > -keith > > > > > ___ > > 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 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Expose more EDID fields to userspace
Stéphane Marchesin writes: > I don't have strong feelings for against this approach, but if we do this, > I think we should ensure we keep providing the original EDID to user space. > The contents of EDIDs have so many implications that even the kernel isn't > always in the best position to decide if a rewrite is a good idea. Yeah, I think I've talked myself out of modifying EDID too. What I'm thinking this afternoon is that we should ensure that every value derived from EDID, (and potentially modified by the kernel), needs to be exposed to user space so that it too can play with the same information. If we could get a common EDID parsing library shared between kernel and user space, that would at least give everyone the same view of the data. > For a simple example, we can look at the max pixel clock which is reported > in the EDID. If the EDID gets rewritten with a lower pixel clock that > matches what the link can do, user space loses the ability to pop up a UI > dialog telling the user that if they were using a better cable, they could > get higher resolutions. Something similar already happens today with some > display dongles which decide to rewrite EDIDs based on their own > limitations. It prevents user space from showing a dialog recommending a > better dongle. Of course one could argue the dongle is protecting itself > here :) Oh, that's a fine point -- what this exposes is that user space should be able to see the lower pixel clock value which the kernel is using. And doing that separate from the EDID data means it can explain what's going on. > FWIW for Chrome OS we do parse the color space in user space, since as you > mention this isn't available through the DRM properties. If this isn't used by the kernel, then doing it in user space is probably the right plan. > Tangentially related, the content of these color points is often very... > "buggy". We have to do some sanity checking before deciding to use it or > not. That's why I think that even with all the information parsed by the > kernel, you still need another layer... Right, having a shared library and database for these kinds of quirks would be awesome. > Yes I like the idea of parsing in user space, since it doesn't require new > kernel changes at all, and typically updating a user space library is > simpler than changing kernel versions. Frankly it feels to me that the > kernel doesn't really have a business here except passing through the raw > EDID contents to a component which knows better. Agreed, as long as we also fix the kernel to tell user space what it's doing with the EDID data, especially where it's modifying values. -- -keith 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: support gpu aliases defined in DT data
Tomi Valkeinen writes:
> The DRM device minor numbers are allocated according to the registration
> order. This causes confusion in cases where the registration order can
> change, or when, say, a modesetting capable device is preferred to be
> card0, and a rendering device is preferred to be card1.
>
> This patch adds similar functionality that is used in some other
> subsystems, where device minor numbers can be defined in DT bindings'
> aliases node.
>
> For example, this sets the DRM device minor number to 1 for the 'dss'
> device.
>
> aliases {
> gpu1 = &dss;
> };
This would be really nice to have. Given that there's plenty of
userspace code that opens the first available DRM node (whether or not
that is Correct Behavior), making their behavior not dependent on the
random probe order is a good thing.
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: support gpu aliases defined in DT data
On Thu, Jan 17, 2019 at 7:26 AM Daniel Vetter wrote: > > On Thu, Jan 17, 2019 at 2:04 PM Tomi Valkeinen wrote: > > > > On 17/01/19 14:33, Daniel Vetter wrote: > > > On Thu, Jan 17, 2019 at 01:19:18PM +0200, Tomi Valkeinen wrote: > > >> The DRM device minor numbers are allocated according to the registration > > >> order. This causes confusion in cases where the registration order can > > >> change, or when, say, a modesetting capable device is preferred to be > > >> card0, and a rendering device is preferred to be card1. > > >> > > >> This patch adds similar functionality that is used in some other > > >> subsystems, where device minor numbers can be defined in DT bindings' > > >> aliases node. > > > > > > What other subsystem? I thought that minor numbers shouldn't be made uapi, > > > and that udev or similar is supposed to give you stable names ... Is that > > > not the case on SoC? You thought correctly. The problem is lots of people like their stable names. > > I think at least i2c, spi and uart use DT aliases. Well, yes. UARTs were largely to not break 'console=ttySx' and/or getty's on a ttySx when moving to DT. That is largely a solved problem now (though everyone still puts them in). SPI and I2C sneaked in I guess. Those are really only for folks twiddling with SPI and I2C directly from userspace. There's been discussions in the past how not use aliases, but no one has cared enough to implement. > Commits/code implementing would be best. > > > I also have my doubts about this, but thought to post this to get some > > comments, as it does make life quite a bit easier =). > > Yeah I think "soc without udev" seems reasonable assumption, I just > think this is something the overall soc community should agree on as a > good thing to do. I guess since the of stuff you're using is generic > that's all happened already, so should amount to gathering a pile of > acks and then merging it. Mesa/libdrm already has lots of code to open the correct devices and not care about minor numbers. What's the problem here? Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.
https://bugs.freedesktop.org/show_bug.cgi?id=105733 --- Comment #55 from l...@protonmail.ch --- I have a very similar problem, although the few differences is that my entire screen becomes one single color, which doesn't seem to be entirely random. Some times it is grey, other times a blueish tint, but never colors like black. In addition, num lock etc. are still responsive for a small while, although it seems that a delay in the response time is added rapidly each second, very soon seeming completely unresponsive. My system: CPU: AMD Ryzen 5 1600 GPU: Sapphire NITRO+ RX 580 4 GB Motherboard: ASUS ROG STRIX X470-F Kernel: 4.20.1 Distribution: NixOS WM: Sway or i3, happens in both I am using DVI-D, if that is at all relevant. Oddly enough, even though the symptoms have stayed the exact same the entire time, the error messages I get very widely. At one point I was getting the "GPU fault detected" errors, at other times it would say that an sdma0 ring or gfx ring timed out, and now I have no errors at all when it happens, which seems to have happened after I switched from an HDMI display to a DVI-D display (it also seems to have become much more infrequent oddly enough?). Another interesting thing is that when I was using 4.18.12 or lower, I could avoid this problem entirely by flipping my VBIOS switch away from the IO ports. In addition, when it starts happening, if I reboot my system by just turning it off by holding down the power button and then turning it on normally, it will happen soon again after launching my WM. This is seemingly avoidable by completely disconnecting it from power, e.g. by turning my PSU off. This might actually be a completely unrelated bug, but the symptoms seem to fit enough, that it could be the same bug. It could perhaps also be a hardware bug, since it is very odd that the errors I get change, or maybe it is multiple bugs that seem to be the same? In addition, I can't find a definite way to reproduce my issue instantly other than just waiting for it to happen, although of course graphics intensive work does accelerate it considerably. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm: Add __printf verification
Add a few __printf attribute specifiers to routines that
could use them.
Signed-off-by: Joe Perches
---
drivers/gpu/drm/msm/msm_drv.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 9f51be5a637c..927e5d86f7c1 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -334,6 +334,7 @@ void msm_gem_kernel_put(struct drm_gem_object *bo,
struct drm_gem_object *msm_gem_import(struct drm_device *dev,
struct dma_buf *dmabuf, struct sg_table *sgt);
+__printf(2, 3)
void msm_gem_object_set_name(struct drm_gem_object *bo, const char *fmt, ...);
int msm_framebuffer_prepare(struct drm_framebuffer *fb,
@@ -397,12 +398,14 @@ void msm_framebuffer_describe(struct drm_framebuffer *fb,
struct seq_file *m);
int msm_debugfs_late_init(struct drm_device *dev);
int msm_rd_debugfs_init(struct drm_minor *minor);
void msm_rd_debugfs_cleanup(struct msm_drm_private *priv);
+__printf(3, 4)
void msm_rd_dump_submit(struct msm_rd_state *rd, struct msm_gem_submit *submit,
const char *fmt, ...);
int msm_perf_debugfs_init(struct drm_minor *minor);
void msm_perf_debugfs_cleanup(struct msm_drm_private *priv);
#else
static inline int msm_debugfs_late_init(struct drm_device *dev) { return 0; }
+__printf(3, 4)
static inline void msm_rd_dump_submit(struct msm_rd_state *rd, struct
msm_gem_submit *submit,
const char *fmt, ...) {}
static inline void msm_rd_debugfs_cleanup(struct msm_drm_private *priv) {}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.
https://bugs.freedesktop.org/show_bug.cgi?id=105733 --- Comment #56 from l...@protonmail.ch --- Also, forgot to mention, but the new GPU recovery thing doesn't work, and it would make the following error in dmesg: jan 16 16:43:26 las kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled seq=2792, emitted seq=2795 jan 16 16:43:26 las kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process pid 0 thread pid 0 jan 16 16:43:26 las kernel: amdgpu :08:00.0: GPU reset begin! jan 16 16:43:26 las kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=7013, emitted seq=7015 jan 16 16:43:26 las kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process sway pid 1328 thread sway:cs0 pid 1329 jan 16 16:43:26 las kernel: amdgpu :08:00.0: GPU reset begin! jan 16 16:43:26 las kernel: amdgpu: [powerplay] failed to send message 261 ret is 0 jan 16 16:43:27 las kernel: amdgpu: [powerplay] last message was failed ret is 0 jan 16 16:43:28 las kernel: amdgpu: [powerplay] failed to send message 261 ret is 0 lines 955-1010/1010 (END) (Fetched through `journalctl -ekb`) This however stopped once I switched to DVI-D, since I now get no errors at all. -- 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: Armada DRM: bridge with componentized devices
On Thursday, January 17, 2019 6:26:25 PM CET Russell King - ARM Linux admin wrote: > On Tue, Jan 15, 2019 at 11:47:00PM +0100, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > Subject: [PATCH] driver core: Fix adding device links to probing suppliers > > > > Currently, it is not valid to add a device link from a consumer > > driver ->probe() callback to a supplier that is still probing too, but > > generally this is a valid use case. For example, if the consumer has > > just acquired a resource that can only be available when the supplier > > is functional, adding a device link to that supplier right away > > should be safe (and even desirable arguably), but device_link_add() > > doesn't handle that case correctly and the initial state of the link > > created by it is wrong then. > > > > To address this problem, change the initial state of device links > > added between a probing supplier and a probing consumer to > > DL_STATE_CONSUMER_PROBE and update device_links_driver_bound() to > > skip such links on the supplier side. > > > > With this change, if the supplier probe completes first, > > device_links_driver_bound() called for it will skip the link state > > update and when it is called for the consumer, the link state will > > be updated to "active". In turn, if the consumer probe completes > > first, device_links_driver_bound() called for it will change the > > state of the link to "active" and when it is called for the > > supplier, the link status update will be skipped. > > > > However, in principle the supplier or consumer probe may still fail > > after the link has been added, so modify device_links_no_driver() to > > change device links in the "active" or "consumer probe" state to > > "dormant" on the supplier side and update __device_links_no_driver() > > to change the link state to "available" only if it is "consumer > > probe" or "active". > > > > Then, if the supplier probe fails first, the leftover link to the > > probing consumer will become "dormant" and device_links_no_driver() > > called for the consumer (when its probe fails) will clean it up. > > In turn, if the consumer probe fails first, it will either drop the > > link, or change its state to "available" and, in the latter case, > > when device_links_no_driver() is called for the supplier, it will > > update the link state to "dormant". [If the supplier probe fails, > > but the consumer probe succeeds, which should not happen as long as > > the consumer driver is correct, the link still will be around, but > > it will be "dormant" until the supplier is probed again.] > > > > Signed-off-by: Rafael J. Wysocki > > Hi Rafael, Hi, > I've tried this with Armada DRM without using the component helper for > bridges, and it seems to work up to a point (I have a vga bridge here > to allow further testing): > > [2.323441] armada-drm display-subsystem: assigned reserved memory node > framebuffer > [2.332001] armada-drm display-subsystem: bound f182.lcd-controller > (ops armada_lcd_ops) > [2.340790] armada-drm display-subsystem: bound f181.lcd-controller > (ops armada_lcd_ops) > [2.349345] armada-drm display-subsystem: > node=/i2c-mux/i2c@0/hdmi-encoder@70 > [2.356719] armada-drm display-subsystem: panel=fdfb bridge= (null) > ret=0 > [2.364038] armada-drm display-subsystem: Linked as a consumer to 1-0070 > [2.370818] armada-drm display-subsystem: bridge=ee8cda2c ret=0 > [2.376894] armada-drm display-subsystem: node=/vga-bridge > [2.382453] armada-drm display-subsystem: panel=fdfb bridge=(null) > ret=0 > [2.389762] armada-drm display-subsystem: Linked as a consumer to > vga-bridge > [2.396883] armada-drm display-subsystem: bridge=ef3bec40 ret=0 > > When I remove the HDMI encoder: > > root@cubox:/sys/bus/i2c/drivers/tda998x# echo 1-0070 > unbind > > then I get: > > [ 1013.824860] Console: switching to colour dummy device 80x30 > [ 1013.866785] armada-drm display-subsystem: Dropping the link to > vga-bridge > [ 1013.867126] armada-drm display-subsystem: Dropping the link to 1-0070 > > which looks like it did what was expected - the DRM device is indeed > unbound, the nodes in /dev/dri are gone. OK, thanks! This tells me that the patch is an improvement, so I'm going to move on to integrate it. > When rebinding the HDMI encoder: > > [ 1015.864703] tda998x 1-0070: found TDA19988 > [ 1015.898078] tda9950 1-0034: TDA9950 CEC interface, hardware version 3.3 > [ 1015.941374] Registered IR keymap rc-cec > [ 1015.941684] rc rc0: tda9950 as > /devices/platform/mbus/mbus:internal-regs/f1011000.i2c/i2c-0/i2c-1/1-0070/rc/rc0 > [ 1015.942439] input: tda9950 as > /devices/platform/mbus/mbus:internal-regs/f1011000.i2c/i2c-0/i2c-1/1-0070/rc/rc0/input2 > [ 1016.982921] alloc_contig_range: [10390, 10490) PFNs busy > [ 1016.987400] kirkwood-spdif-audio audio-subsystem: snd-soc-dummy-dai <-> > kirkwood-fe mapping ok > [ 1016.987591] kirkwood-spdif-audio audio-subsy
Re: [PATCH v5 1/2] drm/sched: Refactor ring mirror list handling.
On 01/17/2019 10:29 AM, Koenig, Christian wrote: Am 17.01.19 um 16:22 schrieb Grodzovsky, Andrey: On 01/17/2019 02:45 AM, Christian König wrote: Am 16.01.19 um 18:17 schrieb Grodzovsky, Andrey: On 01/16/2019 11:02 AM, Koenig, Christian wrote: Am 16.01.19 um 16:45 schrieb Grodzovsky, Andrey: On 01/16/2019 02:46 AM, Christian König wrote: Am 15.01.19 um 23:01 schrieb Grodzovsky, Andrey: On 01/11/2019 05:03 PM, Andrey Grodzovsky wrote: On 01/11/2019 02:11 PM, Koenig, Christian wrote: Am 11.01.19 um 16:37 schrieb Grodzovsky, Andrey: On 01/11/2019 04:42 AM, Koenig, Christian wrote: Am 10.01.19 um 16:56 schrieb Grodzovsky, Andrey: [SNIP] But we will not be adding the cb back in drm_sched_stop anymore, now we are only going to add back the cb in drm_sched_startr after rerunning those jobs in drm_sched_resubmit_jobs and assign them a new parent there anyway. Yeah, but when we find that we don't need to reset anything anymore then adding the callbacks again won't be possible any more. Christian. I am not sure I understand it, can u point me to example of how this will happen ? I am attaching my latest patches with waiting only for the last job's fence here just so we are on same page regarding the code. Well the whole idea is to prepare all schedulers, then check once more if the offending job hasn't completed in the meantime. If the job completed we need to be able to rollback everything and continue as if nothing had happened. Christian. Oh, but this piece of functionality - skipping HW ASIC reset in case the guilty job done is totally missing form this patch series and so needs to be added. So what you say actually is that for the case were we skip HW asic reset because the guilty job did complete we also need to skip resubmitting the jobs in drm_sched_resubmit_jobs and hence preserve the old parent fence pointer for reuse ? If so I would like to add all this functionality as a third patch since the first 2 patches are more about resolving race condition with jobs in flight while doing reset - what do you think ? Yeah, sounds perfectly fine to me. Christian. I realized there is another complication now for XGMI hive use case, we currently skip gpu recover for adev in case another gpu recover for different adev in same hive is running, under the assumption that we are going to reset all devices in hive anyway because that should cover our own dev too. But if we chose to skip now HW asic reset if our guilty job did finish we will aslo not HW reset any other devices in the hive even if one of them might actually had a bad job, wanted to do gpu recover but skipped it because our recover was in progress in that time. My general idea on that is to keep a list of guilty jobs per hive, when you start gpu recover you first add you guilty job to the hive and trylock hive->reset_lock. Any owner of hive->reset_lock (gpu recovery in progress) once he finished his recovery and released hive->reset_lock should go over hive->bad_jobs_list and if at least one of them is still not signaled (not done) trigger another gpu recovery and so on. If you do manage to trylock you also go over the list, clean it and perform recovery. This list probably needs to be protected with per hive lock. I also think we can for now at least finish reviewing the first 2 patches and submit them since as I said before they are not dealing with this topic and fixing existing race conditions. If you are OK with that I can send for review the last iteration of the first 2 patches where I wait for the last fence in ring mirror list. Andrey I implemented HW reset avoidance including XGMI use case according to the plan i specified. Patch is attached but I can't test it yet due to XGMI regression in PSP which is supposed to be fixed soon. Please take a look. Looks a bit too complicated on first glance. In general we should probably get away from handling a hive in any special way. Yes, I guess i can do it the same way as the generic handling in amdgpu_device_gpu_recover - there is a list of devices to process which is of size 1 for non xgmi use case or more then 1 for XGMI. Multiple timeout jobs in a hive are identical to multiple timeout jobs on different engines on a single device. How about the following handling: 1. Add the timeout job to a list. 2. Try to grab a lock to handle the reset, if that doesn't work because there is already a reset underway return immediately. 3. Stop all schedulers on all affected devices including stopping the timeouts and detaching all callbacks. 4. Double check the list of timed out jobs, if all hw fences of all jobs are completed now we actually don't need to do anything. What if all the jobs on the timed out list did complete but other job (or jobs) for which we removed the time out timer became hanged ? Wouldn't we miss a required reset in this case and wouldn't even have any indication of their hang ? If the timeout triggers before we disable it we will have the jo
