Re: [Letux-kernel] [PATCH v3 0/5] drm/panel-simple: Add panel parameters for ortustech-com37h3m05dtc/99dtc and sharp-lq070y3dg3b
Hi Sam, > Am 17.07.2019 um 20:14 schrieb Sam Ravnborg : > > Hi Nikolaus. > >> BTW: should also be applied to 5.2 > The drm bits are reviewed. The DT bits needs OK from DT people. > When we have OK from DT people we can apply them all to drm-misc-next. I got OK on irc from Rob to process these. All patches are now applied to drm-misc-next. >>> >>> Thanks for taking care of this! >> >> I have checked but it seems they are still not merged into linux-next. > > They will appear in next merge window. They were to late to hit current > merge window, as the cut-of time is around .rc5 in the drm subsystem. > And this is not really a fix so not stable material. have finally arrived. I just wasn't patient enough :) BR and thanks, Nikolaus
Re: [RFC] Expanding drm_mode_modeinfo flags
On Tue, Jul 23, 2019 at 1:50 AM Jeykumar Sankaran wrote: > > On 2019-07-19 07:29, Sean Paul wrote: > > On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote: > >> On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote: > >> > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote: > >> > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote: > >> > > > On 2019-07-16 02:07, Daniel Vetter wrote: > >> > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote: > > > > /snip > > > >> > > > > > drm: add mode flags in uapi for seamless mode switch > >> > > > > > >> > > > > I think the uapi is the trivial part here, the real deal is how > >> > > > > userspace > >> > > > > uses this. Can you pls post the patches for your compositor? > >> > > > > > >> > > > > Also note that we already allow userspace to tell the kernel > >> > > > > whether > >> > > > > flickering is ok or not for a modeset. msm driver could use that > >> > > > > to at > >> > > > > least tell userspace whether a modeset change is possible. So you > >> > > > > can > >> > > > > already implement glitch-free modeset changes for at least video > >> > > > > mode. > >> > > > > -Daniel > >> > > > > >> > > > I believe you are referring to the below tv property of the > >> > > > connector. > >> > > > > >> > > > /** > >> > > > * @tv_flicker_reduction_property: Optional TV property to control > >> > > > the > >> > > > * flicker reduction mode. > >> > > > */ > >> > > > struct drm_property *tv_flicker_reduction_property; > >> > > > >> > > Not even close :-) > >> > > > >> > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. > >> > > This > >> > > is not a property of a mode, this is a property of a _transition_ > >> > > between > >> > > configurations. Some transitions can be done flicker free, others > >> > > can't. > >> > > >> > Agree that an atomic flag on a commit is the way to accomplish this. > >> > It's pretty > >> > similar to the psr transitions, where we want to reuse most of the atomic > >> > circuitry, but in a specialized way. We'd also have to be careful to only > >> > involve the drm objects which are seamless modeset aware (you could > >> > imagine > >> > a bridge chain where the bridges downstream of the first bridge don't > >> > care). > >> > > >> > > > >> > > There's then still the question of how to pick video vs command mode, > >> > > but > >> > > imo better to start with implementing the transition behaviour > >> > > correctly > >> > > first. > >> > > >> > Connector property? Possibly a terrible idea, but I wonder if we could > >> > [re]use > >> > the vrr properties for command mode. The docs state that the driver has > >> > the > >> > option of putting upper and lower bounds on the refresh rate. > >> > >> Not really sure why this needs new props and whatnot. This is kinda > >> what > >> the i915 "fastset" stuff already does: > >> 1. userspace asks for something to be changed via atomic > >> 2. driver calculates whether a modeset is actually required > >> 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET > >> 4. if (need_modeset) heavyweight_commit() else lightweight_commit() > >> > >> Ie. why should userspace really care about anything except the > >> "flickers are OK" vs. "flickers not wanted" thing? > > > > Agree, I don't think the seamless modeset (ie: changing resolution > > without > > flicker) needs a property. Just need to test the commit without > > ALLOW_MODESET > > and commit it if the test passes. > > > > Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to > check > whether the modeset can be done seamless. But since there are no error > code returns, > the client cannot distinguish the test_only commit failures from other > invalid config failures. > > Also, note that when the client sees two 1080p modes (vid/cmd) and it is > interested only > to do *only* seamless switches, without any additional flag it cannot > distinguish between > these two 1080p modes. The client has to invoke two test_only commits > with these > modes to identify the seamless one. Is that a preferred approach? > > Intel's "fastset" calculates the need for modeset internally based on > the > configuration and chooses the best commit path. But the requirement here > is to expose the information up-front since the use case cannot afford > to fall back to the normal modeset when it has requested for a seamless > one. > > >> > >> Also what's the benefit of using video mode if your panel supportes > >> command mode? Can you turn off the memory in the panel and actually > >> save power that way? And if there is a benefit can't the driver just > >> automagically switch between the two based on how often things are > >> getting updated? That would match how eDP PSR already works. > > > > I'm guessing video mode might have some latency benefits over command > > mode? > > > > Sean > > Yes. Pretty much those are reasons we need to switch to
Re: [PATCH 1/3] mm/gup: introduce __put_user_pages()
On 7/22/19 10:53 PM, Christoph Hellwig wrote: On Mon, Jul 22, 2019 at 03:34:13PM -0700, john.hubb...@gmail.com wrote: +enum pup_flags_t { + PUP_FLAGS_CLEAN = 0, + PUP_FLAGS_DIRTY = 1, + PUP_FLAGS_LOCK = 2, + PUP_FLAGS_DIRTY_LOCK= 3, +}; Well, the enum defeats the ease of just being able to pass a boolean expression to the function, which would simplify a lot of the caller, so if we need to support the !locked version I'd rather see that as a separate helper. But do we actually have callers where not using the _lock version is not a bug? set_page_dirty makes sense in the context of a file systems that have a reference to the inode the page hangs off, but that is (almost?) never the case for get_user_pages. I'm seeing about 18 places where set_page_dirty() is used, in the call site conversions so far, and about 20 places where set_page_dirty_lock() is used. So without knowing how many of the former (if any) represent bugs, you can see why the proposal here supports both DIRTY and DIRTY_LOCK. Anyway, yes, I could change it, based on your estimation that most of the set_page_dirty() calls really should be set_page_dirty_lock(). In that case, we would end up with approximately the following: /* Here, "dirty" really means, "call set_page_dirty_lock()": */ void __put_user_pages(struct page **pages, unsigned long npages, bool dirty); /* Here, "dirty" really means, "call set_page_dirty()": */ void __put_user_pages_unlocked(struct page **pages, unsigned long npages, bool dirty); ? thanks, -- John Hubbard NVIDIA
dri-devel@lists.freedesktop.org
pon., 22 lip 2019 o 18:09 Andy Shevchenko napisał(a): > > On Mon, Jul 22, 2019 at 05:03:02PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski > > > > Instead of dereferencing pdev each time, use a helper variable for > > the associated device pointer. > > > static int gpio_backlight_probe(struct platform_device *pdev) > > { > > - struct gpio_backlight_platform_data *pdata = > > - dev_get_platdata(&pdev->dev); > > + struct gpio_backlight_platform_data *pdata; > > struct backlight_properties props; > > struct backlight_device *bl; > > struct gpio_backlight *gbl; > > enum gpiod_flags flags; > > + struct device *dev; > > Can't we do > > struct device dev = &pdev->dev; > struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev); > > ? It fits 80 nicely. > IMO it's more readable like that with the reverse christmas tree layout. Bart
Re: [PATCH v2 2/7] backlight: gpio: simplify the platform data handling
pon., 22 lip 2019 o 18:06 Andy Shevchenko napisał(a): > > On Mon, Jul 22, 2019 at 05:02:57PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski > > > > Now that the last user of platform data (sh ecovec24) defines a proper > > GPIO lookup and sets the 'default-on' device property, we can drop the > > platform_data-specific GPIO handling and unify a big chunk of code. > > > > The only field used from the platform data is now the fbdev pointer. > > > -static int gpio_backlight_probe_dt(struct platform_device *pdev, > > -struct gpio_backlight *gbl) > > -{ > > - struct device *dev = &pdev->dev; > > - enum gpiod_flags flags; > > - int ret; > > - > > - gbl->def_value = device_property_read_bool(dev, "default-on"); > > - flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW; > > - > > - gbl->gpiod = devm_gpiod_get(dev, NULL, flags); > > - if (IS_ERR(gbl->gpiod)) { > > - ret = PTR_ERR(gbl->gpiod); > > - > > - if (ret != -EPROBE_DEFER) { > > - dev_err(dev, > > - "Error: The gpios parameter is missing or > > invalid.\n"); > > - } > > - return ret; > > - } > > - > > - return 0; > > -} > > Why not leave this function (perhaps with different name)? > > -- > With Best Regards, > Andy Shevchenko > > Why would we do that if the entire probe() function is now less than 50 lines long? Also: it gets inlined by the compiler anyway. It doesn't make sense IMO. Bart
[PATCH V2 08/10] video: pxafb: Remove cpufreq policy notifier
The cpufreq policy notifier's CPUFREQ_ADJUST notification is going to get removed soon. The notifier callback pxafb_freq_policy() isn't doing anything apart from printing a debug message on CPUFREQ_ADJUST notification. There is no point in keeping an otherwise empty callback and registering the notifier. Remove it. Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: Viresh Kumar --- drivers/video/fbdev/pxafb.c | 21 - drivers/video/fbdev/pxafb.h | 1 - 2 files changed, 22 deletions(-) diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c index 4282cb117b92..f70c9f79622e 100644 --- a/drivers/video/fbdev/pxafb.c +++ b/drivers/video/fbdev/pxafb.c @@ -1678,24 +1678,6 @@ pxafb_freq_transition(struct notifier_block *nb, unsigned long val, void *data) } return 0; } - -static int -pxafb_freq_policy(struct notifier_block *nb, unsigned long val, void *data) -{ - struct pxafb_info *fbi = TO_INF(nb, freq_policy); - struct fb_var_screeninfo *var = &fbi->fb.var; - struct cpufreq_policy *policy = data; - - switch (val) { - case CPUFREQ_ADJUST: - pr_debug("min dma period: %d ps, " - "new clock %d kHz\n", pxafb_display_dma_period(var), - policy->max); - /* TODO: fill in min/max values */ - break; - } - return 0; -} #endif #ifdef CONFIG_PM @@ -2400,11 +2382,8 @@ static int pxafb_probe(struct platform_device *dev) #ifdef CONFIG_CPU_FREQ fbi->freq_transition.notifier_call = pxafb_freq_transition; - fbi->freq_policy.notifier_call = pxafb_freq_policy; cpufreq_register_notifier(&fbi->freq_transition, CPUFREQ_TRANSITION_NOTIFIER); - cpufreq_register_notifier(&fbi->freq_policy, - CPUFREQ_POLICY_NOTIFIER); #endif /* diff --git a/drivers/video/fbdev/pxafb.h b/drivers/video/fbdev/pxafb.h index b641289c8a99..86b1e9ab1a38 100644 --- a/drivers/video/fbdev/pxafb.h +++ b/drivers/video/fbdev/pxafb.h @@ -162,7 +162,6 @@ struct pxafb_info { #ifdef CONFIG_CPU_FREQ struct notifier_block freq_transition; - struct notifier_block freq_policy; #endif struct regulator *lcd_supply; -- 2.21.0.rc0.269.g1a574e7a288b
[PATCH V2 07/10] video: sa1100fb: Remove cpufreq policy notifier
The cpufreq policy notifier's CPUFREQ_ADJUST notification is going to get removed soon. The notifier callback sa1100fb_freq_policy() isn't doing anything apart from printing a debug message on CPUFREQ_ADJUST notification. There is no point in keeping an otherwise empty callback and registering the notifier. Remove it. Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: Viresh Kumar --- drivers/video/fbdev/sa1100fb.c | 27 --- drivers/video/fbdev/sa1100fb.h | 1 - 2 files changed, 28 deletions(-) diff --git a/drivers/video/fbdev/sa1100fb.c b/drivers/video/fbdev/sa1100fb.c index f7f8dee044b1..ae2bcfee338a 100644 --- a/drivers/video/fbdev/sa1100fb.c +++ b/drivers/video/fbdev/sa1100fb.c @@ -1005,31 +1005,6 @@ sa1100fb_freq_transition(struct notifier_block *nb, unsigned long val, } return 0; } - -static int -sa1100fb_freq_policy(struct notifier_block *nb, unsigned long val, -void *data) -{ - struct sa1100fb_info *fbi = TO_INF(nb, freq_policy); - struct cpufreq_policy *policy = data; - - switch (val) { - case CPUFREQ_ADJUST: - dev_dbg(fbi->dev, "min dma period: %d ps, " - "new clock %d kHz\n", sa1100fb_min_dma_period(fbi), - policy->max); - /* todo: fill in min/max values */ - break; - case CPUFREQ_NOTIFY: - do {} while(0); - /* todo: panic if min/max values aren't fulfilled -* [can't really happen unless there's a bug in the -* CPU policy verififcation process * -*/ - break; - } - return 0; -} #endif #ifdef CONFIG_PM @@ -1242,9 +1217,7 @@ static int sa1100fb_probe(struct platform_device *pdev) #ifdef CONFIG_CPU_FREQ fbi->freq_transition.notifier_call = sa1100fb_freq_transition; - fbi->freq_policy.notifier_call = sa1100fb_freq_policy; cpufreq_register_notifier(&fbi->freq_transition, CPUFREQ_TRANSITION_NOTIFIER); - cpufreq_register_notifier(&fbi->freq_policy, CPUFREQ_POLICY_NOTIFIER); #endif /* This driver cannot be unloaded at the moment */ diff --git a/drivers/video/fbdev/sa1100fb.h b/drivers/video/fbdev/sa1100fb.h index 7a1a9ca33cec..d0aa33b0b88a 100644 --- a/drivers/video/fbdev/sa1100fb.h +++ b/drivers/video/fbdev/sa1100fb.h @@ -64,7 +64,6 @@ struct sa1100fb_info { #ifdef CONFIG_CPU_FREQ struct notifier_block freq_transition; - struct notifier_block freq_policy; #endif const struct sa1100fb_mach_info *inf; -- 2.21.0.rc0.269.g1a574e7a288b
[PATCH V2 00/10] cpufreq: Migrate users of policy notifiers to QoS requests
Hello, Now that cpufreq core supports taking QoS requests for min/max cpu frequencies, lets migrate rest of the users to using them instead of the policy notifiers. The CPUFREQ_NOTIFY and CPUFREQ_ADJUST events of the policy notifiers are removed as a result, but we have to add CPUFREQ_CREATE_POLICY and CPUFREQ_REMOVE_POLICY events to it for the acpi stuff specifically, though they are also used by arch_topology stuff now. So the policy notifiers aren't completely removed. Boot tested on my x86 PC and ARM hikey board. This has already gone through build bot for a few days now. V1->V2: - Added Acked-by tags - Reordered to keep cleanups at the bottom - Rebased over 5.3-rc1 -- viresh Viresh Kumar (10): cpufreq: Add policy create/remove notifiers thermal: cpu_cooling: Switch to QoS requests instead of cpufreq notifier powerpc: macintosh: Switch to QoS requests instead of cpufreq notifier cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq notifier ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier arch_topology: Use CPUFREQ_CREATE_POLICY instead of CPUFREQ_NOTIFY video: sa1100fb: Remove cpufreq policy notifier video: pxafb: Remove cpufreq policy notifier cpufreq: Remove CPUFREQ_ADJUST and CPUFREQ_NOTIFY policy notifier events Documentation: cpufreq: Update policy notifier documentation Documentation/cpu-freq/core.txt| 16 +-- drivers/acpi/processor_driver.c| 44 - drivers/acpi/processor_perflib.c | 106 +--- drivers/acpi/processor_thermal.c | 81 --- drivers/base/arch_topology.c | 2 +- drivers/cpufreq/cpufreq.c | 51 -- drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++- drivers/cpufreq/ppc_cbe_cpufreq.h | 8 ++ drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++--- drivers/macintosh/windfarm_cpufreq_clamp.c | 77 ++- drivers/thermal/cpu_cooling.c | 110 + drivers/video/fbdev/pxafb.c| 21 drivers/video/fbdev/pxafb.h| 1 - drivers/video/fbdev/sa1100fb.c | 27 - drivers/video/fbdev/sa1100fb.h | 1 - include/acpi/processor.h | 22 +++-- include/linux/cpufreq.h| 4 +- 17 files changed, 327 insertions(+), 359 deletions(-) -- 2.21.0.rc0.269.g1a574e7a288b
Re: [PATCH v4 13/23] drm: zte: Provide ddc symlink in vga connector sysfs directory
On Thu, Jul 11, 2019 at 01:26:40PM +0200, Andrzej Pietrasiewicz wrote: > Use the ddc pointer provided by the generic connector. > > Signed-off-by: Andrzej Pietrasiewicz Acked-by: Shawn Guo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 12/23] drm: zte: Provide ddc symlink in hdmi connector sysfs directory
On Thu, Jul 11, 2019 at 01:26:39PM +0200, Andrzej Pietrasiewicz wrote: > Use the ddc pointer provided by the generic connector. > > Signed-off-by: Andrzej Pietrasiewicz Acked-by: Shawn Guo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110749] [Vega 11] [amdgpu retry page fault VM_L2_PROTECTION_FAULT_STATUS] System lock up during playing Steam version of Saints Row 3
https://bugs.freedesktop.org/show_bug.cgi?id=110749 --- Comment #5 from weden...@yandex.ru --- Seems like another case of https://bugs.freedesktop.org/show_bug.cgi?id=105251 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105718] amdgpu reported fan speed looks too high (dual fan Sapphire Pulse Vega 56)
https://bugs.freedesktop.org/show_bug.cgi?id=105718 --- Comment #3 from weden...@yandex.ru --- I have exactly the same issue with Sapphire Pulse Vega 56. It also reports some unreasonably high value (something around 3000rpm) as max fans RPM. I've found this thread https://www.hwinfo.com/forum/Thread-Solved-SAPPHIRE-PULSE-Radeon%E2%84%A2-RX-Vega56-8G-HBM2-Wrong-FAN-RPM. Apparently this issue was solved in windows with AMD driver update. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] mm/gup: introduce __put_user_pages()
On Mon, Jul 22, 2019 at 03:34:13PM -0700, john.hubb...@gmail.com wrote: > +enum pup_flags_t { > + PUP_FLAGS_CLEAN = 0, > + PUP_FLAGS_DIRTY = 1, > + PUP_FLAGS_LOCK = 2, > + PUP_FLAGS_DIRTY_LOCK= 3, > +}; Well, the enum defeats the ease of just being able to pass a boolean expression to the function, which would simplify a lot of the caller, so if we need to support the !locked version I'd rather see that as a separate helper. But do we actually have callers where not using the _lock version is not a bug? set_page_dirty makes sense in the context of a file systems that have a reference to the inode the page hangs off, but that is (almost?) never the case for get_user_pages.
[PATCH] drm/mediatek: make imported PRIME buffers contiguous
This driver requires imported PRIME buffers to appear contiguously in its IO address space. Make sure this is the case by setting the maximum DMA segment size to a better value than the default 64K on the DMA device, and use said DMA device when importing PRIME buffers. Signed-off-by: Alexandre Courbot --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 47 -- drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 ++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 95fdbd0fbcac..4ad4770fab13 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -213,6 +213,7 @@ static int mtk_drm_kms_init(struct drm_device *drm) struct mtk_drm_private *private = drm->dev_private; struct platform_device *pdev; struct device_node *np; + struct device *dma_dev; int ret; if (!iommu_present(&platform_bus_type)) @@ -275,7 +276,27 @@ static int mtk_drm_kms_init(struct drm_device *drm) goto err_component_unbind; } - private->dma_dev = &pdev->dev; + dma_dev = &pdev->dev; + private->dma_dev = dma_dev; + + /* +* Configure the DMA segment size to make sure we get contiguous IOVA +* when importing PRIME buffers. +*/ + if (!dma_dev->dma_parms) { + private->dma_parms_allocated = true; + dma_dev->dma_parms = + devm_kzalloc(drm->dev, sizeof(*dma_dev->dma_parms), +GFP_KERNEL); + } + if (!dma_dev->dma_parms) + goto err_component_unbind; + + ret = dma_set_max_seg_size(dma_dev, (unsigned int)DMA_BIT_MASK(32)); + if (ret) { + dev_err(dma_dev, "Failed to set DMA segment size\n"); + goto err_unset_dma_parms; + } /* * We don't use the drm_irq_install() helpers provided by the DRM @@ -285,13 +306,16 @@ static int mtk_drm_kms_init(struct drm_device *drm) drm->irq_enabled = true; ret = drm_vblank_init(drm, MAX_CRTC); if (ret < 0) - goto err_component_unbind; + goto err_unset_dma_parms; drm_kms_helper_poll_init(drm); drm_mode_config_reset(drm); return 0; +err_unset_dma_parms: + if (private->dma_parms_allocated) + dma_dev->dma_parms = NULL; err_component_unbind: component_unbind_all(drm->dev, drm); err_config_cleanup: @@ -302,9 +326,14 @@ static int mtk_drm_kms_init(struct drm_device *drm) static void mtk_drm_kms_deinit(struct drm_device *drm) { + struct mtk_drm_private *private = drm->dev_private; + drm_kms_helper_poll_fini(drm); drm_atomic_helper_shutdown(drm); + if (private->dma_parms_allocated) + private->dma_dev->dma_parms = NULL; + component_unbind_all(drm->dev, drm); drm_mode_config_cleanup(drm); } @@ -320,6 +349,18 @@ static const struct file_operations mtk_drm_fops = { .compat_ioctl = drm_compat_ioctl, }; +/* + * We need to override this because the device used to import the memory is + * not dev->dev, as drm_gem_prime_import() expects. + */ +struct drm_gem_object *mtk_drm_gem_prime_import(struct drm_device *dev, + struct dma_buf *dma_buf) +{ + struct mtk_drm_private *private = dev->dev_private; + + return drm_gem_prime_import_dev(dev, dma_buf, private->dma_dev); +} + static struct drm_driver mtk_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC, @@ -331,7 +372,7 @@ static struct drm_driver mtk_drm_driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, .gem_prime_export = drm_gem_prime_export, - .gem_prime_import = drm_gem_prime_import, + .gem_prime_import = mtk_drm_gem_prime_import, .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table, .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table, .gem_prime_mmap = mtk_drm_gem_mmap_buf, diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h index 598ff3e70446..e03fea12ff59 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h @@ -51,6 +51,8 @@ struct mtk_drm_private { } commit; struct drm_atomic_state *suspend_state; + + bool dma_parms_allocated; }; extern struct platform_driver mtk_ddp_driver; -- 2.22.0.657.g960e92d24f-goog
[PATCH] drm/syncobj: extend syncobj query ability
user space needs a flexiable query ability. So that umd can get last signaled or submitted point. Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea Signed-off-by: Chunming Zhou Cc: Lionel Landwerlin Cc: Christian König --- drivers/gpu/drm/drm_syncobj.c | 36 +-- include/uapi/drm/drm.h| 3 ++- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 3d400905100b..f70dedf3ef4f 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1197,9 +1197,6 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE)) return -EOPNOTSUPP; - if (args->pad != 0) - return -EINVAL; - if (args->count_handles == 0) return -EINVAL; @@ -1268,9 +1265,6 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE)) return -EOPNOTSUPP; - if (args->pad != 0) - return -EINVAL; - if (args->count_handles == 0) return -EINVAL; @@ -1291,23 +1285,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, if (chain) { struct dma_fence *iter, *last_signaled = NULL; - dma_fence_chain_for_each(iter, fence) { - if (!iter) - break; - dma_fence_put(last_signaled); - last_signaled = dma_fence_get(iter); - if (!to_dma_fence_chain(last_signaled)->prev_seqno) - /* It is most likely that timeline has -* unorder points. */ - break; + if (args->flags & + DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) { + point = fence->seqno; + } else { + dma_fence_chain_for_each(iter, fence) { + if (!iter) + break; + dma_fence_put(last_signaled); + last_signaled = dma_fence_get(iter); + if (!to_dma_fence_chain(last_signaled)->prev_seqno) + /* It is most likely that timeline has + * unorder points. */ + break; + } + point = dma_fence_is_signaled(last_signaled) ? + last_signaled->seqno : + to_dma_fence_chain(last_signaled)->prev_seqno; } - point = dma_fence_is_signaled(last_signaled) ? - last_signaled->seqno : - to_dma_fence_chain(last_signaled)->prev_seqno; dma_fence_put(last_signaled); } else { point = 0; } + dma_fence_put(fence); ret = copy_to_user(&points[i], &point, sizeof(uint64_t)); ret = ret ? -EFAULT : 0; if (ret) diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 661d73f9a919..fd987ce24d9f 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -777,11 +777,12 @@ struct drm_syncobj_array { __u32 pad; }; +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */ struct drm_syncobj_timeline_array { __u64 handles; __u64 points; __u32 count_handles; - __u32 pad; + __u32 flags; }; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 4/5] dma-buf: heaps: Add CMA heap to dmabuf heaps
On Thu, Jul 18, 2019 at 3:08 AM Christoph Hellwig wrote: > > This and the previous one seem very much duplicated boilerplate > code. So yes, there is some duplicate boilerplate between the system and cma heaps particularly in the allocation function, where we allocate and set up the helper buffer structure, allocate the pages, then create and export the dmabuf. I took a pass at trying to minimize some of this earlier, but those efforts ended up adding helper functions that take a ton of arguments, and had some trouble properly handling error paths without leaking memory, so I left it as is. I'll try to take another pass myself but if you have specific suggestions for improving here, I'd be happy to try them. > Why can't just normal branches for allocating and freeing > normal pages vs cma? We even have an existing helper for that > with dma_alloc_contiguous(). Apologies, I'm not sure I'm understanding your suggestion here. dma_alloc_contiguous() does have some interesting optimizations (avoiding allocating single page from cma), though its focus on default area vs specific device area doesn't quite match up the usage model for dma heaps. Instead of allocating memory for a single device, we want to be able to allow userland, for a given usage mode, to be able to allocate a dmabuf from a specific heap of memory which will satisfy the usage mode for that buffer pipeline (across multiple devices). Now, indeed, the system and cma heaps in this patchset are a bit simple/trivial (though useful with my devices that require contiguous buffers for the display driver), but more complex ION heaps have traditionally stayed out of tree in vendor code, in many cases making incompatible tweaks to the ION core dmabuf exporter logic. That's why dmabuf heaps is trying to destage ION in a way that allows heaps to implement their exporter logic themselves, so we can start pulling those more complicated heaps out of their vendor hidey-holes and get some proper upstream review. But I suspect I just am confused as to what your suggesting. Maybe could you expand a bit? Apologies for being a bit dense. Thanks again for the input! -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] net/xdp: convert put_page() to put_user_page*()
On 7/22/19 5:25 PM, Ira Weiny wrote: On Mon, Jul 22, 2019 at 03:34:15PM -0700, john.hubb...@gmail.com wrote: From: John Hubbard For pages that were retained via get_user_pages*(), release those pages via the new put_user_page*() routines, instead of via put_page() or release_pages(). This is part a tree-wide conversion, as described in commit fc1d8e7cca2d ("mm: introduce put_user_page*(), placeholder versions"). Cc: Björn Töpel Cc: Magnus Karlsson Cc: David S. Miller Cc: net...@vger.kernel.org Signed-off-by: John Hubbard --- net/xdp/xdp_umem.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c index 83de74ca729a..0325a17915de 100644 --- a/net/xdp/xdp_umem.c +++ b/net/xdp/xdp_umem.c @@ -166,14 +166,7 @@ void xdp_umem_clear_dev(struct xdp_umem *umem) static void xdp_umem_unpin_pages(struct xdp_umem *umem) { - unsigned int i; - - for (i = 0; i < umem->npgs; i++) { - struct page *page = umem->pgs[i]; - - set_page_dirty_lock(page); - put_page(page); - } + put_user_pages_dirty_lock(umem->pgs, umem->npgs); What is the difference between this and __put_user_pages(umem->pgs, umem->npgs, PUP_FLAGS_DIRTY_LOCK); ? No difference. I'm a bit concerned with adding another form of the same interface. We should either have 1 call with flags (enum in this case) or multiple calls. Given the previous discussion lets move in the direction of having the enum but don't introduce another caller of the "old" interface. I disagree that this is a "problem". There is no maintenance pitfall here; there are merely two ways to call the put_user_page*() API. Both are correct, and neither one will get you into trouble. Not only that, but there is ample precedent for this approach in other kernel APIs. So I think on this patch NAK from me. I also don't like having a __* call in the exported interface but there is a __get_user_pages_fast() call so I guess there is precedent. :-/ I thought about this carefully, and looked at other APIs. And I noticed that things like __get_user_pages*() are how it's often done: * The leading underscores are often used for the more elaborate form of the call (as oppposed to decorating the core function name with "_flags", for example). * There are often calls in which you can either call the simpler form, or the form with flags and additional options, and yes, you'll get the same result. Obviously, this stuff is all subject to a certain amount of opinion, but I think I'm on really solid ground as far as precedent goes. So I'm pushing back on the NAK... :) thanks, -- John Hubbard NVIDIA
Re: [PATCH v6 2/5] dma-buf: heaps: Add heap helpers
On Thu, Jul 18, 2019 at 3:06 AM Christoph Hellwig wrote: > > > +void INIT_HEAP_HELPER_BUFFER(struct heap_helper_buffer *buffer, > > + void (*free)(struct heap_helper_buffer *)) > > Please use a lower case naming following the naming scheme for the > rest of the file. Yes! Apologies as this was a hold-over from when the initialization function was an inline function in the style of INIT_WORK/INIT_LIST_HEAD. No longer appropriate that its a function. I'll change it. > > +static void *dma_heap_map_kernel(struct heap_helper_buffer *buffer) > > +{ > > + void *vaddr; > > + > > + vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL); > > + if (!vaddr) > > + return ERR_PTR(-ENOMEM); > > + > > + return vaddr; > > +} > > Unless I'm misreading the patches this is used for the same pages that > also might be dma mapped. In this case you need to use > flush_kernel_vmap_range and invalidate_kernel_vmap_range in the right > places to ensure coherency between the vmap and device view. Please > also document the buffer ownership, as this really can get complicated. Forgive me I wasn't familiar with those calls, but this seems like it would apply to all dma-buf exporters if so, and I don't see any similar flush_kernel_vmap_range calls there (both functions are seemingly only used by xfs, md and bio). We do have the dma_heap_dma_buf_begin_cpu_access()/dma_heap_dma_buf_end_cpu_access() hooks (see more on these below) which sync the buffers for each attachment (via dma_sync_sg_for_cpu/device), and are used around cpu access to the buffers. Are you suggesting the flush_kernel_vmap_range() call be added to those hooks or is the dma_sync_sg_for_* calls sufficient there? > > +static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf) > > +{ > > + struct vm_area_struct *vma = vmf->vma; > > + struct heap_helper_buffer *buffer = vma->vm_private_data; > > + > > + vmf->page = buffer->pages[vmf->pgoff]; > > + get_page(vmf->page); > > + > > + return 0; > > +} > > Is there any exlusion between mmap / vmap and the device accessing > the data? Without that you are going to run into a lot of coherency > problems. This has actually been a concern of mine recently, but at the higher dma-buf core level. Conceptually, there is the dma_buf_map_attachment() and dma_buf_unmap_attachment() calls drivers use to map the buffer to an attached device, and there are the dma_buf_begin_cpu_access() and dma_buf_end_cpu_access() calls which are to be done when touching the cpu mapped pages. These look like serializing functions, but actually don't seem to have any enforcement mechanism to exclude parallel access. To me it seems like adding the exclusion between those operations should be done at the dmabuf core level, and would actually be helpful for optimizing some of the cache maintenance rules w/ dmabuf. Does this sound like something closer to what your suggesting, or am I misunderstanding your point? Again, I really appreciate the review and feedback! Thanks so much! -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: manual merge of the drm-intel tree with the kspp-gustavo tree
Hi all, Today's linux-next merge of the drm-intel tree got a conflict in: drivers/gpu/drm/i915/display/intel_dp.c between commit: b6ac32eac063 ("drm/i915: Mark expected switch fall-throughs") from the kspp-gustavo tree and commit: bc85328ff431 ("drm/i915: Move the TypeC port handling code to a separate file") 4f36afb26cbe ("drm/i915: Sanitize the TypeC FIA lane configuration decoding") from the drm-intel tree. I fixed it up (bc85328ff431 moved the function updated by b6ac32eac063 and 4f36afb26cbe added an equivalt fixup) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell pgpJkTNeA90lm.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
On Mon, Jul 22, 2019 at 4:54 PM Stephen Boyd wrote: > > Quoting Brendan Higgins (2019-07-22 15:30:49) > > On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd wrote: > > > > > > > > > What's the calling context of the assertions and expectations? I still > > > don't like the fact that string stream needs to allocate buffers and > > > throw them into a list somewhere because the calling context matters > > > there. > > > > The calling context is the same as before, which is anywhere. > > Ok. That's concerning then. Yeah. Luis suggested just not supporting the IRQ context until later. See my later comment. > > > I'd prefer we just wrote directly to the console/log via printk > > > instead. That way things are simple because we use the existing > > > buffering path of printk, but maybe there's some benefit to the string > > > stream that I don't see? Right now it looks like it builds a string and > > > then dumps it to printk so I'm sort of lost what the benefit is over > > > just writing directly with printk. > > > > It's just buffering it so the whole string gets printed uninterrupted. > > If we were to print out piecemeal to printk, couldn't we have another > > call to printk come in causing it to garble the KUnit message we are > > in the middle of printing? > > Yes, printing piecemeal by calling printk many times could lead to > interleaving of messages if something else comes in such as an interrupt > printing something. Printk has some support to hold "records" but I'm > not sure how that would work here because KERN_CONT talks about only > being used early on in boot code. I haven't looked at printk in detail > though so maybe I'm all wrong and KERN_CONT just works? It says KERN_CONT is not SMP safe, and it isn't supposed to contain newlines, so it doesn't sound like it does any buffering for you. I looked at it a while ago and those comments agreed with my understanding of the code, but I could be wrong. > Can printk be called once with whatever is in the struct? Unfortunately, no. That is part of what I was trying to illustrate with this patch. Most of the messages are deterministic, but hardcoding all the possible message types would lead to a massive number of hard coded strings. However, even this would break down for the mocking formatters. All the different ways a function can be called are just too complex to encode into a finite set of hard coded fmt strings. > Otherwise if > this is about making printk into a structured log then maybe printk > isn't the proper solution anyway. Maybe a dev interface should be used > instead that can handle starting and stopping tests (via ioctl) in > addition to reading test results, records, etc. with read() and a > clearing of the records. Then the seqfile API works naturally. All of Ehhh...I wouldn't mind providing such an interface, but I would really like to be able to provide the results without having to depend on a userland doing something to get test results. That has always been a pretty important goal for me. > this is a bit premature, but it looks like you're going down the path of > making something akin to ftrace that stores binary formatted > assertion/expectation records in a lockless ring buffer that then > formats those records when the user asks for them. Like you said, I think it is a bit premature to go that far. In anycase, I don't see a way to get rid of string_stream, without significantly sacrificing usability. > I can imagine someone wanting to write unit tests that check conditions > from a simulated hardirq context via irq works (a driver mock > framework?), so this doesn't seem far off. Yep, I actually presented the first pieces of that in the RFC v1 that I linked to you earlier in this discussion. I have a more fleshed out example here: https://kunit.googlesource.com/linux/+/e10484ad2f9fc7926412ec84739fe105981b4771/drivers/i2c/busses/i2c-aspeed-test.c I actually already have some people at Google playing around with it. So yeah, not far off at all! However, in these cases we are not actually running in the IRQ context (despite the fact that we are testing IRQ code) because we provide a fake IRQ chip, or some other fake mechanism that triggers the IRQ. Still, I could see someone wanting to do it in a non-fake-IRQ context. Luis' suggestion was just to hold off on the IRQ safe stuff at the outset, since that is going to require a lot more effort to review. I know that's kind of the future coding argument again, but maybe the answer will be to just restrict what features an IRQ user has access to (maybe just really simple expectations, for example). I mean, we will probably have to restrict what they are allowed to use anyway. Luis, do you have any ideas? Cheers
Re: [PATCH v2] drm/tegra: sor: Enable HDA interrupts at plugin
22.07.2019 12:27, Viswanath L пишет: > HDMI plugout calls runtime suspend, which clears interrupt registers > and causes audio functionality to break on subsequent plugin; setting > interrupt registers in sor_audio_prepare() solves the issue Hello Viswanath, A dot should be in the end of sentence, please. And should be better to s/plugin/plug-in/ in the commit's message/title because 'plugin' sounds as a noun. Please don't version patch as v2 if v1 wasn't ever sent out. You should add a stable tag here to get patch backported into stable kernel versions: Cc: > Signed-off-by: Viswanath L The kernel upstreaming rules require a full name. I'm pretty sure that L isn't yours surname. > --- > drivers/gpu/drm/tegra/sor.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c > index 5be5a08..0470cfe 100644 > --- a/drivers/gpu/drm/tegra/sor.c > +++ b/drivers/gpu/drm/tegra/sor.c > @@ -2164,6 +2164,15 @@ static void tegra_sor_audio_prepare(struct tegra_sor > *sor) > > value = SOR_AUDIO_HDA_PRESENSE_ELDV | SOR_AUDIO_HDA_PRESENSE_PD; > tegra_sor_writel(sor, value, SOR_AUDIO_HDA_PRESENSE); > + > + /* > + * Enable and unmask the HDA codec SCRATCH0 register interrupt. This > + * is used for interoperability between the HDA codec driver and the > + * HDMI/DP driver. > + */ > + value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0; > + tegra_sor_writel(sor, value, SOR_INT_ENABLE); > + tegra_sor_writel(sor, value, SOR_INT_MASK); > } > > static void tegra_sor_audio_unprepare(struct tegra_sor *sor) > @@ -2913,15 +2922,6 @@ static int tegra_sor_init(struct host1x_client *client) > if (err < 0) > return err; > > - /* > - * Enable and unmask the HDA codec SCRATCH0 register interrupt. This > - * is used for interoperability between the HDA codec driver and the > - * HDMI/DP driver. > - */ > - value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0; > - tegra_sor_writel(sor, value, SOR_INT_ENABLE); > - tegra_sor_writel(sor, value, SOR_INT_MASK); > - > return 0; > } > >
Re: [PATCH 3/3] net/xdp: convert put_page() to put_user_page*()
On Mon, Jul 22, 2019 at 03:34:15PM -0700, john.hubb...@gmail.com wrote: > From: John Hubbard > > For pages that were retained via get_user_pages*(), release those pages > via the new put_user_page*() routines, instead of via put_page() or > release_pages(). > > This is part a tree-wide conversion, as described in commit fc1d8e7cca2d > ("mm: introduce put_user_page*(), placeholder versions"). > > Cc: Björn Töpel > Cc: Magnus Karlsson > Cc: David S. Miller > Cc: net...@vger.kernel.org > Signed-off-by: John Hubbard > --- > net/xdp/xdp_umem.c | 9 + > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c > index 83de74ca729a..0325a17915de 100644 > --- a/net/xdp/xdp_umem.c > +++ b/net/xdp/xdp_umem.c > @@ -166,14 +166,7 @@ void xdp_umem_clear_dev(struct xdp_umem *umem) > > static void xdp_umem_unpin_pages(struct xdp_umem *umem) > { > - unsigned int i; > - > - for (i = 0; i < umem->npgs; i++) { > - struct page *page = umem->pgs[i]; > - > - set_page_dirty_lock(page); > - put_page(page); > - } > + put_user_pages_dirty_lock(umem->pgs, umem->npgs); What is the difference between this and __put_user_pages(umem->pgs, umem->npgs, PUP_FLAGS_DIRTY_LOCK); ? I'm a bit concerned with adding another form of the same interface. We should either have 1 call with flags (enum in this case) or multiple calls. Given the previous discussion lets move in the direction of having the enum but don't introduce another caller of the "old" interface. So I think on this patch NAK from me. I also don't like having a __* call in the exported interface but there is a __get_user_pages_fast() call so I guess there is precedent. :-/ Ira > > kfree(umem->pgs); > umem->pgs = NULL; > -- > 2.22.0 >
Re: [Intel-gfx] [PATCH] drm/i915: Mark expected switch fall-throughs
Hi Gustavo, could you please rebase on top of drm-tip and resend it please? Thanks, Rodrigo. On Mon, Jul 22, 2019 at 01:12:44PM -0500, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch > cases where we are expecting to fall through. > > This patch fixes the following warnings: > > drivers/gpu/drm/i915/gem/i915_gem_mman.c: In function ‘i915_gem_fault’: > drivers/gpu/drm/i915/gem/i915_gem_mman.c:342:6: warning: this statement may > fall through [-Wimplicit-fallthrough=] >if (!i915_terminally_wedged(i915)) > ^ > drivers/gpu/drm/i915/gem/i915_gem_mman.c:345:2: note: here > case -EAGAIN: > ^~~~ > > drivers/gpu/drm/i915/gem/i915_gem_pages.c: In function ‘i915_gem_object_map’: > ./include/linux/compiler.h:78:22: warning: this statement may fall through > [-Wimplicit-fallthrough=] > # define unlikely(x) __builtin_expect(!!(x), 0) > ^~ > ./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’ > unlikely(__ret_warn_on); \ > ^~~~ > drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’ > #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \ > ^~~~ > drivers/gpu/drm/i915/gem/i915_gem_pages.c:270:3: note: in expansion of macro > ‘MISSING_CASE’ >MISSING_CASE(type); >^~~~ > drivers/gpu/drm/i915/gem/i915_gem_pages.c:272:2: note: here > case I915_MAP_WB: > ^~~~ > > drivers/gpu/drm/i915/i915_gpu_error.c: In function > ‘error_record_engine_registers’: > ./include/linux/compiler.h:78:22: warning: this statement may fall through > [-Wimplicit-fallthrough=] > # define unlikely(x) __builtin_expect(!!(x), 0) > ^~ > ./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’ > unlikely(__ret_warn_on); \ > ^~~~ > drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’ > #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \ > ^~~~ > drivers/gpu/drm/i915/i915_gpu_error.c:1196:5: note: in expansion of macro > ‘MISSING_CASE’ > MISSING_CASE(engine->id); > ^~~~ > drivers/gpu/drm/i915/i915_gpu_error.c:1197:4: note: here > case RCS0: > ^~~~ > > drivers/gpu/drm/i915/display/intel_dp.c: In function > ‘intel_dp_get_fia_supported_lane_count’: > ./include/linux/compiler.h:78:22: warning: this statement may fall through > [-Wimplicit-fallthrough=] > # define unlikely(x) __builtin_expect(!!(x), 0) > ^~ > ./include/asm-generic/bug.h:136:2: note: in expansion of macro ‘unlikely’ > unlikely(__ret_warn_on); \ > ^~~~ > drivers/gpu/drm/i915/i915_utils.h:49:25: note: in expansion of macro ‘WARN’ > #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \ > ^~~~ > drivers/gpu/drm/i915/display/intel_dp.c:233:3: note: in expansion of macro > ‘MISSING_CASE’ >MISSING_CASE(lane_info); >^~~~ > drivers/gpu/drm/i915/display/intel_dp.c:234:2: note: here > case 1: > ^~~~ > > drivers/gpu/drm/i915/display/intel_display.c: In function > ‘check_digital_port_conflicts’: > CC [M] drivers/gpu/drm/nouveau/nvkm/engine/disp/cursgv100.o > drivers/gpu/drm/i915/display/intel_display.c:12043:7: warning: this statement > may fall through [-Wimplicit-fallthrough=] > if (WARN_ON(!HAS_DDI(to_i915(dev >^ > drivers/gpu/drm/i915/display/intel_display.c:12046:3: note: here >case INTEL_OUTPUT_DP: >^~~~ > > Also, notice that the Makefile is modified in order to stop > ignoring fall-through warnings. The -Wimplicit-fallthrough > option will be enabled globally in v5.3. > > Warning level 3 was used: -Wimplicit-fallthrough=3 > > This patch is part of the ongoing efforts to enable > -Wimplicit-fallthrough. > > Signed-off-by: Gustavo A. R. Silva > --- > drivers/gpu/drm/i915/Makefile| 1 - > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > drivers/gpu/drm/i915/display/intel_dp.c | 1 + > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- > drivers/gpu/drm/i915/gem/i915_gem_pages.c| 2 +- > drivers/gpu/drm/i915/i915_gpu_error.c| 1 + > 6 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 91355c2ea8a5..8cace65f50ce 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -16,7 +16,6 @@ subdir-ccflags-y := -Wall -Wextra > subdir-ccflags-y += $(call cc-disable-warning, unused-parameter) > subdir-ccflags-y += $(call cc-disable-warning, type-limits) > subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers) > -subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough) > subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable) > # clang warnings > subdir-ccflags-y += $(call cc-dis
Re: [PATCH v3 0/4] backlight: Expose brightness curve type through sysfs
On Tue, Jul 09, 2019 at 12:00:03PM -0700, Matthias Kaehlcke wrote: > Backlight brightness curves can have different shapes. The two main > types are linear and non-linear curves. The human eye doesn't > perceive linearly increasing/decreasing brightness as linear (see > also 88ba95bedb79 "backlight: pwm_bl: Compute brightness of LED > linearly to human eye"), hence many backlights use non-linear (often > logarithmic) brightness curves. The type of curve is currently opaque > to userspace, so userspace often relies on more or less reliable > heuristics (like the number of brightness levels) to decide whether > to treat a backlight device as linear or non-linear. > > Export the type of the brightness curve via a new sysfs attribute. > > Matthias Kaehlcke (4): > MAINTAINERS: Add entry for stable backlight sysfs ABI documentation > backlight: Expose brightness curve type through sysfs > backlight: pwm_bl: Set scale type for CIE 1931 curves > backlight: pwm_bl: Set scale type for brightness curves specified in > the DT > > .../ABI/testing/sysfs-class-backlight | 26 ++ > MAINTAINERS | 2 ++ > drivers/video/backlight/backlight.c | 19 ++ > drivers/video/backlight/pwm_bl.c | 35 ++- > include/linux/backlight.h | 8 + > 5 files changed, 89 insertions(+), 1 deletion(-) > create mode 100644 Documentation/ABI/testing/sysfs-class-backlight ping, any comments on v3? Thanks Matthias
Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
Quoting Brendan Higgins (2019-07-22 15:30:49) > On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd wrote: > > > > > > What's the calling context of the assertions and expectations? I still > > don't like the fact that string stream needs to allocate buffers and > > throw them into a list somewhere because the calling context matters > > there. > > The calling context is the same as before, which is anywhere. Ok. That's concerning then. > > > I'd prefer we just wrote directly to the console/log via printk > > instead. That way things are simple because we use the existing > > buffering path of printk, but maybe there's some benefit to the string > > stream that I don't see? Right now it looks like it builds a string and > > then dumps it to printk so I'm sort of lost what the benefit is over > > just writing directly with printk. > > It's just buffering it so the whole string gets printed uninterrupted. > If we were to print out piecemeal to printk, couldn't we have another > call to printk come in causing it to garble the KUnit message we are > in the middle of printing? Yes, printing piecemeal by calling printk many times could lead to interleaving of messages if something else comes in such as an interrupt printing something. Printk has some support to hold "records" but I'm not sure how that would work here because KERN_CONT talks about only being used early on in boot code. I haven't looked at printk in detail though so maybe I'm all wrong and KERN_CONT just works? Can printk be called once with whatever is in the struct? Otherwise if this is about making printk into a structured log then maybe printk isn't the proper solution anyway. Maybe a dev interface should be used instead that can handle starting and stopping tests (via ioctl) in addition to reading test results, records, etc. with read() and a clearing of the records. Then the seqfile API works naturally. All of this is a bit premature, but it looks like you're going down the path of making something akin to ftrace that stores binary formatted assertion/expectation records in a lockless ring buffer that then formats those records when the user asks for them. I can imagine someone wanting to write unit tests that check conditions from a simulated hardirq context via irq works (a driver mock framework?), so this doesn't seem far off.
Re: [RFC] Expanding drm_mode_modeinfo flags
On 2019-07-19 07:29, Sean Paul wrote: On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote: On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote: > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote: > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote: > > > On 2019-07-16 02:07, Daniel Vetter wrote: > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote: /snip > > > > > drm: add mode flags in uapi for seamless mode switch > > > > > > > > I think the uapi is the trivial part here, the real deal is how > > > > userspace > > > > uses this. Can you pls post the patches for your compositor? > > > > > > > > Also note that we already allow userspace to tell the kernel whether > > > > flickering is ok or not for a modeset. msm driver could use that to at > > > > least tell userspace whether a modeset change is possible. So you can > > > > already implement glitch-free modeset changes for at least video mode. > > > > -Daniel > > > > > > I believe you are referring to the below tv property of the connector. > > > > > > /** > > > * @tv_flicker_reduction_property: Optional TV property to control the > > > * flicker reduction mode. > > > */ > > > struct drm_property *tv_flicker_reduction_property; > > > > Not even close :-) > > > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This > > is not a property of a mode, this is a property of a _transition_ between > > configurations. Some transitions can be done flicker free, others can't. > > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty > similar to the psr transitions, where we want to reuse most of the atomic > circuitry, but in a specialized way. We'd also have to be careful to only > involve the drm objects which are seamless modeset aware (you could imagine > a bridge chain where the bridges downstream of the first bridge don't care). > > > > > There's then still the question of how to pick video vs command mode, but > > imo better to start with implementing the transition behaviour correctly > > first. > > Connector property? Possibly a terrible idea, but I wonder if we could [re]use > the vrr properties for command mode. The docs state that the driver has the > option of putting upper and lower bounds on the refresh rate. Not really sure why this needs new props and whatnot. This is kinda what the i915 "fastset" stuff already does: 1. userspace asks for something to be changed via atomic 2. driver calculates whether a modeset is actually required 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET 4. if (need_modeset) heavyweight_commit() else lightweight_commit() Ie. why should userspace really care about anything except the "flickers are OK" vs. "flickers not wanted" thing? Agree, I don't think the seamless modeset (ie: changing resolution without flicker) needs a property. Just need to test the commit without ALLOW_MODESET and commit it if the test passes. Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to check whether the modeset can be done seamless. But since there are no error code returns, the client cannot distinguish the test_only commit failures from other invalid config failures. Also, note that when the client sees two 1080p modes (vid/cmd) and it is interested only to do *only* seamless switches, without any additional flag it cannot distinguish between these two 1080p modes. The client has to invoke two test_only commits with these modes to identify the seamless one. Is that a preferred approach? Intel's "fastset" calculates the need for modeset internally based on the configuration and chooses the best commit path. But the requirement here is to expose the information up-front since the use case cannot afford to fall back to the normal modeset when it has requested for a seamless one. Also what's the benefit of using video mode if your panel supportes command mode? Can you turn off the memory in the panel and actually save power that way? And if there is a benefit can't the driver just automagically switch between the two based on how often things are getting updated? That would match how eDP PSR already works. I'm guessing video mode might have some latency benefits over command mode? Sean Yes. Pretty much those are reasons we need to switch to video mode. But instead of making the decision internal to the driver based on the frequency of frame updates, we have proprietary use cases where the client has to trigger the switch explicitly. So we are trying to find ways to represent the same resolution in both video/cmd modes. Thanks and Regards, Jeykumar S. -- Ville Syrjälä Intel -- Jeykumar S ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204145] amdgpu video playback causes host to hard reset (checkstop) on POWER9 with RX 580
https://bugzilla.kernel.org/show_bug.cgi?id=204145 Michael Ellerman (mich...@ellerman.id.au) changed: What|Removed |Added Status|NEW |RESOLVED CC||mich...@ellerman.id.au Resolution|--- |CODE_FIX --- Comment #25 from Michael Ellerman (mich...@ellerman.id.au) --- This is in my fixes branch which I'll send to Linus later this week: https://git.kernel.org/powerpc/c/b4fc36e60f25cf22bf8b7b015a701015740c3743 -- 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
[PATCH 3/3] net/xdp: convert put_page() to put_user_page*()
From: John Hubbard For pages that were retained via get_user_pages*(), release those pages via the new put_user_page*() routines, instead of via put_page() or release_pages(). This is part a tree-wide conversion, as described in commit fc1d8e7cca2d ("mm: introduce put_user_page*(), placeholder versions"). Cc: Björn Töpel Cc: Magnus Karlsson Cc: David S. Miller Cc: net...@vger.kernel.org Signed-off-by: John Hubbard --- net/xdp/xdp_umem.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c index 83de74ca729a..0325a17915de 100644 --- a/net/xdp/xdp_umem.c +++ b/net/xdp/xdp_umem.c @@ -166,14 +166,7 @@ void xdp_umem_clear_dev(struct xdp_umem *umem) static void xdp_umem_unpin_pages(struct xdp_umem *umem) { - unsigned int i; - - for (i = 0; i < umem->npgs; i++) { - struct page *page = umem->pgs[i]; - - set_page_dirty_lock(page); - put_page(page); - } + put_user_pages_dirty_lock(umem->pgs, umem->npgs); kfree(umem->pgs); umem->pgs = NULL; -- 2.22.0
[PATCH 2/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()
From: John Hubbard For pages that were retained via get_user_pages*(), release those pages via the new put_user_page*() routines, instead of via put_page() or release_pages(). This is part a tree-wide conversion, as described in commit fc1d8e7cca2d ("mm: introduce put_user_page*(), placeholder versions"). Also reverse the order of a comparison, in order to placate checkpatch.pl. Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Hubbard --- drivers/gpu/drm/via/via_dmablit.c | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c index 062067438f1d..754f2bb97d61 100644 --- a/drivers/gpu/drm/via/via_dmablit.c +++ b/drivers/gpu/drm/via/via_dmablit.c @@ -171,7 +171,6 @@ via_map_blit_for_device(struct pci_dev *pdev, static void via_free_sg_info(struct pci_dev *pdev, drm_via_sg_info_t *vsg) { - struct page *page; int i; switch (vsg->state) { @@ -186,13 +185,9 @@ via_free_sg_info(struct pci_dev *pdev, drm_via_sg_info_t *vsg) kfree(vsg->desc_pages); /* fall through */ case dr_via_pages_locked: - for (i = 0; i < vsg->num_pages; ++i) { - if (NULL != (page = vsg->pages[i])) { - if (!PageReserved(page) && (DMA_FROM_DEVICE == vsg->direction)) - SetPageDirty(page); - put_page(page); - } - } + __put_user_pages(vsg->pages, vsg->num_pages, +(vsg->direction == DMA_FROM_DEVICE) ? +PUP_FLAGS_DIRTY : PUP_FLAGS_CLEAN); /* fall through */ case dr_via_pages_alloc: vfree(vsg->pages); -- 2.22.0
[PATCH 0/3] introduce __put_user_pages(), convert a few call sites
From: John Hubbard As discussed in [1] just now, this adds a more capable variation of put_user_pages() to the API set, and uses it to simplify both the main implementation, and (especially) the call sites. Thanks to Christoph for the simplifying ideas, and Matthew for (again) recommending an enum in the API. Matthew, I seem to recall you asked for enums before this, so I'm sorry it took until now for me to add them. :) The new __put_user_pages() takes an enum that handles the various combinations of needing to call set_page_dirty() or set_page_dirty_lock(), before calling put_user_page(). I'm using the same CC list as in [1], even though IB is no longer included in the series. That's everyone can see what the end result turns out to be. Notes about the remaining patches to come: There are about 50+ patches in my tree [2], and I'll be sending out the remaining ones in a few more groups: * The block/bio related changes (Jerome mostly wrote those, but I've had to move stuff around extensively, and add a little code) * mm/ changes * other subsystem patches * an RFC that shows the current state of the tracking patch set. That can only be applied after all call sites are converted, but it's good to get an early look at it. This is part a tree-wide conversion, as described in commit fc1d8e7cca2d ("mm: introduce put_user_page*(), placeholder versions"). [1] https://lore.kernel.org/r/20190722093355.gb29...@lst.de [2] https://github.com/johnhubbard/linux/tree/gup_dma_core John Hubbard (3): mm/gup: introduce __put_user_pages() drivers/gpu/drm/via: convert put_page() to put_user_page*() net/xdp: convert put_page() to put_user_page*() drivers/gpu/drm/via/via_dmablit.c | 11 +-- include/linux/mm.h| 58 +++- mm/gup.c | 149 +++--- net/xdp/xdp_umem.c| 9 +- 4 files changed, 135 insertions(+), 92 deletions(-) -- 2.22.0
[PATCH 1/3] mm/gup: introduce __put_user_pages()
From: John Hubbard Add a more capable variation of put_user_pages() to the API set, and call it from the simple ones. The new __put_user_pages() takes an enum that handles the various combinations of needing to call set_page_dirty() or set_page_dirty_lock(), before calling put_user_page(). Cc: Matthew Wilcox Cc: Jan Kara Cc: Christoph Hellwig Signed-off-by: John Hubbard --- include/linux/mm.h | 58 ++- mm/gup.c | 137 ++--- 2 files changed, 124 insertions(+), 71 deletions(-) diff --git a/include/linux/mm.h b/include/linux/mm.h index 0334ca97c584..7218585681b2 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1057,8 +1057,62 @@ static inline void put_user_page(struct page *page) put_page(page); } -void put_user_pages_dirty(struct page **pages, unsigned long npages); -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages); +enum pup_flags_t { + PUP_FLAGS_CLEAN = 0, + PUP_FLAGS_DIRTY = 1, + PUP_FLAGS_LOCK = 2, + PUP_FLAGS_DIRTY_LOCK= 3, +}; + +void __put_user_pages(struct page **pages, unsigned long npages, + enum pup_flags_t flags); + +/** + * put_user_pages_dirty() - release and dirty an array of gup-pinned pages + * @pages: array of pages to be marked dirty and released. + * @npages: number of pages in the @pages array. + * + * "gup-pinned page" refers to a page that has had one of the get_user_pages() + * variants called on that page. + * + * For each page in the @pages array, make that page (or its head page, if a + * compound page) dirty, if it was previously listed as clean. Then, release + * the page using put_user_page(). + * + * Please see the put_user_page() documentation for details. + * + * set_page_dirty(), which does not lock the page, is used here. + * Therefore, it is the caller's responsibility to ensure that this is + * safe. If not, then put_user_pages_dirty_lock() should be called instead. + * + */ +static inline void put_user_pages_dirty(struct page **pages, + unsigned long npages) +{ + __put_user_pages(pages, npages, PUP_FLAGS_DIRTY); +} + +/** + * put_user_pages_dirty_lock() - release and dirty an array of gup-pinned pages + * @pages: array of pages to be marked dirty and released. + * @npages: number of pages in the @pages array. + * + * For each page in the @pages array, make that page (or its head page, if a + * compound page) dirty, if it was previously listed as clean. Then, release + * the page using put_user_page(). + * + * Please see the put_user_page() documentation for details. + * + * This is just like put_user_pages_dirty(), except that it invokes + * set_page_dirty_lock(), instead of set_page_dirty(). + * + */ +static inline void put_user_pages_dirty_lock(struct page **pages, +unsigned long npages) +{ + __put_user_pages(pages, npages, PUP_FLAGS_DIRTY_LOCK); +} + void put_user_pages(struct page **pages, unsigned long npages); #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) diff --git a/mm/gup.c b/mm/gup.c index 98f13ab37bac..6831ef064d76 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -29,87 +29,86 @@ struct follow_page_context { unsigned int page_mask; }; -typedef int (*set_dirty_func_t)(struct page *page); - -static void __put_user_pages_dirty(struct page **pages, - unsigned long npages, - set_dirty_func_t sdf) -{ - unsigned long index; - - for (index = 0; index < npages; index++) { - struct page *page = compound_head(pages[index]); - - /* -* Checking PageDirty at this point may race with -* clear_page_dirty_for_io(), but that's OK. Two key cases: -* -* 1) This code sees the page as already dirty, so it skips -* the call to sdf(). That could happen because -* clear_page_dirty_for_io() called page_mkclean(), -* followed by set_page_dirty(). However, now the page is -* going to get written back, which meets the original -* intention of setting it dirty, so all is well: -* clear_page_dirty_for_io() goes on to call -* TestClearPageDirty(), and write the page back. -* -* 2) This code sees the page as clean, so it calls sdf(). -* The page stays dirty, despite being written back, so it -* gets written back again in the next writeback cycle. -* This is harmless. -*/ - if (!PageDirty(page)) - sdf(page); - - put_user_page(page); - } -} - /** - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages + * __put_user_pages() - releas
Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd wrote: > > Quoting Brendan Higgins (2019-07-18 17:08:34) > > On Thu, Jul 18, 2019 at 12:22:33PM -0700, Brendan Higgins wrote: > > > > I started poking around with your suggestion while we are waiting. A > > couple early observations: > > > > 1) It is actually easier to do than I previously thought and will probably > >help with getting more of the planned TAP output stuff working. > > > >That being said, this is still a pretty substantial undertaking and > >will likely take *at least* a week to implement and properly review. > >Assuming everything goes extremely well (no unexpected issues on my > >end, very responsive reviewers, etc). > > > > 2) It *will* eliminate the need for kunit_stream. > > > > 3) ...but, it *will not* eliminate the need for string_stream. > > > > Based on my early observations, I do think it is worth doing, but I > > don't think it is worth trying to make it in this patchset (unless I > > have already missed the window, or it is going to be open for a while): > > The merge window is over. Typically code needs to be settled a few weeks > before it opens (i.e. around -rc4 or -rc5) for most maintainers to pick > up patches for the next merge window. Yeah, it closed on Sunday, right? I thought we might be able to squeak in since it was *mostly* settled, and Shuah sent me an email two weeks ago which I interpreted to mean she was still willing to take it. In any case, it doesn't matter now. > > I do think it will make things much cleaner, but I don't think it will > > achieve your desired goal of getting rid of an unstructured > > {kunit|string}_stream style interface; it just adds a layer on top of it > > that makes it harder to misuse. > > Ok. > > > > > I attached a patch of what I have so far at the end of this email so you > > can see what I am talking about. And of course, if you agree with my > > assessment, so we can start working on it as a future patch. > > > > A couple things in regard to the patch I attached: > > > > 1) I wrote it pretty quickly so there are almost definitely mistakes. > >You should consider it RFC. I did verify it compiles though. > > > > 2) Also, I did use kunit_stream in writing it: all occurences should be > >pretty easy to replace with string_stream; nevertheless, the reason > >for this is just to make it easier to play with the current APIs. I > >wanted to have something working before I went through a big tedious > >refactoring. So sorry if it causes any confusion. > > > > 3) I also based the patch on all the KUnit patches I have queued up > >(includes things like mocking and such) since I want to see how this > >serialization thing will work with mocks and matchers and things like > >that. > > Great! > > > > > From 53d475d3d56afcf92b452c6d347dbedfa1a17d34 Mon Sep 17 00:00:00 2001 > > From: Brendan Higgins > > Date: Thu, 18 Jul 2019 16:08:52 -0700 > > Subject: [PATCH v1] DO NOT MERGE: started playing around with the > > serialization api > > > > --- > > include/kunit/assert.h | 130 ++ > > include/kunit/mock.h | 4 + > > kunit/Makefile | 3 +- > > kunit/assert.c | 179 + > > kunit/mock.c | 6 +- > > 5 files changed, 318 insertions(+), 4 deletions(-) > > create mode 100644 include/kunit/assert.h > > create mode 100644 kunit/assert.c > > > > diff --git a/include/kunit/assert.h b/include/kunit/assert.h > > new file mode 100644 > > index 0..e054fdff4642f > > --- /dev/null > > +++ b/include/kunit/assert.h > > @@ -0,0 +1,130 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Assertion and expectation serialization API. > > + * > > + * Copyright (C) 2019, Google LLC. > > + * Author: Brendan Higgins > > + */ > > + > > +#ifndef _KUNIT_ASSERT_H > > +#define _KUNIT_ASSERT_H > > + > > +#include > > +#include > > + > > +enum kunit_assert_type { > > + KUNIT_ASSERTION, > > + KUNIT_EXPECTATION, > > +}; > > + > > +struct kunit_assert { > > + enum kunit_assert_type type; > > + const char *line; > > + const char *file; > > + struct va_format message; > > + void (*format)(struct kunit_assert *assert, > > + struct kunit_stream *stream); > > Would passing in the test help too? Yeah, it would probably be good to put one in `struct kunit_assert`. > > +}; > > + > > +void kunit_base_assert_format(struct kunit_assert *assert, > > + struct kunit_stream *stream); > > + > > +void kunit_assert_print_msg(struct kunit_assert *assert, > > + struct kunit_stream *stream); > > + > > +struct kunit_unary_assert { > > + struct kunit_assert assert; > > + const char *condition; > > + bool expected_true; > > +}; > > + > > +void kunit_unary_assert_format(struct kunit_assert *assert, > > + struct kunit_s
[PATCH] drm/exynos: fix missing decrement of retry counter
From: Colin Ian King Currently the retry counter is not being decremented, leading to a potential infinite spin if the scalar_reads don't change state. Addresses-Coverity: ("Infinite loop") Fixes: 280e54c9f614 ("drm/exynos: scaler: Reset hardware before starting the operation") Signed-off-by: Colin Ian King --- drivers/gpu/drm/exynos/exynos_drm_scaler.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_scaler.c b/drivers/gpu/drm/exynos/exynos_drm_scaler.c index 9af096479e1c..b24ba948b725 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_scaler.c +++ b/drivers/gpu/drm/exynos/exynos_drm_scaler.c @@ -94,12 +94,12 @@ static inline int scaler_reset(struct scaler_context *scaler) scaler_write(SCALER_CFG_SOFT_RESET, SCALER_CFG); do { cpu_relax(); - } while (retry > 1 && + } while (--retry > 1 && scaler_read(SCALER_CFG) & SCALER_CFG_SOFT_RESET); do { cpu_relax(); scaler_write(1, SCALER_INT_EN); - } while (retry > 0 && scaler_read(SCALER_INT_EN) != 1); + } while (--retry > 0 && scaler_read(SCALER_INT_EN) != 1); return retry ? 0 : -EIO; } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Fix up broken merge
On Mon, Jul 22, 2019 at 11:41 PM Chris Wilson wrote: > > Quoting Daniel Vetter (2019-07-22 22:37:59) > > Maxime didn't really compile-test this :-/ > > > > We need to re-apply > > > > commit e4fa8457b2197118538a1400b75c898f9faaf164 > > Author: Daniel Vetter > > Date: Fri Jun 14 22:35:25 2019 +0200 > > > > drm/prime: Align gem_prime_export with obj_funcs.export > > > > plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It > > moved in > > > > commit 10be98a77c558f8cfb823cd2777171fbb35040f6 > > Author: Chris Wilson > > Date: Tue May 28 10:29:49 2019 +0100 > > > > drm/i915: Move more GEM objects under gem/ > > > > v2: Remember the selftests (Chris). > > > > Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next") > > Cc: Maxime Ripard > > Cc: Chris Wilson > > Signed-off-by: Daniel Vetter > > Matches the same compile fix and git rm as I did locally, > Reviewed-by: Chris Wilson And pushed with Dave's irc-ack. Thanks for helping along to get this rectified. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Fix up broken merge
Quoting Daniel Vetter (2019-07-22 22:37:59) > Maxime didn't really compile-test this :-/ > > We need to re-apply > > commit e4fa8457b2197118538a1400b75c898f9faaf164 > Author: Daniel Vetter > Date: Fri Jun 14 22:35:25 2019 +0200 > > drm/prime: Align gem_prime_export with obj_funcs.export > > plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It > moved in > > commit 10be98a77c558f8cfb823cd2777171fbb35040f6 > Author: Chris Wilson > Date: Tue May 28 10:29:49 2019 +0100 > > drm/i915: Move more GEM objects under gem/ > > v2: Remember the selftests (Chris). > > Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next") > Cc: Maxime Ripard > Cc: Chris Wilson > Signed-off-by: Daniel Vetter Matches the same compile fix and git rm as I did locally, Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: Fix up broken merge
Maxime didn't really compile-test this :-/ We need to re-apply commit e4fa8457b2197118538a1400b75c898f9faaf164 Author: Daniel Vetter Date: Fri Jun 14 22:35:25 2019 +0200 drm/prime: Align gem_prime_export with obj_funcs.export plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It moved in commit 10be98a77c558f8cfb823cd2777171fbb35040f6 Author: Chris Wilson Date: Tue May 28 10:29:49 2019 +0100 drm/i915: Move more GEM objects under gem/ v2: Remember the selftests (Chris). Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next") Cc: Maxime Ripard Cc: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c| 5 +- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 8 +- drivers/gpu/drm/i915/i915_gem_dmabuf.c| 336 --- .../gpu/drm/i915/selftests/i915_gem_dmabuf.c | 404 -- 4 files changed, 6 insertions(+), 747 deletions(-) delete mode 100644 drivers/gpu/drm/i915/i915_gem_dmabuf.c delete mode 100644 drivers/gpu/drm/i915/selftests/i915_gem_dmabuf.c diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index cbf1701d3acc..570b20ad9e58 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -204,8 +204,7 @@ static const struct dma_buf_ops i915_dmabuf_ops = { .end_cpu_access = i915_gem_end_cpu_access, }; -struct dma_buf *i915_gem_prime_export(struct drm_device *dev, - struct drm_gem_object *gem_obj, int flags) +struct dma_buf *i915_gem_prime_export(struct drm_gem_object *gem_obj, int flags) { struct drm_i915_gem_object *obj = to_intel_bo(gem_obj); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); @@ -222,7 +221,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev, return ERR_PTR(ret); } - return drm_gem_dmabuf_export(dev, &exp_info); + return drm_gem_dmabuf_export(gem_obj->dev, &exp_info); } static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj) diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c index e3a64edef918..d85d1ce273ca 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c @@ -20,7 +20,7 @@ static int igt_dmabuf_export(void *arg) if (IS_ERR(obj)) return PTR_ERR(obj); - dmabuf = i915_gem_prime_export(&i915->drm, &obj->base, 0); + dmabuf = i915_gem_prime_export(&obj->base, 0); i915_gem_object_put(obj); if (IS_ERR(dmabuf)) { pr_err("i915_gem_prime_export failed with err=%d\n", @@ -44,7 +44,7 @@ static int igt_dmabuf_import_self(void *arg) if (IS_ERR(obj)) return PTR_ERR(obj); - dmabuf = i915_gem_prime_export(&i915->drm, &obj->base, 0); + dmabuf = i915_gem_prime_export(&obj->base, 0); if (IS_ERR(dmabuf)) { pr_err("i915_gem_prime_export failed with err=%d\n", (int)PTR_ERR(dmabuf)); @@ -219,7 +219,7 @@ static int igt_dmabuf_export_vmap(void *arg) if (IS_ERR(obj)) return PTR_ERR(obj); - dmabuf = i915_gem_prime_export(&i915->drm, &obj->base, 0); + dmabuf = i915_gem_prime_export(&obj->base, 0); if (IS_ERR(dmabuf)) { pr_err("i915_gem_prime_export failed with err=%d\n", (int)PTR_ERR(dmabuf)); @@ -266,7 +266,7 @@ static int igt_dmabuf_export_kmap(void *arg) if (IS_ERR(obj)) return PTR_ERR(obj); - dmabuf = i915_gem_prime_export(&i915->drm, &obj->base, 0); + dmabuf = i915_gem_prime_export(&obj->base, 0); i915_gem_object_put(obj); if (IS_ERR(dmabuf)) { err = PTR_ERR(dmabuf); diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c deleted file mode 100644 index 54ecab91b3a9.. --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ /dev/null @@ -1,336 +0,0 @@ -/* - * Copyright 2012 Red Hat Inc - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A
Re: [Intel-gfx] [PATCH] drm/i915: Fix up broken merge
Quoting Daniel Vetter (2019-07-22 22:21:01) > Maxime didn't really compile-test this :-/ > > We need to re-apply > > commit e4fa8457b2197118538a1400b75c898f9faaf164 > Author: Daniel Vetter > Date: Fri Jun 14 22:35:25 2019 +0200 > > drm/prime: Align gem_prime_export with obj_funcs.export > > plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It > moved in > > commit 10be98a77c558f8cfb823cd2777171fbb35040f6 > Author: Chris Wilson > Date: Tue May 28 10:29:49 2019 +0100 > > drm/i915: Move more GEM objects under gem/ > > Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next") > Cc: Maxime Ripard > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 5 +- > drivers/gpu/drm/i915/i915_gem_dmabuf.c | 336 - 4 little fixes for the change in iface for gem/selftests/i915_gem_dmabuf.c (plus git rm selftests/i915_gem_dmabuf.c) -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 0/4] Add a generic driver for LED-based backlight
On 7/22/19 9:06 AM, Lee Jones wrote: > On Thu, 18 Jul 2019, Jacek Anaszewski wrote: > >> On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote: >>> This series aims to add a led-backlight driver, similar to pwm-backlight, >>> but using a LED class device underneath. >>> >>> A few years ago (2015), Tomi Valkeinen posted a series implementing a >>> backlight driver on top of a LED device: >>> https://patchwork.kernel.org/patch/7293991/ >>> https://patchwork.kernel.org/patch/7294001/ >>> https://patchwork.kernel.org/patch/7293981/ >>> >>> The discussion stopped because Tomi lacked the time to work on it. >>> >>> changes in v4: >>> - fix dev_err() messages and commit logs following the advices of Pavel >>> - cosmetic changes (indents, getting rid of "? 1 : 0" in >>> led_match_led_node()) >>> >>> changes in v3: >>> - dt binding: don't limit the brightness range to 0-255. Use the range of >>> the underlying LEDs. as a side-effect, all LEDs must now have the same >>> range >>> - driver: Adapt to dt binding update. >>> - driver: rework probe() for clarity and remove the remaining goto. >>> >>> changes in v2: >>> - handle more than one LED. >>> - don't make the backlight device a child of the LED controller. >>> - make brightness-levels and default-brightness-level optional >>> - removed the option to use a GPIO enable. >>> - removed the option to use a regulator. It should be handled by the LED >>> core >>> - don't make any change to the LED core (not needed anymore) >>> >>> Jean-Jacques Hiblot (2): >>> leds: Add managed API to get a LED from a device driver >>> dt-bindings: backlight: Add led-backlight binding >>> >>> Tomi Valkeinen (2): >>> leds: Add of_led_get() and led_put() >>> backlight: add led-backlight driver >>> >>> .../bindings/leds/backlight/led-backlight.txt | 28 ++ >>> drivers/leds/led-class.c | 92 ++ >>> drivers/video/backlight/Kconfig | 7 + >>> drivers/video/backlight/Makefile | 1 + >>> drivers/video/backlight/led_bl.c | 268 ++ >>> include/linux/leds.h | 6 + >>> 6 files changed, 402 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/leds/backlight/led-backlight.txt >>> create mode 100644 drivers/video/backlight/led_bl.c >>> >> >> For the whole set: >> >> Acked-by: Jacek Anaszewski >> >> Lee - we need to create immutable branch for this set since there will >> be some interfering changes in the LED core in this cycle. >> >> I can create the branch and send the pull request once we will >> obtain the ack from Rob for DT bindings, unless you have other >> preference. > > We also require a review to be conducted by Daniel Thompson. > > After which, an immutable branch sounds like a good idea. I'd like to > create this myself if you don't mind. Sure, thanks. -- Best regards, Jacek Anaszewski
[PATCH] drm/i915: Fix up broken merge
Maxime didn't really compile-test this :-/ We need to re-apply commit e4fa8457b2197118538a1400b75c898f9faaf164 Author: Daniel Vetter Date: Fri Jun 14 22:35:25 2019 +0200 drm/prime: Align gem_prime_export with obj_funcs.export plus make sure i915_gem_dma_buf.c doesn't get zombie-resurrect. It moved in commit 10be98a77c558f8cfb823cd2777171fbb35040f6 Author: Chris Wilson Date: Tue May 28 10:29:49 2019 +0100 drm/i915: Move more GEM objects under gem/ Fixes: 03b0f2ce735e ("Merge v5.3-rc1 into drm-misc-next") Cc: Maxime Ripard Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 5 +- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 336 - 2 files changed, 2 insertions(+), 339 deletions(-) delete mode 100644 drivers/gpu/drm/i915/i915_gem_dmabuf.c diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index cbf1701d3acc..570b20ad9e58 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -204,8 +204,7 @@ static const struct dma_buf_ops i915_dmabuf_ops = { .end_cpu_access = i915_gem_end_cpu_access, }; -struct dma_buf *i915_gem_prime_export(struct drm_device *dev, - struct drm_gem_object *gem_obj, int flags) +struct dma_buf *i915_gem_prime_export(struct drm_gem_object *gem_obj, int flags) { struct drm_i915_gem_object *obj = to_intel_bo(gem_obj); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); @@ -222,7 +221,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev, return ERR_PTR(ret); } - return drm_gem_dmabuf_export(dev, &exp_info); + return drm_gem_dmabuf_export(gem_obj->dev, &exp_info); } static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj) diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c deleted file mode 100644 index 54ecab91b3a9.. --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ /dev/null @@ -1,336 +0,0 @@ -/* - * Copyright 2012 Red Hat Inc - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - * DEALINGS IN THE SOFTWARE. - * - * Authors: - * Dave Airlie - */ - -#include -#include - - -#include "i915_drv.h" - -static struct drm_i915_gem_object *dma_buf_to_obj(struct dma_buf *buf) -{ - return to_intel_bo(buf->priv); -} - -static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachment, -enum dma_data_direction dir) -{ - struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf); - struct sg_table *st; - struct scatterlist *src, *dst; - int ret, i; - - ret = i915_gem_object_pin_pages(obj); - if (ret) - goto err; - - /* Copy sg so that we make an independent mapping */ - st = kmalloc(sizeof(struct sg_table), GFP_KERNEL); - if (st == NULL) { - ret = -ENOMEM; - goto err_unpin_pages; - } - - ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL); - if (ret) - goto err_free; - - src = obj->mm.pages->sgl; - dst = st->sgl; - for (i = 0; i < obj->mm.pages->nents; i++) { - sg_set_page(dst, sg_page(src), src->length, 0); - dst = sg_next(dst); - src = sg_next(src); - } - - if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) { - ret = -ENOMEM; - goto err_free_sg; - } - - return st; - -err_free_sg: - sg_free_table(st); -err_free: - kfree(st); -err_unpin_pages: - i915_gem_object_unpin_pages(obj); -err: - return ERR_PTR(ret); -} - -static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment, - struct sg_table *sg, -
[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault
https://bugs.freedesktop.org/show_bug.cgi?id=105251 --- Comment #78 from deltasquared --- Created attachment 144846 --> https://bugs.freedesktop.org/attachment.cgi?id=144846&action=edit vega_crasher after patch, colour shaded central output, on ryzen 2200G with vega 8 graphics Screenshot 2/2 of vega_crasher after patch. It seems to indeterministically switch between shaded and black central regions - I can only assume this is down to whether or not the offending index ends up out of bounds? If it helps I can attempt more tests with an asserts-enabled build, though that will take some more time, a resource I am out of for today. (Will have to look at how to do that also - just a question of a debug build or another flag that needs passing?) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Enable backlight when trigger is activated
Hi! > >> This looks fishy. > >> > >> Maybe you should use a default-state = "keep" instead? (and you'll have > >> to support it in the LED driver). > >> > >> That'll give you proper "don't touch the LED if it was turned on" behavior, > >> which is what you seem to want. > > > > Actually no, that's not what I want. LED should go on if the display > > is active, as soon as trigger is activated. > > > > Unfortunately, I have see no good way to tell if the display is > > active (and display is usually active when trigger is activated). > > default-state DT property can be also set to "on" > (see Documentation/devicetree/bindings/leds/common.txt). Ok, thanks for the hint, that could work. (I thought we were using default trigger to set the LED "on"). Now...this gives me option of 0% or 100% brightness, while best would be 10% brightness but I guess we can live with that ;-). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault
https://bugs.freedesktop.org/show_bug.cgi?id=105251 --- Comment #77 from deltasquared --- Created attachment 144845 --> https://bugs.freedesktop.org/attachment.cgi?id=144845&action=edit vega_crasher after patch, black central output, on ryzen 2200G with vega 8 graphics Screenshot time (1/2). It seems sometimes vega_crasher will render black - I haven't thoroughly looked over the code so I'm not sure if this is the adformentioned incorrect result (where an assert would have been hit). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller
On Mon, Jul 22, 2019 at 04:24:26PM -0400, Sean Paul wrote: > On Mon, Jul 22, 2019 at 11:19:45AM -0700, Matthias Kaehlcke wrote: > > The DDC/CI protocol involves sending a multi-byte request to the > > display via I2C, which is typically followed by a multi-byte > > response. The internal I2C controller only allows single byte > > reads/writes or reads of 8 sequential bytes, hence DDC/CI is not > > supported when the internal I2C controller is used. The I2C > > This is very likely a stupid question, but I didn't see an answer for it, so > I'll just ask :) > > If the controller supports xfers of 8 bytes and 1 bytes, could you just split > up any of these transactions into len/8+len%8 transactions? The controller interprets all transfers to be register accesses. It is not possible to just send the sequence '0x0a 0x0b 0x0c' as three byte transfers, the controller expects an address for each byte and (supposedly) sends it over the wire, which typically isn't what you want. Also the 8-byte reads only seem to be supported in certain configurations ("when the DWC_HDMI_TX_20 parameter is enabled"). > > transfers complete without errors, however the data in the response > > is garbage. Abort transfers to/from slave address 0x37 (DDC) with > > -EOPNOTSUPP, to make it evident that the communication is failing. > > > > Signed-off-by: Matthias Kaehlcke > > --- > > Changes in v2: > > - changed DDC_I2C_ADDR to DDC_CI_ADDR > > --- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > index 045b1b13fd0e..28933629f3c7 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > @@ -35,6 +35,7 @@ > > > > #include > > > > +#define DDC_CI_ADDR0x37 > > #define DDC_SEGMENT_ADDR 0x30 > > > > #define HDMI_EDID_LEN 512 > > @@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap, > > u8 addr = msgs[0].addr; > > int i, ret = 0; > > > > + if (addr == DDC_CI_ADDR) > > + /* > > +* The internal I2C controller does not support the multi-byte > > +* read and write operations needed for DDC/CI. > > +*/ > > + return -EOPNOTSUPP; > > + > > dev_dbg(hdmi->dev, "xfer: num: %d, addr: %#x\n", num, addr); > > > > for (i = 0; i < num; i++) { >
[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault
https://bugs.freedesktop.org/show_bug.cgi?id=105251 --- Comment #76 from deltasquared --- (In reply to deltasquared from comment #75) > L CALLBACK: type = 0x8251, severity = 0x826b, message = LLVM diagnostic GL_CALLBACK rather on that first line. terminal copypaste fail. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller
Hi, On Mon, Jul 22, 2019 at 1:24 PM Sean Paul wrote: > > On Mon, Jul 22, 2019 at 11:19:45AM -0700, Matthias Kaehlcke wrote: > > The DDC/CI protocol involves sending a multi-byte request to the > > display via I2C, which is typically followed by a multi-byte > > response. The internal I2C controller only allows single byte > > reads/writes or reads of 8 sequential bytes, hence DDC/CI is not > > supported when the internal I2C controller is used. The I2C > > This is very likely a stupid question, but I didn't see an answer for it, so > I'll just ask :) > > If the controller supports xfers of 8 bytes and 1 bytes, could you just split > up any of these transactions into len/8+len%8 transactions? It's not quite that easy, I think. Specifically a 1-byte transfer isn't really a 1-byte transfer. It always sticks this on the wire for a 1-byte write: Start Slave address (7 bits) + write (1 bit) (wait ack) Register address 1 byte of data wait for ack Stop ...or for a 1-byte read: Start Slave address (7 bits) + write (1 bit) (wait ack) Register address (wait ack) Repeated Start (1 bit) Slave address (7 bits) + read (1 bit) (read 1 byte of data) Ack Stop Putting more than one of those in a row is not the same thing as just doing a whole bunch of reads or a whole bunch of writes with no "stop" in between. As far as I could find out about DDC/CI it's part of the spec to _not_ send the stop between the reads / writes. -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault
https://bugs.freedesktop.org/show_bug.cgi?id=105251 --- Comment #75 from deltasquared --- After compiling mesa-git on commit 0661c357c60 from the AUR pkgbuild, I can now confirm my system seems to have become impervious to the above "vega_crasher" program. Output from said program after resizing and moving vega_crasher's window a bit, in case it was important: L CALLBACK: type = 0x8251, severity = 0x826b, message = LLVM diagnostic (remark): :0:0: 9 instructions in function GL CALLBACK: type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS: 16 VGPRS: 24 Code Size: 52 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0 Spilled VGPRs: 0 PrivMem VGPRs: 0 GL CALLBACK: type = 0x8251, severity = 0x826b, message = LLVM diagnostic (remark): :0:0: 12 instructions in function GL CALLBACK: type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS: 16 VGPRS: 8 Code Size: 92 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0 Spilled VGPRs: 0 PrivMem VGPRs: 0 GL CALLBACK: type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS: 16 VGPRS: 24 Code Size: 44 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0 Spilled VGPRs: 0 PrivMem VGPRs: 0 GL CALLBACK: type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS: 16 VGPRS: 8 Code Size: 80 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0 Spilled VGPRs: 0 PrivMem VGPRs: 0 GL CALLBACK: type = 0x8251, severity = 0x826b, message = LLVM diagnostic (remark): :0:0: 2 instructions in function GL CALLBACK: type = 0x8251, severity = 0x826b, message = LLVM diagnostic (remark): :0:0: 3 instructions in function GL CALLBACK: type = 0x8251, severity = 0x826b, message = LLVM diagnostic (remark): :0:0: 4 instructions in function GL CALLBACK: type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS: 16 VGPRS: 24 Code Size: 44 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0 Spilled VGPRs: 0 PrivMem VGPRs: 0 GL CALLBACK: type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS: 16 VGPRS: 8 Code Size: 136 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0 Spilled VGPRs: 0 PrivMem VGPRs: 0 GL CALLBACK: type = 0x8251, severity = 0x826b, message = LLVM diagnostic (remark): :0:0: 16 instructions in function GL CALLBACK: type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS: 24 VGPRS: 24 Code Size: 92 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0 Spilled VGPRs: 0 PrivMem VGPRs: 0 GL CALLBACK: type = 0x8251, severity = 0x826b, message = Shader Stats: SGPRS: 24 VGPRS: 24 Code Size: 88 LDS: 0 Scratch: 0 Max Waves: 10 Spilled SGPRs: 0 Spilled VGPRs: 0 PrivMem VGPRs: 0 Minetest will take longer to test as the pkgbuild doesn't enable asserts, and also because of adformentioned $dayjob. I guess in that case I'd only know if I saw garbled output; it was never very consistent when it occured but would always be during the loading bar screen (but when it did happen some very colourful blocky corruption would result). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller
On Mon, Jul 22, 2019 at 01:12:40PM -0700, Doug Anderson wrote: > Hi, > > On Mon, Jul 22, 2019 at 11:19 AM Matthias Kaehlcke wrote: > > > > The DDC/CI protocol involves sending a multi-byte request to the > > display via I2C, which is typically followed by a multi-byte > > response. The internal I2C controller only allows single byte > > reads/writes or reads of 8 sequential bytes, hence DDC/CI is not > > supported when the internal I2C controller is used. The I2C > > transfers complete without errors, however the data in the response > > is garbage. Abort transfers to/from slave address 0x37 (DDC) with > > -EOPNOTSUPP, to make it evident that the communication is failing. > > > > Signed-off-by: Matthias Kaehlcke > > --- > > Changes in v2: > > - changed DDC_I2C_ADDR to DDC_CI_ADDR > > --- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > index 045b1b13fd0e..28933629f3c7 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > @@ -35,6 +35,7 @@ > > > > #include > > > > +#define DDC_CI_ADDR0x37 > > #define DDC_SEGMENT_ADDR 0x30 > > > > #define HDMI_EDID_LEN 512 > > @@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap, > > u8 addr = msgs[0].addr; > > int i, ret = 0; > > > > + if (addr == DDC_CI_ADDR) > > + /* > > +* The internal I2C controller does not support the > > multi-byte > > +* read and write operations needed for DDC/CI. > > +*/ > > + return -EOPNOTSUPP; > > + > > In theory we could also solve this by detecting (in other parts of the > function) the bad multi-byte read/write operations. That would maybe > be more generic (AKA it would more properly handle random userspace > tools fiddling with i2c-dev) but add complexity to the code. But how do you know it's an unsupported operation, and not the driver knowing the wacky limitations doing something valid. E.g. you get the sequence: 0x01 0x0a 0x0b 0x0c 0x0d This could be interpreted as "send the above bytes to the slave" or as "send 0x0a to address 0x01, 0x0b to 0x02, 0x0c to 0x03 and 0x0d to 0x04" (at least by this driver ;-) . The latter could be intended. > ...possibly a better long-term solution is to just totally stop > emulating a regular i2c adapter when the hardware just doesn't support > that. In theory we could remove the "drm_get_edid()" call in > dw_hdmi_connector_get_modes() and replace it with a direct call to > drm_do_get_edid() if we're using the built-in adapter. Possibly > "tda998x_drv.c" would be a good reference. If we did that, we could > remove all the weird / hacky i2c code in this driver. yes, that would be another and probably better option than trying to detect unsupported transaction. > Since the bigger cleanup seems like a bit much to ask, I'm OK with > this as a stopgap to at least prevent userspace tools from getting > confused. Thus: > > Reviewed-by: Douglas Anderson Thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/4] drm/via: drop use of deprecated headers drmP.h and drm_os_linux.h
Added subject On Mon, Jul 22, 2019 at 10:35:41PM +0200, Sam Ravnborg wrote: > The first three patches prepare for the removal of drmP.h. > The last patch remove use of drmP.h and replace with necessary > include files to fix build. > > Build tested with various configs and various architectures. > > I had preferred that the via driver was replaced by the > openchrome driver, but until we have it then we need > to deal with the legacy via driver when removing old cruft > in the drm subsystem. > > v3: > - Use static inline functions for the read/write operations (Emil) > - Use dedicated *_mask_or() and *_mask_and() (Emil) > - Replace DRM_WAIT_ON in same path that introduces VIA_WAIT_ON (Emil) > - Collected r-b's > - Changelog adjustments > - Rebased on top of drm-misc-next > > v2: > - Add a copy of DRM_WAIT_ON to the via driver, keeping > the changes to this legacy driver to a minimum. > This also gives much more confidence that the > driver continues to work as there is no changes > in logic. Therefore dropped "RFT". > - Added Cc: Michel Dänzer to all > patches, as Michael have commented on the series. > > Sam > > Sam Ravnborg (4): > drm/via: drop use of DRM(READ|WRITE) macros > drm/via: copy DRM_WAIT_ON as VIA_WAIT_ON and use it > drm/via: make via_drv.h self-contained > drm/via: drop use of drmP.h > > drivers/gpu/drm/via/via_dma.c | 43 +++- > drivers/gpu/drm/via/via_dmablit.c | 41 ++- > drivers/gpu/drm/via/via_drv.c | 7 +++- > drivers/gpu/drm/via/via_drv.h | 83 > +++--- > drivers/gpu/drm/via/via_irq.c | 54 + > drivers/gpu/drm/via/via_map.c | 6 ++- > drivers/gpu/drm/via/via_mm.c | 7 +++- > drivers/gpu/drm/via/via_verifier.c | 22 +- > drivers/gpu/drm/via/via_video.c| 5 ++- > 9 files changed, 182 insertions(+), 86 deletions(-) > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/9] drm/tinydrm/mipi-dbi: Select DRM_KMS_HELPER
On 7/22/19 5:43 AM, Noralf Trønnes wrote: mipi-dbi uses several KMS helper functions but that build dependency is not expressed. Select DRM_KMS_HELPER to fix that. Reported-by: kbuild test robot Signed-off-by: Noralf Trønnes --- Reviewed-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/4] drm/via: drop use of DRM(READ|WRITE) macros
The DRM_READ, DRM_WRITE macros comes from the deprecated drm_os_linux.h header file. Remove their use to remove this dependency. Replace the use of the macros with static inline variants. v3: - Use static inline functions, rather than macros (Emil) - Use dedicated mask variants for byte access (Emil) Signed-off-by: Sam Ravnborg Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Emil Velikov Cc: Michel Dänzer --- drivers/gpu/drm/via/via_dma.c | 34 +++--- drivers/gpu/drm/via/via_dmablit.c | 24 drivers/gpu/drm/via/via_drv.h | 40 ++ drivers/gpu/drm/via/via_irq.c | 46 +++--- drivers/gpu/drm/via/via_verifier.c | 12 5 files changed, 92 insertions(+), 64 deletions(-) diff --git a/drivers/gpu/drm/via/via_dma.c b/drivers/gpu/drm/via/via_dma.c index d17d8f245c1a..344a12b63967 100644 --- a/drivers/gpu/drm/via/via_dma.c +++ b/drivers/gpu/drm/via/via_dma.c @@ -430,14 +430,14 @@ static int via_hook_segment(drm_via_private_t *dev_priv, diff = (uint32_t) (ptr - reader) - dev_priv->dma_diff; count = 1000; while (diff == 0 && count--) { - paused = (VIA_READ(0x41c) & 0x8000); + paused = (via_read(dev_priv, 0x41c) & 0x8000); if (paused) break; reader = *(dev_priv->hw_addr_ptr); diff = (uint32_t) (ptr - reader) - dev_priv->dma_diff; } - paused = VIA_READ(0x41c) & 0x8000; + paused = via_read(dev_priv, 0x41c) & 0x8000; if (paused && !no_pci_fire) { reader = *(dev_priv->hw_addr_ptr); @@ -454,10 +454,10 @@ static int via_hook_segment(drm_via_private_t *dev_priv, * doesn't make a difference. */ - VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16)); - VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_hi); - VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_lo); - VIA_READ(VIA_REG_TRANSPACE); + via_write(dev_priv, VIA_REG_TRANSET, (HC_ParaType_PreCR << 16)); + via_write(dev_priv, VIA_REG_TRANSPACE, pause_addr_hi); + via_write(dev_priv, VIA_REG_TRANSPACE, pause_addr_lo); + via_read(dev_priv, VIA_REG_TRANSPACE); } } return paused; @@ -467,10 +467,10 @@ static int via_wait_idle(drm_via_private_t *dev_priv) { int count = 1000; - while (!(VIA_READ(VIA_REG_STATUS) & VIA_VR_QUEUE_BUSY) && --count) + while (!(via_read(dev_priv, VIA_REG_STATUS) & VIA_VR_QUEUE_BUSY) && --count) ; - while (count && (VIA_READ(VIA_REG_STATUS) & + while (count && (via_read(dev_priv, VIA_REG_STATUS) & (VIA_CMD_RGTR_BUSY | VIA_2D_ENG_BUSY | VIA_3D_ENG_BUSY))) --count; @@ -536,21 +536,21 @@ static void via_cmdbuf_start(drm_via_private_t *dev_priv) via_flush_write_combine(); (void) *(volatile uint32_t *)dev_priv->last_pause_ptr; - VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16)); - VIA_WRITE(VIA_REG_TRANSPACE, command); - VIA_WRITE(VIA_REG_TRANSPACE, start_addr_lo); - VIA_WRITE(VIA_REG_TRANSPACE, end_addr_lo); + via_write(dev_priv, VIA_REG_TRANSET, (HC_ParaType_PreCR << 16)); + via_write(dev_priv, VIA_REG_TRANSPACE, command); + via_write(dev_priv, VIA_REG_TRANSPACE, start_addr_lo); + via_write(dev_priv, VIA_REG_TRANSPACE, end_addr_lo); - VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_hi); - VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_lo); + via_write(dev_priv, VIA_REG_TRANSPACE, pause_addr_hi); + via_write(dev_priv, VIA_REG_TRANSPACE, pause_addr_lo); wmb(); - VIA_WRITE(VIA_REG_TRANSPACE, command | HC_HAGPCMNT_MASK); - VIA_READ(VIA_REG_TRANSPACE); + via_write(dev_priv, VIA_REG_TRANSPACE, command | HC_HAGPCMNT_MASK); + via_read(dev_priv, VIA_REG_TRANSPACE); dev_priv->dma_diff = 0; count = 1000; - while (!(VIA_READ(0x41c) & 0x8000) && count--); + while (!(via_read(dev_priv, 0x41c) & 0x8000) && count--); reader = *(dev_priv->hw_addr_ptr); ptr = ((volatile char *)dev_priv->last_pause_ptr - dev_priv->dma_ptr) + diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c index 062067438f1d..e1557dd67083 100644 --- a/drivers/gpu/drm/via/via_dmablit.c +++ b/drivers/gpu/drm/via/via_dmablit.c @@ -214,16 +214,16 @@ via_fire_dmablit(struct drm_device *dev, drm_via_sg_info_t *vsg, int engine) { drm_via_private_t *dev_priv = (drm_via_private_t *)dev->dev_private; - VIA_WRITE(VIA_PCI_DMA_MAR0 + engine*0x10, 0); - VIA_WRI
[PATCH v3 3/4] drm/via: make via_drv.h self-contained
Added include of header files to make via_drv.h self-contained. v3: - Reworded changelog a little - to reflect that more than one header files are added Signed-off-by: Sam Ravnborg Reviewed-by: Emil Velikov Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Michel Dänzer --- drivers/gpu/drm/via/via_drv.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/via/via_drv.h b/drivers/gpu/drm/via/via_drv.h index 3eb92e81655e..88deb9d8b765 100644 --- a/drivers/gpu/drm/via/via_drv.h +++ b/drivers/gpu/drm/via/via_drv.h @@ -24,13 +24,16 @@ #ifndef _VIA_DRV_H_ #define _VIA_DRV_H_ +#include #include #include #include #include +#include #include #include +#include #define DRIVER_AUTHOR "Various" -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/9] drm/tinydrm/mipi-dbi: Remove CMA helper dependency
On 7/22/19 5:43 AM, Noralf Trønnes wrote: mipi-dbi depends on the CMA helper through it's use of drm_fb_cma_get_gem_obj(). This is an unnecessary dependency to drag in for drivers that only want to use the MIPI DBI interface part. Avoid this by open coding the function. Signed-off-by: Noralf Trønnes --- Reviewed-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[no subject]
The first three patches prepare for the removal of drmP.h. The last patch remove use of drmP.h and replace with necessary include files to fix build. Build tested with various configs and various architectures. I had preferred that the via driver was replaced by the openchrome driver, but until we have it then we need to deal with the legacy via driver when removing old cruft in the drm subsystem. v3: - Use static inline functions for the read/write operations (Emil) - Use dedicated *_mask_or() and *_mask_and() (Emil) - Replace DRM_WAIT_ON in same path that introduces VIA_WAIT_ON (Emil) - Collected r-b's - Changelog adjustments - Rebased on top of drm-misc-next v2: - Add a copy of DRM_WAIT_ON to the via driver, keeping the changes to this legacy driver to a minimum. This also gives much more confidence that the driver continues to work as there is no changes in logic. Therefore dropped "RFT". - Added Cc: Michel Dänzer to all patches, as Michael have commented on the series. Sam Sam Ravnborg (4): drm/via: drop use of DRM(READ|WRITE) macros drm/via: copy DRM_WAIT_ON as VIA_WAIT_ON and use it drm/via: make via_drv.h self-contained drm/via: drop use of drmP.h drivers/gpu/drm/via/via_dma.c | 43 +++- drivers/gpu/drm/via/via_dmablit.c | 41 ++- drivers/gpu/drm/via/via_drv.c | 7 +++- drivers/gpu/drm/via/via_drv.h | 83 +++--- drivers/gpu/drm/via/via_irq.c | 54 + drivers/gpu/drm/via/via_map.c | 6 ++- drivers/gpu/drm/via/via_mm.c | 7 +++- drivers/gpu/drm/via/via_verifier.c | 22 +- drivers/gpu/drm/via/via_video.c| 5 ++- 9 files changed, 182 insertions(+), 86 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/4] drm/via: drop use of drmP.h
Drop use of the deprecated drmP.h header. While touching the files divide include files in blocks and sort the files alphabetically. v2: - Replace all uses of DRM_WAIT_ON() with VIA_WAIT_ON() and thus avoiding to pull in drm_os_linux.h v3: - DRM_WAIT_ON replacement moved to earlier patch (Emil) Signed-off-by: Sam Ravnborg Reviewed-by: Emil Velikov Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Michel Dänzer --- drivers/gpu/drm/via/via_dma.c | 9 - drivers/gpu/drm/via/via_dmablit.c | 13 - drivers/gpu/drm/via/via_drv.c | 7 +-- drivers/gpu/drm/via/via_irq.c | 4 +++- drivers/gpu/drm/via/via_map.c | 6 +- drivers/gpu/drm/via/via_mm.c | 7 ++- drivers/gpu/drm/via/via_verifier.c | 10 +- drivers/gpu/drm/via/via_video.c| 3 ++- 8 files changed, 42 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/via/via_dma.c b/drivers/gpu/drm/via/via_dma.c index 344a12b63967..1208445e341d 100644 --- a/drivers/gpu/drm/via/via_dma.c +++ b/drivers/gpu/drm/via/via_dma.c @@ -34,8 +34,15 @@ *Thomas Hellstrom. */ -#include +#include +#include + +#include +#include +#include +#include #include + #include "via_drv.h" #include "via_3d_reg.h" diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c index 2ec93d976a43..feaa538026a0 100644 --- a/drivers/gpu/drm/via/via_dmablit.c +++ b/drivers/gpu/drm/via/via_dmablit.c @@ -34,13 +34,16 @@ * the same DMA mappings? */ -#include -#include -#include "via_drv.h" -#include "via_dmablit.h" - #include #include +#include + +#include +#include +#include + +#include "via_dmablit.h" +#include "via_drv.h" #define VIA_PGDN(x) (((unsigned long)(x)) & PAGE_MASK) #define VIA_PGOFF(x) (((unsigned long)(x)) & ~PAGE_MASK) diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c index af6a12d3c058..666a16de84f9 100644 --- a/drivers/gpu/drm/via/via_drv.c +++ b/drivers/gpu/drm/via/via_drv.c @@ -24,11 +24,14 @@ #include -#include +#include +#include +#include +#include #include + #include "via_drv.h" -#include static int via_driver_open(struct drm_device *dev, struct drm_file *file) { diff --git a/drivers/gpu/drm/via/via_irq.c b/drivers/gpu/drm/via/via_irq.c index 5ac26d203f44..07a8d6b95a68 100644 --- a/drivers/gpu/drm/via/via_irq.c +++ b/drivers/gpu/drm/via/via_irq.c @@ -35,8 +35,10 @@ * The refresh rate is also calculated for video playback sync purposes. */ -#include +#include +#include #include + #include "via_drv.h" #define VIA_REG_INTERRUPT 0x200 diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c index 2ad865870372..431c150df014 100644 --- a/drivers/gpu/drm/via/via_map.c +++ b/drivers/gpu/drm/via/via_map.c @@ -21,8 +21,12 @@ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER * DEALINGS IN THE SOFTWARE. */ -#include + +#include +#include +#include #include + #include "via_drv.h" static int via_do_init_map(struct drm_device *dev, drm_via_init_t *init) diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c index 4217d66a5cc6..45cc9e900260 100644 --- a/drivers/gpu/drm/via/via_mm.c +++ b/drivers/gpu/drm/via/via_mm.c @@ -25,8 +25,13 @@ * Authors: Thomas Hellström */ -#include +#include + +#include +#include +#include #include + #include "via_drv.h" #define VIA_MM_ALIGN_SHIFT 4 diff --git a/drivers/gpu/drm/via/via_verifier.c b/drivers/gpu/drm/via/via_verifier.c index 002c883b0d4b..8d8135f424ef 100644 --- a/drivers/gpu/drm/via/via_verifier.c +++ b/drivers/gpu/drm/via/via_verifier.c @@ -28,13 +28,13 @@ * be very slow. */ -#include "via_3d_reg.h" -#include -#include +#include #include -#include "via_verifier.h" +#include + +#include "via_3d_reg.h" #include "via_drv.h" -#include +#include "via_verifier.h" typedef enum { state_command, diff --git a/drivers/gpu/drm/via/via_video.c b/drivers/gpu/drm/via/via_video.c index cf53612c945e..53b1f58f99b4 100644 --- a/drivers/gpu/drm/via/via_video.c +++ b/drivers/gpu/drm/via/via_video.c @@ -25,8 +25,9 @@ * Video and XvMC related functions. */ -#include +#include #include + #include "via_drv.h" void via_init_futex(drm_via_private_t *dev_priv) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 6/9] drm/tinydrm/Kconfig: drivers: Select BACKLIGHT_CLASS_DEVICE
On 7/22/19 5:43 AM, Noralf Trønnes wrote: The mipi_dbi helper is missing a dependency on DRM_KMS_HELPER and putting that in revealed this problem: drivers/video/fbdev/Kconfig:12:error: recursive dependency detected! drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER drivers/gpu/drm/Kconfig:75: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER drivers/gpu/drm/Kconfig:69: symbol DRM_KMS_HELPER is selected by TINYDRM_MIPI_DBI drivers/gpu/drm/tinydrm/Kconfig:11: symbol TINYDRM_MIPI_DBI is selected by TINYDRM_HX8357D drivers/gpu/drm/tinydrm/Kconfig:15: symbol TINYDRM_HX8357D depends on BACKLIGHT_CLASS_DEVICE drivers/video/backlight/Kconfig:144:symbol BACKLIGHT_CLASS_DEVICE is selected by FB_BACKLIGHT drivers/video/fbdev/Kconfig:187:symbol FB_BACKLIGHT depends on FB A symbol that selects DRM_KMS_HELPER can not depend on BACKLIGHT_CLASS_DEVICE. The reason for this is that DRM_KMS_FB_HELPER selects FB instead of depending on it. The tinydrm drivers have somehow gotten away with depending on BACKLIGHT_CLASS_DEVICE because DRM_TINYDRM selects DRM_KMS_HELPER and the drivers depend on that symbol. An audit shows that all DRM drivers that select DRM_KMS_HELPER and use BACKLIGHT_CLASS_DEVICE, selects it: DRM_TILCDC, DRM_GMA500, DRM_SHMOBILE, DRM_NOUVEAU, DRM_FSL_DCU, DRM_I915, DRM_RADEON, DRM_AMDGPU, DRM_PARADE_PS8622 Documentation/kbuild/kconfig-language.txt has a note regarding select: 1. 'select should be used with care since it doesn't visit dependencies.' This is not a problem since BACKLIGHT_CLASS_DEVICE doesn't have any dependencies. 2. 'In general use select only for non-visible symbols' BACKLIGHT_CLASS_DEVICE is user visible. The real solution to this would be to have DRM_KMS_FB_HELPER depend on the user visible symbol FB. That is a can of worms I'm not willing to tackle. I fear that such a change will result in me handling difficult fallouts for the next weeks. So I'm following DRM suite here. Signed-off-by: Noralf Trønnes --- Reviewed-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/4] drm/via: copy DRM_WAIT_ON as VIA_WAIT_ON and use it
VIA_WAIT_ON() is a direct copy of DRM_WAIT_ON() from drm_os_linux.h. The copy is made so we can avoid the dependency on the legacy header. A more involved approach had been to introduce wait_event_* but for this legacy driver the simpler and more safe approach with a copy of the macro was selected. Added the relevant header files for the functions used in VIA_WAIT_ON. v3: - Updated users of DRM_WAIT_ON => VIA_WAIT_ON (Emil) - Updated $subject (Emil) Signed-off-by: Sam Ravnborg Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Emil Velikov Cc: Michel Dänzer --- drivers/gpu/drm/via/via_dmablit.c | 4 +-- drivers/gpu/drm/via/via_drv.h | 42 ++- drivers/gpu/drm/via/via_irq.c | 4 +-- drivers/gpu/drm/via/via_video.c | 2 +- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c index e1557dd67083..2ec93d976a43 100644 --- a/drivers/gpu/drm/via/via_dmablit.c +++ b/drivers/gpu/drm/via/via_dmablit.c @@ -436,7 +436,7 @@ via_dmablit_sync(struct drm_device *dev, uint32_t handle, int engine) int ret = 0; if (via_dmablit_active(blitq, engine, handle, &queue)) { - DRM_WAIT_ON(ret, *queue, 3 * HZ, + VIA_WAIT_ON(ret, *queue, 3 * HZ, !via_dmablit_active(blitq, engine, handle, NULL)); } DRM_DEBUG("DMA blit sync handle 0x%x engine %d returned %d\n", @@ -687,7 +687,7 @@ via_dmablit_grab_slot(drm_via_blitq_t *blitq, int engine) while (blitq->num_free == 0) { spin_unlock_irqrestore(&blitq->blit_lock, irqsave); - DRM_WAIT_ON(ret, blitq->busy_queue, HZ, blitq->num_free > 0); + VIA_WAIT_ON(ret, blitq->busy_queue, HZ, blitq->num_free > 0); if (ret) return (-EINTR == ret) ? -EAGAIN : ret; diff --git a/drivers/gpu/drm/via/via_drv.h b/drivers/gpu/drm/via/via_drv.h index 8c16c208ae83..3eb92e81655e 100644 --- a/drivers/gpu/drm/via/via_drv.h +++ b/drivers/gpu/drm/via/via_drv.h @@ -24,8 +24,13 @@ #ifndef _VIA_DRV_H_ #define _VIA_DRV_H_ -#include +#include +#include +#include +#include + #include +#include #define DRIVER_AUTHOR "Various" @@ -148,6 +153,41 @@ static inline void via_write8_mask_and(struct drm_via_private *dev_priv, writeb(val & mask, (void __iomem *)(dev_priv->mmio->handle + reg)); } +/* + * Poll in a loop waiting for 'contidition' to be true. + * Note: A direct replacement with wait_event_interruptible_timeout() + * will not work unless driver is updated to emit wake_up() + * in relevant places that can impact the 'condition' + * + * Returns: + * ret keeps current value if 'condition' becomes true + * ret = -BUSY if timeout happens + * ret = -EINTR if a signal interrupted the waiting period + */ +#define VIA_WAIT_ON( ret, queue, timeout, condition ) \ +do { \ + DECLARE_WAITQUEUE(entry, current); \ + unsigned long end = jiffies + (timeout);\ + add_wait_queue(&(queue), &entry); \ + \ + for (;;) { \ + __set_current_state(TASK_INTERRUPTIBLE);\ + if (condition) \ + break; \ + if (time_after_eq(jiffies, end)) { \ + ret = -EBUSY; \ + break; \ + } \ + schedule_timeout((HZ/100 > 1) ? HZ/100 : 1);\ + if (signal_pending(current)) { \ + ret = -EINTR; \ + break; \ + } \ + } \ + __set_current_state(TASK_RUNNING); \ + remove_wait_queue(&(queue), &entry);\ +} while (0) + extern const struct drm_ioctl_desc via_ioctls[]; extern int via_max_ioctl; diff --git a/drivers/gpu/drm/via/via_irq.c b/drivers/gpu/drm/via/via_irq.c index 0ee0b767c3d7..5ac26d203f44 100644 --- a/drivers/gpu/drm/via/via_irq.c +++ b/drivers/gpu/drm/via/via_irq.c @@ -233,12 +233,12 @@ via_driver_irq_wait(struct drm_device *dev, unsigned int irq, int force_sequence cur_irq = dev_priv->via_irqs + real_irq; if (masks[real_irq][2] && !force_sequence) { - DRM_WAIT_ON(ret, cur_irq->irq_queue, 3 * HZ, + VIA_WAIT_ON(ret, cur_irq
[PATCH] video: fbdev: pvr2fb: remove unnecessary comparison of unsigned integer with < 0
There is no need to compare *var->xoffset* or *var->yoffset* with < 0 because such variables are of type unsigned, making it impossible to hold a negative value. Fix this by removing such comparisons. Addresses-Coverity-ID: 1451964 ("Unsigned compared against 0") Signed-off-by: Gustavo A. R. Silva --- drivers/video/fbdev/pvr2fb.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c index 7ff4b6b84282..0a3b2b7c7891 100644 --- a/drivers/video/fbdev/pvr2fb.c +++ b/drivers/video/fbdev/pvr2fb.c @@ -458,13 +458,11 @@ static int pvr2fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) set_color_bitfields(var); if (var->vmode & FB_VMODE_YWRAP) { - if (var->xoffset || var->yoffset < 0 || - var->yoffset >= var->yres_virtual) { + if (var->xoffset || var->yoffset >= var->yres_virtual) { var->xoffset = var->yoffset = 0; } else { if (var->xoffset > var->xres_virtual - var->xres || - var->yoffset > var->yres_virtual - var->yres || - var->xoffset < 0 || var->yoffset < 0) + var->yoffset > var->yres_virtual - var->yres) var->xoffset = var->yoffset = 0; } } else { -- 2.22.0
Re: [PATCH v2 9/9] MAINTAINERS: Remove tinydrm entry
On 7/22/19 5:43 AM, Noralf Trønnes wrote: tinydrm is just a collection of tiny drivers now. Add T: drm-misc entry for tinydrm drivers that lacks it. Cc: David Lechner Reviewed-by: Sam Ravnborg Signed-off-by: Noralf Trønnes --- Acked-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 8/9] drm/tinydrm: Move mipi-dbi
On 7/22/19 5:43 AM, Noralf Trønnes wrote: This moves mipi-dbi to be a core helper with the name drm_mipi_dbi. Fixup include's in drivers. Move the docs entry and delete tinydrm.rst. Delete the last tinydrm todo entry. v2: Make DRM_MIPI_DBI tristate to enable it being built as a module. Cc: Eric Anholt Cc: David Lechner Reviewed-by: Sam Ravnborg Signed-off-by: Noralf Trønnes --- Acked-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110671] Regression: DP outputs out of sync on dual-DP tiled 5k screen
https://bugs.freedesktop.org/show_bug.cgi?id=110671 --- Comment #9 from Denys --- Broken in 5.3-rc1 Workaround to revert 5fc0cbfad4564856ee0f323d3f88a7cff19cc3f1 is still working. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/9] drm/tinydrm: Split struct mipi_dbi in two
On 7/22/19 5:43 AM, Noralf Trønnes wrote: Split struct mipi_dbi into an interface part and a display pipeline part. The interface part can be used by drivers that need to initialize the controller, but that won't upload the framebuffer over this interface. MIPI DBI supports 3 interface types: - A. Motorola 6800 type parallel bus - B. Intel 8080 type parallel bus - C. SPI type with 3 options: I've embedded the SPI type specifics in the mipi_dbi struct to avoid adding unnecessary complexity. If more interface types will be supported in the future, the type specifics might have to be split out. Rename functions to match the new struct mipi_dbi_dev: - drm_to_mipi_dbi() -> drm_to_mipi_dbi_dev(). - mipi_dbi_init*() -> mipi_dbi_dev_init*(). Cc: Eric Anholt Cc: David Lechner Reviewed-by: Sam Ravnborg Signed-off-by: Noralf Trønnes --- Acked-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Enable backlight when trigger is activated
Hi Pavel, On 7/22/19 9:50 AM, Pavel Machek wrote: > Hi! > >>> Configuring backlight trigger from dts results in backlight off during >>> boot. Machine looks dead upon boot, which is not good. >>> >>> Fix that by enabling LED on trigger activation. > >>> +++ b/drivers/leds/trigger/ledtrig-backlight.c >>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led) >>> n->old_status = UNBLANK; >>> n->notifier.notifier_call = fb_notifier_callback; >>> >>> + led_set_brightness(led, LED_ON); >>> + >> >> This looks fishy. >> >> Maybe you should use a default-state = "keep" instead? (and you'll have >> to support it in the LED driver). >> >> That'll give you proper "don't touch the LED if it was turned on" behavior, >> which is what you seem to want. > > Actually no, that's not what I want. LED should go on if the display > is active, as soon as trigger is activated. > > Unfortunately, I have see no good way to tell if the display is > active (and display is usually active when trigger is activated). default-state DT property can be also set to "on" (see Documentation/devicetree/bindings/leds/common.txt). You could make use of LED_INIT_DEFAULT_TRIGGER flag and parse DT property in the activate op. Similar approach has been applied e.g. in ledtrig-pattern.c. -- Best regards, Jacek Anaszewski
Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller
On Mon, Jul 22, 2019 at 11:19:45AM -0700, Matthias Kaehlcke wrote: > The DDC/CI protocol involves sending a multi-byte request to the > display via I2C, which is typically followed by a multi-byte > response. The internal I2C controller only allows single byte > reads/writes or reads of 8 sequential bytes, hence DDC/CI is not > supported when the internal I2C controller is used. The I2C This is very likely a stupid question, but I didn't see an answer for it, so I'll just ask :) If the controller supports xfers of 8 bytes and 1 bytes, could you just split up any of these transactions into len/8+len%8 transactions? Sean > transfers complete without errors, however the data in the response > is garbage. Abort transfers to/from slave address 0x37 (DDC) with > -EOPNOTSUPP, to make it evident that the communication is failing. > > Signed-off-by: Matthias Kaehlcke > --- > Changes in v2: > - changed DDC_I2C_ADDR to DDC_CI_ADDR > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 045b1b13fd0e..28933629f3c7 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -35,6 +35,7 @@ > > #include > > +#define DDC_CI_ADDR 0x37 > #define DDC_SEGMENT_ADDR 0x30 > > #define HDMI_EDID_LEN512 > @@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap, > u8 addr = msgs[0].addr; > int i, ret = 0; > > + if (addr == DDC_CI_ADDR) > + /* > + * The internal I2C controller does not support the multi-byte > + * read and write operations needed for DDC/CI. > + */ > + return -EOPNOTSUPP; > + > dev_dbg(hdmi->dev, "xfer: num: %d, addr: %#x\n", num, addr); > > for (i = 0; i < num; i++) { > -- > 2.22.0.657.g960e92d24f-goog > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/9] drm/tinydrm: Rename remaining variable mipi -> dbidev
On 7/22/19 5:43 AM, Noralf Trønnes wrote: struct mipi_dbi is going to be split into an interface part and a display pipeline part. The interface part can be used by drivers that need to initialize the controller, but that won't upload the framebuffer over this interface. tinydrm uses the variable name 'mipi' but this is not a good name since MIPI refers to a lot of standards. This patch changes the variable name to 'dbidev' where it refers to the pipeline part of struct mipi_dbi. Cc: Eric Anholt Cc: David Lechner Reviewed-by: Sam Ravnborg Signed-off-by: Noralf Trønnes --- Acked-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/9] drm/tinydrm: Rename variable mipi -> dbi
On 7/22/19 5:43 AM, Noralf Trønnes wrote: struct mipi_dbi is going to be split into an interface part and a display pipeline part. The interface part can be used by drivers that need to initialize the controller, but that won't upload the framebuffer over this interface. tinydrm uses the variable name 'mipi' but this is not a good name since MIPI refers to a lot of standards. This patch changes the variable name to 'dbi' where it refers to the interface part of struct mipi_dbi. Functions that use both future parts will have both variables temporarily pointing to the same structure. Cc: Eric Anholt Cc: David Lechner Reviewed-by: Sam Ravnborg Signed-off-by: Noralf Trønnes --- Acked-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105251] [Vega10] GPU lockup on boot: VMC page fault
https://bugs.freedesktop.org/show_bug.cgi?id=105251 --- Comment #74 from deltasquared --- (In reply to Juan A. Suarez from comment #73) > It could be good if people could report here if this improved with this MR. I can utilise the mesa-git package in the arch user repository to compile from latest sources. I will then test both vega_crasher and minetest with that package installed to see what occurs. Stay tuned for updates, though it may take a couple of days while I juggle $dayjob. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller
Hi, On Mon, Jul 22, 2019 at 11:19 AM Matthias Kaehlcke wrote: > > The DDC/CI protocol involves sending a multi-byte request to the > display via I2C, which is typically followed by a multi-byte > response. The internal I2C controller only allows single byte > reads/writes or reads of 8 sequential bytes, hence DDC/CI is not > supported when the internal I2C controller is used. The I2C > transfers complete without errors, however the data in the response > is garbage. Abort transfers to/from slave address 0x37 (DDC) with > -EOPNOTSUPP, to make it evident that the communication is failing. > > Signed-off-by: Matthias Kaehlcke > --- > Changes in v2: > - changed DDC_I2C_ADDR to DDC_CI_ADDR > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 045b1b13fd0e..28933629f3c7 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -35,6 +35,7 @@ > > #include > > +#define DDC_CI_ADDR0x37 > #define DDC_SEGMENT_ADDR 0x30 > > #define HDMI_EDID_LEN 512 > @@ -322,6 +323,13 @@ static int dw_hdmi_i2c_xfer(struct i2c_adapter *adap, > u8 addr = msgs[0].addr; > int i, ret = 0; > > + if (addr == DDC_CI_ADDR) > + /* > +* The internal I2C controller does not support the multi-byte > +* read and write operations needed for DDC/CI. > +*/ > + return -EOPNOTSUPP; > + In theory we could also solve this by detecting (in other parts of the function) the bad multi-byte read/write operations. That would maybe be more generic (AKA it would more properly handle random userspace tools fiddling with i2c-dev) but add complexity to the code. ...possibly a better long-term solution is to just totally stop emulating a regular i2c adapter when the hardware just doesn't support that. In theory we could remove the "drm_get_edid()" call in dw_hdmi_connector_get_modes() and replace it with a direct call to drm_do_get_edid() if we're using the built-in adapter. Possibly "tda998x_drv.c" would be a good reference. If we did that, we could remove all the weird / hacky i2c code in this driver. Since the bigger cleanup seems like a bit much to ask, I'm OK with this as a stopgap to at least prevent userspace tools from getting confused. Thus: Reviewed-by: Douglas Anderson ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: Improve the help text for DRM_ANALOGIX_ANX78XX
On Mon, Jul 22, 2019 at 04:40:49PM -0300, Fabio Estevam wrote: > Improve the help text for DRM_ANALOGIX_ANX78XX by adding the missing > "power" word. > > After this change the help text matches with the ANX7814 > product description from the Analogix website: > > https://www.analogix.com/en/products/convertersbridges/anx7814 > > Signed-off-by: Fabio Estevam Thanks for your patch! I've applied it to drm-misc-next Sean > --- > drivers/gpu/drm/bridge/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index ee777469293a..a6eec908c43e 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -21,7 +21,7 @@ config DRM_ANALOGIX_ANX78XX > select DRM_KMS_HELPER > select REGMAP_I2C > ---help--- > - ANX78XX is an ultra-low Full-HD SlimPort transmitter > + ANX78XX is an ultra-low power Full-HD SlimPort transmitter > designed for portable devices. The ANX78XX transforms > the HDMI output of an application processor to MyDP > or DisplayPort. > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: silence variable 'conn' set but not used
On Mon, Jul 22, 2019 at 03:14:46PM -0400, Qian Cai wrote: > The "struct drm_connector" iteration cursor from > "for_each_new_connector_in_state" is never used in atomic_remove_fb() > which generates a compilation warning, > > drivers/gpu/drm/drm_framebuffer.c: In function 'atomic_remove_fb': > drivers/gpu/drm/drm_framebuffer.c:838:24: warning: variable 'conn' set > but not used [-Wunused-but-set-variable] > > Silence it by marking "conn" __maybe_unused. > > Signed-off-by: Qian Cai Thanks for your patch! I've applied it to drm-misc-fixes Sean > --- > drivers/gpu/drm/drm_framebuffer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_framebuffer.c > b/drivers/gpu/drm/drm_framebuffer.c > index 0b72468e8131..57564318ceea 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -835,7 +835,7 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) > struct drm_device *dev = fb->dev; > struct drm_atomic_state *state; > struct drm_plane *plane; > - struct drm_connector *conn; > + struct drm_connector *conn __maybe_unused; > struct drm_connector_state *conn_state; > int i, ret; > unsigned plane_mask; > -- > 1.8.3.1 > -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH] drm/amdkfd: Fix missing break in switch statement
On 7/22/19 2:10 PM, Alex Deucher wrote: > On Sun, Jul 21, 2019 at 6:12 PM Gustavo A. R. Silva > wrote: >> >> Add missing break statement in order to prevent the code from falling >> through to case CHIP_NAVI10. >> >> This bug was found thanks to the ongoing efforts to enable >> -Wimplicit-fallthrough. >> >> Fixes: 14328aa58ce5 ("drm/amdkfd: Add navi10 support to amdkfd. (v3)") >> Cc: sta...@vger.kernel.org >> Signed-off-by: Gustavo A. R. Silva > > Applied. Thanks! > By the way, Alex, I'm planning to add these fixes to my tree. I want to send a pull-request to Linus for v5.3-rc2 this afternoon. We want to have the -Wimplicit-fallthrough option globally enabled in v5.3, and these are some of the last fall-through warnings remaining in the kernel. Can I have your Ack or Signed-off-by for all these drm patches? Thanks! -- Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
Quoting Brendan Higgins (2019-07-18 17:08:34) > On Thu, Jul 18, 2019 at 12:22:33PM -0700, Brendan Higgins wrote: > > I started poking around with your suggestion while we are waiting. A > couple early observations: > > 1) It is actually easier to do than I previously thought and will probably >help with getting more of the planned TAP output stuff working. > >That being said, this is still a pretty substantial undertaking and >will likely take *at least* a week to implement and properly review. >Assuming everything goes extremely well (no unexpected issues on my >end, very responsive reviewers, etc). > > 2) It *will* eliminate the need for kunit_stream. > > 3) ...but, it *will not* eliminate the need for string_stream. > > Based on my early observations, I do think it is worth doing, but I > don't think it is worth trying to make it in this patchset (unless I > have already missed the window, or it is going to be open for a while): The merge window is over. Typically code needs to be settled a few weeks before it opens (i.e. around -rc4 or -rc5) for most maintainers to pick up patches for the next merge window. > I do think it will make things much cleaner, but I don't think it will > achieve your desired goal of getting rid of an unstructured > {kunit|string}_stream style interface; it just adds a layer on top of it > that makes it harder to misuse. Ok. > > I attached a patch of what I have so far at the end of this email so you > can see what I am talking about. And of course, if you agree with my > assessment, so we can start working on it as a future patch. > > A couple things in regard to the patch I attached: > > 1) I wrote it pretty quickly so there are almost definitely mistakes. >You should consider it RFC. I did verify it compiles though. > > 2) Also, I did use kunit_stream in writing it: all occurences should be >pretty easy to replace with string_stream; nevertheless, the reason >for this is just to make it easier to play with the current APIs. I >wanted to have something working before I went through a big tedious >refactoring. So sorry if it causes any confusion. > > 3) I also based the patch on all the KUnit patches I have queued up >(includes things like mocking and such) since I want to see how this >serialization thing will work with mocks and matchers and things like >that. Great! > > From 53d475d3d56afcf92b452c6d347dbedfa1a17d34 Mon Sep 17 00:00:00 2001 > From: Brendan Higgins > Date: Thu, 18 Jul 2019 16:08:52 -0700 > Subject: [PATCH v1] DO NOT MERGE: started playing around with the > serialization api > > --- > include/kunit/assert.h | 130 ++ > include/kunit/mock.h | 4 + > kunit/Makefile | 3 +- > kunit/assert.c | 179 + > kunit/mock.c | 6 +- > 5 files changed, 318 insertions(+), 4 deletions(-) > create mode 100644 include/kunit/assert.h > create mode 100644 kunit/assert.c > > diff --git a/include/kunit/assert.h b/include/kunit/assert.h > new file mode 100644 > index 0..e054fdff4642f > --- /dev/null > +++ b/include/kunit/assert.h > @@ -0,0 +1,130 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Assertion and expectation serialization API. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#ifndef _KUNIT_ASSERT_H > +#define _KUNIT_ASSERT_H > + > +#include > +#include > + > +enum kunit_assert_type { > + KUNIT_ASSERTION, > + KUNIT_EXPECTATION, > +}; > + > +struct kunit_assert { > + enum kunit_assert_type type; > + const char *line; > + const char *file; > + struct va_format message; > + void (*format)(struct kunit_assert *assert, > + struct kunit_stream *stream); Would passing in the test help too? > +}; > + > +void kunit_base_assert_format(struct kunit_assert *assert, > + struct kunit_stream *stream); > + > +void kunit_assert_print_msg(struct kunit_assert *assert, > + struct kunit_stream *stream); > + > +struct kunit_unary_assert { > + struct kunit_assert assert; > + const char *condition; > + bool expected_true; > +}; > + > +void kunit_unary_assert_format(struct kunit_assert *assert, > + struct kunit_stream *stream); > + > +struct kunit_ptr_not_err_assert { > + struct kunit_assert assert; > + const char *text; > + void *value; > +}; > + > +void kunit_ptr_not_err_assert_format(struct kunit_assert *assert, > +struct kunit_stream *stream); > + > +struct kunit_binary_assert { > + struct kunit_assert assert; > + const char *operation; > + const char *left_text; > + long long left_value; > + const char *right_text; > + long long right_value; > +}; > + > +void kunit_binary_assert_format(struct kunit_assert
Re: Need 5.3-rc1 in drm-misc-next
Den 22.07.2019 21.44, skrev Maxime Ripard: > Hi! > > On Mon, Jul 22, 2019 at 01:20:35PM +0200, Noralf Trønnes wrote: >> Hi drm-misc maintainers, >> >> I have this series: >> >> drm/tinydrm: Remove tinydrm.ko >> https://patchwork.freedesktop.org/series/63811/ >> >> That depends on this -rc1 commit: >> >> e6f3f7e4dc76 ("spi: Add spi_is_bpw_supported()") >> >> I would would be nice if it would show up in drm-misc-next soon. > > I just did it > Thanks Maxime, that was fast! Noralf. > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/11] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats()
On 7/19/19 10:59 AM, Noralf Trønnes wrote: The MIPI DBI standard support more pixel formats than what this helper supports. Add an init function that lets the driver use different format(s). This avoids open coding mipi_dbi_init() in st7586. st7586 sets preferred_depth but this is not necessary since it only supports one format. v2: Forgot to remove the mipi->rotation assignment in st7586, mipi_dbi_init_with_formats() handles it. Cc: David Lechner Acked-by: David Lechner Signed-off-by: Noralf Trønnes --- drivers/gpu/drm/tinydrm/mipi-dbi.c | 91 +- drivers/gpu/drm/tinydrm/st7586.c | 33 ++- include/drm/tinydrm/mipi-dbi.h | 5 ++ 3 files changed, 74 insertions(+), 55 deletions(-) Tested whole series on st7586. Seems to be still working. Just putting Tested-by here since this patch is the only one that changed st7586 significantly. Tested-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdkfd: Fix missing break in switch statement
On 7/22/19 2:45 PM, Alex Deucher wrote: >> >> By the way, Alex, I'm planning to add these fixes to my tree. I want >> to send a pull-request to Linus for v5.3-rc2 this afternoon. We want >> to have the -Wimplicit-fallthrough option globally enabled in v5.3, >> and these are some of the last fall-through warnings remaining in >> the kernel. >> >> Can I have your Ack or Signed-off-by for all these drm patches? > > I didn't realize you were sending these yourself. I was going to > include them in my upcoming -fixes pull. Feel free to add my RB to > all three. > Awesome! :) Thanks -- Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdkfd: Fix missing break in switch statement
On Mon, Jul 22, 2019 at 3:19 PM Gustavo A. R. Silva wrote: > > > > On 7/22/19 2:10 PM, Alex Deucher wrote: > > On Sun, Jul 21, 2019 at 6:12 PM Gustavo A. R. Silva > > wrote: > >> > >> Add missing break statement in order to prevent the code from falling > >> through to case CHIP_NAVI10. > >> > >> This bug was found thanks to the ongoing efforts to enable > >> -Wimplicit-fallthrough. > >> > >> Fixes: 14328aa58ce5 ("drm/amdkfd: Add navi10 support to amdkfd. (v3)") > >> Cc: sta...@vger.kernel.org > >> Signed-off-by: Gustavo A. R. Silva > > > > Applied. Thanks! > > > > By the way, Alex, I'm planning to add these fixes to my tree. I want > to send a pull-request to Linus for v5.3-rc2 this afternoon. We want > to have the -Wimplicit-fallthrough option globally enabled in v5.3, > and these are some of the last fall-through warnings remaining in > the kernel. > > Can I have your Ack or Signed-off-by for all these drm patches? I didn't realize you were sending these yourself. I was going to include them in my upcoming -fixes pull. Feel free to add my RB to all three. Alex ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Need 5.3-rc1 in drm-misc-next
Hi! On Mon, Jul 22, 2019 at 01:20:35PM +0200, Noralf Trønnes wrote: > Hi drm-misc maintainers, > > I have this series: > > drm/tinydrm: Remove tinydrm.ko > https://patchwork.freedesktop.org/series/63811/ > > That depends on this -rc1 commit: > > e6f3f7e4dc76 ("spi: Add spi_is_bpw_supported()") > > I would would be nice if it would show up in drm-misc-next soon. I just did it Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/bridge: Improve the help text for DRM_ANALOGIX_ANX78XX
Improve the help text for DRM_ANALOGIX_ANX78XX by adding the missing "power" word. After this change the help text matches with the ANX7814 product description from the Analogix website: https://www.analogix.com/en/products/convertersbridges/anx7814 Signed-off-by: Fabio Estevam --- drivers/gpu/drm/bridge/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index ee777469293a..a6eec908c43e 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -21,7 +21,7 @@ config DRM_ANALOGIX_ANX78XX select DRM_KMS_HELPER select REGMAP_I2C ---help--- - ANX78XX is an ultra-low Full-HD SlimPort transmitter + ANX78XX is an ultra-low power Full-HD SlimPort transmitter designed for portable devices. The ANX78XX transforms the HDMI output of an application processor to MyDP or DisplayPort. -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm: stop abusing dma_map/unmap for cache
On Sun, Jun 30, 2019 at 05:47:22AM -0700, Rob Clark wrote: > From: Rob Clark > > Recently splats like this started showing up: > >WARNING: CPU: 4 PID: 251 at drivers/iommu/dma-iommu.c:451 > __iommu_dma_unmap+0xb8/0xc0 >Modules linked in: ath10k_snoc ath10k_core fuse msm ath mac80211 uvcvideo > cfg80211 videobuf2_vmalloc videobuf2_memops vide >CPU: 4 PID: 251 Comm: kworker/u16:4 Tainted: GW > 5.2.0-rc5-next-20190619+ #2317 >Hardware name: LENOVO 81JL/LNVNB161216, BIOS 9UCN23WW(V1.06) 10/25/2018 >Workqueue: msm msm_gem_free_work [msm] >pstate: 80c5 (Nzcv daif +PAN +UAO) >pc : __iommu_dma_unmap+0xb8/0xc0 >lr : __iommu_dma_unmap+0x54/0xc0 >sp : 119abce0 >x29: 119abce0 x28: >x27: 8001f9946648 x26: 8001ec271068 >x25: x24: 8001ea3580a8 >x23: 8001f95ba010 x22: 80018e83ba88 >x21: 8001e548f000 x20: f000 >x19: 1000 x18: c1fe >x17: x16: >x15: 15b70068 x14: 0005 >x13: 0003142cc1be1768 x12: 0001 >x11: 8001f6de9100 x10: 0009 >x9 : 15b78000 x8 : >x7 : 0001 x6 : f000 >x5 : 0fff x4 : 1065dbc8 >x3 : 000d x2 : 1000 >x1 : f000 x0 : >Call trace: > __iommu_dma_unmap+0xb8/0xc0 > iommu_dma_unmap_sg+0x98/0xb8 > put_pages+0x5c/0xf0 [msm] > msm_gem_free_work+0x10c/0x150 [msm] > process_one_work+0x1e0/0x330 > worker_thread+0x40/0x438 > kthread+0x12c/0x130 > ret_from_fork+0x10/0x18 >---[ end trace afc0dc5ab81a06bf ]--- > > Not quite sure what triggered that, but we really shouldn't be abusing > dma_{map,unmap}_sg() for cache maint. > > Signed-off-by: Rob Clark > Cc: Stephen Boyd Applied to -misc-fixes Thanks, Sean > --- > drivers/gpu/drm/msm/msm_gem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c > index d31d9f927887..3b84cbdcafa3 100644 > --- a/drivers/gpu/drm/msm/msm_gem.c > +++ b/drivers/gpu/drm/msm/msm_gem.c > @@ -108,7 +108,7 @@ static struct page **get_pages(struct drm_gem_object *obj) >* because display controller, GPU, etc. are not coherent: >*/ > if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED)) > - dma_map_sg(dev->dev, msm_obj->sgt->sgl, > + dma_sync_sg_for_device(dev->dev, msm_obj->sgt->sgl, > msm_obj->sgt->nents, DMA_BIDIRECTIONAL); > } > > @@ -138,7 +138,7 @@ static void put_pages(struct drm_gem_object *obj) >* GPU, etc. are not coherent: >*/ > if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED)) > - dma_unmap_sg(obj->dev->dev, msm_obj->sgt->sgl, > + dma_sync_sg_for_cpu(obj->dev->dev, > msm_obj->sgt->sgl, >msm_obj->sgt->nents, >DMA_BIDIRECTIONAL); > > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH] drm/amdgpu/gfx10: Fix missing break in switch statement
On 7/22/19 2:12 PM, Alex Deucher wrote: > On Sun, Jul 21, 2019 at 6:39 PM Gustavo A. R. Silva > wrote: >> >> Add missing break statement in order to prevent the code from falling >> through to case AMDGPU_IRQ_STATE_ENABLE. >> >> This bug was found thanks to the ongoing efforts to enable >> -Wimplicit-fallthrough. >> >> Fixes: a644d85a5cd4 ("drm/amdgpu: add gfx v10 implementation (v10)") >> Cc: sta...@vger.kernel.org >> Signed-off-by: Gustavo A. R. Silva > > Applied. Thanks! > Awesome! Glad to help. :) Thanks -- Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: silence variable 'conn' set but not used
The "struct drm_connector" iteration cursor from "for_each_new_connector_in_state" is never used in atomic_remove_fb() which generates a compilation warning, drivers/gpu/drm/drm_framebuffer.c: In function 'atomic_remove_fb': drivers/gpu/drm/drm_framebuffer.c:838:24: warning: variable 'conn' set but not used [-Wunused-but-set-variable] Silence it by marking "conn" __maybe_unused. Signed-off-by: Qian Cai --- drivers/gpu/drm/drm_framebuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index 0b72468e8131..57564318ceea 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -835,7 +835,7 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) struct drm_device *dev = fb->dev; struct drm_atomic_state *state; struct drm_plane *plane; - struct drm_connector *conn; + struct drm_connector *conn __maybe_unused; struct drm_connector_state *conn_state; int i, ret; unsigned plane_mask; -- 1.8.3.1
Re: [PATCH] drm/amdkfd/kfd_mqd_manager_v10: Avoid fall-through warning
Applied. Thanks! Alex On Mon, Jul 22, 2019 at 2:14 PM Liu, Shaoyun wrote: > > Reviewed-by: shaoyunl > > On 2019-07-22 1:47 p.m., Gustavo A. R. Silva wrote: > > In preparation to enabling -Wimplicit-fallthrough, this patch silences > > the following warning: > > > > drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v10.c: In function > > ‘mqd_manager_init_v10’: > > ./include/linux/dynamic_debug.h:122:52: warning: this statement may fall > > through [-Wimplicit-fallthrough=] > > #define __dynamic_func_call(id, fmt, func, ...) do { \ > > ^ > > ./include/linux/dynamic_debug.h:143:2: note: in expansion of macro > > ‘__dynamic_func_call’ > >__dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) > >^~~ > > ./include/linux/dynamic_debug.h:153:2: note: in expansion of macro > > ‘_dynamic_func_call’ > >_dynamic_func_call(fmt, __dynamic_pr_debug, \ > >^~ > > ./include/linux/printk.h:336:2: note: in expansion of macro > > ‘dynamic_pr_debug’ > >dynamic_pr_debug(fmt, ##__VA_ARGS__) > >^~~~ > > drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v10.c:432:3: note: in > > expansion of macro ‘pr_debug’ > > pr_debug("%s@%i\n", __func__, __LINE__); > > ^~~~ > > drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_mqd_manager_v10.c:433:2: note: here > >case KFD_MQD_TYPE_COMPUTE: > >^~~~ > > > > by removing the call to pr_debug() in KFD_MQD_TYPE_CP: > > > > "The mqd init for CP and COMPUTE will have the same routine." [1] > > > > This bug was found thanks to the ongoing efforts to enable > > -Wimplicit-fallthrough. > > > > [1] > > https://lore.kernel.org/lkml/c735a1cc-a545-50fb-44e7-c0ad93ee8...@amd.com/ > > > > Signed-off-by: Gustavo A. R. Silva > > --- > > drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c > > b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c > > index 4f8a6ffc5775..9cd3eb2d90bd 100644 > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c > > @@ -429,7 +429,6 @@ struct mqd_manager *mqd_manager_init_v10(enum > > KFD_MQD_TYPE type, > > > > switch (type) { > > case KFD_MQD_TYPE_CP: > > - pr_debug("%s@%i\n", __func__, __LINE__); > > case KFD_MQD_TYPE_COMPUTE: > > pr_debug("%s@%i\n", __func__, __LINE__); > > mqd->allocate_mqd = allocate_mqd; > ___ > 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 3/3] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls
Am 22.07.19 um 19:40 schrieb Emil Velikov: > From: Emil Velikov > > As mentioned by Christian, for drivers which support only primary nodes > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS. > > For others, this check in particular will be a noop. So let's remove it > as suggested by Christian. > > Cc: Alex Deucher > Cc: Christian König > Cc: amd-...@lists.freedesktop.org > Cc: Daniel Vetter > Signed-off-by: Emil Velikov Looks like I can't convince you that opening up the primary node like this is a bad idea. Well at least we don't need to add any new flags, so feel free to add my Acked-by. Christian. > --- > drivers/gpu/drm/drm_ioctl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 09f7f8e33fa3..a397177af369 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -647,8 +647,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0), > > - DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, > drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, > drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, > drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, > drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, > 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0), ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu/gfx10: Fix missing break in switch statement
On Sun, Jul 21, 2019 at 6:39 PM Gustavo A. R. Silva wrote: > > Add missing break statement in order to prevent the code from falling > through to case AMDGPU_IRQ_STATE_ENABLE. > > This bug was found thanks to the ongoing efforts to enable > -Wimplicit-fallthrough. > > Fixes: a644d85a5cd4 ("drm/amdgpu: add gfx v10 implementation (v10)") > Cc: sta...@vger.kernel.org > Signed-off-by: Gustavo A. R. Silva Applied. Thanks! Alex > --- > drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c > index 1675d5837c3c..35e8e29139b1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c > @@ -4611,6 +4611,7 @@ gfx_v10_0_set_gfx_eop_interrupt_state(struct > amdgpu_device *adev, > cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0, > TIME_STAMP_INT_ENABLE, 0); > WREG32(cp_int_cntl_reg, cp_int_cntl); > + break; > case AMDGPU_IRQ_STATE_ENABLE: > cp_int_cntl = RREG32(cp_int_cntl_reg); > cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0, > -- > 2.22.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: [v3 1/4] dt-bindngs: display: panel: Add BOE tv101wum-n16 panel bindings
$Subject ~= s/bindngs/bindings/ Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()
On 7/22/19 12:07 PM, Matthew Wilcox wrote: > On Mon, Jul 22, 2019 at 11:53:54AM -0700, John Hubbard wrote: >> On 7/22/19 2:33 AM, Christoph Hellwig wrote: >>> On Sun, Jul 21, 2019 at 09:30:10PM -0700, john.hubb...@gmail.com wrote: for (i = 0; i < vsg->num_pages; ++i) { if (NULL != (page = vsg->pages[i])) { if (!PageReserved(page) && (DMA_FROM_DEVICE == vsg->direction)) - SetPageDirty(page); - put_page(page); + put_user_pages_dirty(&page, 1); + else + put_user_page(page); } >>> >>> Can't just pass a dirty argument to put_user_pages? Also do we really >> >> Yes, and in fact that would help a lot more than the single page case, >> which is really just cosmetic after all. >> >>> need a separate put_user_page for the single page case? >>> put_user_pages_dirty? >> >> Not really. I'm still zeroing in on the ideal API for all these call sites, >> and I agree that the approach below is cleaner. > > so enum { CLEAN = 0, DIRTY = 1, LOCK = 2, DIRTY_LOCK = 3 }; > ? > Sure. In fact, I just applied something similar to bio_release_pages() locally, in order to reconcile Christoph's and Jerome's approaches (they each needed to add a bool arg), so I'm all about the enums in the arg lists. :) I'm going to post that one shortly, let's see how it goes over. heh. thanks, -- John Hubbard NVIDIA
Re: [PATCH] drm/amdkfd: Fix missing break in switch statement
On Sun, Jul 21, 2019 at 6:12 PM Gustavo A. R. Silva wrote: > > Add missing break statement in order to prevent the code from falling > through to case CHIP_NAVI10. > > This bug was found thanks to the ongoing efforts to enable > -Wimplicit-fallthrough. > > Fixes: 14328aa58ce5 ("drm/amdkfd: Add navi10 support to amdkfd. (v3)") > Cc: sta...@vger.kernel.org > Signed-off-by: Gustavo A. R. Silva Applied. Thanks! Alex > --- > drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > index 792371442195..4e3fc284f6ac 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > @@ -668,6 +668,7 @@ static int kfd_fill_gpu_cache_info(struct kfd_dev *kdev, > case CHIP_RAVEN: > pcache_info = raven_cache_info; > num_of_cache_types = ARRAY_SIZE(raven_cache_info); > + break; > case CHIP_NAVI10: > pcache_info = navi10_cache_info; > num_of_cache_types = ARRAY_SIZE(navi10_cache_info); > -- > 2.22.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: [PATCH 1/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()
On Mon, Jul 22, 2019 at 11:53:54AM -0700, John Hubbard wrote: > On 7/22/19 2:33 AM, Christoph Hellwig wrote: > > On Sun, Jul 21, 2019 at 09:30:10PM -0700, john.hubb...@gmail.com wrote: > >>for (i = 0; i < vsg->num_pages; ++i) { > >>if (NULL != (page = vsg->pages[i])) { > >>if (!PageReserved(page) && (DMA_FROM_DEVICE == > >> vsg->direction)) > >> - SetPageDirty(page); > >> - put_page(page); > >> + put_user_pages_dirty(&page, 1); > >> + else > >> + put_user_page(page); > >>} > > > > Can't just pass a dirty argument to put_user_pages? Also do we really > > Yes, and in fact that would help a lot more than the single page case, > which is really just cosmetic after all. > > > need a separate put_user_page for the single page case? > > put_user_pages_dirty? > > Not really. I'm still zeroing in on the ideal API for all these call sites, > and I agree that the approach below is cleaner. so enum { CLEAN = 0, DIRTY = 1, LOCK = 2, DIRTY_LOCK = 3 }; ?
Re: [PATCH 3/3] gup: new put_user_page_dirty*() helpers
On 7/21/19 9:30 PM, john.hubb...@gmail.com wrote: > From: John Hubbard > > While converting call sites to use put_user_page*() [1], quite a few > places ended up needing a single-page routine to put and dirty a > page. > > Provide put_user_page_dirty() and put_user_page_dirty_lock(), > and use them in a few places: net/xdp, drm/via/, drivers/infiniband. > Please disregard this one, I'm going to drop it, as per the discussion in patch 1. thanks, -- John Hubbard NVIDIA > Cc: Jason Gunthorpe > Cc: Jan Kara > Signed-off-by: John Hubbard > --- > drivers/gpu/drm/via/via_dmablit.c| 2 +- > drivers/infiniband/core/umem.c | 2 +- > drivers/infiniband/hw/usnic/usnic_uiom.c | 2 +- > include/linux/mm.h | 10 ++ > net/xdp/xdp_umem.c | 2 +- > 5 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/via/via_dmablit.c > b/drivers/gpu/drm/via/via_dmablit.c > index 219827ae114f..d30b2d75599f 100644 > --- a/drivers/gpu/drm/via/via_dmablit.c > +++ b/drivers/gpu/drm/via/via_dmablit.c > @@ -189,7 +189,7 @@ via_free_sg_info(struct pci_dev *pdev, drm_via_sg_info_t > *vsg) > for (i = 0; i < vsg->num_pages; ++i) { > if (NULL != (page = vsg->pages[i])) { > if (!PageReserved(page) && (DMA_FROM_DEVICE == > vsg->direction)) > - put_user_pages_dirty(&page, 1); > + put_user_page_dirty(page); > else > put_user_page(page); > } > diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c > index 08da840ed7ee..a7337cc3ca20 100644 > --- a/drivers/infiniband/core/umem.c > +++ b/drivers/infiniband/core/umem.c > @@ -55,7 +55,7 @@ static void __ib_umem_release(struct ib_device *dev, struct > ib_umem *umem, int d > for_each_sg_page(umem->sg_head.sgl, &sg_iter, umem->sg_nents, 0) { > page = sg_page_iter_page(&sg_iter); > if (umem->writable && dirty) > - put_user_pages_dirty_lock(&page, 1); > + put_user_page_dirty_lock(page); > else > put_user_page(page); > } > diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c > b/drivers/infiniband/hw/usnic/usnic_uiom.c > index 0b0237d41613..d2ded624fb2a 100644 > --- a/drivers/infiniband/hw/usnic/usnic_uiom.c > +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c > @@ -76,7 +76,7 @@ static void usnic_uiom_put_pages(struct list_head > *chunk_list, int dirty) > page = sg_page(sg); > pa = sg_phys(sg); > if (dirty) > - put_user_pages_dirty_lock(&page, 1); > + put_user_page_dirty_lock(page); > else > put_user_page(page); > usnic_dbg("pa: %pa\n", &pa); > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 0334ca97c584..c0584c6d9d78 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -1061,6 +1061,16 @@ void put_user_pages_dirty(struct page **pages, > unsigned long npages); > void put_user_pages_dirty_lock(struct page **pages, unsigned long npages); > void put_user_pages(struct page **pages, unsigned long npages); > > +static inline void put_user_page_dirty(struct page *page) > +{ > + put_user_pages_dirty(&page, 1); > +} > + > +static inline void put_user_page_dirty_lock(struct page *page) > +{ > + put_user_pages_dirty_lock(&page, 1); > +} > + > #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) > #define SECTION_IN_PAGE_FLAGS > #endif > diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c > index 9cbbb96c2a32..1d122e52c6de 100644 > --- a/net/xdp/xdp_umem.c > +++ b/net/xdp/xdp_umem.c > @@ -171,7 +171,7 @@ static void xdp_umem_unpin_pages(struct xdp_umem *umem) > for (i = 0; i < umem->npgs; i++) { > struct page *page = umem->pgs[i]; > > - put_user_pages_dirty_lock(&page, 1); > + put_user_page_dirty_lock(page); > } > > kfree(umem->pgs); >
Re: [PATCH 0/6] drm/tinydrm: Move mipi_dbi
Den 22.07.2019 20.06, skrev Eric Anholt: > Noralf Trønnes writes: > >> This series ticks off the last tinydrm todo entry and moves out mipi_dbi >> to be a core helper. >> >> It splits struct mipi_dbi into an interface part and a display pipeline >> part (upload framebuffer over SPI). I also took the opportunity to >> rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with >> the use of the 'dsi' variable name in the MIPI DSI helper. >> >> Note: >> This depends on series: drm/tinydrm: Remove tinydrm.ko >> >> Series is also available here: >> https://github.com/notro/linux/tree/move_mipi_dbi > > Congratulations on making it to this stage. This looks like a fine > conclusion to tinydrm. > Thanks, it took a while, but really nice to finally get here. There are only two patches left to do now, one to remove the menuconfig so the drivers are visible by default and people can start putting their tiny drivers here, and secondly to change the folder name to 'tiny' as Daniel wants it. > Acked-by: Eric Anholt > Thanks, care to take a look at version 2 of the series? I had to add 3 patches to deal with a kconfig dependency that I had missed out on. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()
On 7/22/19 2:33 AM, Christoph Hellwig wrote: > On Sun, Jul 21, 2019 at 09:30:10PM -0700, john.hubb...@gmail.com wrote: >> for (i = 0; i < vsg->num_pages; ++i) { >> if (NULL != (page = vsg->pages[i])) { >> if (!PageReserved(page) && (DMA_FROM_DEVICE == >> vsg->direction)) >> -SetPageDirty(page); >> -put_page(page); >> +put_user_pages_dirty(&page, 1); >> +else >> +put_user_page(page); >> } > > Can't just pass a dirty argument to put_user_pages? Also do we really Yes, and in fact that would help a lot more than the single page case, which is really just cosmetic after all. > need a separate put_user_page for the single page case? > put_user_pages_dirty? Not really. I'm still zeroing in on the ideal API for all these call sites, and I agree that the approach below is cleaner. > > Also the PageReserved check looks bogus, as I can't see how a reserved > page can end up here. So IMHO the above snippled should really look > something like this: > > put_user_pages(vsg->pages[i], vsg->num_pages, > vsg->direction == DMA_FROM_DEVICE); > > in the end. > Agreed. thanks, -- John Hubbard NVIDIA
Re: [PATCH v3 6/7] dt-bindings: Add ANX6345 DP/eDP transmitter binding
On Mon, Jul 22, 2019 at 8:12 AM Torsten Duwe wrote: > > The anx6345 is an ultra-low power DisplayPort/eDP transmitter designed > for portable devices. > > Add a binding document for it. I believe you'll have to convert it to yaml format. > > Signed-off-by: Icenowy Zheng > Signed-off-by: Vasily Khoruzhick > Reviewed-by: Rob Herring > Signed-off-by: Torsten Duwe > Reviewed-by: Laurent Pinchart > --- > .../devicetree/bindings/display/bridge/anx6345.txt | 57 > ++ > 1 file changed, 57 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/anx6345.txt > > diff --git a/Documentation/devicetree/bindings/display/bridge/anx6345.txt > b/Documentation/devicetree/bindings/display/bridge/anx6345.txt > new file mode 100644 > index ..0af092d101c5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/anx6345.txt > @@ -0,0 +1,57 @@ > +Analogix ANX6345 eDP Transmitter > + > + > +The ANX6345 is an ultra-low power Full-HD eDP transmitter designed for > +portable devices. > + > +Required properties: > + > + - compatible : "analogix,anx6345" > + - reg : I2C address of the device > + - reset-gpios : Which GPIO to use for reset (active low) > + - dvdd12-supply : Regulator for 1.2V digital core power. > + - dvdd25-supply : Regulator for 2.5V digital core power. > + - Video port 0 for LVTTL input, using the DT bindings defined in [1]. > + > +Optional properties: > + > + - Video port 1 for eDP output (panel or connector) using the DT bindings > + defined in [1]. > + > +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt > + > +Example: > + > +anx6345: anx6345@38 { > + compatible = "analogix,anx6345"; > + reg = <0x38>; > + reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */ > + dvdd25-supply = <®_dldo2>; > + dvdd12-supply = <®_fldo1>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + anx6345_in: port@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0>; > + anx6345_in_tcon0: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&tcon0_out_anx6345>; > + }; > + }; > + > + anx6345_out: port@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + > + anx6345_out_panel: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&panel_in_edp>; > + }; > + }; > + }; > +}; > -- > 2.16.4 >
Re: [PATCH v3 5/7] drm/bridge: Add Analogix anx6345 support
On Mon, Jul 22, 2019 at 8:11 AM Torsten Duwe wrote: > > From: Icenowy Zheng > > The ANX6345 is an ultra-low power DisplayPower/eDP transmitter designed > for portable devices. This driver adds initial support for RGB to eDP > mode, without HPD and interrupts. > > This is a configuration usually seen in eDP applications. > > Signed-off-by: Icenowy Zheng > Signed-off-by: Vasily Khoruzhick > Signed-off-by: Torsten Duwe > --- > drivers/gpu/drm/bridge/analogix/Kconfig| 12 + > drivers/gpu/drm/bridge/analogix/Makefile | 1 + > drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 797 > + > 3 files changed, 810 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > > --- a/drivers/gpu/drm/bridge/analogix/Kconfig > +++ b/drivers/gpu/drm/bridge/analogix/Kconfig > @@ -1,6 +1,18 @@ > # SPDX-License-Identifier: GPL-2.0-only > +config DRM_ANALOGIX_ANX6345 > + tristate "Analogix ANX6345 bridge" > + select DRM_ANALOGIX_DP > + select DRM_KMS_HELPER > + select REGMAP_I2C > + help > + ANX6345 is an ultra-low Full-HD DisplayPort/eDP > + transmitter designed for portable devices. The > + ANX6345 transforms the LVTTL RGB output of an > + application processor to eDP or DisplayPort. > + > config DRM_ANALOGIX_ANX78XX > tristate "Analogix ANX78XX bridge" > + select DRM_ANALOGIX_DP > select DRM_KMS_HELPER > select REGMAP_I2C > help > --- a/drivers/gpu/drm/bridge/analogix/Makefile > +++ b/drivers/gpu/drm/bridge/analogix/Makefile > @@ -1,4 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0-only > analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o analogix-i2c-dptx.o > +obj-$(CONFIG_DRM_ANALOGIX_ANX6345) += analogix-anx6345.o > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o > --- /dev/null > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > @@ -0,0 +1,793 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright(c) 2016, Analogix Semiconductor. > + * Copyright(c) 2017, Icenowy Zheng > + * > + * Based on anx7808 driver obtained from chromeos with copyright: > + * Copyright(c) 2013, Google Inc. > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "analogix-i2c-dptx.h" > +#include "analogix-i2c-txcommon.h" > + > +#define POLL_DELAY 5 /* us */ > +#define POLL_TIMEOUT 500 /* us */ > + > +#define I2C_IDX_DPTX 0 > +#define I2C_IDX_TXCOM 1 > + > +static const u8 anx6345_i2c_addresses[] = { > + [I2C_IDX_DPTX] = ANALOGIX_I2C_DPTX, > + [I2C_IDX_TXCOM] = ANALOGIX_I2C_TXCOMMON, > +}; > +#define I2C_NUM_ADDRESSES ARRAY_SIZE(anx6345_i2c_addresses) > + > +struct anx6345 { > + struct drm_dp_aux aux; > + struct drm_bridge bridge; > + struct i2c_client *client; > + struct edid *edid; > + struct drm_connector connector; > + struct drm_dp_link link; > + struct drm_panel *panel; > + struct regulator *dvdd12; > + struct regulator *dvdd25; > + struct gpio_desc *gpiod_reset; > + struct mutex lock; /* protect EDID access */ > + > + /* I2C Slave addresses of ANX6345 are mapped as DPTX and SYS */ > + struct i2c_client *i2c_clients[I2C_NUM_ADDRESSES]; > + struct regmap *map[I2C_NUM_ADDRESSES]; > + > + u16 chipid; > + u8 dpcd[DP_RECEIVER_CAP_SIZE]; > + > + bool powered; > +}; > + > +static inline struct anx6345 *connector_to_anx6345(struct drm_connector *c) > +{ > + return container_of(c, struct anx6345, connector); > +} > + > +static inline struct anx6345 *bridge_to_anx6345(struct drm_bridge *bridge) > +{ > + return container_of(bridge, struct anx6345, bridge); > +} > + > +static int anx6345_set_bits(struct regmap *map, u8 reg, u8 mask) > +{ > + return regmap_update_bits(map, reg, mask, mask); > +} > + > +static int anx6345_clear_bits(struct regmap *map, u8 reg, u8 mask) > +{ > + return regmap_update_bits(map, reg, mask, 0); > +} > + > +static ssize_t anx6345_aux_transfer(struct drm_dp_aux *aux, > + struct drm_dp_aux_msg *msg) > +{ > + struct anx6345 *anx6345 = container_of(aux, struct anx6345, aux); > + > + return anx_dp_aux_transfer(anx6345->map[I2C_IDX_DPTX], msg); > +} > + > +static int anx6345_dp_link_training(struct anx6345 *anx6345) > +{ > + unsigned int value; > + u8 dp_bw; > + int err; > + > + err = anx6345_clear_bits(anx6345->map[I2C_IDX_TXCOM], > +SP_POWERDOWN_CTRL_REG, > +SP_TOTAL_PD); > + if (err) > + ret
Re: [PATCH v2 0/7] Add anx6345 DP/eDP bridge for Olimex Teres-I
On Mon, Jul 22, 2019 at 8:04 AM Torsten Duwe wrote: > > ANX6345 LVTTL->eDP video bridge, driver with device tree bindings. > > Changes from v2: > > * use SPDX-IDs throughout > > * removed the panel output again, as it was not what Maxime had in mind. > At least the Teres-I does very well without. No connector spec, no > "quirks"[1], > just plain EDID at work. You still need a panel to put backlight in there. Otherwise backlight will stay on when display is turned off. > > * binding clarifications and cosmetic changes as suggested by Andrzej > > Changes from v1: > > * fixed up copyright information. Most code changes are only moves and thus > retain copyright and module ownership. Even the new analogix-anx6345.c > originates > from the old 1495-line analogix-anx78xx.c, with 306 insertions and 987 > deletions > (ignoring the trivial anx78xx -> anx6345 replacements) 306 new vs. 508 > old... > > * fixed all minor formatting issues brought up > > * merged previously separate new analogix_dp_i2c module into existing > analogix_dp > > * split additional defines into a preparatory patch > > * renamed the factored-out common functions anx_aux_* -> anx_dp_aux_*, because > anx_...aux_transfer was exported globally. Besides, it is now GPL-only > exported. > > * moved chip ID read into a separate function. > > * keep the chip powered after a successful probe. > (There's a good chance that this is the only display during boot!) > > * updated the binding document: LVTTL input is now required, only the output > side > description is optional. > > Laurent: I have also looked into the drm_panel_bridge infrastructure, > but it's not that trivial to convert these drivers to it. > > Changes from the respective previous versions: > > * the reset polarity is corrected in DT and the driver; > things should be clearer now. > > * as requested, add a panel (the known innolux,n116bge) and connect > the ports. > > * renamed dvdd?? to *-supply to match the established scheme > > * trivial update to the #include list, to make it compile in 5.2 > > -- > [1] I hesitate to associate information about a connected panel with that > term. > But should it be neccessary for maximum power saving (e.g. pinebooks), > it would be good to have something here, regardless of nomenclature. > > Torsten
Re: [PATCH] drm: fix out-of-bounds access with short VSDB blocks
On Mon, Jul 22, 2019 at 02:38:34PM +, Simon Ser wrote: > From: Simon Ser > > The VSDB parsing code contains a few len >= N checks, accessing db[N] on > success. However if len == N, db[N] is out-of-bounds. > > This commit changes the checks to test for len > N. I'm not familiar with this at all, but is db[0] the header and db[1] to db[len] the data block? In that case, it's not out of bounds. I looked up the spec and it says: Length of following data block (in bytes) (02h) Sean > > Signed-off-by: Simon Ser > --- > drivers/gpu/drm/drm_edid.c | 24 > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 82a4ceed3fcf..13d632f14172 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -3569,7 +3569,7 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, > const u8 *db, u8 len, > vic_len = db[8 + offset] >> 5; > hdmi_3d_len = db[8 + offset] & 0x1f; > > - for (i = 0; i < vic_len && len >= (9 + offset + i); i++) { > + for (i = 0; i < vic_len && len > (9 + offset + i); i++) { > u8 vic; > > vic = db[9 + offset + i]; > @@ -3971,11 +3971,11 @@ drm_parse_hdr_metadata_block(struct drm_connector > *connector, const u8 *db) > connector->hdr_sink_metadata.hdmi_type1.metadata_type = > hdr_metadata_type(db); > > - if (len >= 4) > + if (len > 4) > connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; > - if (len >= 5) > + if (len > 5) > connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; > - if (len >= 6) > + if (len > 6) > connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; > } > > @@ -3984,19 +3984,19 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector > *connector, const u8 *db) > { > u8 len = cea_db_payload_len(db); > > - if (len >= 6 && (db[6] & (1 << 7))) > + if (len > 6 && (db[6] & (1 << 7))) > connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= > DRM_ELD_SUPPORTS_AI; > - if (len >= 8) { > + if (len > 8) { > connector->latency_present[0] = db[8] >> 7; > connector->latency_present[1] = (db[8] >> 6) & 1; > } > - if (len >= 9) > + if (len > 9) > connector->video_latency[0] = db[9]; > - if (len >= 10) > + if (len > 10) > connector->audio_latency[0] = db[10]; > - if (len >= 11) > + if (len > 11) > connector->video_latency[1] = db[11]; > - if (len >= 12) > + if (len > 12) > connector->audio_latency[1] = db[12]; > > DRM_DEBUG_KMS("HDMI: latency present %d %d, " > @@ -4559,9 +4559,9 @@ drm_parse_hdmi_vsdb_video(struct drm_connector > *connector, const u8 *db) > struct drm_display_info *info = &connector->display_info; > u8 len = cea_db_payload_len(db); > > - if (len >= 6) > + if (len > 6) > info->dvi_dual = db[6] & 1; > - if (len >= 7) > + if (len > 7) > info->max_tmds_clock = db[7] * 5000; > > DRM_DEBUG_KMS("HDMI: DVI dual %d, " > -- > 2.22.0 > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] drm/via: drop use of DRM(READ|WRITE) macros
Hi Email. > > > IMHO a far better idea is to expand these macros as static inline > > > functions. > > > The extra bonus here is that the pseudo-magical VIA_BASE will also > > > disappear. > > > > > > Since all the VIA_READ8 are used for masking, one might as well drop > > > them in favour of via_mask(). Like this: static inline u32 via_read(struct drm_via_private *dev_priv, u32 reg) { return readl((void __iomem *)(dev_priv->mmio->handle + reg)); } static inline void via_write(struct drm_via_private *dev_priv, u32 reg, u32 val) { writel(val, (void __iomem *)(dev_priv->mmio->handle + reg)); } static inline void via_write8(struct drm_via_private *dev_priv, u32 reg, u32 val) { writeb(val, (void __iomem *)(dev_priv->mmio->handle + reg)); } static inline void via_write8_mask_or(struct drm_via_private *dev_priv, u32 reg, u32 mask) { u32 val; val = readb((void __iomem *)(dev_priv->mmio->handle + reg)); writeb(val | mask, (void __iomem *)(dev_priv->mmio->handle + reg)); } static inline void via_write8_mask_and(struct drm_via_private *dev_priv, u32 reg, u32 mask) { u32 val; val = readb((void __iomem *)(dev_priv->mmio->handle + reg)); writeb(val & mask, (void __iomem *)(dev_priv->mmio->handle + reg)); } Patches are almost ready, but if there is any quick feedback let me know. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panel: simple: Doxygenize 'struct panel_desc'; rename a few functions
Hi, On Sun, Jul 21, 2019 at 2:38 AM Sam Ravnborg wrote: > > Hi Doug. > > On Wed, Jul 17, 2019 at 07:33:17PM +0200, Sam Ravnborg wrote: > > Hi Doug. > > > > On Fri, Jul 12, 2019 at 09:33:33AM -0700, Douglas Anderson wrote: > > > This attempts to address outstanding review feedback from commit > > > b8a2948fa2b3 ("drm/panel: simple: Add ability to override typical > > > timing"). Specifically: > > > > > > * It was requested that I document (in the structure definition) that > > > the device tree override had no effect if 'struct drm_display_mode' > > > was used in the panel description. I have provided full Doxygen > > > comments for 'struct panel_desc' to accomplish that. > > > * panel_simple_get_fixed_modes() was thought to be a confusing name, > > > so it has been renamed to panel_simple_get_display_modes(). > > > * panel_simple_parse_override_mode() was thought to be better named as > > > panel_simple_parse_panel_timing_node(). > > > > > > Suggested-by: Sam Ravnborg > > > Signed-off-by: Douglas Anderson > > > > Thanks. > > > > I updated the $subject to: > > drm/panel: simple: document panel_desc; rename a few functions > > > > And pushed out to drm-misc-next. > > I see the following error printed at each boot: > > /panel: could not find node 'panel-timing' > > The error originates from this snip (from panel-simple.c): > >if (!of_get_display_timing(dev->of_node, "panel-timing", &dt)) > panel_simple_parse_panel_timing_node(dev, panel, &dt); > > of_get_display_timing() returns -2 (-ENOENT). > > In the setup I use there is no panel-timing node as the timing specified > in panel-simple.c is fine. > This is the typical setup - and there should not in the normal case > be printed error messages like this during boot. > > Will you please take a look at this. Breadcrumbs: series has been posted to address this. PTAL. https://lkml.kernel.org/r/20190722182439.44844-1-diand...@chromium.org -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] video: amba-clcd: Spout an error if of_get_display_timing() gives an error
In the patch ("video: of: display_timing: Don't yell if no timing node is present") we'll stop spouting an error directly in of_get_display_timing() if no node is present. Presumably amba-clcd should take charge of spouting its own error now. NOTE: we'll print two errors if the node was present but there were problems parsing the timing node (one in of_parse_display_timing() and this new one). Since this is a fatal error for the driver's probe (and presumably someone will be debugging), this should be OK. Signed-off-by: Douglas Anderson --- drivers/video/fbdev/amba-clcd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c index 89324e42a033..7de43be6ef2c 100644 --- a/drivers/video/fbdev/amba-clcd.c +++ b/drivers/video/fbdev/amba-clcd.c @@ -561,8 +561,10 @@ static int clcdfb_of_get_dpi_panel_mode(struct device_node *node, struct videomode video; err = of_get_display_timing(node, "panel-timing", &timing); - if (err) + if (err) { + pr_err("%pOF: problems parsing panel-timing (%d)\n", node, err); return err; + } videomode_from_timing(&timing, &video); -- 2.22.0.657.g960e92d24f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] video: of: display_timing: Add of_node_put() in of_get_display_timing()
>From code inspection it can be seen that of_get_display_timing() is lacking an of_node_put(). Add it. Fixes: ffa3fd21de8a ("videomode: implement public of_get_display_timing()") Signed-off-by: Douglas Anderson --- drivers/video/of_display_timing.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c index f5c1c469c0af..5eedae0799f0 100644 --- a/drivers/video/of_display_timing.c +++ b/drivers/video/of_display_timing.c @@ -119,6 +119,7 @@ int of_get_display_timing(const struct device_node *np, const char *name, struct display_timing *dt) { struct device_node *timing_np; + int ret; if (!np) return -EINVAL; @@ -129,7 +130,11 @@ int of_get_display_timing(const struct device_node *np, const char *name, return -ENOENT; } - return of_parse_display_timing(timing_np, dt); + ret = of_parse_display_timing(timing_np, dt); + + of_node_put(timing_np); + + return ret; } EXPORT_SYMBOL_GPL(of_get_display_timing); -- 2.22.0.657.g960e92d24f-goog
[PATCH 2/4] video: of: display_timing: Don't yell if no timing node is present
There may be cases (like in panel-simple.c) where we have a sane fallback if no timings are specified in the device tree. Let's get rid of the unconditional pr_err(). We can add error messages in individual drivers if it makes sense. NOTE: we'll still print errors if the node is present but there are problems parsing the timings. Fixes: b8a2948fa2b3 ("drm/panel: simple: Add ability to override typical timing") Reported-by: Sam Ravnborg Signed-off-by: Douglas Anderson --- drivers/video/of_display_timing.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c index 5eedae0799f0..abc9ada798ee 100644 --- a/drivers/video/of_display_timing.c +++ b/drivers/video/of_display_timing.c @@ -125,10 +125,8 @@ int of_get_display_timing(const struct device_node *np, const char *name, return -EINVAL; timing_np = of_get_child_by_name(np, name); - if (!timing_np) { - pr_err("%pOF: could not find node '%s'\n", np, name); + if (!timing_np) return -ENOENT; - } ret = of_parse_display_timing(timing_np, dt); -- 2.22.0.657.g960e92d24f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm: panel-lvds: Spout an error if of_get_display_timing() gives an error
In the patch ("video: of: display_timing: Don't yell if no timing node is present") we'll stop spouting an error directly in of_get_display_timing() if no node is present. Presumably panel-lvds should take charge of spouting its own error now. NOTE: we'll print two errors if the node was present but there were problems parsing the timing node (one in of_parse_display_timing() and this new one). Since this is a fatal error for the driver's probe (and presumably someone will be debugging), this should be OK. Signed-off-by: Douglas Anderson --- drivers/gpu/drm/panel/panel-lvds.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c index 1ec57d0806a8..ad47cc95459e 100644 --- a/drivers/gpu/drm/panel/panel-lvds.c +++ b/drivers/gpu/drm/panel/panel-lvds.c @@ -147,8 +147,11 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds) int ret; ret = of_get_display_timing(np, "panel-timing", &timing); - if (ret < 0) + if (ret < 0) { + dev_err(lvds->dev, "%pOF: problems parsing panel-timing (%d)\n", + np, ret); return ret; + } videomode_from_timing(&timing, &lvds->video_mode); -- 2.22.0.657.g960e92d24f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel