[PATCH v1 3/5] fbtft: Drop useless #ifdef CONFIG_OF and dead code
First of all there is no need to guard GPIO request by CONFIG_OF. It works for everybody independently on resource provider. While here, rename the function to reflect the above. Moreover, since we have a global dependency to OF, the rest of conditional compilation is no-op, i.e. it's always be true. Due to above drop useless #ifdef CONFIG_OF and therefore dead code. Signed-off-by: Andy Shevchenko --- drivers/staging/fbtft/fbtft-core.c | 19 ++- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 2122c4407bdb..ff8cb6670ea1 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -70,7 +70,6 @@ void fbtft_dbg_hex(const struct device *dev, int groupsize, } EXPORT_SYMBOL(fbtft_dbg_hex); -#ifdef CONFIG_OF static int fbtft_request_one_gpio(struct fbtft_par *par, const char *name, int index, struct gpio_desc **gpiop) @@ -92,14 +91,11 @@ static int fbtft_request_one_gpio(struct fbtft_par *par, return ret; } -static int fbtft_request_gpios_dt(struct fbtft_par *par) +static int fbtft_request_gpios(struct fbtft_par *par) { int i; int ret; - if (!par->info->device->of_node) - return -EINVAL; - ret = fbtft_request_one_gpio(par, "reset", 0, &par->gpio.reset); if (ret) return ret; @@ -135,7 +131,6 @@ static int fbtft_request_gpios_dt(struct fbtft_par *par) return 0; } -#endif #ifdef CONFIG_FB_BACKLIGHT static int fbtft_backlight_update_status(struct backlight_device *bd) @@ -898,7 +893,6 @@ int fbtft_unregister_framebuffer(struct fb_info *fb_info) } EXPORT_SYMBOL(fbtft_unregister_framebuffer); -#ifdef CONFIG_OF /** * fbtft_init_display_dt() - Device Tree init_display() function * @par: Driver data @@ -977,7 +971,6 @@ static int fbtft_init_display_dt(struct fbtft_par *par) return 0; } -#endif /** * fbtft_init_display() - Generic init_display() function @@ -1138,7 +1131,6 @@ static int fbtft_verify_gpios(struct fbtft_par *par) return 0; } -#ifdef CONFIG_OF /* returns 0 if the property is not present */ static u32 fbtft_of_value(struct device_node *node, const char *propname) { @@ -1184,17 +1176,10 @@ static struct fbtft_platform_data *fbtft_probe_dt(struct device *dev) pdata->display.backlight = 1; if (of_find_property(node, "init", NULL)) pdata->display.fbtftops.init_display = fbtft_init_display_dt; - pdata->display.fbtftops.request_gpios = fbtft_request_gpios_dt; + pdata->display.fbtftops.request_gpios = fbtft_request_gpios; return pdata; } -#else -static struct fbtft_platform_data *fbtft_probe_dt(struct device *dev) -{ - dev_err(dev, "Missing platform data\n"); - return ERR_PTR(-EINVAL); -} -#endif /** * fbtft_probe_common() - Generic device probe() helper function -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] msm: disp: dpu1: add support to access hw irqs regs depending on revision
Current code assumes that all the irqs registers offsets can be accessed in all the hw revisions; this is not the case for some targets that should not access some of the irq registers. This change adds the support to selectively remove the irqs that are not supported in some of the hw revisions. Changes in v1: - Add support to selectively remove the hw irqs that are not not supported. Changes in v2: - Remove unrelated changes. Changes in v3: - Remove change-id (Stephen Boyd). - Add colon in variable description to match kernel-doc (Stephen Boyd). - Change macro-y way of variable description (Jordon Crouse). - Remove unnecessary if checks (Jordon Crouse). - Remove extra blank line (Jordon Crouse). Changes in v4: - Remove checkpatch errors. Signed-off-by: Shubhashree Dhar --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 22 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 1 + 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 04c8c44..88f2664 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -421,6 +421,7 @@ static void sdm845_cfg_init(struct dpu_mdss_cfg *dpu_cfg) .reg_dma_count = 1, .dma_cfg = sdm845_regdma, .perf = sdm845_perf_data, + .mdss_irqs = 0x3ff, }; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index ec76b868..0fd3f50 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -646,6 +646,7 @@ struct dpu_perf_cfg { * @dma_formatsSupported formats for dma pipe * @cursor_formats Supported formats for cursor pipe * @vig_formatsSupported formats for vig pipe + * @mdss_irqs: Bitmap with the irqs supported by the target */ struct dpu_mdss_cfg { u32 hwversion; @@ -684,6 +685,8 @@ struct dpu_mdss_cfg { struct dpu_format_extended *dma_formats; struct dpu_format_extended *cursor_formats; struct dpu_format_extended *vig_formats; + + unsigned long mdss_irqs; }; struct dpu_mdss_hw_cfg_handler { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index 8bfa7d0..d84a84f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -800,8 +800,8 @@ static void dpu_hw_intr_dispatch_irq(struct dpu_hw_intr *intr, start_idx = reg_idx * 32; end_idx = start_idx + 32; - if (start_idx >= ARRAY_SIZE(dpu_irq_map) || - end_idx > ARRAY_SIZE(dpu_irq_map)) + if (!test_bit(reg_idx, &intr->irq_mask) || + start_idx >= ARRAY_SIZE(dpu_irq_map)) continue; /* @@ -955,8 +955,11 @@ static int dpu_hw_intr_clear_irqs(struct dpu_hw_intr *intr) if (!intr) return -EINVAL; - for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) - DPU_REG_WRITE(&intr->hw, dpu_intr_set[i].clr_off, 0x); + for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) { + if (test_bit(i, &intr->irq_mask)) + DPU_REG_WRITE(&intr->hw, + dpu_intr_set[i].clr_off, 0x); + } /* ensure register writes go through */ wmb(); @@ -971,8 +974,11 @@ static int dpu_hw_intr_disable_irqs(struct dpu_hw_intr *intr) if (!intr) return -EINVAL; - for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) - DPU_REG_WRITE(&intr->hw, dpu_intr_set[i].en_off, 0x); + for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) { + if (test_bit(i, &intr->irq_mask)) + DPU_REG_WRITE(&intr->hw, + dpu_intr_set[i].en_off, 0x); + } /* ensure register writes go through */ wmb(); @@ -991,6 +997,9 @@ static void dpu_hw_intr_get_interrupt_statuses(struct dpu_hw_intr *intr) spin_lock_irqsave(&intr->irq_lock, irq_flags); for (i = 0; i < ARRAY_SIZE(dpu_intr_set); i++) { + if (!test_bit(i, &intr->irq_mask)) + continue; + /* Read interrupt status */ intr->save_irq_status[i] = DPU_REG_READ(&intr->hw, dpu_intr_set[i].status_off); @@ -1115,6 +1124,7 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr, return ERR_PTR(-ENOMEM); } + intr->irq_mask = m->mdss_irqs; spin_lock_init(&i
RE: [PATCH] video: hyperv_fb: Fix hibernation for the deferred IO feature
> -Original Message- > From: Wei Hu > Sent: 2019年11月21日 10:47 > To: Dexuan Cui ; KY Srinivasan ; > Haiyang Zhang ; Stephen Hemminger > ; sas...@kernel.org; b.zolnier...@samsung.com; > linux-hyp...@vger.kernel.org; dri-devel@lists.freedesktop.org; linux- > fb...@vger.kernel.org; linux-ker...@vger.kernel.org; Michael Kelley > ; Sasha Levin > Subject: RE: [PATCH] video: hyperv_fb: Fix hibernation for the deferred IO > feature > > > -Original Message- > > From: Dexuan Cui > > Sent: Wednesday, November 20, 2019 3:14 PM > > To: KY Srinivasan ; Haiyang Zhang > > ; Stephen Hemminger > ; > > sas...@kernel.org; b.zolnier...@samsung.com; linux-hyp...@vger.kernel.org; > > dri-devel@lists.freedesktop.org; linux-fb...@vger.kernel.org; linux- > > ker...@vger.kernel.org; Michael Kelley ; Sasha > Levin > > > > Cc: Wei Hu ; Dexuan Cui > > Subject: [PATCH] video: hyperv_fb: Fix hibernation for the deferred IO > > feature > > > > fb_deferred_io_work() can access the vmbus ringbuffer by calling > > fbdefio->deferred_io() -> synthvid_deferred_io() -> synthvid_update(). > > > > Because the vmbus ringbuffer is inaccessible between hvfb_suspend() and > > hvfb_resume(), we must cancel info->deferred_work before calling > > vmbus_close() and then reschedule it after we reopen the channel in > > hvfb_resume(). > > > > Fixes: a4ddb11d297e ("video: hyperv: hyperv_fb: Support deferred IO for > > Hyper-V frame buffer driver") > > Fixes: 824946a8b6fb ("video: hyperv_fb: Add the support of hibernation") > > Signed-off-by: Dexuan Cui > > --- > > > > This patch fixes the 2 aforementioned patches on Sasha Levin's Hyper-V > > tree's > > hyperv-next branch: > > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern > > el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fhyperv%2Flinux.git%2Flog > > %2F%3Fh%3Dhyperv- > > > next&data=02%7C01%7Cweh%40microsoft.com%7C451143ff78f04401d9 > > 6f08d76d893a84%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637 > > > 098308493217121&sdata=P2fo%2F1TJUMIj5FtJCOp2QwDrghhVfPSCEJ4f1 > > vkOXvI%3D&reserved=0 > > > > The 2 aforementioned patches have not appeared in the mainline yet, so > please > > pick up this patch onto he same hyperv-next branch. > > > > drivers/video/fbdev/hyperv_fb.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/video/fbdev/hyperv_fb.c > > b/drivers/video/fbdev/hyperv_fb.c > > index 4cd27e5172a1..08bc0dfb5ce7 100644 > > --- a/drivers/video/fbdev/hyperv_fb.c > > +++ b/drivers/video/fbdev/hyperv_fb.c > > @@ -1194,6 +1194,7 @@ static int hvfb_suspend(struct hv_device *hdev) > > fb_set_suspend(info, 1); > > > > cancel_delayed_work_sync(&par->dwork); > > + cancel_delayed_work_sync(&info->deferred_work); > > > > par->update_saved = par->update; > > par->update = false; > > @@ -1227,6 +1228,7 @@ static int hvfb_resume(struct hv_device *hdev) > > par->fb_ready = true; > > par->update = par->update_saved; > > > > + schedule_delayed_work(&info->deferred_work, info->fbdefio->delay); > > schedule_delayed_work(&par->dwork, HVFB_UPDATE_DELAY); > > > > /* 0 means do resume */ > > -- > > 2.19.1 > > Signed-off-by: Wei Hu Sorry, please disregard the Signed-off-by line I added above. It was my mistake. should be: Reviewed-by: Wei Hu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/2] drm/rect: Keep the clipped dst rectangle in place
On 11/20/19 6:11 PM, Ville Syrjälä wrote: > On Wed, Nov 20, 2019 at 05:43:40PM +0100, Daniel Vetter wrote: >> On Wed, Nov 20, 2019 at 06:25:12PM +0200, Ville Syrjala wrote: >>> From: Ville Syrjälä >>> >>> Now that we've constrained the clipped source rectangle such >>> that it can't have negative dimensions doing the same for the >>> dst rectangle seems appropriate. Should at least result in >>> the clipped src and dst rectangles being a bit more consistent >>> with each other. >>> >>> Cc: Benjamin Gaignard >>> Cc: Maarten Lankhorst >>> Signed-off-by: Ville Syrjälä >> selftests for this stuff? Looks like the prime example, write testcase >> proving code is busted, fix it, everyone celebrate? > Yeah, seems like a good idea. Though I'll have to figure out if it's > actually broken or not ;) > > Hmm. Ouch. There's seems to be a div by zero lurking in there if > dst_w/h == 0. I wonder why nothing has hit that. At least W=1 warnings have disappear with these patches ;-) Benjamin >> -Daniel >> >>> --- >>> drivers/gpu/drm/drm_rect.c | 22 +++--- >>> 1 file changed, 11 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c >>> index 7762b6e9278d..229325fcf333 100644 >>> --- a/drivers/gpu/drm/drm_rect.c >>> +++ b/drivers/gpu/drm/drm_rect.c >>> @@ -52,14 +52,14 @@ bool drm_rect_intersect(struct drm_rect *r1, const >>> struct drm_rect *r2) >>> } >>> EXPORT_SYMBOL(drm_rect_intersect); >>> >>> -static u32 clip_scaled(u32 src, u32 dst, u32 clip) >>> +static u32 clip_scaled(int src, int dst, int *clip) >>> { >>> u64 tmp; >>> >>> /* Only clip what we have. Keeps the result bounded as well. */ >>> - clip = min(clip, dst); >>> + *clip = min(*clip, dst); >>> >>> - tmp = mul_u32_u32(src, dst - clip); >>> + tmp = mul_u32_u32(src, dst - *clip); >>> >>> /* >>> * Round toward 1.0 when clipping so that we don't accidentally >>> @@ -92,34 +92,34 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct >>> drm_rect *dst, >>> diff = clip->x1 - dst->x1; >>> if (diff > 0) { >>> u32 new_src_w = clip_scaled(drm_rect_width(src), >>> - drm_rect_width(dst), diff); >>> + drm_rect_width(dst), &diff); >>> >>> src->x1 = src->x2 - new_src_w; >>> - dst->x1 = clip->x1; >>> + dst->x1 += diff; >>> } >>> diff = clip->y1 - dst->y1; >>> if (diff > 0) { >>> u32 new_src_h = clip_scaled(drm_rect_height(src), >>> - drm_rect_height(dst), diff); >>> + drm_rect_height(dst), &diff); >>> >>> src->y1 = src->y2 - new_src_h; >>> - dst->y1 = clip->y1; >>> + dst->y1 += diff; >>> } >>> diff = dst->x2 - clip->x2; >>> if (diff > 0) { >>> u32 new_src_w = clip_scaled(drm_rect_width(src), >>> - drm_rect_width(dst), diff); >>> + drm_rect_width(dst), &diff); >>> >>> src->x2 = src->x1 + new_src_w; >>> - dst->x2 = clip->x2; >>> + dst->x2 -= diff; >>> } >>> diff = dst->y2 - clip->y2; >>> if (diff > 0) { >>> u32 new_src_h = clip_scaled(drm_rect_height(src), >>> - drm_rect_height(dst), diff); >>> + drm_rect_height(dst), &diff); >>> >>> src->y2 = src->y1 + new_src_h; >>> - dst->y2 = clip->y2; >>> + dst->y2 -= diff; >>> } >>> >>> return drm_rect_visible(dst); >>> -- >>> 2.23.0 >>> >>> ___ >>> Intel-gfx mailing list >>> intel-...@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 3/5] fbtft: Drop useless #ifdef CONFIG_OF and dead code
On Wed, Nov 20, 2019 at 04:04:17PM +0100, Noralf Trønnes wrote: > Den 20.11.2019 15.43, skrev Noralf Trønnes: > > Den 20.11.2019 10.57, skrev Andy Shevchenko: > >> First of all there is no need to guard GPIO request by CONFIG_OF. > >> It works for everybody independently on resource provider. While here, > >> rename the function to reflect the above. > >> > >> Moreover, since we have a global dependency to OF, the rest of > >> conditional compilation is no-op, i.e. it's always be true. > >> > >> Due to above drop useless #ifdef CONFIG_OF and therefore dead code. > >> > >> Signed-off-by: Andy Shevchenko > >> --- > >> drivers/staging/fbtft/fbtft-core.c | 19 ++- > >> 1 file changed, 2 insertions(+), 17 deletions(-) > >> > >> diff --git a/drivers/staging/fbtft/fbtft-core.c > >> b/drivers/staging/fbtft/fbtft-core.c > > > > > > > >> @@ -1184,17 +1176,10 @@ static struct fbtft_platform_data > >> *fbtft_probe_dt(struct device *dev) > >>pdata->display.backlight = 1; > >>if (of_find_property(node, "init", NULL)) > >>pdata->display.fbtftops.init_display = fbtft_init_display_dt; > >> - pdata->display.fbtftops.request_gpios = fbtft_request_gpios_dt; > >> + pdata->display.fbtftops.request_gpios = fbtft_request_gpios; > > > > You can ditch the .request_gpios callback and call fbtft_request_gpios() > > directly in fbtft_register_framebuffer(). That will make it safe to drop > > the OF dependency, otherwise .request_gpios will be NULL in the non-DT > > case. This is one of the bugs that follwed the gpio refactoring. > > Really difficult to read this fbtft code (that I wrote...). > The NULL deref can only happen when dev->platform_data is set. That > can't happen, in mainline at least, now that fbtft_device is gone. Hmm... If I read code correctly this patch doesn't change this logic. We have non-NULL ->request_gpios() in case of pdata != NULL if and only if supplier gives it to us. The above assignment happens only for DT case (fbtft_properties_read() is guarded against non-DT, okay non-fwnode, cases). > > You can also ditch the .request_gpios_match callback if you want, it > > isn't called anymore (it is set in fb_agm1264k-fl). I guess both improvements can be done later since they are not affecting the logic in this series. -- With Best Regards, Andy Shevchenko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] dt-bindings: vendor-prefixes: Add Shenzhen Frida LCD Co., Ltd.
Add an entry for Shenzhen Frida LCD Co., Ltd. Signed-off-by: Paul Cercueil --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 967e78c5ec0a..9c6e1b42435b 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -337,6 +337,8 @@ patternProperties: description: Firefly "^focaltech,.*": description: FocalTech Systems Co.,Ltd + "^frida,.*": +description: Shenzhen Frida LCD Co., Ltd. "^friendlyarm,.*": description: Guangzhou FriendlyARM Computer Tech Co., Ltd "^fsl,.*": -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 5/5] fbtft: Drop OF dependency
Now, since driver became OF independent, no need to keep OF dependency. Signed-off-by: Andy Shevchenko --- drivers/staging/fbtft/Kconfig | 2 +- drivers/staging/fbtft/fbtft.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig index cb61c2a772bd..54751d9fc0ff 100644 --- a/drivers/staging/fbtft/Kconfig +++ b/drivers/staging/fbtft/Kconfig @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 menuconfig FB_TFT tristate "Support for small TFT LCD display modules" - depends on FB && SPI && OF + depends on FB && SPI depends on GPIOLIB || COMPILE_TEST select FB_SYS_FILLRECT select FB_SYS_COPYAREA diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h index 9b6bdb62093d..5f782da51959 100644 --- a/drivers/staging/fbtft/fbtft.h +++ b/drivers/staging/fbtft/fbtft.h @@ -309,7 +309,7 @@ MODULE_DEVICE_TABLE(of, dt_ids); \ static struct spi_driver fbtft_driver_spi_driver = { \ .driver = {\ .name = _name, \ - .of_match_table = of_match_ptr(dt_ids),\ + .of_match_table = dt_ids, \ }, \ .probe = fbtft_driver_probe_spi, \ .remove = fbtft_driver_remove_spi, \ @@ -319,7 +319,7 @@ static struct platform_driver fbtft_driver_platform_driver = { \ .driver = {\ .name = _name, \ .owner = THIS_MODULE, \ - .of_match_table = of_match_ptr(dt_ids),\ + .of_match_table = dt_ids, \ }, \ .probe = fbtft_driver_probe_pdev, \ .remove = fbtft_driver_remove_pdev,\ -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 1/5] fbtft: Make sure string is NULL terminated
New GCC warns about inappropriate use of strncpy(): drivers/staging/fbtft/fbtft-core.c: In function ‘fbtft_framebuffer_alloc’: drivers/staging/fbtft/fbtft-core.c:665:2: warning: ‘strncpy’ specified bound 16 equals destination size [-Wstringop-truncation] 665 | strncpy(info->fix.id, dev->driver->name, 16); | ^~~~ Later on the copy is being used with the assumption to be NULL terminated. Make sure string is NULL terminated by switching to snprintf(). Signed-off-by: Andy Shevchenko --- drivers/staging/fbtft/fbtft-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index a0a67aa517f0..61f0286fb157 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -666,7 +666,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display, fbdefio->deferred_io = fbtft_deferred_io; fb_deferred_io_init(info); - strncpy(info->fix.id, dev->driver->name, 16); + snprintf(info->fix.id, sizeof(info->fix.id), "%s", dev->driver->name); info->fix.type = FB_TYPE_PACKED_PIXELS; info->fix.visual = FB_VISUAL_TRUECOLOR; info->fix.xpanstep = 0; -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: make various variable in fixed31_32.h 'global' instead of 'static'
fixed31_32.h declare various variables 'static const', it's very ugly and waste of memory. All files that including the header file will have a copy of those variables of their own. And that's the reason why there will be numerous gcc '-Wunused-but-set-variable' warnings related to the variables. Fix it by initializing the variables in a new file "fixed31_32.c", and declare them 'extern' in "fixed31_32.h". Fixes: eb0e515464e4 ("drm/amd/display: get rid of 32.32 unsigned fixed point") Signed-off-by: yu kuai --- BTW, this is the best I can think of, there may be better sulotion. drivers/gpu/drm/amd/display/amdgpu_dm/Makefile | 2 +- .../gpu/drm/amd/display/amdgpu_dm/fixed31_32.c | 17 + .../gpu/drm/amd/display/include/fixed31_32.h| 16 3 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/fixed31_32.c diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile index 9a3b7bf8ab0b..8ce291a0279b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile @@ -25,7 +25,7 @@ -AMDGPUDM = amdgpu_dm.o amdgpu_dm_irq.o amdgpu_dm_mst_types.o amdgpu_dm_color.o +AMDGPUDM = amdgpu_dm.o amdgpu_dm_irq.o amdgpu_dm_mst_types.o amdgpu_dm_color.o fixed31_32.o ifneq ($(CONFIG_DRM_AMD_DC),) AMDGPUDM += amdgpu_dm_services.o amdgpu_dm_helpers.o amdgpu_dm_pp_smu.o diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/fixed31_32.c b/drivers/gpu/drm/amd/display/amdgpu_dm/fixed31_32.c new file mode 100644 index ..1f51587e342b --- /dev/null +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/fixed31_32.c @@ -0,0 +1,17 @@ +/* + * Author: yu kuai + */ + +struct fixed31_32 { + long long value; +}; + +const struct fixed31_32 dc_fixpt_zero = { 0 }; +const struct fixed31_32 dc_fixpt_epsilon = { 1LL }; +const struct fixed31_32 dc_fixpt_half = { 0x8000LL }; +const struct fixed31_32 dc_fixpt_one = { 0x1LL }; + +const struct fixed31_32 dc_fixpt_two_pi = { 26986075409LL }; +const struct fixed31_32 dc_fixpt_ln2 = { 2977044471LL }; +const struct fixed31_32 dc_fixpt_ln2_div_2 = { 1488522236LL }; + diff --git a/drivers/gpu/drm/amd/display/include/fixed31_32.h b/drivers/gpu/drm/amd/display/include/fixed31_32.h index 291215362e3f..d8dbe96f9b19 100644 --- a/drivers/gpu/drm/amd/display/include/fixed31_32.h +++ b/drivers/gpu/drm/amd/display/include/fixed31_32.h @@ -64,14 +64,14 @@ struct fixed31_32 { * Useful constants */ -static const struct fixed31_32 dc_fixpt_zero = { 0 }; -static const struct fixed31_32 dc_fixpt_epsilon = { 1LL }; -static const struct fixed31_32 dc_fixpt_half = { 0x8000LL }; -static const struct fixed31_32 dc_fixpt_one = { 0x1LL }; - -static const struct fixed31_32 dc_fixpt_two_pi = { 26986075409LL }; -static const struct fixed31_32 dc_fixpt_ln2 = { 2977044471LL }; -static const struct fixed31_32 dc_fixpt_ln2_div_2 = { 1488522236LL }; +extern const struct fixed31_32 dc_fixpt_zero; +extern const struct fixed31_32 dc_fixpt_epsilon; +extern const struct fixed31_32 dc_fixpt_half; +extern const struct fixed31_32 dc_fixpt_one; + +extern const struct fixed31_32 dc_fixpt_two_pi; +extern const struct fixed31_32 dc_fixpt_ln2; +extern const struct fixed31_32 dc_fixpt_ln2_div_2; /* * @brief -- 2.17.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] msm:disp:dpu1: add support for display for SC7180 target
Add display hw catalog changes for SC7180 target. Changes in v1: - Configure register offsets and capabilities for the display hw blocks. Changes in v2: - mdss_irq data type has changed in the dependent patch, accomodate the necessary changes. - Add co-developed-by tags in the commit msg (Stephen Boyd). This patch has dependency on the below series https://patchwork.kernel.org/patch/11251761/ Co-developed-by: Shubhashree Dhar Signed-off-by: Shubhashree Dhar Co-developed-by: Raviteja Tamatam Signed-off-by: Raviteja Tamatam Signed-off-by: Kalyan Thota --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 189 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 1 + drivers/gpu/drm/msm/msm_drv.c | 4 +- 5 files changed, 187 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 88f2664..1cf4509 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -11,11 +11,17 @@ #include "dpu_hw_catalog_format.h" #include "dpu_kms.h" -#define VIG_SDM845_MASK \ - (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_SCALER_QSEED3) | BIT(DPU_SSPP_QOS) |\ +#define VIG_MASK \ + (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) |\ BIT(DPU_SSPP_CSC_10BIT) | BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_QOS_8LVL) |\ BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_EXCL_RECT)) +#define VIG_SDM845_MASK \ + (VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3)) + +#define VIG_SC7180_MASK \ + (VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED4)) + #define DMA_SDM845_MASK \ (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) | BIT(DPU_SSPP_QOS_8LVL) |\ BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\ @@ -27,6 +33,9 @@ #define MIXER_SDM845_MASK \ (BIT(DPU_MIXER_SOURCESPLIT) | BIT(DPU_DIM_LAYER)) +#define MIXER_SC7180_MASK \ + (BIT(DPU_DIM_LAYER)) + #define PINGPONG_SDM845_MASK BIT(DPU_PINGPONG_DITHER) #define PINGPONG_SDM845_SPLIT_MASK \ @@ -60,6 +69,16 @@ .has_idle_pc = true, }; +static const struct dpu_caps sc7180_dpu_caps = { + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH, + .max_mixer_blendstages = 0x9, + .qseed_type = DPU_SSPP_SCALER_QSEED4, + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, + .ubwc_version = DPU_HW_UBWC_VER_20, + .has_dim_layer = true, + .has_idle_pc = true, +}; + static struct dpu_mdp_cfg sdm845_mdp[] = { { .name = "top_0", .id = MDP_TOP, @@ -85,6 +104,23 @@ }, }; +static struct dpu_mdp_cfg sc7180_mdp[] = { + { + .name = "top_0", .id = MDP_TOP, + .base = 0x0, .len = 0x494, + .features = 0, + .highest_bank_bit = 0x3, + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { + .reg_off = 0x2AC, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { + .reg_off = 0x2AC, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_DMA1] = { + .reg_off = 0x2B4, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { + .reg_off = 0x2BC, .bit_off = 8}, + }, +}; + /* * CTL sub blocks config */ @@ -116,6 +152,24 @@ }, }; +static struct dpu_ctl_cfg sc7180_ctl[] = { + { + .name = "ctl_0", .id = CTL_0, + .base = 0x1000, .len = 0xE4, + .features = BIT(DPU_CTL_ACTIVE_CFG) + }, + { + .name = "ctl_1", .id = CTL_1, + .base = 0x1200, .len = 0xE4, + .features = BIT(DPU_CTL_ACTIVE_CFG) + }, + { + .name = "ctl_2", .id = CTL_2, + .base = 0x1400, .len = 0xE4, + .features = BIT(DPU_CTL_ACTIVE_CFG) + }, +}; + /* * SSPP sub blocks config */ @@ -203,9 +257,23 @@ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1), }; +static struct dpu_sspp_cfg sc7180_sspp[] = { + SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SC7180_MASK, + sdm845_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0), + SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK, + sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0), + SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK, + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1), + SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK, + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0), +}; + /* * MIXER sub blocks config */ + +/* SDM845 */ + static con
Re: [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens
Hi Jani, On Wed, Nov 20, 2019 at 7:11 AM Jani Nikula wrote: > > On Tue, 12 Nov 2019, Rajat Jain wrote: > > On Mon, Nov 4, 2019 at 11:41 AM Rajat Jain wrote: > >> > >> Certain laptops now come with panels that have integrated privacy > >> screens on them. This patch adds support for such panels by adding > >> a privacy-screen property to the intel_connector for the panel, that > >> the userspace can then use to control and check the status. > >> > >> Identifying the presence of privacy screen, and controlling it, is done > >> via ACPI _DSM methods. > >> > >> Currently, this is done only for the Intel display ports. But in future, > >> this can be done for any other ports if the hardware becomes available > >> (e.g. external monitors supporting integrated privacy screens?). > >> > >> Signed-off-by: Rajat Jain > >> Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548 > > > > > > Hi Folks, > > > > I posted a v2 taking care of the comments I received (also split it > > into 3 patches now, and resused some ACPI code I found in i915 > > driver). . Wondering if any one got a chance to look at this? > > For future reference, please post the updated series standalone, *not* > in reply to long, old threads. Besides myself, it'll also help our CI > find your patches and crunch a bunch of tests on them. Will do. > > Also, do you have an open userspace for this? See [1]. I think this > looks like good stuff to me, but then I'm not responsible for any > userspace component that would actually use this. Not sure what you meant but the user for this on Chromebooks (what I work on) would be the Chrome browser most likely. Thanks & Best Regards, Rajat > > BR, > Jani. > > > [1] > https://www.kernel.org/doc/html/latest/gpu/drm-uapi.html#open-source-userspace-requirements > > > > > > > Thanks, > > > > Rajat > > > >> --- > >> v2: Formed by splitting the original patch into multiple patches. > >> - All code has been moved into i915 now. > >> - Privacy screen is a i915 property > >> - Have a local state variable to store the prvacy screen. Don't read > >> it from hardware. > >> > >> drivers/gpu/drm/i915/Makefile | 3 +- > >> drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++- > >> .../gpu/drm/i915/display/intel_connector.c| 35 ++ > >> .../gpu/drm/i915/display/intel_connector.h| 1 + > >> .../drm/i915/display/intel_display_types.h| 4 ++ > >> drivers/gpu/drm/i915/display/intel_dp.c | 5 ++ > >> .../drm/i915/display/intel_privacy_screen.c | 70 +++ > >> .../drm/i915/display/intel_privacy_screen.h | 25 +++ > >> include/uapi/drm/i915_drm.h | 14 > >> 9 files changed, 166 insertions(+), 4 deletions(-) > >> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c > >> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h > >> > >> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > >> index 2587ea834f06..3589ebcf27bc 100644 > >> --- a/drivers/gpu/drm/i915/Makefile > >> +++ b/drivers/gpu/drm/i915/Makefile > >> @@ -185,7 +185,8 @@ i915-y += \ > >> display/intel_tc.o > >> i915-$(CONFIG_ACPI) += \ > >> display/intel_acpi.o \ > >> - display/intel_opregion.o > >> + display/intel_opregion.o \ > >> + display/intel_privacy_screen.o > >> i915-$(CONFIG_DRM_FBDEV_EMULATION) += \ > >> display/intel_fbdev.o > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > >> b/drivers/gpu/drm/i915/display/intel_atomic.c > >> index d3fb75bb9eb1..378772d3449c 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_atomic.c > >> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > >> @@ -37,6 +37,7 @@ > >> #include "intel_atomic.h" > >> #include "intel_display_types.h" > >> #include "intel_hdcp.h" > >> +#include "intel_privacy_screen.h" > >> #include "intel_sprite.h" > >> > >> /** > >> @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct > >> drm_connector *connector, > >> struct drm_i915_private *dev_priv = to_i915(dev); > >> struct intel_digital_connector_state *intel_conn_state = > >> to_intel_digital_connector_state(state); > >> + struct intel_connector *intel_connector = > >> to_intel_connector(connector); > >> > >> if (property == dev_priv->force_audio_property) > >> *val = intel_conn_state->force_audio; > >> else if (property == dev_priv->broadcast_rgb_property) > >> *val = intel_conn_state->broadcast_rgb; > >> + else if (property == intel_connector->privacy_screen_property) > >> + *val = intel_conn_state->privacy_screen_status; > >> else { > >> DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n", > >> property->base.id, property->name); > >> @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct > >> drm_conn
RE: [PATCH] video: hyperv_fb: Fix hibernation for the deferred IO feature
> -Original Message- > From: Dexuan Cui > Sent: Wednesday, November 20, 2019 3:14 PM > To: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger ; > sas...@kernel.org; b.zolnier...@samsung.com; linux-hyp...@vger.kernel.org; > dri-devel@lists.freedesktop.org; linux-fb...@vger.kernel.org; linux- > ker...@vger.kernel.org; Michael Kelley ; Sasha Levin > > Cc: Wei Hu ; Dexuan Cui > Subject: [PATCH] video: hyperv_fb: Fix hibernation for the deferred IO feature > > fb_deferred_io_work() can access the vmbus ringbuffer by calling > fbdefio->deferred_io() -> synthvid_deferred_io() -> synthvid_update(). > > Because the vmbus ringbuffer is inaccessible between hvfb_suspend() and > hvfb_resume(), we must cancel info->deferred_work before calling > vmbus_close() and then reschedule it after we reopen the channel in > hvfb_resume(). > > Fixes: a4ddb11d297e ("video: hyperv: hyperv_fb: Support deferred IO for > Hyper-V frame buffer driver") > Fixes: 824946a8b6fb ("video: hyperv_fb: Add the support of hibernation") > Signed-off-by: Dexuan Cui > --- > > This patch fixes the 2 aforementioned patches on Sasha Levin's Hyper-V tree's > hyperv-next branch: > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern > el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fhyperv%2Flinux.git%2Flog > %2F%3Fh%3Dhyperv- > next&data=02%7C01%7Cweh%40microsoft.com%7C451143ff78f04401d9 > 6f08d76d893a84%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637 > 098308493217121&sdata=P2fo%2F1TJUMIj5FtJCOp2QwDrghhVfPSCEJ4f1 > vkOXvI%3D&reserved=0 > > The 2 aforementioned patches have not appeared in the mainline yet, so please > pick up this patch onto he same hyperv-next branch. > > drivers/video/fbdev/hyperv_fb.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c > index 4cd27e5172a1..08bc0dfb5ce7 100644 > --- a/drivers/video/fbdev/hyperv_fb.c > +++ b/drivers/video/fbdev/hyperv_fb.c > @@ -1194,6 +1194,7 @@ static int hvfb_suspend(struct hv_device *hdev) > fb_set_suspend(info, 1); > > cancel_delayed_work_sync(&par->dwork); > + cancel_delayed_work_sync(&info->deferred_work); > > par->update_saved = par->update; > par->update = false; > @@ -1227,6 +1228,7 @@ static int hvfb_resume(struct hv_device *hdev) > par->fb_ready = true; > par->update = par->update_saved; > > + schedule_delayed_work(&info->deferred_work, info->fbdefio->delay); > schedule_delayed_work(&par->dwork, HVFB_UPDATE_DELAY); > > /* 0 means do resume */ > -- > 2.19.1 Signed-off-by: Wei Hu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] msm:disp:dpu1: setup display datapath for SC7180 target
Add changes to setup display datapath on SC7180 target. Changes in v1: - Add changes to support ctl_active on SC7180 target. - While selecting the number of mixers in the topology consider the interface width. Changes in v2: - Spawn topology mixer selection into separate patch (Rob Clark). - Add co-developed-by tags in the commit msg (Stephen Boyd). Changes in v3: - Fix kernel checkpatch errors in v2. This patch has dependency on the below series https://patchwork.kernel.org/patch/11253747/ Co-developed-by: Shubhashree Dhar Signed-off-by: Shubhashree Dhar Co-developed-by: Raviteja Tamatam Signed-off-by: Raviteja Tamatam Signed-off-by: Kalyan Thota --- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 21 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 84 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 24 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 28 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 6 ++ 6 files changed, 159 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index b9c84fb..8cc8ad12 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -280,6 +280,14 @@ static void dpu_encoder_phys_vid_setup_timing_engine( phys_enc->hw_intf->ops.setup_timing_gen(phys_enc->hw_intf, &timing_params, fmt); phys_enc->hw_ctl->ops.setup_intf_cfg(phys_enc->hw_ctl, &intf_cfg); + + /* setup which pp blk will connect to this intf */ + if (phys_enc->hw_intf->ops.bind_pingpong_blk) + phys_enc->hw_intf->ops.bind_pingpong_blk( + phys_enc->hw_intf, + true, + phys_enc->hw_pp->idx); + spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags); programmable_fetch_config(phys_enc, &timing_params); @@ -435,6 +443,7 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) { struct dpu_hw_ctl *ctl; u32 flush_mask = 0; + u32 intf_flush_mask = 0; ctl = phys_enc->hw_ctl; @@ -459,10 +468,18 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) ctl->ops.get_bitmask_intf(ctl, &flush_mask, phys_enc->hw_intf->idx); ctl->ops.update_pending_flush(ctl, flush_mask); + if (ctl->ops.get_bitmask_active_intf) + ctl->ops.get_bitmask_active_intf(ctl, &intf_flush_mask, + phys_enc->hw_intf->idx); + + if (ctl->ops.update_pending_intf_flush) + ctl->ops.update_pending_intf_flush(ctl, intf_flush_mask); + skip_flush: DPU_DEBUG_VIDENC(phys_enc, -"update pending flush ctl %d flush_mask %x\n", -ctl->idx - CTL_0, flush_mask); + "update pending flush ctl %d flush_mask 0%x intf_mask 0x%x\n", + ctl->idx - CTL_0, flush_mask, intf_flush_mask); + /* ctl_flush & timing engine enable will be triggered by framework */ if (phys_enc->enable_state == DPU_ENC_DISABLED) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 1cf4509..0ee2b6c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -374,6 +374,7 @@ {\ .name = _name, .id = _id, \ .base = _base, .len = 0x280, \ + .features = BIT(DPU_CTL_ACTIVE_CFG), \ .type = _type, \ .controller_id = _ctrl_id, \ .prog_fetch_lines_worst_case = 24 \ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c index 179e8d5..2ce4b5a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -22,11 +22,15 @@ #define CTL_PREPARE 0x0d0 #define CTL_SW_RESET 0x030 #define CTL_LAYER_EXTN_OFFSET 0x40 +#define CTL_INTF_ACTIVE 0x0F4 +#define CTL_INTF_FLUSH0x110 +#define CTL_INTF_MASTER 0x134 #define CTL_MIXER_BORDER_OUTBIT(24) #define CTL_FLUSH_MASK_CTL BIT(17) #define DPU_REG_RESET_TIMEOUT_US2000 +#define INTF_IDX 31 static struct dpu_ctl_cfg *_ctl_offset(enum dpu_ctl ctl, struct dpu_mdss_cfg *m, @@ -100,11 +104,27 @@ static inline void dpu_hw_ctl_update_pending_flush(struct dpu_hw_ctl *ctx, ctx->pending_flush_mask |= flushbits; } +static inline void dpu_hw_ctl_update_pending_intf_flush(struct dpu_hw_ctl *ctx, + u32 flushbits) +{ + ctx->pending_intf_flush_mask |= flushbits; +} + static u32 dpu_hw_ctl_get_pending_flush(struc
[PATCH v2] msm:disp:dpu1: setup display datapath for SC7180 target
Add changes to setup display datapath on SC7180 target. Changes in v1: - Add changes to support ctl_active on SC7180 target. - While selecting the number of mixers in the topology consider the interface width. Changes in v2: - Spawn topology mixer selection into seperate patch (Rob Clark). - Add co-developed-by tags in the commit msg (Stephen Boyd). This patch has dependency on the below series https://patchwork.kernel.org/patch/11253539/ Co-developed-by: Shubhashree Dhar Signed-off-by: Shubhashree Dhar Co-developed-by: Raviteja Tamatam Signed-off-by: Raviteja Tamatam Signed-off-by: Kalyan Thota --- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 21 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 84 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 24 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 28 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 6 ++ 6 files changed, 159 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index b9c84fb..8cc8ad12 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -280,6 +280,14 @@ static void dpu_encoder_phys_vid_setup_timing_engine( phys_enc->hw_intf->ops.setup_timing_gen(phys_enc->hw_intf, &timing_params, fmt); phys_enc->hw_ctl->ops.setup_intf_cfg(phys_enc->hw_ctl, &intf_cfg); + + /* setup which pp blk will connect to this intf */ + if (phys_enc->hw_intf->ops.bind_pingpong_blk) + phys_enc->hw_intf->ops.bind_pingpong_blk( + phys_enc->hw_intf, + true, + phys_enc->hw_pp->idx); + spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags); programmable_fetch_config(phys_enc, &timing_params); @@ -435,6 +443,7 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) { struct dpu_hw_ctl *ctl; u32 flush_mask = 0; + u32 intf_flush_mask = 0; ctl = phys_enc->hw_ctl; @@ -459,10 +468,18 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) ctl->ops.get_bitmask_intf(ctl, &flush_mask, phys_enc->hw_intf->idx); ctl->ops.update_pending_flush(ctl, flush_mask); + if (ctl->ops.get_bitmask_active_intf) + ctl->ops.get_bitmask_active_intf(ctl, &intf_flush_mask, + phys_enc->hw_intf->idx); + + if (ctl->ops.update_pending_intf_flush) + ctl->ops.update_pending_intf_flush(ctl, intf_flush_mask); + skip_flush: DPU_DEBUG_VIDENC(phys_enc, -"update pending flush ctl %d flush_mask %x\n", -ctl->idx - CTL_0, flush_mask); + "update pending flush ctl %d flush_mask 0%x intf_mask 0x%x\n", + ctl->idx - CTL_0, flush_mask, intf_flush_mask); + /* ctl_flush & timing engine enable will be triggered by framework */ if (phys_enc->enable_state == DPU_ENC_DISABLED) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 1cf4509..0ee2b6c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -374,6 +374,7 @@ {\ .name = _name, .id = _id, \ .base = _base, .len = 0x280, \ + .features = BIT(DPU_CTL_ACTIVE_CFG), \ .type = _type, \ .controller_id = _ctrl_id, \ .prog_fetch_lines_worst_case = 24 \ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c index 179e8d5..2ce4b5a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -22,11 +22,15 @@ #define CTL_PREPARE 0x0d0 #define CTL_SW_RESET 0x030 #define CTL_LAYER_EXTN_OFFSET 0x40 +#define CTL_INTF_ACTIVE 0x0F4 +#define CTL_INTF_FLUSH0x110 +#define CTL_INTF_MASTER 0x134 #define CTL_MIXER_BORDER_OUTBIT(24) #define CTL_FLUSH_MASK_CTL BIT(17) #define DPU_REG_RESET_TIMEOUT_US2000 +#define INTF_IDX 31 static struct dpu_ctl_cfg *_ctl_offset(enum dpu_ctl ctl, struct dpu_mdss_cfg *m, @@ -100,11 +104,27 @@ static inline void dpu_hw_ctl_update_pending_flush(struct dpu_hw_ctl *ctx, ctx->pending_flush_mask |= flushbits; } +static inline void dpu_hw_ctl_update_pending_intf_flush(struct dpu_hw_ctl *ctx, + u32 flushbits) +{ + ctx->pending_intf_flush_mask |= flushbits; +} + static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx) { return ctx->pending_flush
[PATCH 0/2] fix inappropriate use of declaring variable 'static' in fixed31_32.h
The first patch remove two set but not used variable. The second patch make the variables in fixed31_32.h 'global' instead of 'static'. yu kuai (2): drm/amd/display: remove set but not used variable 'dc_fixpt_e' and 'dc_fixpt_pi' drm/amd/display: make various variables in fixed31_32.h 'global' instead of 'static' drivers/gpu/drm/amd/display/amdgpu_dm/Makefile | 2 +- .../gpu/drm/amd/display/amdgpu_dm/fixed31_32.c | 13 + .../gpu/drm/amd/display/include/fixed31_32.h | 18 -- 3 files changed, 22 insertions(+), 11 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/fixed31_32.c -- 2.17.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] msm:disp:dpu1: add support for display for SC7180 target
Add display hw catalog changes for SC7180 target. Changes in v1: - Configure register offsets and capabilities for the display hw blocks. Changes in v2: - mdss_irq data type has changed in the dependent patch, accommodate the necessary changes. - Add co-developed-by tags in the commit msg (Stephen Boyd). Changes in v3: - fix kernel checkpatch errors in v2 This patch has dependency on the below series https://patchwork.kernel.org/patch/11253647/ Co-developed-by: Shubhashree Dhar Signed-off-by: Shubhashree Dhar Co-developed-by: Raviteja Tamatam Signed-off-by: Raviteja Tamatam Signed-off-by: Kalyan Thota --- .../devicetree/bindings/display/msm/dpu.txt| 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 189 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 1 + drivers/gpu/drm/msm/msm_drv.c | 4 +- 6 files changed, 190 insertions(+), 15 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt index a61dd40..512f022 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu.txt +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -8,7 +8,7 @@ The DPU display controller is found in SDM845 SoC. MDSS: Required properties: -- compatible: "qcom,sdm845-mdss" +- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss" - reg: physical base address and length of contoller's registers. - reg-names: register region names. The following region is required: * "mdss" @@ -41,7 +41,7 @@ Optional properties: MDP: Required properties: -- compatible: "qcom,sdm845-dpu" +- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu" - reg: physical base address and length of controller's registers. - reg-names : register region names. The following region is required: * "mdp" diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 88f2664..1cf4509 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -11,11 +11,17 @@ #include "dpu_hw_catalog_format.h" #include "dpu_kms.h" -#define VIG_SDM845_MASK \ - (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_SCALER_QSEED3) | BIT(DPU_SSPP_QOS) |\ +#define VIG_MASK \ + (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) |\ BIT(DPU_SSPP_CSC_10BIT) | BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_QOS_8LVL) |\ BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_EXCL_RECT)) +#define VIG_SDM845_MASK \ + (VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3)) + +#define VIG_SC7180_MASK \ + (VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED4)) + #define DMA_SDM845_MASK \ (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) | BIT(DPU_SSPP_QOS_8LVL) |\ BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\ @@ -27,6 +33,9 @@ #define MIXER_SDM845_MASK \ (BIT(DPU_MIXER_SOURCESPLIT) | BIT(DPU_DIM_LAYER)) +#define MIXER_SC7180_MASK \ + (BIT(DPU_DIM_LAYER)) + #define PINGPONG_SDM845_MASK BIT(DPU_PINGPONG_DITHER) #define PINGPONG_SDM845_SPLIT_MASK \ @@ -60,6 +69,16 @@ .has_idle_pc = true, }; +static const struct dpu_caps sc7180_dpu_caps = { + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH, + .max_mixer_blendstages = 0x9, + .qseed_type = DPU_SSPP_SCALER_QSEED4, + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, + .ubwc_version = DPU_HW_UBWC_VER_20, + .has_dim_layer = true, + .has_idle_pc = true, +}; + static struct dpu_mdp_cfg sdm845_mdp[] = { { .name = "top_0", .id = MDP_TOP, @@ -85,6 +104,23 @@ }, }; +static struct dpu_mdp_cfg sc7180_mdp[] = { + { + .name = "top_0", .id = MDP_TOP, + .base = 0x0, .len = 0x494, + .features = 0, + .highest_bank_bit = 0x3, + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { + .reg_off = 0x2AC, .bit_off = 0}, + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { + .reg_off = 0x2AC, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_DMA1] = { + .reg_off = 0x2B4, .bit_off = 8}, + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { + .reg_off = 0x2BC, .bit_off = 8}, + }, +}; + /* * CTL sub blocks config */ @@ -116,6 +152,24 @@ }, }; +static struct dpu_ctl_cfg sc7180_ctl[] = { + { + .name = "ctl_0", .id = CTL_0, + .base = 0x1000, .len = 0xE4, + .features = BIT(DPU_CTL_ACTIVE_CFG) + }, + { + .name = "ctl_1", .id = CTL_1, + .base = 0x1200, .len = 0xE4, + .features = BIT(DPU_CTL_ACTIVE_CFG) + }, + { + .name = "ctl_2", .id = CTL_2, + .base = 0x1400, .len = 0xE4, + .features = BIT(DPU_CTL_ACTIVE_CFG) + }, +}; + /***
Re: locking&resource refcounting for ttm_bo_kmap/dma_buf_vmap
Hi Am 20.11.19 um 12:47 schrieb Daniel Vetter: > Hi all, > > I've been looking at dma_buf_v(un)map, with a goal to standardize > locking for at least dynamic dma-buf exporters/importers, most likely > by requiring dma_resv_lock. And I got questions around how this is > supposed to work, since a big chunk of drivers seem to entirely lack > locking around ttm_bo_kmap. Two big ones: > > - ttm_bo_kmap looks at bo->mem to figure out what/where to kmap to get > at that buffer. bo->mem is supposed to be protected with > dma_resv_lock, but at least amgpu/nouveau/radeon/qxl don't grab that > in their prime vmap function. > > - between the vmap and vunmap something needs to make sure the backing > storage doesn't move around. I didn't find that either anywhere, > ttm_bo_kmap simply seems to set up the mapping, leaving locking and > refcounting to callers. You have to pin and unpin storage before and after mapping. > - vram helpers have at least locking, but I'm still missing the > refcounting. vmwgfx doesn't bother with vmap. There's a ref counter for pinning [1] and there's a counter for mapping. [2] Are you looking for something else? Best regards Thomas [1] https://cgit.freedesktop.org/drm/drm-tip/tree/include/drm/drm_gem_vram_helper.h?id=8d3996ceedcd5c64f5a354e9dcc64db4a1f72dd6#n69 [2] https://cgit.freedesktop.org/drm/drm-tip/tree/include/drm/drm_gem_vram_helper.h?id=8d3996ceedcd5c64f5a354e9dcc64db4a1f72dd6#n63 > > What am I missing? > > Thanks, Daniel > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V13 4/6] mdev: introduce mediated virtio bus
On Wed, Nov 20, 2019 at 10:14:26AM +0800, Jason Wang wrote: > > > I don't quite get the question here. > > In the driver model the bus_type and foo_device are closely > > linked. > > I don't get the definition of "closely linked" here. Do you think the bus > and device implement virtual bus series are closely linked? If yes, how did > they achieve that? I mean if you have a 'foo_device' then it should be on a 'foo_bus' and not on some 'bar_bus', as that is how the driver core generally works. > > Creating 'mdev_device' instances and overriding the bus_type > > is a very abusive thing to do. > > Ok, mdev_device (without this series) had: > > struct mdev_device { > struct device dev; > struct mdev_parent *parent; > guid_t uuid; > void *driver_data; > struct list_head next; > struct kobject *type_kobj; > struct device *iommu_device; > bool active; > }; > > So it's nothing bus or VFIO specific. And what virtual bus had is: What do mean? 'struct mdev_parent *parent' is the VFIO specific stuff. I haven't figured out what the confusing mdev_parent is supposed to be, or whhy the VFIO ops are linked to the parent or not the device.. Honestly the whole mdev thing has a very strange take on how to use the driver core. > > Abusing it for other things is not appropriate. ie creating an > > instance and not filling in most of the vfio focused ops is an abusive > > thing to do. > > Well, it's only half of the mdev_parent_ops in mdev_parent, various methods > could be done do be more generic to avoid duplication of codes. No? There are many ways to avoid duplicating code. Taking something well defined, and bolting on something unrelated just to share a bit of code is a very poor way to avoid code duplication. > I'm sure you will need to handle other issues besides GUID which had been > settled by mdev e.g IOMMU and types when starting to write a real hardware > driver. The iommu framework already handles that, the mdev stuff contributes very little from what I can see. > > Most likely, at least for virtio-net, everyone else will be able to > > use devlink as well, making it much less clear if that GUID lifecycle > > stuff is a good idea or not. > > This assumption is wrong, we hard already had at least two concrete examples > of vDPA device that doesn't use devlink: > > - Intel IFC where virtio is done at VF level > - Ali Cloud ECS instance where virtio is done at PF level Again, you don't explain why they couldn't use devlink. Or, why do we need GUID lifecycle stuff when these PCI devices can only create a single virtio and can just go ahead and do that as soon as they are probed. The GUID stuff was invented for slicing, which you say is not happening in these cases. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/amd/display: remove set but not used variable 'dc_fixpt_e' and 'dc_fixpt_pi'
'dc_fixpt_e' and 'dc_fixpt_pi' are set in 'fixed31_32.h'. However, they are never used and so can be removed. Fixes: eb0e515464e4 ("drm/amd/display: get rid of 32.32 unsigned fixed point") Signed-off-by: yu kuai --- drivers/gpu/drm/amd/display/include/fixed31_32.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/include/fixed31_32.h b/drivers/gpu/drm/amd/display/include/fixed31_32.h index 89ef9f6860e5..291215362e3f 100644 --- a/drivers/gpu/drm/amd/display/include/fixed31_32.h +++ b/drivers/gpu/drm/amd/display/include/fixed31_32.h @@ -69,9 +69,7 @@ static const struct fixed31_32 dc_fixpt_epsilon = { 1LL }; static const struct fixed31_32 dc_fixpt_half = { 0x8000LL }; static const struct fixed31_32 dc_fixpt_one = { 0x1LL }; -static const struct fixed31_32 dc_fixpt_pi = { 13493037705LL }; static const struct fixed31_32 dc_fixpt_two_pi = { 26986075409LL }; -static const struct fixed31_32 dc_fixpt_e = { 11674931555LL }; static const struct fixed31_32 dc_fixpt_ln2 = { 2977044471LL }; static const struct fixed31_32 dc_fixpt_ln2_div_2 = { 1488522236LL }; -- 2.17.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
Hi, > > update-object-after-move works fine. > > > > try zap mappings with madvise(dontneed) has no bad effects (after vram > > move, trying to force re-creating the ptes). > > Well if it's broken the zapping wouldn't work :-) > > > didn't try the memory pressure thing yet. > > I'm surprised ... and I have no idea how/why it keeps working. > > For my paranoia, can you instrument the ttm page fault handler, and double > check that we get new faults after the move, and set up new ptes for the > same old mapping, but now pointing at the new place in vram? Hmm, only the drm device mapping is faulted in after moving it, the dma-buf mapping is not. Fixed by: -- cut here From 3a7f6c6dbf3b1e4b412c2b283b2ea4edff9f33b5 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 21 Nov 2019 08:39:17 +0100 Subject: [PATCH] drm: share address space for dma bufs --- drivers/gpu/drm/drm_prime.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0814211b0f3f..07c88d2aedee 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv) struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, struct dma_buf_export_info *exp_info) { + struct drm_gem_object *obj = exp_info->priv; struct dma_buf *dma_buf; dma_buf = dma_buf_export(exp_info); @@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, return dma_buf; drm_dev_get(dev); - drm_gem_object_get(exp_info->priv); + drm_gem_object_get(obj); + dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping; return dma_buf; } -- 2.18.1 -- cut here git branch: https://git.kraxel.org/cgit/linux/log/?h=drm-mmap-debug cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
Hi, > Aside: the amdgpu isn't great because it's racy, userspace could have > guessed the fd and already started an mmap before we managed to update > stuff. Can't see that race. When I read the code correctly the fd is created and installed (using dma_buf_fd) only after dmabuf setup is finished. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm/komeda: Enable new product D32 support
D32 is simple version of D71, the difference is: - Only has one pipeline - Drop the periph block and merge it to GCU v2: Rebase. Signed-off-by: James Qian Wang (Arm Technology China) --- .../drm/arm/display/include/malidp_product.h | 3 +- .../arm/display/komeda/d71/d71_component.c| 2 +- .../gpu/drm/arm/display/komeda/d71/d71_dev.c | 43 --- .../gpu/drm/arm/display/komeda/d71/d71_regs.h | 13 ++ .../gpu/drm/arm/display/komeda/komeda_drv.c | 1 + 5 files changed, 44 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h b/drivers/gpu/drm/arm/display/include/malidp_product.h index 96e2e4016250..dbd3d4765065 100644 --- a/drivers/gpu/drm/arm/display/include/malidp_product.h +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h @@ -18,7 +18,8 @@ #define MALIDP_CORE_ID_STATUS(__core_id) (((__u32)(__core_id)) & 0xFF) /* Mali-display product IDs */ -#define MALIDP_D71_PRODUCT_ID 0x0071 +#define MALIDP_D71_PRODUCT_ID 0x0071 +#define MALIDP_D32_PRODUCT_ID 0x0032 union komeda_config_id { struct { diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c index 6dadf4413ef3..c7f7e9c545c7 100644 --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c @@ -1274,7 +1274,7 @@ static int d71_timing_ctrlr_init(struct d71_dev *d71, ctrlr = to_ctrlr(c); - ctrlr->supports_dual_link = true; + ctrlr->supports_dual_link = d71->supports_dual_link; return 0; } diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c index 9b3bf353b6cc..2d429e310e5b 100644 --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c @@ -371,23 +371,33 @@ static int d71_enum_resources(struct komeda_dev *mdev) goto err_cleanup; } - /* probe PERIPH */ + /* Only the legacy HW has the periph block, the newer merges the periph +* into GCU +*/ value = malidp_read32(d71->periph_addr, BLK_BLOCK_INFO); - if (BLOCK_INFO_BLK_TYPE(value) != D71_BLK_TYPE_PERIPH) { - DRM_ERROR("access blk periph but got blk: %d.\n", - BLOCK_INFO_BLK_TYPE(value)); - err = -EINVAL; - goto err_cleanup; + if (BLOCK_INFO_BLK_TYPE(value) != D71_BLK_TYPE_PERIPH) + d71->periph_addr = NULL; + + if (d71->periph_addr) { + /* probe PERIPHERAL in legacy HW */ + value = malidp_read32(d71->periph_addr, PERIPH_CONFIGURATION_ID); + + d71->max_line_size = value & PERIPH_MAX_LINE_SIZE ? 4096 : 2048; + d71->max_vsize = 4096; + d71->num_rich_layers= value & PERIPH_NUM_RICH_LAYERS ? 2 : 1; + d71->supports_dual_link = !!(value & PERIPH_SPLIT_EN); + d71->integrates_tbu = !!(value & PERIPH_TBU_EN); + } else { + value = malidp_read32(d71->gcu_addr, GCU_CONFIGURATION_ID0); + d71->max_line_size = GCU_MAX_LINE_SIZE(value); + d71->max_vsize = GCU_MAX_NUM_LINES(value); + + value = malidp_read32(d71->gcu_addr, GCU_CONFIGURATION_ID1); + d71->num_rich_layers= GCU_NUM_RICH_LAYERS(value); + d71->supports_dual_link = GCU_DISPLAY_SPLIT_EN(value); + d71->integrates_tbu = GCU_DISPLAY_TBU_EN(value); } - value = malidp_read32(d71->periph_addr, PERIPH_CONFIGURATION_ID); - - d71->max_line_size = value & PERIPH_MAX_LINE_SIZE ? 4096 : 2048; - d71->max_vsize = 4096; - d71->num_rich_layers= value & PERIPH_NUM_RICH_LAYERS ? 2 : 1; - d71->supports_dual_link = value & PERIPH_SPLIT_EN ? true : false; - d71->integrates_tbu = value & PERIPH_TBU_EN ? true : false; - for (i = 0; i < d71->num_pipelines; i++) { pipe = komeda_pipeline_add(mdev, sizeof(struct d71_pipeline), &d71_pipeline_funcs); @@ -415,7 +425,7 @@ static int d71_enum_resources(struct komeda_dev *mdev) } /* loop the register blks and probe */ - i = 2; /* exclude GCU and PERIPH */ + i = 1; /* exclude GCU */ offset = D71_BLOCK_SIZE; /* skip GCU */ while (i < d71->num_blocks) { blk_base = mdev->reg_base + (offset >> 2); @@ -425,9 +435,9 @@ static int d71_enum_resources(struct komeda_dev *mdev) err = d71_probe_block(d71, &blk, blk_base); if (err) goto err_cleanup; - i++; } + i++; offset += D71_BLOCK_SIZE; } @@ -603,6 +613,7 @@ d71_identify(u32 __iomem
[PATCH v2 0/2] drm/komeda: Add new product "D32" support
Hi All: This series enables new product "D32" support v2: Rebase james qian wang (Arm Technology China) (2): drm/komeda: Update the chip identify drm/komeda: Enable new product D32 support .../drm/arm/display/include/malidp_product.h | 3 +- .../arm/display/komeda/d71/d71_component.c| 2 +- .../gpu/drm/arm/display/komeda/d71/d71_dev.c | 70 +-- .../gpu/drm/arm/display/komeda/d71/d71_regs.h | 13 .../gpu/drm/arm/display/komeda/komeda_dev.c | 60 .../gpu/drm/arm/display/komeda/komeda_dev.h | 14 +--- .../gpu/drm/arm/display/komeda/komeda_drv.c | 10 +-- 7 files changed, 102 insertions(+), 70 deletions(-) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/komeda: Update the chip identify
1. Drop komeda-CORE product id comparison and put it into the d71_identify 2. Update pipeline node DT-binding: (a). Skip the needless pipeline DT node. (b). Return fail if the essential pipeline DT node is missing. With these changes, for one family chips no need to change the DT. v2: Rebase Signed-off-by: James Qian Wang (Arm Technology China) --- .../gpu/drm/arm/display/komeda/d71/d71_dev.c | 27 +++-- .../gpu/drm/arm/display/komeda/komeda_dev.c | 60 ++- .../gpu/drm/arm/display/komeda/komeda_dev.h | 14 + .../gpu/drm/arm/display/komeda/komeda_drv.c | 9 +-- 4 files changed, 58 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c index 822b23a1ce75..9b3bf353b6cc 100644 --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c @@ -594,10 +594,27 @@ static const struct komeda_dev_funcs d71_chip_funcs = { const struct komeda_dev_funcs * d71_identify(u32 __iomem *reg_base, struct komeda_chip_info *chip) { - chip->arch_id = malidp_read32(reg_base, GLB_ARCH_ID); - chip->core_id = malidp_read32(reg_base, GLB_CORE_ID); - chip->core_info = malidp_read32(reg_base, GLB_CORE_INFO); - chip->bus_width = D71_BUS_WIDTH_16_BYTES; + const struct komeda_dev_funcs *funcs; + u32 product_id; - return &d71_chip_funcs; + chip->core_id = malidp_read32(reg_base, GLB_CORE_ID); + + product_id = MALIDP_CORE_ID_PRODUCT_ID(chip->core_id); + + switch (product_id) { + case MALIDP_D71_PRODUCT_ID: + funcs = &d71_chip_funcs; + break; + default: + funcs = NULL; + DRM_ERROR("Unsupported product: 0x%x\n", product_id); + } + + if (funcs) { + chip->arch_id = malidp_read32(reg_base, GLB_ARCH_ID); + chip->core_info = malidp_read32(reg_base, GLB_CORE_INFO); + chip->bus_width = D71_BUS_WIDTH_16_BYTES; + } + + return funcs; } diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c index 4dd4699d4e3d..8e0bce46555b 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c @@ -116,22 +116,14 @@ static struct attribute_group komeda_sysfs_attr_group = { .attrs = komeda_sysfs_entries, }; -static int komeda_parse_pipe_dt(struct komeda_dev *mdev, struct device_node *np) +static int komeda_parse_pipe_dt(struct komeda_pipeline *pipe) { - struct komeda_pipeline *pipe; + struct device_node *np = pipe->of_node; struct clk *clk; - u32 pipe_id; - int ret = 0; - - ret = of_property_read_u32(np, "reg", &pipe_id); - if (ret != 0 || pipe_id >= mdev->n_pipelines) - return -EINVAL; - - pipe = mdev->pipelines[pipe_id]; clk = of_clk_get_by_name(np, "pxclk"); if (IS_ERR(clk)) { - DRM_ERROR("get pxclk for pipeline %d failed!\n", pipe_id); + DRM_ERROR("get pxclk for pipeline %d failed!\n", pipe->id); return PTR_ERR(clk); } pipe->pxlclk = clk; @@ -145,7 +137,6 @@ static int komeda_parse_pipe_dt(struct komeda_dev *mdev, struct device_node *np) of_graph_get_port_by_id(np, KOMEDA_OF_PORT_OUTPUT); pipe->dual_link = pipe->of_output_links[0] && pipe->of_output_links[1]; - pipe->of_node = of_node_get(np); return 0; } @@ -154,7 +145,9 @@ static int komeda_parse_dt(struct device *dev, struct komeda_dev *mdev) { struct platform_device *pdev = to_platform_device(dev); struct device_node *child, *np = dev->of_node; - int ret; + struct komeda_pipeline *pipe; + u32 pipe_id = U32_MAX; + int ret = -1; mdev->irq = platform_get_irq(pdev, 0); if (mdev->irq < 0) { @@ -169,31 +162,44 @@ static int komeda_parse_dt(struct device *dev, struct komeda_dev *mdev) ret = 0; for_each_available_child_of_node(np, child) { - if (of_node_cmp(child->name, "pipeline") == 0) { - ret = komeda_parse_pipe_dt(mdev, child); - if (ret) { - DRM_ERROR("parse pipeline dt error!\n"); - of_node_put(child); - break; + if (of_node_name_eq(child, "pipeline")) { + of_property_read_u32(child, "reg", &pipe_id); + if (pipe_id >= mdev->n_pipelines) { + DRM_WARN("Skip the redundant DT node: pipeline-%u.\n", +pipe_id); + continue; } + mdev->pipelines[pipe_id]->of_node = of_node_get(child); }
Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
On 9.10.2019 18.50, Daniel Vetter wrote: > On Thu, Oct 03, 2019 at 11:31:25AM -0700, Juston Li wrote: >> From: Daniel Stone >> >> getfb2 allows us to pass multiple planes and modifiers, just like addfb2 >> over addfb. >> >> Changes since v1: >> - unused modifiers set to 0 instead of DRM_FORMAT_MOD_INVALID >> - update ioctl number >> >> Signed-off-by: Daniel Stone >> Signed-off-by: Juston Li > > Looks all good from a very quick glance (kernel, libdrm, igt), but where's > the userspace? Link to weston/drm_hwc/whatever MR good enough. > -Daniel xserver too https://lists.x.org/archives/xorg-devel/2018-March/056363.html to fix https://gitlab.freedesktop.org/xorg/xserver/issues/33 which forces Ubuntu to disable CCS compression, and I'd like to get rid of that patch. Thanks Juston for pushing this! -- t ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 01/24] mm/gup: pass flags arg to __gup_device_* functions
On 11/21/19 12:06 AM, Christoph Hellwig wrote: On Wed, Nov 20, 2019 at 11:13:31PM -0800, John Hubbard wrote: A subsequent patch requires access to gup flags, so pass the flags argument through to the __gup_device_* functions. Looks fine, but why not fold this into the patch using the flags. Yes, I'll do that. Also you can use up your full 73 chars per line in the commit log. OK. thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 02/24] mm/gup: factor out duplicate code from four routines
On 11/21/19 12:03 AM, Christoph Hellwig wrote: On Wed, Nov 20, 2019 at 11:13:32PM -0800, John Hubbard wrote: There are four locations in gup.c that have a fair amount of code duplication. This means that changing one requires making the same changes in four places, not to mention reading the same code four times, and wondering if there are subtle differences. Factor out the common code into static functions, thus reducing the overall line count and the code's complexity. Also, take the opportunity to slightly improve the efficiency of the error cases, by doing a mass subtraction of the refcount, surrounded by get_page()/put_page(). Also, further simplify (slightly), by waiting until the the successful end of each routine, to increment *nr. Any reason for the spurious underscore in the function name? argghh, I just fixed that, but applied the fix to the wrong patch! So now patch 17 ("mm/gup: track FOLL_PIN pages") is improperly renaming it, instead of this patch naming it correctly in the first place. Will fix. Otherwise this looks fine and might be a worthwhile cleanup to feed Andrew for 5.5 independent of the gut of the changes. Reviewed-by: Christoph Hellwig Thanks for the reviews! Say, it sounds like your view here is that this series should be targeted at 5.6 (not 5.5), is that what you have in mind? And get the preparatory patches (1-9, and maybe even 10-16) into 5.5? thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: build failure after merge of the tip tree
On Thu, Nov 21, 2019 at 02:54:03PM +1100, Stephen Rothwell wrote: > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > index f940526c5889..63e734a125fb 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > @@ -473,7 +473,7 @@ TRACE_EVENT(amdgpu_ib_pipe_sync, > TP_PROTO(struct amdgpu_job *sched_job, struct dma_fence *fence), > TP_ARGS(sched_job, fence), > TP_STRUCT__entry( > - __string(ring, sched_job->base.sched->name); > + __string(ring, sched_job->base.sched->name) >__field(uint64_t, id) >__field(struct dma_fence *, fence) >__field(uint64_t, ctx) Correct, ';' there is invalid and now results in very verbose compile errors :-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 06/24] goldish_pipe: rename local pin_user_pages() routine
On 11/21/19 12:08 AM, Christoph Hellwig wrote: On Wed, Nov 20, 2019 at 11:13:36PM -0800, John Hubbard wrote: +static int pin_goldfish_pages(unsigned long first_page, + unsigned long last_page, + unsigned int last_page_size, + int is_write, + struct page *pages[MAX_BUFFERS_PER_COMMAND], + unsigned int *iter_last_page_size) Why not goldfish_pin_pages? Normally we put the module / subsystem in front. Heh, is that how it's supposed to go? Sure, I'll change it. :) Also can we get this queued up for 5.5 to get some trivial changes out of the way? Is that a question to Andrew, or a request for me to send this as a separate patch email (or both)? thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: locking&resource refcounting for ttm_bo_kmap/dma_buf_vmap
On Thu, Nov 21, 2019 at 8:59 AM Thomas Zimmermann wrote: > > Hi > > Am 20.11.19 um 12:47 schrieb Daniel Vetter: > > Hi all, > > > > I've been looking at dma_buf_v(un)map, with a goal to standardize > > locking for at least dynamic dma-buf exporters/importers, most likely > > by requiring dma_resv_lock. And I got questions around how this is > > supposed to work, since a big chunk of drivers seem to entirely lack > > locking around ttm_bo_kmap. Two big ones: > > > > - ttm_bo_kmap looks at bo->mem to figure out what/where to kmap to get > > at that buffer. bo->mem is supposed to be protected with > > dma_resv_lock, but at least amgpu/nouveau/radeon/qxl don't grab that > > in their prime vmap function. > > > > - between the vmap and vunmap something needs to make sure the backing > > storage doesn't move around. I didn't find that either anywhere, > > ttm_bo_kmap simply seems to set up the mapping, leaving locking and > > refcounting to callers. > > You have to pin and unpin storage before and after mapping. Yeah, but you = the exporter, not the importer. I.e. the vmap callback should pin and vunmap should unpin. It's not a huge deal since currently all users of dma_buf_vmap I've found do a dma_buf_attach (even if they never use the attachment directly), and with drm prime helpers the attach results in a pin. Or that's at least the story I told myself to believe this is all not a totally fireworks show right now :-) > > - vram helpers have at least locking, but I'm still missing the > > refcounting. vmwgfx doesn't bother with vmap. > > There's a ref counter for pinning [1] and there's a counter for mapping. > [2] Are you looking for something else? vram helpers actually looked pretty good in this regard. If you look at other callers of ttm_bo_kmap, they're a lot more fast&loose here. -Daniel > > Best regards > Thomas > > [1] > https://cgit.freedesktop.org/drm/drm-tip/tree/include/drm/drm_gem_vram_helper.h?id=8d3996ceedcd5c64f5a354e9dcc64db4a1f72dd6#n69 > [2] > https://cgit.freedesktop.org/drm/drm-tip/tree/include/drm/drm_gem_vram_helper.h?id=8d3996ceedcd5c64f5a354e9dcc64db4a1f72dd6#n63 > > > > > What am I missing? > > > > Thanks, Daniel > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- 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 v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Thu, Nov 21, 2019 at 09:10:21AM +0100, Gerd Hoffmann wrote: > Hi, > > > Aside: the amdgpu isn't great because it's racy, userspace could have > > guessed the fd and already started an mmap before we managed to update > > stuff. > > Can't see that race. When I read the code correctly the fd is created > and installed (using dma_buf_fd) only after dmabuf setup is finished. Right, I mixed things up. Looks all good. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Thu, Nov 21, 2019 at 09:02:59AM +0100, Gerd Hoffmann wrote: > Hi, > > > > update-object-after-move works fine. > > > > > > try zap mappings with madvise(dontneed) has no bad effects (after vram > > > move, trying to force re-creating the ptes). > > > > Well if it's broken the zapping wouldn't work :-) > > > > > didn't try the memory pressure thing yet. > > > > I'm surprised ... and I have no idea how/why it keeps working. > > > > For my paranoia, can you instrument the ttm page fault handler, and double > > check that we get new faults after the move, and set up new ptes for the > > same old mapping, but now pointing at the new place in vram? > > Hmm, only the drm device mapping is faulted in after moving it, > the dma-buf mapping is not. Fixed by: Ah yes, that's more what I'd expect to happen, and the below is what I'd expect to fix things up. I think we should move it up ahead of the device callback (so that drivers can overwrite) and then push as a fix. Separate from a possible patch to undo the fake offset removal. -Daniel > > -- cut here > From 3a7f6c6dbf3b1e4b412c2b283b2ea4edff9f33b5 Mon Sep 17 00:00:00 2001 > From: Gerd Hoffmann > Date: Thu, 21 Nov 2019 08:39:17 +0100 > Subject: [PATCH] drm: share address space for dma bufs > > --- > drivers/gpu/drm/drm_prime.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 0814211b0f3f..07c88d2aedee 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct > drm_prime_file_private *prime_fpriv) > struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, > struct dma_buf_export_info *exp_info) > { > + struct drm_gem_object *obj = exp_info->priv; > struct dma_buf *dma_buf; > > dma_buf = dma_buf_export(exp_info); > @@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device > *dev, > return dma_buf; > > drm_dev_get(dev); > - drm_gem_object_get(exp_info->priv); > + drm_gem_object_get(obj); > + dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping; > > return dma_buf; > } > -- > 2.18.1 > > -- cut here > > git branch: https://git.kraxel.org/cgit/linux/log/?h=drm-mmap-debug > > cheers, > Gerd > -- 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 v7 09/24] vfio, mm: fix get_user_pages_remote() and FOLL_LONGTERM
On 11/21/19 12:10 AM, Christoph Hellwig wrote: Should this be two patches, one for th core infrastructure and one for the user? These changes also look like another candidate to pre-load. OK, I'll split them up. thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 05/24] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On 11/21/19 12:05 AM, Christoph Hellwig wrote: So while this looks correct and I still really don't see the major benefit of the new code organization, especially as it bloats all put_page callers. I'd love to see code size change stats for an allyesconfig on this commit. Right, I'm running that now, will post the results. (btw, if there is a script and/or standard format I should use, I'm all ears. I'll dig through lwn...) thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: build failure after merge of the tip tree
sure, this is a issue. but it wiil build succesed on local drm-next branch. and you can submit this patch to fix this issue. thanks. Reviewed-by: Kevin Wang From: Stephen Rothwell Sent: Thursday, November 21, 2019 11:54 AM To: Thomas Gleixner; Ingo Molnar; H. Peter Anvin; Peter Zijlstra; Dave Airlie; DRI Cc: Linux Next Mailing List; Linux Kernel Mailing List; Wang, Kevin(Yang); Deucher, Alexander Subject: linux-next: build failure after merge of the tip tree Hi all, After merging the tip tree, today's linux-next build (x86_64 allmodconfig) failed like this: In file included from include/trace/define_trace.h:102, from drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:502, from drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c:29: include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:476:52: error: expected expression before ';' token 476 | __string(ring, sched_job->base.sched->name); |^ include/trace/trace_events.h:435:2: note: in definition of macro 'DECLARE_EVENT_CLASS' 435 | tstruct\ | ^~~ include/trace/trace_events.h:77:9: note: in expansion of macro 'PARAMS' 77 | PARAMS(tstruct), \ | ^~ include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:472:1: note: in expansion of macro 'TRACE_EVENT' 472 | TRACE_EVENT(amdgpu_ib_pipe_sync, | ^~~ include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:475:6: note: in expansion of macro 'TP_STRUCT__entry' 475 | TP_STRUCT__entry( | ^~~~ Caused by commit 2c2fdb8bca29 ("drm/amdgpu: fix amdgpu trace event print string format error") from the drm tree interacting with commit 60fdad00827c ("ftrace: Rework event_create_dir()") from the tip tree. I have added the following merge fix patch: From: Stephen Rothwell Date: Thu, 21 Nov 2019 14:46:00 +1100 Subject: [PATCH] merge fix for "ftrace: Rework event_create_dir()" Signed-off-by: Stephen Rothwell --- drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h index f940526c5889..63e734a125fb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h @@ -473,7 +473,7 @@ TRACE_EVENT(amdgpu_ib_pipe_sync, TP_PROTO(struct amdgpu_job *sched_job, struct dma_fence *fence), TP_ARGS(sched_job, fence), TP_STRUCT__entry( -__string(ring, sched_job->base.sched->name); +__string(ring, sched_job->base.sched->name) __field(uint64_t, id) __field(struct dma_fence *, fence) __field(uint64_t, ctx) -- 2.23.0 -- Cheers, Stephen Rothwell ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
On Thu, Nov 21, 2019 at 9:26 AM Timo Aaltonen wrote: > > On 9.10.2019 18.50, Daniel Vetter wrote: > > On Thu, Oct 03, 2019 at 11:31:25AM -0700, Juston Li wrote: > >> From: Daniel Stone > >> > >> getfb2 allows us to pass multiple planes and modifiers, just like addfb2 > >> over addfb. > >> > >> Changes since v1: > >> - unused modifiers set to 0 instead of DRM_FORMAT_MOD_INVALID > >> - update ioctl number > >> > >> Signed-off-by: Daniel Stone > >> Signed-off-by: Juston Li > > > > Looks all good from a very quick glance (kernel, libdrm, igt), but where's > > the userspace? Link to weston/drm_hwc/whatever MR good enough. > > -Daniel > > xserver too > > https://lists.x.org/archives/xorg-devel/2018-March/056363.html > > to fix > > https://gitlab.freedesktop.org/xorg/xserver/issues/33 > > which forces Ubuntu to disable CCS compression, and I'd like to get rid > of that patch. > > Thanks Juston for pushing this! Acked-by: Daniel Vetter ... but someone needs to review all the patches and make sure kernel patch + igt work and pass intel CI and then push it all, and given the pile of committers we have that's not me I think. Just in case people expect me to push this forward, I only jumped in here to make sure new uapi is done by the book and checks all the boxes. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 17/24] mm/gup: track FOLL_PIN pages
On Wed 20-11-19 23:13:47, John Hubbard wrote: > Add tracking of pages that were pinned via FOLL_PIN. > > As mentioned in the FOLL_PIN documentation, callers who effectively set > FOLL_PIN are required to ultimately free such pages via put_user_page(). > The effect is similar to FOLL_GET, and may be thought of as "FOLL_GET > for DIO and/or RDMA use". > > Pages that have been pinned via FOLL_PIN are identifiable via a > new function call: > >bool page_dma_pinned(struct page *page); > > What to do in response to encountering such a page, is left to later > patchsets. There is discussion about this in [1], [2], and [3]. > > This also changes a BUG_ON(), to a WARN_ON(), in follow_page_mask(). > > [1] Some slow progress on get_user_pages() (Apr 2, 2019): > https://lwn.net/Articles/784574/ > [2] DMA and get_user_pages() (LPC: Dec 12, 2018): > https://lwn.net/Articles/774411/ > [3] The trouble with get_user_pages() (Apr 30, 2018): > https://lwn.net/Articles/753027/ > > Suggested-by: Jan Kara > Suggested-by: Jérôme Glisse > Signed-off-by: John Hubbard Thanks for the patch! We are mostly getting there. Some smaller comments below. > +/** > + * try_pin_compound_head() - mark a compound page as being used by > + * pin_user_pages*(). > + * > + * This is the FOLL_PIN counterpart to try_get_compound_head(). > + * > + * @page:pointer to page to be marked > + * @Return: true for success, false for failure > + */ > +__must_check bool try_pin_compound_head(struct page *page, int refs) > +{ > + page = try_get_compound_head(page, GUP_PIN_COUNTING_BIAS * refs); > + if (!page) > + return false; > + > + __update_proc_vmstat(page, NR_FOLL_PIN_REQUESTED, refs); > + return true; > +} > + > +#ifdef CONFIG_DEV_PAGEMAP_OPS > +static bool __put_devmap_managed_user_page(struct page *page) Probably call this __unpin_devmap_managed_user_page()? To match the later conversion of put_user_page() to unpin_user_page()? > +{ > + bool is_devmap = page_is_devmap_managed(page); > + > + if (is_devmap) { > + int count = page_ref_sub_return(page, GUP_PIN_COUNTING_BIAS); > + > + __update_proc_vmstat(page, NR_FOLL_PIN_RETURNED, 1); > + /* > + * devmap page refcounts are 1-based, rather than 0-based: if > + * refcount is 1, then the page is free and the refcount is > + * stable because nobody holds a reference on the page. > + */ > + if (count == 1) > + free_devmap_managed_page(page); > + else if (!count) > + __put_page(page); > + } > + > + return is_devmap; > +} > +#else > +static bool __put_devmap_managed_user_page(struct page *page) > +{ > + return false; > +} > +#endif /* CONFIG_DEV_PAGEMAP_OPS */ > + > +/** > + * put_user_page() - release a dma-pinned page > + * @page:pointer to page to be released > + * > + * Pages that were pinned via pin_user_pages*() must be released via either > + * put_user_page(), or one of the put_user_pages*() routines. This is so that > + * such pages can be separately tracked and uniquely handled. In particular, > + * interactions with RDMA and filesystems need special handling. > + */ > +void put_user_page(struct page *page) > +{ > + page = compound_head(page); > + > + /* > + * For devmap managed pages we need to catch refcount transition from > + * GUP_PIN_COUNTING_BIAS to 1, when refcount reach one it means the > + * page is free and we need to inform the device driver through > + * callback. See include/linux/memremap.h and HMM for details. > + */ > + if (__put_devmap_managed_user_page(page)) > + return; > + > + if (page_ref_sub_and_test(page, GUP_PIN_COUNTING_BIAS)) > + __put_page(page); > + > + __update_proc_vmstat(page, NR_FOLL_PIN_RETURNED, 1); > +} > +EXPORT_SYMBOL(put_user_page); > + > /** > * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned > pages > * @pages: array of pages to be maybe marked dirty, and definitely released. > @@ -237,10 +327,11 @@ static struct page *follow_page_pte(struct > vm_area_struct *vma, > } > > page = vm_normal_page(vma, address, pte); > - if (!page && pte_devmap(pte) && (flags & FOLL_GET)) { > + if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) { > /* > - * Only return device mapping pages in the FOLL_GET case since > - * they are only valid while holding the pgmap reference. > + * Only return device mapping pages in the FOLL_GET or FOLL_PIN > + * case since they are only valid while holding the pgmap > + * reference. >*/ > *pgmap = get_dev_pagemap(pte_pfn(pte), *pgmap); > if (*pgmap) > @@ -283,6 +374,11 @@ static struct page *follow_page_pte(struct > vm_area_struct *vma, >
Re: [PATCH v7 02/24] mm/gup: factor out duplicate code from four routines
On Thu 21-11-19 00:29:59, John Hubbard wrote: > On 11/21/19 12:03 AM, Christoph Hellwig wrote: > > Otherwise this looks fine and might be a worthwhile cleanup to feed > > Andrew for 5.5 independent of the gut of the changes. > > > > Reviewed-by: Christoph Hellwig > > > > Thanks for the reviews! Say, it sounds like your view here is that this > series should be targeted at 5.6 (not 5.5), is that what you have in mind? > And get the preparatory patches (1-9, and maybe even 10-16) into 5.5? Yeah, actually I feel the same. The merge window is going to open on Sunday and the series isn't still fully baked and happily sitting in linux-next (and larger changes should really sit in linux-next for at least a week, preferably two, before the merge window opens to get some reasonable test coverage). So I'd take out the independent easy patches that are already reviewed, get them merged into Andrew's (or whatever other appropriate tree) now so that they get at least a week of testing in linux-next before going upstream. And the more involved bits will have to wait for 5.6 - which means let's just continue working on them as we do now because ideally in 4 weeks we should have them ready with all the reviews so that they can be picked up and integrated into linux-next. Honza -- Jan Kara SUSE Labs, CR ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 6/6] drm/komeda: Expose side_by_side by sysfs/config_id
On Thu, Nov 21, 2019 at 07:12:55AM +, james qian wang (Arm Technology China) wrote: > There are some restrictions if HW works on side_by_side, expose it via > config_id to user. > > Signed-off-by: James Qian Wang (Arm Technology China) > > --- > drivers/gpu/drm/arm/display/include/malidp_product.h | 3 ++- > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h > b/drivers/gpu/drm/arm/display/include/malidp_product.h > index 1053b11352eb..96e2e4016250 100644 > --- a/drivers/gpu/drm/arm/display/include/malidp_product.h > +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h > @@ -27,7 +27,8 @@ union komeda_config_id { > n_scalers:2, /* number of scalers per pipeline */ > n_layers:3, /* number of layers per pipeline */ > n_richs:3, /* number of rich layers per pipeline */ > - reserved_bits:6; > + side_by_side:1, /* if HW works on side_by_side mode */ > + reserved_bits:5; > }; > __u32 value; > }; > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > index c3fa4835cb8d..4dd4699d4e3d 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > @@ -83,6 +83,7 @@ config_id_show(struct device *dev, struct device_attribute > *attr, char *buf) Uh, this sysfs file here looks a lot like uapi for some compositor to decide what to do. Do you have the userspace for this? Also a few more thoughts on this: - You can't just add more fields to uapi structs. - This doesn't really feel like it was ever reviewed to fit into atomic. - sysfs should be one value per file, not a smorgasbrod of things stuffed into a binary structure. -Daniel > memset(&config_id, 0, sizeof(config_id)); > > config_id.max_line_sz = pipe->layers[0]->hsize_in.end; > + config_id.side_by_side = mdev->side_by_side; > config_id.n_pipelines = mdev->n_pipelines; > config_id.n_scalers = pipe->n_scalers; > config_id.n_layers = pipe->n_layers; > -- > 2.20.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 02/24] mm/gup: factor out duplicate code from four routines
On Thu 21-11-19 00:29:59, John Hubbard wrote: > > > > Otherwise this looks fine and might be a worthwhile cleanup to feed > > Andrew for 5.5 independent of the gut of the changes. > > > > Reviewed-by: Christoph Hellwig > > > > Thanks for the reviews! Say, it sounds like your view here is that this > series should be targeted at 5.6 (not 5.5), is that what you have in mind? > And get the preparatory patches (1-9, and maybe even 10-16) into 5.5? One more note :) If you are going to push pin_user_pages() interfaces (which I'm fine with), it would probably make sense to push also the put_user_pages() -> unpin_user_pages() renaming so that that inconsistency in naming does not exist in the released upstream kernel. Honza -- Jan Kara SUSE Labs, CR ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm: Don't init ww_mutec acquire ctx before needed
On Wed, Nov 20, 2019 at 04:24:48PM -0800, Rob Clark wrote: > On Wed, Nov 20, 2019 at 2:56 AM Daniel Vetter wrote: > > > > For locking semantics it really doesn't matter when we grab the > > ticket. But for lockdep validation it does: the acquire ctx is a fake > > lockdep. Since other drivers might want to do a full multi-lock dance > > in their fault-handler, not just lock a single dma_resv. Therefore we > > must init the acquire_ctx only after we've done all the copy_*_user or > > anything else that might trigger a pagefault. For msm this means we > > need to move it past submit_lookup_objects. > > > > Aside: Why is msm still using struct_mutex, it seems to be using > > dma_resv_lock for general buffer state protection? > > > > v2: > > - Add comment to explain why the ww ticket setup is separate (Rob) > > - Fix up error handling, we need to make sure we don't call > > ww_acquire_fini without _init. > > > > Signed-off-by: Daniel Vetter > > Cc: Rob Clark > > Cc: Sean Paul > > Cc: linux-arm-...@vger.kernel.org > > Cc: freedr...@lists.freedesktop.org > > found a few minutes to take this for a spin and seems fine.. t-b && r-b Thanks for taking this for a spin, entire series applied. -Daniel > > BR, > -R > > > --- > > drivers/gpu/drm/msm/msm_gem_submit.c | 9 ++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > > b/drivers/gpu/drm/msm/msm_gem_submit.c > > index fb1fdd7fa3ef..385d4965a8d0 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > @@ -54,7 +54,6 @@ static struct msm_gem_submit *submit_create(struct > > drm_device *dev, > > > > INIT_LIST_HEAD(&submit->node); > > INIT_LIST_HEAD(&submit->bo_list); > > - ww_acquire_init(&submit->ticket, &reservation_ww_class); > > > > return submit; > > } > > @@ -390,8 +389,6 @@ static void submit_cleanup(struct msm_gem_submit > > *submit) > > list_del_init(&msm_obj->submit_entry); > > drm_gem_object_put(&msm_obj->base); > > } > > - > > - ww_acquire_fini(&submit->ticket); > > } > > > > int msm_ioctl_gem_submit(struct drm_device *dev, void *data, > > @@ -408,6 +405,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > > *data, > > struct msm_ringbuffer *ring; > > int out_fence_fd = -1; > > struct pid *pid = get_pid(task_pid(current)); > > + bool has_ww_ticket = false; > > unsigned i; > > int ret, submitid; > > if (!gpu) > > @@ -489,6 +487,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > > *data, > > if (ret) > > goto out; > > > > + /* copy_*_user while holding a ww ticket upsets lockdep */ > > + ww_acquire_init(&submit->ticket, &reservation_ww_class); > > + has_ww_ticket = true; > > ret = submit_lock_objects(submit); > > if (ret) > > goto out; > > @@ -588,6 +589,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > > *data, > > > > out: > > submit_cleanup(submit); > > + if (has_ww_ticket) > > + ww_acquire_fini(&submit->ticket); > > if (ret) > > msm_gem_submit_free(submit); > > out_unlock: > > -- > > 2.24.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 v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > with the branch and patch applied: > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt Thanks for testing. Too bad it did not help :( I suppose there is no change if you increase the delay to say 1s? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Thu, Nov 21, 2019 at 09:49:53AM +0100, Daniel Vetter wrote: > On Thu, Nov 21, 2019 at 09:02:59AM +0100, Gerd Hoffmann wrote: > > Hi, > > > > > > update-object-after-move works fine. > > > > > > > > try zap mappings with madvise(dontneed) has no bad effects (after vram > > > > move, trying to force re-creating the ptes). > > > > > > Well if it's broken the zapping wouldn't work :-) > > > > > > > didn't try the memory pressure thing yet. > > > > > > I'm surprised ... and I have no idea how/why it keeps working. > > > > > > For my paranoia, can you instrument the ttm page fault handler, and double > > > check that we get new faults after the move, and set up new ptes for the > > > same old mapping, but now pointing at the new place in vram? > > > > Hmm, only the drm device mapping is faulted in after moving it, > > the dma-buf mapping is not. Fixed by: > > Ah yes, that's more what I'd expect to happen, and the below is what I'd > expect to fix things up. I think we should move it up ahead of the device > callback (so that drivers can overwrite) and then push as a fix. Separate > from a possible patch to undo the fake offset removal. > -Daniel Is there a more elegant way than copying the intel list on non-intel patches to kick intel ci? cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 1/5] fbtft: Make sure string is NULL terminated
On 20/11/2019 09:57, Andy Shevchenko wrote: > New GCC warns about inappropriate use of strncpy(): > > drivers/staging/fbtft/fbtft-core.c: In function ‘fbtft_framebuffer_alloc’: > drivers/staging/fbtft/fbtft-core.c:665:2: warning: ‘strncpy’ specified bound > 16 equals destination size [-Wstringop-truncation] > 665 | strncpy(info->fix.id, dev->driver->name, 16); > | ^~~~ > > Later on the copy is being used with the assumption to be NULL terminated. > Make sure string is NULL terminated by switching to snprintf(). Even better would be strscpy(): strscpy(info->fix.id, dev->driver->name, sizeof(info->fix.id)); Steve > > Signed-off-by: Andy Shevchenko > --- > drivers/staging/fbtft/fbtft-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/fbtft/fbtft-core.c > b/drivers/staging/fbtft/fbtft-core.c > index a0a67aa517f0..61f0286fb157 100644 > --- a/drivers/staging/fbtft/fbtft-core.c > +++ b/drivers/staging/fbtft/fbtft-core.c > @@ -666,7 +666,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct > fbtft_display *display, > fbdefio->deferred_io = fbtft_deferred_io; > fb_deferred_io_init(info); > > - strncpy(info->fix.id, dev->driver->name, 16); > + snprintf(info->fix.id, sizeof(info->fix.id), "%s", dev->driver->name); > info->fix.type = FB_TYPE_PACKED_PIXELS; > info->fix.visual = FB_VISUAL_TRUECOLOR; > info->fix.xpanstep = 0; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 6/6] drm/komeda: Expose side_by_side by sysfs/config_id
On Thu, Nov 21, 2019 at 10:49:26AM +0100, Daniel Vetter wrote: > On Thu, Nov 21, 2019 at 07:12:55AM +, james qian wang (Arm Technology > China) wrote: > > There are some restrictions if HW works on side_by_side, expose it via > > config_id to user. > > > > Signed-off-by: James Qian Wang (Arm Technology China) > > > > --- > > drivers/gpu/drm/arm/display/include/malidp_product.h | 3 ++- > > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 1 + > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h > > b/drivers/gpu/drm/arm/display/include/malidp_product.h > > index 1053b11352eb..96e2e4016250 100644 > > --- a/drivers/gpu/drm/arm/display/include/malidp_product.h > > +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h > > @@ -27,7 +27,8 @@ union komeda_config_id { > > n_scalers:2, /* number of scalers per pipeline */ > > n_layers:3, /* number of layers per pipeline */ > > n_richs:3, /* number of rich layers per pipeline */ > > - reserved_bits:6; > > + side_by_side:1, /* if HW works on side_by_side mode */ > > + reserved_bits:5; > > }; > > __u32 value; > > }; > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > index c3fa4835cb8d..4dd4699d4e3d 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > @@ -83,6 +83,7 @@ config_id_show(struct device *dev, struct > > device_attribute *attr, char *buf) > > Uh, this sysfs file here looks a lot like uapi for some compositor to > decide what to do. Do you have the userspace for this? Yes, our HWC driver uses this config_id and product_id for identifying the HW caps. > Also a few more thoughts on this: > - You can't just add more fields to uapi structs. > - This doesn't really feel like it was ever reviewed to fit into atomic. > - sysfs should be one value per file, not a smorgasbrod of things stuffed > into a binary structure. I will sent a series and split this struct to multiple files. | This doesn't really feel like it was ever reviewed to fit into atomic These values don't have atomic problem, since config_id is for representing the HW caps info, which are not configurable. Thanks James > -Daniel > > > memset(&config_id, 0, sizeof(config_id)); > > > > config_id.max_line_sz = pipe->layers[0]->hsize_in.end; > > + config_id.side_by_side = mdev->side_by_side; > > config_id.n_pipelines = mdev->n_pipelines; > > config_id.n_scalers = pipe->n_scalers; > > config_id.n_layers = pipe->n_layers; > > -- > > 2.20.1 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Thu, Nov 21, 2019 at 11:18 AM Gerd Hoffmann wrote: > > On Thu, Nov 21, 2019 at 09:49:53AM +0100, Daniel Vetter wrote: > > On Thu, Nov 21, 2019 at 09:02:59AM +0100, Gerd Hoffmann wrote: > > > Hi, > > > > > > > > update-object-after-move works fine. > > > > > > > > > > try zap mappings with madvise(dontneed) has no bad effects (after vram > > > > > move, trying to force re-creating the ptes). > > > > > > > > Well if it's broken the zapping wouldn't work :-) > > > > > > > > > didn't try the memory pressure thing yet. > > > > > > > > I'm surprised ... and I have no idea how/why it keeps working. > > > > > > > > For my paranoia, can you instrument the ttm page fault handler, and > > > > double > > > > check that we get new faults after the move, and set up new ptes for the > > > > same old mapping, but now pointing at the new place in vram? > > > > > > Hmm, only the drm device mapping is faulted in after moving it, > > > the dma-buf mapping is not. Fixed by: > > > > Ah yes, that's more what I'd expect to happen, and the below is what I'd > > expect to fix things up. I think we should move it up ahead of the device > > callback (so that drivers can overwrite) and then push as a fix. Separate > > from a possible patch to undo the fake offset removal. > > -Daniel > > Is there a more elegant way than copying the intel list on non-intel > patches to kick intel ci? Nope, not really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm: share address space for dma bufs
Use the shared address space of the drm device (see drm_open() in drm_file.c) for dma-bufs too. That removes a difference betweem drm device mmap vmas and dma-buf mmap vmas and fixes corner cases like unmaps not working properly. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/drm_prime.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index a9633bd241bb..c3fc341453c0 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv) struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, struct dma_buf_export_info *exp_info) { + struct drm_gem_object *obj = exp_info->priv; struct dma_buf *dma_buf; dma_buf = dma_buf_export(exp_info); @@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, return dma_buf; drm_dev_get(dev); - drm_gem_object_get(exp_info->priv); + drm_gem_object_get(obj); + dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping; return dma_buf; } -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] drm: mmap fixups
Tested on qemu, with bochs (vram helpers) and cirrus (shmem helpers). Cc'ing intel-gfx for CI coverage. Gerd Hoffmann (2): drm: call drm_gem_object_funcs.mmap with fake offset drm: share address space for dma bufs include/drm/drm_gem.h | 4 +--- drivers/gpu/drm/drm_gem.c | 3 --- drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ drivers/gpu/drm/drm_prime.c| 7 ++- drivers/gpu/drm/ttm/ttm_bo_vm.c| 7 --- 5 files changed, 10 insertions(+), 14 deletions(-) -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: call drm_gem_object_funcs.mmap with fake offset
The fake offset is going to stay, so change the calling convention for drm_gem_object_funcs.mmap to include the fake offset. Update all users accordingly. Signed-off-by: Gerd Hoffmann --- include/drm/drm_gem.h | 4 +--- drivers/gpu/drm/drm_gem.c | 3 --- drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ drivers/gpu/drm/drm_prime.c| 3 +++ drivers/gpu/drm/ttm/ttm_bo_vm.c| 7 --- 5 files changed, 7 insertions(+), 13 deletions(-) diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 97a48165642c..0b375069cd48 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -159,9 +159,7 @@ struct drm_gem_object_funcs { * * The callback is used by by both drm_gem_mmap_obj() and * drm_gem_prime_mmap(). When @mmap is present @vm_ops is not -* used, the @mmap callback must set vma->vm_ops instead. The @mmap -* callback is always called with a 0 offset. The caller will remove -* the fake offset as necessary. +* used, the @mmap callback must set vma->vm_ops instead. */ int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 2f2b889096b0..56f42e0f2584 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, return -EINVAL; if (obj->funcs && obj->funcs->mmap) { - /* Remove the fake offset */ - vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); - ret = obj->funcs->mmap(obj, vma); if (ret) return ret; diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 0810d3ef6961..a421a2eed48a 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) struct drm_gem_shmem_object *shmem; int ret; + /* Remove the fake offset */ + vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); + shmem = to_drm_gem_shmem_obj(obj); ret = drm_gem_shmem_get_pages(shmem); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0814211b0f3f..a9633bd241bb 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) int ret; if (obj->funcs && obj->funcs->mmap) { + /* Add the fake offset */ + vma->vm_pgoff += drm_vma_node_start(&obj->vma_node); + ret = obj->funcs->mmap(obj, vma); if (ret) return ret; diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index e6495ca2630b..3e8c3de91ae4 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap); int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo) { ttm_bo_get(bo); - - /* -* FIXME: &drm_gem_object_funcs.mmap is called with the fake offset -* removed. Add it back here until the rest of TTM works without it. -*/ - vma->vm_pgoff += drm_vma_node_start(&bo->base.vma_node); - ttm_bo_mmap_vma_setup(bo, vma); return 0; } -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg wrote: > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > with the branch and patch applied: > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > Thanks for testing. Too bad it did not help :( I suppose there is no > change if you increase the delay to say 1s? Well, look at the original patch in this thread. What it does is to prevent the device (GPU in this particular case) from going into a PCI low-power state before invoking AML to power it down (the AML is still invoked after this patch AFAICS), so why would that have anything to do with the delays? The only reason would be the AML running too early, but that doesn't seem likely. IMO more likely is that the AML does something which cannot be done to a device in a PCI low-power state. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:03 PM Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg > wrote: > > > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > > with the branch and patch applied: > > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > > > Thanks for testing. Too bad it did not help :( I suppose there is no > > change if you increase the delay to say 1s? > > Well, look at the original patch in this thread. > > What it does is to prevent the device (GPU in this particular case) > from going into a PCI low-power state before invoking AML to power it > down (the AML is still invoked after this patch AFAICS), so why would > that have anything to do with the delays? > > The only reason would be the AML running too early, but that doesn't > seem likely. IMO more likely is that the AML does something which > cannot be done to a device in a PCI low-power state. BTW, I'm wondering if anyone has tried to skip the AML instead of skipping the PCI PM in this case (as of 5.4-rc that would be a similar patch to skip the invocations of __pci_start/complete_power_transition() in pci_set_power_state() for the affected device). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:08 PM Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 12:03 PM Rafael J. Wysocki wrote: > > > > On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg > > wrote: > > > > > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > > > with the branch and patch applied: > > > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > > > > > Thanks for testing. Too bad it did not help :( I suppose there is no > > > change if you increase the delay to say 1s? > > > > Well, look at the original patch in this thread. > > > > What it does is to prevent the device (GPU in this particular case) > > from going into a PCI low-power state before invoking AML to power it > > down (the AML is still invoked after this patch AFAICS), so why would > > that have anything to do with the delays? > > > > The only reason would be the AML running too early, but that doesn't > > seem likely. IMO more likely is that the AML does something which > > cannot be done to a device in a PCI low-power state. > > BTW, I'm wondering if anyone has tried to skip the AML instead of > skipping the PCI PM in this case (as of 5.4-rc that would be a similar > patch to skip the invocations of > __pci_start/complete_power_transition() in pci_set_power_state() for > the affected device). Moving the dev->broken_nv_runpm test into pci_platform_power_transition() (also for transitions into D0) would be sufficient for that test if I'm not mistaken. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:03:52PM +0100, Rafael J. Wysocki wrote: > On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg > wrote: > > > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > > with the branch and patch applied: > > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > > > Thanks for testing. Too bad it did not help :( I suppose there is no > > change if you increase the delay to say 1s? > > Well, look at the original patch in this thread. > > What it does is to prevent the device (GPU in this particular case) > from going into a PCI low-power state before invoking AML to power it > down (the AML is still invoked after this patch AFAICS), so why would > that have anything to do with the delays? Yes, I know what it does :) I was just thinking that maybe it's still the link that does not come up when we go back to D0 I guess that's not the case here. > The only reason would be the AML running too early, but that doesn't > seem likely. IMO more likely is that the AML does something which > cannot be done to a device in a PCI low-power state. It may very well be the case. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > last week or so I found systems where the GPU was under the "PCI > > Express Root Port" (name from lspci) and on those systems all of that > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > which also explains Mikas case that Thunderbolt stuff works as devices > > never get populated under this particular bridge controller, but under > > those "Root Port"s > > It always is a PCIe port, but its location within the SoC may matter. Exactly. Intel hardware has PCIe ports on CPU side (these are called PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is still the same. > Also some custom AML-based power management is involved and that may > be making specific assumptions on the configuration of the SoC and the > GPU at the time of its invocation which unfortunately are not known to > us. > > However, it looks like the AML invoked to power down the GPU from > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > that point, so it looks like that AML tries to access device memory on > the GPU (beyond the PCI config space) or similar which is not > accessible in PCI power states below D0. Or the PCI config space of the GPU when the parent root port is in D3hot (as it is the case here). Also then the GPU config space is not accessible. I took a look at the HP Omen ACPI tables which has similar problem and there is also check for Windows 7 (but not Linux) so I think one alternative workaround would be to add these devices into acpi_osi_dmi_table[] where .callback is set to dmi_disable_osi_win8 (or pass 'acpi_osi="!Windows 2012"' in the kernel command line). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:17 PM Mika Westerberg wrote: > > On Thu, Nov 21, 2019 at 12:03:52PM +0100, Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg > > wrote: > > > > > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > > > with the branch and patch applied: > > > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > > > > > Thanks for testing. Too bad it did not help :( I suppose there is no > > > change if you increase the delay to say 1s? > > > > Well, look at the original patch in this thread. > > > > What it does is to prevent the device (GPU in this particular case) > > from going into a PCI low-power state before invoking AML to power it > > down (the AML is still invoked after this patch AFAICS), so why would > > that have anything to do with the delays? > > Yes, I know what it does :) I was just thinking that maybe it's still > the link that does not come up when we go back to D0 I guess that's not > the case here. I'm not sure why that would be related to putting the device into, say, PCI D3 before invoking AML to remove power from it. If it is not in PCI D3 at this point, the AML still runs and still removes power from it IIUC, so on the way back the situation is the same regardless: the device has no power which (again) needs to be restored by AML. That (in principle) should not depend on what happened to the device before it lost power. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg wrote: > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > last week or so I found systems where the GPU was under the "PCI > > > Express Root Port" (name from lspci) and on those systems all of that > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > which also explains Mikas case that Thunderbolt stuff works as devices > > > never get populated under this particular bridge controller, but under > > > those "Root Port"s > > > > It always is a PCIe port, but its location within the SoC may matter. > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > still the same. > > > Also some custom AML-based power management is involved and that may > > be making specific assumptions on the configuration of the SoC and the > > GPU at the time of its invocation which unfortunately are not known to > > us. > > > > However, it looks like the AML invoked to power down the GPU from > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > that point, so it looks like that AML tries to access device memory on > > the GPU (beyond the PCI config space) or similar which is not > > accessible in PCI power states below D0. > > Or the PCI config space of the GPU when the parent root port is in D3hot > (as it is the case here). Also then the GPU config space is not > accessible. Why would the parent port be in D3hot at that point? Wouldn't that be a suspend ordering violation? > I took a look at the HP Omen ACPI tables which has similar problem and > there is also check for Windows 7 (but not Linux) so I think one > alternative workaround would be to add these devices into > acpi_osi_dmi_table[] where .callback is set to dmi_disable_osi_win8 (or > pass 'acpi_osi="!Windows 2012"' in the kernel command line). I'd like to understand the facts that have been established so far before deciding what to do about them. :-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > wrote: > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > last week or so I found systems where the GPU was under the "PCI > > > > Express Root Port" (name from lspci) and on those systems all of that > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > > which also explains Mikas case that Thunderbolt stuff works as devices > > > > never get populated under this particular bridge controller, but under > > > > those "Root Port"s > > > > > > It always is a PCIe port, but its location within the SoC may matter. > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > still the same. > > > > > Also some custom AML-based power management is involved and that may > > > be making specific assumptions on the configuration of the SoC and the > > > GPU at the time of its invocation which unfortunately are not known to > > > us. > > > > > > However, it looks like the AML invoked to power down the GPU from > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > that point, so it looks like that AML tries to access device memory on > > > the GPU (beyond the PCI config space) or similar which is not > > > accessible in PCI power states below D0. > > > > Or the PCI config space of the GPU when the parent root port is in D3hot > > (as it is the case here). Also then the GPU config space is not > > accessible. > > Why would the parent port be in D3hot at that point? Wouldn't that be > a suspend ordering violation? No. We put the GPU into D3hot first, then the root port and then turn off the power resource (which is attached to the root port) resulting the topology entering D3cold. > > I took a look at the HP Omen ACPI tables which has similar problem and > > there is also check for Windows 7 (but not Linux) so I think one > > alternative workaround would be to add these devices into > > acpi_osi_dmi_table[] where .callback is set to dmi_disable_osi_win8 (or > > pass 'acpi_osi="!Windows 2012"' in the kernel command line). > > I'd like to understand the facts that have been established so far > before deciding what to do about them. :-) Yes, I agree :) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 4/7] drm/sun4i: dsi: Handle bus clock explicitly
Hi Maxime, On Sun, Nov 3, 2019 at 11:02 PM Maxime Ripard wrote: > > On Fri, Nov 01, 2019 at 07:42:55PM +0530, Jagan Teki wrote: > > Hi Maxime, > > > > On Tue, Oct 29, 2019 at 2:24 PM Maxime Ripard wrote: > > > > > > On Tue, Oct 29, 2019 at 04:03:56AM +0530, Jagan Teki wrote: > > > > > > explicit handling of common clock would require since the A64 > > > > > > doesn't need to mention the clock-names explicitly in dts since it > > > > > > support only one bus clock. > > > > > > > > > > > > Also pass clk_id NULL instead "bus" to regmap clock init function > > > > > > since the single clock variants no need to mention clock-names > > > > > > explicitly. > > > > > > > > > > You don't need explicit clock handling. Passing NULL as the argument > > > > > in regmap_init_mmio_clk will make it use the first clock, which is the > > > > > bus clock. > > > > > > > > Indeed I tried that, since NULL clk_id wouldn't enable the bus clock > > > > during regmap_mmio_gen_context code, passing NULL triggering vblank > > > > timeout. > > > > > > There's a bunch of users of NULL in tree, so finding out why NULL > > > doesn't work is the way forward. > > > > I'd have looked the some of the users before checking the code as > > well. As I said passing NULL clk_id to devm_regmap_init_mmio_clk => > > __devm_regmap_init_mmio_clk would return before processing the clock. > > > > Here is the code snippet on the tree just to make sure I'm on the same > > page or not. > > > > static struct regmap_mmio_context *regmap_mmio_gen_context(struct device > > *dev, > > const char *clk_id, > > void __iomem *regs, > > const struct regmap_config *config) > > { > > --- > > -- > > if (clk_id == NULL) > > return ctx; > > > > ctx->clk = clk_get(dev, clk_id); > > if (IS_ERR(ctx->clk)) { > > ret = PTR_ERR(ctx->clk); > > goto err_free; > > } > > > > ret = clk_prepare(ctx->clk); > > if (ret < 0) { > > clk_put(ctx->clk); > > goto err_free; > > } > > - > > --- > > } > > > > Yes, I did check on the driver in the tree before committing explicit > > clock handle, which make similar requirements like us in [1]. this > > imx2 wdt driver is handling the explicit clock as well. I'm sure this > > driver is updated as I have seen few changes related to this driver in > > ML. > > I guess we have two ways to go at this then. > > Either we remove the return, but it might have a few side-effects, or > we call clk_get with NULL or bus depending on the case, and then call > regmap_mmio_attach_clk. Thanks for the inputs. Please have a look at this snippet, I have used your second suggestions. let me know if you have any comments? diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 8fa90cfc2ac8..91c95e56d870 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -1109,24 +1109,36 @@ static int sun6i_dsi_probe(struct platform_device *pdev) return PTR_ERR(dsi->regulator); } -dsi->regs = devm_regmap_init_mmio_clk(dev, "bus", base, - &sun6i_dsi_regmap_config); -if (IS_ERR(dsi->regs)) { -dev_err(dev, "Couldn't create the DSI encoder regmap\n"); -return PTR_ERR(dsi->regs); -} - dsi->reset = devm_reset_control_get_shared(dev, NULL); if (IS_ERR(dsi->reset)) { dev_err(dev, "Couldn't get our reset line\n"); return PTR_ERR(dsi->reset); } +dsi->regs = regmap_init_mmio(dev, base, &sun6i_dsi_regmap_config); +if (IS_ERR(dsi->regs)) { +dev_err(dev, "Couldn't init regmap\n"); +return PTR_ERR(dsi->regs); +} + +dsi->bus_clk = devm_clk_get(dev, NULL); +if (IS_ERR(dsi->bus_clk)) { +dev_err(dev, "Couldn't get the DSI bus clock\n"); +ret = PTR_ERR(dsi->bus_clk); +goto err_regmap; +} else { +printk("Jagan.. Got the BUS clock\n"); +ret = regmap_mmio_attach_clk(dsi->regs, dsi->bus_clk); +if (ret) +goto err_bus_clk; +} + if (dsi->variant->has_mod_clk) { dsi->mod_clk = devm_clk_get(dev, "mod"); if (IS_ERR(dsi->mod_clk)) { dev_err(dev, "Couldn't get the DSI mod clock\n"); -return PTR_ERR(dsi->mod_clk); +ret = PTR_ERR(dsi->mod_clk); +goto err_attach_clk; } } @@ -1167,6 +1179,14 @@ static int sun6i_dsi_probe(struct platform_device *pdev) err_unprotect_clk: if (dsi->variant->has_mod_clk) clk_rate_exclusive_put(dsi->mod_clk); +err_attach_clk: +if (!IS_ERR(dsi->bus_clk)) +regmap_mmio_detach_clk(dsi->regs); +err_bus_clk: +if (!IS_ERR(dsi->bus_clk)) +
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:46 PM Mika Westerberg wrote: > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > wrote: > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > Express Root Port" (name from lspci) and on those systems all of that > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > > > which also explains Mikas case that Thunderbolt stuff works as devices > > > > > never get populated under this particular bridge controller, but under > > > > > those "Root Port"s > > > > > > > > It always is a PCIe port, but its location within the SoC may matter. > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > still the same. > > > yeah, I meant the bridge controller with the ID 0x1901 is on the CPU side. And if the Nvidia GPU is on a port on the PCH side it all seems to work just fine. > > > > Also some custom AML-based power management is involved and that may > > > > be making specific assumptions on the configuration of the SoC and the > > > > GPU at the time of its invocation which unfortunately are not known to > > > > us. > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > that point, so it looks like that AML tries to access device memory on > > > > the GPU (beyond the PCI config space) or similar which is not > > > > accessible in PCI power states below D0. > > > > > > Or the PCI config space of the GPU when the parent root port is in D3hot > > > (as it is the case here). Also then the GPU config space is not > > > accessible. > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > a suspend ordering violation? > > No. We put the GPU into D3hot first, then the root port and then turn > off the power resource (which is attached to the root port) resulting > the topology entering D3cold. > If the kernel does a D0 -> D3hot -> D0 cycle this works as well, but the power savings are way lower, so I kind of prefer skipping D3hot instead of D3cold. Skipping D3hot doesn't seem to make any difference in power savings in my testing. > > > I took a look at the HP Omen ACPI tables which has similar problem and > > > there is also check for Windows 7 (but not Linux) so I think one > > > alternative workaround would be to add these devices into > > > acpi_osi_dmi_table[] where .callback is set to dmi_disable_osi_win8 (or > > > pass 'acpi_osi="!Windows 2012"' in the kernel command line). > > > > I'd like to understand the facts that have been established so far > > before deciding what to do about them. :-) > > Yes, I agree :) > Yeah.. and I think those would be too many as we know of several models with this laptops from Lenovo, Dell and HP and random other models from random other OEMs. I think we won't ever be able to blacklist all models if we go that way as those might be just way too many. Also I know from some reports on bumblebee bugs (hitting the same issue essentially) that the acpi_osi overwrite didn't help every user. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote: > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > wrote: > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > Express Root Port" (name from lspci) and on those systems all of that > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > > > which also explains Mikas case that Thunderbolt stuff works as devices > > > > > never get populated under this particular bridge controller, but under > > > > > those "Root Port"s > > > > > > > > It always is a PCIe port, but its location within the SoC may matter. > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > still the same. > > > > > > > Also some custom AML-based power management is involved and that may > > > > be making specific assumptions on the configuration of the SoC and the > > > > GPU at the time of its invocation which unfortunately are not known to > > > > us. > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > that point, so it looks like that AML tries to access device memory on > > > > the GPU (beyond the PCI config space) or similar which is not > > > > accessible in PCI power states below D0. > > > > > > Or the PCI config space of the GPU when the parent root port is in D3hot > > > (as it is the case here). Also then the GPU config space is not > > > accessible. > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > a suspend ordering violation? > > No. We put the GPU into D3hot first, then the root port and then turn > off the power resource (which is attached to the root port) resulting > the topology entering D3cold. I don't see that happening in the AML though. Basically the difference is that when Windows 7 or Linux (the _REV==5 check) then we directly do link disable whereas in Windows 8+ we invoke LKDS() method that puts the link into L2/L3. None of the fields they access seem to touch the GPU itself. LKDS() for the first PEG port looks like this: P0L2 = One Sleep (0x10) Local0 = Zero While (P0L2) { If ((Local0 > 0x04)) { Break } Sleep (0x10) Local0++ } One thing that comes to mind is that the loop can end even if P0L2 is not cleared as it does only 5 iterations with 16 ms sleep between. Maybe Sleep() is implemented differently in Windows? I mean Linux may be "faster" here and return prematurely and if we leave the port into D0 this does not happen, or something. I'm just throwing out ideas :) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 1:52 PM Mika Westerberg wrote: > > On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote: > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > wrote: > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > > Express Root Port" (name from lspci) and on those systems all of > > > > > > that > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > > > > which also explains Mikas case that Thunderbolt stuff works as > > > > > > devices > > > > > > never get populated under this particular bridge controller, but > > > > > > under > > > > > > those "Root Port"s > > > > > > > > > > It always is a PCIe port, but its location within the SoC may matter. > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > > still the same. > > > > > > > > > Also some custom AML-based power management is involved and that may > > > > > be making specific assumptions on the configuration of the SoC and the > > > > > GPU at the time of its invocation which unfortunately are not known to > > > > > us. > > > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > > that point, so it looks like that AML tries to access device memory on > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > accessible in PCI power states below D0. > > > > > > > > Or the PCI config space of the GPU when the parent root port is in D3hot > > > > (as it is the case here). Also then the GPU config space is not > > > > accessible. > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > > a suspend ordering violation? > > > > No. We put the GPU into D3hot first, then the root port and then turn > > off the power resource (which is attached to the root port) resulting > > the topology entering D3cold. > > I don't see that happening in the AML though. > > Basically the difference is that when Windows 7 or Linux (the _REV==5 > check) then we directly do link disable whereas in Windows 8+ we invoke > LKDS() method that puts the link into L2/L3. None of the fields they > access seem to touch the GPU itself. > > LKDS() for the first PEG port looks like this: > >P0L2 = One >Sleep (0x10) >Local0 = Zero >While (P0L2) >{ > If ((Local0 > 0x04)) > { > Break > } > > Sleep (0x10) > Local0++ >} > > One thing that comes to mind is that the loop can end even if P0L2 is > not cleared as it does only 5 iterations with 16 ms sleep between. Maybe > Sleep() is implemented differently in Windows? I mean Linux may be > "faster" here and return prematurely and if we leave the port into D0 > this does not happen, or something. I'm just throwing out ideas :) > keep in mind, that I am able to hit this bug with my python script: https://raw.githubusercontent.com/karolherbst/pci-stub-runpm/master/nv_runpm_bug_test.py ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sun4i: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- drivers/gpu/drm/sun4i/Kconfig | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig index 37e90e42943f..5755f0432e77 100644 --- a/drivers/gpu/drm/sun4i/Kconfig +++ b/drivers/gpu/drm/sun4i/Kconfig @@ -17,18 +17,18 @@ config DRM_SUN4I if DRM_SUN4I config DRM_SUN4I_HDMI - tristate "Allwinner A10 HDMI Controller Support" - default DRM_SUN4I - help + tristate "Allwinner A10 HDMI Controller Support" + default DRM_SUN4I + help Choose this option if you have an Allwinner SoC with an HDMI controller. config DRM_SUN4I_HDMI_CEC - bool "Allwinner A10 HDMI CEC Support" - depends on DRM_SUN4I_HDMI - select CEC_CORE - select CEC_PIN - help + bool "Allwinner A10 HDMI CEC Support" + depends on DRM_SUN4I_HDMI + select CEC_CORE + select CEC_PIN + help Choose this option if you have an Allwinner SoC with an HDMI controller and want to use CEC. -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vc4: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- drivers/gpu/drm/vc4/Kconfig | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig index 7c2317efd5b7..118e8a426b1a 100644 --- a/drivers/gpu/drm/vc4/Kconfig +++ b/drivers/gpu/drm/vc4/Kconfig @@ -22,9 +22,9 @@ config DRM_VC4 our display setup. config DRM_VC4_HDMI_CEC - bool "Broadcom VC4 HDMI CEC Support" - depends on DRM_VC4 - select CEC_CORE - help + bool "Broadcom VC4 HDMI CEC Support" + depends on DRM_VC4 + select CEC_CORE + help Choose this option if you have a Broadcom VC4 GPU and want to use CEC. -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- drivers/gpu/drm/amd/acp/Kconfig | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/acp/Kconfig b/drivers/gpu/drm/amd/acp/Kconfig index d968c2471412..19bae9100da4 100644 --- a/drivers/gpu/drm/amd/acp/Kconfig +++ b/drivers/gpu/drm/amd/acp/Kconfig @@ -2,11 +2,11 @@ menu "ACP (Audio CoProcessor) Configuration" config DRM_AMD_ACP - bool "Enable AMD Audio CoProcessor IP support" - depends on DRM_AMDGPU - select MFD_CORE - select PM_GENERIC_DOMAINS if PM - help + bool "Enable AMD Audio CoProcessor IP support" + depends on DRM_AMDGPU + select MFD_CORE + select PM_GENERIC_DOMAINS if PM + help Choose this option to enable ACP IP support for AMD SOCs. This adds the ACP (Audio CoProcessor) IP driver and wires it up into the amdgpu driver. The ACP block provides the DMA -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205589] Green screen crash with 3400G
https://bugzilla.kernel.org/show_bug.cgi?id=205589 --- Comment #4 from p...@spth.de --- Today, I've also installed a 5.1.21 kernel from Ubuntu on my Debian GNU/Linux testing system, and logged into XFCE thrice. I did not see the green screen crash with that kernel, though one I got a crash (system frozen, only mouse pointer still moveable) a few minutes after logging in. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205049] garbled graphics
https://bugzilla.kernel.org/show_bug.cgi?id=205049 --- Comment #15 from Pierre-Eric Pelloux-Prayer (pierre-eric.pelloux-pra...@amd.com) --- I opened a Merge Request for Mesa to address this issue: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2836 Testing it and reporting the results (here or in the MR comments) would be appreciated. Thanks! -- 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 1/2] drm: call drm_gem_object_funcs.mmap with fake offset
On Thu, Nov 21, 2019 at 11:38:06AM +0100, Gerd Hoffmann wrote: > The fake offset is going to stay, so change the calling convention for > drm_gem_object_funcs.mmap to include the fake offset. Update all users > accordingly. Please add to the commit message: Note that this reverts 83b8a6f242ea ("drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap") and on top then adds the fake offset to drm_gem_prime_mmap to make sure all paths leading to obj->funcs->mmap are consistent. Fixes: 83b8a6f242ea ("drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap") Cc: Gerd Hoffmann Cc: Daniel Vetter Cc: Rob Herring With that also Reviewed-by: Daniel Vetter -Daniel > > Signed-off-by: Gerd Hoffmann > --- > include/drm/drm_gem.h | 4 +--- > drivers/gpu/drm/drm_gem.c | 3 --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ > drivers/gpu/drm/drm_prime.c| 3 +++ > drivers/gpu/drm/ttm/ttm_bo_vm.c| 7 --- > 5 files changed, 7 insertions(+), 13 deletions(-) > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index 97a48165642c..0b375069cd48 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -159,9 +159,7 @@ struct drm_gem_object_funcs { >* >* The callback is used by by both drm_gem_mmap_obj() and >* drm_gem_prime_mmap(). When @mmap is present @vm_ops is not > - * used, the @mmap callback must set vma->vm_ops instead. The @mmap > - * callback is always called with a 0 offset. The caller will remove > - * the fake offset as necessary. > + * used, the @mmap callback must set vma->vm_ops instead. >*/ > int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 2f2b889096b0..56f42e0f2584 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, > unsigned long obj_size, > return -EINVAL; > > if (obj->funcs && obj->funcs->mmap) { > - /* Remove the fake offset */ > - vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); > - > ret = obj->funcs->mmap(obj, vma); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 0810d3ef6961..a421a2eed48a 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct > vm_area_struct *vma) > struct drm_gem_shmem_object *shmem; > int ret; > > + /* Remove the fake offset */ > + vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); > + > shmem = to_drm_gem_shmem_obj(obj); > > ret = drm_gem_shmem_get_pages(shmem); > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 0814211b0f3f..a9633bd241bb 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct > vm_area_struct *vma) > int ret; > > if (obj->funcs && obj->funcs->mmap) { > + /* Add the fake offset */ > + vma->vm_pgoff += drm_vma_node_start(&obj->vma_node); > + > ret = obj->funcs->mmap(obj, vma); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index e6495ca2630b..3e8c3de91ae4 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap); > int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo) > { > ttm_bo_get(bo); > - > - /* > - * FIXME: &drm_gem_object_funcs.mmap is called with the fake offset > - * removed. Add it back here until the rest of TTM works without it. > - */ > - vma->vm_pgoff += drm_vma_node_start(&bo->base.vma_node); > - > ttm_bo_mmap_vma_setup(bo, vma); > return 0; > } > -- > 2.18.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/1] drm: Prefer pcie_capability_read_word()
On Mon, Nov 18, 2019 at 12:42:25PM -0500, Alex Deucher wrote: > On Mon, Nov 18, 2019 at 3:37 AM Frederick Lawler wrote: > > > > Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability") > > added accessors for the PCI Express Capability so that drivers didn't > > need to be aware of differences between v1 and v2 of the PCI > > Express Capability. > > > > Replace pci_read_config_word() and pci_write_config_word() calls with > > pcie_capability_read_word() and pcie_capability_write_word(). > > > > Signed-off-by: Frederick Lawler > > > > --- > > V2 > > - Squash both drm commits into one > > - Rebase ontop of d46eac1e658b > > --- > > drivers/gpu/drm/amd/amdgpu/cik.c | 63 --- > > drivers/gpu/drm/amd/amdgpu/si.c | 71 +++ > > drivers/gpu/drm/radeon/cik.c | 70 ++ > > drivers/gpu/drm/radeon/si.c | 73 > > Can you split this into two patches? One for amdgpu and one for radeon? I split this, and I also went back and split the related patches that preceded this one. I'll post the resulting series for reference. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: share address space for dma bufs
On Thu, Nov 21, 2019 at 11:38:07AM +0100, Gerd Hoffmann wrote: > Use the shared address space of the drm device (see drm_open() in > drm_file.c) for dma-bufs too. That removes a difference betweem drm > device mmap vmas and dma-buf mmap vmas and fixes corner cases like > unmaps not working properly. > > Signed-off-by: Gerd Hoffmann > --- > drivers/gpu/drm/drm_prime.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index a9633bd241bb..c3fc341453c0 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct > drm_prime_file_private *prime_fpriv) > struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, > struct dma_buf_export_info *exp_info) > { > + struct drm_gem_object *obj = exp_info->priv; > struct dma_buf *dma_buf; > > dma_buf = dma_buf_export(exp_info); > @@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device > *dev, > return dma_buf; > > drm_dev_get(dev); > - drm_gem_object_get(exp_info->priv); > + drm_gem_object_get(obj); > + dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping; Can you pls also throw the change to amdgpu_gem_prime_export on top here? Imo makes a lot more sense that way. With that added I'm happy to r-b. -Daniel > > return dma_buf; > } > -- > 2.18.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: call drm_gem_object_funcs.mmap with fake offset
On Thu, Nov 21, 2019 at 02:52:59PM +0100, Daniel Vetter wrote: > On Thu, Nov 21, 2019 at 11:38:06AM +0100, Gerd Hoffmann wrote: > > The fake offset is going to stay, so change the calling convention for > > drm_gem_object_funcs.mmap to include the fake offset. Update all users > > accordingly. > > Please add to the commit message: > > Note that this reverts 83b8a6f242ea ("drm/gem: Fix mmap fake offset > handling for drm_gem_object_funcs.mmap") and on top then adds the fake > offset to drm_gem_prime_mmap to make sure all paths leading to > obj->funcs->mmap are consistent. > > Fixes: 83b8a6f242ea ("drm/gem: Fix mmap fake offset handling for > drm_gem_object_funcs.mmap") > Cc: Gerd Hoffmann > Cc: Daniel Vetter > Cc: Rob Herring > > With that also Reviewed-by: Daniel Vetter Also added Rob to cc here. Rob, can you pls take a look an ack? The sage took another turn :-) -Daniel > -Daniel > > > > Signed-off-by: Gerd Hoffmann > > --- > > include/drm/drm_gem.h | 4 +--- > > drivers/gpu/drm/drm_gem.c | 3 --- > > drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ > > drivers/gpu/drm/drm_prime.c| 3 +++ > > drivers/gpu/drm/ttm/ttm_bo_vm.c| 7 --- > > 5 files changed, 7 insertions(+), 13 deletions(-) > > > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > > index 97a48165642c..0b375069cd48 100644 > > --- a/include/drm/drm_gem.h > > +++ b/include/drm/drm_gem.h > > @@ -159,9 +159,7 @@ struct drm_gem_object_funcs { > > * > > * The callback is used by by both drm_gem_mmap_obj() and > > * drm_gem_prime_mmap(). When @mmap is present @vm_ops is not > > -* used, the @mmap callback must set vma->vm_ops instead. The @mmap > > -* callback is always called with a 0 offset. The caller will remove > > -* the fake offset as necessary. > > +* used, the @mmap callback must set vma->vm_ops instead. > > */ > > int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > index 2f2b889096b0..56f42e0f2584 100644 > > --- a/drivers/gpu/drm/drm_gem.c > > +++ b/drivers/gpu/drm/drm_gem.c > > @@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, > > unsigned long obj_size, > > return -EINVAL; > > > > if (obj->funcs && obj->funcs->mmap) { > > - /* Remove the fake offset */ > > - vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); > > - > > ret = obj->funcs->mmap(obj, vma); > > if (ret) > > return ret; > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > > b/drivers/gpu/drm/drm_gem_shmem_helper.c > > index 0810d3ef6961..a421a2eed48a 100644 > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > @@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, > > struct vm_area_struct *vma) > > struct drm_gem_shmem_object *shmem; > > int ret; > > > > + /* Remove the fake offset */ > > + vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); > > + > > shmem = to_drm_gem_shmem_obj(obj); > > > > ret = drm_gem_shmem_get_pages(shmem); > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > index 0814211b0f3f..a9633bd241bb 100644 > > --- a/drivers/gpu/drm/drm_prime.c > > +++ b/drivers/gpu/drm/drm_prime.c > > @@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, > > struct vm_area_struct *vma) > > int ret; > > > > if (obj->funcs && obj->funcs->mmap) { > > + /* Add the fake offset */ > > + vma->vm_pgoff += drm_vma_node_start(&obj->vma_node); > > + > > ret = obj->funcs->mmap(obj, vma); > > if (ret) > > return ret; > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c > > b/drivers/gpu/drm/ttm/ttm_bo_vm.c > > index e6495ca2630b..3e8c3de91ae4 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > > @@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap); > > int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object > > *bo) > > { > > ttm_bo_get(bo); > > - > > - /* > > -* FIXME: &drm_gem_object_funcs.mmap is called with the fake offset > > -* removed. Add it back here until the rest of TTM works without it. > > -*/ > > - vma->vm_pgoff += drm_vma_node_start(&bo->base.vma_node); > > - > > ttm_bo_mmap_vma_setup(bo, vma); > > return 0; > > } > > -- > > 2.18.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/7] PCI: Add #defines for Enter Compliance, Transmit Margin
From: Bjorn Helgaas Add definitions for the Enter Compliance and Transmit Margin fields of the PCIe Link Control 2 register. Link: https://lore.kernel.org/r/20191112173503.176611-2-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- include/uapi/linux/pci_regs.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 29d6e93fd15e..5869e5778a05 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -673,6 +673,8 @@ #define PCI_EXP_LNKCTL2_TLS_8_0GT 0x0003 /* Supported Speed 8GT/s */ #define PCI_EXP_LNKCTL2_TLS_16_0GT0x0004 /* Supported Speed 16GT/s */ #define PCI_EXP_LNKCTL2_TLS_32_0GT0x0005 /* Supported Speed 32GT/s */ +#define PCI_EXP_LNKCTL2_ENTER_COMP0x0010 /* Enter Compliance */ +#define PCI_EXP_LNKCTL2_TX_MARGIN 0x0380 /* Transmit Margin */ #define PCI_EXP_LNKSTA250 /* Link Status 2 */ #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 52 /* v2 endpoints with link end here */ #define PCI_EXP_SLTCAP252 /* Slot Capabilities 2 */ -- 2.24.0.432.g9d3f5f5b63-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/7] drm/amdgpu: Prefer pcie_capability_read_word()
From: Frederick Lawler Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability") added accessors for the PCI Express Capability so that drivers didn't need to be aware of differences between v1 and v2 of the PCI Express Capability. Replace pci_read_config_word() and pci_write_config_word() calls with pcie_capability_read_word() and pcie_capability_write_word(). [bhelgaas: fix a couple remaining instances in cik.c] Link: https://lore.kernel.org/r/20191118003513.10852-1-f...@fredlawl.com Signed-off-by: Frederick Lawler Signed-off-by: Bjorn Helgaas --- drivers/gpu/drm/amd/amdgpu/cik.c | 71 drivers/gpu/drm/amd/amdgpu/si.c | 71 2 files changed, 90 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c index 3067bb874032..38b06ae6357a 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik.c +++ b/drivers/gpu/drm/amd/amdgpu/cik.c @@ -1384,7 +1384,6 @@ static int cik_set_vce_clocks(struct amdgpu_device *adev, u32 evclk, u32 ecclk) static void cik_pcie_gen3_enable(struct amdgpu_device *adev) { struct pci_dev *root = adev->pdev->bus->self; - int bridge_pos, gpu_pos; u32 speed_cntl, current_data_rate; int i; u16 tmp16; @@ -1419,12 +1418,7 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) DRM_INFO("enabling PCIE gen 2 link speeds, disable with amdgpu.pcie_gen2=0\n"); } - bridge_pos = pci_pcie_cap(root); - if (!bridge_pos) - return; - - gpu_pos = pci_pcie_cap(adev->pdev); - if (!gpu_pos) + if (!pci_is_pcie(root) || !pci_is_pcie(adev->pdev)) return; if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) { @@ -1434,14 +1428,17 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp; - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL, &bridge_cfg); - pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, &gpu_cfg); + pcie_capability_read_word(root, PCI_EXP_LNKCTL, + &bridge_cfg); + pcie_capability_read_word(adev->pdev, PCI_EXP_LNKCTL, + &gpu_cfg); tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL, tmp16); + pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, tmp16); + pcie_capability_write_word(adev->pdev, PCI_EXP_LNKCTL, + tmp16); tmp = RREG32_PCIE(ixPCIE_LC_STATUS1); max_lw = (tmp & PCIE_LC_STATUS1__LC_DETECTED_LINK_WIDTH_MASK) >> @@ -1465,15 +1462,23 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) for (i = 0; i < 10; i++) { /* check status */ - pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_DEVSTA, &tmp16); + pcie_capability_read_word(adev->pdev, + PCI_EXP_DEVSTA, + &tmp16); if (tmp16 & PCI_EXP_DEVSTA_TRPND) break; - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL, &bridge_cfg); - pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, &gpu_cfg); + pcie_capability_read_word(root, PCI_EXP_LNKCTL, + &bridge_cfg); + pcie_capability_read_word(adev->pdev, + PCI_EXP_LNKCTL, + &gpu_cfg); - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &bridge_cfg2); - pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &gpu_cfg2); + pcie_capability_read_word(root, PCI_EXP_LNKCTL2, + &bridge_cfg2); + pcie_capability_read_word(adev->pdev, + PCI_EXP_LNKCTL2, + &gpu_cfg2); tmp = RREG32_PCIE(ixPCIE_LC_CNTL4);
[PATCH v2 0/7] PCI: Prefer pcie_capability_read_word()
From: Bjorn Helgaas Use pcie_capability_read_word() and similar instead of using pci_read_config_word() directly. Add #defines to replace some magic numbers. Fix typos in use of Transmit Margin field. These are currently on my pci/misc branch for v5.5. Let me know if you see any issues. Bjorn Helgaas (5): PCI: Add #defines for Enter Compliance, Transmit Margin drm/amdgpu: Correct Transmit Margin masks drm/amdgpu: Replace numbers with PCI_EXP_LNKCTL2 definitions drm/radeon: Correct Transmit Margin masks drm/radeon: Replace numbers with PCI_EXP_LNKCTL2 definitions Frederick Lawler (2): drm/amdgpu: Prefer pcie_capability_read_word() drm/radeon: Prefer pcie_capability_read_word() drivers/gpu/drm/amd/amdgpu/cik.c | 95 +++ drivers/gpu/drm/amd/amdgpu/si.c | 97 drivers/gpu/drm/radeon/cik.c | 94 +++ drivers/gpu/drm/radeon/si.c | 97 include/uapi/linux/pci_regs.h| 2 + 5 files changed, 243 insertions(+), 142 deletions(-) -- 2.24.0.432.g9d3f5f5b63-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/7] drm/radeon: Prefer pcie_capability_read_word()
From: Frederick Lawler Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability") added accessors for the PCI Express Capability so that drivers didn't need to be aware of differences between v1 and v2 of the PCI Express Capability. Replace pci_read_config_word() and pci_write_config_word() calls with pcie_capability_read_word() and pcie_capability_write_word(). Link: https://lore.kernel.org/r/20191118003513.10852-1-f...@fredlawl.com Signed-off-by: Frederick Lawler Signed-off-by: Bjorn Helgaas --- drivers/gpu/drm/radeon/cik.c | 70 +- drivers/gpu/drm/radeon/si.c | 73 +++- 2 files changed, 90 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index a280442c81aa..09a4709e67f0 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9504,7 +9504,6 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) { struct pci_dev *root = rdev->pdev->bus->self; enum pci_bus_speed speed_cap; - int bridge_pos, gpu_pos; u32 speed_cntl, current_data_rate; int i; u16 tmp16; @@ -9546,12 +9545,7 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n"); } - bridge_pos = pci_pcie_cap(root); - if (!bridge_pos) - return; - - gpu_pos = pci_pcie_cap(rdev->pdev); - if (!gpu_pos) + if (!pci_is_pcie(root) || !pci_is_pcie(rdev->pdev)) return; if (speed_cap == PCIE_SPEED_8_0GT) { @@ -9561,14 +9555,17 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp; - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL, &bridge_cfg); - pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL, &gpu_cfg); + pcie_capability_read_word(root, PCI_EXP_LNKCTL, + &bridge_cfg); + pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL, + &gpu_cfg); tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL, tmp16); + pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL, tmp16); + pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL, + tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_STATUS1); max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK) >> LC_DETECTED_LINK_WIDTH_SHIFT; @@ -9586,15 +9583,23 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) for (i = 0; i < 10; i++) { /* check status */ - pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_DEVSTA, &tmp16); + pcie_capability_read_word(rdev->pdev, + PCI_EXP_DEVSTA, + &tmp16); if (tmp16 & PCI_EXP_DEVSTA_TRPND) break; - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL, &bridge_cfg); - pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL, &gpu_cfg); + pcie_capability_read_word(root, PCI_EXP_LNKCTL, + &bridge_cfg); + pcie_capability_read_word(rdev->pdev, + PCI_EXP_LNKCTL, + &gpu_cfg); - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &bridge_cfg2); - pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &gpu_cfg2); + pcie_capability_read_word(root, PCI_EXP_LNKCTL2, + &bridge_cfg2); + pcie_capability_read_word(rdev->pdev, + PCI_EXP_LNKCTL2, + &gpu_cfg2); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); tmp |= LC_SET_QUIESCE; @@ -9607,32 +9612,45 @@ static void cik_pcie_gen3_enable(struct radeon
[PATCH 6/7] drm/radeon: Replace numbers with PCI_EXP_LNKCTL2 definitions
From: Bjorn Helgaas Replace hard-coded magic numbers with the descriptive PCI_EXP_LNKCTL2 definitions. No functional change intended. Link: https://lore.kernel.org/r/20191112173503.176611-4-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- drivers/gpu/drm/radeon/cik.c | 22 ++ drivers/gpu/drm/radeon/si.c | 22 ++ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 14cdfdf78bde..a280442c81aa 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9619,13 +9619,19 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (bridge_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (gpu_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); @@ -9641,13 +9647,13 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~0xf; + tmp16 &= ~PCI_EXP_LNKCTL2_TLS; if (speed_cap == PCIE_SPEED_8_0GT) - tmp16 |= 3; /* gen3 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */ else if (speed_cap == PCIE_SPEED_5_0GT) - tmp16 |= 2; /* gen2 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */ else - tmp16 |= 1; /* gen1 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */ pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL); diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index 9b7042d3ef1b..529e70a42019 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -7202,13 +7202,19 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (bridge_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (gpu_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); @@ -7224,13 +7230,13 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl); pci_read_config_word(rdev->pdev, gpu_pos + PC
[PATCH 5/7] drm/radeon: Correct Transmit Margin masks
From: Bjorn Helgaas Previously we masked PCIe Link Control 2 register values with "7 << 9", which was apparently intended to be the Transmit Margin field, but instead was the high order bit of Transmit Margin, the Enter Modified Compliance bit, and the Compliance SOS bit. Correct the mask to "7 << 7", which is the Transmit Margin field. Link: https://lore.kernel.org/r/20191112173503.176611-3-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- drivers/gpu/drm/radeon/cik.c | 8 drivers/gpu/drm/radeon/si.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 62eab82a64f9..14cdfdf78bde 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9619,13 +9619,13 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index 05894d198a79..9b7042d3ef1b 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -7202,13 +7202,13 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); -- 2.24.0.432.g9d3f5f5b63-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/7] drm/amdgpu: Correct Transmit Margin masks
From: Bjorn Helgaas Previously we masked PCIe Link Control 2 register values with "7 << 9", which was apparently intended to be the Transmit Margin field, but instead was the high order bit of Transmit Margin, the Enter Modified Compliance bit, and the Compliance SOS bit. Correct the mask to "7 << 7", which is the Transmit Margin field. Link: https://lore.kernel.org/r/20191112173503.176611-3-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/cik.c | 8 drivers/gpu/drm/amd/amdgpu/si.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c index b81bb414fcb3..13a5696d2a6a 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik.c +++ b/drivers/gpu/drm/amd/amdgpu/cik.c @@ -1498,13 +1498,13 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE(ixPCIE_LC_CNTL4); diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c index 493af42152f2..1e350172dc7b 100644 --- a/drivers/gpu/drm/amd/amdgpu/si.c +++ b/drivers/gpu/drm/amd/amdgpu/si.c @@ -1737,13 +1737,13 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev) pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, tmp16); pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); -- 2.24.0.432.g9d3f5f5b63-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/7] drm/amdgpu: Replace numbers with PCI_EXP_LNKCTL2 definitions
From: Bjorn Helgaas Replace hard-coded magic numbers with the descriptive PCI_EXP_LNKCTL2 definitions. No functional change intended. Link: https://lore.kernel.org/r/20191112173503.176611-4-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/cik.c | 22 ++ drivers/gpu/drm/amd/amdgpu/si.c | 22 ++ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c index 13a5696d2a6a..3067bb874032 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik.c +++ b/drivers/gpu/drm/amd/amdgpu/cik.c @@ -1498,13 +1498,19 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (bridge_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (gpu_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE(ixPCIE_LC_CNTL4); @@ -1521,13 +1527,13 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) WREG32_PCIE(ixPCIE_LC_SPEED_CNTL, speed_cntl); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~0xf; + tmp16 &= ~PCI_EXP_LNKCTL2_TLS; if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) - tmp16 |= 3; /* gen3 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */ else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2) - tmp16 |= 2; /* gen2 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */ else - tmp16 |= 1; /* gen1 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */ pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); speed_cntl = RREG32_PCIE(ixPCIE_LC_SPEED_CNTL); diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c index 1e350172dc7b..a7dcb0d0f039 100644 --- a/drivers/gpu/drm/amd/amdgpu/si.c +++ b/drivers/gpu/drm/amd/amdgpu/si.c @@ -1737,13 +1737,19 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev) pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, tmp16); pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (bridge_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (gpu_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); @@ -1758,13 +1764,13 @@ static void si_pcie_gen3_enable
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 1:52 PM Mika Westerberg wrote: > > On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote: > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > wrote: > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > > Express Root Port" (name from lspci) and on those systems all of > > > > > > that > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > > > > which also explains Mikas case that Thunderbolt stuff works as > > > > > > devices > > > > > > never get populated under this particular bridge controller, but > > > > > > under > > > > > > those "Root Port"s > > > > > > > > > > It always is a PCIe port, but its location within the SoC may matter. > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > > still the same. > > > > > > > > > Also some custom AML-based power management is involved and that may > > > > > be making specific assumptions on the configuration of the SoC and the > > > > > GPU at the time of its invocation which unfortunately are not known to > > > > > us. > > > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > > that point, so it looks like that AML tries to access device memory on > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > accessible in PCI power states below D0. > > > > > > > > Or the PCI config space of the GPU when the parent root port is in D3hot > > > > (as it is the case here). Also then the GPU config space is not > > > > accessible. > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > > a suspend ordering violation? > > > > No. We put the GPU into D3hot first, OK Does this involve any AML, like a _PS3 under the GPU object? > > then the root port and then turn > > off the power resource (which is attached to the root port) resulting > > the topology entering D3cold. > > I don't see that happening in the AML though. Which AML do you mean, specifically? The _OFF method for the root port's _PR3 power resource or something else? > Basically the difference is that when Windows 7 or Linux (the _REV==5 > check) then we directly do link disable whereas in Windows 8+ we invoke > LKDS() method that puts the link into L2/L3. None of the fields they > access seem to touch the GPU itself. So that may be where the problem is. Putting the downstream component into PCI D[1-3] is expected to put the link into L1, so I'm not sure how that plays with the later attempt to put it into L2/L3 Ready. Also, L2/L3 Ready is expected to be transient, so finally power should be removed somehow. > LKDS() for the first PEG port looks like this: > >P0L2 = One >Sleep (0x10) >Local0 = Zero >While (P0L2) >{ > If ((Local0 > 0x04)) > { > Break > } > > Sleep (0x10) > Local0++ >} > > One thing that comes to mind is that the loop can end even if P0L2 is > not cleared as it does only 5 iterations with 16 ms sleep between. Maybe > Sleep() is implemented differently in Windows? I mean Linux may be > "faster" here and return prematurely and if we leave the port into D0 > this does not happen, or something. I'm just throwing out ideas :) But this actually works for the downstream component in D0, doesn't it? Also, if the downstream component is in D0, the port actually should stay in D0 too, so what would happen with the $subject patch applied? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 1:53 PM Karol Herbst wrote: > > On Thu, Nov 21, 2019 at 12:46 PM Mika Westerberg > wrote: > > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > wrote: > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > > Express Root Port" (name from lspci) and on those systems all of > > > > > > that > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > > > > which also explains Mikas case that Thunderbolt stuff works as > > > > > > devices > > > > > > never get populated under this particular bridge controller, but > > > > > > under > > > > > > those "Root Port"s > > > > > > > > > > It always is a PCIe port, but its location within the SoC may matter. > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > > still the same. > > > > > > yeah, I meant the bridge controller with the ID 0x1901 is on the CPU > side. And if the Nvidia GPU is on a port on the PCH side it all seems > to work just fine. But that may involve different AML too, may it not? > > > > > Also some custom AML-based power management is involved and that may > > > > > be making specific assumptions on the configuration of the SoC and the > > > > > GPU at the time of its invocation which unfortunately are not known to > > > > > us. > > > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > > that point, so it looks like that AML tries to access device memory on > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > accessible in PCI power states below D0. > > > > > > > > Or the PCI config space of the GPU when the parent root port is in D3hot > > > > (as it is the case here). Also then the GPU config space is not > > > > accessible. > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > > a suspend ordering violation? > > > > No. We put the GPU into D3hot first, then the root port and then turn > > off the power resource (which is attached to the root port) resulting > > the topology entering D3cold. > > > > If the kernel does a D0 -> D3hot -> D0 cycle this works as well, but > the power savings are way lower, so I kind of prefer skipping D3hot > instead of D3cold. Skipping D3hot doesn't seem to make any difference > in power savings in my testing. OK What exactly did you do to skip D3cold in your testing? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 4:47 PM Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 1:53 PM Karol Herbst wrote: > > > > On Thu, Nov 21, 2019 at 12:46 PM Mika Westerberg > > wrote: > > > > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > > wrote: > > > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > > > Express Root Port" (name from lspci) and on those systems all of > > > > > > > that > > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 > > > > > > > one, > > > > > > > which also explains Mikas case that Thunderbolt stuff works as > > > > > > > devices > > > > > > > never get populated under this particular bridge controller, but > > > > > > > under > > > > > > > those "Root Port"s > > > > > > > > > > > > It always is a PCIe port, but its location within the SoC may > > > > > > matter. > > > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > > > still the same. > > > > > > > > > yeah, I meant the bridge controller with the ID 0x1901 is on the CPU > > side. And if the Nvidia GPU is on a port on the PCH side it all seems > > to work just fine. > > But that may involve different AML too, may it not? > > > > > > > Also some custom AML-based power management is involved and that may > > > > > > be making specific assumptions on the configuration of the SoC and > > > > > > the > > > > > > GPU at the time of its invocation which unfortunately are not known > > > > > > to > > > > > > us. > > > > > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > > > that point, so it looks like that AML tries to access device memory > > > > > > on > > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > > accessible in PCI power states below D0. > > > > > > > > > > Or the PCI config space of the GPU when the parent root port is in > > > > > D3hot > > > > > (as it is the case here). Also then the GPU config space is not > > > > > accessible. > > > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > > > a suspend ordering violation? > > > > > > No. We put the GPU into D3hot first, then the root port and then turn > > > off the power resource (which is attached to the root port) resulting > > > the topology entering D3cold. > > > > > > > If the kernel does a D0 -> D3hot -> D0 cycle this works as well, but > > the power savings are way lower, so I kind of prefer skipping D3hot > > instead of D3cold. Skipping D3hot doesn't seem to make any difference > > in power savings in my testing. > > OK > > What exactly did you do to skip D3cold in your testing? > For that I poked into the PCI registers directly and skipped doing the ACPI calls and simply checked for the idle power consumption on my laptop. But I guess I should retest with calling pci_d3cold_disable from nouveau instead? Or is there a different preferable way of testing this? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd: Fix Kconfig indentation
Reviewed-by: Alex Deucher From: Krzysztof Kozlowski Sent: Thursday, November 21, 2019 8:29 AM To: linux-ker...@vger.kernel.org Cc: Krzysztof Kozlowski ; Deucher, Alexander ; Koenig, Christian ; Zhou, David(ChunMing) ; David Airlie ; Daniel Vetter ; amd-...@lists.freedesktop.org ; dri-devel@lists.freedesktop.org Subject: [PATCH] drm/amd: Fix Kconfig indentation Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- drivers/gpu/drm/amd/acp/Kconfig | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/acp/Kconfig b/drivers/gpu/drm/amd/acp/Kconfig index d968c2471412..19bae9100da4 100644 --- a/drivers/gpu/drm/amd/acp/Kconfig +++ b/drivers/gpu/drm/amd/acp/Kconfig @@ -2,11 +2,11 @@ menu "ACP (Audio CoProcessor) Configuration" config DRM_AMD_ACP - bool "Enable AMD Audio CoProcessor IP support" - depends on DRM_AMDGPU - select MFD_CORE - select PM_GENERIC_DOMAINS if PM - help + bool "Enable AMD Audio CoProcessor IP support" + depends on DRM_AMDGPU + select MFD_CORE + select PM_GENERIC_DOMAINS if PM + help Choose this option to enable ACP IP support for AMD SOCs. This adds the ACP (Audio CoProcessor) IP driver and wires it up into the amdgpu driver. The ACP block provides the DMA -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v2 0/7] PCI: Prefer pcie_capability_read_word()
> -Original Message- > From: amd-gfx On Behalf Of > Bjorn Helgaas > Sent: Thursday, November 21, 2019 9:02 AM > To: linux-...@vger.kernel.org > Cc: Zhou, David(ChunMing) ; Frederick Lawler > ; Dave Airlie ; linux- > ker...@vger.kernel.org; dri-devel@lists.freedesktop.org; Bjorn Helgaas > ; amd-...@lists.freedesktop.org; Daniel Vetter > ; Alex Deucher ; Koenig, > Christian > Subject: [PATCH v2 0/7] PCI: Prefer pcie_capability_read_word() > > From: Bjorn Helgaas > > Use pcie_capability_read_word() and similar instead of using > pci_read_config_word() directly. Add #defines to replace some magic > numbers. Fix typos in use of Transmit Margin field. > > These are currently on my pci/misc branch for v5.5. Let me know if you see > any issues. > Series is: Reviewed-by: Alex Deucher > > Bjorn Helgaas (5): > PCI: Add #defines for Enter Compliance, Transmit Margin > drm/amdgpu: Correct Transmit Margin masks > drm/amdgpu: Replace numbers with PCI_EXP_LNKCTL2 definitions > drm/radeon: Correct Transmit Margin masks > drm/radeon: Replace numbers with PCI_EXP_LNKCTL2 definitions > > Frederick Lawler (2): > drm/amdgpu: Prefer pcie_capability_read_word() > drm/radeon: Prefer pcie_capability_read_word() > > drivers/gpu/drm/amd/amdgpu/cik.c | 95 +++ > drivers/gpu/drm/amd/amdgpu/si.c | 97 > drivers/gpu/drm/radeon/cik.c | 94 +++ > drivers/gpu/drm/radeon/si.c | 97 > include/uapi/linux/pci_regs.h| 2 + > 5 files changed, 243 insertions(+), 142 deletions(-) > > -- > 2.24.0.432.g9d3f5f5b63-goog > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 5:06 PM Karol Herbst wrote: > > On Thu, Nov 21, 2019 at 4:47 PM Rafael J. Wysocki wrote: > > > > On Thu, Nov 21, 2019 at 1:53 PM Karol Herbst wrote: > > > > > > On Thu, Nov 21, 2019 at 12:46 PM Mika Westerberg > > > wrote: > > > > > > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > > > wrote: > > > > > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > > > > Express Root Port" (name from lspci) and on those systems all > > > > > > > > of that > > > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 > > > > > > > > one, > > > > > > > > which also explains Mikas case that Thunderbolt stuff works as > > > > > > > > devices > > > > > > > > never get populated under this particular bridge controller, > > > > > > > > but under > > > > > > > > those "Root Port"s > > > > > > > > > > > > > > It always is a PCIe port, but its location within the SoC may > > > > > > > matter. > > > > > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP > > > > > > is > > > > > > still the same. > > > > > > > > > > > > yeah, I meant the bridge controller with the ID 0x1901 is on the CPU > > > side. And if the Nvidia GPU is on a port on the PCH side it all seems > > > to work just fine. > > > > But that may involve different AML too, may it not? > > > > > > > > > Also some custom AML-based power management is involved and that > > > > > > > may > > > > > > > be making specific assumptions on the configuration of the SoC > > > > > > > and the > > > > > > > GPU at the time of its invocation which unfortunately are not > > > > > > > known to > > > > > > > us. > > > > > > > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > > > > that point, so it looks like that AML tries to access device > > > > > > > memory on > > > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > > > accessible in PCI power states below D0. > > > > > > > > > > > > Or the PCI config space of the GPU when the parent root port is in > > > > > > D3hot > > > > > > (as it is the case here). Also then the GPU config space is not > > > > > > accessible. > > > > > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > > > > a suspend ordering violation? > > > > > > > > No. We put the GPU into D3hot first, then the root port and then turn > > > > off the power resource (which is attached to the root port) resulting > > > > the topology entering D3cold. > > > > > > > > > > If the kernel does a D0 -> D3hot -> D0 cycle this works as well, but > > > the power savings are way lower, so I kind of prefer skipping D3hot > > > instead of D3cold. Skipping D3hot doesn't seem to make any difference > > > in power savings in my testing. > > > > OK > > > > What exactly did you do to skip D3cold in your testing? > > > > For that I poked into the PCI registers directly and skipped doing the > ACPI calls and simply checked for the idle power consumption on my > laptop. That doesn't involve the PCIe port PM, however. > But I guess I should retest with calling pci_d3cold_disable > from nouveau instead? Or is there a different preferable way of > testing this? There is a sysfs attribute called "d3cold_allowed" which can be used for "blocking" D3cold, so can you please retest using that? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Intel-gfx] [PATCH 2/2] drm: share address space for dma bufs
>-Original Message- >From: Intel-gfx On Behalf Of Gerd >Hoffmann >Sent: Thursday, November 21, 2019 5:38 AM >To: dri-devel@lists.freedesktop.org >Cc: David Airlie ; intel-...@lists.freedesktop.org; open list >; Maxime Ripard ; Gerd >Hoffmann >Subject: [Intel-gfx] [PATCH 2/2] drm: share address space for dma bufs > >Use the shared address space of the drm device (see drm_open() in >drm_file.c) for dma-bufs too. That removes a difference betweem drm >device mmap vmas and dma-buf mmap vmas and fixes corner cases like >unmaps not working properly. Hi Gerd, Just want to make sure I understand this... So unmaps will not work correctly for mappings when a driver does a drm_vma_node_unamp()? I.e. the dmabuf mappings will not get cleaned up correctly? This is a day one bug? Thanks, Mike >Signed-off-by: Gerd Hoffmann >--- > drivers/gpu/drm/drm_prime.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >index a9633bd241bb..c3fc341453c0 100644 >--- a/drivers/gpu/drm/drm_prime.c >+++ b/drivers/gpu/drm/drm_prime.c >@@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct >drm_prime_file_private *prime_fpriv) > struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, > struct dma_buf_export_info *exp_info) > { >+ struct drm_gem_object *obj = exp_info->priv; > struct dma_buf *dma_buf; > > dma_buf = dma_buf_export(exp_info); >@@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct >drm_device *dev, > return dma_buf; > > drm_dev_get(dev); >- drm_gem_object_get(exp_info->priv); >+ drm_gem_object_get(obj); >+ dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping; > > return dma_buf; > } >-- >2.18.1 > >___ >Intel-gfx mailing list >intel-...@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-fixes
Hi Dave and Daniel, A special thanks to our CI and to Chris here. https://intel-gfx-ci.01.org/tree/drm-intel-fixes/index.html For finding and providing the quick fix for 5.4 on time to avoid the bad corruption with fbdev mmap. Plus other kernel oops and corruption fixes. There was a conflict here with drm-next but it was easy to solve and it is recorded on drm-rerere so that might be transparent now. More details below. Here goes drm-intel-fixes-2019-11-21: - Fix kernel oops on dumb_create ioctl on no crtc situation - Fix bad ugly colored flash on VLV/CHV related to gamma LUT update - Fix unity of the frequencies reported on PMU - Fix kernel oops on set_page_dirty using better locks around it - Protect the request pointer with RCU to prevent it being freed while we might need still - Make pool objects read-only - Restore physical addresses for fb_mmap to avoid corrupted page table Thanks, Rodrigo. The following changes since commit af42d3466bdc8f39806b26f593604fdc54140bcb: Linux 5.4-rc8 (2019-11-17 14:47:30 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2019-11-21 for you to fetch changes up to 71d122629c04707eb7f65447fb2f5bd092d98ce3: drm/i915/fbdev: Restore physical addresses for fb_mmap() (2019-11-21 00:09:22 -0800) - Fix kernel oops on dumb_create ioctl on no crtc situation - Fix bad ugly colored flash on VLV/CHV related to gamma LUT update - Fix unity of the frequencies reported on PMU - Fix kernel oops on set_page_dirty using better locks around it - Protect the request pointer with RCU to prevent it being freed while we might need still - Make pool objects read-only - Restore physical addresses for fb_map to avoid corrupted page table Chris Wilson (4): drm/i915/pmu: "Frequency" is reported as accumulated cycles drm/i915/userptr: Try to acquire the page lock around set_page_dirty() drm/i915: Protect request peeking with RCU drm/i915/fbdev: Restore physical addresses for fb_mmap() Matthew Auld (1): drm/i915: make pool objects read-only Ville Syrjälä (2): drm/i915: Don't oops in dumb_create ioctl if we have no crtcs drm/i915: Preload LUTs if the hw isn't currently using them drivers/gpu/drm/i915/display/intel_atomic.c| 1 + drivers/gpu/drm/i915/display/intel_color.c | 61 ++ drivers/gpu/drm/i915/display/intel_display.c | 9 drivers/gpu/drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/i915/display/intel_fbdev.c | 9 ++-- drivers/gpu/drm/i915/gem/i915_gem_userptr.c| 22 +++- drivers/gpu/drm/i915/gt/intel_engine_pool.c| 2 + drivers/gpu/drm/i915/i915_pmu.c| 4 +- drivers/gpu/drm/i915/i915_scheduler.c | 50 ++ 9 files changed, 141 insertions(+), 18 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v20 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
Hi, cc'ing the drm/bridge maintainers which I think are missing (Andrzej Hajda and Neil Armstrong) Missatge de Matthias Brugger del dia dl., 7 d’oct. 2019 a les 13:04: > > > > On 07/10/2019 10:22, Ulrich Hecht wrote: > > From: Jitao Shi > > > > This patch adds drm_bridge driver for parade DSI to eDP bridge chip. > > > > Signed-off-by: Jitao Shi > > Reviewed-by: Daniel Kurtz > > Reviewed-by: Enric Balletbo i Serra > > [uli: followed API changes, removed FW update feature] > > Signed-off-by: Ulrich Hecht > > Now it works, thanks for pushing this forward :) > > Tested-by: Matthias Brugger > This bridge is really needed in case you have an Acer Chromebook R13, otherwise, you don't have the display working. Would be nice have it upstream :-), If it helps, I tested this patch on top of current 5.4-rc8 and I get the display working, so Tested-by: Enric Balletbo i Serra Thanks, Enric > > --- > > > > Changes since v19: > > - fixed return value of ps8640_probe() when no panel is found > > > > Changes since v18: > > - followed DRM API changes > > - use DEVICE_ATTR_RO() > > - remove firmware update code > > - add SPDX identifier > > > > Changes since v17: > > - remove some unused head files. > > - add macros for ps8640 pages. > > - remove ddc_i2c client > > - add mipi_dsi_device_register_full > > - remove the manufacturer from the name and i2c_device_id > > > > Changes since v16: > > - Disable ps8640 DSI MCS Function. > > - Rename gpios name more clearly. > > - Tune the ps8640 power on sequence. > > > > Changes since v15: > > - Drop drm_connector_(un)register calls from parade ps8640. > >The main DRM driver mtk_drm_drv now calls > >drm_connector_register_all() after drm_dev_register() in the > >mtk_drm_bind() function. That function should iterate over all > >connectors and call drm_connector_register() for each of them. > >So, remove drm_connector_(un)register calls from parade ps8640. > > > > Changes since v14: > > - update copyright info. > > - change bridge_to_ps8640 and connector_to_ps8640 to inline function. > > - fix some coding style. > > - use sizeof as array counter. > > - use drm_get_edid when read edid. > > - add mutex when firmware updating. > > > > Changes since v13: > > - add const on data, ps8640_write_bytes(struct i2c_client *client, const > > u8 *data, u16 data_len) > > - fix PAGE2_SW_REST tyro. > > - move the buf[3] init to entrance of the function. > > > > Changes since v12: > > - fix hw_chip_id build warning > > > > Changes since v11: > > - Remove depends on I2C, add DRM depends > > - Reuse ps8640_write_bytes() in ps8640_write_byte() > > - Use timer check for polling like the routines in > > - Fix no drm_connector_unregister/drm_connector_cleanup when > > ps8640_bridge_attach fail > > - Check the ps8640 hardware id in ps8640_validate_firmware > > - Remove fw_version check > > - Move ps8640_validate_firmware before ps8640_enter_bl > > - Add ddc_i2c unregister when probe fail and ps8640_remove > > > > > > drivers/gpu/drm/bridge/Kconfig | 12 + > > drivers/gpu/drm/bridge/Makefile| 1 + > > drivers/gpu/drm/bridge/parade-ps8640.c | 672 > > + > > 3 files changed, 685 insertions(+) > > create mode 100644 drivers/gpu/drm/bridge/parade-ps8640.c > > > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > > index 1cc9f50..61c6415 100644 > > --- a/drivers/gpu/drm/bridge/Kconfig > > +++ b/drivers/gpu/drm/bridge/Kconfig > > @@ -82,6 +82,18 @@ config DRM_PARADE_PS8622 > > ---help--- > > Parade eDP-LVDS bridge chip driver. > > > > +config DRM_PARADE_PS8640 > > + tristate "Parade PS8640 MIPI DSI to eDP Converter" > > + depends on DRM > > + depends on OF > > + select DRM_KMS_HELPER > > + select DRM_MIPI_DSI > > + select DRM_PANEL > > + help > > + Choose this option if you have PS8640 for display > > + The PS8640 is a high-performance and low-power > > + MIPI DSI to eDP converter > > + > > config DRM_SIL_SII8620 > > tristate "Silicon Image SII8620 HDMI/MHL bridge" > > depends on OF > > diff --git a/drivers/gpu/drm/bridge/Makefile > > b/drivers/gpu/drm/bridge/Makefile > > index 4934fcf..14660ab 100644 > > --- a/drivers/gpu/drm/bridge/Makefile > > +++ b/drivers/gpu/drm/bridge/Makefile > > @@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o > > obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += > > megachips-stdp-ge-b850v3-fw.o > > obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o > > obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o > > +obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o > > obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o > > obj-$(CONFIG_DRM_SII902X) += sii902x.o > > obj-$(CONFIG_DRM_SII9234) += sii9234.o > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c > > b/drivers/gpu/drm/bridge/parade-ps8640.c > > new file mode 100644 > > index 000..ac2701
[PATCH][next] drm/amdgpu: remove redundant assignment to pointer write_frame
From: Colin Ian King The pointer write_frame is being initialized with a value that is never read and it is being updated later with a new value. The initialization is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 2a8a08aa6eaf..c02f9ffe5c6b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -1728,7 +1728,7 @@ int psp_ring_cmd_submit(struct psp_context *psp, int index) { unsigned int psp_write_ptr_reg = 0; - struct psp_gfx_rb_frame *write_frame = psp->km_ring.ring_mem; + struct psp_gfx_rb_frame *write_frame; struct psp_ring *ring = &psp->km_ring; struct psp_gfx_rb_frame *ring_buffer_start = ring->ring_mem; struct psp_gfx_rb_frame *ring_buffer_end = ring_buffer_start + -- 2.24.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 05/24] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On Thu, Nov 21, 2019 at 12:57 AM John Hubbard wrote: > > On 11/21/19 12:05 AM, Christoph Hellwig wrote: > > So while this looks correct and I still really don't see the major > > benefit of the new code organization, especially as it bloats all > > put_page callers. > > > > I'd love to see code size change stats for an allyesconfig on this > > commit. > > > > Right, I'm running that now, will post the results. (btw, if there is > a script and/or standard format I should use, I'm all ears. I'll dig > through lwn...) > Just run: size vmlinux ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3/RFC 0/4] AFBC rework and support for Rockchip
v2..v3: - addressed (some) comments from Daniel Stone, Liviu Dudau, Daniel Vetter and Brian Starkey - thank you all In this iteration some rework has been done. The checking logic is now moved to framebuffer_check() so it is common to all drivers. But the common part is not good for komeda, so this series is not good for merging yet. I kindly ask for feedback whether the changes are in the right direction. I also kindly ask for input on how to accommodate komeda. The CONFIG_DRM_AFBC option has been eliminated in favour of adding drm_afbc.c to drm_kms_helper. v1..v2: This series adds AFBC support for Rockchip. It is inspired by: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/factory-gru-9017.B-chromeos-4.4/drivers/gpu/drm/rockchip/rockchip_drm_vop.c - addressed comments from Daniel Stone, Ayan Halder, Mihail Atanassov - coding style fixes Andrzej Pietrasiewicz (4): drm/arm: Factor out generic afbc helpers drm/malidp: use afbc helpers drm/komeda: Use afbc helper drm/rockchip: Add support for afbc drivers/gpu/drm/Makefile | 2 +- .../arm/display/komeda/komeda_framebuffer.c | 17 +-- drivers/gpu/drm/arm/malidp_drv.c | 121 +++ drivers/gpu/drm/drm_afbc.c| 84 +++ drivers/gpu/drm/drm_fourcc.c | 11 +- drivers/gpu/drm/drm_framebuffer.c | 71 - drivers/gpu/drm/rockchip/rockchip_drm_fb.c| 29 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 142 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 ++ drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 84 ++- include/drm/drm_afbc.h| 35 + 11 files changed, 485 insertions(+), 123 deletions(-) create mode 100644 drivers/gpu/drm/drm_afbc.c create mode 100644 include/drm/drm_afbc.h -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3/RFC 4/4] drm/rockchip: Add support for afbc
This patch adds support for afbc handling. afbc is a compressed format which reduces the necessary memory bandwidth. Co-developed-by: Mark Yao Signed-off-by: Mark Yao Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 29 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 142 +++- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 ++ drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 84 +++- 4 files changed, 263 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index ca01234c037c..7eaa3fdc03b2 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -18,6 +19,8 @@ #include "rockchip_drm_fb.h" #include "rockchip_drm_gem.h" +#define ROCKCHIP_MAX_AFBC_WIDTH2560 + static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = { .destroy = drm_gem_fb_destroy, .create_handle = drm_gem_fb_create_handle, @@ -32,6 +35,32 @@ rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cm int ret; int i; + if (drm_is_afbc(mode_cmd->modifier[0])) { + struct drm_afbc afbc; + + drm_afbc_get_parameters(mode_cmd, &afbc); + + if (afbc.offset) { + DRM_WARN("AFBC plane offset must be zero!\n"); + + return ERR_PTR(-EINVAL); + } + + if (afbc.tile_w != 16 || afbc.tile_h != 16) { + DRM_WARN("Unsupported afbc tile w/h [%d/%d]\n", +afbc.tile_w, afbc.tile_h); + + return ERR_PTR(-EINVAL); + } + + if (afbc.width > ROCKCHIP_MAX_AFBC_WIDTH) { + DRM_WARN("Unsupported width %d>%d\n", +afbc.width, ROCKCHIP_MAX_AFBC_WIDTH); + + return ERR_PTR(-EINVAL); + } + } + fb = kzalloc(sizeof(*fb), GFP_KERNEL); if (!fb) return ERR_PTR(-ENOMEM); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index d04b3492bdac..31f72ba61361 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -91,9 +91,22 @@ #define VOP_WIN_TO_INDEX(vop_win) \ ((vop_win) - (vop_win)->vop->win) +#define VOP_AFBC_SET(vop, name, v) \ + do { \ + if ((vop)->data->afbc) \ + vop_reg_set((vop), &(vop)->data->afbc->name, \ + 0, ~0, v, #name); \ + } while (0) + #define to_vop(x) container_of(x, struct vop, crtc) #define to_vop_win(x) container_of(x, struct vop_win, base) +#define AFBC_FMT_RGB5650x0 +#define AFBC_FMT_U8U8U8U8 0x5 +#define AFBC_FMT_U8U8U80x4 + +#define AFBC_TILE_16x16BIT(4) + /* * The coefficients of the following matrix are all fixed points. * The format is S2.10 for the 3x3 part of the matrix, and S9.12 for the offsets. @@ -166,6 +179,7 @@ struct vop { /* optional internal rgb encoder */ struct rockchip_rgb *rgb; + struct vop_win *afbc_win; struct vop_win win[]; }; @@ -274,6 +288,29 @@ static enum vop_data_format vop_convert_format(uint32_t format) } } +static int vop_convert_afbc_format(uint32_t format) +{ + switch (format) { + case DRM_FORMAT_XRGB: + case DRM_FORMAT_ARGB: + case DRM_FORMAT_XBGR: + case DRM_FORMAT_ABGR: + return AFBC_FMT_U8U8U8U8; + case DRM_FORMAT_RGB888: + case DRM_FORMAT_BGR888: + return AFBC_FMT_U8U8U8; + case DRM_FORMAT_RGB565: + case DRM_FORMAT_BGR565: + return AFBC_FMT_RGB565; + /* either of the below should not be reachable */ + default: + DRM_WARN_ONCE("unsupported AFBC format[%08x]\n", format); + return -EINVAL; + } + + return -EINVAL; +} + static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src, uint32_t dst, bool is_horizontal, int vsu_mode, int *vskiplines) @@ -598,6 +635,15 @@ static int vop_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) vop_win_disable(vop, vop_win); } } + + if (vop->data->afbc) { + /* +* Disable AFBC and forget there was a vop window with AFBC +*/ + VOP_AFBC_SET(vop, enable, 0); + vop->afbc_win = NULL; + } + spin_unlock(&vop->reg_lock); vop_cfg_done(vop); @@ -710,6 +756,34 @@ static void vop_plane_destroy(struct drm_pl
[PATCHv3/RFC 2/4] drm/malidp: use afbc helpers
There are afbc helpers available. Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/arm/malidp_drv.c | 121 +-- 1 file changed, 20 insertions(+), 101 deletions(-) diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index 37d92a06318e..ff8364680676 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -35,8 +36,6 @@ #include "malidp_hw.h" #define MALIDP_CONF_VALID_TIMEOUT 250 -#define AFBC_HEADER_SIZE 16 -#define AFBC_SUPERBLK_ALIGNMENT128 static void malidp_write_gamma_table(struct malidp_hw_device *hwdev, u32 data[MALIDP_COEFFTAB_NUM_COEFFS]) @@ -269,112 +268,32 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = { .atomic_commit_tail = malidp_atomic_commit_tail, }; -static bool -malidp_verify_afbc_framebuffer_caps(struct drm_device *dev, - const struct drm_mode_fb_cmd2 *mode_cmd) -{ - if (malidp_format_mod_supported(dev, mode_cmd->pixel_format, - mode_cmd->modifier[0]) == false) - return false; - - if (mode_cmd->offsets[0] != 0) { - DRM_DEBUG_KMS("AFBC buffers' plane offset should be 0\n"); - return false; - } - - switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) { - case AFBC_SIZE_16X16: - if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) { - DRM_DEBUG_KMS("AFBC buffers must be aligned to 16 pixels\n"); - return false; - } - break; - default: - DRM_DEBUG_KMS("Unsupported AFBC block size\n"); - return false; - } - - return true; -} - -static bool -malidp_verify_afbc_framebuffer_size(struct drm_device *dev, - struct drm_file *file, - const struct drm_mode_fb_cmd2 *mode_cmd) +static struct drm_framebuffer * +malidp_fb_create(struct drm_device *dev, struct drm_file *file, +const struct drm_mode_fb_cmd2 *mode_cmd) { - int n_superblocks = 0; - const struct drm_format_info *info; - struct drm_gem_object *objs = NULL; - u32 afbc_superblock_size = 0, afbc_superblock_height = 0; - u32 afbc_superblock_width = 0, afbc_size = 0; - int bpp = 0; - - switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) { - case AFBC_SIZE_16X16: - afbc_superblock_height = 16; - afbc_superblock_width = 16; - break; - default: - DRM_DEBUG_KMS("AFBC superblock size is not supported\n"); - return false; - } - - info = drm_get_format_info(dev, mode_cmd); - - n_superblocks = (mode_cmd->width / afbc_superblock_width) * - (mode_cmd->height / afbc_superblock_height); - - bpp = malidp_format_get_bpp(info->format); - - afbc_superblock_size = (bpp * afbc_superblock_width * afbc_superblock_height) - / BITS_PER_BYTE; - - afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE, AFBC_SUPERBLK_ALIGNMENT); - afbc_size += n_superblocks * ALIGN(afbc_superblock_size, AFBC_SUPERBLK_ALIGNMENT); - - if ((mode_cmd->width * bpp) != (mode_cmd->pitches[0] * BITS_PER_BYTE)) { - DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) " - "should be same as width (=%u) * bpp (=%u)\n", - (mode_cmd->pitches[0] * BITS_PER_BYTE), - mode_cmd->width, bpp); - return false; - } - - objs = drm_gem_object_lookup(file, mode_cmd->handles[0]); - if (!objs) { - DRM_DEBUG_KMS("Failed to lookup GEM object\n"); - return false; - } + if (mode_cmd->modifier[0]) { + struct drm_afbc afbc; - if (objs->size < afbc_size) { - DRM_DEBUG_KMS("buffer size (%zu) too small for AFBC buffer size = %u\n", - objs->size, afbc_size); - drm_gem_object_put_unlocked(objs); - return false; - } + if (malidp_format_mod_supported(dev, mode_cmd->pixel_format, + mode_cmd->modifier[0]) == false) + return ERR_PTR(-EINVAL); - drm_gem_object_put_unlocked(objs); + drm_afbc_get_parameters(mode_cmd, &afbc); - return true; -} + if (afbc.tile_w != 16 || afbc.tile_h != 16) { + DRM_DEV_DEBUG(dev->dev, + "Unsupported AFBC block size %dx%d\n", + afbc.tile_w, afbc.tile_h);
[PATCHv3/RFC 1/4] drm/arm: Factor out generic afbc helpers
These are useful for other users of afbc, e.g. rockchip. Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_afbc.c| 84 +++ drivers/gpu/drm/drm_fourcc.c | 11 +++- drivers/gpu/drm/drm_framebuffer.c | 71 +- include/drm/drm_afbc.h| 35 + 5 files changed, 199 insertions(+), 4 deletions(-) create mode 100644 drivers/gpu/drm/drm_afbc.c create mode 100644 include/drm/drm_afbc.h diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index d9bcc9f2a0a4..3a58f30b83a6 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -44,7 +44,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o drm_probe_helper drm_simple_kms_helper.o drm_modeset_helper.o \ drm_scdc_helper.o drm_gem_framebuffer_helper.o \ drm_atomic_state_helper.o drm_damage_helper.o \ - drm_format_helper.o drm_self_refresh_helper.o + drm_format_helper.o drm_self_refresh_helper.o drm_afbc.o drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o diff --git a/drivers/gpu/drm/drm_afbc.c b/drivers/gpu/drm/drm_afbc.c new file mode 100644 index ..f308c4719546 --- /dev/null +++ b/drivers/gpu/drm/drm_afbc.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * (C) 2019 Collabora Ltd. + * + * author: Andrzej Pietrasiewicz + * + */ +#include + +#include +#include +#include +#include +#include +#include + +/** + * drm_afbc_get_superblk_wh - extract afbc block width/height from modifier + * @modifier: the modifier to be looked at + * @w: address of a place to store the block width + * @h: address of a place to store the block height + * + * Returns: true if the modifier describes a supported block size + */ +bool drm_afbc_get_superblk_wh(u64 modifier, u32 *w, u32 *h) +{ + switch (modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) { + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16: + *w = 16; + *h = 16; + break; + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8: + *w = 32; + *h = 8; + break; + case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4: + /* fall through */ + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4: + /* fall through */ + default: + DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n", + modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK); + return false; + } + return true; +} +EXPORT_SYMBOL_GPL(drm_afbc_get_superblk_wh); + +/** + * drm_afbc_get_parameters - extract afbc parameters from mode command + * @mode_cmd: mode command to be looked at + * @afbc: address of a struct to be filled in + */ +void drm_afbc_get_parameters(const struct drm_mode_fb_cmd2 *mode_cmd, +struct drm_afbc *afbc) +{ + drm_afbc_get_superblk_wh(mode_cmd->modifier[0], +&afbc->tile_w, &afbc->tile_h); + afbc->width = mode_cmd->pitches[0]; + afbc->height = + DIV_ROUND_UP(mode_cmd->height, afbc->tile_h) * afbc->tile_h; + afbc->offset = mode_cmd->offsets[0]; +} +EXPORT_SYMBOL(drm_afbc_get_parameters); + +/** + * drm_is_afbc - test if the modifier describes an afbc buffer + * @modifier - modifier to be tested + * + * Returns: true if the modifier describes an afbc buffer + */ +bool drm_is_afbc(u64 modifier) +{ + /* is it ARM AFBC? */ + if ((modifier & DRM_FORMAT_MOD_ARM_AFBC(0)) == 0) + return false; + + /* Block size must be known */ + if ((modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) == 0) + return false; + + return true; +} +EXPORT_SYMBOL_GPL(drm_is_afbc); diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index c630064ccf41..8d9f197cc0ab 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -322,8 +323,14 @@ drm_get_format_info(struct drm_device *dev, { const struct drm_format_info *info = NULL; - if (dev->mode_config.funcs->get_format_info) - info = dev->mode_config.funcs->get_format_info(mode_cmd); + /* bypass driver callback if afbc */ + if (!drm_is_afbc(mode_cmd->modifier[0])) + if (dev->mode_config.funcs->get_format_info) { + const struct drm_mode_config_funcs *funcs; + + funcs = dev->mode_config.funcs; + info = funcs->get_format_info(mode_cmd); + } if (!info) info = drm_format_info(mode_cmd->pixel_format); diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index 57564318ceea..303eea624a02
[PATCHv3/RFC 3/4] drm/komeda: Use afbc helper
AFBC helpers are available. Use those which increase code readability. Signed-off-by: Andrzej Pietrasiewicz --- .../drm/arm/display/komeda/komeda_framebuffer.c | 17 +++-- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c index 1b01a625f40e..f7edde3ac319 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c @@ -4,6 +4,7 @@ * Author: James.Qian.Wang * */ +#include #include #include #include @@ -52,20 +53,8 @@ komeda_fb_afbc_size_check(struct komeda_fb *kfb, struct drm_file *file, return -ENOENT; } - switch (fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) { - case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8: - alignment_w = 32; - alignment_h = 8; - break; - case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16: - alignment_w = 16; - alignment_h = 16; - break; - default: - WARN(1, "Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n", -fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK); - break; - } + if (!drm_afbc_get_superblk_wh(fb->modifier, &alignment_w, &alignment_h)) + return -EINVAL; /* tiled header afbc */ if (fb->modifier & AFBC_FORMAT_MOD_TILED) { -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amdgpu: remove redundant assignment to pointer write_frame
Applied. thanks! Alex On Thu, Nov 21, 2019 at 11:54 AM Colin King wrote: > > From: Colin Ian King > > The pointer write_frame is being initialized with a value that is > never read and it is being updated later with a new value. The > initialization is redundant and can be removed. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index 2a8a08aa6eaf..c02f9ffe5c6b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -1728,7 +1728,7 @@ int psp_ring_cmd_submit(struct psp_context *psp, > int index) > { > unsigned int psp_write_ptr_reg = 0; > - struct psp_gfx_rb_frame *write_frame = psp->km_ring.ring_mem; > + struct psp_gfx_rb_frame *write_frame; > struct psp_ring *ring = &psp->km_ring; > struct psp_gfx_rb_frame *ring_buffer_start = ring->ring_mem; > struct psp_gfx_rb_frame *ring_buffer_end = ring_buffer_start + > -- > 2.24.0 > > ___ > 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: linux-next: build failure after merge of the tip tree
On Wed, Nov 20, 2019 at 10:54 PM Stephen Rothwell wrote: > > Hi all, > > After merging the tip tree, today's linux-next build (x86_64 allmodconfig) > failed like this: > > In file included from include/trace/define_trace.h:102, > from drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:502, > from drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c:29: > include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:476:52: error: > expected expression before ';' token > 476 | __string(ring, sched_job->base.sched->name); > |^ > include/trace/trace_events.h:435:2: note: in definition of macro > 'DECLARE_EVENT_CLASS' > 435 | tstruct\ > | ^~~ > include/trace/trace_events.h:77:9: note: in expansion of macro 'PARAMS' >77 | PARAMS(tstruct), \ > | ^~ > include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:472:1: note: in > expansion of macro 'TRACE_EVENT' > 472 | TRACE_EVENT(amdgpu_ib_pipe_sync, > | ^~~ > include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:475:6: note: in > expansion of macro 'TP_STRUCT__entry' > 475 | TP_STRUCT__entry( > | ^~~~ > > Caused by commit > > 2c2fdb8bca29 ("drm/amdgpu: fix amdgpu trace event print string format > error") > > from the drm tree interacting with commit > > 60fdad00827c ("ftrace: Rework event_create_dir()") > > from the tip tree. > > I have added the following merge fix patch: Applied. Thanks! Alex > > From: Stephen Rothwell > Date: Thu, 21 Nov 2019 14:46:00 +1100 > Subject: [PATCH] merge fix for "ftrace: Rework event_create_dir()" > > Signed-off-by: Stephen Rothwell > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > index f940526c5889..63e734a125fb 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > @@ -473,7 +473,7 @@ TRACE_EVENT(amdgpu_ib_pipe_sync, > TP_PROTO(struct amdgpu_job *sched_job, struct dma_fence *fence), > TP_ARGS(sched_job, fence), > TP_STRUCT__entry( > -__string(ring, sched_job->base.sched->name); > +__string(ring, sched_job->base.sched->name) > __field(uint64_t, id) > __field(struct dma_fence *, fence) > __field(uint64_t, ctx) > -- > 2.23.0 > > -- > Cheers, > Stephen Rothwell > ___ > 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 v4 6/6] drm/komeda: Expose side_by_side by sysfs/config_id
On Thu, 21 Nov 2019 at 20:21, james qian wang (Arm Technology China) wrote: > > On Thu, Nov 21, 2019 at 10:49:26AM +0100, Daniel Vetter wrote: > > On Thu, Nov 21, 2019 at 07:12:55AM +, james qian wang (Arm Technology > > China) wrote: > > > There are some restrictions if HW works on side_by_side, expose it via > > > config_id to user. > > > > > > Signed-off-by: James Qian Wang (Arm Technology China) > > > > > > --- > > > drivers/gpu/drm/arm/display/include/malidp_product.h | 3 ++- > > > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 1 + > > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h > > > b/drivers/gpu/drm/arm/display/include/malidp_product.h > > > index 1053b11352eb..96e2e4016250 100644 > > > --- a/drivers/gpu/drm/arm/display/include/malidp_product.h > > > +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h > > > @@ -27,7 +27,8 @@ union komeda_config_id { > > > n_scalers:2, /* number of scalers per pipeline */ > > > n_layers:3, /* number of layers per pipeline */ > > > n_richs:3, /* number of rich layers per pipeline */ > > > - reserved_bits:6; > > > + side_by_side:1, /* if HW works on side_by_side mode */ > > > + reserved_bits:5; > > > }; > > > __u32 value; > > > }; > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > > index c3fa4835cb8d..4dd4699d4e3d 100644 > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > > @@ -83,6 +83,7 @@ config_id_show(struct device *dev, struct > > > device_attribute *attr, char *buf) > > > > Uh, this sysfs file here looks a lot like uapi for some compositor to > > decide what to do. Do you have the userspace for this? > > Yes, our HWC driver uses this config_id and product_id for identifying the > HW caps. > This seems like it should be done more in the kernel, why does userspace needs all that info, to make more informed decisions? How would drm_hwcomposer get the same result? I'd prefer we just remove the sysfs nodes from upstream unless we have an upstream user for them. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 04:43:24PM +0100, Rafael J. Wysocki wrote: > On Thu, Nov 21, 2019 at 1:52 PM Mika Westerberg > wrote: > > > > On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote: > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > > wrote: > > > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > > > Express Root Port" (name from lspci) and on those systems all of > > > > > > > that > > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 > > > > > > > one, > > > > > > > which also explains Mikas case that Thunderbolt stuff works as > > > > > > > devices > > > > > > > never get populated under this particular bridge controller, but > > > > > > > under > > > > > > > those "Root Port"s > > > > > > > > > > > > It always is a PCIe port, but its location within the SoC may > > > > > > matter. > > > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > > > still the same. > > > > > > > > > > > Also some custom AML-based power management is involved and that may > > > > > > be making specific assumptions on the configuration of the SoC and > > > > > > the > > > > > > GPU at the time of its invocation which unfortunately are not known > > > > > > to > > > > > > us. > > > > > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > > > that point, so it looks like that AML tries to access device memory > > > > > > on > > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > > accessible in PCI power states below D0. > > > > > > > > > > Or the PCI config space of the GPU when the parent root port is in > > > > > D3hot > > > > > (as it is the case here). Also then the GPU config space is not > > > > > accessible. > > > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > > > a suspend ordering violation? > > > > > > No. We put the GPU into D3hot first, > > OK > > Does this involve any AML, like a _PS3 under the GPU object? I don't see _PS3 (nor _PS0) for that object. If I read it right the GPU itself is not described in ACPI tables at all. > > > then the root port and then turn > > > off the power resource (which is attached to the root port) resulting > > > the topology entering D3cold. > > > > I don't see that happening in the AML though. > > Which AML do you mean, specifically? The _OFF method for the root > port's _PR3 power resource or something else? The root port's _OFF method for the power resource returned by its _PR3. > > Basically the difference is that when Windows 7 or Linux (the _REV==5 > > check) then we directly do link disable whereas in Windows 8+ we invoke > > LKDS() method that puts the link into L2/L3. None of the fields they > > access seem to touch the GPU itself. > > So that may be where the problem is. > > Putting the downstream component into PCI D[1-3] is expected to put > the link into L1, so I'm not sure how that plays with the later > attempt to put it into L2/L3 Ready. That should be fine. What I've seen the link goes into L1 when downstream component is put to D-state (not D0) and then it is put back to L0 when L2/3 ready is propagated. Eventually it goes into L2 or L3. > Also, L2/L3 Ready is expected to be transient, so finally power should > be removed somehow. There is GPIO for both power and PERST, I think the line here: \_SB.SGOV (0x01010004, Zero) is the one that removes power. > > LKDS() for the first PEG port looks like this: > > > >P0L2 = One > >Sleep (0x10) > >Local0 = Zero > >While (P0L2) > >{ > > If ((Local0 > 0x04)) > > { > > Break > > } > > > > Sleep (0x10) > > Local0++ > >} > > > > One thing that comes to mind is that the loop can end even if P0L2 is > > not cleared as it does only 5 iterations with 16 ms sleep between. Maybe > > Sleep() is implemented differently in Windows? I mean Linux may be > > "faster" here and return prematurely and if we leave the port into D0 > > this does not happen, or something. I'm just throwing out ideas :) > > But this actually works for the downstream component in D0, doesn't it? It does and that leaves the link in L0 so it could be that then the above AML works better or something. That reminds me, ASPM may have something to do with this as well. > Also, if the downstream component is in D0, the port actually should > stay in D0 too, so what would happen with the $subject patch applied? Parent port cannot be lower D-state than the child so I agree it should stay in D0 as well. However, it seems that what happens
Re: [PATCH] drm/shmem: Add docbook comments for drm_gem_shmem_object madvise fields
On Fri, Nov 1, 2019 at 4:37 PM Rob Herring wrote: > > Add missing docbook comments to madvise fields in struct > drm_gem_shmem_object which fixes these warnings: > > include/drm/drm_gem_shmem_helper.h:87: warning: Function parameter or member > 'madv' not described in 'drm_gem_shmem_object' > include/drm/drm_gem_shmem_helper.h:87: warning: Function parameter or member > 'madv_list' not described in 'drm_gem_shmem_object' > > Fixes: 17acb9f35ed7 ("drm/shmem: Add madvise state and purge helpers") > Reported-by: Sean Paul > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Rob Herring Kerneldoc for the functions you've added in 17acb9f35ed7 ("drm/shmem: Add madvise state and purge helpers") is also missing. Can you pls fix that too? Thanks, Daniel > --- > include/drm/drm_gem_shmem_helper.h | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/include/drm/drm_gem_shmem_helper.h > b/include/drm/drm_gem_shmem_helper.h > index e2e9947b4770..901eafb09209 100644 > --- a/include/drm/drm_gem_shmem_helper.h > +++ b/include/drm/drm_gem_shmem_helper.h > @@ -44,7 +44,20 @@ struct drm_gem_shmem_object { > */ > unsigned int pages_use_count; > > + /** > +* @madv: State for madvise > +* > +* 0 is active/inuse. > +* A negative value is the object is purged. > +* Positive values are driver specific and not used by the helpers. > +*/ > int madv; > + > + /** > +* @madv_list: List entry for madvise tracking > +* > +* Typically used by drivers to track purgeable objects > +*/ > struct list_head madv_list; > > /** > -- > 2.20.1 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 09/24] vfio, mm: fix get_user_pages_remote() and FOLL_LONGTERM
On Wed, 20 Nov 2019 23:13:39 -0800 John Hubbard wrote: > As it says in the updated comment in gup.c: current FOLL_LONGTERM > behavior is incompatible with FAULT_FLAG_ALLOW_RETRY because of the > FS DAX check requirement on vmas. > > However, the corresponding restriction in get_user_pages_remote() was > slightly stricter than is actually required: it forbade all > FOLL_LONGTERM callers, but we can actually allow FOLL_LONGTERM callers > that do not set the "locked" arg. > > Update the code and comments accordingly, and update the VFIO caller > to take advantage of this, fixing a bug as a result: the VFIO caller > is logically a FOLL_LONGTERM user. > > Also, remove an unnessary pair of calls that were releasing and > reacquiring the mmap_sem. There is no need to avoid holding mmap_sem > just in order to call page_to_pfn(). > > Also, move the DAX check ("if a VMA is DAX, don't allow long term > pinning") from the VFIO call site, all the way into the internals > of get_user_pages_remote() and __gup_longterm_locked(). That is: > get_user_pages_remote() calls __gup_longterm_locked(), which in turn > calls check_dax_vmas(). It's lightly explained in the comments as well. > > Thanks to Jason Gunthorpe for pointing out a clean way to fix this, > and to Dan Williams for helping clarify the DAX refactoring. > > Reviewed-by: Jason Gunthorpe > Reviewed-by: Ira Weiny > Suggested-by: Jason Gunthorpe > Cc: Dan Williams > Cc: Jerome Glisse > Signed-off-by: John Hubbard > --- > drivers/vfio/vfio_iommu_type1.c | 30 +- > mm/gup.c| 27 ++- > 2 files changed, 27 insertions(+), 30 deletions(-) Tested with device assignment and Intel mdev vGPU assignment with QEMU userspace: Tested-by: Alex Williamson Acked-by: Alex Williamson Feel free to include for 19/24 as well. Thanks, Alex > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index d864277ea16f..c7a111ad9975 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -340,7 +340,6 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned > long vaddr, > { > struct page *page[1]; > struct vm_area_struct *vma; > - struct vm_area_struct *vmas[1]; > unsigned int flags = 0; > int ret; > > @@ -348,33 +347,14 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned > long vaddr, > flags |= FOLL_WRITE; > > down_read(&mm->mmap_sem); > - if (mm == current->mm) { > - ret = get_user_pages(vaddr, 1, flags | FOLL_LONGTERM, page, > - vmas); > - } else { > - ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags, page, > - vmas, NULL); > - /* > - * The lifetime of a vaddr_get_pfn() page pin is > - * userspace-controlled. In the fs-dax case this could > - * lead to indefinite stalls in filesystem operations. > - * Disallow attempts to pin fs-dax pages via this > - * interface. > - */ > - if (ret > 0 && vma_is_fsdax(vmas[0])) { > - ret = -EOPNOTSUPP; > - put_page(page[0]); > - } > - } > - up_read(&mm->mmap_sem); > - > + ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags | FOLL_LONGTERM, > + page, NULL, NULL); > if (ret == 1) { > *pfn = page_to_pfn(page[0]); > - return 0; > + ret = 0; > + goto done; > } > > - down_read(&mm->mmap_sem); > - > vaddr = untagged_addr(vaddr); > > vma = find_vma_intersection(mm, vaddr, vaddr + 1); > @@ -384,7 +364,7 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned > long vaddr, > if (is_invalid_reserved_pfn(*pfn)) > ret = 0; > } > - > +done: > up_read(&mm->mmap_sem); > return ret; > } > diff --git a/mm/gup.c b/mm/gup.c > index 14fcdc502166..cce2c9676853 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -29,6 +29,13 @@ struct follow_page_context { > unsigned int page_mask; > }; > > +static __always_inline long __gup_longterm_locked(struct task_struct *tsk, > + struct mm_struct *mm, > + unsigned long start, > + unsigned long nr_pages, > + struct page **pages, > + struct vm_area_struct **vmas, > + unsigned int flags); > /* > * Return the compound head page with ref appropriately incremented, > * or NULL if that failed. > @@ -1167,13 +1174,23 @@ long get_user_pages_remote(struct task_struct *tsk, > struct mm_struct *mm, >