Re: [PATCH v5 3/3] PM/runtime:Replace jiffies based accounting with ktime based accounting
On Mon, 21 Jan 2019 at 23:53, Rafael J. Wysocki wrote: > > On Mon, Jan 21, 2019 at 4:17 PM Vincent Guittot > wrote: > > > > On Fri, 18 Jan 2019 at 13:08, Guenter Roeck wrote: > > > > > > On 1/18/19 3:05 AM, Rafael J. Wysocki wrote: > > > > On Fri, Jan 18, 2019 at 11:53 AM Vincent Guittot > > > > wrote: > > > >> > > > >> On Fri, 18 Jan 2019 at 11:42, Vincent Guittot > > > >> wrote: > > > >>> > > > >>> Hi Guenter, > > > >>> > > > >>> Le Thursday 17 Jan 2019 à 14:16:28 (-0800), Guenter Roeck a écrit : > > > On Fri, Dec 21, 2018 at 11:33:56AM +0100, Vincent Guittot wrote: > > > > From: Thara Gopinath > > > > > > > > This patch replaces jiffies based accounting for runtime_active_time > > > > and runtime_suspended_time with ktime base accounting. This makes > > > > the > > > > runtime debug counters inline with genpd and other pm subsytems > > > > which > > > > uses ktime based accounting. > > > > > > > > timekeeping is initialized before pm_runtime_init() so ktime_get() > > > > will > > > > be ready before first call. In fact, timekeeping_init() is called > > > > early > > > > in start_kernel() which is way before driver_init() (and that's when > > > > devices can start to be initialized) called from rest_init() via > > > > kernel_init_freeable() and do_basic_setup(). > > > > > > > This is not (always) correct. My qemu "collie" boot test fails with > > > this > > > patch applied. Reverting the patch fixes the problem. Bisect log > > > attached. > > > > > > >>> > > > >>> Can you try the patch below ? > > > >>> ktime_get_mono_fast_ns() has the advantage of being init with dummy > > > >>> clock so > > > >>> it can be used at early_init. > > > >> > > > >> Another possibility would be delay the init of the gpiochip > > > > > > > > Well, right. > > > > > > > > Initializing devices before timekeeping doesn't feel particularly > > > > robust from the design perspective. > > > > > > > > How exactly does that happen? > > > > > > > > > > With an added 'initialized' flag and backtrace into the timekeeping code, > > > with the change suggested earlier applied: > > > > > > [ cut here ] > > > WARNING: CPU: 0 PID: 0 at kernel/time/timekeeping.c:453 > > > ktime_get_mono_fast_ns+0x114/0x12c > > > Timekeeping not initialized > > > CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc2-next-20190117-dirty #2 > > > Hardware name: Sharp-Collie > > > Backtrace: > > > [] (dump_backtrace) from [] (show_stack+0x18/0x1c) > > > r7:0009 r6: r5:c065ba90 r4:c06d3e54 > > > [] (show_stack) from [] (dump_stack+0x20/0x28) > > > [] (dump_stack) from [] (__warn+0xcc/0xf4) > > > [] (__warn) from [] (warn_slowpath_fmt+0x4c/0x6c) > > > r8:df407b08 r7: r6:c0c01550 r5:c065bad8 r4:c06dd028 > > > [] (warn_slowpath_fmt) from [] > > > (ktime_get_mono_fast_ns+0x114/0x12c) > > > r3: r2:c065bad8 > > > r5: r4:df407b08 > > > [] (ktime_get_mono_fast_ns) from [] > > > (pm_runtime_init+0x38/0xb8) > > > r9:c06c9a5c r8:df407b08 r7: r6:c0c01550 r5: r4:df407b08 > > > [] (pm_runtime_init) from [] > > > (device_initialize+0xb0/0xec) > > > r7: r6:c0c01550 r5: r4:df407b08 > > > [] (device_initialize) from [] > > > (gpiochip_add_data_with_key+0x9c/0x884) > > > r7: r6:c06fca34 r5: r4: > > > [] (gpiochip_add_data_with_key) from [] > > > (sa1100_init_gpio+0x40/0x98) > > > r10:dfffcd60 r9:c06c9a5c r8:c06dd020 r7:c06dd028 r6: r5: > > > r4:c06fca34 > > > [] (sa1100_init_gpio) from [] > > > (sa1100_init_irq+0x2c/0x3c) > > > r7:c06dd028 r6: r5:c0713300 r4:c06e1070 > > > [] (sa1100_init_irq) from [] (init_IRQ+0x20/0x28) > > > r5:c0713300 r4: > > > [] (init_IRQ) from [] (start_kernel+0x254/0x4cc) > > > [] (start_kernel) from [<>] ( (null)) > > > r10:717f r9:6901b119 r8:c100 r7:0092 r6:313d r5:0053 > > > r4:c06a7330 > > > ---[ end trace 91e1bd00dd7cce32 ]--- > > > > Does it means that only the pm_runtime_init is done before > > timekeeping_init() but no update_pm_runtime_accounting() ? > > This platform calls device_initialize(), via sa1100_init_irq(), from > init_IRQ() which is in the start_kernel() code path before > timekeeping_init(). That's the initialization of structure fields > alone. > > Runtime PM really cannot be used legitimately before driver_init(), > because it needs bus types to be there at least. > > > In this case, we can keep using ktimeçget in > > update_pm_runtime_accounting() and find a solution to deal with > > early_call of pm_runtime_init() > > Given the above, I think that initializing accounting_timestamp in > pm_runtime_init() to anything different from 0 is a mistake. I agree > > Note that update_pm_runtime_accounting() ignores the delta value if > power.disable_depth is not zero anyway, so it really should be > sufficient to update accounti
Re: [PATCH libdrm] xf86drm: Add drmIsMaster()
On Tue, Jan 22, 2019 at 3:26 AM Christopher James Halse Rogers wrote: > > On 18 December 2018 7:07:01 pm NZDT, Christopher James Halse Rogers > wrote: >> >> On 18 December 2018 4:35:37 am AEDT, Emil Velikov >> wrote: >>> >>> Hi Christopher, >>> >>> On Tue, 20 Nov 2018 at 03:37, Christopher James Halse Rogers >>> wrote: We can't use drmSetMaster to query whether or not a drm fd is master because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd. >>> Can you please mention the exact use case here? You mentioned it over >>> IRC although it'll be nice to have it here for posterity. >> >> >> Certainly! >> >> The particular use-case I was hitting was testing my display server in a >> container, where container-root is not real-root but the implicit-master FD >> you get by opening the DRM node when there is no current master would be >> sufficient. >> >> Just assuming the FD we get is master and failing later breaks the platform >> probing; for example, when run under X11 if we don't check master we'll load >> the KMS platform and then fail, rather than noticing that KMS won't work and >> using our X11 backend. >> >> > Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect whether or not the fd is master. >>> I'm wondering if we cannot extent DRM_IOCTL_GET_CLIENT or another >>> IOCTL. >>> What do you think? May I interest you in writing an RFC for the >>> kernel-side? >> >> >> I think if I was going to do kernel-side changes if probably just make it so >> that IOCTL_SET_MASTER just unconditionally succeeded on an fd that was >> already master? Use-case seems all reasonable, I was waiting for a respin with my suggestion ... -Daniel >> >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > Ping! > > Is there anything more you'd like to know about my use-case here? Are there > any objections to adding drmIsMaster()? > ___ > 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/vkms: Fix license inconsistent
On Mon, Jan 21, 2019 at 06:58:57PM -0200, Rodrigo Siqueira wrote: > Fixes license inconsistent related to the VKMS driver and remove the > redundant boilerplate comment. > > Fixes: 854502fa0a38 ("drm/vkms: Add basic CRTC initialization") > > Signed-off-by: Rodrigo Siqueira > --- > drivers/gpu/drm/vkms/vkms_crc.c| 3 ++- > drivers/gpu/drm/vkms/vkms_crtc.c | 8 +--- > drivers/gpu/drm/vkms/vkms_drv.c| 7 +-- > drivers/gpu/drm/vkms/vkms_drv.h| 2 ++ > drivers/gpu/drm/vkms/vkms_gem.c| 8 +--- > drivers/gpu/drm/vkms/vkms_output.c | 8 +--- > drivers/gpu/drm/vkms/vkms_plane.c | 8 +--- > 7 files changed, 9 insertions(+), 35 deletions(-) This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 9/9] drm/mediatek: add dpi dual edge support
On Wed, 2019-01-09 at 17:58 +0100, Matthias Brugger wrote: > > On 04/01/2019 08:03, chunhui dai wrote: > > DPI sample on rising and falling edge. It can reduce half data io. > > > > Signed-off-by: Jitao Shi > > Signed-off-by: chunhui dai > > --- > > drivers/gpu/drm/mediatek/mtk_dpi.c | 30 ++ > > 1 file changed, 30 insertions(+) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c > > b/drivers/gpu/drm/mediatek/mtk_dpi.c > > index 4a2f4a6..acb4f47 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > > @@ -117,6 +117,7 @@ struct mtk_dpi_conf { > > unsigned int (*cal_factor)(int clock); > > u32 reg_h_fre_con; > > bool edge_sel_en; > > + bool dual_edge; > > }; > > > > static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 > > mask) > > @@ -353,6 +354,15 @@ static void mtk_dpi_config_disable_edge(struct mtk_dpi > > *dpi) > > mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN); > > } > > > > +static void mtk_dpi_config_dual_edge(struct mtk_dpi *dpi) > > I think it is clearer if you rename the function to something like: > mtk_dpi_enable_dual_edge and call it in mtk_dpi_set_display_mode if > dpi->conf->dual_edge is true. > > Regards, > Matthias > I'll fix it in next patch. Best Regards Jitao > > +{ > > + if (dpi->conf->dual_edge) { > > + mtk_dpi_mask(dpi, DPI_DDR_SETTING, DDR_EN | > > + DDR_4PHASE, DDR_EN | DDR_4PHASE); > > + mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, EDGE_SEL, EDGE_SEL); > > + } > > +} > > + > > static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, > > enum mtk_dpi_out_color_format format) > > { > > @@ -509,6 +519,7 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, > > mtk_dpi_config_color_format(dpi, dpi->color_format); > > mtk_dpi_config_2n_h_fre(dpi); > > mtk_dpi_config_disable_edge(dpi); > > + mtk_dpi_config_dual_edge(dpi); > > mtk_dpi_sw_reset(dpi, false); > > > > return 0; > > @@ -669,6 +680,16 @@ static unsigned int mt2701_calculate_factor(int clock) > > return 1; > > } > > > > +static unsigned int mt8183_calculate_factor(int clock) > > +{ > > + if (clock <= 27000) > > + return 8; > > + else if (clock <= 167000) > > + return 4; > > + else > > + return 2; > > +} > > + > > static const struct mtk_dpi_conf mt8173_conf = { > > .cal_factor = mt8173_calculate_factor, > > .reg_h_fre_con = 0xe0, > > @@ -680,6 +701,12 @@ static unsigned int mt2701_calculate_factor(int clock) > > .edge_sel_en = true, > > }; > > > > +static const struct mtk_dpi_conf mt8183_conf = { > > + .cal_factor = mt8183_calculate_factor, > > + .reg_h_fre_con = 0xe0, > > + .dual_edge = true, > > +}; > > + > > static int mtk_dpi_probe(struct platform_device *pdev) > > { > > struct device *dev = &pdev->dev; > > @@ -775,6 +802,9 @@ static int mtk_dpi_remove(struct platform_device *pdev) > > { .compatible = "mediatek,mt8173-dpi", > > .data = &mt8173_conf, > > }, > > + { .compatible = "mediatek,mt8183-dpi", > > + .data = &mt8183_conf, > > + }, > > { }, > > }; > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 9/9] drm/mediatek: add dpi dual edge support
Hi CK, Ok, I'll send it again in an independent patch. Best Regards Jitao On Wed, 2019-01-16 at 14:52 +0800, CK Hu (胡俊光) wrote: > Hi, Chunhui: > > > -Original Message- > > From: chunhui dai [mailto:chunhui@mediatek.com] > > Sent: Friday, January 04, 2019 3:04 PM > > To: --to=Michael Turquette; Stephen Boyd; CK Hu (胡俊光) > > Cc: Matthias Brugger; Philipp Zabel; David Airlie; Chunhui Dai (戴春晖); Sean > > Wang; Ryder Lee (李庚諺); Colin Ian King; linux-...@vger.kernel.org; > > linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > > linux-media...@lists.infradead.org; dri-devel@lists.freedesktop.org; > > srv_heupstream; Bibby Hsieh (謝濟遠); JamesJJ Liao (廖建智); Jitao Shi (石记 > > 涛) > > Subject: [PATCH 9/9] drm/mediatek: add dpi dual edge support > > > > DPI sample on rising and falling edge. It can reduce half data io. > > This patch looks like a patch for MT8183. For MT8173 and MT2701, dual_edge is > false. > For now, we have not support MT8183 yet. > So you could just set dual_edge to false and remove MT8183 part in this patch. > You could send the MT8183 part in an independent patch, not in a series of > MT2701. > > Regards, > CK > > > > > Signed-off-by: Jitao Shi > > Signed-off-by: chunhui dai > > --- > > drivers/gpu/drm/mediatek/mtk_dpi.c | 30 > > ++ > > 1 file changed, 30 insertions(+) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c > > b/drivers/gpu/drm/mediatek/mtk_dpi.c > > index 4a2f4a6..acb4f47 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > > @@ -117,6 +117,7 @@ struct mtk_dpi_conf { > > unsigned int (*cal_factor)(int clock); > > u32 reg_h_fre_con; > > bool edge_sel_en; > > + bool dual_edge; > > }; > > > > static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 > > mask) > > @@ -353,6 +354,15 @@ static void mtk_dpi_config_disable_edge(struct > > mtk_dpi *dpi) > > mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN); } > > > > +static void mtk_dpi_config_dual_edge(struct mtk_dpi *dpi) { > > + if (dpi->conf->dual_edge) { > > + mtk_dpi_mask(dpi, DPI_DDR_SETTING, DDR_EN | > > + DDR_4PHASE, DDR_EN | DDR_4PHASE); > > + mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, EDGE_SEL, EDGE_SEL); > > + } > > +} > > + > > static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, > > enum mtk_dpi_out_color_format format) > > { @@ > > -509,6 +519,7 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, > > mtk_dpi_config_color_format(dpi, dpi->color_format); > > mtk_dpi_config_2n_h_fre(dpi); > > mtk_dpi_config_disable_edge(dpi); > > + mtk_dpi_config_dual_edge(dpi); > > mtk_dpi_sw_reset(dpi, false); > > > > return 0; > > @@ -669,6 +680,16 @@ static unsigned int mt2701_calculate_factor(int > > clock) > > return 1; > > } > > > > +static unsigned int mt8183_calculate_factor(int clock) { > > + if (clock <= 27000) > > + return 8; > > + else if (clock <= 167000) > > + return 4; > > + else > > + return 2; > > +} > > + > > static const struct mtk_dpi_conf mt8173_conf = { > > .cal_factor = mt8173_calculate_factor, > > .reg_h_fre_con = 0xe0, > > @@ -680,6 +701,12 @@ static unsigned int mt2701_calculate_factor(int > > clock) > > .edge_sel_en = true, > > }; > > > > +static const struct mtk_dpi_conf mt8183_conf = { > > + .cal_factor = mt8183_calculate_factor, > > + .reg_h_fre_con = 0xe0, > > + .dual_edge = true, > > +}; > > + > > static int mtk_dpi_probe(struct platform_device *pdev) { > > struct device *dev = &pdev->dev; > > @@ -775,6 +802,9 @@ static int mtk_dpi_remove(struct platform_device > > *pdev) > > { .compatible = "mediatek,mt8173-dpi", > > .data = &mt8173_conf, > > }, > > + { .compatible = "mediatek,mt8183-dpi", > > + .data = &mt8183_conf, > > + }, > > { }, > > }; > > > > -- > > 1.9.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109217] clock management is disabled for the 4K resolution with polaris 10
https://bugs.freedesktop.org/show_bug.cgi?id=109217 fin4...@hotmail.com changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201991] amdgpu: clock management is disabled for the 4K resolution with polaris 10
https://bugzilla.kernel.org/show_bug.cgi?id=201991 fin4...@hotmail.com changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |CODE_FIX -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH/RFC] drm: Remove unused Renesas SH Mobile DRM driver
Hello, On Mon, Jan 21, 2019 at 11:18:26AM +0100, Daniel Vetter wrote: > On Mon, Jan 21, 2019 at 11:03:44AM +0100, Geert Uytterhoeven wrote: > > On Mon, Jan 21, 2019 at 10:35 AM Daniel Vetter wrote: > >> On Fri, Jan 18, 2019 at 05:22:58PM +0100, Geert Uytterhoeven wrote: > >>> Since its incarnation in v3.7 almost 7 years ago, no users of the SH > >>> Mobile DRM driver have appeared. > >>> > >>> Hence remove the driver. It can be resurrected from git history, > >>> if/when needed. > >>> > >>> Signed-off-by: Geert Uytterhoeven > >> > >> I'd prefer removing the fbdev variant tbh ... > > > > I understand ;-) > > > > But sh_mobile_lcdc_fb is used on 5 legacy SuperH platforms, and 1 ARM > > platform. At least the latter is working fine. > > > >> Not sure why exactly the switch never happened. > > > > Me neither. I^HGoogle can't even find posted patches of board integration. > > > > Note that both drivers lack DT support, which would be very desirable on > > ARM, as graphics is the single remaining working driver on the Atmark > > Techno Armadillo 800 EVA board not using DT. > > Ah I wondered why the drm driver isn't just automatically picked up in > today's neat world of DT (if you enable it in .config instead of the fbdev > one). It's not even using DT yet :-/ I'd prefer dropping the fbdev driver too, but that would require porting the SH boards to the DRM driver, and implement DT support for the ARM board. I don't think anyone is willing to invest time in this, so I'm fine dropping this driver as the boards are pretty much dead. I would, however, also drop the fbdev driver in that case. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] xf86drm: Add drmIsMaster()
On 18 December 2018 7:07:01 pm NZDT, Christopher James Halse Rogers wrote: >On 18 December 2018 4:35:37 am AEDT, Emil Velikov > wrote: >>Hi Christopher, >> >>On Tue, 20 Nov 2018 at 03:37, Christopher James Halse Rogers >> wrote: >>> >>> We can't use drmSetMaster to query whether or not a drm fd is master >>> because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd. >>> >>Can you please mention the exact use case here? You mentioned it over >>IRC although it'll be nice to have it here for posterity. > >Certainly! > >The particular use-case I was hitting was testing my display server in >a container, where container-root is not real-root but the >implicit-master FD you get by opening the DRM node when there is no >current master would be sufficient. > >Just assuming the FD we get is master and failing later breaks the >platform probing; for example, when run under X11 if we don't check >master we'll load the KMS platform and then fail, rather than noticing >that KMS won't work and using our X11 backend. > >> >>> Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is >>> DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect >>> whether or not the fd is master. >>> >>I'm wondering if we cannot extent DRM_IOCTL_GET_CLIENT or another >>IOCTL. >>What do you think? May I interest you in writing an RFC for the >>kernel-side? > >I think if I was going to do kernel-side changes if probably just make >it so that IOCTL_SET_MASTER just unconditionally succeeded on an fd >that was already master? >___ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel Ping! Is there anything more you'd like to know about my use-case here? Are there any objections to adding drmIsMaster()?___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] drm/msm: Cleanup A6XX opp-level reading
Hi Douglas, Thank you for the patch! Yet something to improve: [auto build test ERROR on v5.0-rc2] [also build test ERROR on next-20190116] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/drm-msm-Fix-A6XX-support-for-opp-level/20190118-042538 config: arm-imx_v6_v7_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 8.2.0-11) 8.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=8.2.0 make.cross ARCH=arm All errors (new ones prefixed by >>): drivers/gpu/drm/msm/adreno/a6xx_gmu.c: In function 'a6xx_gmu_get_arc_level': >> drivers/gpu/drm/msm/adreno/a6xx_gmu.c:944:8: error: implicit declaration of >> function 'dev_pm_opp_get_level'; did you mean 'dev_pm_opp_get_freq'? >> [-Werror=implicit-function-declaration] val = dev_pm_opp_get_level(opp); ^~~~ dev_pm_opp_get_freq cc1: some warnings being treated as errors vim +944 drivers/gpu/drm/msm/adreno/a6xx_gmu.c 929 930 /* Return the 'arc-level' for the given frequency */ 931 static unsigned int a6xx_gmu_get_arc_level(struct device *dev, 932 unsigned long freq) 933 { 934 struct dev_pm_opp *opp; 935 unsigned int val; 936 937 if (!freq) 938 return 0; 939 940 opp = dev_pm_opp_find_freq_exact(dev, freq, true); 941 if (IS_ERR(opp)) 942 return 0; 943 > 944 val = dev_pm_opp_get_level(opp); 945 946 dev_pm_opp_put(opp); 947 948 return val; 949 } 950 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/6] dt-bindings: display: armada: Add display subsystem binding
On Mon, Jan 21, 2019 at 11:53 AM Russell King - ARM Linux admin wrote: > > On Mon, Jan 21, 2019 at 10:07:11AM -0600, Rob Herring wrote: > > On Mon, Jan 21, 2019 at 9:46 AM Lubomir Rintel wrote: > > > > > > On Mon, 2019-01-21 at 09:35 -0600, Rob Herring wrote: > > > > On Sun, Jan 20, 2019 at 11:26 AM Lubomir Rintel wrote: > > > > > The Marvell Armada DRM master device is a virtual device needed to > > > > > list all > > > > > nodes that comprise the graphics subsystem. > > > > > > > > > > Signed-off-by: Lubomir Rintel > > > > > --- > > > > > .../display/armada/marvell-armada-drm.txt | 24 > > > > > +++ > > > > > 1 file changed, 24 insertions(+) > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > > > > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > > index de4cca9432c8..3dbfa8047f0b 100644 > > > > > --- > > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > > +++ > > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > > @@ -1,3 +1,27 @@ > > > > > +Marvell Armada DRM master device > > > > > + > > > > > + > > > > > +The Marvell Armada DRM master device is a virtual device needed to > > > > > list all > > > > > +nodes that comprise the graphics subsystem. > > > > > + > > > > > +Required properties: > > > > > + > > > > > + - compatible: value should be "marvell,dove-display-subsystem", > > > > > + "marvell,armada-display-subsystem" > > > > > + - ports: a list of phandles pointing to display interface ports of > > > > > CRTC > > > > > + devices > > > > > + - memory-region: phandle to a node describing memory to be used for > > > > > the > > > > > + framebuffer > > > > > + > > > > > +Example: > > > > > + > > > > > + display-subsystem { > > > > > + compatible = "marvell,dove-display-subsystem", > > > > > +"marvell,armada-display-subsystem"; > > > > > + memory-region = <&display_reserved>; > > > > > + ports = <&lcd0_port>; > > > > > > > > If there is only one device, you don't need this virtual node. > > > > > > By "one device" you mean one LCD controller (CRTC)? > > > > Yes. > > How does that work (as far as the Linux implementation) ? I can't see > a way that could work, while allowing the flexibility that Armada DRM > allows (two completely independent LCD controllers as two separate DRM > devices vs one DRM device containing both LCD controllers.) > > > > I suppose in the (single CRTC) example case, the display-subsystem node > > > used to associate it with the memory region reserved for allocating the > > > frame buffers from. Could that be done differently? > > > > Move memory-region to the LCD controller node. > > That doesn't work - it would appear in the wrong part of the driver. Why? You can fetch properties from other nodes. If you have 2 CRTCs, do you have 1 or 2 reserved memory regions? I'd think 2 with each one in the corresponding LCDC that uses them would be more flexible. Or just get the data out of the /reserved-memory node directly. Surely it has a compatible that you can find it with. > > > Also, if the node is indeed made optional, then it's going to > > > complicate things on the DRM side. Currently the driver that binds to > > > the node creates the DRM device once it sees all the components > > > connected to the ports appear. If we loose it, then the LCD controller > > > driver would somehow need to find out that it's alone and create the > > > DRM device itself. > > > > DT is not the only way to create devices. The DRM driver can bind to > > the LCDC node and then create a child CRTC device (or even multiple > > ones for h/w with multiple pipelines). > > That seems completely upside down and rediculous to me - are you > really suggesting that we should have some kind of virtual device > in DT, and omit the _real_ physical devices for that, having the > driver create the device with all the appropriate SoC resources? We create child platform devices that inherit from the parent in DT all the time. MFD child drivers are a common case. Sometime the child devices have DT nodes and sometimes they don't. Otherwise, do it the other way around. Create a virtual DRM device conditioned on the SoC: if (of_machine_is_compatible("foo,bar")) platform_device_register_simple(...) > > > You'll also notice that there are only 3 cases of this virtual node in > > the tree: STi, i.MX IPU, and Rockchip. That's because we've deprecated > > doing these virtual nodes for some time now. IOW, there are several > > examples of how to do this without a virtual node. > > This driver has been in-tree with this setup for some time, although > the documentation has been missing (we actually have a _lot_ of > instances of that.) However, we have no in-tree DT using it. The current Ar
[PATCH] dma-buf: Enhance dma-fence tracing
Rather than every backend and GPU driver reinventing the same wheel for user level debugging of HW execution, the common dma-fence framework should include the tracing infrastructure required for most client API level flow visualisation. With these common dma-fence level tracepoints, the userspace tools can establish a detailed view of the client <-> HW flow across different kernels. There is a strong ask to have this available, so that the userspace developer can effectively assess if they're doing a good job about feeding the beast of a GPU hardware. In the case of needing to look into more fine-grained details of how kernel internals work towards the goal of feeding the beast, the tools may optionally amend the dma-fence tracing information with the driver implementation specific. But for such cases, the tools should have a graceful degradation in case the expected extra tracepoints have changed or their format differs from the expected, as the kernel implementation internals are not expected to stay the same. It is important to distinguish between tracing for the purpose of client flow visualisation and tracing for the purpose of low-level kernel debugging. The latter is highly implementation specific, tied to a particular HW and driver, whereas the former addresses a common goal of user level tracing and likely a common set of userspace tools. Having made the distinction that these tracepoints will be consumed for client API tooling, we raise the spectre of tracepoint ABI stability. It is hoped that by defining a common set of dma-fence tracepoints, we avoid the pitfall of exposing low level details and so restrict ourselves only to the high level flow that is applicable to all drivers and hardware. Thus the reserved guarantee that this set of tracepoints will be stable (with the emphasis on depicting client <-> HW flow as opposed to driver <-> HW). In terms of specific changes to the dma-fence tracing, we remove the emission of the strings for every tracepoint (reserving them for dma_fence_init for cases where they have unique dma_fence_ops, and preferring to have descriptors for the whole fence context). strings do not pack as well into the ftrace ringbuffer and we would prefer to reduce the amount of indirect callbacks required for frequent tracepoint emission. Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Cc: Alex Deucher Cc: "Christian König" Cc: Eric Anholt Cc: Pierre-Loup Griffais Cc: Michael Sartain Cc: Steven Rostedt --- drivers/dma-buf/dma-fence.c | 9 +- drivers/gpu/drm/i915/i915_gem_clflush.c | 5 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 - drivers/gpu/drm/i915/i915_request.c | 16 +- drivers/gpu/drm/i915/i915_timeline.c| 5 + drivers/gpu/drm/i915/i915_trace.h | 134 --- drivers/gpu/drm/i915/intel_guc_submission.c | 10 ++ drivers/gpu/drm/i915/intel_lrc.c| 6 + drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + include/trace/events/dma_fence.h| 177 +++- 10 files changed, 214 insertions(+), 151 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 3aa8733f832a..5c93ed34b1ff 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -27,8 +27,15 @@ #define CREATE_TRACE_POINTS #include +EXPORT_TRACEPOINT_SYMBOL(dma_fence_context_create); +EXPORT_TRACEPOINT_SYMBOL(dma_fence_context_destroy); + +EXPORT_TRACEPOINT_SYMBOL(dma_fence_await); EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit); -EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal); +EXPORT_TRACEPOINT_SYMBOL(dma_fence_execute_start); +EXPORT_TRACEPOINT_SYMBOL(dma_fence_execute_end); +EXPORT_TRACEPOINT_SYMBOL(dma_fence_wait_start); +EXPORT_TRACEPOINT_SYMBOL(dma_fence_wait_end); static DEFINE_SPINLOCK(dma_fence_stub_lock); static struct dma_fence dma_fence_stub; diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c index 8e74c23cbd91..435c1303ecc8 100644 --- a/drivers/gpu/drm/i915/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c @@ -22,6 +22,8 @@ * */ +#include + #include "i915_drv.h" #include "intel_frontbuffer.h" #include "i915_gem_clflush.h" @@ -73,6 +75,7 @@ static void i915_clflush_work(struct work_struct *work) struct clflush *clflush = container_of(work, typeof(*clflush), work); struct drm_i915_gem_object *obj = clflush->obj; + trace_dma_fence_execute_start(&clflush->dma, smp_processor_id()); if (i915_gem_object_pin_pages(obj)) { DRM_ERROR("Failed to acquire obj->pages for clflushing\n"); goto out; @@ -83,6 +86,7 @@ static void i915_clflush_work(struct work_struct *work) i915_gem_object_unpin_pages(obj); out: + trace_dma_fence_execute_end(&clflush->dma, smp_processor_id()); i915_gem_object_put(obj); dma_fence_signal(&clflush->dma); @@ -97,6 +101,7 @@ i915_clf
Re: [PATCH 4/6] dt-bindings: display: armada: Add display subsystem binding
On Mon, Jan 21, 2019 at 09:45:22PM +0100, Lubomir Rintel wrote: > On Mon, 2019-01-21 at 17:53 +, Russell King - ARM Linux admin > wrote: > > On Mon, Jan 21, 2019 at 10:07:11AM -0600, Rob Herring wrote: > > > On Mon, Jan 21, 2019 at 9:46 AM Lubomir Rintel wrote: > > > > On Mon, 2019-01-21 at 09:35 -0600, Rob Herring wrote: > > > > > On Sun, Jan 20, 2019 at 11:26 AM Lubomir Rintel > > > > > wrote: > > > > > > The Marvell Armada DRM master device is a virtual device needed to > > > > > > list all > > > > > > nodes that comprise the graphics subsystem. > > > > > > > > > > > > Signed-off-by: Lubomir Rintel > > > > > > --- > > > > > > .../display/armada/marvell-armada-drm.txt | 24 > > > > > > +++ > > > > > > 1 file changed, 24 insertions(+) > > > > > > > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > > > > > > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > > > index de4cca9432c8..3dbfa8047f0b 100644 > > > > > > --- > > > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > > > +++ > > > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > > > @@ -1,3 +1,27 @@ > > > > > > +Marvell Armada DRM master device > > > > > > + > > > > > > + > > > > > > +The Marvell Armada DRM master device is a virtual device needed to > > > > > > list all > > > > > > +nodes that comprise the graphics subsystem. > > > > > > + > > > > > > +Required properties: > > > > > > + > > > > > > + - compatible: value should be "marvell,dove-display-subsystem", > > > > > > + "marvell,armada-display-subsystem" > > > > > > + - ports: a list of phandles pointing to display interface ports > > > > > > of CRTC > > > > > > + devices > > > > > > + - memory-region: phandle to a node describing memory to be used > > > > > > for the > > > > > > + framebuffer > > > > > > + > > > > > > +Example: > > > > > > + > > > > > > + display-subsystem { > > > > > > + compatible = "marvell,dove-display-subsystem", > > > > > > +"marvell,armada-display-subsystem"; > > > > > > + memory-region = <&display_reserved>; > > > > > > + ports = <&lcd0_port>; > > > > > > > > > > If there is only one device, you don't need this virtual node. > > > > > > > > By "one device" you mean one LCD controller (CRTC)? > > > > > > Yes. > > > > How does that work (as far as the Linux implementation) ? I can't see > > a way that could work, while allowing the flexibility that Armada DRM > > allows (two completely independent LCD controllers as two separate DRM > > devices vs one DRM device containing both LCD controllers.) > > > > > > I suppose in the (single CRTC) example case, the display-subsystem node > > > > used to associate it with the memory region reserved for allocating the > > > > frame buffers from. Could that be done differently? > > > > > > Move memory-region to the LCD controller node. > > > > That doesn't work - it would appear in the wrong part of the driver. > > > > > > Also, if the node is indeed made optional, then it's going to > > > > complicate things on the DRM side. Currently the driver that binds to > > > > the node creates the DRM device once it sees all the components > > > > connected to the ports appear. If we loose it, then the LCD controller > > > > driver would somehow need to find out that it's alone and create the > > > > DRM device itself. > > > > > > DT is not the only way to create devices. The DRM driver can bind to > > > the LCDC node and then create a child CRTC device (or even multiple > > > ones for h/w with multiple pipelines). > > > > That seems completely upside down and rediculous to me - are you > > really suggesting that we should have some kind of virtual device > > in DT, and omit the _real_ physical devices for that, having the > > driver create the device with all the appropriate SoC resources? > > Hmm, that's not how I read that. My understanding (putting aside > practicality of the solution) is that Rob was merely suggesting that > for the single LCDC case there would be just a single LCDC node in DT > and the driver that binds to it would create the DRM device & CRTC > device pair. How would we know that was the case when the driver binds to the CRTC node? There is no back-link from the CRTC to the display-subsystem when there's a display-subsystem node present, so there's no way for the CRTC driver to know whether it should create the DRM device or not. I just can't see how this works at a technical level. > > > You'll also notice that there are only 3 cases of this virtual node in > > > the tree: STi, i.MX IPU, and Rockchip. That's because we've deprecated > > > doing these virtual nodes for some time now. IOW, there are several > > > examples of how to do this without a virtual node. > > > > Thi
[Bug 109370] [Runelite GPU plugin] Enabling GPU plugin produces incorrect rendering
https://bugs.freedesktop.org/show_bug.cgi?id=109370 --- Comment #2 from MIka R --- Created attachment 143181 --> https://bugs.freedesktop.org/attachment.cgi?id=143181&action=edit Apitrace AMD RX 480 -- 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 v5 3/3] PM/runtime:Replace jiffies based accounting with ktime based accounting
On Mon, Jan 21, 2019 at 4:17 PM Vincent Guittot wrote: > > On Fri, 18 Jan 2019 at 13:08, Guenter Roeck wrote: > > > > On 1/18/19 3:05 AM, Rafael J. Wysocki wrote: > > > On Fri, Jan 18, 2019 at 11:53 AM Vincent Guittot > > > wrote: > > >> > > >> On Fri, 18 Jan 2019 at 11:42, Vincent Guittot > > >> wrote: > > >>> > > >>> Hi Guenter, > > >>> > > >>> Le Thursday 17 Jan 2019 à 14:16:28 (-0800), Guenter Roeck a écrit : > > On Fri, Dec 21, 2018 at 11:33:56AM +0100, Vincent Guittot wrote: > > > From: Thara Gopinath > > > > > > This patch replaces jiffies based accounting for runtime_active_time > > > and runtime_suspended_time with ktime base accounting. This makes the > > > runtime debug counters inline with genpd and other pm subsytems which > > > uses ktime based accounting. > > > > > > timekeeping is initialized before pm_runtime_init() so ktime_get() > > > will > > > be ready before first call. In fact, timekeeping_init() is called > > > early > > > in start_kernel() which is way before driver_init() (and that's when > > > devices can start to be initialized) called from rest_init() via > > > kernel_init_freeable() and do_basic_setup(). > > > > > This is not (always) correct. My qemu "collie" boot test fails with > > this > > patch applied. Reverting the patch fixes the problem. Bisect log > > attached. > > > > >>> > > >>> Can you try the patch below ? > > >>> ktime_get_mono_fast_ns() has the advantage of being init with dummy > > >>> clock so > > >>> it can be used at early_init. > > >> > > >> Another possibility would be delay the init of the gpiochip > > > > > > Well, right. > > > > > > Initializing devices before timekeeping doesn't feel particularly > > > robust from the design perspective. > > > > > > How exactly does that happen? > > > > > > > With an added 'initialized' flag and backtrace into the timekeeping code, > > with the change suggested earlier applied: > > > > [ cut here ] > > WARNING: CPU: 0 PID: 0 at kernel/time/timekeeping.c:453 > > ktime_get_mono_fast_ns+0x114/0x12c > > Timekeeping not initialized > > CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc2-next-20190117-dirty #2 > > Hardware name: Sharp-Collie > > Backtrace: > > [] (dump_backtrace) from [] (show_stack+0x18/0x1c) > > r7:0009 r6: r5:c065ba90 r4:c06d3e54 > > [] (show_stack) from [] (dump_stack+0x20/0x28) > > [] (dump_stack) from [] (__warn+0xcc/0xf4) > > [] (__warn) from [] (warn_slowpath_fmt+0x4c/0x6c) > > r8:df407b08 r7: r6:c0c01550 r5:c065bad8 r4:c06dd028 > > [] (warn_slowpath_fmt) from [] > > (ktime_get_mono_fast_ns+0x114/0x12c) > > r3: r2:c065bad8 > > r5: r4:df407b08 > > [] (ktime_get_mono_fast_ns) from [] > > (pm_runtime_init+0x38/0xb8) > > r9:c06c9a5c r8:df407b08 r7: r6:c0c01550 r5: r4:df407b08 > > [] (pm_runtime_init) from [] > > (device_initialize+0xb0/0xec) > > r7: r6:c0c01550 r5: r4:df407b08 > > [] (device_initialize) from [] > > (gpiochip_add_data_with_key+0x9c/0x884) > > r7: r6:c06fca34 r5: r4: > > [] (gpiochip_add_data_with_key) from [] > > (sa1100_init_gpio+0x40/0x98) > > r10:dfffcd60 r9:c06c9a5c r8:c06dd020 r7:c06dd028 r6: r5: > > r4:c06fca34 > > [] (sa1100_init_gpio) from [] > > (sa1100_init_irq+0x2c/0x3c) > > r7:c06dd028 r6: r5:c0713300 r4:c06e1070 > > [] (sa1100_init_irq) from [] (init_IRQ+0x20/0x28) > > r5:c0713300 r4: > > [] (init_IRQ) from [] (start_kernel+0x254/0x4cc) > > [] (start_kernel) from [<>] ( (null)) > > r10:717f r9:6901b119 r8:c100 r7:0092 r6:313d r5:0053 > > r4:c06a7330 > > ---[ end trace 91e1bd00dd7cce32 ]--- > > Does it means that only the pm_runtime_init is done before > timekeeping_init() but no update_pm_runtime_accounting() ? This platform calls device_initialize(), via sa1100_init_irq(), from init_IRQ() which is in the start_kernel() code path before timekeeping_init(). That's the initialization of structure fields alone. Runtime PM really cannot be used legitimately before driver_init(), because it needs bus types to be there at least. > In this case, we can keep using ktimeçget in > update_pm_runtime_accounting() and find a solution to deal with > early_call of pm_runtime_init() Given the above, I think that initializing accounting_timestamp in pm_runtime_init() to anything different from 0 is a mistake. Note that update_pm_runtime_accounting() ignores the delta value if power.disable_depth is not zero anyway, so it really should be sufficient to update accounting_timestamp when enabling runtime PM - and I'm not sure why it is not updated in pm_runtime_enable() for that matter (that looks like a bug to me). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Split out drm_probe_helper.h
Hi Daniel et al. > > > > 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. I have now a few patches queued, but the result is not too pretty. I did the following: - For all files in include/drm/*.h the set of include files were adjusted to the minimum number of files required to make them build without any other files included first. Created one .c file for each .h file. Then included the .h file and adjusted to the minimal set of include files. In the process a lot of forwards were added. - Deleted drmP.h - Fixed build of a few drivers: sti, tilcdc, gma500, tve200, via Some observations: - Killing all the includes not needed in the headers files results in a a lot of extra changes. Examples: drm_modseset_helper_vtables.h is no longer included by anyone, so needs to be added in many files drm_atomic_state_helper.h is no longer included by anyone so likewise needs to be added in many files - It is very tedious to do this properly. The process I followed was: - delete / comment out all include files - add back the obvious from a quick scan of the code - build - fix - build - fix - build - fix ... - next file... - The result is errorprone as only the allyesconfig + allmodconfig variants are tested. But reallife configurations are more diverse. Current diffstat: 111 files changed, 771 insertions(+), 401 deletions(-) This is for the 5 drivers alone and not the header cleanup. So long story short - this is not good and not the way forward. I will try to come up with a few improvements to make the headers files selfcontained, but restricted to the changes that add forwards/include to avoid the chrunch in all the drivers. And then post for review a few patches to clean up some headers. If the cleanup gets a go I will try to persuade the introduction of these. This will include, but will not be limited to, the above mentioned drm_crtc_helper.h header file. For now too much time was already spent on this, so it is at the moment pushed back on my TODO list. This mail serve also as a kind of "where had I left", when/if I pick this up again. If there are anyone that knows some tooling that can help in the process of adjusting the header files I am all ears. Sam ___ 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 1/21/19 5:22 AM, Brian Starkey wrote: > Hi, > > Sorry for being a bit sporadic on this. I was out travelling last week > with little time for email. > > On Fri, Jan 18, 2019 at 11:16:31AM -0600, Andrew F. Davis wrote: >> On 1/17/19 7:11 PM, Liam Mark wrote: >>> On Thu, 17 Jan 2019, Andrew F. Davis wrote: >>> 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 i
[PATCH] drm/vkms: Fix license inconsistent
Fixes license inconsistent related to the VKMS driver and remove the redundant boilerplate comment. Fixes: 854502fa0a38 ("drm/vkms: Add basic CRTC initialization") Signed-off-by: Rodrigo Siqueira --- drivers/gpu/drm/vkms/vkms_crc.c| 3 ++- drivers/gpu/drm/vkms/vkms_crtc.c | 8 +--- drivers/gpu/drm/vkms/vkms_drv.c| 7 +-- drivers/gpu/drm/vkms/vkms_drv.h| 2 ++ drivers/gpu/drm/vkms/vkms_gem.c| 8 +--- drivers/gpu/drm/vkms/vkms_output.c | 8 +--- drivers/gpu/drm/vkms/vkms_plane.c | 8 +--- 7 files changed, 9 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c index 9d9e8146db90..d7b409a3c0f8 100644 --- a/drivers/gpu/drm/vkms/vkms_crc.c +++ b/drivers/gpu/drm/vkms/vkms_crc.c @@ -1,4 +1,5 @@ -// SPDX-License-Identifier: GPL-2.0 +// SPDX-License-Identifier: GPL-2.0+ + #include "vkms_drv.h" #include #include diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 177bbcb38306..eb56ee893761 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -1,10 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - */ +// SPDX-License-Identifier: GPL-2.0+ #include "vkms_drv.h" #include diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 2a16b86196dc..944908c486f3 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -1,9 +1,4 @@ -/* - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - */ +// SPDX-License-Identifier: GPL-2.0+ /** * DOC: vkms (Virtual Kernel Modesetting) diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index e4469cd3d254..81f1cfbeb936 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -1,3 +1,5 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + #ifndef _VKMS_DRV_H_ #define _VKMS_DRV_H_ diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c index 80311daed47a..138b0bb325cf 100644 --- a/drivers/gpu/drm/vkms/vkms_gem.c +++ b/drivers/gpu/drm/vkms/vkms_gem.c @@ -1,10 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - */ +// SPDX-License-Identifier: GPL-2.0+ #include diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 271a0eb9042c..4173e4f48334 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -1,10 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - */ +// SPDX-License-Identifier: GPL-2.0+ #include "vkms_drv.h" #include diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 418817600ad1..0e67d2d42f0c 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -1,10 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - */ +// SPDX-License-Identifier: GPL-2.0+ #include "vkms_drv.h" #include -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm: Add a debug print for drm_modeset_backoff()
From: Ville Syrjälä Logs can get confusing when some operations are done multiple times due to the ww mutex backoff. Add a debug print into drm_modeset_backoff() so that at least the reason for the odd looking logs will be obvious. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modeset_lock.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 81dd11901ffd..1277ff18d993 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -295,6 +295,8 @@ int drm_modeset_backoff(struct drm_modeset_acquire_ctx *ctx) { struct drm_modeset_lock *contended = ctx->contended; + DRM_DEBUG_KMS("Retrying to avoid deadlock\n"); + ctx->contended = NULL; if (WARN_ON(!contended)) -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm: Add debug prints for the various object lookup errors
From: Ville Syrjälä Only some of the drm mode object lookups have a corresponding debug print for the lookup failure. That makes logs a bit hard to parse when you can't see where the bad object ID is being used. Add a bunch more debug prints, and unify their appearance. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic_uapi.c | 5 + drivers/gpu/drm/drm_color_mgmt.c | 8 ++-- drivers/gpu/drm/drm_connector.c | 5 - drivers/gpu/drm/drm_crtc.c| 12 +++- drivers/gpu/drm/drm_encoder.c | 4 +++- drivers/gpu/drm/drm_framebuffer.c | 4 +++- drivers/gpu/drm/drm_mode_object.c | 17 ++--- drivers/gpu/drm/drm_plane.c | 13 + drivers/gpu/drm/drm_property.c| 12 +--- drivers/gpu/drm/drm_vblank.c | 8 ++-- 10 files changed, 66 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 9a1f41adfc67..06390307e5a3 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1321,11 +1321,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, obj = drm_mode_object_find(dev, file_priv, obj_id, DRM_MODE_OBJECT_ANY); if (!obj) { + DRM_DEBUG_ATOMIC("Unknown object ID %d\n", obj_id); ret = -ENOENT; goto out; } if (!obj->properties) { + DRM_DEBUG_ATOMIC("Object ID %d has no properties\n", +obj_id); drm_mode_object_put(obj); ret = -ENOENT; goto out; @@ -1352,6 +1355,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, prop = drm_mode_obj_find_prop_id(obj, prop_id); if (!prop) { + DRM_DEBUG_ATOMIC("Unknown property ID %d\n", +prop_id); drm_mode_object_put(obj); ret = -ENOENT; goto out; diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 07dcf47daafe..a99ee15b8328 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -245,8 +245,10 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev, return -EOPNOTSUPP; crtc = drm_crtc_find(dev, file_priv, crtc_lut->crtc_id); - if (!crtc) + if (!crtc) { + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_lut->crtc_id); return -ENOENT; + } if (crtc->funcs->gamma_set == NULL) return -ENOSYS; @@ -313,8 +315,10 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev, return -EOPNOTSUPP; crtc = drm_crtc_find(dev, file_priv, crtc_lut->crtc_id); - if (!crtc) + if (!crtc) { + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_lut->crtc_id); return -ENOENT; + } /* memcpy into gamma store */ if (crtc_lut->gamma_size != crtc->gamma_size) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 847539645558..8745eb132fd4 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1952,8 +1952,11 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo)); connector = drm_connector_lookup(dev, file_priv, out_resp->connector_id); - if (!connector) + if (!connector) { + DRM_DEBUG_KMS("Unknown connector ID %d\n", + out_resp->connector_id); return -ENOENT; + } drm_connector_for_each_possible_encoder(connector, encoder, i) encoders_count++; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 7dabbaf033a1..e5f234ffcd23 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -369,8 +369,10 @@ int drm_mode_getcrtc(struct drm_device *dev, return -EOPNOTSUPP; crtc = drm_crtc_find(dev, file_priv, crtc_resp->crtc_id); - if (!crtc) + if (!crtc) { + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_resp->crtc_id); return -ENOENT; + } plane = crtc->primary; @@ -586,8 +588,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, } else { fb = drm_framebuffer_lookup(dev, file_priv, crtc_req->fb_id); if (!fb) { - DRM_DEBUG_KMS("Unknown FB ID%d\n", - crtc_req->fb_id); + DRM_DEBUG_KMS("Unknown FB ID %d\n", + crtc_req->fb_id);
[PATCH 2/3] drm: Sync errno values for property lookup errors
From: Ville Syrjälä Use ENOENT consistently for the case where the requested property isn't found, and EINVAL for the case where the object has no properties whatsoever. Currenrly these are handled differently in the atomic and legacy codepaths. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic_uapi.c | 2 +- drivers/gpu/drm/drm_mode_object.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 06390307e5a3..2a54f826cf65 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1330,7 +1330,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, DRM_DEBUG_ATOMIC("Object ID %d has no properties\n", obj_id); drm_mode_object_put(obj); - ret = -ENOENT; + ret = -EINVAL; goto out; } diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index e8dac94d576d..31730d935842 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -527,6 +527,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, property = drm_mode_obj_find_prop_id(arg_obj, arg->prop_id); if (!property) { DRM_DEBUG_KMS("Unknown property ID %d\n", arg->prop_id); + ret = -ENOENT; goto out_unref; } -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes
On 1/21/19 2:20 PM, Liam Mark wrote: > On Mon, 21 Jan 2019, Andrew F. Davis wrote: > >> On 1/21/19 1:44 PM, Liam Mark wrote: >>> On Mon, 21 Jan 2019, Christoph Hellwig wrote: >>> On Sat, Jan 19, 2019 at 08:50:41AM -0800, Laura Abbott wrote: >> And who is going to decide which ones to pass? And who documents >> which ones are safe? >> >> I'd much rather have explicit, well documented dma-buf flags that >> might get translated to the DMA API flags, which are not error checked, >> not very well documented and way to easy to get wrong. >> > > I'm not sure having flags in dma-buf really solves anything > given drivers can use the attributes directly with dma_map > anyway, which is what we're looking to do. The intention > is for the driver creating the dma_buf attachment to have > the knowledge of which flags to use. Well, there are very few flags that you can simply use for all calls of dma_map*. And given how badly these flags are defined I just don't want people to add more places where they indirectly use these flags, as it will be more than enough work to clean up the current mess. What flag(s) do you want to pass this way, btw? Maybe that is where the problem is. >>> >>> The main use case is for allowing clients to pass in >>> DMA_ATTR_SKIP_CPU_SYNC in order to skip the default cache maintenance >>> which happens in dma_buf_map_attachment and dma_buf_unmap_attachment. In >>> ION the buffers aren't usually accessed from the CPU so this allows >>> clients to often avoid doing unnecessary cache maintenance. >>> >> >> How can a client know that no CPU access has occurred that needs to be >> flushed out? >> > > I have left this to clients, but if they own the buffer they can have the > knowledge as to whether CPU access is needed in that use case (example for > post-processing). > > For example with the previous version of ION we left all decisions of > whether cache maintenance was required up to the client, they would use > the ION cache maintenance IOCTL to force cache maintenance only when it > was required. > In these cases almost all of the access was being done by the device and > in the rare cases CPU access was required clients would initiate the > required cache maintenance before and after the CPU access. > I think we have different definitions of "client", I'm talking about the DMA-BUF client (the importer), that is who can set this flag. It seems you mean the userspace application, which has no control over this flag. > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes
On 1/21/19 1:44 PM, Liam Mark wrote: > On Mon, 21 Jan 2019, Christoph Hellwig wrote: > >> On Sat, Jan 19, 2019 at 08:50:41AM -0800, Laura Abbott wrote: And who is going to decide which ones to pass? And who documents which ones are safe? I'd much rather have explicit, well documented dma-buf flags that might get translated to the DMA API flags, which are not error checked, not very well documented and way to easy to get wrong. >>> >>> I'm not sure having flags in dma-buf really solves anything >>> given drivers can use the attributes directly with dma_map >>> anyway, which is what we're looking to do. The intention >>> is for the driver creating the dma_buf attachment to have >>> the knowledge of which flags to use. >> >> Well, there are very few flags that you can simply use for all calls of >> dma_map*. And given how badly these flags are defined I just don't want >> people to add more places where they indirectly use these flags, as >> it will be more than enough work to clean up the current mess. >> >> What flag(s) do you want to pass this way, btw? Maybe that is where >> the problem is. >> > > The main use case is for allowing clients to pass in > DMA_ATTR_SKIP_CPU_SYNC in order to skip the default cache maintenance > which happens in dma_buf_map_attachment and dma_buf_unmap_attachment. In > ION the buffers aren't usually accessed from the CPU so this allows > clients to often avoid doing unnecessary cache maintenance. > How can a client know that no CPU access has occurred that needs to be flushed out? > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
On 2019-01-21 7:28 p.m., Ard Biesheuvel wrote: > On Mon, 21 Jan 2019 at 19:24, Michel Dänzer wrote: >> On 2019-01-21 7:20 p.m., Ard Biesheuvel wrote: >>> On Mon, 21 Jan 2019 at 19:04, Michel Dänzer wrote: On 2019-01-21 6:59 p.m., Ard Biesheuvel wrote: > On Mon, 21 Jan 2019 at 18:55, Michel Dänzer wrote: >> On 2019-01-21 5:30 p.m., Ard Biesheuvel wrote: >>> On Mon, 21 Jan 2019 at 17:22, Christoph Hellwig >>> wrote: >>> Until that happens we should just change the driver ifdefs to default the hacks to off and only enable them on setups where we 100% positively know that they actually work. And document that fact in big fat comments. >>> >>> Well, as I mentioned in my commit log as well, if we default to off >>> unless CONFIG_X86, we may break working setups on MIPS and Power where >>> the device is in fact non-cache coherent, and relies on this >>> 'optimization' to get things working. >> >> FWIW, the amdgpu driver doesn't rely on non-snooped transfers for >> correct basic operation (the scenario Christian brought up is a very >> specialized use-case), so that shouldn't be an issue. > > The point is that this is only true for x86. > > On other architectures, the use of non-cached mappings on the CPU side > means that you /do/ rely on non-snooped transfers, since if those > transfers turn out not to snoop inadvertently, the accesses are > incoherent with the CPU's view of memory. The driver generally only uses non-cached mappings if drm_arch/device_can_wc_memory returns true. >>> >>> Indeed. And so we should take care to only return 'true' from that >>> function if it is guaranteed that non-cached CPU mappings are coherent >>> with the mappings used by the GPU, either because that is always the >>> case (like on x86), or because we know that the platform in question >>> implements NoSnoop correctly throughout the interconnect. >>> >>> What seems to be complicating matters is that in some cases, the >>> device is non-cache coherent to begin with, so regardless of whether >>> the NoSnoop attribute is used or not, those accesses will not snoop in >>> the caches and be coherent with the non-cached mappings used by the >>> CPU. So if we restrict this optimization [on non-X86] to platforms >>> that are known to implement NoSnoop correctly, we may break platforms >>> that are implicitly NoSnoop all the time. >> >> Since the driver generally doesn't rely on non-snooped accesses for >> correctness, that couldn't "break" anything that hasn't always been broken. > > Again, that is only true on x86. > > On other architectures, DMA writes from the device may allocate in the > caches, and be invisible to the CPU when it uses non-cached mappings. Let me try one last time: If drm_arch_can_wc_memory returns false, the driver falls back to the normal mode of operation, using a cacheable CPU mapping and snooped GPU transfers, even if userspace asks (as a performance optimization) for a write-combined CPU mapping and non-snooped GPU transfers via AMDGPU_GEM_CREATE_CPU_GTT_USWC. This normal mode of operation is also used for the ring buffers at the heart of the driver's operation. If there is a platform where this normal mode of operation doesn't work, the driver could never have worked reliably on that platform, since before AMDGPU_GEM_CREATE_CPU_GTT_USWC or drm_arch_can_wc_memory even existed. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201815] Mouse Pointer Disappears when touching top of the screen
https://bugzilla.kernel.org/show_bug.cgi?id=201815 siyia (eutychio...@gmail.com) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |PATCH_ALREADY_AVAILABLE --- Comment #15 from siyia (eutychio...@gmail.com) --- Finally!!! i applied the fix in kernel 4.20.3 and it works!!!thx i ve been waiting for a month for this -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
On 2019-01-21 7:20 p.m., Ard Biesheuvel wrote: > On Mon, 21 Jan 2019 at 19:04, Michel Dänzer wrote: >> >> On 2019-01-21 6:59 p.m., Ard Biesheuvel wrote: >>> On Mon, 21 Jan 2019 at 18:55, Michel Dänzer wrote: On 2019-01-21 5:30 p.m., Ard Biesheuvel wrote: > On Mon, 21 Jan 2019 at 17:22, Christoph Hellwig > wrote: > >> Until that happens we should just change the driver ifdefs to default >> the hacks to off and only enable them on setups where we 100% >> positively know that they actually work. And document that fact >> in big fat comments. > > Well, as I mentioned in my commit log as well, if we default to off > unless CONFIG_X86, we may break working setups on MIPS and Power where > the device is in fact non-cache coherent, and relies on this > 'optimization' to get things working. FWIW, the amdgpu driver doesn't rely on non-snooped transfers for correct basic operation (the scenario Christian brought up is a very specialized use-case), so that shouldn't be an issue. >>> >>> The point is that this is only true for x86. >>> >>> On other architectures, the use of non-cached mappings on the CPU side >>> means that you /do/ rely on non-snooped transfers, since if those >>> transfers turn out not to snoop inadvertently, the accesses are >>> incoherent with the CPU's view of memory. >> >> The driver generally only uses non-cached mappings if >> drm_arch/device_can_wc_memory returns true. >> > > Indeed. And so we should take care to only return 'true' from that > function if it is guaranteed that non-cached CPU mappings are coherent > with the mappings used by the GPU, either because that is always the > case (like on x86), or because we know that the platform in question > implements NoSnoop correctly throughout the interconnect. > > What seems to be complicating matters is that in some cases, the > device is non-cache coherent to begin with, so regardless of whether > the NoSnoop attribute is used or not, those accesses will not snoop in > the caches and be coherent with the non-cached mappings used by the > CPU. So if we restrict this optimization [on non-X86] to platforms > that are known to implement NoSnoop correctly, we may break platforms > that are implicitly NoSnoop all the time. Since the driver generally doesn't rely on non-snooped accesses for correctness, that couldn't "break" anything that hasn't always been broken. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
On 2019-01-21 6:59 p.m., Ard Biesheuvel wrote: > On Mon, 21 Jan 2019 at 18:55, Michel Dänzer wrote: >> >> On 2019-01-21 5:30 p.m., Ard Biesheuvel wrote: >>> On Mon, 21 Jan 2019 at 17:22, Christoph Hellwig wrote: >>> Until that happens we should just change the driver ifdefs to default the hacks to off and only enable them on setups where we 100% positively know that they actually work. And document that fact in big fat comments. >>> >>> Well, as I mentioned in my commit log as well, if we default to off >>> unless CONFIG_X86, we may break working setups on MIPS and Power where >>> the device is in fact non-cache coherent, and relies on this >>> 'optimization' to get things working. >> >> FWIW, the amdgpu driver doesn't rely on non-snooped transfers for >> correct basic operation (the scenario Christian brought up is a very >> specialized use-case), so that shouldn't be an issue. >> > > The point is that this is only true for x86. > > On other architectures, the use of non-cached mappings on the CPU side > means that you /do/ rely on non-snooped transfers, since if those > transfers turn out not to snoop inadvertently, the accesses are > incoherent with the CPU's view of memory. The driver generally only uses non-cached mappings if drm_arch/device_can_wc_memory returns true. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
On 2019-01-21 5:30 p.m., Ard Biesheuvel wrote: > On Mon, 21 Jan 2019 at 17:22, Christoph Hellwig wrote: > >> Until that happens we should just change the driver ifdefs to default >> the hacks to off and only enable them on setups where we 100% >> positively know that they actually work. And document that fact >> in big fat comments. > > Well, as I mentioned in my commit log as well, if we default to off > unless CONFIG_X86, we may break working setups on MIPS and Power where > the device is in fact non-cache coherent, and relies on this > 'optimization' to get things working. FWIW, the amdgpu driver doesn't rely on non-snooped transfers for correct basic operation (the scenario Christian brought up is a very specialized use-case), so that shouldn't be an issue. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/6] dt-bindings: display: armada: Add display subsystem binding
On Mon, Jan 21, 2019 at 10:07:11AM -0600, Rob Herring wrote: > On Mon, Jan 21, 2019 at 9:46 AM Lubomir Rintel wrote: > > > > On Mon, 2019-01-21 at 09:35 -0600, Rob Herring wrote: > > > On Sun, Jan 20, 2019 at 11:26 AM Lubomir Rintel wrote: > > > > The Marvell Armada DRM master device is a virtual device needed to list > > > > all > > > > nodes that comprise the graphics subsystem. > > > > > > > > Signed-off-by: Lubomir Rintel > > > > --- > > > > .../display/armada/marvell-armada-drm.txt | 24 +++ > > > > 1 file changed, 24 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > index de4cca9432c8..3dbfa8047f0b 100644 > > > > --- > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > +++ > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > > @@ -1,3 +1,27 @@ > > > > +Marvell Armada DRM master device > > > > + > > > > + > > > > +The Marvell Armada DRM master device is a virtual device needed to > > > > list all > > > > +nodes that comprise the graphics subsystem. > > > > + > > > > +Required properties: > > > > + > > > > + - compatible: value should be "marvell,dove-display-subsystem", > > > > + "marvell,armada-display-subsystem" > > > > + - ports: a list of phandles pointing to display interface ports of > > > > CRTC > > > > + devices > > > > + - memory-region: phandle to a node describing memory to be used for > > > > the > > > > + framebuffer > > > > + > > > > +Example: > > > > + > > > > + display-subsystem { > > > > + compatible = "marvell,dove-display-subsystem", > > > > +"marvell,armada-display-subsystem"; > > > > + memory-region = <&display_reserved>; > > > > + ports = <&lcd0_port>; > > > > > > If there is only one device, you don't need this virtual node. > > > > By "one device" you mean one LCD controller (CRTC)? > > Yes. How does that work (as far as the Linux implementation) ? I can't see a way that could work, while allowing the flexibility that Armada DRM allows (two completely independent LCD controllers as two separate DRM devices vs one DRM device containing both LCD controllers.) > > I suppose in the (single CRTC) example case, the display-subsystem node > > used to associate it with the memory region reserved for allocating the > > frame buffers from. Could that be done differently? > > Move memory-region to the LCD controller node. That doesn't work - it would appear in the wrong part of the driver. > > Also, if the node is indeed made optional, then it's going to > > complicate things on the DRM side. Currently the driver that binds to > > the node creates the DRM device once it sees all the components > > connected to the ports appear. If we loose it, then the LCD controller > > driver would somehow need to find out that it's alone and create the > > DRM device itself. > > DT is not the only way to create devices. The DRM driver can bind to > the LCDC node and then create a child CRTC device (or even multiple > ones for h/w with multiple pipelines). That seems completely upside down and rediculous to me - are you really suggesting that we should have some kind of virtual device in DT, and omit the _real_ physical devices for that, having the driver create the device with all the appropriate SoC resources? > You'll also notice that there are only 3 cases of this virtual node in > the tree: STi, i.MX IPU, and Rockchip. That's because we've deprecated > doing these virtual nodes for some time now. IOW, there are several > examples of how to do this without a virtual node. This driver has been in-tree with this setup for some time, although the documentation has been missing (we actually have a _lot_ of instances of that.) However, we have no in-tree DT using it. I don't really see how to satisfy your comments without totally restructuring the driver, which is going to be quite a big chunk of work. I'm not sure I have the motivation to do that right now. -- 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 1/2] drm: Add DRM_DEV_INFO_RATELIMITED
Hi Kristian, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.0-rc2 next-20190116] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Kristian-H-Kristensen/drm-Add-DRM_DEV_INFO_RATELIMITED/20190121-162608 reproduce: make htmldocs All warnings (new ones prefixed by >>): net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.avg_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info' kernel/rcu/tree.c:711: warning: Excess function parameter 'irq' description in 'rcu_nmi_exit' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf' include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array' include/linux/firmware/intel/stratix10-svc-client.h:1: warning: no structured comments found include/linux/gpio/driver.h:371: warning: Function parameter or member 'init_valid_mask' not described in 'gpio_chip' include/linux/iio/hw-consumer.h:1: warning: no structured comments found include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry' drivers/mtd/nand/raw/nand_base.c:420: warning: Function parameter or member 'chip' not described in 'nand_fill_oob' drivers/mtd/nand/raw/nand_bbt.c:173: warning: Function parameter or member 'this' not described in 'read_bbt' drivers/mtd/nand/raw/nand_bbt.c:173: warning: Excess function parameter 'chip' description in 'read_bbt' include/linux/regulator/machine.h:199: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints' include/linux/regulator/driver.h:228: warning: Function parameter or member 'resume' not described in 'regulator_ops' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb' drivers/slimbus/stream.c:1: warning: no structured comments found include/linux/spi/spi.h:180: warning: Function parameter or member 'driver_override' not described in 'spi_device' drivers/target/target_core_device.c:1: warning: no structured comments found drivers/usb/typec/bus.c:1: warning: no structured comments found drivers/usb/typec/class.c:1: warning: no structured comments found include/linux/w1.h:281: warning: Function parameter or member 'of_match_table' not described in 'w1_family' fs/direct-io.c:257: warning: Excess function parameter 'offset' description in 'dio_complete' fs/file_table.c:1: warning: no structured comments found fs/libfs.c:477: warning: Excess function parameter 'available' description in
Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory
On Mon, Jan 21, 2019 at 12:54 PM Brian Starkey wrote: > > Hi Daniel, > > 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. 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 > > > > I share a similar view to Liviu here. I think this new requirement > raises the bar more than you intended. > > By saying that all new features must be tested by igt, you're also > implying that a driver must run igt (at some basic level); before the > developers working on that driver can start trying to implement new > features. That puts an additional hurdle in the way of adding stuff > to KMS for people who aren't already using igt. > > I'm all for testing, and UAPI being well proven before we merge it, > and even for a central KMS test suite. However, when we (Arm Mali-DP > people) have tried to implement things in igt it's been a battle, > because of various built-in assumptions which it made. > > For example, most meaningful igt tests rely on CRC. Much of our HW > doesn't have CRC. CRC could be implemented in theory using writeback, > but that currently doesn't exist. That means you're effectively saying > that we (Arm) can't implement any new cross-device KMS features until > we've either: > > a) also implemented writeback-based CRC in igt OR > b) implemented the new feature in someone else's driver which does > support CRC. We didn't just pick crcs for lols (or because that's all intel supports), we picked it because it will work for both hw with crc and hw with writeback. I checked with a pile of driver writers way back (over irc), and the interface we picked is something pretty much all display blocks (except the _very_ simple ones) should be able to support. Same discussion also happened again when made the crc interfaces in debugfs more generic. > That seems a bit out of order to me. It would be like me saying "all > KMS drivers must use Arm's test suite, which uses writeback and pixel > checking", and you'd be in a pickle because you don't have writeback. > > In a similar vein, I remember having to fix igt on devices which > didn't have cursor planes, before I
Re: [linux-sunxi] Re: HDMI/DVI spurious failure
On Tue, Jan 22, 2019 at 1:18 AM Jernej Škrabec wrote: > > Dne ponedeljek, 21. januar 2019 ob 16:07:28 CET je Priit Laes napisal(a): > > On Mon, Jan 21, 2019 at 02:25:17PM +0100, Maxime Ripard wrote: > > > On Fri, Jan 18, 2019 at 02:51:26PM +, Priit Laes wrote: > > > > On Fri, Jan 18, 2019 at 03:04:18PM +0100, Maxime Ripard wrote: > > > > > On Fri, Jan 18, 2019 at 10:10:53AM +, Priit Laes wrote: > > > > > > > > > > > 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? > > > > > > > > > > > > Can't really test this display without this patch and I do not have > > > > > > other > > > > > > HDMI/DVI screens. And this issue does not happen with other HDMI > > > > > > displays > > > > > > that I have here. > > > > > > > > > > Can't you just force the monitor to be reported as present? It's not > > > > > great and we don't want to merge it, but that would allow you to test > > > > > that setup without too many interferences. > > > > > > > > Baseline is clean u-boot / linux. U-boot does not detect/enable display. > > > > > > > > 1) Booting Linux with drm.debug=0xe > > > > > > > > * Linux does not detect/enable display > > > > > > > > 2) Booting with drm.debug=0xe video=HDMI-A-1:640x480@60e > > > > > > > > * Linux detects display, but display is garbled, and proper edid data is > > > > detected: > > > > > > > > [snip] > > > > pll-video1 000 32700 > > > > 0 0 5> > > > > >pll-video1-2x 000 65400 > > > >0 0 5> > > > > > hdmi-tmds00025153846 > > > > 0 0 5> > > > > > hdmi-ddc 000 89835 > > > > 0 0 5> > > > > > [/snip] > > > > > > > > 3) Booting with drm.debug=0xe video=HDMI-A-1:640x480@60e > > > > And also one extra patch for Linux where HDMI DDC clock marked as > > > > critical > > > > > > > > Linux detects and initializes display properly: > > > > [snip] > > > > pll-video1 110 32700 > > > > 0 0 5> > > > > >pll-video1-2x 110 65400 > > > >0 0 5> > > > > > hdmi-tmds11025153846 > > > > 0 0 5> > > > > > hdmi-ddc 110 89835 > > > > 0 0 5> > > > > > [/snip] > > > > > > I guess you'll need to track down when the hdmi-tmds and hdmi-ddc are > > > enabled and disabled, and if it makes sense :/ > > > > OK, figured out the cause. > > > > Apparently, for each ddc poll we enable ddc clock which is a child of TMDS > > clock. After transfer is done, we disable the clock and this also tears down > > the parent because its only user has gone missing.. :( > > > > > > So basically, patch below also works, but I guess we should override > > the sun4i_tmds_ops.disable to properly account for
Re: [PATCH] drm/etnaviv: don't restrict to certain architectures
On Wed, Jan 16, 2019 at 04:27:58PM +0100, Lucas Stach wrote: > The Vivante GPU cores are found in many different SoCs and the driver > does not depend on anything architecture specific, so just drop the > architecture restriction. With my Debian kernel team member hat on I don't like changes like this. I don't know in which machines the etnaviv IP is available. Should the Debian kernel enable it on x86? powerpc? riscv? If MXC + DOVE isn't enough, it's your opportunity to not waste time of people who don't know etnaviv by heart and only expand the dependency carefully. Having said that I see the benefit of being able to enable the driver on a wide variety of machines. It's flexible and everyone who doesn't want that driver can still just disable it. But when making this change, please consider also all the people who will see ETNAVIV (DRM support for Vivante GPU IP cores) (DRM_ETNAVIV) [N/y/m/?] (NEW) in their next run of make oldconfig and need to decide if this driver is useful for them. Also note they only see DRM driver for Vivante GPUs. when pressing '?'. Best regards 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
[Bug 102646] Screen flickering under amdgpu-experimental [buggy auto power profile]
https://bugs.freedesktop.org/show_bug.cgi?id=102646 --- Comment #65 from bmil...@gmail.com --- Please look at this devs, still no solution in mind? -- 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 4/6] dt-bindings: display: armada: Add display subsystem binding
On Mon, Jan 21, 2019 at 9:46 AM Lubomir Rintel wrote: > > On Mon, 2019-01-21 at 09:35 -0600, Rob Herring wrote: > > On Sun, Jan 20, 2019 at 11:26 AM Lubomir Rintel wrote: > > > The Marvell Armada DRM master device is a virtual device needed to list > > > all > > > nodes that comprise the graphics subsystem. > > > > > > Signed-off-by: Lubomir Rintel > > > --- > > > .../display/armada/marvell-armada-drm.txt | 24 +++ > > > 1 file changed, 24 insertions(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > index de4cca9432c8..3dbfa8047f0b 100644 > > > --- > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > +++ > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > > > @@ -1,3 +1,27 @@ > > > +Marvell Armada DRM master device > > > + > > > + > > > +The Marvell Armada DRM master device is a virtual device needed to list > > > all > > > +nodes that comprise the graphics subsystem. > > > + > > > +Required properties: > > > + > > > + - compatible: value should be "marvell,dove-display-subsystem", > > > + "marvell,armada-display-subsystem" > > > + - ports: a list of phandles pointing to display interface ports of CRTC > > > + devices > > > + - memory-region: phandle to a node describing memory to be used for the > > > + framebuffer > > > + > > > +Example: > > > + > > > + display-subsystem { > > > + compatible = "marvell,dove-display-subsystem", > > > +"marvell,armada-display-subsystem"; > > > + memory-region = <&display_reserved>; > > > + ports = <&lcd0_port>; > > > > If there is only one device, you don't need this virtual node. > > By "one device" you mean one LCD controller (CRTC)? Yes. > I suppose in the (single CRTC) example case, the display-subsystem node > used to associate it with the memory region reserved for allocating the > frame buffers from. Could that be done differently? Move memory-region to the LCD controller node. > Also, if the node is indeed made optional, then it's going to > complicate things on the DRM side. Currently the driver that binds to > the node creates the DRM device once it sees all the components > connected to the ports appear. If we loose it, then the LCD controller > driver would somehow need to find out that it's alone and create the > DRM device itself. DT is not the only way to create devices. The DRM driver can bind to the LCDC node and then create a child CRTC device (or even multiple ones for h/w with multiple pipelines). You'll also notice that there are only 3 cases of this virtual node in the tree: STi, i.MX IPU, and Rockchip. That's because we've deprecated doing these virtual nodes for some time now. IOW, there are several examples of how to do this without a virtual node. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 9/9] drm/bridge: cdns: Convert to phy framework
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. Acked-by: Sean Paul Signed-off-by: Maxime Ripard --- drivers/gpu/drm/bridge/Kconfig| 1 +- drivers/gpu/drm/bridge/cdns-dsi.c | 485 +++ 2 files changed, 60 insertions(+), 426 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 796874e76308..713a70b86046 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_CONF0x0 #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_OVERHEAD6 #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_ENBIT(0) -#define DPHY_CMN_TX_MODE_ENBIT(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_REGBIT(0) -#define DPHY_CMN_IPDIV(x) ((x) << 1) -#define DPHY_CMN_OPDIV_FROM_REGBIT(6) -#define DPHY_CMN_OPDIV(x) ((x) << 7) - -#define DPHY_PSM_CFG DPHY_PCS(0x4) -#define DPHY_PSM_CFG_FROM_REG BIT(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_dphy *dphy; + struct phy *dphy; }; static inline struct cdns_dsi *input_to_dsi(struct cdns_dsi_input *input) @@ -554,175 +488,6 @@ static unsigned int mode_to_dpi_hfp(const struct drm_display_mode *mode, return mode->crtc_hsync_start - mode->crtc_hdisplay; } -static int c
[PATCH v5 1/9] phy: dphy: Remove unused header
The videomode.h header inclusion is an artifact from the patches development, remove it. Suggested-by: Sakari Ailus Acked-by: Sakari Ailus Signed-off-by: Maxime Ripard --- include/linux/phy/phy-mipi-dphy.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mipi-dphy.h index c08aacc0ac35..9cf97cd1d303 100644 --- a/include/linux/phy/phy-mipi-dphy.h +++ b/include/linux/phy/phy-mipi-dphy.h @@ -6,8 +6,6 @@ #ifndef __PHY_MIPI_DPHY_H_ #define __PHY_MIPI_DPHY_H_ -#include - /** * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set * -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 0/9] phy: Add configuration interface for MIPI D-PHY devices
Hi, Here is a set of patches to allow the phy framework consumers to test and apply runtime configurations. This is needed to support more phy classes that require tuning based on parameters depending on the current use case of the device, in addition to the power state management already provided by the current functions. A first test bed for that API are the MIPI D-PHY devices. There's a number of solutions that have been used so far to support these phy, most of the time being an ad-hoc driver in the consumer. That approach has a big shortcoming though, which is that this is quite difficult to deal with consumers integrated with multiple variants of phy, of multiple consumers integrated with the same phy. The latter case can be found in the Cadence DSI bridge, and the CSI transceiver and receivers. All of them are integrated with the same phy, or can be integrated with different phy, depending on the implementation. I've looked at all the MIPI DSI drivers I could find, and gathered all the parameters I could find. The interface should be complete, and most of the drivers can be converted in the future. The current set converts two of them: the above mentionned Cadence DSI driver so that the v4l2 drivers can use them, and the Allwinner MIPI-DSI driver. Let me know what you think, Maxime Changes from v4: - Removed regression on the variable calculation - Fixed the wakeup unit - Collected Sean Acked-by on the last patch - Collected Sakari Reviewed-by on the first patch Changes from v3 - Rebased on 5.0-rc1 - Added the fixes suggested by Sakari Changes from v2: - Rebased on next - Changed the interface to accomodate for the new submodes - Changed the timings units from nanoseconds to picoseconds - Added minimum and maximum boundaries to the documentation - Moved the clock enabling to phy_power_on in the Cadence DPHY driver - Exported the phy_configure and phy_validate symbols - Rework the phy pll divider computation in the cadence dphy driver Changes from v1: - Rebased on top of 4.20-rc1 - Removed the bus mode and timings parameters from the MIPI D-PHY parameters, since that shouldn't have any impact on the PHY itself. - Reworked the Cadence DSI and D-PHY drivers to take this into account. - Remove the mode parameter from phy_configure - Added phy_configure and phy_validate stubs - Return -EOPNOTSUPP in phy_configure and phy_validate when the operation is not implemented Maxime Ripard (9): phy: dphy: Remove unused header phy: dphy: Change units of wakeup and init parameters phy: dphy: Clarify lanes parameter documentation sun6i: dsi: Convert to generic phy handling phy: Move Allwinner A31 D-PHY driver to drivers/phy/ drm/bridge: cdns: Separate DSI and D-PHY configuration dt-bindings: phy: Move the Cadence D-PHY bindings phy: Add Cadence D-PHY support drm/bridge: cdns: Convert to phy framework Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +- Documentation/devicetree/bindings/phy/cdns,dphy.txt | 20 +- drivers/gpu/drm/bridge/Kconfig| 1 +- drivers/gpu/drm/bridge/cdns-dsi.c | 538 +-- drivers/gpu/drm/sun4i/Kconfig | 3 +- drivers/gpu/drm/sun4i/Makefile| 5 +- drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 292 + drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c| 31 +- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h| 17 +- drivers/phy/allwinner/Kconfig | 12 +- drivers/phy/allwinner/Makefile| 1 +- drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 318 - drivers/phy/cadence/Kconfig | 13 +- drivers/phy/cadence/Makefile | 1 +- drivers/phy/cadence/cdns-dphy.c | 389 +- drivers/phy/phy-core-mipi-dphy.c | 8 +- include/linux/phy/phy-mipi-dphy.h | 13 +- 17 files changed, 894 insertions(+), 789 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c create mode 100644 drivers/phy/cadence/cdns-dphy.c base-commit: bfeffd155283772bbe78c6a05dec7c0128ee500c -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 8/9] phy: Add Cadence D-PHY support
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. Acked-by: Sakari Ailus 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 ..cde12b3aa4d4 --- /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_ENBIT(0) +#define DPHY_CMN_TX_MODE_ENBIT(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_REGBIT(0) +#define DPHY_CMN_IPDIV(x) ((x) << 1) +#define DPHY_CMN_OPDIV_FROM_REGBIT(6) +#define DPHY_CMN_OPDIV(x) ((x) << 7) + +#define DPHY_PSM_CFG DPHY_PCS(0x4) +#define DPHY_PSM_CFG_FROM_REG BIT(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_OVERHEAD6 +#define DSI_NULL_FRAME_OVERHEAD6 +#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_ALL = 3, +}; + +struct cdns_dphy; +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_d
[PATCH v5 5/9] phy: Move Allwinner A31 D-PHY driver to drivers/phy/
Now that our MIPI D-PHY driver has been converted to the phy framework, let's move it into the drivers/phy directory. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/sun4i/Kconfig | 10 +- drivers/gpu/drm/sun4i/Makefile | 1 +- drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 318 +- drivers/phy/allwinner/Kconfig | 12 +- drivers/phy/allwinner/Makefile | 1 +- drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 318 +- 6 files changed, 332 insertions(+), 328 deletions(-) delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig index 2b8db82c4bab..1dbbc3a1b763 100644 --- a/drivers/gpu/drm/sun4i/Kconfig +++ b/drivers/gpu/drm/sun4i/Kconfig @@ -45,20 +45,12 @@ config DRM_SUN6I_DSI default MACH_SUN8I select CRC_CCITT select DRM_MIPI_DSI - select DRM_SUN6I_DPHY + select PHY_SUN6I_MIPI_DPHY help Choose this option if you want have an Allwinner SoC with MIPI-DSI support. If M is selected the module will be called sun6i_mipi_dsi. -config DRM_SUN6I_DPHY - tristate "Allwinner A31 MIPI D-PHY Support" - select GENERIC_PHY_MIPI_DPHY - help - Choose this option if you have an Allwinner SoC with - MIPI-DSI support. If M is selected, the module will be - called sun6i_mipi_dphy. - config DRM_SUN8I_DW_HDMI tristate "Support for Allwinner version of DesignWare HDMI" depends on DRM_SUN4I diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile index 1e2320d824b5..0d04f2447b01 100644 --- a/drivers/gpu/drm/sun4i/Makefile +++ b/drivers/gpu/drm/sun4i/Makefile @@ -34,7 +34,6 @@ ifdef CONFIG_DRM_SUN4I_BACKEND obj-$(CONFIG_DRM_SUN4I)+= sun4i-frontend.o endif obj-$(CONFIG_DRM_SUN4I_HDMI) += sun4i-drm-hdmi.o -obj-$(CONFIG_DRM_SUN6I_DPHY) += sun6i_mipi_dphy.o obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i_mipi_dsi.o obj-$(CONFIG_DRM_SUN8I_DW_HDMI)+= sun8i-drm-hdmi.o obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c deleted file mode 100644 index 79c8af5c7c1d.. --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c +++ /dev/null @@ -1,318 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * Copyright (c) 2016 Allwinnertech Co., Ltd. - * Copyright (C) 2017-2018 Bootlin - * - * Maxime Ripard - */ - -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -#define SUN6I_DPHY_GCTL_REG0x00 -#define SUN6I_DPHY_GCTL_LANE_NUM(n)n) - 1) & 3) << 4) -#define SUN6I_DPHY_GCTL_EN BIT(0) - -#define SUN6I_DPHY_TX_CTL_REG 0x04 -#define SUN6I_DPHY_TX_CTL_HS_TX_CLK_CONT BIT(28) - -#define SUN6I_DPHY_TX_TIME0_REG0x10 -#define SUN6I_DPHY_TX_TIME0_HS_TRAIL(n)(((n) & 0xff) << 24) -#define SUN6I_DPHY_TX_TIME0_HS_PREPARE(n) (((n) & 0xff) << 16) -#define SUN6I_DPHY_TX_TIME0_LP_CLK_DIV(n) ((n) & 0xff) - -#define SUN6I_DPHY_TX_TIME1_REG0x14 -#define SUN6I_DPHY_TX_TIME1_CLK_POST(n)(((n) & 0xff) << 24) -#define SUN6I_DPHY_TX_TIME1_CLK_PRE(n) (((n) & 0xff) << 16) -#define SUN6I_DPHY_TX_TIME1_CLK_ZERO(n)(((n) & 0xff) << 8) -#define SUN6I_DPHY_TX_TIME1_CLK_PREPARE(n) ((n) & 0xff) - -#define SUN6I_DPHY_TX_TIME2_REG0x18 -#define SUN6I_DPHY_TX_TIME2_CLK_TRAIL(n) ((n) & 0xff) - -#define SUN6I_DPHY_TX_TIME3_REG0x1c - -#define SUN6I_DPHY_TX_TIME4_REG0x20 -#define SUN6I_DPHY_TX_TIME4_HS_TX_ANA1(n) (((n) & 0xff) << 8) -#define SUN6I_DPHY_TX_TIME4_HS_TX_ANA0(n) ((n) & 0xff) - -#define SUN6I_DPHY_ANA0_REG0x4c -#define SUN6I_DPHY_ANA0_REG_PWSBIT(31) -#define SUN6I_DPHY_ANA0_REG_DMPC BIT(28) -#define SUN6I_DPHY_ANA0_REG_DMPD(n)(((n) & 0xf) << 24) -#define SUN6I_DPHY_ANA0_REG_SLV(n) (((n) & 7) << 12) -#define SUN6I_DPHY_ANA0_REG_DEN(n) (((n) & 0xf) << 8) - -#define SUN6I_DPHY_ANA1_REG0x50 -#define SUN6I_DPHY_ANA1_REG_VTTMODEBIT(31) -#define SUN6I_DPHY_ANA1_REG_CSMPS(n) (((n) & 3) << 28) -#define SUN6I_DPHY_ANA1_REG_SVTT(n)(((n) & 0xf) << 24) - -#define SUN6I_DPHY_ANA2_REG0x54 -#define SUN6I_DPHY_ANA2_EN_P2S_CPU(n) (((n) & 0xf) << 24) -#define SUN6I_DPHY_ANA2_EN_P2S_CPU_MASKGENMASK(27, 24) -#define SUN6I_DPHY_ANA2_EN_CK_CPU BIT(4) -#define SUN6I_DPHY_ANA2_REG_ENIB BIT(1) - -#define SUN6I_DPHY_ANA3_REG0x58 -#define SUN6I_DPHY_ANA3_EN_VTTD(n) (((n) & 0xf) << 28) -#define SUN6I_DPHY_ANA3_EN_V
[PATCH v5 2/9] phy: dphy: Change units of wakeup and init parameters
The Init and wakeup D-PHY parameters are in the micro/milliseconds range, putting the values real close to the types limits if they were in picoseconds. Move them to microseconds which should be better fit. Suggested-by: Sakari Ailus Acked-by: Sakari Ailus Signed-off-by: Maxime Ripard --- drivers/phy/phy-core-mipi-dphy.c | 8 include/linux/phy/phy-mipi-dphy.h | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/phy/phy-core-mipi-dphy.c b/drivers/phy/phy-core-mipi-dphy.c index 465fa1b91a5f..14e0551cd319 100644 --- a/drivers/phy/phy-core-mipi-dphy.c +++ b/drivers/phy/phy-core-mipi-dphy.c @@ -65,12 +65,12 @@ int phy_mipi_dphy_get_default_config(unsigned long pixel_clock, */ cfg->hs_trail = max(4 * 8 * ui, 6 + 4 * 4 * ui); - cfg->init = 1; + cfg->init = 100; cfg->lpx = 6; cfg->ta_get = 5 * cfg->lpx; cfg->ta_go = 4 * cfg->lpx; cfg->ta_sure = 2 * cfg->lpx; - cfg->wakeup = 10; + cfg->wakeup = 1000; cfg->hs_clk_rate = hs_clk_rate; cfg->lanes = lanes; @@ -143,7 +143,7 @@ int phy_mipi_dphy_config_validate(struct phy_configure_opts_mipi_dphy *cfg) if (cfg->hs_trail < max(8 * ui, 6 + 4 * ui)) return -EINVAL; - if (cfg->init < 1) + if (cfg->init < 100) return -EINVAL; if (cfg->lpx < 5) @@ -158,7 +158,7 @@ int phy_mipi_dphy_config_validate(struct phy_configure_opts_mipi_dphy *cfg) if (cfg->ta_sure < cfg->lpx || cfg->ta_sure > (2 * cfg->lpx)) return -EINVAL; - if (cfg->wakeup < 10) + if (cfg->wakeup < 1000) return -EINVAL; return 0; diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mipi-dphy.h index 9cf97cd1d303..627d28080d3a 100644 --- a/include/linux/phy/phy-mipi-dphy.h +++ b/include/linux/phy/phy-mipi-dphy.h @@ -190,10 +190,10 @@ struct phy_configure_opts_mipi_dphy { /** * @init: * -* Time, in picoseconds for the initialization period to +* Time, in microseconds for the initialization period to * complete. * -* Minimum value: 1 ps +* Minimum value: 100 us */ unsigned intinit; @@ -244,11 +244,11 @@ struct phy_configure_opts_mipi_dphy { /** * @wakeup: * -* Time, in picoseconds, that a transmitter drives a Mark-1 +* Time, in microseconds, that a transmitter drives a Mark-1 * state prior to a Stop state in order to initiate an exit * from ULPS. * -* Minimum value: 10 ps +* Minimum value: 1000 us */ unsigned intwakeup; -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 6/9] drm/bridge: cdns: Separate DSI and D-PHY configuration
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 | 101 ++- 1 file changed, 73 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c index ce9496d13986..796874e76308 100644 --- a/drivers/gpu/drm/bridge/cdns-dsi.c +++ b/drivers/gpu/drm/bridge/cdns-dsi.c @@ -545,6 +545,15 @@ 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, + bool mode_valid_check) +{ + if (mode_valid_check) + return mode->hsync_start - mode->hdisplay; + + 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 +740,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 +764,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 +773,91 @@ 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, mode_valid_check), +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_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_cfg->hfp = dpi_to_dsi_timing(dpi_hfp, bpp, DSI_HFP_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_htotal, +
[PATCH v5 7/9] dt-bindings: phy: Move the Cadence D-PHY bindings
The Cadence D-PHY bindings was defined as part of the DSI block so far. However, since it's now going to be a separate driver, we need to move the binding to a file of its own. Acked-by: Sakari Ailus Signed-off-by: Maxime Ripard --- Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +--- Documentation/devicetree/bindings/phy/cdns,dphy.txt | 20 +++- 2 files changed, 20 insertions(+), 21 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt index f5725bb6c61c..525a4bfd8634 100644 --- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt +++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt @@ -31,28 +31,7 @@ Required subnodes: - one subnode per DSI device connected on the DSI bus. Each DSI device should contain a reg property encoding its virtual channel. -Cadence DPHY - - -Cadence DPHY block. - -Required properties: -- compatible: should be set to "cdns,dphy". -- reg: physical base address and length of the DPHY registers. -- clocks: DPHY reference clocks. -- clock-names: must contain "psm" and "pll_ref". -- #phy-cells: must be set to 0. - - Example: - dphy0: dphy@fd0e{ - compatible = "cdns,dphy"; - reg = <0x0 0xfd0e 0x0 0x1000>; - clocks = <&psm_clk>, <&pll_ref_clk>; - clock-names = "psm", "pll_ref"; - #phy-cells = <0>; - }; - dsi0: dsi@fd0c { compatible = "cdns,dsi"; reg = <0x0 0xfd0c 0x0 0x1000>; diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.txt b/Documentation/devicetree/bindings/phy/cdns,dphy.txt new file mode 100644 index ..1095bc4e72d9 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/cdns,dphy.txt @@ -0,0 +1,20 @@ +Cadence DPHY + + +Cadence DPHY block. + +Required properties: +- compatible: should be set to "cdns,dphy". +- reg: physical base address and length of the DPHY registers. +- clocks: DPHY reference clocks. +- clock-names: must contain "psm" and "pll_ref". +- #phy-cells: must be set to 0. + +Example: + dphy0: dphy@fd0e{ + compatible = "cdns,dphy"; + reg = <0x0 0xfd0e 0x0 0x1000>; + clocks = <&psm_clk>, <&pll_ref_clk>; + clock-names = "psm", "pll_ref"; + #phy-cells = <0>; + }; -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 4/9] sun6i: dsi: Convert to generic phy handling
Now that we have everything in place in the PHY framework to deal in a generic way with MIPI D-PHY phys, let's convert our PHY driver and its associated DSI driver to that new API. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/sun4i/Kconfig | 11 +- drivers/gpu/drm/sun4i/Makefile | 6 +- drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 164 ++--- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 31 ++--- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 17 +--- 5 files changed, 126 insertions(+), 103 deletions(-) diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig index c2c042287c19..2b8db82c4bab 100644 --- a/drivers/gpu/drm/sun4i/Kconfig +++ b/drivers/gpu/drm/sun4i/Kconfig @@ -45,10 +45,19 @@ config DRM_SUN6I_DSI default MACH_SUN8I select CRC_CCITT select DRM_MIPI_DSI + select DRM_SUN6I_DPHY help Choose this option if you want have an Allwinner SoC with MIPI-DSI support. If M is selected the module will be called - sun6i-dsi + sun6i_mipi_dsi. + +config DRM_SUN6I_DPHY + tristate "Allwinner A31 MIPI D-PHY Support" + select GENERIC_PHY_MIPI_DPHY + help + Choose this option if you have an Allwinner SoC with + MIPI-DSI support. If M is selected, the module will be + called sun6i_mipi_dphy. config DRM_SUN8I_DW_HDMI tristate "Support for Allwinner version of DesignWare HDMI" diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile index 0eb38ac8e86e..1e2320d824b5 100644 --- a/drivers/gpu/drm/sun4i/Makefile +++ b/drivers/gpu/drm/sun4i/Makefile @@ -24,9 +24,6 @@ sun4i-tcon-y += sun4i_lvds.o sun4i-tcon-y += sun4i_tcon.o sun4i-tcon-y += sun4i_rgb.o -sun6i-dsi-y+= sun6i_mipi_dphy.o -sun6i-dsi-y+= sun6i_mipi_dsi.o - obj-$(CONFIG_DRM_SUN4I)+= sun4i-drm.o obj-$(CONFIG_DRM_SUN4I)+= sun4i-tcon.o obj-$(CONFIG_DRM_SUN4I)+= sun4i_tv.o @@ -37,7 +34,8 @@ ifdef CONFIG_DRM_SUN4I_BACKEND obj-$(CONFIG_DRM_SUN4I)+= sun4i-frontend.o endif obj-$(CONFIG_DRM_SUN4I_HDMI) += sun4i-drm-hdmi.o -obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i-dsi.o +obj-$(CONFIG_DRM_SUN6I_DPHY) += sun6i_mipi_dphy.o +obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i_mipi_dsi.o obj-$(CONFIG_DRM_SUN8I_DW_HDMI)+= sun8i-drm-hdmi.o obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o obj-$(CONFIG_DRM_SUN8I_TCON_TOP) += sun8i_tcon_top.o diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c index e4d19431fa0e..79c8af5c7c1d 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c @@ -8,11 +8,14 @@ #include #include +#include #include +#include #include #include -#include "sun6i_mipi_dsi.h" +#include +#include #define SUN6I_DPHY_GCTL_REG0x00 #define SUN6I_DPHY_GCTL_LANE_NUM(n)n) - 1) & 3) << 4) @@ -81,12 +84,46 @@ #define SUN6I_DPHY_DBG5_REG0xf4 -int sun6i_dphy_init(struct sun6i_dphy *dphy, unsigned int lanes) +struct sun6i_dphy { + struct clk *bus_clk; + struct clk *mod_clk; + struct regmap *regs; + struct reset_control*reset; + + struct phy *phy; + struct phy_configure_opts_mipi_dphy config; +}; + +static int sun6i_dphy_init(struct phy *phy) { + struct sun6i_dphy *dphy = phy_get_drvdata(phy); + reset_control_deassert(dphy->reset); clk_prepare_enable(dphy->mod_clk); clk_set_rate_exclusive(dphy->mod_clk, 15000); + return 0; +} + +static int sun6i_dphy_configure(struct phy *phy, union phy_configure_opts *opts) +{ + struct sun6i_dphy *dphy = phy_get_drvdata(phy); + int ret; + + ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy); + if (ret) + return ret; + + memcpy(&dphy->config, opts, sizeof(dphy->config)); + + return 0; +} + +static int sun6i_dphy_power_on(struct phy *phy) +{ + struct sun6i_dphy *dphy = phy_get_drvdata(phy); + u8 lanes_mask = GENMASK(dphy->config.lanes - 1, 0); + regmap_write(dphy->regs, SUN6I_DPHY_TX_CTL_REG, SUN6I_DPHY_TX_CTL_HS_TX_CLK_CONT); @@ -111,16 +148,9 @@ int sun6i_dphy_init(struct sun6i_dphy *dphy, unsigned int lanes) SUN6I_DPHY_TX_TIME4_HS_TX_ANA1(3)); regmap_write(dphy->regs, SUN6I_DPHY_GCTL_REG, -SUN6I_DPHY_GCTL_LANE_NUM(lanes) | +SUN6I_DPHY_GCTL_LANE_NUM(dphy->config.lanes) | SUN6I_DPHY_GCTL_EN); - return 0; -} - -int sun6i_dphy_power_on(struct sun6i_dphy *dphy, unsigned int lanes) -{ - u8 lanes_mask = GENMASK(lanes - 1, 0);
[PATCH v5 3/9] phy: dphy: Clarify lanes parameter documentation
The lanes parameter is not solely about the number of lanes, but it also carries the fact that those are the first lanes in use during the transmission. It was implicit so far, so make sure it's explicit now. Suggested-by: Sakari Ailus Acked-by: Sakari Ailus Signed-off-by: Maxime Ripard --- include/linux/phy/phy-mipi-dphy.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mipi-dphy.h index 627d28080d3a..a877ffee845d 100644 --- a/include/linux/phy/phy-mipi-dphy.h +++ b/include/linux/phy/phy-mipi-dphy.h @@ -269,7 +269,8 @@ struct phy_configure_opts_mipi_dphy { /** * @lanes: * -* Number of active data lanes used for the transmissions. +* Number of active, consecutive, data lanes, starting from +* lane 0, used for the transmissions. */ unsigned char lanes; }; -- git-series 0.9.1 ___ 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 Thu, Jan 17, 2019 at 08:33:38AM -0500, Sean Paul wrote: > > @@ -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. ah, right, thanks for spotting this. I'll resend a version with those changes, thanks! 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 4/6] dt-bindings: display: armada: Add display subsystem binding
On Sun, Jan 20, 2019 at 11:26 AM Lubomir Rintel wrote: > > The Marvell Armada DRM master device is a virtual device needed to list all > nodes that comprise the graphics subsystem. > > Signed-off-by: Lubomir Rintel > --- > .../display/armada/marvell-armada-drm.txt | 24 +++ > 1 file changed, 24 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > index de4cca9432c8..3dbfa8047f0b 100644 > --- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > +++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt > @@ -1,3 +1,27 @@ > +Marvell Armada DRM master device > + > + > +The Marvell Armada DRM master device is a virtual device needed to list all > +nodes that comprise the graphics subsystem. > + > +Required properties: > + > + - compatible: value should be "marvell,dove-display-subsystem", > + "marvell,armada-display-subsystem" > + - ports: a list of phandles pointing to display interface ports of CRTC > + devices > + - memory-region: phandle to a node describing memory to be used for the > + framebuffer > + > +Example: > + > + display-subsystem { > + compatible = "marvell,dove-display-subsystem", > +"marvell,armada-display-subsystem"; > + memory-region = <&display_reserved>; > + ports = <&lcd0_port>; If there is only one device, you don't need this virtual node. > + }; > + > Marvell Armada LCD controller > = > > -- > 2.20.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/i915/dsi: Adjust crtc_clock for burst_mode_ratio
Hi, On 21-01-19 15:26, Hans de Goede wrote: Hi, On 15-01-19 16:00, Ville Syrjälä wrote: On Sat, Dec 01, 2018 at 12:31:47PM +0100, Hans de Goede wrote: On devices with a burst_mode_ratio which is not 100 (1:1), the pclk will have a different value then drm_display_mode.clock . On a Prowise PT301 tablet where vbt.lfp_lvds_vbt_mode.clock is 66100 and burst_mode_ratio is 130 this leads to the following errors: [drm:pipe_config_err [i915]] *ERROR* mismatch in pixel_rate (expected 66100, found 86458) [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_clock (expected 66100, found 86458) [drm:pipe_config_err [i915]] *ERROR* mismatch in port_clock (expected 66100, found 86458) This commit makes intel_dsi_compute_config() set pipe_config.adjusted_mode.crtc_clock, taking the burst_mode_ratio into account fixing this. Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/vlv_dsi.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c index c21cbfa9653c..d72ccf557a9c 100644 --- a/drivers/gpu/drm/i915/vlv_dsi.c +++ b/drivers/gpu/drm/i915/vlv_dsi.c @@ -347,6 +347,10 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder, return false; } + adjusted_mode->crtc_clock = + DIV_ROUND_UP(adjusted_mode->crtc_clock * + intel_dsi->burst_mode_ratio, 100); Hmm. Won't this cause incorrect refresh rate to be used in eg. vblank timestmap calculations? I guess so. Note that this patch does not change any values actually written to the hardware. It seems that devices which actually use the burst mode are quite rare (this is the first one encounter in probably over 40 different byt/cht devices I've tested). I've a feeling that the entire pipeline is actually running at the higher rate and that the framerate really is 30% higher. Looking at the code, it seems that what a burst_mode_ratio of 130'does is make all the values in the "modeline" except for the visual area 30% larger, which means that we are probably already messing up the vblank calculations anyways, since using either the uncorrected or the corrected clock is wrong when using htotal from the original modeline, as looking at txbyteclkhs we will use bigger values for all drm_display_mode values except for the active region. I think that the right way to deal with this is to isolate the burst_ratio handling to intel_dsi_vbt.c and adjust the modeline coming from the VBT by multiplying the clock and all timing parameters (except h/vdisplay) there by the burst_ratio and then recalculating h/vtotal. This should lead to getting the vblank timestamp stuff right and allows removing of burst_mode_ratio from all code except for the vbt code. If that is too invasive, given that this setup is quite rate, then I suggest we just go with this patch. My main concern is fixing the WARN_ON. This patch successfully does that. OTOH if the pipe is really fetching data at the higher burst rate then we should rather want to calculate the watermarks/cdclk based on that higher rate. Right, the more I think about this, the more I believe calculating a new modeline correcting for burst_ratio inside the vbt code and dropping burst_mode_ratio handling everywhere else is the right thing to do. Regards, Hans p.s. The 4th patch in this series is independent of the others, it fixes a small bug (not freeing a resource) in an exit error path which I noticed. It would be great if someone can review the 4th patch then I can push that one too and then this patch will be the only unmerged patch from this series. Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 3/3] PM/runtime:Replace jiffies based accounting with ktime based accounting
On Fri, 18 Jan 2019 at 13:08, Guenter Roeck wrote: > > On 1/18/19 3:05 AM, Rafael J. Wysocki wrote: > > On Fri, Jan 18, 2019 at 11:53 AM Vincent Guittot > > wrote: > >> > >> On Fri, 18 Jan 2019 at 11:42, Vincent Guittot > >> wrote: > >>> > >>> Hi Guenter, > >>> > >>> Le Thursday 17 Jan 2019 à 14:16:28 (-0800), Guenter Roeck a écrit : > On Fri, Dec 21, 2018 at 11:33:56AM +0100, Vincent Guittot wrote: > > From: Thara Gopinath > > > > This patch replaces jiffies based accounting for runtime_active_time > > and runtime_suspended_time with ktime base accounting. This makes the > > runtime debug counters inline with genpd and other pm subsytems which > > uses ktime based accounting. > > > > timekeeping is initialized before pm_runtime_init() so ktime_get() will > > be ready before first call. In fact, timekeeping_init() is called early > > in start_kernel() which is way before driver_init() (and that's when > > devices can start to be initialized) called from rest_init() via > > kernel_init_freeable() and do_basic_setup(). > > > This is not (always) correct. My qemu "collie" boot test fails with this > patch applied. Reverting the patch fixes the problem. Bisect log > attached. > > >>> > >>> Can you try the patch below ? > >>> ktime_get_mono_fast_ns() has the advantage of being init with dummy clock > >>> so > >>> it can be used at early_init. > >> > >> Another possibility would be delay the init of the gpiochip > > > > Well, right. > > > > Initializing devices before timekeeping doesn't feel particularly > > robust from the design perspective. > > > > How exactly does that happen? > > > > With an added 'initialized' flag and backtrace into the timekeeping code, > with the change suggested earlier applied: > > [ cut here ] > WARNING: CPU: 0 PID: 0 at kernel/time/timekeeping.c:453 > ktime_get_mono_fast_ns+0x114/0x12c > Timekeeping not initialized > CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc2-next-20190117-dirty #2 > Hardware name: Sharp-Collie > Backtrace: > [] (dump_backtrace) from [] (show_stack+0x18/0x1c) > r7:0009 r6: r5:c065ba90 r4:c06d3e54 > [] (show_stack) from [] (dump_stack+0x20/0x28) > [] (dump_stack) from [] (__warn+0xcc/0xf4) > [] (__warn) from [] (warn_slowpath_fmt+0x4c/0x6c) > r8:df407b08 r7: r6:c0c01550 r5:c065bad8 r4:c06dd028 > [] (warn_slowpath_fmt) from [] > (ktime_get_mono_fast_ns+0x114/0x12c) > r3: r2:c065bad8 > r5: r4:df407b08 > [] (ktime_get_mono_fast_ns) from [] > (pm_runtime_init+0x38/0xb8) > r9:c06c9a5c r8:df407b08 r7: r6:c0c01550 r5: r4:df407b08 > [] (pm_runtime_init) from [] (device_initialize+0xb0/0xec) > r7: r6:c0c01550 r5: r4:df407b08 > [] (device_initialize) from [] > (gpiochip_add_data_with_key+0x9c/0x884) > r7: r6:c06fca34 r5: r4: > [] (gpiochip_add_data_with_key) from [] > (sa1100_init_gpio+0x40/0x98) > r10:dfffcd60 r9:c06c9a5c r8:c06dd020 r7:c06dd028 r6: r5: > r4:c06fca34 > [] (sa1100_init_gpio) from [] (sa1100_init_irq+0x2c/0x3c) > r7:c06dd028 r6: r5:c0713300 r4:c06e1070 > [] (sa1100_init_irq) from [] (init_IRQ+0x20/0x28) > r5:c0713300 r4: > [] (init_IRQ) from [] (start_kernel+0x254/0x4cc) > [] (start_kernel) from [<>] ( (null)) > r10:717f r9:6901b119 r8:c100 r7:0092 r6:313d r5:0053 > r4:c06a7330 > ---[ end trace 91e1bd00dd7cce32 ]--- Does it means that only the pm_runtime_init is done before timekeeping_init() but no update_pm_runtime_accounting() ? In this case, we can keep using ktimeçget in update_pm_runtime_accounting() and find a solution to deal with early_call of pm_runtime_init() Vincent > > Guenter ___ 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 1/18/19 3:43 PM, Liam Mark wrote: > On Fri, 18 Jan 2019, Andrew F. Davis wrote: > >> On 1/17/19 7:04 PM, Liam Mark wrote: >>> On Thu, 17 Jan 2019, Andrew F. Davis wrote: >>> 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
Re: [PATCH 00/14] Misc ION cleanups and adding unmapped heap
On 1/18/19 2:19 PM, Laura Abbott wrote: > On 1/16/19 8:05 AM, Andrew F. Davis wrote: >> On 1/15/19 12:58 PM, Laura Abbott wrote: >>> On 1/15/19 9:47 AM, Andrew F. Davis wrote: On 1/14/19 8:39 PM, Laura Abbott wrote: > On 1/11/19 10:05 AM, Andrew F. Davis wrote: >> Hello all, >> >> This is a set of (hopefully) non-controversial cleanups for the ION >> framework and current set of heaps. These were found as I start to >> familiarize myself with the framework to help in whatever way I >> can in getting all this up to the standards needed for de-staging. >> >> I would like to get some ideas of what is left to work on to get ION >> out of staging. Has there been some kind of agreement on what ION >> should >> eventually end up being? To me it looks like it is being whittled >> away at >> to it's most core functions. To me that is looking like being a >> DMA-BUF >> user-space front end, simply advertising available memory backings >> in a >> system and providing allocations as DMA-BUF handles. If this is the >> case >> then it looks close to being ready to me at least, but I would >> love to >> hear any other opinions and concerns. >> > > Yes, at this point the only functionality that people are really > depending on is the ability to allocate a dma_buf easily from > userspace. > >> Back to this patchset, the last patch may be a bit different than the >> others, it adds an unmapped heaps type and creation helper. I >> wanted to >> get this in to show off another heap type and maybe some issues we >> may >> have with the current ION framework. The unmapped heap is used >> when the >> backing memory should not (or cannot) be touched. Currently this kind >> of heap is used for firewalled secure memory that can be allocated >> like >> normal heap memory but only used by secure devices (OP-TEE, crypto >> HW, >> etc). It is basically just copied from the "carveout" heap type with >> the >> only difference being it is not mappable to userspace and we do not >> clear >> the memory (as we should not map it either). So should this really >> be a >> new heap type? Or maybe advertised as a carveout heap but with an >> additional allocation flag? Perhaps we do away with "types" >> altogether >> and just have flags, coherent/non-coherent, mapped/unmapped, etc. >> >> Maybe more thinking will be needed afterall.. >> > > So the cleanup looks okay (I need to finish reviewing) but I'm not a > fan of adding another heaptype without solving the problem of adding > some sort of devicetree binding or other method of allocating and > placing Ion heaps. That plus uncached buffers are one of the big > open problems that need to be solved for destaging Ion. See > https://lore.kernel.org/lkml/20181120164636.jcw7li2uaa3cmwc3@DESKTOP-E1NTVVP.localdomain/ > > > > for some background on that problem. > I'm under the impression that adding heaps like carveouts/chunk will be rather system specific and so do not lend themselves well to a universal DT style exporter. For instance a carveout memory space can be reported by a device at runtime, then the driver managing that device should go and use the carveout heap helpers to export that heap. If this is the case then I'm not sure it is a problem for the ION core framework to solve, but rather the users of it to figure out how best to create the various heaps. All Ion needs to do is allow exporting and advertising them IMHO. >>> >>> I think it is a problem for the Ion core framework to take care of. >>> Ion is useless if you don't actually have the heaps. Nobody has >>> actually gotten a full Ion solution end-to-end with a carveout heap >>> working in mainline because any proposals have been rejected. I think >>> we need at least one example in mainline of how creating a carveout >>> heap would work. >> >> In our evil vendor trees we have several examples. The issue being that >> Ion is still staging and attempts for generic DT heap definitions >> haven't seemed to go so well. So for now we just keep it specific to our >> platforms until upstream makes a direction decision. >> > > Yeah, it's been a bit of a chicken and egg in that this has been > blocking Ion getting out of staging but we don't actually have > in-tree users because it's still in staging. > I could post some of our Ion exporter patches anyway, might not have a chance at getting in while it's still in staging but could show there are users trying. Kinda the reason I posted the unmapped heap. The OP-TEE folks have been using it out-of-tree waiting for Ion destaging, but without more users/examples upstream it seems to hold back destaging work in the first place.. >>> Thanks for the background thread link,
Re: [PATCH] drm/dp: use DRM_DEBUG_DP() instead of drm_dbg for logging
On Mon, Jan 21, 2019 at 01:27:58PM +0200, Jani Nikula wrote: > We have a wrapper for a reason. > > Signed-off-by: Jani Nikula Reviewed-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_dp_helper.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 26835d174939..4def0bface85 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -194,11 +194,11 @@ drm_dp_dump_access(const struct drm_dp_aux *aux, > const char *arrow = request == DP_AUX_NATIVE_READ ? "->" : "<-"; > > if (ret > 0) > - drm_dbg(DRM_UT_DP, "%s: 0x%05x AUX %s (ret=%3d) %*ph\n", > - aux->name, offset, arrow, ret, min(ret, 20), buffer); > + DRM_DEBUG_DP("%s: 0x%05x AUX %s (ret=%3d) %*ph\n", > + aux->name, offset, arrow, ret, min(ret, 20), > buffer); > else > - drm_dbg(DRM_UT_DP, "%s: 0x%05x AUX %s (ret=%3d)\n", > - aux->name, offset, arrow, ret); > + DRM_DEBUG_DP("%s: 0x%05x AUX %s (ret=%3d)\n", > + aux->name, offset, arrow, ret); > } > > /** > -- > 2.20.1 > > ___ > 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
[Bug 109375] Regression: dark notebook display (no backlight) after resume
https://bugs.freedesktop.org/show_bug.cgi?id=109375 Alex Deucher changed: What|Removed |Added CC||christian.frank.uwb@googlem ||ail.com --- Comment #5 from Alex Deucher --- *** Bug 109388 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109388] Lenovo E585 ryzen 2500u, regression with display power saving in 5.0-rc1
https://bugs.freedesktop.org/show_bug.cgi?id=109388 Alex Deucher changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |DUPLICATE --- Comment #6 from Alex Deucher --- *** This bug has been marked as a duplicate of bug 109375 *** -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201815] Mouse Pointer Disappears when touching top of the screen
https://bugzilla.kernel.org/show_bug.cgi?id=201815 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #14 from Alex Deucher (alexdeuc...@gmail.com) --- Fixed here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ae1cf20df7a9c60ff5ef41c3315c33c1a5fafd77 Will show up in stable soon. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/14] staging: android: ion: Allow heap name to be null
On 1/18/19 1:53 PM, Laura Abbott wrote: > On 1/16/19 9:12 AM, Andrew F. Davis wrote: >> On 1/16/19 9:28 AM, Brian Starkey wrote: >>> Hi Andrew, >>> >>> On Fri, Jan 11, 2019 at 12:05:20PM -0600, Andrew F. Davis wrote: The heap name can be used for debugging but otherwise does not seem to be required and no other part of the code will fail if left NULL except here. We can make it required and check for it at some point, for now lets just prevent this from causing a NULL pointer exception. >>> >>> I'm not so keen on this one. In the "new" API with heap querying, the >>> name string is the only way to identify the heap. I think Laura >>> mentioned at XDC2017 that it was expected that userspace should use >>> the strings to find the heap they want. >>> >> >> Right now the names are only for debug. I accidentally left the name >> null once and got a kernel crash. This is the only spot where it is >> needed so I fixed it up. The other option is to make the name mandatory >> and properly error out, I don't want to do that right now until the >> below discussion is had to see if names really do matter or not. >> > > Yes, the heap names are part of the query API and are the expected > way to identify individual heaps for the API at the moment so having > a null heap name is incorrect. The heap name seemed like the best way > to identify heaps to userspace but if you have an alternative proposal > I'd be interested. > Not sure I have a better proposal right now, I'll re-work this patch to force heap names to be populated before ion_device_add_heap() instead. (do you think that function name is now is a misnomer? how do you feel about renaming that to just ion_add_heap()?) Andrew > Thanks, > Laura > >> > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/i915/dsi: Adjust crtc_clock for burst_mode_ratio
Hi, On 15-01-19 16:00, Ville Syrjälä wrote: On Sat, Dec 01, 2018 at 12:31:47PM +0100, Hans de Goede wrote: On devices with a burst_mode_ratio which is not 100 (1:1), the pclk will have a different value then drm_display_mode.clock . On a Prowise PT301 tablet where vbt.lfp_lvds_vbt_mode.clock is 66100 and burst_mode_ratio is 130 this leads to the following errors: [drm:pipe_config_err [i915]] *ERROR* mismatch in pixel_rate (expected 66100, found 86458) [drm:pipe_config_err [i915]] *ERROR* mismatch in base.adjusted_mode.crtc_clock (expected 66100, found 86458) [drm:pipe_config_err [i915]] *ERROR* mismatch in port_clock (expected 66100, found 86458) This commit makes intel_dsi_compute_config() set pipe_config.adjusted_mode.crtc_clock, taking the burst_mode_ratio into account fixing this. Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/vlv_dsi.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c index c21cbfa9653c..d72ccf557a9c 100644 --- a/drivers/gpu/drm/i915/vlv_dsi.c +++ b/drivers/gpu/drm/i915/vlv_dsi.c @@ -347,6 +347,10 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder, return false; } + adjusted_mode->crtc_clock = + DIV_ROUND_UP(adjusted_mode->crtc_clock * +intel_dsi->burst_mode_ratio, 100); Hmm. Won't this cause incorrect refresh rate to be used in eg. vblank timestmap calculations? I guess so. Note that this patch does not change any values actually written to the hardware. It seems that devices which actually use the burst mode are quite rare (this is the first one encounter in probably over 40 different byt/cht devices I've tested). I've a feeling that the entire pipeline is actually running at the higher rate and that the framerate really is 30% higher. Looking at the code, it seems that what a burst_mode_ratio of 130'does is make all the values in the "modeline" except for the visual area 30% larger, which means that we are probably already messing up the vblank calculations anyways, since using either the uncorrected or the corrected clock is wrong when using htotal from the original modeline, as looking at txbyteclkhs we will use bigger values for all drm_display_mode values except for the active region. I think that the right way to deal with this is to isolate the burst_ratio handling to intel_dsi_vbt.c and adjust the modeline coming from the VBT by multiplying the clock and all timing parameters (except h/vdisplay) there by the burst_ratio and then recalculating h/vtotal. This should lead to getting the vblank timestamp stuff right and allows removing of burst_mode_ratio from all code except for the vbt code. If that is too invasive, given that this setup is quite rate, then I suggest we just go with this patch. My main concern is fixing the WARN_ON. This patch successfully does that. OTOH if the pipe is really fetching data at the higher burst rate then we should rather want to calculate the watermarks/cdclk based on that higher rate. Right, the more I think about this, the more I believe calculating a new modeline correcting for burst_ratio inside the vbt code and dropping burst_mode_ratio handling everywhere else is the right thing to do. Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm-intel:drm-intel-next-queued 2/2] drivers/gpu//drm/i915/i915_reset.c:689:13: error: format '%lld' expects argument of type 'long long int', but argument 5 has type 'unsigned int'
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued head: 9f58892ea9962002399132fd3f40c6a273f8d9e1 commit: 9f58892ea9962002399132fd3f40c6a273f8d9e1 [2/2] drm/i915: Pull all the reset functionality together into i915_reset.c config: i386-randconfig-x007-201903 (attached as .config) compiler: gcc-8 (Debian 8.2.0-14) 8.2.0 reproduce: git checkout 9f58892ea9962002399132fd3f40c6a273f8d9e1 # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): In file included from include/linux/sched/mm.h:5, from drivers/gpu//drm/i915/i915_reset.c:7: drivers/gpu//drm/i915/i915_reset.c: In function 'reset_request': >> drivers/gpu//drm/i915/i915_reset.c:689:13: error: format '%lld' expects >> argument of type 'long long int', but argument 5 has type 'unsigned int' >> [-Werror=format=] GEM_TRACE("%s pardoned global=%d (fence %llx:%lld), current %d\n", ^~~ drivers/gpu//drm/i915/i915_reset.c:691:25: rq->fence.context, rq->fence.seqno, ~~~ include/linux/kernel.h:683:33: note: in definition of macro '__trace_printk_check_format' trace_printk_check_format(fmt, ##args); \ ^~~ include/linux/kernel.h:720:3: note: in expansion of macro 'do_trace_printk' do_trace_printk(fmt, ##__VA_ARGS__); \ ^~~ drivers/gpu//drm/i915/i915_gem.h:66:24: note: in expansion of macro 'trace_printk' #define GEM_TRACE(...) trace_printk(__VA_ARGS__) ^~~~ drivers/gpu//drm/i915/i915_reset.c:689:3: note: in expansion of macro 'GEM_TRACE' GEM_TRACE("%s pardoned global=%d (fence %llx:%lld), current %d\n", ^ drivers/gpu//drm/i915/i915_reset.c:689:13: error: format '%lld' expects argument of type 'long long int', but argument 6 has type 'unsigned int' [-Werror=format=] GEM_TRACE("%s pardoned global=%d (fence %llx:%lld), current %d\n", ^~~ drivers/gpu//drm/i915/i915_reset.c:691:25: rq->fence.context, rq->fence.seqno, ~~~ include/linux/kernel.h:736:29: note: in definition of macro 'do_trace_printk' __trace_printk(_THIS_IP_, fmt, ##args); \ ^~~ drivers/gpu//drm/i915/i915_gem.h:66:24: note: in expansion of macro 'trace_printk' #define GEM_TRACE(...) trace_printk(__VA_ARGS__) ^~~~ drivers/gpu//drm/i915/i915_reset.c:689:3: note: in expansion of macro 'GEM_TRACE' GEM_TRACE("%s pardoned global=%d (fence %llx:%lld), current %d\n", ^ drivers/gpu//drm/i915/i915_reset.c: In function 'nop_submit_request': drivers/gpu//drm/i915/i915_reset.c:804:12: error: format '%lld' expects argument of type 'long long int', but argument 4 has type 'unsigned int' [-Werror=format=] GEM_TRACE("%s fence %llx:%lld -> -EIO\n", ^~ drivers/gpu//drm/i915/i915_reset.c:806:29: request->fence.context, request->fence.seqno); include/linux/kernel.h:683:33: note: in definition of macro '__trace_printk_check_format' trace_printk_check_format(fmt, ##args); \ ^~~ include/linux/kernel.h:720:3: note: in expansion of macro 'do_trace_printk' do_trace_printk(fmt, ##__VA_ARGS__); \ ^~~ drivers/gpu//drm/i915/i915_gem.h:66:24: note: in expansion of macro 'trace_printk' #define GEM_TRACE(...) trace_printk(__VA_ARGS__) ^~~~ drivers/gpu//drm/i915/i915_reset.c:804:2: note: in expansion of macro 'GEM_TRACE' GEM_TRACE("%s fence %llx:%lld -> -EIO\n", ^ drivers/gpu//drm/i915/i915_reset.c:804:12: error: format '%lld' expects argument of type 'long long int', but argument 5 has type 'unsigned int' [-Werror=format=] GEM_TRACE("%s fence %llx:%lld -> -EIO\n", ^~ drivers/gpu//drm/i915/i915_reset.c:806:29: request->fence.context, request->fence.seqno); include/linux/kernel.h:736:29: note: in definition of macro 'do_trace_printk' __trace_printk(_THIS_IP_, fmt, ##args); \ ^~~ drivers/gpu//drm/i915/i915_gem.h:66:24: note: in expansion of macro 'trace_printk' #define GEM_TRACE(...) trace_printk(__VA_ARGS__) ^~~~ drivers/gpu//drm/i915/i915_reset.c:804:2: note: in expansion of macro 'GEM_TRACE' GEM_TRACE("%s fence %llx:%lld -> -EIO\n", ^ cc1: all warnings being treated as errors vim +689 drivers/gpu//drm/i915/i915_reset.c 659 660
[PATCH] drm/komeda: Fix a signedness bug
The mdev->irq value comes from platform_get_irq() so it can't be more than INT_MAX and if it's unsigned then it breaks the error handling in komeda_parse_dt(). Fixes: 29e56aec911d ("drm/komeda: Add DT parsing") Signed-off-by: Dan Carpenter --- drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h index a0bf7050037a..a52da6180c69 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h @@ -82,7 +82,7 @@ struct komeda_dev { struct clk *mclk; /** @irq: irq number */ - u32 irq; + int irq; int n_pipelines; struct komeda_pipeline *pipelines[KOMEDA_MAX_PIPELINES]; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 8/9] phy: Add Cadence D-PHY support
Hi Sean, On Thu, Jan 17, 2019 at 08:53:22AM -0500, Sean Paul wrote: > On Wed, Jan 09, 2019 at 10:33:25AM +0100, Maxime Ripard wrote: > > + opts->wakeup = cdns_dphy_get_wakeup_time_ns(dphy) * 1000; > > This should be "/ 1000" since the units of wakeup is us now (thanks to patch > 2). > You've made this change in patch 9, so I think it's just a misplaced fixup. Good catch, this was fixed in patch 9, but it definitely belongs there. Thanks! 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 09/11] drm/tinydrm: Remove tinydrm_device
Den 21.01.2019 10.29, skrev Daniel Vetter: > On Sun, Jan 20, 2019 at 12:43:16PM +0100, Noralf Trønnes wrote: >> No more users left so it can go alongside its helpers. >> Update the tinydrm docs description and remove todo entry. >> >> Signed-off-by: Noralf Trønnes >> --- >> Documentation/gpu/tinydrm.rst | 26 +-- >> Documentation/gpu/todo.rst| 4 - >> drivers/gpu/drm/tinydrm/core/Makefile | 2 +- >> drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 169 -- >> .../gpu/drm/tinydrm/core/tinydrm-helpers.c| 2 + >> include/drm/tinydrm/tinydrm.h | 42 - >> 6 files changed, 10 insertions(+), 235 deletions(-) >> delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c >> delete mode 100644 include/drm/tinydrm/tinydrm.h > > Looks great. Acked-by: Daniel Vetter I'm glad that I've finally come to the point where I'm able to get rid of the midlayer smell that you have talked about :-) >> >> diff --git a/Documentation/gpu/tinydrm.rst b/Documentation/gpu/tinydrm.rst >> index 1ca726474af4..19969b989efb 100644 >> --- a/Documentation/gpu/tinydrm.rst >> +++ b/Documentation/gpu/tinydrm.rst >> @@ -1,24 +1,12 @@ >> -== >> -drm/tinydrm Driver library >> -== >> + >> +drm/tinydrm Tiny DRM drivers >> + >> >> -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c >> - :doc: overview >> +tinydrm is a collection of DRM drivers that are so small they can fit in a >> +single source file. > > Since it's now just a collection of tiny drivers I think we should also > rename the directory. Maybe in a follow-up patch, once this has all > settled. I think just /tiny/ or /tinydrivers/ or so would be better. >> >> -Core functionality >> -== >> - >> -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c >> - :doc: core >> - >> -.. kernel-doc:: include/drm/tinydrm/tinydrm.h >> - :internal: >> - >> -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c >> - :export: > > Yay! > >> - >> -Additional helpers >> -== >> +Helpers >> +=== >> >> .. kernel-doc:: include/drm/tinydrm/tinydrm-helpers.h >> :internal: > > Out of curiosity, what's left? From a quick look I think we could move the > memcp/swab helpers into a drm_framebuffer_helper.c file (maybe move the fb > argument first, since that's the main thing for ocd). Yep. > > And the spi stuff is kinda just spi helpers I guess? Could we move those > into the spi subsystem, or was that idea already nacked? Meghana started on this, but didn't finish. I don't think there was any blockers, so it's just to pick it up where she left it. There's also the mipi_dbi helper which should be moved along side drm_mipi_dsi. My plan is to do this when I'm done with the modesetting part of drm_client. Noralf. > >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst >> index 38360ede1221..3495aec7a8d4 100644 >> --- a/Documentation/gpu/todo.rst >> +++ b/Documentation/gpu/todo.rst >> @@ -435,10 +435,6 @@ those drivers as simple as possible, so lots of room >> for refactoring: >>one of the ideas for having a shared dsi/dbi helper, abstracting away the >>transport details more. >> >> -- Quick aside: The unregister devm stuff is kinda getting the lifetimes of >> - a drm_device wrong. Doesn't matter, since everyone else gets it wrong >> - too :-) > > Hey I even wrote this already :-) > >> - >> Contact: Noralf Trønnes, Daniel Vetter >> >> AMD DC Display Driver >> diff --git a/drivers/gpu/drm/tinydrm/core/Makefile >> b/drivers/gpu/drm/tinydrm/core/Makefile >> index bf2df7326df7..f88ea7ad302f 100644 >> --- a/drivers/gpu/drm/tinydrm/core/Makefile >> +++ b/drivers/gpu/drm/tinydrm/core/Makefile >> @@ -1,3 +1,3 @@ >> -tinydrm-y := tinydrm-core.o tinydrm-helpers.o >> +tinydrm-y := tinydrm-helpers.o >> >> obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o >> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c >> b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c >> deleted file mode 100644 >> index e4a77feaacd6.. >> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c >> +++ /dev/null >> @@ -1,169 +0,0 @@ >> -/* >> - * Copyright (C) 2016 Noralf Trønnes >> - * >> - * This program is free software; you can redistribute it and/or modify >> - * it under the terms of the GNU General Public License as published by >> - * the Free Software Foundation; either version 2 of the License, or >> - * (at your option) any later version. >> - */ >> - >> -#include >> -#include >> -#include >> -#include >> -#include >> -#include >> -#include >> -#include >> -#include >> -#include >> -#include >> - >> -/** >> - * DOC: overview >> - * >> - * This library provides driver helpers for very simple display hardware. >> - * >> - * It is based on &drm_simple_display_pipe coupled with a &drm_connector >> which >> - * has o
Re: [linux-sunxi] Re: HDMI/DVI spurious failure
On Fri, Jan 18, 2019 at 02:51:26PM +, Priit Laes wrote: > On Fri, Jan 18, 2019 at 03:04:18PM +0100, Maxime Ripard wrote: > > On Fri, Jan 18, 2019 at 10:10:53AM +, Priit Laes wrote: > > > > > > > > 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? > > > > > > Can't really test this display without this patch and I do not have other > > > HDMI/DVI screens. And this issue does not happen with other HDMI displays > > > that I have here. > > > > Can't you just force the monitor to be reported as present? It's not > > great and we don't want to merge it, but that would allow you to test > > that setup without too many interferences. > > Baseline is clean u-boot / linux. U-boot does not detect/enable display. > > 1) Booting Linux with drm.debug=0xe > > * Linux does not detect/enable display > > 2) Booting with drm.debug=0xe video=HDMI-A-1:640x480@60e > > * Linux detects display, but display is garbled, and proper edid data is > detected: > > [snip] > pll-video1 000 32700 0 > 0 5 >pll-video1-2x 000 65400 0 > 0 5 > hdmi-tmds00025153846 0 > 0 5 > hdmi-ddc 000 89835 0 > 0 5 > [/snip] > > 3) Booting with drm.debug=0xe video=HDMI-A-1:640x480@60e > And also one extra patch for Linux where HDMI DDC clock marked as critical > > Linux detects and initializes display properly: > [snip] > pll-video1 110 32700 0 > 0 5 >pll-video1-2x 110 65400 0 > 0 5 > hdmi-tmds11025153846 0 > 0 5 > hdmi-ddc 110 89835 0 > 0 5 > [/snip] I guess you'll need to track down when the hdmi-tmds and hdmi-ddc are enabled and disabled, and if it makes sense :/ 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 00/11] drm/tinydrm: Remove tinydrm_device
Den 21.01.2019 09.34, skrev Sam Ravnborg: > Hi Noralf. > > On Sun, Jan 20, 2019 at 12:43:07PM +0100, Noralf Trønnes wrote: >> This patchset is part of the effort to remove tinydrm.ko. It removes >> struct tinydrm_device and tinydrm.h. >> >> While doing this refactoring I have ensured that device unplug is >> working. > > Very nice series, it looks good the way the core get a little extra > functionality thus the tinydrm drivers are now more normal drivers. Yes, I'm happy to see these drivers act more like normal drivers now. Daniel wasn't happy about the tinydrm midlayer so I'm glad it's going away. > > A few comments for some patches, and those where I felt confident > that I could follow the code got a "Reviewed-by:". Thanks for going through the patches. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/11] drm/tinydrm/repaper: Use devm_drm_dev_*()
Den 20.01.2019 23.25, skrev Sam Ravnborg: > On Sun, Jan 20, 2019 at 11:22:36PM +0100, Sam Ravnborg wrote: >> Hi Noralf. >> >> On Sun, Jan 20, 2019 at 12:43:14PM +0100, Noralf Trønnes wrote: >>> Use devm_drm_dev_init(), devm_drm_dev_register_with_fbdev() and drop >>> using tinydrm_device. >>> >>> Signed-off-by: Noralf Trønnes >>> --- >>> drivers/gpu/drm/tinydrm/repaper.c | 63 --- >>> 1 file changed, 40 insertions(+), 23 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/tinydrm/repaper.c >>> b/drivers/gpu/drm/tinydrm/repaper.c >>> index 3141241ca8f0..4c8205565668 100644 >>> --- a/drivers/gpu/drm/tinydrm/repaper.c >>> +++ b/drivers/gpu/drm/tinydrm/repaper.c >>> @@ -34,7 +34,7 @@ >>> #include >>> #include >>> #include >>> -#include >>> +#include >>> #include >>> >>> #define REPAPER_RID_G2_COG_ID 0x12 >>> @@ -60,7 +60,8 @@ enum repaper_epd_border_byte { >>> }; >>> >>> struct repaper_epd { >>> - struct tinydrm_device tinydrm; >>> + struct drm_device base; >>> + struct drm_simple_display_pipe pipe; >>> struct spi_device *spi; >>> >>> struct gpio_desc *panel_on; >>> @@ -89,10 +90,9 @@ struct repaper_epd { >>> bool partial; >>> }; >>> >>> -static inline struct repaper_epd * >>> -epd_from_tinydrm(struct tinydrm_device *tdev) >>> +static inline struct repaper_epd *drm_to_epd(struct drm_device *drm) >>> { >>> - return container_of(tdev, struct repaper_epd, tinydrm); >>> + return container_of(drm, struct repaper_epd, base); >>> } >>> >>> static int repaper_spi_transfer(struct spi_device *spi, u8 header, >>> @@ -530,8 +530,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb) >>> { >>> struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); >>> struct dma_buf_attachment *import_attach = cma_obj->base.import_attach; >>> - struct tinydrm_device *tdev = fb->dev->dev_private; >>> - struct repaper_epd *epd = epd_from_tinydrm(tdev); >>> + struct repaper_epd *epd = drm_to_epd(fb->dev); >>> struct drm_rect clip; >>> u8 *buf = NULL; >>> int ret = 0; >>> @@ -646,8 +645,7 @@ static void repaper_pipe_enable(struct >>> drm_simple_display_pipe *pipe, >>> struct drm_crtc_state *crtc_state, >>> struct drm_plane_state *plane_state) >>> { >>> - struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); >>> - struct repaper_epd *epd = epd_from_tinydrm(tdev); >>> + struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev); >>> struct spi_device *spi = epd->spi; >>> struct device *dev = &spi->dev; >>> bool dc_ok = false; >>> @@ -781,8 +779,7 @@ static void repaper_pipe_enable(struct >>> drm_simple_display_pipe *pipe, >>> >>> static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe) >>> { >>> - struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); >>> - struct repaper_epd *epd = epd_from_tinydrm(tdev); >>> + struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev); >>> struct spi_device *spi = epd->spi; >>> unsigned int line; >>> >>> @@ -856,6 +853,23 @@ static const struct drm_simple_display_pipe_funcs >>> repaper_pipe_funcs = { >>> .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, >>> }; >>> >>> +static const struct drm_mode_config_funcs repaper_mode_config_funcs = { >>> + .fb_create = drm_gem_fb_create_with_dirty, >>> + .atomic_check = drm_atomic_helper_check, >>> + .atomic_commit = drm_atomic_helper_commit, >>> +}; >>> + >>> +static void repaper_release(struct drm_device *drm) >>> +{ >>> + struct repaper_epd *epd = drm_to_epd(drm); >>> + >>> + DRM_DEBUG_DRIVER("\n"); >>> + >>> + drm_mode_config_cleanup(drm); >>> + drm_dev_fini(drm); >>> + kfree(epd); >>> +} >>> + >>> static const uint32_t repaper_formats[] = { >>> DRM_FORMAT_XRGB, >>> }; >>> @@ -894,6 +908,7 @@ static struct drm_driver repaper_driver = { >>> .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | >>> DRIVER_ATOMIC, >>> .fops = &repaper_fops, >>> + .release= repaper_release, >>> DRM_GEM_CMA_VMAP_DRIVER_OPS, >>> .name = "repaper", >>> .desc = "Pervasive Displays RePaper e-ink panels", >>> @@ -927,7 +942,6 @@ static int repaper_probe(struct spi_device *spi) >>> const struct of_device_id *match; >>> struct drm_connector *connector; >>> struct device *dev = &spi->dev; >>> - struct tinydrm_device *tdev; >>> enum repaper_model model; >>> const char *thermal_zone; >>> struct repaper_epd *epd; >>> @@ -952,10 +966,20 @@ static int repaper_probe(struct spi_device *spi) >>> } >>> } >>> >>> - epd = devm_kzalloc(dev, sizeof(*epd), GFP_KERNEL); >>> + epd = kzalloc(sizeof(*epd), GFP_KERNEL); >>> if (!epd) >>> return -ENOMEM; >>> >>> + drm = &epd->base; >>> + ret = devm_drm_dev_init(dev, drm, &repaper_driver); >>> + if
Re: [PATCH 01/11] drm: Add devm_drm_dev_init/register
Den 21.01.2019 07.11, skrev Sam Ravnborg: > Hi Noralf. > > On Sun, Jan 20, 2019 at 12:43:08PM +0100, Noralf Trønnes wrote: >> This adds resource managed (devres) versions of drm_dev_init() and >> drm_dev_register(). >> >> Also added is devm_drm_dev_register_with_fbdev() which sets up generic >> fbdev emulation as well. >> >> devm_drm_dev_register() isn't exported since there are no users. > Is it relevant to use it outside this patchset - then it should be > documented/exported now to enable use. As a rule we don't export functions that doesn't have any users. It can be exported when a user shows up. But maybe it would make sense to put all the documentation in devm_drm_dev_register() instead of duplicating it all in the two functions when they are both in use. Or it can be moved to devm_drm_dev_register() when it gets a user. I'm good with either, don't know what Daniel prefers. > >> >> Signed-off-by: Noralf Trønnes >> --- >> Documentation/driver-model/devres.txt | 4 + >> drivers/gpu/drm/drm_drv.c | 106 ++ >> include/drm/drm_drv.h | 6 ++ >> 3 files changed, 116 insertions(+) >> >> diff --git a/Documentation/driver-model/devres.txt >> b/Documentation/driver-model/devres.txt >> index b277cafce71e..6eebc28d4c21 100644 >> --- a/Documentation/driver-model/devres.txt >> +++ b/Documentation/driver-model/devres.txt >> @@ -254,6 +254,10 @@ DMA >>dmam_pool_create() >>dmam_pool_destroy() >> >> +DRM >> + devm_drm_dev_init() >> + devm_drm_dev_register_with_fbdev() >> + >> GPIO >>devm_gpiod_get() >>devm_gpiod_get_index() >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >> index 381581b01d48..12129772be45 100644 >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c >> @@ -36,6 +36,7 @@ >> >> #include >> #include >> +#include >> #include >> >> #include "drm_crtc_internal.h" >> @@ -871,6 +872,111 @@ void drm_dev_unregister(struct drm_device *dev) >> } >> EXPORT_SYMBOL(drm_dev_unregister); >> >> +static void devm_drm_dev_init_release(void *data) >> +{ >> +drm_dev_put(data); >> +} >> + >> +/** >> + * devm_drm_dev_init - Resource managed drm_dev_init() >> + * @parent: Parent device object >> + * @dev: DRM device >> + * @driver: DRM driver >> + * >> + * Managed drm_dev_init(). The DRM device initialized with this function is >> + * automatically released on driver detach. You must supply a >> + * &drm_driver.release callback to control the finalization explicitly. >> + * >> + * Note: This function must be used together with >> + * devm_drm_dev_register_with_fbdev(). > *must* be used, or can be used? > Reading the code this looks like something drivers that do not offer > fbdev emulation could also benefit from. They must be used together to not break ref counting. When devm_drm_dev_register() gets a user it will read: * Note: This function must be used together with * devm_drm_dev_register() or devm_drm_dev_register_with_fbdev(). Noralf. > > With the two comments processed this has: > Reviewed-by: Sam Ravnborg > > Sam > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/11] drm: Add devm_drm_dev_init/register
Den 21.01.2019 10.55, skrev Daniel Vetter: > On Mon, Jan 21, 2019 at 10:10:14AM +0100, Daniel Vetter wrote: >> On Sun, Jan 20, 2019 at 12:43:08PM +0100, Noralf Trønnes wrote: >>> This adds resource managed (devres) versions of drm_dev_init() and >>> drm_dev_register(). >>> >>> Also added is devm_drm_dev_register_with_fbdev() which sets up generic >>> fbdev emulation as well. >>> >>> devm_drm_dev_register() isn't exported since there are no users. >>> >>> Signed-off-by: Noralf Trønnes >> >>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >>> index 381581b01d48..12129772be45 100644 >>> --- a/drivers/gpu/drm/drm_drv.c >>> +++ b/drivers/gpu/drm/drm_drv.c >>> @@ -36,6 +36,7 @@ >>> >>> #include >>> #include >>> +#include >>> #include >>> >>> #include "drm_crtc_internal.h" >>> @@ -871,6 +872,111 @@ void drm_dev_unregister(struct drm_device *dev) >>> } >>> EXPORT_SYMBOL(drm_dev_unregister); >>> >>> +static void devm_drm_dev_init_release(void *data) >>> +{ >>> + drm_dev_put(data); > > We need drm_dev_unplug() here, or this isn't safe. This function is only used to cover the error path if probe fails before devm_drm_dev_register() is called. devm_drm_dev_register_release() is the one that calls unplug. There are comments about this in the functions. > >>> +} >>> + >>> +/** >>> + * devm_drm_dev_init - Resource managed drm_dev_init() >>> + * @parent: Parent device object >>> + * @dev: DRM device >>> + * @driver: DRM driver >>> + * >>> + * Managed drm_dev_init(). The DRM device initialized with this function is >>> + * automatically released on driver detach. You must supply a > > I think a bit more clarity here would be good: > > "... automatically released on driver unbind by callind drm_dev_unplug()." > >>> + * &drm_driver.release callback to control the finalization explicitly. > > I think a loud warning for these is in order: > > "WARNING: > > "In generally it is unsafe to use devm functions for drm structures > because the lifetimes of &drm_device and the underlying &device do not > match. This here works because it doesn't immediately free anything, but > only calls drm_dev_unplug(), which internally decrements the &drm_device > refcount through drm_dev_put(). > > "All other drm structures must still be explicitly released in the > &drm_driver.release callback." > > While thinking about this I just realized that with this design we have no > good place to call drm_atomic_helper_shutdown(). Which we need to, or all > kinds of things will leak badly (connectors, fb, ...), but there's no > place to call it: > - unbind is too early, since we haven't yet called drm_dev_unplug, and the > drm_dev_unregister in there must be called _before_ we start to shut > down anything. > - drm_driver.release is way too late. > > Ofc for a real hotunplug there's no point in shutting down the hw (it's > already gone), but for a driver unload/unbind it would be nice if this > happens automatically and in the right order. > > So not sure what to do here really. How about this change: (it breaks the rule of pulling helpers into the core, so maybe we should put the devm_ functions into the simple KMS helper instead?) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 12129772be45..7ed9550baff6 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -34,7 +34,9 @@ #include #include +#include #include +#include #include #include #include @@ -355,17 +357,7 @@ void drm_dev_exit(int idx) } EXPORT_SYMBOL(drm_dev_exit); -/** - * drm_dev_unplug - unplug a DRM device - * @dev: DRM device - * - * This unplugs a hotpluggable DRM device, which makes it inaccessible to - * userspace operations. Entry-points can use drm_dev_enter() and - * drm_dev_exit() to protect device resources in a race free manner. This - * essentially unregisters the device like drm_dev_unregister(), but can be - * called while there are still open users of @dev. - */ -void drm_dev_unplug(struct drm_device *dev) +static void __drm_dev_unplug(struct drm_device *dev, bool shutdown) { /* * After synchronizing any critical read section is guaranteed to see @@ -378,11 +370,32 @@ void drm_dev_unplug(struct drm_device *dev) drm_dev_unregister(dev); + if (shutdown) + drm_kms_helper_poll_fini(dev); + mutex_lock(&drm_global_mutex); - if (dev->open_count == 0) + if (dev->open_count == 0) { + if (shutdown) + drm_atomic_helper_shutdown(dev); drm_dev_put(dev); + } mutex_unlock(&drm_global_mutex); } + +/** + * drm_dev_unplug - unplug a DRM device + * @dev: DRM device + * + * This unplugs a hotpluggable DRM device, which makes it inaccessible to + * userspace operations. Entry-points can use drm_dev_enter() and + * drm_dev_exit() to protect device resources in a race free manner. This + * essentially unregisters the device like drm_dev_unreg
Re: [PATCH 4/4] staging: android: ion: Support for mapping with dma mapping attributes
Hi Liam, On Fri, Jan 18, 2019 at 10:37:47AM -0800, Liam Mark wrote: > Add support for configuring dma mapping attributes when mapping > and unmapping memory through dma_buf_map_attachment and > dma_buf_unmap_attachment. > > For example this will allow ION clients to skip cache maintenance, by > using DMA_ATTR_SKIP_CPU_SYNC, for buffers which are clean and haven't been > accessed by the CPU. How can a client know that the buffer won't be accessed by the CPU in the future though? I don't think we can push this decision to clients, because they are lacking information about what else is going on with the buffer. It needs to be done by the exporter, IMO. Thanks, -Brian > > Signed-off-by: Liam Mark > --- > 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 1fe633a7fdba..0aae845b20ba 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -268,8 +268,8 @@ static struct sg_table *ion_map_dma_buf(struct > dma_buf_attachment *attachment, > table = a->table; > > mutex_lock(&buffer->lock); > - if (!dma_map_sg(attachment->dev, table->sgl, table->nents, > - direction)) { > + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents, > + direction, attachment->dma_map_attrs)) { > mutex_unlock(&buffer->lock); > return ERR_PTR(-ENOMEM); > } > @@ -287,7 +287,8 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment > *attachment, > struct ion_buffer *buffer = attachment->dmabuf->priv; > > mutex_lock(&buffer->lock); > - dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); > + dma_unmap_sg_attrs(attachment->dev, table->sgl, table->nents, direction, > +attachment->dma_map_attrs); > a->dma_mapped = false; > mutex_unlock(&buffer->lock); > } > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: Make igts for cross-driver stuff mandatory
Hi Daniel, 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. 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 > I share a similar view to Liviu here. I think this new requirement raises the bar more than you intended. By saying that all new features must be tested by igt, you're also implying that a driver must run igt (at some basic level); before the developers working on that driver can start trying to implement new features. That puts an additional hurdle in the way of adding stuff to KMS for people who aren't already using igt. I'm all for testing, and UAPI being well proven before we merge it, and even for a central KMS test suite. However, when we (Arm Mali-DP people) have tried to implement things in igt it's been a battle, because of various built-in assumptions which it made. For example, most meaningful igt tests rely on CRC. Much of our HW doesn't have CRC. CRC could be implemented in theory using writeback, but that currently doesn't exist. That means you're effectively saying that we (Arm) can't implement any new cross-device KMS features until we've either: a) also implemented writeback-based CRC in igt OR b) implemented the new feature in someone else's driver which does support CRC. That seems a bit out of order to me. It would be like me saying "all KMS drivers must use Arm's test suite, which uses writeback and pixel checking", and you'd be in a pickle because you don't have writeback. In a similar vein, I remember having to fix igt on devices which didn't have cursor planes, before I could even think about writing tests. If you can prove that issues like these are all resolved now in igt, then great! Otherwise, I don't think igt is "ready" to be used as a requirement for merging KMS kernel API. Thanks, -Brian > > > > 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 > >
[Bug 109384] Bad DisplayPort enumeration and wrong refresh rate with Hawaii chip
https://bugs.freedesktop.org/show_bug.cgi?id=109384 Michel Dänzer changed: What|Removed |Added CC||harry.wentl...@amd.com, ||nicholas.kazlaus...@amd.com --- Comment #6 from Michel Dänzer --- (In reply to fabio0x from comment #0) > - I get microstuttering, particularly noticeable when I drag windows around. See bug 106175. -- 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/dp: use DRM_DEBUG_DP() instead of drm_dbg for logging
We have a wrapper for a reason. Signed-off-by: Jani Nikula --- drivers/gpu/drm/drm_dp_helper.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 26835d174939..4def0bface85 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -194,11 +194,11 @@ drm_dp_dump_access(const struct drm_dp_aux *aux, const char *arrow = request == DP_AUX_NATIVE_READ ? "->" : "<-"; if (ret > 0) - drm_dbg(DRM_UT_DP, "%s: 0x%05x AUX %s (ret=%3d) %*ph\n", - aux->name, offset, arrow, ret, min(ret, 20), buffer); + DRM_DEBUG_DP("%s: 0x%05x AUX %s (ret=%3d) %*ph\n", +aux->name, offset, arrow, ret, min(ret, 20), buffer); else - drm_dbg(DRM_UT_DP, "%s: 0x%05x AUX %s (ret=%3d)\n", - aux->name, offset, arrow, ret); + DRM_DEBUG_DP("%s: 0x%05x AUX %s (ret=%3d)\n", +aux->name, offset, arrow, ret); } /** -- 2.20.1 ___ 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
Hi, Sorry for being a bit sporadic on this. I was out travelling last week with little time for email. On Fri, Jan 18, 2019 at 11:16:31AM -0600, Andrew F. Davis wrote: > On 1/17/19 7:11 PM, Liam Mark wrote: > > On Thu, 17 Jan 2019, Andrew F. Davis wrote: > > > >> 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).
[Bug 109403] amdgpu randomly hangs while streaming or when CPU is busy on X399 with TR 1950X
https://bugs.freedesktop.org/show_bug.cgi?id=109403 Ivan Avdeev <1...@provod.gl> changed: What|Removed |Added CC||1...@provod.gl --- Comment #1 from Ivan Avdeev <1...@provod.gl> --- Created attachment 143176 --> https://bugs.freedesktop.org/attachment.cgi?id=143176&action=edit dmesg-5.0-rc2 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109403] amdgpu randomly hangs while streaming or when CPU is busy on X399 with TR 1950X
https://bugs.freedesktop.org/show_bug.cgi?id=109403 Bug ID: 109403 Summary: amdgpu randomly hangs while streaming or when CPU is busy on X399 with TR 1950X Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: 1...@provod.gl I've been experiencing random GPU hangs since I upgraded to Threadripper about a year ago. Specs: - Motherboard: ASUS Prime X399-A, all bios versions from stock until current 0808 - CPU: Threadripper 1950X, 32 threads - GPU: MSI Radeon RX Vega 64 Air Boost 8G OC (was also happening on ASUS R9 Fury X on the same machine; this GPU was generally stable on previous box) - Displays: - 2x DELL U2412M 1920x1200x60 (DP) - 1x ASUS MG279Q 2560x1440x144 (DP) - Kernel versions: 4.20, 5.0-rc2 (has been happening since from at least 4.14; earlier versions weren't tried). - linux-firmware: 20181218 - Mesa: 18.3.1 - X: 1.20.3 - libdrm: 2.4.96 - Possibly relevant kernel options: amd_iommu=on vfio-pci.ids=10de:1005,10de:0e1a,1912:0014,1106:3483 iommu=pt vfio-pci.disable_vga=1 hpet=disable nohpet amdgpu.ppfeaturemask=0xfffd7fff amdgpu.gpu_recovery=1 pcie_aspm=off The problem manifests itself usually like this: 1. Screen suddenly freezes (sometimes it is possible to move mouse cursor for a few seconds, but it will freeze eventually too) 2. GPU fan speeds up and remain high 3. Every process that talks to GPU freezes and becomes impossible to kill. 4. Can SSH into the machine and everything else besides the GPU works ok. 5. dmesg contains a message like this: [Jan21 00:03] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=17188686, emitted seq=17188689 [ +0.32] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process X pid 9315 thread X:cs0 pid 9335 or with a bit more stuff happening before: [Jan18 19:43] amdgpu :44:00.0: [gfxhub] VMC page fault (src_id:0 ring:158 vmid:6 pasid:32771, for process superposition pid 11225 thread superposit:cs0 pid 11308) [ +0.03] amdgpu :44:00.0: in page starting at address 0x800010607000 from 27 [ +0.02] amdgpu :44:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x0060153D [ +0.05] amdgpu :44:00.0: [gfxhub] VMC page fault (src_id:0 ring:158 vmid:6 pasid:32771, for process superposition pid 11225 thread superposit:cs0 pid 11308) [ +0.02] amdgpu :44:00.0: in page starting at address 0x800010609000 from 27 [ +0.01] amdgpu :44:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x [ +0.04] amdgpu :44:00.0: [gfxhub] VMC page fault (src_id:0 ring:158 vmid:6 pasid:32771, for process superposition pid 11225 thread superposit:cs0 pid 11308) [ +0.01] amdgpu :44:00.0: in page starting at address 0x800010607000 from 27 [ +0.02] amdgpu :44:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x [ +0.04] amdgpu :44:00.0: [gfxhub] VMC page fault (src_id:0 ring:158 vmid:6 pasid:32771, for process superposition pid 11225 thread superposit:cs0 pid 11308) [ +0.01] amdgpu :44:00.0: in page starting at address 0x800010609000 from 27 [ +0.01] amdgpu :44:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x [ +0.04] amdgpu :44:00.0: [gfxhub] VMC page fault (src_id:0 ring:158 vmid:6 pasid:32771, for process superposition pid 11225 thread superposit:cs0 pid 11308) [ +0.02] amdgpu :44:00.0: in page starting at address 0x800010607000 from 27 [ +0.01] amdgpu :44:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x [ +0.04] amdgpu :44:00.0: [gfxhub] VMC page fault (src_id:0 ring:158 vmid:6 pasid:32771, for process superposition pid 11225 thread superposit:cs0 pid 11308) [ +0.01] amdgpu :44:00.0: in page starting at address 0x800010609000 from 27 [ +0.01] amdgpu :44:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x [ +0.04] amdgpu :44:00.0: [gfxhub] VMC page fault (src_id:0 ring:158 vmid:6 pasid:32771, for process superposition pid 11225 thread superposit:cs0 pid 11308) [ +0.01] amdgpu :44:00.0: in page starting at address 0x800010607000 from 27 [ +0.01] amdgpu :44:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x [ +0.04] amdgpu :44:00.0: [gfxhub] VMC page fault (src_id:0 ring:158 vmid:6 pasid:32771, for process superposition pid 11225 thread superposit:cs0 pid 11308) [ +0.02
Re: [PATCH/RFC] drm: Remove unused Renesas SH Mobile DRM driver
On Mon, Jan 21, 2019 at 11:03:44AM +0100, Geert Uytterhoeven wrote: > Hi Daniel, > > On Mon, Jan 21, 2019 at 10:35 AM Daniel Vetter wrote: > > On Fri, Jan 18, 2019 at 05:22:58PM +0100, Geert Uytterhoeven wrote: > > > Since its incarnation in v3.7 almost 7 years ago, no users of the SH > > > Mobile DRM driver have appeared. > > > > > > Hence remove the driver. It can be resurrected from git history, > > > if/when needed. > > > > > > Signed-off-by: Geert Uytterhoeven > > > > I'd prefer removing the fbdev variant tbh ... > > I understand ;-) > > But sh_mobile_lcdc_fb is used on 5 legacy SuperH platforms, and 1 ARM > platform. At least the latter is working fine. > > > Not sure why exactly the switch never happened. > > Me neither. I^HGoogle can't even find posted patches of board integration. > > Note that both drivers lack DT support, which would be very desirable on > ARM, as graphics is the single remaining working driver on the Atmark > Techno Armadillo 800 EVA board not using DT. Ah I wondered why the drm driver isn't just automatically picked up in today's neat world of DT (if you enable it in .config instead of the fbdev one). It's not even using DT yet :-/ -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
Am 21.01.19 um 11:06 schrieb Ard Biesheuvel: > Currently, the DRM code assumes that PCI devices are always cache > coherent for DMA, and that this can be selectively overridden for > some buffers using non-cached mappings on the CPU side and PCIe > NoSnoop transactions on the bus side. > > Whether the NoSnoop part is implemented correctly is highly platform > specific. Whether it /matters/ if NoSnoop is implemented correctly or > not is architecture specific: on x86, such transactions are coherent > with the CPU whether the NoSnoop attribute is honored or not. On other > architectures, it depends on whether such transactions may allocate in > caches that are non-coherent with the CPU's uncached mappings. > > Bottom line is that we should not rely on this optimization to work > correctly for cache coherent devices in the general case. On the > other hand, disabling this optimization for non-coherent devices > is likely to cause breakage as well, since the driver will assume > cache coherent PCIe if this optimization is turned off. > > So rename drm_arch_can_wc_memory() to drm_device_can_wc_memory(), and > pass the drm_device pointer into it so we can base the return value > on whether the device is cache coherent or not if not running on > X86. > > Cc: Christian Koenig > Cc: Alex Deucher > Cc: David Zhou > Cc: Huang Rui > Cc: Junwei Zhang > Cc: Michel Daenzer > Cc: David Airlie > Cc: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Will Deacon > Reported-by: Carsten Haitzler > Signed-off-by: Ard Biesheuvel Looks sane to me, but I can't fully judge if the check is actually correct or not. So Acked-by: Christian König > --- > This is a followup to '[RFC PATCH] drm/ttm: force cached mappings for system > RAM on ARM' > > https://lore.kernel.org/linux-arm-kernel/20190110072841.3283-1-ard.biesheu...@linaro.org/ > > Without t > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- > drivers/gpu/drm/radeon/radeon_object.c | 2 +- > include/drm/drm_cache.h| 19 +++ > 3 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index 728e15e5d68a..777fa251838f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -480,7 +480,7 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, > /* For architectures that don't support WC memory, >* mask out the WC flag from the BO >*/ > - if (!drm_arch_can_wc_memory()) > + if (!drm_device_can_wc_memory(adev->ddev)) > bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC; > #endif > > diff --git a/drivers/gpu/drm/radeon/radeon_object.c > b/drivers/gpu/drm/radeon/radeon_object.c > index 833e909706a9..610889bf6ab5 100644 > --- a/drivers/gpu/drm/radeon/radeon_object.c > +++ b/drivers/gpu/drm/radeon/radeon_object.c > @@ -249,7 +249,7 @@ int radeon_bo_create(struct radeon_device *rdev, > /* For architectures that don't support WC memory, >* mask out the WC flag from the BO >*/ > - if (!drm_arch_can_wc_memory()) > + if (!drm_device_can_wc_memory(rdev->ddev)) > bo->flags &= ~RADEON_GEM_GTT_WC; > #endif > > diff --git a/include/drm/drm_cache.h b/include/drm/drm_cache.h > index bfe1639df02d..ced63b1207a3 100644 > --- a/include/drm/drm_cache.h > +++ b/include/drm/drm_cache.h > @@ -33,6 +33,8 @@ > #ifndef _DRM_CACHE_H_ > #define _DRM_CACHE_H_ > > +#include > +#include > #include > > void drm_clflush_pages(struct page *pages[], unsigned long num_pages); > @@ -41,15 +43,16 @@ void drm_clflush_virt_range(void *addr, unsigned long > length); > u64 drm_get_max_iomem(void); > > > -static inline bool drm_arch_can_wc_memory(void) > +static inline bool drm_device_can_wc_memory(struct drm_device *ddev) > { > -#if defined(CONFIG_PPC) && !defined(CONFIG_NOT_COHERENT_CACHE) > - return false; > -#elif defined(CONFIG_MIPS) && defined(CONFIG_CPU_LOONGSON3) > - return false; > -#else > - return true; > -#endif > + if (IS_ENABLED(CONFIG_PPC)) > + return IS_ENABLED(CONFIG_NOT_COHERENT_CACHE); > + else if (IS_ENABLED(CONFIG_MIPS)) > + return !IS_ENABLED(CONFIG_CPU_LOONGSON3); > + else if (IS_ENABLED(CONFIG_X86)) > + return true; > + > + return !dev_is_dma_coherent(ddev->dev); > } > > #endif ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH/RFC] drm: Remove unused Renesas SH Mobile DRM driver
Hi Daniel, On Mon, Jan 21, 2019 at 10:35 AM Daniel Vetter wrote: > On Fri, Jan 18, 2019 at 05:22:58PM +0100, Geert Uytterhoeven wrote: > > Since its incarnation in v3.7 almost 7 years ago, no users of the SH > > Mobile DRM driver have appeared. > > > > Hence remove the driver. It can be resurrected from git history, > > if/when needed. > > > > Signed-off-by: Geert Uytterhoeven > > I'd prefer removing the fbdev variant tbh ... I understand ;-) But sh_mobile_lcdc_fb is used on 5 legacy SuperH platforms, and 1 ARM platform. At least the latter is working fine. > Not sure why exactly the switch never happened. Me neither. I^HGoogle can't even find posted patches of board integration. Note that both drivers lack DT support, which would be very desirable on ARM, as graphics is the single remaining working driver on the Atmark Techno Armadillo 800 EVA board not using DT. 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
[Bug 201497] [amdgpu]: '*ERROR* No EDID read' is back in 4.19
https://bugzilla.kernel.org/show_bug.cgi?id=201497 Ivan Molodetskikh (yalt...@gmail.com) changed: What|Removed |Added CC||yalt...@gmail.com --- Comment #5 from Ivan Molodetskikh (yalt...@gmail.com) --- Created attachment 280619 --> https://bugzilla.kernel.org/attachment.cgi?id=280619&action=edit 4.20 log I'm hitting the same issue with one of my monitors (ASUS VG248QE, G-Sync modded). I tried extracting EDID from kernel 4.18 and forcing it with drm.edid_firmware as you can see in the log, but that doesn't seem to do anything. My GPU is AMD RX 580. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/11] drm: Add devm_drm_dev_init/register
On Mon, Jan 21, 2019 at 10:10:14AM +0100, Daniel Vetter wrote: > On Sun, Jan 20, 2019 at 12:43:08PM +0100, Noralf Trønnes wrote: > > This adds resource managed (devres) versions of drm_dev_init() and > > drm_dev_register(). > > > > Also added is devm_drm_dev_register_with_fbdev() which sets up generic > > fbdev emulation as well. > > > > devm_drm_dev_register() isn't exported since there are no users. > > > > Signed-off-by: Noralf Trønnes > > devm_ considered harmful unfortunately. You cannot allocate any structures > which might outlive the lifetime of your device using devm_ on the device. > Since drm_device has it's own lifetime, that structure and _everything_ we > allocate tied to it (i.e. drm_encoder/plane/connector/crtc/whatever) > cannot be allocated using devm_ infrastructure tied to the underlying > parent device. > > Yes this means almost all the devm usage in drm drivers is fundamentally > broken. You also generally don't unplug gpus, so no one cares :-/ > > What could instead is add a struct device to drm_device somehow, use that > struct device to manage the lifetime of the drm_device (would still need > our special open counter maybe), and then use devm to allocate all the drm > bits tied to the drm_device. > > This here unfortunataly doesn't work, even if it looks like a really neat > idea. Strike all that, because I wasn't awak yet. Sorry for the confusion. > > --- > > Documentation/driver-model/devres.txt | 4 + > > drivers/gpu/drm/drm_drv.c | 106 ++ > > include/drm/drm_drv.h | 6 ++ > > 3 files changed, 116 insertions(+) > > > > diff --git a/Documentation/driver-model/devres.txt > > b/Documentation/driver-model/devres.txt > > index b277cafce71e..6eebc28d4c21 100644 > > --- a/Documentation/driver-model/devres.txt > > +++ b/Documentation/driver-model/devres.txt > > @@ -254,6 +254,10 @@ DMA > >dmam_pool_create() > >dmam_pool_destroy() > > > > +DRM > > + devm_drm_dev_init() > > + devm_drm_dev_register_with_fbdev() > > + > > GPIO > >devm_gpiod_get() > >devm_gpiod_get_index() > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > index 381581b01d48..12129772be45 100644 > > --- a/drivers/gpu/drm/drm_drv.c > > +++ b/drivers/gpu/drm/drm_drv.c > > @@ -36,6 +36,7 @@ > > > > #include > > #include > > +#include > > #include > > > > #include "drm_crtc_internal.h" > > @@ -871,6 +872,111 @@ void drm_dev_unregister(struct drm_device *dev) > > } > > EXPORT_SYMBOL(drm_dev_unregister); > > > > +static void devm_drm_dev_init_release(void *data) > > +{ > > + drm_dev_put(data); We need drm_dev_unplug() here, or this isn't safe. > > +} > > + > > +/** > > + * devm_drm_dev_init - Resource managed drm_dev_init() > > + * @parent: Parent device object > > + * @dev: DRM device > > + * @driver: DRM driver > > + * > > + * Managed drm_dev_init(). The DRM device initialized with this function is > > + * automatically released on driver detach. You must supply a I think a bit more clarity here would be good: "... automatically released on driver unbind by callind drm_dev_unplug()." > > + * &drm_driver.release callback to control the finalization explicitly. I think a loud warning for these is in order: "WARNING: "In generally it is unsafe to use devm functions for drm structures because the lifetimes of &drm_device and the underlying &device do not match. This here works because it doesn't immediately free anything, but only calls drm_dev_unplug(), which internally decrements the &drm_device refcount through drm_dev_put(). "All other drm structures must still be explicitly released in the &drm_driver.release callback." While thinking about this I just realized that with this design we have no good place to call drm_atomic_helper_shutdown(). Which we need to, or all kinds of things will leak badly (connectors, fb, ...), but there's no place to call it: - unbind is too early, since we haven't yet called drm_dev_unplug, and the drm_dev_unregister in there must be called _before_ we start to shut down anything. - drm_driver.release is way too late. Ofc for a real hotunplug there's no point in shutting down the hw (it's already gone), but for a driver unload/unbind it would be nice if this happens automatically and in the right order. So not sure what to do here really. -Daniel > > + * > > + * Note: This function must be used together with > > + * devm_drm_dev_register_with_fbdev(). > > + * > > + * RETURNS: > > + * 0 on success, or error code on failure. > > + */ > > +int devm_drm_dev_init(struct device *parent, > > + struct drm_device *dev, > > + struct drm_driver *driver) > > +{ > > + int ret; > > + > > + if (WARN_ON(!parent || !driver->release)) > > + return -EINVAL; > > + > > + ret = drm_dev_init(dev, driver, parent); > > + if (ret) > > + return ret; > > + > > + /* > > +* This is a temporary release action that is
Re: [PATCH 2/4] drm/i915/dsi: Enable dithering for 6 bpc panels
Hi, On 15-01-19 15:55, Ville Syrjälä wrote: On Sat, Dec 01, 2018 at 12:31:46PM +0100, Hans de Goede wrote: The display engine has 2 dithering enable bits which both need to be set for dithering to happen, 1 in the PIPECONF register which is taken care of by i9xx_set_pipeconf() and a second bit at the encoder level. The dsi code was not setting the encoder level dithering enable bit causing dithering to be disabled, this commit fixes this. Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/vlv_dsi.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c index c10def5efa22..c21cbfa9653c 100644 --- a/drivers/gpu/drm/i915/vlv_dsi.c +++ b/drivers/gpu/drm/i915/vlv_dsi.c @@ -711,6 +711,10 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder, LANE_CONFIGURATION_DUAL_LINK_B : LANE_CONFIGURATION_DUAL_LINK_A; } + + if (intel_dsi->pixel_format != MIPI_DSI_FMT_RGB888) + temp |= DITHERING_ENABLE; The docs say this was only made to work in C0 stepping. Not sure any BYT-Ts were ever shipped with B2/3, nor am I sure if setting the bit would have any effect there. IMO let's just set the bit and hope for the best. Reviewed-by: Ville Syrjälä Thank you, I've pushed patches 1 and 2 of this series to dinq. Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/i915/dsi: Fix pipe_bpp for handling for 6 bpc pixel-formats
Hi, On 15-01-19 15:51, Ville Syrjälä wrote: On Sat, Dec 01, 2018 at 12:31:45PM +0100, Hans de Goede wrote: There are 3 problems with the dsi code's pipe_bpp handling for 6 bpc pixel-formats which this commit addresses: 1) It assumes that the pipe_bpp is the same as the bpp going over the dsi lanes. This assumption is not valid for MIPI_DSI_FMT_RGB666, where pipe_bpp should be 18 so that we do proper dithering but we actually send 24 bpp over the dsi lanes (MIPI_DSI_FMT_RGB666_PACKED sends 18 bpp). This assumption is enforced by an assert in *_dsi_get_pclk(). This assert triggers on the initial hw-state readback on BYT/CHT devices which use MIPI_DSI_FMT_RGB666, such as the Prowise PT301 tablet. PIPECONF is set to 6BPC / 18 bpp by the GOP, while mipi_dsi_pixel_format_to_bpp() returns 24. This commits switches the calculations in *_dsi_get_pclk() to use the bpp from mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format) which returns the bpp going over the mipi lanes and drops the assert. 2) On BXT bxt_dsi_get_pipe_config() wrongly overrides the pipe_bpp which i9xx_get_pipe_config() reads from PIPECONF with the return value from mipi_dsi_pixel_format_to_bpp(). This avoids the assert from 1. but is wrong since the pipe is actually running at the value configured in PIPECONF. This commit drops the override of pipe_bpp from bxt_dsi_get_pipe_config(). 3) The dsi encoder's compute_config() never assigns a value to pipe_bpp, unlike most other encoders. Falling back on compute_baseline_pipe_bpp() which always picks 24. 24 is only correct for MIPI_DSI_FMT_RGB88 for the others we should use 18 bpp so that we correctly do 6bpc color dithering. This commit adds code to intel_dsi_compute_config() to properly set pipe_bpp based on intel_dsi->pixel_format. Signed-off-by: Hans de Goede lgtm Reviewed-by: Ville Syrjälä Thank you. That said, I think we could make everything less confusing by doing something like this: compute_config() { port_clock = bitrate; } get_config() { port_clock = readout from pll; crtc_clock = derive from port_clock; } Currently the code assumes that port_clock == crtc_clock, if make port_clock reflect the actual pll clock, without compensating for number of lanes and bpp, I think we need to make changes in more places. Regards, Hans --- drivers/gpu/drm/i915/intel_dsi.h | 4 ++-- drivers/gpu/drm/i915/vlv_dsi.c | 17 drivers/gpu/drm/i915/vlv_dsi_pll.c | 31 ++ 3 files changed, 17 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h index c888c219835f..c796a2962a43 100644 --- a/drivers/gpu/drm/i915/intel_dsi.h +++ b/drivers/gpu/drm/i915/intel_dsi.h @@ -160,7 +160,7 @@ int vlv_dsi_pll_compute(struct intel_encoder *encoder, void vlv_dsi_pll_enable(struct intel_encoder *encoder, const struct intel_crtc_state *config); void vlv_dsi_pll_disable(struct intel_encoder *encoder); -u32 vlv_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp, +u32 vlv_dsi_get_pclk(struct intel_encoder *encoder, struct intel_crtc_state *config); void vlv_dsi_reset_clocks(struct intel_encoder *encoder, enum port port); @@ -170,7 +170,7 @@ int bxt_dsi_pll_compute(struct intel_encoder *encoder, void bxt_dsi_pll_enable(struct intel_encoder *encoder, const struct intel_crtc_state *config); void bxt_dsi_pll_disable(struct intel_encoder *encoder); -u32 bxt_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp, +u32 bxt_dsi_get_pclk(struct intel_encoder *encoder, struct intel_crtc_state *config); void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port); diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c index be3af5f6c7a0..c10def5efa22 100644 --- a/drivers/gpu/drm/i915/vlv_dsi.c +++ b/drivers/gpu/drm/i915/vlv_dsi.c @@ -322,6 +322,11 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder, /* DSI uses short packets for sync events, so clear mode flags for DSI */ adjusted_mode->flags = 0; + if (intel_dsi->pixel_format == MIPI_DSI_FMT_RGB888) + pipe_config->pipe_bpp = 24; + else + pipe_config->pipe_bpp = 18; + if (IS_GEN9_LP(dev_priv)) { /* Enable Frame time stamp based scanline reporting */ adjusted_mode->private_flags |= @@ -1097,10 +1102,8 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder, } fmt = I915_READ(MIPI_DSI_FUNC_PRG(port)) & VID_MODE_FORMAT_MASK; - pipe_config->pipe_bpp = - mipi_dsi_pixel_format_to_bpp( - pixel_format_from_register_bits(fmt)); - bpp = pipe_config->pipe_bpp; + bpp = mipi_dsi_pixel_format_to_bpp( + pixel_format_from_register_bits(fmt)); /* Enable Fram
Re: [PATCH 2/2] drm/msm: Use DRM_DEV_INFO_RATELIMITED for shrinker messages
On Fri, 18 Jan 2019, "Kristian H. Kristensen" wrote: > Otherwise we get hard to track down "Purging: 123123 bytes" messages in > the log. > > Signed-off-by: Kristian H. Kristensen > --- > drivers/gpu/drm/msm/msm_gem_shrinker.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c > b/drivers/gpu/drm/msm/msm_gem_shrinker.c > index b72d8e6cd51d7..8161923892f55 100644 > --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c > +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c > @@ -98,7 +98,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct > shrink_control *sc) > mutex_unlock(&dev->struct_mutex); > > if (freed > 0) > - pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT); > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %lu bytes\n", freed > << PAGE_SHIFT); I'm not opposed to the patches per se, but it does seem a bit odd to be printing info level messages in a way that might need ratelimiting. Is this a hint you should perhaps make it debug? BR, Jani. > > return freed; > } > @@ -134,7 +134,7 @@ msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned > long event, void *ptr) > *(unsigned long *)ptr += unmapped; > > if (unmapped > 0) > - pr_info_ratelimited("Purging %u vmaps\n", unmapped); > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %u vmaps\n", > unmapped); > > return NOTIFY_DONE; > } -- 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/RFC] drm: Remove unused Renesas SH Mobile DRM driver
On Fri, Jan 18, 2019 at 05:22:58PM +0100, Geert Uytterhoeven wrote: > Since its incarnation in v3.7 almost 7 years ago, no users of the SH > Mobile DRM driver have appeared. > > Hence remove the driver. It can be resurrected from git history, > if/when needed. > > Signed-off-by: Geert Uytterhoeven I'd prefer removing the fbdev variant tbh ... Not sure why exactly the switch never happened. -Daniel > --- > Against drm/drm-next > > MAINTAINERS | 2 - > drivers/gpu/drm/Kconfig | 2 - > drivers/gpu/drm/Makefile | 1 - > drivers/gpu/drm/shmobile/Kconfig | 14 - > drivers/gpu/drm/shmobile/Makefile | 8 - > .../gpu/drm/shmobile/shmob_drm_backlight.c| 86 --- > .../gpu/drm/shmobile/shmob_drm_backlight.h| 19 - > drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 689 -- > drivers/gpu/drm/shmobile/shmob_drm_crtc.h | 53 -- > drivers/gpu/drm/shmobile/shmob_drm_drv.c | 311 > drivers/gpu/drm/shmobile/shmob_drm_drv.h | 41 -- > drivers/gpu/drm/shmobile/shmob_drm_kms.c | 146 > drivers/gpu/drm/shmobile/shmob_drm_kms.h | 29 - > drivers/gpu/drm/shmobile/shmob_drm_plane.c| 259 --- > drivers/gpu/drm/shmobile/shmob_drm_plane.h| 18 - > drivers/gpu/drm/shmobile/shmob_drm_regs.h | 307 > include/linux/platform_data/shmob_drm.h | 91 --- > 17 files changed, 2076 deletions(-) > delete mode 100644 drivers/gpu/drm/shmobile/Kconfig > delete mode 100644 drivers/gpu/drm/shmobile/Makefile > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_backlight.c > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_backlight.h > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_crtc.c > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_crtc.h > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_drv.c > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_drv.h > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_kms.c > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_kms.h > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_plane.c > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_plane.h > delete mode 100644 drivers/gpu/drm/shmobile/shmob_drm_regs.h > delete mode 100644 include/linux/platform_data/shmob_drm.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4c0879650caf3c10..61009a7be8862d68 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5098,8 +5098,6 @@ L: linux-renesas-...@vger.kernel.org > T: git git://linuxtv.org/pinchartl/media drm/du/next > S: Supported > F: drivers/gpu/drm/rcar-du/ > -F: drivers/gpu/drm/shmobile/ > -F: include/linux/platform_data/shmob_drm.h > F: Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > F: Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > F: Documentation/devicetree/bindings/display/renesas,du.txt > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index bd943a71756ca81b..3df7b8905ca8b736 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -273,8 +273,6 @@ source "drivers/gpu/drm/atmel-hlcdc/Kconfig" > > source "drivers/gpu/drm/rcar-du/Kconfig" > > -source "drivers/gpu/drm/shmobile/Kconfig" > - > source "drivers/gpu/drm/sun4i/Kconfig" > > source "drivers/gpu/drm/omapdrm/Kconfig" > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index f0c1f8731a2761ed..9b5bee490a899468 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -82,7 +82,6 @@ obj-$(CONFIG_DRM_AST) += ast/ > obj-$(CONFIG_DRM_ARMADA) += armada/ > obj-$(CONFIG_DRM_ATMEL_HLCDC)+= atmel-hlcdc/ > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/ > -obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/ > obj-y+= omapdrm/ > obj-$(CONFIG_DRM_SUN4I) += sun4i/ > obj-y+= tilcdc/ > diff --git a/drivers/gpu/drm/shmobile/Kconfig > b/drivers/gpu/drm/shmobile/Kconfig > deleted file mode 100644 > index 61bbe8e8bcc51db9.. > --- a/drivers/gpu/drm/shmobile/Kconfig > +++ /dev/null > @@ -1,14 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0 > -config DRM_SHMOBILE > - tristate "DRM Support for SH Mobile" > - 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 > - help > - Choose this option if you have an SH Mobile chipset. > - If M is selected the module will be called shmob-drm. > - > diff --git a/drivers/gpu/drm/shmobile/Makefile > b/drivers/gpu/drm/shmobile/Makefile > deleted file mode 100644 > index 861edafed8562c87.. > --- a/drivers/gpu/drm/shmobile/Makefile > +++ /dev/null > @@ -1,8 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0 > -shmob-drm-y := shmob_drm_backligh
Re: [PATCH] drm/bridge: lvds-encoder: remove surplus NULL checks
On Fri, Jan 18, 2019 at 11:11:38PM +, Peter Rosin wrote: > The gpio API explicitly allows skipping the NULL check, precisely to > allow for neat support for optional gpios. Which is exactly what is at > play here. > > Reported-by: Andrzej Hajda > Signed-off-by: Peter Rosin lgtm, Reviewed-by: Daniel Vetter btw plan to stick around a bit in drm land, i.e. want commit rights for drm-misc? -Daniel > --- > drivers/gpu/drm/bridge/lvds-encoder.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c > b/drivers/gpu/drm/bridge/lvds-encoder.c > index 36d8557bc097..584007eaf6e1 100644 > --- a/drivers/gpu/drm/bridge/lvds-encoder.c > +++ b/drivers/gpu/drm/bridge/lvds-encoder.c > @@ -36,8 +36,7 @@ static void lvds_encoder_enable(struct drm_bridge *bridge) >struct lvds_encoder, >bridge); > > - if (lvds_encoder->powerdown_gpio) > - gpiod_set_value_cansleep(lvds_encoder->powerdown_gpio, 0); > + gpiod_set_value_cansleep(lvds_encoder->powerdown_gpio, 0); > } > > static void lvds_encoder_disable(struct drm_bridge *bridge) > @@ -46,8 +45,7 @@ static void lvds_encoder_disable(struct drm_bridge *bridge) >struct lvds_encoder, >bridge); > > - if (lvds_encoder->powerdown_gpio) > - gpiod_set_value_cansleep(lvds_encoder->powerdown_gpio, 1); > + gpiod_set_value_cansleep(lvds_encoder->powerdown_gpio, 1); > } > > static struct drm_bridge_funcs funcs = { > -- > 2.11.0 > > ___ > 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 v1 1/1] drm: fix drm_can_sleep() comment
On Sun, Jan 20, 2019 at 06:12:17PM +0100, Sam Ravnborg wrote: > Reversed logic when writing the original comment, now fixed. > > Fixes: e9eafcb58921 ("drm: move drm_can_sleep() to drm_util.h") > Reported-by: Laurent Pinchart > Signed-off-by: Sam Ravnborg > Cc: Laurent Pinchart > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter Thanks, merged. -Daniel > --- > include/drm/drm_util.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h > index 8163d35f8327..07b8e9f04599 100644 > --- a/include/drm/drm_util.h > +++ b/include/drm/drm_util.h > @@ -71,7 +71,7 @@ > * FIXME: All users of drm_can_sleep should be removed (see todo.rst) > * > * Returns: > - * True if kgdb is active or we are in an atomic context or irqs are disabled > + * False if kgdb is active, we are in atomic context or irqs are disabled. > */ > static inline bool drm_can_sleep(void) > { > -- > 2.12.0 > -- 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 09/11] drm/tinydrm: Remove tinydrm_device
On Sun, Jan 20, 2019 at 12:43:16PM +0100, Noralf Trønnes wrote: > No more users left so it can go alongside its helpers. > Update the tinydrm docs description and remove todo entry. > > Signed-off-by: Noralf Trønnes > --- > Documentation/gpu/tinydrm.rst | 26 +-- > Documentation/gpu/todo.rst| 4 - > drivers/gpu/drm/tinydrm/core/Makefile | 2 +- > drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 169 -- > .../gpu/drm/tinydrm/core/tinydrm-helpers.c| 2 + > include/drm/tinydrm/tinydrm.h | 42 - > 6 files changed, 10 insertions(+), 235 deletions(-) > delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c > delete mode 100644 include/drm/tinydrm/tinydrm.h Looks great. Acked-by: Daniel Vetter > > diff --git a/Documentation/gpu/tinydrm.rst b/Documentation/gpu/tinydrm.rst > index 1ca726474af4..19969b989efb 100644 > --- a/Documentation/gpu/tinydrm.rst > +++ b/Documentation/gpu/tinydrm.rst > @@ -1,24 +1,12 @@ > -== > -drm/tinydrm Driver library > -== > + > +drm/tinydrm Tiny DRM drivers > + > > -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c > - :doc: overview > +tinydrm is a collection of DRM drivers that are so small they can fit in a > +single source file. Since it's now just a collection of tiny drivers I think we should also rename the directory. Maybe in a follow-up patch, once this has all settled. I think just /tiny/ or /tinydrivers/ or so would be better. > > -Core functionality > -== > - > -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c > - :doc: core > - > -.. kernel-doc:: include/drm/tinydrm/tinydrm.h > - :internal: > - > -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c > - :export: Yay! > - > -Additional helpers > -== > +Helpers > +=== > > .. kernel-doc:: include/drm/tinydrm/tinydrm-helpers.h > :internal: Out of curiosity, what's left? From a quick look I think we could move the memcp/swab helpers into a drm_framebuffer_helper.c file (maybe move the fb argument first, since that's the main thing for ocd). And the spi stuff is kinda just spi helpers I guess? Could we move those into the spi subsystem, or was that idea already nacked? > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 38360ede1221..3495aec7a8d4 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -435,10 +435,6 @@ those drivers as simple as possible, so lots of room for > refactoring: >one of the ideas for having a shared dsi/dbi helper, abstracting away the >transport details more. > > -- Quick aside: The unregister devm stuff is kinda getting the lifetimes of > - a drm_device wrong. Doesn't matter, since everyone else gets it wrong > - too :-) Hey I even wrote this already :-) > - > Contact: Noralf Trønnes, Daniel Vetter > > AMD DC Display Driver > diff --git a/drivers/gpu/drm/tinydrm/core/Makefile > b/drivers/gpu/drm/tinydrm/core/Makefile > index bf2df7326df7..f88ea7ad302f 100644 > --- a/drivers/gpu/drm/tinydrm/core/Makefile > +++ b/drivers/gpu/drm/tinydrm/core/Makefile > @@ -1,3 +1,3 @@ > -tinydrm-y := tinydrm-core.o tinydrm-helpers.o > +tinydrm-y := tinydrm-helpers.o > > obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o > diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c > b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c > deleted file mode 100644 > index e4a77feaacd6.. > --- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c > +++ /dev/null > @@ -1,169 +0,0 @@ > -/* > - * Copyright (C) 2016 Noralf Trønnes > - * > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License as published by > - * the Free Software Foundation; either version 2 of the License, or > - * (at your option) any later version. > - */ > - > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > - > -/** > - * DOC: overview > - * > - * This library provides driver helpers for very simple display hardware. > - * > - * It is based on &drm_simple_display_pipe coupled with a &drm_connector > which > - * has only one fixed &drm_display_mode. The framebuffers are backed by the > - * cma helper and have support for framebuffer flushing (dirty). > - * fbdev support is also included. > - * > - */ > - > -/** > - * DOC: core > - * > - * The driver allocates &tinydrm_device, initializes it using > - * devm_tinydrm_init(), sets up the pipeline using > tinydrm_display_pipe_init() > - * and registers the DRM device using devm_tinydrm_register(). > - */ > - > -static const struct drm_mode_config_funcs tinydrm_mode_config_funcs = { > - .fb_create = drm_gem_fb_create_with_dirty, > - .atomic_check = drm_atomic_helper_check, > -
Re: [PATCH 03/11] drm/simple-kms-helper: Add drm_simple_connector_create()
On Sun, Jan 20, 2019 at 12:43:10PM +0100, Noralf Trønnes wrote: > This adds a function that creates a simple connector that has only one > static mode. Additionally add a helper to set &drm_mode_config width > and height from the static mode. > > Signed-off-by: Noralf Trønnes > --- > drivers/gpu/drm/drm_simple_kms_helper.c | 122 > include/drm/drm_simple_kms_helper.h | 6 ++ > 2 files changed, 128 insertions(+) > > diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c > b/drivers/gpu/drm/drm_simple_kms_helper.c > index 917812448d1b..ca29975afefe 100644 > --- a/drivers/gpu/drm/drm_simple_kms_helper.c > +++ b/drivers/gpu/drm/drm_simple_kms_helper.c > @@ -11,6 +11,8 @@ > #include > #include > #include > +#include > +#include > #include > #include > #include > @@ -299,4 +301,124 @@ int drm_simple_display_pipe_init(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_simple_display_pipe_init); > > +static const struct drm_connector_helper_funcs drm_simple_connector_hfuncs = > { > + /* dummy for the atomic helper */ > +}; > + > +static int drm_simple_connector_fill_modes(struct drm_connector *connector, > +uint32_t maxX, uint32_t maxY) > +{ > + return 1; > +} > + > +static void drm_simple_connector_destroy(struct drm_connector *connector) > +{ > + drm_connector_cleanup(connector); > + kfree(connector); > +} > + > +static const struct drm_connector_funcs drm_simple_connector_funcs = { > + .reset = drm_atomic_helper_connector_reset, > + .fill_modes = drm_simple_connector_fill_modes, > + .destroy = drm_simple_connector_destroy, > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > +}; > + > +/** > + * drm_simple_connector_create - Create a connector with one static mode > + * @dev: DRM device > + * @connector_type: Connector type > + * @mode: Supported display mode > + * @rotation: Initial @mode rotation in degrees We have rotation properties for this, pls don't use degress here. Also would be good to wire this up with the rotation property Hans added recently, for pre-rotated screens (see the kerneldoc for "panel orientation" and drm_connector_init_panel_orientation_property()). So that userspace knows how to rotate its rendering to make everything look correct in the end again. > + * > + * This function creates a &drm_connector that has one fixed > &drm_display_mode > + * which will be rotated according to @rotation. From a functionality pov this is very close to a panel wrapped into a bridge. I think it would be good to differentiate a bit between these two cases more. After all there was a really long discussion about how the panel stuff does or does not exactly fit for tinydrm, would be good to summarize this here and at least point at drm_panel_bridge_add(). Also would be good to explain in the overview DOC comment that this is a fully independent part of the simple helpers, and drivers can use one or the other. -Daniel > + * > + * Returns: > + * Pointer to connector on success, or ERR_PTR on failure. > + */ > +struct drm_connector * > +drm_simple_connector_create(struct drm_device *dev, int connector_type, > + const struct drm_display_mode *mode, > + unsigned int rotation) > +{ > + struct drm_display_mode *mode_dup = NULL; > + struct drm_connector *connector; > + int ret; > + > + connector = kzalloc(sizeof(*connector), GFP_KERNEL); > + if (!connector) > + return ERR_PTR(-ENOMEM); > + > + drm_connector_helper_add(connector, &drm_simple_connector_hfuncs); > + ret = drm_connector_init(dev, connector, &drm_simple_connector_funcs, > + connector_type); > + if (ret) > + goto err_free; > + > + connector->status = connector_status_connected; > + > + mode_dup = drm_mode_duplicate(dev, mode); > + if (!mode_dup) { > + ret = -ENOMEM; > + goto err_cleanup; > + } > + > + if (rotation == 90 || rotation == 270) { > + swap(mode_dup->hdisplay, mode_dup->vdisplay); > + swap(mode_dup->hsync_start, mode_dup->vsync_start); > + swap(mode_dup->hsync_end, mode_dup->vsync_end); > + swap(mode_dup->htotal, mode_dup->vtotal); > + swap(mode_dup->width_mm, mode_dup->height_mm); > + } else if (rotation != 0 && rotation != 180) { > + DRM_ERROR("Illegal rotation value %u\n", rotation); > + ret = -EINVAL; > + goto err_cleanup; > + } > + > + mode_dup->type |= DRM_MODE_TYPE_PREFERRED; > + if (mode_dup->name[0] == '\0') > + drm_mode_set_name(mode_dup); > + > + list_add(&mode_dup->head, &connector->modes); > + > + connector->display_info.width_mm = mode_dup->width_mm; > + connector->display_info.height_mm = mode_dup->
Re: [PATCH] drm: panel-orientation-quirks: Add quirk for Lenovo Ideapad D330
There are also 1280x800 versions of the Ideapad D330. I don't know the dmi strings for the lower versions, but they could be equal between versions of the same model. Could be possible to add orientation_data matrix elements with the same dmi strings and different driver data to complain both resolutions? If I or someone could get dmi strings from one of these models I can write patch v2. If someone can review this and explain the situation of two resolutions in the same model, would be great. Is needed to provide a way to verify the dmi strings? El jue., 3 ene. 2019 a las 21:40, David Santamaría Rogado () escribió: > > Lenovo Ideapad D330 Pentium CPU version has 1920x1200 LCD. Console > ouput gets rotated at boot as Miix 310. > > Signed-off-by: David Santamaría Rogado > --- > drivers/gpu/drm/drm_panel_orientation_quirks.c | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c > b/drivers/gpu/drm/drm_panel_orientation_quirks.c > index 52e445bb1aa58..521aff99b08a6 100644 > --- a/drivers/gpu/drm/drm_panel_orientation_quirks.c > +++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c > @@ -80,6 +80,12 @@ static const struct drm_dmi_panel_orientation_data > lcd800x1280_rightside_up = { > .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP, > }; > > +static const struct drm_dmi_panel_orientation_data > lcd1200x1920_rightside_up = { > + .width = 1200, > + .height = 1920, > + .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP, > +}; > + > static const struct dmi_system_id orientation_data[] = { > { /* Acer One 10 (S1003) */ > .matches = { > @@ -148,6 +154,13 @@ static const struct dmi_system_id orientation_data[] = { >DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "Lenovo MIIX 320-10ICR"), > }, > .driver_data = (void *)&lcd800x1280_rightside_up, > + }, { /* Lenovo Ideapad D330 */ > + .matches = { > + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"), > + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "81H3"), > + DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad D330-10IGM"), > + }, > + .driver_data = (void *)&lcd1200x1920_rightside_up, > }, { /* VIOS LTH17 */ > .matches = { >DMI_EXACT_MATCH(DMI_SYS_VENDOR, "VIOS"), ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] dt-bindings: display: armada: Add display subsystem binding
The Marvell Armada DRM master device is a virtual device needed to list all nodes that comprise the graphics subsystem. Signed-off-by: Lubomir Rintel --- .../display/armada/marvell-armada-drm.txt | 24 +++ 1 file changed, 24 insertions(+) diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt index de4cca9432c8..3dbfa8047f0b 100644 --- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt +++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt @@ -1,3 +1,27 @@ +Marvell Armada DRM master device + + +The Marvell Armada DRM master device is a virtual device needed to list all +nodes that comprise the graphics subsystem. + +Required properties: + + - compatible: value should be "marvell,dove-display-subsystem", + "marvell,armada-display-subsystem" + - ports: a list of phandles pointing to display interface ports of CRTC + devices + - memory-region: phandle to a node describing memory to be used for the + framebuffer + +Example: + + display-subsystem { + compatible = "marvell,dove-display-subsystem", +"marvell,armada-display-subsystem"; + memory-region = <&display_reserved>; + ports = <&lcd0_port>; + }; + Marvell Armada LCD controller = -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6] dt-bindings: display: armada: Rename the binding doc file
It's going to document more than just marvell,dove-lcd: more components of the display subsystems with more compatible strings. It seems to make sense to organize this the way it is done in Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt Signed-off-by: Lubomir Rintel --- .../armada/{marvell,dove-lcd.txt => marvell-armada-drm.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Documentation/devicetree/bindings/display/armada/{marvell,dove-lcd.txt => marvell-armada-drm.txt} (100%) diff --git a/Documentation/devicetree/bindings/display/armada/marvell,dove-lcd.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt similarity index 100% rename from Documentation/devicetree/bindings/display/armada/marvell,dove-lcd.txt rename to Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/armada: add mmp2 support
Heavily based on the Armada 510 (Dove) support. Like with 510 support, this also just supports a single source clock -- the "Display 1" clock as generated by the APMU. This one was chosen because the OLPC XO 1.75 laptop uses it for its internal panel. If anyone uses this to drive a MIPI or HDMI encoder, they may want to extend this to choose a different source for the pixel clock -- it should be a reasonably straightforward thing to do. The data sheet is not available, but James Cameron of OLPC kindly provided some details about the LCD_SCLK_DIV register. Link: https://lists.freedesktop.org/archives/dri-devel/2018-December/201021.html Signed-off-by: Lubomir Rintel --- drivers/gpu/drm/armada/Makefile | 1 + drivers/gpu/drm/armada/armada_610.c | 93 drivers/gpu/drm/armada/armada_crtc.c | 4 ++ drivers/gpu/drm/armada/armada_drm.h | 1 + drivers/gpu/drm/armada/armada_hw.h | 10 +++ 5 files changed, 109 insertions(+) create mode 100644 drivers/gpu/drm/armada/armada_610.c diff --git a/drivers/gpu/drm/armada/Makefile b/drivers/gpu/drm/armada/Makefile index 9bc3c3213724..5bbf86324cda 100644 --- a/drivers/gpu/drm/armada/Makefile +++ b/drivers/gpu/drm/armada/Makefile @@ -2,6 +2,7 @@ armada-y := armada_crtc.o armada_drv.o armada_fb.o armada_fbdev.o \ armada_gem.o armada_overlay.o armada_plane.o armada_trace.o armada-y += armada_510.o +armada-y += armada_610.o armada-$(CONFIG_DEBUG_FS) += armada_debugfs.o obj-$(CONFIG_DRM_ARMADA) := armada.o diff --git a/drivers/gpu/drm/armada/armada_610.c b/drivers/gpu/drm/armada/armada_610.c new file mode 100644 index ..278b204038ea --- /dev/null +++ b/drivers/gpu/drm/armada/armada_610.c @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2012 Russell King + * Copyright (C) 2018,2019 Lubomir Rintel + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Armada MMP2 variant support + */ +#include +#include +#include +#include "armada_crtc.h" +#include "armada_drm.h" +#include "armada_hw.h" + +static int armada610_crtc_init(struct armada_crtc *dcrtc, struct device *dev) +{ + struct clk *clk; + + clk = devm_clk_get(dev, "disp0"); + if (IS_ERR(clk)) + return PTR_ERR(clk) == -ENOENT ? -EPROBE_DEFER : PTR_ERR(clk); + + dcrtc->extclk[0] = clk; + + return 0; +} + +/* + * This gets called with sclk = NULL to test whether the mode is + * supportable, and again with sclk != NULL to set the clocks up for + * that. The former can return an error, but the latter is expected + * not to. + */ +static int armada610_crtc_compute_clock(struct armada_crtc *dcrtc, + const struct drm_display_mode *mode, uint32_t *sclk) +{ + struct clk *clk = dcrtc->extclk[0]; + uint32_t ret, rate, ref, div; + + if (IS_ERR(clk)) + return PTR_ERR(clk); + + rate = mode->clock * 1000; + ref = clk_get_rate(clk); + div = DIV_ROUND_UP(ref, rate); + + if (div < 2) + return -EINVAL; + + if (dcrtc->clk != clk) { + ret = clk_prepare_enable(clk); + if (ret) + return ret; + dcrtc->clk = clk; + } + + if (sclk) { + *sclk = 0x1000; /* No idea */ + *sclk |= 1 << 8; /* MIPI clock bypass */ + *sclk |= SCLK_610_DISP0; + *sclk |= div; + } + + return 0; +} + +static void armada610_crtc_disable(struct armada_crtc *dcrtc) +{ + if (!IS_ERR(dcrtc->clk)) { + clk_disable_unprepare(dcrtc->clk); + dcrtc->clk = ERR_PTR(-EINVAL); + } +} + +static void armada610_crtc_enable(struct armada_crtc *dcrtc, + const struct drm_display_mode *mode) +{ + if (IS_ERR(dcrtc->clk)) { + dcrtc->clk = dcrtc->extclk[0]; + WARN_ON(clk_prepare_enable(dcrtc->clk)); + } +} + +const struct armada_variant armada610_ops = { + .init = armada610_crtc_init, + .compute_clock = armada610_crtc_compute_clock, + .disable = armada610_crtc_disable, + .enable = armada610_crtc_enable, +}; diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index da9360688b55..927be8898eb7 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -884,6 +884,10 @@ static const struct of_device_id armada_lcd_of_match[] = { .compatible = "marvell,dove-lcd", .data = &armada510_ops, }, + { + .compatible = "marvell,mmp2-lcd", + .data = &armada610_ops, + }, {} }; MODULE_DEVICE_TABLE(of, armada_lcd_of_match); diff --git a/drivers/gpu/drm/armada/armada_drm.h b/drivers/gpu/drm
[PATCH 0/6] Bindings for Armada display subsystem
Hi, this patch set extends the bindings documentation of Armada LCDC to cover the rest of the display subsystem. It is based on what was implemented by Russel's patch set that implements the dt bindings for armada-drm [1]. [1] https://lists.freedesktop.org/archives/dri-devel/2018-July/182893.html There are some differencies though: * Compatible strings for the MMP2 platform were added * The compatible strings for the display-subsystem node and the framebuffer node include "marvell,armada-display-subsystem" and "marvell,armada-framebuffer" since they are hardware agnostic and supportable with the same driver code. * The "bus-width property" was added. More in the individual patches. Thank you, Lubo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next] drm/stm: ltdc: remove set but not used variable 'src_h'
Fixes gcc '-Wunused-but-set-variable' warning: drivers/gpu/drm/stm/ltdc.c: In function 'ltdc_plane_atomic_check': drivers/gpu/drm/stm/ltdc.c:694:13: warning: variable 'src_y' set but not used [-Wunused-but-set-variable] u32 src_x, src_y, src_w, src_h; ^ ^ drivers/gpu/drm/stm/ltdc.c:694:6: warning: variable 'src_x' set but not used [-Wunused-but-set-variable] u32 src_x, src_y, src_w, src_h; ^ Signed-off-by: YueHaibing --- drivers/gpu/drm/stm/ltdc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 61dd661..a91e041 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -691,7 +691,7 @@ static int ltdc_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { struct drm_framebuffer *fb = state->fb; - u32 src_x, src_y, src_w, src_h; + u32 src_w, src_h; DRM_DEBUG_DRIVER("\n"); @@ -699,8 +699,6 @@ static int ltdc_plane_atomic_check(struct drm_plane *plane, return 0; /* convert src_ from 16:16 format */ - src_x = state->src_x >> 16; - src_y = state->src_y >> 16; src_w = state->src_w >> 16; src_h = state->src_h >> 16; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6] dt-bindings: display: armada: Document bus-width property
This makes it possible to choose a different pixel format for the endpoint. Modelled after what other LCD controllers use, including marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps more. Signed-off-by: Lubomir Rintel --- .../bindings/display/armada/marvell-armada-drm.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt index 53524077db25..0a734f6d5a1e 100644 --- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt +++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt @@ -49,6 +49,11 @@ Required child nodes: - port: video output port with endpoints, as described by Documentation/devicetree/bindings/graph.txt + The endpoints can optionally specify the following property: + + - bus-width: recognized values are <12>, <16>, <18> and <24>, that +select "rgb444", "rgb565", "rgb666" or "rgb888" pixel format +respectively. Defaults to <24> if unspecified. Example: @@ -61,6 +66,7 @@ Example: lcd0_port: port { lcd0_rgb_out: endpoint { + bus-width = <24>; remote-endpoint = <&encoder_rgb_in>; }; }; -- 2.20.1 ___ 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 Fri, Jan 18, 2019 at 10:51:10PM +0100, Jernej Škrabec wrote: > Dne četrtek, 17. januar 2019 ob 08:24:02 CET je Priit Laes napisal(a): > > 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 110 > > 5000 0 120 5 mmc2_output 110 > > 5000 060 5 ahb 18 18 > > 0 3 0 0 5 ahb-mmc2 11 > > 0 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
[PATCH 2/6] dt-bindings: display: armada: Improve the LCDC documentation
The port is a child, not a property. And it deserves an example. Also, make the title a bit more visually distinguishable -- this will look better when the documentation of other Adrmada DRM nodes will be present. Signed-off-by: Lubomir Rintel --- .../display/armada/marvell-armada-drm.txt| 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt index 46525ea3e646..2606a8efc956 100644 --- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt +++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt @@ -1,10 +1,11 @@ -Device Tree bindings for Armada DRM CRTC driver +Marvell Armada LCD controller += Required properties: + - compatible: value should be "marvell,dove-lcd". - reg: base address and size of the LCD controller - interrupts: single interrupt number for the LCD controller - - port: video output port with endpoints, as described by graph.txt Optional properties: @@ -19,6 +20,11 @@ Note: all clocks are optional but at least one must be specified. Further clocks may be added in the future according to requirements of different SoCs. +Required child nodes: + +- port: video output port with endpoints, as described by + Documentation/devicetree/bindings/graph.txt + Example: lcd0: lcd-controller@82 { @@ -27,4 +33,10 @@ Example: interrupts = <47>; clocks = <&si5351 0>; clock-names = "ext_ref_clk_1"; + + lcd0_port: port { + lcd0_rgb_out: endpoint { + remote-endpoint = <&encoder_rgb_in>; + }; + }; }; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/armada: replace the simple-framebuffer
If there's a simple-framebuffer carried over from boot firmware, it's going to stop working once we setup the LCDC for use via DRM. Kick it off from the hardware. Signed-off-by: Lubomir Rintel --- drivers/gpu/drm/armada/armada_drv.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index fa31589b4fc0..9fcf88c91758 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -100,6 +100,17 @@ static int armada_drm_bind(struct device *dev) return ret; } + /* Remove early framebuffers */ + ret = drm_fb_helper_remove_conflicting_framebuffers(NULL, + "armada-drm-fb", + false); + if (ret) { + dev_err(dev, "[" DRM_NAME ":%s] can't kick out simple-fb: %d\n", + __func__, ret); + kfree(priv); + return ret; + } + priv->drm.dev_private = priv; dev_set_drvdata(dev, &priv->drm); -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/6] dt-bindings: display: armada: Add framebuffer reserved-mem binding
This is the binding for memory that is set aside for allocation of Marvell Armada framebuffer objects. Signed-off-by: Lubomir Rintel --- .../display/armada/marvell-armada-drm.txt | 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt index 2606a8efc956..de4cca9432c8 100644 --- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt +++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt @@ -40,3 +40,28 @@ Example: }; }; }; + +Marvell Armada framebuffer reserved memory +== + +Memory set aside for allocation of Marvell Armada framebuffer objects. + +Required properties: + + - compatible: value should be "marvell,dove-framebuffer", + "marvell,armada-framebuffer". + +This binding is compatible with the binding, specified in +Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt.. + +Example: + + reserved-memory { + display_reserved: framebuffer { + compatible = "marvell,dove-framebuffer", +"marvell,armada-framebuffer"; + size = <0x0200>; + alignment = <0x0200>; + no-map; + }; + }; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/6] dt-bindings: display: armada: Add mmp2 compatible strings
The driver will work on a MMP2 as well. Signed-off-by: Lubomir Rintel --- .../bindings/display/armada/marvell-armada-drm.txt | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt index 3dbfa8047f0b..53524077db25 100644 --- a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt +++ b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt @@ -6,8 +6,9 @@ nodes that comprise the graphics subsystem. Required properties: - - compatible: value should be "marvell,dove-display-subsystem", - "marvell,armada-display-subsystem" + - compatible: value should be one of: + - "marvell,dove-display-subsystem", "marvell,armada-display-subsystem" + - "marvell,mmp2-display-subsystem", "marvell,armada-display-subsystem" - ports: a list of phandles pointing to display interface ports of CRTC devices - memory-region: phandle to a node describing memory to be used for the @@ -27,7 +28,7 @@ Marvell Armada LCD controller Required properties: - - compatible: value should be "marvell,dove-lcd". + - compatible: value should be "marvell,dove-lcd" or "marvell,mmp2-lcd" - reg: base address and size of the LCD controller - interrupts: single interrupt number for the LCD controller @@ -72,8 +73,9 @@ Memory set aside for allocation of Marvell Armada framebuffer objects. Required properties: - - compatible: value should be "marvell,dove-framebuffer", - "marvell,armada-framebuffer". + - compatible: value should be one of: + - "marvell,dove-framebuffer", "marvell,armada-framebuffer" + - "marvell,mmp2-framebuffer", "marvell,armada-framebuffer" This binding is compatible with the binding, specified in Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt.. -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/11] drm/tinydrm: Remove tinydrm_display_pipe_init()
On Sun, Jan 20, 2019 at 12:43:11PM +0100, Noralf Trønnes wrote: > Further strip down tinydrm.ko and switch to drm_simple_connector_create(). > > Signed-off-by: Noralf Trønnes > --- > Documentation/gpu/tinydrm.rst | 3 - > drivers/gpu/drm/tinydrm/core/Makefile | 2 +- > drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 183 > drivers/gpu/drm/tinydrm/mipi-dbi.c | 24 ++- > drivers/gpu/drm/tinydrm/repaper.c | 16 +- > drivers/gpu/drm/tinydrm/st7586.c| 19 +- > include/drm/tinydrm/tinydrm.h | 9 - > 7 files changed, 43 insertions(+), 213 deletions(-) > delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > > diff --git a/Documentation/gpu/tinydrm.rst b/Documentation/gpu/tinydrm.rst > index a913644bfc19..1ca726474af4 100644 > --- a/Documentation/gpu/tinydrm.rst > +++ b/Documentation/gpu/tinydrm.rst > @@ -17,9 +17,6 @@ Core functionality > .. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c > :export: > > -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > - :export: > - > Additional helpers > == > > diff --git a/drivers/gpu/drm/tinydrm/core/Makefile > b/drivers/gpu/drm/tinydrm/core/Makefile > index fb221e6f8885..bf2df7326df7 100644 > --- a/drivers/gpu/drm/tinydrm/core/Makefile > +++ b/drivers/gpu/drm/tinydrm/core/Makefile > @@ -1,3 +1,3 @@ > -tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o > +tinydrm-y := tinydrm-core.o tinydrm-helpers.o > > obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o > diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > deleted file mode 100644 > index 323564329535.. > --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > +++ /dev/null > @@ -1,183 +0,0 @@ > -/* > - * Copyright (C) 2016 Noralf Trønnes > - * > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License as published by > - * the Free Software Foundation; either version 2 of the License, or > - * (at your option) any later version. > - */ > - > -#include > -#include > -#include > -#include > -#include > -#include > -#include > - > -struct tinydrm_connector { > - struct drm_connector base; > - struct drm_display_mode mode; > -}; > - > -static inline struct tinydrm_connector * > -to_tinydrm_connector(struct drm_connector *connector) > -{ > - return container_of(connector, struct tinydrm_connector, base); > -} > - > -static int tinydrm_connector_get_modes(struct drm_connector *connector) > -{ > - struct tinydrm_connector *tconn = to_tinydrm_connector(connector); > - struct drm_display_mode *mode; > - > - mode = drm_mode_duplicate(connector->dev, &tconn->mode); > - if (!mode) { > - DRM_ERROR("Failed to duplicate mode\n"); > - return 0; > - } > - > - if (mode->name[0] == '\0') > - drm_mode_set_name(mode); > - > - mode->type |= DRM_MODE_TYPE_PREFERRED; > - drm_mode_probed_add(connector, mode); > - > - if (mode->width_mm) { > - connector->display_info.width_mm = mode->width_mm; > - connector->display_info.height_mm = mode->height_mm; > - } > - > - return 1; > -} > - > -static const struct drm_connector_helper_funcs tinydrm_connector_hfuncs = { > - .get_modes = tinydrm_connector_get_modes, > -}; > - > -static enum drm_connector_status > -tinydrm_connector_detect(struct drm_connector *connector, bool force) > -{ > - if (drm_dev_is_unplugged(connector->dev)) > - return connector_status_disconnected; > - > - return connector->status; > -} > - > -static void tinydrm_connector_destroy(struct drm_connector *connector) > -{ > - struct tinydrm_connector *tconn = to_tinydrm_connector(connector); > - > - drm_connector_cleanup(connector); > - kfree(tconn); > -} > - > -static const struct drm_connector_funcs tinydrm_connector_funcs = { > - .reset = drm_atomic_helper_connector_reset, > - .detect = tinydrm_connector_detect, > - .fill_modes = drm_helper_probe_single_connector_modes, > - .destroy = tinydrm_connector_destroy, > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > -}; > - > -struct drm_connector * > -tinydrm_connector_create(struct drm_device *drm, > - const struct drm_display_mode *mode, > - int connector_type) > -{ > - struct tinydrm_connector *tconn; > - struct drm_connector *connector; > - int ret; > - > - tconn = kzalloc(sizeof(*tconn), GFP_KERNEL); > - if (!tconn) > - return ERR_PTR(-ENOMEM); > - > - drm_mode_copy(&tconn->mode, mode); > - connector = &tconn->base; > - > - drm_connector_helper_add(connector, &tinydrm_connector_hfuncs); > - ret = drm_connector_init(drm, connecto
Re: [PATCH 01/11] drm: Add devm_drm_dev_init/register
On Sun, Jan 20, 2019 at 12:43:08PM +0100, Noralf Trønnes wrote: > This adds resource managed (devres) versions of drm_dev_init() and > drm_dev_register(). > > Also added is devm_drm_dev_register_with_fbdev() which sets up generic > fbdev emulation as well. > > devm_drm_dev_register() isn't exported since there are no users. > > Signed-off-by: Noralf Trønnes devm_ considered harmful unfortunately. You cannot allocate any structures which might outlive the lifetime of your device using devm_ on the device. Since drm_device has it's own lifetime, that structure and _everything_ we allocate tied to it (i.e. drm_encoder/plane/connector/crtc/whatever) cannot be allocated using devm_ infrastructure tied to the underlying parent device. Yes this means almost all the devm usage in drm drivers is fundamentally broken. You also generally don't unplug gpus, so no one cares :-/ What could instead is add a struct device to drm_device somehow, use that struct device to manage the lifetime of the drm_device (would still need our special open counter maybe), and then use devm to allocate all the drm bits tied to the drm_device. This here unfortunataly doesn't work, even if it looks like a really neat idea. -Daniel > --- > Documentation/driver-model/devres.txt | 4 + > drivers/gpu/drm/drm_drv.c | 106 ++ > include/drm/drm_drv.h | 6 ++ > 3 files changed, 116 insertions(+) > > diff --git a/Documentation/driver-model/devres.txt > b/Documentation/driver-model/devres.txt > index b277cafce71e..6eebc28d4c21 100644 > --- a/Documentation/driver-model/devres.txt > +++ b/Documentation/driver-model/devres.txt > @@ -254,6 +254,10 @@ DMA >dmam_pool_create() >dmam_pool_destroy() > > +DRM > + devm_drm_dev_init() > + devm_drm_dev_register_with_fbdev() > + > GPIO >devm_gpiod_get() >devm_gpiod_get_index() > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 381581b01d48..12129772be45 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -36,6 +36,7 @@ > > #include > #include > +#include > #include > > #include "drm_crtc_internal.h" > @@ -871,6 +872,111 @@ void drm_dev_unregister(struct drm_device *dev) > } > EXPORT_SYMBOL(drm_dev_unregister); > > +static void devm_drm_dev_init_release(void *data) > +{ > + drm_dev_put(data); > +} > + > +/** > + * devm_drm_dev_init - Resource managed drm_dev_init() > + * @parent: Parent device object > + * @dev: DRM device > + * @driver: DRM driver > + * > + * Managed drm_dev_init(). The DRM device initialized with this function is > + * automatically released on driver detach. You must supply a > + * &drm_driver.release callback to control the finalization explicitly. > + * > + * Note: This function must be used together with > + * devm_drm_dev_register_with_fbdev(). > + * > + * RETURNS: > + * 0 on success, or error code on failure. > + */ > +int devm_drm_dev_init(struct device *parent, > + struct drm_device *dev, > + struct drm_driver *driver) > +{ > + int ret; > + > + if (WARN_ON(!parent || !driver->release)) > + return -EINVAL; > + > + ret = drm_dev_init(dev, driver, parent); > + if (ret) > + return ret; > + > + /* > + * This is a temporary release action that is used if probing fails > + * before devm_drm_dev_register() is called. > + */ > + ret = devm_add_action(parent, devm_drm_dev_init_release, dev); > + if (ret) > + devm_drm_dev_init_release(dev); > + > + return ret; > +} > +EXPORT_SYMBOL(devm_drm_dev_init); > + > +static void devm_drm_dev_register_release(void *data) > +{ > + drm_dev_unplug(data); > +} > + > +static int devm_drm_dev_register(struct drm_device *dev) > +{ > + int ret; > + > + ret = drm_dev_register(dev, 0); > + if (ret) > + return ret; > + > + /* > + * This has now served it's purpose, remove it to not mess up ref > + * counting. > + */ > + devm_remove_action(dev->dev, devm_drm_dev_init_release, dev); > + > + ret = devm_add_action(dev->dev, devm_drm_dev_register_release, dev); > + if (ret) > + devm_drm_dev_register_release(dev); > + > + return ret; > +} > + > +/** > + * devm_drm_dev_register_with_fbdev - Resource managed drm_dev_register() > + *including generic fbdev emulation > + * @dev: DRM device to register > + * @fbdev_bpp: Preferred bits per pixel for fbdev (optional) > + * > + * Managed drm_dev_register() that also calls drm_fbdev_generic_setup(). > + * The DRM device registered with this function is automatically > unregistered on > + * driver detach using drm_dev_unplug(). > + * > + * Note: This function must be used together with devm_drm_dev_init(). > + * > + * For testing driver detach can be triggered manually by writing to the > driver > + * 'unbind' file. > + * > + * RETURNS: > + * 0 o