[PATCH] drm/rcar-du: Replace drm_dev_unref with drm_dev_put
This patch unifies the naming of DRM functions for reference counting of struct drm_device. The resulting code is more aligned with the rest of the Linux kernel interfaces. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 02aee6cb0e53..9696a3030319 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -407,7 +407,7 @@ static int rcar_du_remove(struct platform_device *pdev) drm_kms_helper_poll_fini(ddev); drm_mode_config_cleanup(ddev); - drm_dev_unref(ddev); + drm_dev_put(ddev); return 0; } -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/shmobile: Replace drm_dev_unref with drm_dev_put
This patch unifies the naming of DRM functions for reference counting of struct drm_device. The resulting code is more aligned with the rest of the Linux kernel interfaces. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/shmobile/shmob_drm_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c index 592572554eb0..8d1ff596c774 100644 --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c @@ -198,7 +198,7 @@ static int shmob_drm_remove(struct platform_device *pdev) drm_kms_helper_poll_fini(ddev); drm_mode_config_cleanup(ddev); drm_irq_uninstall(ddev); - drm_dev_unref(ddev); + drm_dev_put(ddev); return 0; } @@ -294,7 +294,7 @@ static int shmob_drm_probe(struct platform_device *pdev) drm_kms_helper_poll_fini(ddev); drm_mode_config_cleanup(ddev); err_free_drm_dev: - drm_dev_unref(ddev); + drm_dev_put(ddev); return ret; } -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/tve200: Replace drm_dev_unref with drm_dev_put
This patch unifies the naming of DRM functions for reference counting of struct drm_device. The resulting code is more aligned with the rest of the Linux kernel interfaces. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tve200/tve200_drv.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/tve200/tve200_drv.c b/drivers/gpu/drm/tve200/tve200_drv.c index ac344ddb23bc..18e9c8e709f9 100644 --- a/drivers/gpu/drm/tve200/tve200_drv.c +++ b/drivers/gpu/drm/tve200/tve200_drv.c @@ -198,12 +198,12 @@ static int tve200_probe(struct platform_device *pdev) if (IS_ERR(priv->pclk)) { dev_err(dev, "unable to get PCLK\n"); ret = PTR_ERR(priv->pclk); - goto dev_unref; + goto dev_put; } ret = clk_prepare_enable(priv->pclk); if (ret) { dev_err(dev, "failed to enable PCLK\n"); - goto dev_unref; + goto dev_put; } /* This clock is for the pixels (27MHz) */ @@ -249,8 +249,8 @@ static int tve200_probe(struct platform_device *pdev) clk_disable: clk_disable_unprepare(priv->pclk); -dev_unref: - drm_dev_unref(drm); +dev_put: + drm_dev_put(drm); return ret; } @@ -265,7 +265,7 @@ static int tve200_remove(struct platform_device *pdev) drm_panel_bridge_remove(priv->bridge); drm_mode_config_cleanup(drm); clk_disable_unprepare(priv->pclk); - drm_dev_unref(drm); + drm_dev_put(drm); return 0; } -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support
Not all sunxi platforms with the first version of the Display Engine support an alpha component on the plane with the lowest z position (as in: lowest z-pos), that gets blended with the background color. In particular, the A13 is known to have this limitation. However, it was recently discovered that the A20 and A33 are capable of having alpha on their lowest plane. Thus, this introduces a specific quirk to indicate such support, per-platform. Since this was not tested on sun4i and sun6i platforms, a conservative approach is kept and this feature is not supported. Signed-off-by: Paul Kocialkowski --- drivers/gpu/drm/sun4i/sun4i_backend.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c index a3cc398d4d80..cdc4a8a91ea2 100644 --- a/drivers/gpu/drm/sun4i/sun4i_backend.c +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c @@ -35,6 +35,8 @@ struct sun4i_backend_quirks { /* backend <-> TCON muxing selection done in backend */ bool needs_output_muxing; + /* alpha at the lowest z position is not always supported */ + bool supports_lowest_plane_alpha; }; static void sun4i_backend_apply_color_correction(struct sunxi_engine *engine) @@ -484,6 +486,7 @@ static void sun4i_backend_atomic_begin(struct sunxi_engine *engine, static int sun4i_backend_atomic_check(struct sunxi_engine *engine, struct drm_crtc_state *crtc_state) { + struct sun4i_backend *backend = engine_to_sun4i_backend(engine); struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 }; struct drm_atomic_state *state = crtc_state->state; struct drm_device *drm = state->dev; @@ -584,8 +587,9 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine, } /* We can't have an alpha plane at the lowest position */ - if (plane_states[0]->fb->format->has_alpha || - (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)) + if ((plane_states[0]->fb->format->has_alpha || + (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)) && + !backend->quirks->supports_lowest_plane_alpha) return -EINVAL; for (i = 1; i < num_planes; i++) { @@ -970,9 +974,11 @@ static const struct sun4i_backend_quirks sun6i_backend_quirks = { static const struct sun4i_backend_quirks sun7i_backend_quirks = { .needs_output_muxing = true, + .supports_lowest_plane_alpha = true, }; static const struct sun4i_backend_quirks sun8i_a33_backend_quirks = { + .supports_lowest_plane_alpha = true, }; static const struct sun4i_backend_quirks sun9i_backend_quirks = { -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/sun4i: sun4i: Register quirks with the backend structure
In prevision for introducing a new quirk that will be used at atomic plane check time, register the quirks structure with the backend structure. This way, it can easily be grabbed where needed. Signed-off-by: Paul Kocialkowski --- drivers/gpu/drm/sun4i/sun4i_backend.c | 2 ++ drivers/gpu/drm/sun4i/sun4i_backend.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c index 17dca4df00f2..a3cc398d4d80 100644 --- a/drivers/gpu/drm/sun4i/sun4i_backend.c +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c @@ -909,6 +909,8 @@ static int sun4i_backend_bind(struct device *dev, struct device *master, : SUN4I_BACKEND_MODCTL_OUT_LCD0)); } + backend->quirks = quirks; + return 0; err_disable_ram_clk: diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h index e33be2307c67..71de6147b483 100644 --- a/drivers/gpu/drm/sun4i/sun4i_backend.h +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h @@ -187,6 +187,8 @@ struct sun4i_backend { /* Protects against races in the frontend teardown */ spinlock_t frontend_lock; boolfrontend_teardown; + + const struct sun4i_backend_quirks *quirks; }; static inline struct sun4i_backend * -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu/pm: Remove VLA usage
Who's tree should this go through? To answer the question: When Rex is ok with that he pushes it to our internal amd-staging-drm-next tree. Alex then pushes that tree to a public server and at some point sends a pull request for inclusion in drm-next. Regards, Christian. Am 17.07.2018 um 08:23 schrieb Zhu, Rex: Patch is: Reviewed-by: Rex Zhumailto:re...@amd.com>> Best Regards Rex *From:* keesc...@google.com on behalf of Kees Cook *Sent:* Tuesday, July 17, 2018 11:59 AM *To:* Deucher, Alexander *Cc:* LKML; Koenig, Christian; Zhou, David(ChunMing); David Airlie; Zhu, Rex; Huang, Ray; Kuehling, Felix; amd-gfx list; Maling list - DRI developers *Subject:* Re: [PATCH] drm/amdgpu/pm: Remove VLA usage On Wed, Jun 20, 2018 at 11:26 AM, Kees Cook wrote: > In the quest to remove all stack VLA usage from the kernel[1], this > uses the maximum sane buffer size and removes copy/paste code. > > [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com > > Signed-off-by: Kees Cook Friendly ping! Who's tree should this go through? Thanks! -Kees > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 100 +++-- > 1 file changed, 42 insertions(+), 58 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > index b455da487782..5eb98cde22ed 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > @@ -593,40 +593,59 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev, > return snprintf(buf, PAGE_SIZE, "\n"); > } > > -static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev, > - struct device_attribute *attr, > - const char *buf, > - size_t count) > +/* > + * Worst case: 32 bits individually specified, in octal at 12 characters > + * per line (+1 for \n). > + */ > +#define AMDGPU_MASK_BUF_MAX (32 * 13) > + > +static ssize_t amdgpu_read_mask(const char *buf, size_t count, uint32_t *mask) > { > - struct drm_device *ddev = dev_get_drvdata(dev); > - struct amdgpu_device *adev = ddev->dev_private; > int ret; > long level; > - uint32_t mask = 0; > char *sub_str = NULL; > char *tmp; > - char buf_cpy[count]; > + char buf_cpy[AMDGPU_MASK_BUF_MAX + 1]; > const char delimiter[3] = {' ', '\n', '\0'}; > + size_t bytes; > > - memcpy(buf_cpy, buf, count+1); > + *mask = 0; > + > + bytes = min(count, sizeof(buf_cpy) - 1); > + memcpy(buf_cpy, buf, bytes); > + buf_cpy[bytes] = '\0'; > tmp = buf_cpy; > while (tmp[0]) { > - sub_str = strsep(, delimiter); > + sub_str = strsep(, delimiter); > if (strlen(sub_str)) { > ret = kstrtol(sub_str, 0, ); > - > - if (ret) { > - count = -EINVAL; > - goto fail; > - } > - mask |= 1 << level; > + if (ret) > + return -EINVAL; > + *mask |= 1 << level; > } else > break; > } > + > + return 0; > +} > + > +static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t count) > +{ > + struct drm_device *ddev = dev_get_drvdata(dev); > + struct amdgpu_device *adev = ddev->dev_private; > + int ret; > + uint32_t mask = 0; > + > + ret = amdgpu_read_mask(buf, count, ); > + if (ret) > + return ret; > + > if (adev->powerplay.pp_funcs->force_clock_level) > amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask); > > -fail: > return count; > } > > @@ -651,32 +670,15 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev, > struct drm_device *ddev = dev_get_drvdata(dev); > struct amdgpu_device *adev = ddev->dev_private; > int ret; > - long level; > uint32_t mask = 0; > - char *sub_str = NULL; > - char *tmp; > - char buf_cpy[count]; > - const char delimiter[3] = {' ', '\n', '\0'}; > > - memcpy(buf_cpy, buf, count+1); > - tmp = buf_cpy; > - while (tmp[0]) { > - sub_str = strsep(, delimiter); > - if (strlen(sub_str)) { > - ret = kstrtol(sub_str, 0, ); > + ret = amdgpu_read_mask(buf, count, ); > + if (ret) > + return ret; > > - if (ret) { > - count = -EINVAL; > - goto fail; > - } > - mask |= 1 << level; > - }
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Move out non-modeset calls from modeset init and cleanup
Quoting José Roberto de Souza (2018-07-16 23:38:38) > @@ -1395,9 +1379,22 @@ int i915_driver_load(struct pci_dev *pdev, const > struct pci_device_id *ent) > goto out_cleanup_hw; > } > > + ret = intel_irq_install(dev_priv); > + if (ret) > + goto out_cleanup_hw; > + > + /* i915_gem_init() call chain will call > +* intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ); > +*/ > + intel_power_domains_init_hw(dev_priv, false); > + > + ret = i915_gem_init(dev_priv); > + if (ret) > + goto cleanup_irq; > + > ret = i915_load_modeset_init(_priv->drm); > if (ret < 0) > - goto out_cleanup_hw; > + goto cleanup_gem; Bzzt. Order is extremely important. e.g. modeset init needs to reserve portions of the GTT still in use by the BIOS before we wipe it. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH v2 2/3] drm/nouveau: Fix runtime PM leak in nv50_disp_atomic_commit()
On Thu, Jul 12, 2018 at 01:02:53PM -0400, Lyude Paul wrote: > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -1878,7 +1878,7 @@ nv50_disp_atomic_commit(struct drm_device *dev, > nv50_disp_atomic_commit_tail(state); > > drm_for_each_crtc(crtc, dev) { > - if (crtc->state->enable) { > + if (crtc->state->active) { > if (!drm->have_disp_power_ref) { > drm->have_disp_power_ref = true; > return 0; Somewhat tangential comment on this older patch, since you continue to dig around in the runtime PM area: Whenever a crtc is activated or deactivated in nouveau, we iterate over all crtcs and acquire a runtime PM if a crtc is active and previously there was no active one, or we drop a ref if none is active and previously there was an active one. For a while now I've been thinking that it would be more straightforward to acquire a ref whenever a crtc is activated and drop one when a crtc is deactivated, i.e. hold one ref for every active crtc. That way the have_disp_power_ref variable as well as the iteration logic could be removed, leading to a simplification. Just a suggestion anyway. Thanks, Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths
On Tue, Jul 17, 2018 at 9:16 AM, Lukas Wunner wrote: > [cc += linux-pm] > > Hi Lyude, > > First of all, thanks a lot for looking into this. > > On Mon, Jul 16, 2018 at 07:59:25PM -0400, Lyude Paul wrote: >> In order to fix all of the spots that need to have runtime PM get/puts() >> added, we need to ensure that it's possible for us to call >> pm_runtime_get/put() in any context, regardless of how deep, since >> almost all of the spots that are currently missing refs can potentially >> get called in the runtime suspend/resume path. Otherwise, we'll try to >> resume the GPU as we're trying to resume the GPU (and vice-versa) and >> cause the kernel to deadlock. >> >> With this, it should be safe to call the pm runtime functions in any >> context in nouveau with one condition: any point in the driver that >> calls pm_runtime_get*() cannot hold any locks owned by nouveau that >> would be acquired anywhere inside nouveau_pmops_runtime_resume(). >> This includes modesetting locks, i2c bus locks, etc. > [snip] >> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c >> @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *dev) >> return -EBUSY; >> } >> >> + dev->power.disable_depth++; This is effectively equivalent to __pm_runtime_disable(dev, false) except for the locking (which is necessary). >> + > > I'm not sure if that variable is actually private to the PM core. > Grepping through the tree I only find a single occurrence where it's > accessed outside the PM core and that's in amdgpu. So this looks > a little fishy TBH. It may make sense to cc such patches to linux-pm > to get Rafael & other folks involved with the PM core to comment. You are right, power.disable_depth is internal to the PM core. Accessing it (and updating it in particular) directly from drivers is not a good idea. > Also, the disable_depth variable only exists if the kernel was > compiled with CONFIG_PM enabled, but I can't find a "depends on PM" > or something like that in nouveau's Kconfig. Actually, if PM is > not selected, all the nouveau_pmops_*() functions should be #ifdef'ed > away, but oddly there's no #ifdef CONFIG_PM anywhere in nouveau_drm.c. > > Anywayn, if I understand the commit message correctly, you're hitting a > pm_runtime_get_sync() in a code path that itself is called during a > pm_runtime_get_sync(). Could you include stack traces in the commit > message? My gut feeling is that this patch masks a deeper issue, > e.g. if the runtime_resume code path does in fact directly poll outputs, > that would seem wrong. Runtime resume should merely make the card > accessible, i.e. reinstate power if necessary, put into PCI_D0, > restore registers, etc. Output polling should be scheduled > asynchronously. Right. Thanks, Rafael ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panel: simple: Support simple VGA panels
On Tue, Jul 17, 2018 at 12:50 AM Rob Herring wrote: > On Mon, Jul 16, 2018 at 3:23 AM Linus Walleij > wrote: > > +Video Graphics Array > > VGA is more a controller interface than a panel... I don't know what else to call it though, other than formulating someting bureaucratic like "Video Graphics Array Display Resolutions" Or should I use the abbreviation: "VGA Display Resolutions" rather? The Wikipedia article "display resolution" https://en.wikipedia.org/wiki/Display_resolution just calls these three resolutions "VGA", "SVGA " and "XGA". > > +static const struct drm_display_mode video_graphics_array_modes[] = { > > + { > > + /* > > +* This is the most standardized "vanilla" VGA mode there > > is: > > +* 640x480 @ 60 Hz > > Don't we have standard VESA timings already defined as well as timings > that can be calculated based on resolution and refresh rate (called > CVT IIRC). Why duplicate here? They are inside drivers/gpu/drm/drm_edid.c all in static const arrays and enumerated by the index in the DMT spec taken out of XFree86. (drm_dmt_modes[]) I don't know. It is quite encapsulated into that EDID driver and doesn't seem very reusable. To pick out a few from inside EDID seems pretty daunting. I'd like to hear what the DRM folks think about that. > Why don't you just define a 'vga-connector' node Because this is not a VGA connector, it is just the VGA resolution. In other circumstances I do use it. I think you most often have to use that connector with the dumb VGA DAC bridge though, as the VGA connector is analog and what comes out of a display controller is digital. So it needs to make some kind of sense here. In a way (as we discussed before) the whole panel-simple.c thing is a bit bogus, as it is probably in most cases hiding a bridge or a dumb DAC or at least a VGA connector or similar that should've been properly modeled, but in this case I am more certain that it is actually the right choice. I guess I could try to use the dumb VGA bridge and the VGA connector, and it indeed falls back to VGA resolutions if no DDC channel is available but if I go in and say there is a DAC bridge and VGA connector I am essentially lying. > and IIRC, you can > just force the standard modes from userspace with DRM. I guess you mean the kernel command line arguments, lest there will be no boot console. Apart from being a user experience horror story, that requires a VGA connector bridge, as per above. (It's the EDID code that does that command line parsing.) And that requires lying about having a VGA connector bridge. But I can surely lie if everyone thinks that is the best idea :D > Maybe you need > some flag to force a connection in the emulator and perhaps some fake > EDID data to set a default mode. This device tree I'm creating is for ARM RTSM which is a proprietary ARM tool. Sudeep at ARM says it does not emulate any DAC bridge such as found in the Versatile Express machine family. It just expects to have the right resolution parameters written into the registers of the PL111, which in turn of course can only get it from a panel or bridge driver of some sort. The way those emulators work (AFAICT) is that they simply monitor register writes to the resolutions parameters to scale the emulator display window and then they read out the RGB data into that window, pixel by pixel, from the indicated memory area. It works for them. In QEMU, we had the same problem. It didn't support proper reporting of fake EDID on these platforms either, because of not properly modeling the hardware it was emulating, instead relying on the register reads as above. Since it is open source I could simply fix it: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04651.html https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04652.html https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04653.html After this, the QEMU for Versatile Express can properly use the bridge. I do try to be gritty and thorough! :D I don't really know what RTSM is and I can't run it myself. I just have to support it in the refactoring to use DRM for the ARM Versatile Express machines. I cannot change its source code. > There's also some other cases of > "transparent" bridges which don't have any driver. Yeah I think they mostly use panel-simple.c in the DRM case don't they? We come full circle. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] ARM: dts: imx6sl: Add vivante gpu nodes
On Fri, Jul 13, 2018 at 12:39:35PM +0300, Leonard Crestez wrote: > The imx6sl soc has gpu_2d and gpu_vg, no 3d support: > > etnaviv-gpu 220.gpu: model: GC320, revision: 5007 > etnaviv-gpu 2204000.gpu: model: GC355, revision: 1215 > > The IP blocks seem to be already supported. > > Signed-off-by: Leonard Crestez > Reviewed-by: Lucas Stach Applied, thanks. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu/pm: Remove VLA usage
Patch is: Reviewed-by: Rex Zhumailto:re...@amd.com>> Best Regards Rex From: keesc...@google.com on behalf of Kees Cook Sent: Tuesday, July 17, 2018 11:59 AM To: Deucher, Alexander Cc: LKML; Koenig, Christian; Zhou, David(ChunMing); David Airlie; Zhu, Rex; Huang, Ray; Kuehling, Felix; amd-gfx list; Maling list - DRI developers Subject: Re: [PATCH] drm/amdgpu/pm: Remove VLA usage On Wed, Jun 20, 2018 at 11:26 AM, Kees Cook wrote: > In the quest to remove all stack VLA usage from the kernel[1], this > uses the maximum sane buffer size and removes copy/paste code. > > [1] > https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com > > Signed-off-by: Kees Cook Friendly ping! Who's tree should this go through? Thanks! -Kees > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 100 +++-- > 1 file changed, 42 insertions(+), 58 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > index b455da487782..5eb98cde22ed 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > @@ -593,40 +593,59 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device > *dev, > return snprintf(buf, PAGE_SIZE, "\n"); > } > > -static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev, > - struct device_attribute *attr, > - const char *buf, > - size_t count) > +/* > + * Worst case: 32 bits individually specified, in octal at 12 characters > + * per line (+1 for \n). > + */ > +#define AMDGPU_MASK_BUF_MAX(32 * 13) > + > +static ssize_t amdgpu_read_mask(const char *buf, size_t count, uint32_t > *mask) > { > - struct drm_device *ddev = dev_get_drvdata(dev); > - struct amdgpu_device *adev = ddev->dev_private; > int ret; > long level; > - uint32_t mask = 0; > char *sub_str = NULL; > char *tmp; > - char buf_cpy[count]; > + char buf_cpy[AMDGPU_MASK_BUF_MAX + 1]; > const char delimiter[3] = {' ', '\n', '\0'}; > + size_t bytes; > > - memcpy(buf_cpy, buf, count+1); > + *mask = 0; > + > + bytes = min(count, sizeof(buf_cpy) - 1); > + memcpy(buf_cpy, buf, bytes); > + buf_cpy[bytes] = '\0'; > tmp = buf_cpy; > while (tmp[0]) { > - sub_str = strsep(, delimiter); > + sub_str = strsep(, delimiter); > if (strlen(sub_str)) { > ret = kstrtol(sub_str, 0, ); > - > - if (ret) { > - count = -EINVAL; > - goto fail; > - } > - mask |= 1 << level; > + if (ret) > + return -EINVAL; > + *mask |= 1 << level; > } else > break; > } > + > + return 0; > +} > + > +static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t count) > +{ > + struct drm_device *ddev = dev_get_drvdata(dev); > + struct amdgpu_device *adev = ddev->dev_private; > + int ret; > + uint32_t mask = 0; > + > + ret = amdgpu_read_mask(buf, count, ); > + if (ret) > + return ret; > + > if (adev->powerplay.pp_funcs->force_clock_level) > amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask); > > -fail: > return count; > } > > @@ -651,32 +670,15 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device > *dev, > struct drm_device *ddev = dev_get_drvdata(dev); > struct amdgpu_device *adev = ddev->dev_private; > int ret; > - long level; > uint32_t mask = 0; > - char *sub_str = NULL; > - char *tmp; > - char buf_cpy[count]; > - const char delimiter[3] = {' ', '\n', '\0'}; > > - memcpy(buf_cpy, buf, count+1); > - tmp = buf_cpy; > - while (tmp[0]) { > - sub_str = strsep(, delimiter); > - if (strlen(sub_str)) { > - ret = kstrtol(sub_str, 0, ); > + ret = amdgpu_read_mask(buf, count, ); > + if (ret) > + return ret; > > - if (ret) { > - count = -EINVAL; > - goto fail; > - } > - mask |= 1 << level; > - } else > - break; > - } > if (adev->powerplay.pp_funcs->force_clock_level) > amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask); > > -fail: > return count; > } > > @@ -701,33 +703,15 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device > *dev, > struct drm_device *ddev =
[Bug 105760] [4.17-rc1] RIP: smu7_populate_single_firmware_entry.isra.6+0x57/0xc0 [amdgpu] RSP: ffffa17901efb930
https://bugs.freedesktop.org/show_bug.cgi?id=105760 --- Comment #50 from Thomas Martitz --- Created attachment 140660 --> https://bugs.freedesktop.org/attachment.cgi?id=140660=edit dmesg with force_asic_init.diff + 0001-workaround-v2.patch Doesn't seem to make a difference. [ 255.418659] [drm:gfx_v8_0_ring_test_ring] *ERROR* amdgpu: ring 0 test failed (scratch(0xC040)=0xCAFEDEAD) [ 255.418670] [drm:amdgpu_device_ip_resume_phase2] *ERROR* resume of IP block failed -22 [ 255.418675] [drm:amdgpu_device_resume] *ERROR* amdgpu_device_ip_resume failed (-22). -- 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: [Nouveau] [PATCH 2/5] drm/nouveau: Grab RPM ref while probing outputs
On Mon, Jul 16, 2018 at 07:59:26PM -0400, Lyude Paul wrote: > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -2012,10 +2012,18 @@ nv50_disp_atomic_state_alloc(struct drm_device *dev) > return >state; > } > > +static void > +nouveau_output_poll_changed(struct drm_device *dev) > +{ > + pm_runtime_get_sync(dev->dev); > + drm_fb_helper_hotplug_event(dev->fb_helper); > + pm_runtime_put_autosuspend(dev->dev); > +} > + > static const struct drm_mode_config_funcs > nv50_disp_func = { > .fb_create = nouveau_user_framebuffer_create, > - .output_poll_changed = drm_fb_helper_output_poll_changed, > + .output_poll_changed = nouveau_output_poll_changed, It might make sense to provide a generic DRM helper for this. Same for patch 3 in this series. Thanks, Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/6] drm: Let userspace check if driver supports modeset
Quoting José Roberto de Souza (2018-07-16 23:38:36) > GPU accelerators usually don't have display block or the display > driver part can be disable when building driver(for servers it save > some resources) so it is important let userspace check this > capability too. We currently communicate that by having no modeset resources. What does another cap bit accomplish that we don't already know? -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v14 2/2] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
Quoting Sandeep Panda (2018-07-16 01:43:30) > Document the bindings used for the sn65dsi86 DSI to eDP bridge. > > Changes in v1: > - Rephrase the dt-binding descriptions to be more inline with existing >bindings (Andrzej Hajda). > - Add missing dt-binding that are parsed by corresponding driver >(Andrzej Hajda). > > Changes in v2: > - Remove edp panel specific dt-binding entries. Only keep bridge >specific entries (Sean Paul). > - Remove custom-modes dt entry since its usage is removed from driver also > (Sean Paul). > - Remove is-pluggable dt entry since this will not be needed anymore (Sean > Paul). > > Changes in v3: > - Remove irq-gpio dt entry and instead populate is an interrupt >property (Rob Herring). > > Changes in v4: > - Add link to bridge chip datasheet (Stephen Boyd) > - Add vpll and vcc regulator supply bindings (Stephen Boyd) > - Add ref clk optional dt binding (Stephen Boyd) > - Add gpio-controller optional dt binding (Stephen Boyd) > > Changes in v5: > - Use clock property to specify the input refclk (Stephen Boyd). > - Update gpio cell and pwm cell numbers (Stephen Boyd). > > Changes in v6: > - Add property to mention the lane mapping scheme and polarity inversion >(Stephen Boyd). > > Changes in v7: > - Detail description of lane mapping scheme dt property (Andrzej >Hajda/ Rob Herring). > - Removed HDP gpio binding, since the bridge uses IRQ signal to >determine HPD, and IRQ property is already documented in binding. > > Changes in v8: > - Removed unnecessary explanation of lane mapping and polarity dt >property, since these are already explained in media/video-interface >dt binidng (Rob Herring). > > Changes in v9: > - Avoid putting re-definition of lane mapping and polarity dt binding >(Rob Herring). > > Changes in v10: > - Use interrupts-extended property instead of interrupts to specify >interrupt line (Andrzej Hajda). > - Move data-lanes and lane-polarity property example to proper place > (Andrzej Hajda). > > Changes in v11: > - Add a property for suspend gpio function of GPIO1 pin on bridge chip >(Stephen Boyd). > > Changes in v12: > - Remove binding for dedicated DDC line (Andrzej Hajda). > > Signed-off-by: Sandeep Panda > --- Reviewed-by: Stephen Boyd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RESEND] drm/meson: Make DMT timings parameters and pixel clock generic
On Mon, 2018-07-16 at 09:40 +0200, Neil Armstrong wrote: > Remove the modes timings tables for DMT modes and calculate the HW > paremeters from the modes timings. > > Switch the DMT modes pixel clock calculation out of the static frequency > list to a generic calculation from a range of possible PLL dividers. > > This patch is an intermediate step towards usage of the Common Clock > Framwework for PLL setup, by reworking the code to have common > sel_pll() function called by the CEA (HDMI) freq setup and the generic > DMT frequencies setup, we should be able to simply call clk_set_rate() > on the PLL clock handle in a near future. > > The CEA (HDMI) and CVBS modes needs very specific clock paths that CCF will > never be able to determine by itself, so there is still some work to do for > a full handoff to CCF handling the clocks. Patch seems to be a good step forward making the display compatible with CCF indeed. While full automatic handling through CCF might not possible, it would be good if, someday, we could handle the SoC quirks in CCF, removing the need check is the SoC is gxbb, gxl or gxm while setting the clocks. If the display driver needs a detailed control over the clock setup, maybe we could solve the problem by exporting the intermediate clock elements in CCF (such as muxes, ODs, etc...) and let the display driver claim them all ? Anyway, the situation is improving so: Acked-by: Jerome Brunet > > This setup permits setting non-CEA modes like : > - 1600x900-60Hz > - 1280x1024-75Hz > - 1280x1024-60Hz > - 1440x900-60Hz > - 1366x768-60Hz > - 1280x800-60Hz > - 1152x864-75Hz > - 1024x768-75Hz > - 1024x768-70Hz > - 1024x768-60Hz > - 832x624-75Hz > - 800x600-75Hz > - 800x600-72Hz > - 800x600-60Hz > - 640x480-75Hz > - 640x480-73Hz > - 640x480-67Hz > > Signed-off-by: Neil Armstrong > --- > drivers/gpu/drm/meson/meson_dw_hdmi.c | 22 +- > drivers/gpu/drm/meson/meson_vclk.c| 672 > +++--- > drivers/gpu/drm/meson/meson_vclk.h| 4 + > drivers/gpu/drm/meson/meson_venc.c| 377 +++ > drivers/gpu/drm/meson/meson_venc.h| 3 +- > 5 files changed, 358 insertions(+), 720 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c > b/drivers/gpu/drm/meson/meson_dw_hdmi.c > index c9ad456..df7247c 100644 > --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c > +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c > @@ -329,6 +329,12 @@ static void dw_hdmi_set_vclk(struct meson_dw_hdmi > *dw_hdmi, > > vclk_freq = mode->clock; > > + if (!vic) { > + meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, vclk_freq, > + vclk_freq, vclk_freq, false); > + return; > + } > + > if (mode->flags & DRM_MODE_FLAG_DBLCLK) > vclk_freq *= 2; > > @@ -542,10 +548,12 @@ static enum drm_mode_status > dw_hdmi_mode_valid(struct drm_connector *connector, > const struct drm_display_mode *mode) > { > + struct meson_drm *priv = connector->dev->dev_private; > unsigned int vclk_freq; > unsigned int venc_freq; > unsigned int hdmi_freq; > int vic = drm_match_cea_mode(mode); > + enum drm_mode_status status; > > DRM_DEBUG_DRIVER("Modeline %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x > 0x%x\n", > mode->base.id, mode->name, mode->vrefresh, mode->clock, > @@ -556,8 +564,11 @@ dw_hdmi_mode_valid(struct drm_connector *connector, > > /* Check against non-VIC supported modes */ > if (!vic) { > - if (!meson_venc_hdmi_supported_mode(mode)) > - return MODE_BAD; > + status = meson_venc_hdmi_supported_mode(mode); > + if (status != MODE_OK) > + return status; > + > + return meson_vclk_dmt_supported_freq(priv, mode->clock); > /* Check against supported VIC modes */ > } else if (!meson_venc_hdmi_supported_vic(vic)) > return MODE_BAD; > @@ -583,16 +594,11 @@ dw_hdmi_mode_valid(struct drm_connector *connector, > dev_dbg(connector->dev->dev, "%s: vclk:%d venc=%d hdmi=%d\n", __func__, > vclk_freq, venc_freq, hdmi_freq); > > - /* Finally filter by configurable vclk frequencies */ > + /* Finally filter by configurable vclk frequencies for VIC modes */ > switch (vclk_freq) { > - case 25175: > - case 4: > case 54000: > - case 65000: > case 74250: > - case 108000: > case 148500: > - case 162000: > case 297000: > case 594000: > return MODE_OK; > diff --git a/drivers/gpu/drm/meson/meson_vclk.c > b/drivers/gpu/drm/meson/meson_vclk.c > index f051122..0b17788 100644 > --- a/drivers/gpu/drm/meson/meson_vclk.c > +++ b/drivers/gpu/drm/meson/meson_vclk.c > @@ -320,32 +320,23 @@ static void meson_venci_cvbs_clock_config(struct > meson_drm *priv) > CTS_VDAC_EN, CTS_VDAC_EN); > } > > - > +enum { >
[PATCH] drm/i915/selftests: Remove redundant code
err is assigned to -EIO, but this value is never actually used and *err* is updated later on. Remove such reduntant code. Addresses-Coverity-ID: 1471816 ("Unused value") Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/i915/selftests/intel_guc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/selftests/intel_guc.c b/drivers/gpu/drm/i915/selftests/intel_guc.c index 407c98f..c10e75c 100644 --- a/drivers/gpu/drm/i915/selftests/intel_guc.c +++ b/drivers/gpu/drm/i915/selftests/intel_guc.c @@ -206,7 +206,6 @@ static int igt_guc_clients(void *args) if (!available_dbs(guc, guc->execbuf_client->priority)) { pr_err("doorbell not available when it should\n"); - err = -EIO; goto out_db; } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v14 0/2] Add suppport for sn65dsi86 bridge chip
Changes in current patchset: - eDP panels report EDID via DP-AUX channel, so remove support for dedicated DDC line. Sandeep Panda (2): drm/bridge: add support for sn65dsi86 bridge driver dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings .../bindings/display/bridge/ti,sn65dsi86.txt | 87 +++ drivers/gpu/drm/bridge/Kconfig | 9 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/ti-sn65dsi86.c | 676 + 4 files changed, 773 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/5] drm/vkms: Implement CRC debugfs API
On Mon, Jul 16, 2018 at 02:22:56PM -0400, Sean Paul wrote: > On Sat, Jul 14, 2018 at 03:23:32PM +0300, Haneen Mohammed wrote: > > Implement the set_crc_source() callback. > > Compute CRC using crc32 on the visible part of the framebuffer. > > Use ordered workqueue to compute and add CRC at the end of a vblank. > > > > Use appropriate synchronization methods since the crc computation must > > be atomic wrt the generated vblank event for a given atomic update, > > > > Signed-off-by: Haneen Mohammed > > Hey Haneen, > Thanks for revising this patch set. Things are looking good across the series, > just a few more comments :-) > Thank you so much for the review! > > --- > > Changes in v2: > > - Include this patch in this patchset. > > > > drivers/gpu/drm/vkms/Makefile | 2 +- > > drivers/gpu/drm/vkms/vkms_crtc.c | 60 ++- > > drivers/gpu/drm/vkms/vkms_drv.c | 1 + > > drivers/gpu/drm/vkms/vkms_drv.h | 21 +++ > > drivers/gpu/drm/vkms/vkms_plane.c | 10 ++ > > 5 files changed, 85 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile > > index 986297da51bf..37966914f70b 100644 > > --- a/drivers/gpu/drm/vkms/Makefile > > +++ b/drivers/gpu/drm/vkms/Makefile > > @@ -1,3 +1,3 @@ > > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o > > vkms_crc.o > > > > obj-$(CONFIG_DRM_VKMS) += vkms.o > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c > > b/drivers/gpu/drm/vkms/vkms_crtc.c > > index 26babb85ca77..f3a674db33b8 100644 > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > > @@ -10,18 +10,36 @@ > > #include > > #include > > > > -static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) > > +static void _vblank_handle(struct vkms_output *output) > > { > > - struct vkms_output *output = container_of(timer, struct vkms_output, > > - vblank_hrtimer); > > struct drm_crtc *crtc = >crtc; > > - int ret_overrun; > > + struct vkms_crtc_state *state = to_vkms_crtc_state(crtc->state); > > bool ret; > > > > + int crc_enabled = 0; > > + > > + spin_lock(>lock); > > + crc_enabled = output->crc_enabled; > > Aside from the implicit bool -> int cast, I don't think you need this local > var, > just use output->crc_enabled directly below. > > > ret = drm_crtc_handle_vblank(crtc); > > if (!ret) > > DRM_ERROR("vkms failure on handling vblank"); > > This would be more useful with the error code printed. > I think this only returns false on failure. Also I've noticed most of the usage of drm_crtc_handle_vblank don't check the return value, should I do the same as well and drop ret and error message? > > > > + if (state && crc_enabled) { > > + state->n_frame = drm_crtc_accurate_vblank_count(crtc); > > + queue_work(output->crc_workq, >crc_work); > > + } > > + > > + spin_unlock(>lock); > > +} > > + > > +static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) > > +{ > > + struct vkms_output *output = container_of(timer, struct vkms_output, > > + vblank_hrtimer); > > + int ret_overrun; > > + > > + _vblank_handle(output); > > + > > ret_overrun = hrtimer_forward_now(>vblank_hrtimer, > > output->period_ns); > > > > @@ -97,6 +115,8 @@ vkms_atomic_crtc_duplicate_state(struct drm_crtc *crtc) > > > > __drm_atomic_helper_crtc_duplicate_state(crtc, _state->base); > > > > + INIT_WORK(_state->crc_work, vkms_crc_work_handle); > > + > > return _state->base; > > } > > > > @@ -104,11 +124,18 @@ static void vkms_atomic_crtc_destroy_state(struct > > drm_crtc *crtc, > >struct drm_crtc_state *state) > > { > > struct vkms_crtc_state *vkms_state; > > + struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc); > > > > vkms_state = to_vkms_crtc_state(state); > > > > __drm_atomic_helper_crtc_destroy_state(state); > > - kfree(vkms_state); > > + > > + if (vkms_state) { > > + flush_workqueue(vkms_out->crc_workq); > > I'm a little worried about this bit. Since the workqueue is per-output, is it > possible you'll be waiting for more frames to complete than you need to be? > I see, maybe I should flush per work_struct instead? > > + drm_framebuffer_put(_state->data.fb); > > + memset(_state->data, 0, sizeof(struct vkms_crc_data)); > > + kfree(vkms_state); > > + } > > } > > > > static const struct drm_crtc_funcs vkms_crtc_funcs = { > > @@ -120,6 +147,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = { > > .atomic_destroy_state = vkms_atomic_crtc_destroy_state, > > .enable_vblank = vkms_enable_vblank, > >
[PATCH v2] drm/msm/display: negative x/y in cursor move
modesetting X11 driver may provide negative x/y cordinates in mdp5_crtc_cursor_move call when rotation is enabled. Cursor buffer can overlap down to its negative width/height. ROI has to be recalculated for negative x/y indicating using the lower/right corner of the cursor buffer and hotspot must be set in MDP5_LM_CURSOR_XY_SRC_Y MDP5_LM_CURSOR_XY_SRC_X. Signed-off-by: Carsten Behling --- Changes in v2: - fixed format specifier in debug message drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 51 ++- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c index 10271359789e..a7f4a6688fec 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c @@ -65,7 +65,7 @@ struct mdp5_crtc { struct drm_gem_object *scanout_bo; uint64_t iova; uint32_t width, height; - uint32_t x, y; + int x, y; } cursor; }; #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base) @@ -760,20 +760,31 @@ static void get_roi(struct drm_crtc *crtc, uint32_t *roi_w, uint32_t *roi_h) * Cursor Region Of Interest (ROI) is a plane read from cursor * buffer to render. The ROI region is determined by the visibility of * the cursor point. In the default Cursor image the cursor point will -* be at the top left of the cursor image, unless it is specified -* otherwise using hotspot feature. +* be at the top left of the cursor image. * +* Without rotation: * If the cursor point reaches the right (xres - x < cursor.width) or * bottom (yres - y < cursor.height) boundary of the screen, then ROI * width and ROI height need to be evaluated to crop the cursor image * accordingly. * (xres-x) will be new cursor width when x > (xres - cursor.width) * (yres-y) will be new cursor height when y > (yres - cursor.height) +* +* With rotation: +* We get negative x and/or y coordinates. +* (cursor.width - abs(x)) will be new cursor width when x < 0 +* (cursor.height - abs(y)) will be new cursor width when y < 0 */ - *roi_w = min(mdp5_crtc->cursor.width, xres - + if (mdp5_crtc->cursor.x >= 0) + *roi_w = min(mdp5_crtc->cursor.width, xres - mdp5_crtc->cursor.x); - *roi_h = min(mdp5_crtc->cursor.height, yres - + else + *roi_w = mdp5_crtc->cursor.width - abs(mdp5_crtc->cursor.x); + if (mdp5_crtc->cursor.y >= 0) + *roi_h = min(mdp5_crtc->cursor.height, yres - mdp5_crtc->cursor.y); + else + *roi_h = mdp5_crtc->cursor.height - abs(mdp5_crtc->cursor.y); } static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) @@ -783,7 +794,7 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) struct mdp5_kms *mdp5_kms = get_kms(crtc); const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL; uint32_t blendcfg, stride; - uint32_t x, y, width, height; + uint32_t x, y, src_x, src_y, width, height; uint32_t roi_w, roi_h; int lm; @@ -800,6 +811,26 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) get_roi(crtc, _w, _h); + /* If cusror buffer overlaps due to rotation on the +* upper or left screen border the pixel offset inside +* the cursor buffer of the ROI is the positive overlap +* distance. +*/ + if (mdp5_crtc->cursor.x < 0) { + src_x = abs(mdp5_crtc->cursor.x); + x = 0; + } else { + src_x = 0; + } + if (mdp5_crtc->cursor.y < 0) { + src_y = abs(mdp5_crtc->cursor.y); + y = 0; + } else { + src_y = 0; + } + DBG("%s: x=%u, y=%u roi_w=%u roi_h=%u src_x=%u src_y=%u", + crtc->name, x, y, roi_w, roi_h, src_x, src_y); + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride); mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm), MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB)); @@ -812,6 +843,9 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm), MDP5_LM_CURSOR_START_XY_Y_START(y) | MDP5_LM_CURSOR_START_XY_X_START(x)); + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm), + MDP5_LM_CURSOR_XY_SRC_Y(src_y) | + MDP5_LM_CURSOR_XY_SRC_X(src_x)); mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm), mdp5_crtc->cursor.iova); @@ -932,8 +966,9 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) if
Re: [PATCH] drm/amdgpu/pm: Remove VLA usage
On Wed, Jun 20, 2018 at 11:26 AM, Kees Cook wrote: > In the quest to remove all stack VLA usage from the kernel[1], this > uses the maximum sane buffer size and removes copy/paste code. > > [1] > https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com > > Signed-off-by: Kees Cook Friendly ping! Who's tree should this go through? Thanks! -Kees > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 100 +++-- > 1 file changed, 42 insertions(+), 58 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > index b455da487782..5eb98cde22ed 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > @@ -593,40 +593,59 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device > *dev, > return snprintf(buf, PAGE_SIZE, "\n"); > } > > -static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev, > - struct device_attribute *attr, > - const char *buf, > - size_t count) > +/* > + * Worst case: 32 bits individually specified, in octal at 12 characters > + * per line (+1 for \n). > + */ > +#define AMDGPU_MASK_BUF_MAX(32 * 13) > + > +static ssize_t amdgpu_read_mask(const char *buf, size_t count, uint32_t > *mask) > { > - struct drm_device *ddev = dev_get_drvdata(dev); > - struct amdgpu_device *adev = ddev->dev_private; > int ret; > long level; > - uint32_t mask = 0; > char *sub_str = NULL; > char *tmp; > - char buf_cpy[count]; > + char buf_cpy[AMDGPU_MASK_BUF_MAX + 1]; > const char delimiter[3] = {' ', '\n', '\0'}; > + size_t bytes; > > - memcpy(buf_cpy, buf, count+1); > + *mask = 0; > + > + bytes = min(count, sizeof(buf_cpy) - 1); > + memcpy(buf_cpy, buf, bytes); > + buf_cpy[bytes] = '\0'; > tmp = buf_cpy; > while (tmp[0]) { > - sub_str = strsep(, delimiter); > + sub_str = strsep(, delimiter); > if (strlen(sub_str)) { > ret = kstrtol(sub_str, 0, ); > - > - if (ret) { > - count = -EINVAL; > - goto fail; > - } > - mask |= 1 << level; > + if (ret) > + return -EINVAL; > + *mask |= 1 << level; > } else > break; > } > + > + return 0; > +} > + > +static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t count) > +{ > + struct drm_device *ddev = dev_get_drvdata(dev); > + struct amdgpu_device *adev = ddev->dev_private; > + int ret; > + uint32_t mask = 0; > + > + ret = amdgpu_read_mask(buf, count, ); > + if (ret) > + return ret; > + > if (adev->powerplay.pp_funcs->force_clock_level) > amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask); > > -fail: > return count; > } > > @@ -651,32 +670,15 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device > *dev, > struct drm_device *ddev = dev_get_drvdata(dev); > struct amdgpu_device *adev = ddev->dev_private; > int ret; > - long level; > uint32_t mask = 0; > - char *sub_str = NULL; > - char *tmp; > - char buf_cpy[count]; > - const char delimiter[3] = {' ', '\n', '\0'}; > > - memcpy(buf_cpy, buf, count+1); > - tmp = buf_cpy; > - while (tmp[0]) { > - sub_str = strsep(, delimiter); > - if (strlen(sub_str)) { > - ret = kstrtol(sub_str, 0, ); > + ret = amdgpu_read_mask(buf, count, ); > + if (ret) > + return ret; > > - if (ret) { > - count = -EINVAL; > - goto fail; > - } > - mask |= 1 << level; > - } else > - break; > - } > if (adev->powerplay.pp_funcs->force_clock_level) > amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask); > > -fail: > return count; > } > > @@ -701,33 +703,15 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device > *dev, > struct drm_device *ddev = dev_get_drvdata(dev); > struct amdgpu_device *adev = ddev->dev_private; > int ret; > - long level; > uint32_t mask = 0; > - char *sub_str = NULL; > - char *tmp; > - char buf_cpy[count]; > - const char delimiter[3] = {' ', '\n', '\0'}; > - > - memcpy(buf_cpy, buf, count+1); > - tmp = buf_cpy; > > - while (tmp[0]) { > - sub_str = strsep(, delimiter);
Re: [PATCHv3] lib/ratelimit: Lockless ratelimiting
I would be glad if someone helps/bothers to review the change :C Thanks, Dmitry On Tue, 2018-07-03 at 23:56 +0100, Dmitry Safonov wrote: > Currently ratelimit_state is protected with spin_lock. If the .lock > is > taken at the moment of ___ratelimit() call, the message is suppressed > to > make ratelimiting robust. > > That results in the following issue issue: > CPU0 CPU1 > -- -- > printk_ratelimit() printk_ratelimit() > | | > try_spin_lock() try_spin_lock() > | | > time_is_before_jiffies() return 0; // suppress > > So, concurrent call of ___ratelimit() results in silently suppressing > one of the messages, regardless if the limit is reached or not. > And rc->missed is not increased in such case so the issue is covered > from user. > > Convert ratelimiting to use atomic counters and drop spin_lock. > > Note: That might be unexpected, but with the first interval of > messages > storm one can print up to burst*2 messages. So, it doesn't guarantee > that in *any* interval amount of messages is lesser than burst. > But, that differs lightly from previous behavior where one can start > burst=5 interval and print 4 messages on the last milliseconds of > interval + new 5 messages from new interval (totally 9 messages in > lesser than interval value): > >msg0 msg1-msg4 msg0-msg4 > | | | > | | | > |--o-o-|-o|--> (t) > <---> >Lesser than burst > > Dropped dev/random patch since v1 version: > lkml.kernel.org/r/<20180510125211.12583-1-d...@arista.com> > > Dropped `name' in as it's unused in RATELIMIT_STATE_INIT() > > Cc: Andy Shevchenko > Cc: Arnd Bergmann > Cc: David Airlie > Cc: Greg Kroah-Hartman > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Rodrigo Vivi > Cc: "Theodore Ts'o" > Cc: Thomas Gleixner > Cc: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Dmitry Safonov > --- > drivers/char/random.c| 28 --- > drivers/gpu/drm/i915/i915_perf.c | 8 -- > fs/btrfs/super.c | 16 +-- > fs/xfs/scrub/scrub.c | 2 +- > include/linux/ratelimit.h| 31 - > kernel/user.c| 2 +- > lib/ratelimit.c | 59 +++--- > -- > 7 files changed, 73 insertions(+), 73 deletions(-) > > diff --git a/drivers/char/random.c b/drivers/char/random.c > index cd888d4ee605..0be31b3eadab 100644 > --- a/drivers/char/random.c > +++ b/drivers/char/random.c > @@ -439,10 +439,8 @@ static void _crng_backtrack_protect(struct > crng_state *crng, > static void process_random_ready_list(void); > static void _get_random_bytes(void *buf, int nbytes); > > -static struct ratelimit_state unseeded_warning = > - RATELIMIT_STATE_INIT("warn_unseeded_randomness", HZ, 3); > -static struct ratelimit_state urandom_warning = > - RATELIMIT_STATE_INIT("warn_urandom_randomness", HZ, 3); > +static struct ratelimit_state unseeded_warning = > RATELIMIT_STATE_INIT(HZ, 3); > +static struct ratelimit_state urandom_warning = > RATELIMIT_STATE_INIT(HZ, 3); > > static int ratelimit_disable __read_mostly; > > @@ -937,24 +935,22 @@ static void crng_reseed(struct crng_state > *crng, struct entropy_store *r) > crng->init_time = jiffies; > spin_unlock_irqrestore(>lock, flags); > if (crng == _crng && crng_init < 2) { > + unsigned int unseeded_miss, urandom_miss; > + > invalidate_batched_entropy(); > numa_crng_init(); > crng_init = 2; > process_random_ready_list(); > wake_up_interruptible(_init_wait); > pr_notice("random: crng init done\n"); > - if (unseeded_warning.missed) { > - pr_notice("random: %d get_random_xx > warning(s) missed " > - "due to ratelimiting\n", > - unseeded_warning.missed); > - unseeded_warning.missed = 0; > - } > - if (urandom_warning.missed) { > - pr_notice("random: %d urandom warning(s) > missed " > - "due to ratelimiting\n", > - urandom_warning.missed); > - urandom_warning.missed = 0; > - } > + unseeded_miss = > atomic_xchg(_warning.missed, 0); > + if (unseeded_miss) > + pr_notice("random: %u get_random_xx > warning(s) missed " > + "due to ratelimiting\n", > unseeded_miss); > + urandom_miss = atomic_xchg(_warning.missed, > 0); > + if
Re: [PATCH] kernel.h: Add for_each_if()
On Mon, Jul 16 2018, Andy Shevchenko wrote: > On Fri, 2018-07-13 at 16:42 -0700, Randy Dunlap wrote: >> On 07/13/2018 04:37 PM, NeilBrown wrote: > >> >> coding-style.rst says: >> Also, use braces when a loop contains more than a single simple >> statement: > > Independently on a) would we use some macro for condition, or b) fix > macros against this kind of nested conditions, there is another > weirdness we would like to avoid, i.e. > > for_each_foo() { > ... > } else { > ... > } > > It is written according to coding style, but too much weird. Agreed. Too weird. > > So, summarize this discussion I think we would > - keep for_each_if() in DRM subsystem alone > - fix macros which are using positive condition 'if (cond)' by replacing > with 'if (!cond) {} else' form for sake of robustness. > > Do you agree on that? I agree. Thanks, NeilBrown signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v14 2/2] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
Document the bindings used for the sn65dsi86 DSI to eDP bridge. Changes in v1: - Rephrase the dt-binding descriptions to be more inline with existing bindings (Andrzej Hajda). - Add missing dt-binding that are parsed by corresponding driver (Andrzej Hajda). Changes in v2: - Remove edp panel specific dt-binding entries. Only keep bridge specific entries (Sean Paul). - Remove custom-modes dt entry since its usage is removed from driver also (Sean Paul). - Remove is-pluggable dt entry since this will not be needed anymore (Sean Paul). Changes in v3: - Remove irq-gpio dt entry and instead populate is an interrupt property (Rob Herring). Changes in v4: - Add link to bridge chip datasheet (Stephen Boyd) - Add vpll and vcc regulator supply bindings (Stephen Boyd) - Add ref clk optional dt binding (Stephen Boyd) - Add gpio-controller optional dt binding (Stephen Boyd) Changes in v5: - Use clock property to specify the input refclk (Stephen Boyd). - Update gpio cell and pwm cell numbers (Stephen Boyd). Changes in v6: - Add property to mention the lane mapping scheme and polarity inversion (Stephen Boyd). Changes in v7: - Detail description of lane mapping scheme dt property (Andrzej Hajda/ Rob Herring). - Removed HDP gpio binding, since the bridge uses IRQ signal to determine HPD, and IRQ property is already documented in binding. Changes in v8: - Removed unnecessary explanation of lane mapping and polarity dt property, since these are already explained in media/video-interface dt binidng (Rob Herring). Changes in v9: - Avoid putting re-definition of lane mapping and polarity dt binding (Rob Herring). Changes in v10: - Use interrupts-extended property instead of interrupts to specify interrupt line (Andrzej Hajda). - Move data-lanes and lane-polarity property example to proper place (Andrzej Hajda). Changes in v11: - Add a property for suspend gpio function of GPIO1 pin on bridge chip (Stephen Boyd). Changes in v12: - Remove binding for dedicated DDC line (Andrzej Hajda). Signed-off-by: Sandeep Panda --- .../bindings/display/bridge/ti,sn65dsi86.txt | 87 ++ 1 file changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file mode 100644 index ..eac6588ae37a --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt @@ -0,0 +1,87 @@ +SN65DSI86 DSI to eDP bridge chip + + +This is the binding for Texas Instruments SN65DSI86 bridge. +http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf + +Required properties: +- compatible: Must be "ti,sn65dsi86" +- reg: i2c address of the chip, 0x2d as per datasheet +- enable-gpios: OF device-tree gpio specification for bridge_en pin (active high) + +- vccio-supply: A 1.8V supply that powers up the digital IOs. +- vpll-supply: A 1.8V supply that powers up the displayport PLL. +- vcca-supply: A 1.2V supply that powers up the analog circuits. +- vcc-supply: A 1.2V supply that powers up the digital core. + +Optional properties: +- interrupts-extended: Specifier for the SN65DSI86 interrupt line. + +- gpio-controller: Marks the device has a GPIO controller. +- #gpio-cells: Should be two. The first cell is the pin number and + the second cell is used to specify flags. + See ../../gpio/gpio.txt for more information. +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of + the cell formats. + +- clock-names: should be "refclk" +- clocks: Specification for input reference clock. The reference + clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz. + +- data-lanes: See ../../media/video-interface.txt +- lane-polarities: See ../../media/video-interface.txt + +- suspend-gpios: OF device-tree specification for GPIO1 pin on bridge (active low) + +Required nodes: +This device has two video ports. Their connections are modelled using the +OF graph bindings specified in Documentation/devicetree/bindings/graph.txt. + +- Video port 0 for DSI input +- Video port 1 for eDP output + +Example +--- + +edp-bridge@2d { + compatible = "ti,sn65dsi86"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x2d>; + + enable-gpios = < 33 GPIO_ACTIVE_HIGH>; + suspend-gpios = < 34 GPIO_ACTIVE_LOW>; + + interrupts-extended = < 4 IRQ_TYPE_EDGE_FALLING>; + + vccio-supply = <_l17>; + vcca-supply = <_l6>; + vpll-supply = <_l17>; + vcc-supply = <_l6>; + + clock-names = "refclk"; + clocks = <_refclk>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg =
[Bug 105760] [4.17-rc1] RIP: smu7_populate_single_firmware_entry.isra.6+0x57/0xc0 [amdgpu] RSP: ffffa17901efb930
https://bugs.freedesktop.org/show_bug.cgi?id=105760 --- Comment #51 from Thomas Martitz --- Btw, your suggestion to disable runtime pm (amdgpu.runpm=0) doesn't help as far as system suspend/resume is concerned. I think runtime pm generally works, because I see occasional debug outout from smu7_populate_single_firmware_entry() function even before suspending (that suggests to me that the GPU has been suspended regardless of system suspend) -- 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 105760] [4.17-rc1] RIP: smu7_populate_single_firmware_entry.isra.6+0x57/0xc0 [amdgpu] RSP: ffffa17901efb930
https://bugs.freedesktop.org/show_bug.cgi?id=105760 --- Comment #52 from Thomas Martitz --- (In reply to Thomas Martitz from comment #51) > because I see occasional debug outout from > smu7_populate_single_firmware_entry() function even before suspending Forgot to add that the debug output before system suspend doesn't indicate errors (toc->num_entries/toc->structure_version is 0/1 as expected) -- 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: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths
[cc += linux-pm] Hi Lyude, First of all, thanks a lot for looking into this. On Mon, Jul 16, 2018 at 07:59:25PM -0400, Lyude Paul wrote: > In order to fix all of the spots that need to have runtime PM get/puts() > added, we need to ensure that it's possible for us to call > pm_runtime_get/put() in any context, regardless of how deep, since > almost all of the spots that are currently missing refs can potentially > get called in the runtime suspend/resume path. Otherwise, we'll try to > resume the GPU as we're trying to resume the GPU (and vice-versa) and > cause the kernel to deadlock. > > With this, it should be safe to call the pm runtime functions in any > context in nouveau with one condition: any point in the driver that > calls pm_runtime_get*() cannot hold any locks owned by nouveau that > would be acquired anywhere inside nouveau_pmops_runtime_resume(). > This includes modesetting locks, i2c bus locks, etc. [snip] > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *dev) > return -EBUSY; > } > > + dev->power.disable_depth++; > + I'm not sure if that variable is actually private to the PM core. Grepping through the tree I only find a single occurrence where it's accessed outside the PM core and that's in amdgpu. So this looks a little fishy TBH. It may make sense to cc such patches to linux-pm to get Rafael & other folks involved with the PM core to comment. Also, the disable_depth variable only exists if the kernel was compiled with CONFIG_PM enabled, but I can't find a "depends on PM" or something like that in nouveau's Kconfig. Actually, if PM is not selected, all the nouveau_pmops_*() functions should be #ifdef'ed away, but oddly there's no #ifdef CONFIG_PM anywhere in nouveau_drm.c. Anywayn, if I understand the commit message correctly, you're hitting a pm_runtime_get_sync() in a code path that itself is called during a pm_runtime_get_sync(). Could you include stack traces in the commit message? My gut feeling is that this patch masks a deeper issue, e.g. if the runtime_resume code path does in fact directly poll outputs, that would seem wrong. Runtime resume should merely make the card accessible, i.e. reinstate power if necessary, put into PCI_D0, restore registers, etc. Output polling should be scheduled asynchronously. Thanks, Lukas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v14 1/2] drm/bridge: add support for sn65dsi86 bridge driver
Add support for TI's sn65dsi86 dsi2edp bridge chip. The chip converts DSI transmitted signal to eDP signal, which is fed to the connected eDP panel. This chip can be controlled via either i2c interface or dsi interface. Currently in driver all the control registers are being accessed through i2c interface only. Also as of now HPD support has not been added to bridge chip driver. Changes in v1: - Split the dt-bindings and the driver support into separate patches (Andrzej Hajda). - Use of gpiod APIs to parse and configure gpios instead of obsolete ones (Andrzej Hajda). - Use macros to define the register offsets (Andrzej Hajda). Changes in v2: - Separate out edp panel specific HW resource handling from bridge driver and create a separate edp panel drivers to handle panel specific mode information and HW resources (Sean Paul). - Replace pr_* APIs to DRM_* APIs to log error or debug information (Sean Paul). - Remove some of the unnecessary structure/variable from driver (Sean Paul). - Rename the function and structure prefix "sn65dsi86" to "ti_sn_bridge" (Sean Paul / Rob Herring). - Remove most of the hard-coding and modified the bridge init sequence based on current mode (Sean Paul). - Remove the existing function to retrieve the EDID data and implemented this as an i2c_adapter and use drm_get_edid() (Sean Paul). - Remove the dummy irq handler implementation, will add back the proper irq handling later (Sean Paul). - Capture the required enable gpios in a single array based on dt entry instead of having individual descriptor for each gpio (Sean Paul). Changes in v3: - Remove usage of irq_gpio and replace it as "interrupts" property (Rob Herring). - Remove the unnecessary header file inclusions (Sean Paul). - Rearrange the header files in alphabetical order (Sean Paul). - Use regmap interface to perform i2c transactions. - Update Copyright/License field and address other review comments (Jordan Crouse). Changes in v4: - Update License/Copyright (Sean Paul). - Add Kconfig and Makefile changes (Sean Paul). - Drop i2c gpio handling from this bridge driver, since i2c sda/scl gpios will be handled by i2c master. - Update required supplies names. - Remove unnecessary goto statements (Sean Paul). - Add mutex lock to power_ctrl API to avoid race conditions (Sean Paul). - Add support to parse reference clk frequency from dt(optional). - Update the bridge chip enable/disable sequence. Changes in v5: - Fixed Kbuild test service reported warnings. Changes in v6: - Use PM runtime based ref-counting instead of local ref_count mechanism (Stephen Boyd). - Clean up some debug logs and indentations (Sean Paul). - Simplify dp rate calculation (Sean Paul). - Add support to configure refclk based on input REFCLK pin or DACP/N pin (Stephen Boyd). Changes in v7: - Use static supply entries instead of dynamic allocation (Andrzej Hajda). - Defer bridge driver probe if panel is not probed (Andrzej Hajda). - Update of_graph APIs for correct node reference management. (Andrzej Hajda). - Remove local display_mode object (Andrzej Hajda). - Remove version id check function from driver. Changes in v8: - Move dsi register/attach function to bridge driver probe (Andrzej Hajda). - Introduce a new helper function to write 16bit words into consecutive registers (Andrzej Hajda). - Remove unnecessary macros (Andrzej Hajda). Changes in v9: - Remove dsi register/attach from bridge probe, since dsi dev register completion also waits for any panel or bridge to get added. This creates deadlock situation when bridge driver calls dsi dev register and attach before bridge add, in its probe function. - Fix issues faced during testing of bridge driver on actual HW. - Remove unnecessary initializations (Stephen Boyd). - Use local refclk lut size instead of global macro (Sean Paul). Changes in v10: - Use refclk to determine if continuous dsi clock is needed or not. Changes in v11: - Read DPPLL_SRC register to determine continuous clock instead of using refclk handle (Stephen Boyd). Changes in v12: - Explain in comment as in why dsi dev registration is done in bridge_attach (Andrzej Hajda). - Move HPD disable to bridge_pre_enable (Andrzej Hajda). - Make panel/DDC exclusive until HPD support is added (Andrzej Hajda). Changes in v13: - eDP panels report EDID via DP-AUX channel, so remove support for dedicated DDC line (Andrzej Hajda). Signed-off-by: Sandeep Panda --- drivers/gpu/drm/bridge/Kconfig| 9 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-sn65dsi86.c | 676 ++ 3 files changed, 686 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 3b99d5a06c16..8153150acd36 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@
Re: [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Mon, Jul 16, 2018 at 04:12:49PM -0700, Andrew Morton wrote: > On Mon, 16 Jul 2018 13:50:58 +0200 Michal Hocko wrote: > > > From: Michal Hocko > > > > There are several blockable mmu notifiers which might sleep in > > mmu_notifier_invalidate_range_start and that is a problem for the > > oom_reaper because it needs to guarantee a forward progress so it cannot > > depend on any sleepable locks. > > > > Currently we simply back off and mark an oom victim with blockable mmu > > notifiers as done after a short sleep. That can result in selecting a > > new oom victim prematurely because the previous one still hasn't torn > > its memory down yet. > > > > We can do much better though. Even if mmu notifiers use sleepable locks > > there is no reason to automatically assume those locks are held. > > Moreover majority of notifiers only care about a portion of the address > > space and there is absolutely zero reason to fail when we are unmapping an > > unrelated range. Many notifiers do really block and wait for HW which is > > harder to handle and we have to bail out though. > > > > This patch handles the low hanging fruid. > > __mmu_notifier_invalidate_range_start > > gets a blockable flag and callbacks are not allowed to sleep if the > > flag is set to false. This is achieved by using trylock instead of the > > sleepable lock for most callbacks and continue as long as we do not > > block down the call chain. > > I assume device driver developers are wondering "what does this mean > for me". As I understand it, the only time they will see > blockable==false is when their driver is being called in response to an > out-of-memory condition, yes? So it is a very rare thing. I can't say for everyone, but at least for me (mlx5), it is not rare event. I'm seeing OOM very often while I'm running my tests in low memory VMs. Thanks > > Any suggestions regarding how the driver developers can test this code > path? I don't think we presently have a way to fake an oom-killing > event? Perhaps we should add such a thing, given the problems we're > having with that feature. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panel: simple: Support simple VGA panels
On 16.7.2018 11:23, Linus Walleij wrote: The need to support some straight-forward VGA panels that are not adhering to any standard like DPI arise in the ARM RTSM VE Real-Time Systems Model Virtual Executive. This emulator (which is not QEMU) does not model any bridge or panel other than displaying whatever the user defines that they have. Currently a fake "DPI panel" is used for this with the old FBDEV driver, but this is wrong as it is not conforming to any MIPI DPI standards. However the resolution chosen there is standard VGA, and we can cover this with the simple panel by simply adding the most common VGA resolutions as a kind of "simple panel". In difference from other cases where "simple panel" might be hiding something more complex (such as a panel driver or bridge) this case is actually applicable, since we are running inside a simulation with no real hardware on the output. Cc: devicet...@vger.kernel.org Cc: Sudeep Holla Cc: Lorenzo Pieralisi Cc: Liviu Dudau Cc: Robin Murphy Signed-off-by: Linus Walleij --- .../display/panel/video-graphics-array.txt| 21 ++ drivers/gpu/drm/panel/panel-simple.c | 66 +++ 2 files changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/video-graphics-array.txt diff --git a/Documentation/devicetree/bindings/display/panel/video-graphics-array.txt b/Documentation/devicetree/bindings/display/panel/video-graphics-array.txt new file mode 100644 index ..193d24ebe052 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/video-graphics-array.txt @@ -0,0 +1,21 @@ +Video Graphics Array + +This binding is for simple panels using the standardized VGA resolutions +defined by de facto standards beginning with the IBM PS/2 in 1987. + +Required properties: +- compatible: should be "video-graphics-array" + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. + +Example: + +panel { + compatible = "video-graphics-array"; + port { + vga_panel_in: endpoint { + remote-endpoint = <>; + }; + }; +}; diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index cbf1ab404ee7..db4db5a3f75e 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -2069,6 +2069,69 @@ static const struct panel_desc winstar_wf35ltiacd = { .bus_format = MEDIA_BUS_FMT_RGB888_1X24, }; +static const struct drm_display_mode video_graphics_array_modes[] = { + { + /* +* This is the most standardized "vanilla" VGA mode there is: +* 640x480 @ 60 Hz +*/ + .clock = 27175, + .hdisplay = 640, + .hsync_start = 640 + 16, + .hsync_end = 640 + 16 + 96, + .htotal = 640 + 16 + 96 + 48, + .vdisplay = 480, + .vsync_start = 480 + 10, + .vsync_end = 480 + 10 + 2, + .vtotal = 480 + 10 + 2 + 33, + .vrefresh = 60, + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, + }, + { + /* SVGA 800x600 @60 Hz */ + .clock = 4, + .hdisplay = 800, + .hsync_start = 800 + 40, + .hsync_end = 800 + 40 + 128, + .htotal = 800 + 40 + 128 + 88, + .vdisplay = 600, + .vsync_start = 600 + 1, + .vsync_end = 600 + 1 + 4, + .vtotal = 600 + 1 + 4 + 23, + .vrefresh = 60, + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, + }, + { + /* XGA 1024x768 @60 Hz */ + .clock = 65000, + .hdisplay = 1024, + .hsync_start = 1024 + 24, + .hsync_end = 1024 + 24 + 136, + .htotal = 1024 + 24 + 136 + 160, + .vdisplay = 768, + .vsync_start = 768 + 3, + .vsync_end = 768 + 3 + 6, + .vtotal = 768 + 3 + 6 + 29, + .vrefresh = 60, + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, + }, +}; + +static const struct panel_desc video_graphics_array = { + .modes = video_graphics_array_modes, + .num_modes = ARRAY_SIZE(video_graphics_array_modes), + .bpc = 8, + /* +* Some typical 4:3 aspect ratio VGA monitor surely has these dimensions +* of 40x30 mm. I am sure it has not :) s/mm/cm or add a zero. Michal +*/ + .size = { + .width = 400, + .height = 300, + }, + .bus_format = MEDIA_BUS_FMT_RGB888_1X24, +}; + static const struct of_device_id platform_of_match[] = { { .compatible = "ampire,am-480272h3tmqw-t01h", @@ -2286,6 +2349,9 @@ static const struct
[PATCH] drm/msm/display: negative x/y in cursor move
modesetting X11 driver may provide negative x/y cordinates in mdp5_crtc_cursor_move call when rotation is enabled. Cursor buffer can overlap down to its negative width/height. ROI has to be recalculated for negative x/y indicating using the lower/right corner of the cursor buffer and hotspot must be set in MDP5_LM_CURSOR_XY_SRC_Y MDP5_LM_CURSOR_XY_SRC_X. Signed-off-by: Carsten Behling --- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 51 ++- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c index 10271359789e..43a86582876c 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c @@ -65,7 +65,7 @@ struct mdp5_crtc { struct drm_gem_object *scanout_bo; uint64_t iova; uint32_t width, height; - uint32_t x, y; + int x, y; } cursor; }; #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base) @@ -760,20 +760,31 @@ static void get_roi(struct drm_crtc *crtc, uint32_t *roi_w, uint32_t *roi_h) * Cursor Region Of Interest (ROI) is a plane read from cursor * buffer to render. The ROI region is determined by the visibility of * the cursor point. In the default Cursor image the cursor point will -* be at the top left of the cursor image, unless it is specified -* otherwise using hotspot feature. +* be at the top left of the cursor image. * +* Without rotation: * If the cursor point reaches the right (xres - x < cursor.width) or * bottom (yres - y < cursor.height) boundary of the screen, then ROI * width and ROI height need to be evaluated to crop the cursor image * accordingly. * (xres-x) will be new cursor width when x > (xres - cursor.width) * (yres-y) will be new cursor height when y > (yres - cursor.height) +* +* With rotation: +* We get negative x and/or y coordinates. +* (cursor.width - abs(x)) will be new cursor width when x < 0 +* (cursor.height - abs(y)) will be new cursor width when y < 0 */ - *roi_w = min(mdp5_crtc->cursor.width, xres - + if (mdp5_crtc->cursor.x >= 0) + *roi_w = min(mdp5_crtc->cursor.width, xres - mdp5_crtc->cursor.x); - *roi_h = min(mdp5_crtc->cursor.height, yres - + else + *roi_w = mdp5_crtc->cursor.width - abs(mdp5_crtc->cursor.x); + if (mdp5_crtc->cursor.y >= 0) + *roi_h = min(mdp5_crtc->cursor.height, yres - mdp5_crtc->cursor.y); + else + *roi_h = mdp5_crtc->cursor.height - abs(mdp5_crtc->cursor.y); } static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) @@ -783,7 +794,7 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) struct mdp5_kms *mdp5_kms = get_kms(crtc); const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL; uint32_t blendcfg, stride; - uint32_t x, y, width, height; + uint32_t x, y, src_x, src_y, width, height; uint32_t roi_w, roi_h; int lm; @@ -800,6 +811,26 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) get_roi(crtc, _w, _h); + /* If cusror buffer overlaps due to rotation on the +* upper or left screen border the pixel offset inside +* the cursor buffer of the ROI is the positive overlap +* distance. +*/ + if (mdp5_crtc->cursor.x < 0) { + src_x = abs(mdp5_crtc->cursor.x); + x = 0; + } else { + src_x = 0; + } + if (mdp5_crtc->cursor.y < 0) { + src_y = abs(mdp5_crtc->cursor.y); + y = 0; + } else { + src_y = 0; + } + DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d", + x, y, roi_w, roi_h, src_x, src_y); + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride); mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm), MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB)); @@ -812,6 +843,9 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm), MDP5_LM_CURSOR_START_XY_Y_START(y) | MDP5_LM_CURSOR_START_XY_X_START(x)); + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm), + MDP5_LM_CURSOR_XY_SRC_Y(src_y) | + MDP5_LM_CURSOR_XY_SRC_X(src_x)); mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm), mdp5_crtc->cursor.iova); @@ -932,8 +966,9 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) if (unlikely(!crtc->state->enable)) return 0; -
Re: [PATCH] drm/i2c: tda9950: Remove VLA usage
On Wed, Jun 20, 2018 at 11:27 AM, Kees Cook wrote: > In the quest to remove all stack VLA usage from the kernel[1], this > sets the buffer to maximum size and adds a sanity check. > > [1] > https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com > > Signed-off-by: Kees Cook Friendly ping! Who's tree should this go through? Thanks! -Kees > --- > drivers/gpu/drm/i2c/tda9950.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c > index 3f7396caad48..28314433c351 100644 > --- a/drivers/gpu/drm/i2c/tda9950.c > +++ b/drivers/gpu/drm/i2c/tda9950.c > @@ -76,9 +76,12 @@ struct tda9950_priv { > static int tda9950_write_range(struct i2c_client *client, u8 addr, u8 *p, > int cnt) > { > struct i2c_msg msg; > - u8 buf[cnt + 1]; > + u8 buf[CEC_MAX_MSG_SIZE + 3]; > int ret; > > + if (WARN_ON(cnt > sizeof(buf))) > + return -EINVAL; > + > buf[0] = addr; > memcpy(buf + 1, p, cnt); > > -- > 2.17.1 > > > -- > Kees Cook > Pixel Security -- Kees Cook Pixel Security ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] kernel.h: Add for_each_if()
On 07/16/2018 01:11 AM, Andy Shevchenko wrote: > On Fri, 2018-07-13 at 16:42 -0700, Randy Dunlap wrote: >> On 07/13/2018 04:37 PM, NeilBrown wrote: > >> >> coding-style.rst says: >> Also, use braces when a loop contains more than a single simple >> statement: > > Independently on a) would we use some macro for condition, or b) fix > macros against this kind of nested conditions, there is another > weirdness we would like to avoid, i.e. > > for_each_foo() { > ... > } else { > ... > } > > It is written according to coding style, but too much weird. Yeah, that's odd. Looks like else matches the for loop. (!) > So, summarize this discussion I think we would > - keep for_each_if() in DRM subsystem alone > - fix macros which are using positive condition 'if (cond)' by replacing > with 'if (!cond) {} else' form for sake of robustness. > > Do you agree on that? Sure, both of those sound good to me. thanks, -- ~Randy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] ARM: dts: opos6uldev: use OF graph to describe the display
To make use of the new eLCDIF DRM driver OF graph description is required. Describe the display using OF graph nodes. Signed-off-by: Sébastien Szymanski --- arch/arm/boot/dts/imx6ul-opos6uldev.dts | 37 ++--- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/arch/arm/boot/dts/imx6ul-opos6uldev.dts b/arch/arm/boot/dts/imx6ul-opos6uldev.dts index 0e59ee57fd55..8ecdb9ad2b2e 100644 --- a/arch/arm/boot/dts/imx6ul-opos6uldev.dts +++ b/arch/arm/boot/dts/imx6ul-opos6uldev.dts @@ -56,7 +56,7 @@ stdout-path = }; - backlight { + backlight: backlight { compatible = "pwm-backlight"; pwms = < 0 191000>; brightness-levels = <0 4 8 16 32 64 128 255>; @@ -97,6 +97,18 @@ gpios = < 1 GPIO_ACTIVE_HIGH>; }; + panel: panel { + compatible = "armadeus,st0700-adapt"; + power-supply = <_3v3>; + backlight = <>; + + port { + panel_in: endpoint { + remote-endpoint = <_out>; + }; + }; + }; + reg_5v: regulator-5v { compatible = "regulator-fixed"; regulator-name = "5V"; @@ -182,28 +194,11 @@ { pinctrl-names = "default"; pinctrl-0 = <_lcdif>; - display = <>; - lcd-supply = <_3v3>; status = "okay"; - display0: display0 { - bits-per-pixel = <32>; - bus-width = <18>; - - display-timings { - timing0: timing0 { - clock-frequency = <3333>; - hactive = <800>; - vactive = <480>; - hback-porch = <96>; - hfront-porch = <96>; - vback-porch = <20>; - vfront-porch = <21>; - hsync-len = <64>; - vsync-len = <4>; - de-active = <1>; - pixelclk-active = <0>; - }; + port { + lcdif_out: endpoint { + remote-endpoint = <_in>; }; }; }; -- 2.16.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] kernel.h: Add for_each_if()
On Fri, 2018-07-13 at 16:42 -0700, Randy Dunlap wrote: > On 07/13/2018 04:37 PM, NeilBrown wrote: > > coding-style.rst says: > Also, use braces when a loop contains more than a single simple > statement: Independently on a) would we use some macro for condition, or b) fix macros against this kind of nested conditions, there is another weirdness we would like to avoid, i.e. for_each_foo() { ... } else { ... } It is written according to coding style, but too much weird. So, summarize this discussion I think we would - keep for_each_if() in DRM subsystem alone - fix macros which are using positive condition 'if (cond)' by replacing with 'if (!cond) {} else' form for sake of robustness. Do you agree on that? -- Andy Shevchenko Intel Finland Oy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vgem: Replace drm_dev_unref with drm_dev_put
On Mon, Jul 16, 2018 at 1:16 PM, Thomas Zimmermann wrote: > This patch unifies the naming of DRM functions for reference counting > of struct drm_device. The resulting code is more aligned with the rest > of the Linux kernel interfaces. > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/vgem/vgem_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c > index 0e5620f76ee0..ec6af8b920da 100644 > --- a/drivers/gpu/drm/vgem/vgem_drv.c > +++ b/drivers/gpu/drm/vgem/vgem_drv.c > @@ -504,7 +504,7 @@ static int __init vgem_init(void) > static void __exit vgem_exit(void) > { > drm_dev_unregister(_device->drm); > - drm_dev_unref(_device->drm); > + drm_dev_put(_device->drm); > } > > module_init(vgem_init); > -- > 2.18.0 > There are other gpu/drm drivers where drm_dev_unref is used. Do we need to replace drm_dev_unref with drm_dev_put in other places as well ? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit
Blending order is set based on the z position of each DRM plane. The blending order register is currently cleared at each atomic DRM commit, with the intent that each committed plane will set the appropriate bits (based on its z-pos) when enabling the plane. However, it sometimes happens that a particular plane is left unchanged by an atomic commit and thus will not be configured again. In that scenario, blending order is cleared and only the bits relevant for the planes affected by the commit are set. This leaves the planes that did not change without their blending order set in the register, leading to that plane not being displayed. Instead of clearing the blending order register at every atomic commit, this change moves the register's initial clear at bind time and only clears the bits for a specific plane when disabling it or changing its zpos. This way, planes that are left untouched by a DRM atomic commit are no longer disabled. Signed-off-by: Paul Kocialkowski --- drivers/gpu/drm/sun4i/sun8i_mixer.c| 11 +++ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 24 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 24 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 9bd069de3124..12cb7183ce51 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -260,13 +260,6 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) return NULL; } -static void sun8i_mixer_atomic_begin(struct sunxi_engine *engine, -struct drm_crtc_state *old_state) -{ - regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_PIPE_CTL, - SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0); -} - static void sun8i_mixer_commit(struct sunxi_engine *engine) { DRM_DEBUG_DRIVER("Committing changes\n"); @@ -318,7 +311,6 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm, } static const struct sunxi_engine_ops sun8i_engine_ops = { - .atomic_begin = sun8i_mixer_atomic_begin, .commit = sun8i_mixer_commit, .layers_init= sun8i_layers_init, }; @@ -445,6 +437,9 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_MODE(i), SUN8I_MIXER_BLEND_MODE_DEF); + regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL, + SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0); + return 0; err_disable_bus_clk: diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 1a12cb02f2d2..f98fd9a9dd78 100644 --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c @@ -27,7 +27,8 @@ #include "sun8i_ui_scaler.h" static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel, - int overlay, bool enable, unsigned int zpos) + int overlay, bool enable, unsigned int zpos, + unsigned int old_zpos) { u32 val; @@ -43,6 +44,18 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel, SUN8I_MIXER_CHAN_UI_LAYER_ATTR(channel, overlay), SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val); + if (!enable || zpos != old_zpos) { + regmap_update_bits(mixer->engine.regs, + SUN8I_MIXER_BLEND_PIPE_CTL, + SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos), + 0); + + regmap_update_bits(mixer->engine.regs, + SUN8I_MIXER_BLEND_ROUTE, + SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos), + 0); + } + if (enable) { val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos); @@ -242,9 +255,11 @@ static void sun8i_ui_layer_atomic_disable(struct drm_plane *plane, struct drm_plane_state *old_state) { struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane); + unsigned int old_zpos = old_state->normalized_zpos; struct sun8i_mixer *mixer = layer->mixer; - sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0); + sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0, + old_zpos); } static void sun8i_ui_layer_atomic_update(struct drm_plane *plane, @@ -252,11 +267,12 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane *plane, { struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane); unsigned int zpos = plane->state->normalized_zpos; + unsigned int old_zpos =
[Bug 107261] [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 1us * 10 tries - optc1_lock line:628
https://bugs.freedesktop.org/show_bug.cgi?id=107261 Bug ID: 107261 Summary: [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 1us * 10 tries - optc1_lock line:628 Product: DRI Version: DRI git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: pmenzel+bugs.freedesk...@molgen.mpg.de Created attachment 140664 --> https://bugs.freedesktop.org/attachment.cgi?id=140664=edit Linux 4.18-rc5+ messages On a MSI B350M MORTAR with Linux 4.18-rc5+ and Debian Sid/unstable with GNOME I get the error below. ``` $ git log --oneline -1 30b06abfb92b (HEAD -> master, origin/master, origin/HEAD) Merge tag 'pinctrl-v4.18-3' of git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl $ git describe --dirty v4.18-rc5-36-g30b06abfb92b $ sudo dmesg […] [ 171.571430] [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 1us * 10 tries - optc1_lock line:628 [ 171.571485] WARNING: CPU: 2 PID: 917 at drivers/gpu/drm/amd/amdgpu/../display/dc/dc_helper.c:254 generic_reg_wait+0xe7/0x160 [amdgpu] [ 171.571486] Modules linked in: fuse nls_ascii amdkfd nls_cp437 vfat fat wmi_bmof ppdev amdgpu edac_mce_amd ccp rng_core chash snd_hda_codec_realtek efi_pstore gpu_sched kvm ttm irqbypass snd_hda_codec_generic snd_hda_codec_hdmi crct10dif_pclmul sp5100_tco crc32_pclmul pcspkr snd_hda_intel i2c_piix4 ghash_clmulni_intel drm_kms_helper snd_hda_codec snd_hda_core efivars sg snd_hwdep k10temp r8169 snd_pcm drm mii snd_timer snd i2c_algo_bit soundcore parport_pc wmi parport video button acpi_cpufreq efivarfs ip_tables x_tables autofs4 ext4 crc16 mbcache jbd2 fscrypto dm_crypt dm_mod raid10 raid456 libcrc32c crc32c_generic async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 multipath linear md_mod sd_mod evdev hid_generic usbhid hid crc32c_intel ahci libahci xhci_pci aesni_intel [ 171.571521] aes_x86_64 xhci_hcd crypto_simd cryptd glue_helper libata usbcore scsi_mod gpio_amdpt gpio_generic [ 171.571527] CPU: 2 PID: 917 Comm: gnome-shell Not tainted 4.18.0-rc5+ #1 [ 171.571528] Hardware name: MSI MS-7A37/B350M MORTAR (MS-7A37), BIOS 1.F0 04/27/2018 [ 171.571569] RIP: 0010:generic_reg_wait+0xe7/0x160 [amdgpu] [ 171.571569] Code: 44 24 58 8b 54 24 48 89 de 44 89 4c 24 08 48 8b 4c 24 50 48 c7 c7 10 2e b5 c0 e8 f4 e1 ac ff 83 7d 20 01 44 8b 4c 24 08 74 02 <0f> 0b 48 83 c4 10 44 89 c8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 41 0f [ 171.571585] RSP: 0018:b2f003d1f920 EFLAGS: 00010297 [ 171.571587] RAX: RBX: 0001 RCX: [ 171.571587] RDX: RSI: 9d185ec96738 RDI: 9d185ec96738 [ 171.571588] RBP: 9d1843a49800 R08: 0392 R09: 0001 [ 171.571588] R10: R11: b4fcaf4d R12: 000b [ 171.571589] R13: 504d R14: 0100 R15: 0001 [ 171.571590] FS: 7f7488a34ac0() GS:9d185ec8() knlGS: [ 171.571591] CS: 0010 DS: ES: CR0: 80050033 [ 171.571592] CR2: 01b1baab4488 CR3: 0003dc648000 CR4: 003406e0 [ 171.571592] Call Trace: [ 171.571640] optc1_lock+0xa0/0xb0 [amdgpu] [ 171.571684] dcn10_pipe_control_lock.part.28+0x4a/0x70 [amdgpu] [ 171.571728] dcn10_apply_ctx_for_surface+0xee/0x1210 [amdgpu] [ 171.571731] ? _cond_resched+0x15/0x30 [ 171.571774] ? hubbub1_verify_allow_pstate_change_high+0xdd/0x180 [amdgpu] [ 171.571817] ? dcn10_verify_allow_pstate_change_high+0x1d/0x240 [amdgpu] [ 171.571859] ? dcn10_set_bandwidth+0x275/0x2d0 [amdgpu] [ 171.571900] dc_commit_state+0x253/0x550 [amdgpu] [ 171.571940] ? set_freesync_on_streams.part.6+0x4d/0x250 [amdgpu] [ 171.571979] ? mod_freesync_set_user_enable+0x11f/0x150 [amdgpu] [ 171.572023] amdgpu_dm_atomic_commit_tail+0x37c/0xd70 [amdgpu] [ 171.572058] ? amdgpu_bo_pin_restricted+0xd6/0x300 [amdgpu] [ 171.572060] ? _cond_resched+0x15/0x30 [ 171.572061] ? wait_for_completion_timeout+0x3b/0x1a0 [ 171.572063] ? refcount_inc+0x5/0x30 [ 171.572070] commit_tail+0x3d/0x70 [drm_kms_helper] [ 171.572075] drm_atomic_helper_commit+0xb4/0x120 [drm_kms_helper] [ 171.572087] drm_atomic_connector_commit_dpms+0xdb/0x100 [drm] [ 171.572097] drm_mode_obj_set_property_ioctl+0x174/0x270 [drm] [ 171.572107] ? drm_mode_obj_find_prop_id+0x40/0x40 [drm] [ 171.572116] drm_ioctl_kernel+0xa1/0xf0 [drm] [ 171.572126] drm_ioctl+0x1fc/0x390 [drm] [ 171.572135] ? drm_mode_obj_find_prop_id+0x40/0x40 [drm] [ 171.572138] ? eventfd_read+0xed/0x2a0 [ 171.572171] amdgpu_drm_ioctl+0x49/0x80 [amdgpu] [ 171.572174] do_vfs_ioctl+0xa4/0x620 [ 171.572176] ksys_ioctl+0x60/0x90 [ 171.572178] ? ksys_read+0x9c/0xb0 [ 171.572179] __x64_sys_ioctl+0x16/0x20 [ 171.572182]
Re: [Nouveau] [PATCH 4/5] drm/nouveau: Grab RPM ref when i2c bus is in use
On Tue, 17 Jul 2018 at 20:18, Karol Herbst wrote: > > mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd > the Linux glue code to the i2c stuff instead, but this is all done > from inside of nvkm. I think we should move it out into > drm/nouveau/nouveau_i2c.c and do the handling there. Huh? No, this is completely fine. > > On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul wrote: > > The i2c bus can be both accessed by DRM itself, along with any of it's > > devnodes (/sys/class/i2c). So, we need to make sure that all codepaths > > using the i2c bus keep the GPU resumed. > > > > Signed-off-by: Lyude Paul > > Cc: Karol Herbst > > Cc: sta...@vger.kernel.org > > --- > > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > > b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > > index 807a2b67bd64..1de48c990b80 100644 > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > > @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus) > > BUS_TRACE(bus, "release"); > > nvkm_i2c_pad_release(pad); > > mutex_unlock(>mutex); > > + pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev); > > } > > > > int > > nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus) > > { > > struct nvkm_i2c_pad *pad = bus->pad; > > + struct device *dev = pad->i2c->subdev.device->dev; > > int ret; > > + > > BUS_TRACE(bus, "acquire"); > > + > > + ret = pm_runtime_get_sync(dev); > > + if (ret < 0 && ret != -EACCES) > > + return ret; > > + > > mutex_lock(>mutex); > > ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C); > > - if (ret) > > + if (ret) { > > mutex_unlock(>mutex); > > + pm_runtime_put_autosuspend(dev); > > + } > > return ret; > > } > > > > -- > > 2.17.1 > > > > ___ > > Nouveau mailing list > > nouv...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/nouveau > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/suni4: Replace drm_dev_unref with drm_dev_put
On Tue, Jul 17, 2018 at 10:48:14AM +0200, Thomas Zimmermann wrote: > This patch unifies the naming of DRM functions for reference counting > of struct drm_device. The resulting code is more aligned with the rest > of the Linux kernel interfaces. > > Signed-off-by: Thomas Zimmermann Applied, thanks! Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH v2 2/3] drm/nouveau: Fix runtime PM leak in nv50_disp_atomic_commit()
On Tue, Jul 17, 2018 at 09:33:52AM +0200, Lukas Wunner wrote: > On Thu, Jul 12, 2018 at 01:02:53PM -0400, Lyude Paul wrote: > > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > > @@ -1878,7 +1878,7 @@ nv50_disp_atomic_commit(struct drm_device *dev, > > nv50_disp_atomic_commit_tail(state); > > > > drm_for_each_crtc(crtc, dev) { > > - if (crtc->state->enable) { > > + if (crtc->state->active) { > > if (!drm->have_disp_power_ref) { > > drm->have_disp_power_ref = true; > > return 0; > > Somewhat tangential comment on this older patch, since you > continue to dig around in the runtime PM area: > > Whenever a crtc is activated or deactivated in nouveau, we iterate > over all crtcs and acquire a runtime PM if a crtc is active and > previously there was no active one, or we drop a ref if none is > active and previously there was an active one. > > For a while now I've been thinking that it would be more straightforward > to acquire a ref whenever a crtc is activated and drop one when a crtc > is deactivated, i.e. hold one ref for every active crtc. That way the > have_disp_power_ref variable as well as the iteration logic could be > removed, leading to a simplification. Just a suggestion anyway. The current code looks somewhat busted anyway. First problem is that it's accessing crtc->state without the appropriate locks held (unless something always pulls in all crtcs to every commit?). Second issue is that the rpm_put() is called without waiting for nonblocking commits to have finished so it looks like you can currentlly remove the power before the hardware has been properly shut down. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 08/11] media: vsp1: Add support for extended display list headers
Hi Kieran, On Monday, 16 July 2018 20:14:55 EEST Kieran Bingham wrote: > On 24/05/18 12:44, Laurent Pinchart wrote: > > On Thursday, 3 May 2018 16:36:19 EEST Kieran Bingham wrote: > >> Extended display list headers allow pre and post command lists to be > >> executed by the VSP pipeline. This provides the base support for > >> features such as AUTO_FLD (for interlaced support) and AUTO_DISP (for > >> supporting continuous camera preview pipelines. > >> > >> Signed-off-by: Kieran Bingham > >> > >> --- > >> > >> v2: > >> - remove __packed attributes > >> > >> --- > >> > >> drivers/media/platform/vsp1/vsp1.h | 1 +- > >> drivers/media/platform/vsp1/vsp1_dl.c | 83 +- > >> drivers/media/platform/vsp1/vsp1_dl.h | 29 - > >> drivers/media/platform/vsp1/vsp1_drv.c | 7 +- > >> drivers/media/platform/vsp1/vsp1_regs.h | 5 +- > >> 5 files changed, 116 insertions(+), 9 deletions(-) [snip] > >> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > >> b/drivers/media/platform/vsp1/vsp1_dl.c index 56514cd51c51..b64d32535edc > >> 100644 > >> --- a/drivers/media/platform/vsp1/vsp1_dl.c > >> +++ b/drivers/media/platform/vsp1/vsp1_dl.c [snip] > >> +struct vsp1_dl_ext_header { > >> + u32 reserved0; /* alignment padding */ > >> + > >> + u16 pre_ext_cmd_qty; > > > > Should this be called pre_ext_dl_num_cmd to match the datasheet ? > > Yes, renamed. > > >> + u16 flags; > > > > Aren't the flags supposed to come before the pre_ext_dl_num_cmd field ? > > These are out-of-order to account for the fact that they are 16bit values. Ah OK. It makes sense, but is a bit confusing when reading the datasheet. > I felt that keeping them described in the struct was cleaner than shifts > and masks - but clearly this stands out, due to the byte-ordering. > > Would you prefer I re-write this as 32 bit accesses (or even 64bit), > with shifts? Or is a comment sufficient here ? If it doesn't make the code too ugly, using larger accesses would be better I think. Otherwise a comment would do I suppose. > If we rewrite to be 32 bit accesses, would you be happy with the > following naming: > > u32 reserved0; > u32 pre_ext_dl_num_cmd; /* Also stores command flags. */ > u32 pre_ext_dl_plist; > u32 post_ext_dl_num_cmd; > u32 post_ext_dl_plist; > > (Technically the flags are for the whole header, not the just the > pre_ext, which is why I wanted it separated) > > > Actually - now I write that - the 'number of commands' is sort of a flag > or a parameter? so maybe the following is just as appropriate?: > > u32 reserved0; Maybe "u32 zero;" or "u32 padding;" ? The datasheet states this is padding for alignment purpose. > u32 pre_ext_dl_flags; > u32 pre_ext_dl_plist; > u32 post_ext_dl_flags; > u32 post_ext_dl_plist; > > Or they could be named 'options', or parameters? > > Any comments before I hack that in? > > The annoying part is that the 'flags' aren't part of the pre_ext cmds, > they declare whether the pre or post cmd should be executed (or both I > presume, we are yet to see post-cmd usage) I agree with you, having a separate flag field would be nicer, as the flags are shared. I'll chose the easy option of letting you decide what you like best :-) All the above options are equally good to me, provided you add a comment explaining why the flag comes after the num_cmd field if you decide to keep it as a separate field. > >> + u32 pre_ext_cmd_plist; > > > > And pre_ext_dl_plist ? > > > >> + > >> + u32 post_ext_cmd_qty; > >> + u32 post_ext_cmd_plist; > > > > Similar comments for these variables. > > Renamed. > > >> +}; > >> + > >> +struct vsp1_dl_header_extended { > >> + struct vsp1_dl_header header; > >> + struct vsp1_dl_ext_header ext; > >> +}; > >> + > >> struct vsp1_dl_entry { > >>u32 addr; > >>u32 data; > >> }; > >> > >> +struct vsp1_dl_ext_cmd_header { > > > > Isn't this referred to in the datasheet as a body entry, not a header ? > > How about naming it vsp1_dl_ext_cmd_entry ? Or just vsp1_dl_ext_cmd (in > > which case the other structure that goes by the same name would need to be > > renamed) ? > > I think I was getting too creative. The reality is this part is really a > 'header' describing the data in the body, but yes - it should be named > to match a "Pre Extended Display List Body" > > s/vsp1_dl_ext_cmd_header/vsp1_pre_ext_dl_body/ Sounds good to me. > This will then leave "struct vsp1_dl_ext_cmd" which represents the > object instance within the VSP1 driver only. > > >> + u32 cmd; > > This should really have been opcode then too :) Good point. > >> + u32 flags; > >> + u32 data; > >> + u32 reserved; > > > > The datasheet documents this as two 64-bit fields, shouldn't we handle the > > structure the same way ? > > I was trying to separate out the fields for clarity. > > In this instance (unlike the 16bit handling above), the byte ordering of > a 64
[PATCH] drm/rockchip: Replace drm_dev_unref with drm_dev_put
This patch unifies the naming of DRM functions for reference counting of struct drm_device. The resulting code is more aligned with the rest of the Linux kernel interfaces. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index f814d37b1db2..9c846be8fc64 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -184,7 +184,7 @@ static int rockchip_drm_bind(struct device *dev) err_free: drm_dev->dev_private = NULL; dev_set_drvdata(dev, NULL); - drm_dev_unref(drm_dev); + drm_dev_put(drm_dev); return ret; } @@ -204,7 +204,7 @@ static void rockchip_drm_unbind(struct device *dev) drm_dev->dev_private = NULL; dev_set_drvdata(dev, NULL); - drm_dev_unref(drm_dev); + drm_dev_put(drm_dev); } static const struct file_operations rockchip_drm_driver_fops = { -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/zte: Replace drm_dev_unref with drm_dev_put
This patch unifies the naming of DRM functions for reference counting of struct drm_device. The resulting code is more aligned with the rest of the Linux kernel interfaces. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/zte/zx_drm_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c index 6f4205e80378..02ae1caf6e8a 100644 --- a/drivers/gpu/drm/zte/zx_drm_drv.c +++ b/drivers/gpu/drm/zte/zx_drm_drv.c @@ -122,7 +122,7 @@ static int zx_drm_bind(struct device *dev) component_unbind_all(dev, drm); out_unregister: dev_set_drvdata(dev, NULL); - drm_dev_unref(drm); + drm_dev_put(drm); return ret; } @@ -136,7 +136,7 @@ static void zx_drm_unbind(struct device *dev) drm_mode_config_cleanup(drm); component_unbind_all(dev, drm); dev_set_drvdata(dev, NULL); - drm_dev_unref(drm); + drm_dev_put(drm); } static const struct component_master_ops zx_drm_master_ops = { -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit
On Tue, 2018-07-17 at 14:25 +0200, Paul Kocialkowski wrote: > Blending order is set based on the z position of each DRM plane. The > blending order register is currently cleared at each atomic DRM commit, > with the intent that each committed plane will set the appropriate > bits (based on its z-pos) when enabling the plane. > > However, it sometimes happens that a particular plane is left unchanged > by an atomic commit and thus will not be configured again. In that > scenario, blending order is cleared and only the bits relevant for the > planes affected by the commit are set. This leaves the planes that did > not change without their blending order set in the register, leading > to that plane not being displayed. > > Instead of clearing the blending order register at every atomic commit, > this change moves the register's initial clear at bind time and only > clears the bits for a specific plane when disabling it or changing its > zpos. > > This way, planes that are left untouched by a DRM atomic commit are > no longer disabled. This patch was rebased to apply on top of DRM misc. V1 had been based on the first revision of the DE2 z-pos support patch, while a subsequent revision of the patch made it to the kernel tree. Cheers! Paul > Signed-off-by: Paul Kocialkowski > --- > drivers/gpu/drm/sun4i/sun8i_mixer.c| 15 +++ > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 24 > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 24 > 3 files changed, 43 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c > b/drivers/gpu/drm/sun4i/sun8i_mixer.c > index 8e81c24d736e..12cb7183ce51 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > @@ -260,17 +260,6 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 > format) > return NULL; > } > > -static void sun8i_mixer_atomic_begin(struct sunxi_engine *engine, > - struct drm_crtc_state *old_state) > -{ > - /* > - * Disable all pipes at the beginning. They will be enabled > - * again if needed in plane update callback. > - */ > - regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_PIPE_CTL, > -SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0); > -} > - > static void sun8i_mixer_commit(struct sunxi_engine *engine) > { > DRM_DEBUG_DRIVER("Committing changes\n"); > @@ -322,7 +311,6 @@ static struct drm_plane **sun8i_layers_init(struct > drm_device *drm, > } > > static const struct sunxi_engine_ops sun8i_engine_ops = { > - .atomic_begin = sun8i_mixer_atomic_begin, > .commit = sun8i_mixer_commit, > .layers_init= sun8i_layers_init, > }; > @@ -449,6 +437,9 @@ static int sun8i_mixer_bind(struct device *dev, struct > device *master, > regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_MODE(i), >SUN8I_MIXER_BLEND_MODE_DEF); > > + regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL, > +SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0); > + > return 0; > > err_disable_bus_clk: > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > index 518e1921f47e..28c15c6ef1ef 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > @@ -27,7 +27,8 @@ > #include "sun8i_ui_scaler.h" > > static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel, > - int overlay, bool enable, unsigned int zpos) > + int overlay, bool enable, unsigned int zpos, > + unsigned int old_zpos) > { > u32 val; > > @@ -43,6 +44,18 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer > *mixer, int channel, > SUN8I_MIXER_CHAN_UI_LAYER_ATTR(channel, overlay), > SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val); > > + if (!enable || zpos != old_zpos) { > + regmap_update_bits(mixer->engine.regs, > +SUN8I_MIXER_BLEND_PIPE_CTL, > +SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos), > +0); > + > + regmap_update_bits(mixer->engine.regs, > +SUN8I_MIXER_BLEND_ROUTE, > +SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos), > +0); > + } > + > if (enable) { > val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos); > > @@ -242,9 +255,11 @@ static void sun8i_ui_layer_atomic_disable(struct > drm_plane *plane, > struct drm_plane_state *old_state) > { > struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane); > + unsigned int old_zpos = old_state->normalized_zpos; > struct sun8i_mixer
[PATCH v2 2/3] ALSA: hda/i915: Associate audio component with devres
The HD-audio i915 binding code contains a single pointer, hdac_acomp, for allowing the access to audio component from the master bind/unbind callbacks. This was needed because the callbacks pass only the device pointer and we can't guarantee the object type assigned to the drvdata (which is free for each controller driver implementation). And this implementation will be a problem if we support multiple components for different DRM drivers, not only i915. As a solution, allocate the audio component object via devres and associate it with the given device, so that the component callbacks can refer to it via devres_find(). The removal of the object is still done half-manually via devres_destroy() to make the code consistent (although it may work without the explicit call). Also, the snd_hda_i915_register_notifier() had the reference to hdac_acomp as well. In this patch, the corresponding code is removed by passing hdac_bus object to the function, too. Signed-off-by: Takashi Iwai --- include/sound/hda_i915.h | 6 -- sound/hda/hdac_i915.c| 34 +- sound/pci/hda/patch_hdmi.c | 5 +++-- sound/soc/codecs/hdac_hdmi.c | 2 +- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h index f69ea84e7b65..648263791559 100644 --- a/include/sound/hda_i915.h +++ b/include/sound/hda_i915.h @@ -17,7 +17,8 @@ int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id, bool *audio_enabled, char *buffer, int max_bytes); int snd_hdac_i915_init(struct hdac_bus *bus); int snd_hdac_i915_exit(struct hdac_bus *bus); -int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *); +int snd_hdac_i915_register_notifier(struct hdac_bus *bus, + const struct drm_audio_component_audio_ops *ops); #else static inline int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable) { @@ -49,7 +50,8 @@ static inline int snd_hdac_i915_exit(struct hdac_bus *bus) { return 0; } -static inline int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *ops) +static inline int snd_hdac_i915_register_notifier(struct hdac_bus *bus, + const struct drm_audio_component_audio_ops *ops) { return -ENODEV; } diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c index 1a88c1aaf9bb..861b77bbc7df 100644 --- a/sound/hda/hdac_i915.c +++ b/sound/hda/hdac_i915.c @@ -22,7 +22,14 @@ #include #include -static struct drm_audio_component *hdac_acomp; +static void hdac_acomp_release(struct device *dev, void *res) +{ +} + +static struct drm_audio_component *hdac_get_acomp(struct device *dev) +{ + return devres_find(dev, hdac_acomp_release, NULL, NULL); +} /** * snd_hdac_set_codec_wakeup - Enable / disable HDMI/DP codec wakeup @@ -262,7 +269,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_acomp_get_eld); static int hdac_component_master_bind(struct device *dev) { - struct drm_audio_component *acomp = hdac_acomp; + struct drm_audio_component *acomp = hdac_get_acomp(dev); int ret; ret = component_bind_all(dev, acomp); @@ -294,7 +301,7 @@ static int hdac_component_master_bind(struct device *dev) static void hdac_component_master_unbind(struct device *dev) { - struct drm_audio_component *acomp = hdac_acomp; + struct drm_audio_component *acomp = hdac_get_acomp(dev); module_put(acomp->ops->owner); component_unbind_all(dev, acomp); @@ -314,6 +321,7 @@ static int hdac_component_master_match(struct device *dev, void *data) /** * snd_hdac_i915_register_notifier - Register i915 audio component ops + * @bus: HDA core bus * @aops: i915 audio component ops * * This function is supposed to be used only by a HD-audio controller @@ -323,12 +331,13 @@ static int hdac_component_master_match(struct device *dev, void *data) * * Returns zero for success or a negative error code. */ -int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *aops) +int snd_hdac_i915_register_notifier(struct hdac_bus *bus, + const struct drm_audio_component_audio_ops *aops) { - if (!hdac_acomp) + if (!bus->audio_component) return -ENODEV; - hdac_acomp->audio_ops = aops; + bus->audio_component->audio_ops = aops; return 0; } EXPORT_SYMBOL_GPL(snd_hdac_i915_register_notifier); @@ -365,18 +374,19 @@ int snd_hdac_i915_init(struct hdac_bus *bus) struct drm_audio_component *acomp; int ret; - if (WARN_ON(hdac_acomp)) + if (WARN_ON(hdac_get_acomp(dev))) return -EBUSY; if (!i915_gfx_present()) return -ENODEV; - i915_acomp = kzalloc(sizeof(*i915_acomp), GFP_KERNEL); + i915_acomp = devres_alloc(hdac_acomp_release,
[PATCH v2 0/3] Make the audio component binding more generic
Hi, this is a preliminiary patch set to convert the existing i915 / HD-audio component binding to be applicable to other drivers like radeon / amdgpu. This patchset itself doesn't change the functionality but only renames and split to a new drm_audio_component stuff from i915_audio_component. The actual usage of the new API will follow once after this one gets reviewed / accepted. The whole patches (including this patchset) are found in topic/hda-acomp branch of sound.git tree. BTW, since the whole stuff is about the audio binding, I suppose these will go through sound git tree. Let me know if anyone has concerns. Thanks! Takashi === v1->v2: * Change to SPDX for the new drm_audio_component.h * Fix remaining i915 word in drm_audio_component.h comments * Fix NULL dereference in master_bind / _unbind ops === Takashi Iwai (3): drm/i915: Split audio component to a generic type ALSA: hda/i915: Associate audio component with devres ALSA: hda: Make audio component support more generic drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/intel_audio.c | 22 +- include/drm/drm_audio_component.h | 118 ++ include/drm/i915_component.h | 85 +--- include/sound/hda_component.h | 61 ++ include/sound/hda_i915.h | 37 +--- include/sound/hdaudio.h| 6 +- sound/hda/Kconfig | 7 +- sound/hda/Makefile | 1 + sound/hda/hdac_component.c | 335 + sound/hda/hdac_i915.c | 335 ++--- sound/pci/hda/patch_hdmi.c | 57 +++-- sound/soc/codecs/hdac_hdmi.c | 10 +- 13 files changed, 607 insertions(+), 468 deletions(-) create mode 100644 include/drm/drm_audio_component.h create mode 100644 include/sound/hda_component.h create mode 100644 sound/hda/hdac_component.c -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] ALSA: hda: Make audio component support more generic
This is the final step for more generic support of DRM audio component. The generic audio component code is now moved to its own file, and the symbols are renamed from snd_hac_i915_* to snd_hdac_acomp_*, respectively. The generic code is enabled via the new kconfig, CONFIG_SND_HDA_COMPONENT, while CONFIG_SND_HDA_I915 is kept as the super-class. Along with the split, three new callbacks are added to audio_ops: pin2port is for providing the conversion between the pin number and the widget id, and master_bind/master_unbin are called at binding / unbinding the master component, respectively. All these are optional, but used in i915 implementation and also other later implementations. A note about the new snd_hdac_acomp_init() function: there is a slight difference between this and the old snd_hdac_i915_init(). The latter (still) synchronizes with the master component binding, i.e. it assures that the relevant DRM component gets bound when it returns, or gives a negative error. Meanwhile the new function doesn't synchronize but just leaves as is. It's the responsibility by the caller's side to synchronize, or the caller may accept the asynchronous binding on the fly. v1->v2: Fix missing NULL check in master_bind/unbind Signed-off-by: Takashi Iwai --- drivers/gpu/drm/i915/Kconfig | 1 + include/drm/drm_audio_component.h | 23 ++ include/sound/hda_component.h | 61 ++ include/sound/hda_i915.h | 39 +--- sound/hda/Kconfig | 7 +- sound/hda/Makefile| 1 + sound/hda/hdac_component.c| 335 + sound/hda/hdac_i915.c | 343 ++ sound/pci/hda/patch_hdmi.c| 50 +++-- sound/soc/codecs/hdac_hdmi.c | 8 +- 10 files changed, 486 insertions(+), 382 deletions(-) create mode 100644 include/sound/hda_component.h create mode 100644 sound/hda/hdac_component.c diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index dfd95889f4b7..5c607f2c707b 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -23,6 +23,7 @@ config DRM_I915 select SYNC_FILE select IOSF_MBI select CRC32 + select SND_HDA_I915 if SND_HDA_CORE help Choose this option if you have a system that has "Intel Graphics Media Accelerator" or "HD Graphics" integrated graphics, diff --git a/include/drm/drm_audio_component.h b/include/drm/drm_audio_component.h index e85689f212c2..4923b00328c1 100644 --- a/include/drm/drm_audio_component.h +++ b/include/drm/drm_audio_component.h @@ -4,6 +4,8 @@ #ifndef _DRM_AUDIO_COMPONENT_H_ #define _DRM_AUDIO_COMPONENT_H_ +struct drm_audio_component; + /** * struct drm_audio_component_ops - Ops implemented by DRM driver, called by hda driver */ @@ -72,6 +74,27 @@ struct drm_audio_component_audio_ops { * mode). */ void (*pin_eld_notify)(void *audio_ptr, int port, int pipe); + /** +* @pin2port: Check and convert from pin node to port number +* +* Called by HDA driver to check and convert from the pin widget node +* number to a port number in the graphics side. +*/ + int (*pin2port)(void *audio_ptr, int pin); + /** +* @master_bind: (Optional) component master bind callback +* +* Called at binding master component, for HDA codec-specific +* handling of dynamic binding. +*/ + int (*master_bind)(struct device *dev, struct drm_audio_component *); + /** +* @master_unbind: (Optional) component master unbind callback +* +* Called at unbinding master component, for HDA codec-specific +* handling of dynamic unbinding. +*/ + void (*master_unbind)(struct device *dev, struct drm_audio_component *); }; /** diff --git a/include/sound/hda_component.h b/include/sound/hda_component.h new file mode 100644 index ..78626cde7081 --- /dev/null +++ b/include/sound/hda_component.h @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: GPL-2.0 +// HD-Audio helpers to sync with DRM driver + +#ifndef __SOUND_HDA_COMPONENT_H +#define __SOUND_HDA_COMPONENT_H + +#include + +#ifdef CONFIG_SND_HDA_COMPONENT +int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable); +int snd_hdac_display_power(struct hdac_bus *bus, bool enable); +int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid, +int dev_id, int rate); +int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id, + bool *audio_enabled, char *buffer, int max_bytes); +int snd_hdac_acomp_init(struct hdac_bus *bus, + const struct drm_audio_component_audio_ops *aops, + int (*match_master)(struct device *, void *), + size_t extra_size); +int snd_hdac_acomp_exit(struct hdac_bus *bus); +int
Re: [PATCH] drm/imx: Replace drm_dev_unref with drm_dev_put
On Tue, 2018-07-17 at 10:33 +0200, Thomas Zimmermann wrote: > This patch unifies the naming of DRM functions for reference counting > of struct drm_device. The resulting code is more aligned with the rest > of the Linux kernel interfaces. > > Signed-off-by: Thomas Zimmermann Thank you, applied to imx-drm/next. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mediatek: Replace drm_dev_unref with drm_dev_put
On Tue, 2018-07-17 at 10:35 +0200, Thomas Zimmermann wrote: > This patch unifies the naming of DRM functions for reference counting > of struct drm_device. The resulting code is more aligned with the rest > of the Linux kernel interfaces. > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index a2ca90fc403c..5d024a154e1a 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -341,7 +341,7 @@ static int mtk_drm_bind(struct device *dev) > err_deinit: > mtk_drm_kms_deinit(drm); > err_free: > - drm_dev_unref(drm); > + drm_dev_put(drm); > return ret; > } > > @@ -350,7 +350,7 @@ static void mtk_drm_unbind(struct device *dev) > struct mtk_drm_private *private = dev_get_drvdata(dev); > > drm_dev_unregister(private->drm); > - drm_dev_unref(private->drm); > + drm_dev_put(private->drm); > private->drm = NULL; > } > > @@ -504,7 +504,7 @@ static int mtk_drm_remove(struct platform_device *pdev) > > drm_dev_unregister(drm); > mtk_drm_kms_deinit(drm); > - drm_dev_unref(drm); > + drm_dev_put(drm); > > component_master_del(>dev, _drm_ops); > pm_runtime_disable(>dev); Reviewed-by: Philipp Zabel regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] drm/nouveau: Fix bogus indenting in nouveau_hwmon.c
isn't there like 1 space missing for each change? Or maybe my client is messed up, but please align it with the first letter of the parameters not the "(". With that fixed: Reviewed-by: Karol Herbst On Tue, Jul 17, 2018 at 2:07 AM, Lyude Paul wrote: > Signed-off-by: Lyude Paul > --- > drivers/gpu/drm/nouveau/nouveau_hwmon.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c > b/drivers/gpu/drm/nouveau/nouveau_hwmon.c > index 44178b4c3599..d556f24c6c48 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c > +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c > @@ -655,7 +655,7 @@ nouveau_read_string(struct device *dev, enum > hwmon_sensor_types type, u32 attr, > > static int > nouveau_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, > - int channel, long > *val) > +int channel, long *val) > { > switch (type) { > case hwmon_chip: > @@ -677,7 +677,7 @@ nouveau_read(struct device *dev, enum hwmon_sensor_types > type, u32 attr, > > static int > nouveau_write(struct device *dev, enum hwmon_sensor_types type, u32 attr, > - int channel, long val) > + int channel, long val) > { > switch (type) { > case hwmon_temp: > -- > 2.17.1 > > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] drm/nouveau: Don't forget to label dp_aux devices
Reviewed-by: Karol Herbst 2018-07-12 19:13 GMT+02:00 Lyude Paul : > This makes debugging with DP tracing a lot harder to interpret, so name > each i2c based off the name of the encoder that it's for > > Signed-off-by: Lyude Paul > --- > drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_connector.c | 12 ++-- > drivers/gpu/drm/nouveau/nouveau_connector.h | 3 ++- > 4 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c > b/drivers/gpu/drm/nouveau/dispnv04/disp.c > index 501d2d290e9c..45ff1872d894 100644 > --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c > @@ -64,7 +64,7 @@ nv04_display_create(struct drm_device *dev) > for (i = 0; i < dcb->entries; i++) { > struct dcb_output *dcbent = >entry[i]; > > - connector = nouveau_connector_create(dev, dcbent->connector); > + connector = nouveau_connector_create(dev, dcbent); > if (IS_ERR(connector)) > continue; > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c > b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index 9382e99a0bc7..4f8d51590bbb 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -2198,7 +2198,7 @@ nv50_display_create(struct drm_device *dev) > > /* create encoder/connector objects based on VBIOS DCB table */ > for (i = 0, dcbe = >entry[0]; i < dcb->entries; i++, dcbe++) { > - connector = nouveau_connector_create(dev, dcbe->connector); > + connector = nouveau_connector_create(dev, dcbe); > if (IS_ERR(connector)) > continue; > > diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c > b/drivers/gpu/drm/nouveau/nouveau_connector.c > index 7b557c354307..0c5cc600c973 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_connector.c > +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c > @@ -408,8 +408,10 @@ nouveau_connector_destroy(struct drm_connector > *connector) > kfree(nv_connector->edid); > drm_connector_unregister(connector); > drm_connector_cleanup(connector); > - if (nv_connector->aux.transfer) > + if (nv_connector->aux.transfer) { > drm_dp_aux_unregister(_connector->aux); > + kfree(nv_connector->aux.name); > + } > kfree(connector); > } > > @@ -1201,13 +1203,16 @@ drm_conntype_from_dcb(enum dcb_connector_type dcb) > } > > struct drm_connector * > -nouveau_connector_create(struct drm_device *dev, int index) > +nouveau_connector_create(struct drm_device *dev, > +const struct dcb_output *dcbe) > { > const struct drm_connector_funcs *funcs = _connector_funcs; > struct nouveau_drm *drm = nouveau_drm(dev); > struct nouveau_display *disp = nouveau_display(dev); > struct nouveau_connector *nv_connector = NULL; > struct drm_connector *connector; > + char aux_name[48] = {0}; > + int index = dcbe->connector; > int type, ret = 0; > bool dummy; > > @@ -1306,6 +1311,9 @@ nouveau_connector_create(struct drm_device *dev, int > index) > case DRM_MODE_CONNECTOR_eDP: > nv_connector->aux.dev = dev->dev; > nv_connector->aux.transfer = nouveau_connector_aux_xfer; > + snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x", > +dcbe->hasht, dcbe->hashm); > + nv_connector->aux.name = kstrdup(aux_name, GFP_KERNEL); > ret = drm_dp_aux_register(_connector->aux); > if (ret) { > NV_ERROR(drm, "failed to register aux channel\n"); > diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h > b/drivers/gpu/drm/nouveau/nouveau_connector.h > index a4d1a059bd3d..2c5cb51c7c33 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_connector.h > +++ b/drivers/gpu/drm/nouveau/nouveau_connector.h > @@ -35,6 +35,7 @@ > #include "nouveau_crtc.h" > > struct nvkm_i2c_port; > +struct dcb_output; > > struct nouveau_connector { > struct drm_connector base; > @@ -76,7 +77,7 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc) > } > > struct drm_connector * > -nouveau_connector_create(struct drm_device *, int index); > +nouveau_connector_create(struct drm_device *, const struct dcb_output *); > > extern int nouveau_tv_disable; > extern int nouveau_ignorelid; > -- > 2.17.1 > > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [V3] vga_switcheroo: set audio client id according to bound GPU id
On Tue, 17 Jul 2018 10:56:37 +0200, jimqu wrote: > > > > On 2018年07月17日 16:52, Takashi Iwai wrote: > > On Tue, 17 Jul 2018 10:38:58 +0200, > > Lukas Wunner wrote: > >> On Tue, Jul 17, 2018 at 04:20:50PM +0800, Jim Qu wrote: > >>> On modern laptop, there are more and more platforms > >>> have two GPUs, and each of them maybe have audio codec > >>> for HDMP/DP output. For some dGPU which is no output, > >>> audio codec usually is disabled. > >>> > >>> In currect HDA audio driver, it will set all codec as > >>> VGA_SWITCHEROO_DIS, the audio which is binded to UMA > >>> will be suspended if user use debugfs to contorl power > >>> > >>> In HDA driver side, it is difficult to know which GPU > >>> the audio has binded to. So set the bound gpu pci dev > >>> to vga_switcheroo. > >>> > >>> if the audio client is not the third registration, audio > >>> id will set in vga_switcheroo enable function. if the > >>> audio client is the last registration when vga_switcheroo > >>> _ready() get true, we should get audio client id from bound > >>> GPU directly. > >>> > >>> Signed-off-by: Jim Qu > >> Reviewed-by: Lukas Wunner > >> > >> @Takashi, any preference which tree to merge this through? > >> sound or drm-misc, either way would seem fine to me. I think > >> there's going to be one final drm-misc pull sent to Dave this > >> week, after that it's 4.20. > > Since it's basically an audio problem and I'd love to merge it for > > 4.19, I'd prefer taking through sound git tree, unless anyone has > > objection. > > Thanks to Takashi and Lukas great help. Please kindly help merge the > patch into suitable branch. I pushed the fix to topic/vga_switcheroo branch of sound git tree now so that 0day bot can check it. I'll wait for a while and merge it later to for-next branch if nothing happens. The brach is (and will be) based on fresh 4.18-rc5 so that other trees may merge it cleanly. thanks, Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] drm/i915: Split audio component to a generic type
For allowing other drivers to use the DRM audio component, rename the i915_audio_component_* with drm_audio_component_*, and split the generic part into drm_audio_component.h. The i915 specific stuff remains in struct i915_audio_component, which contains drm_audio_component as the base. The license of drm_audio_component.h is kept to MIT as same as the the original i915_component.h. This is a preliminary change for further development, and no functional changes by this patch itself, merely code-split and renames. v1->v2: Use SPDX for drm_audio_component.h, fix remaining i915 argument in drm_audio_component.h Signed-off-by: Takashi Iwai --- drivers/gpu/drm/i915/intel_audio.c | 22 +++ include/drm/drm_audio_component.h | 95 ++ include/drm/i915_component.h | 85 ++ include/sound/hda_i915.h | 6 +- include/sound/hdaudio.h| 6 +- sound/hda/hdac_i915.c | 40 +++-- sound/pci/hda/patch_hdmi.c | 8 +-- sound/soc/codecs/hdac_hdmi.c | 2 +- 8 files changed, 144 insertions(+), 120 deletions(-) create mode 100644 include/drm/drm_audio_component.h diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 3ea566f99450..7dd5605d94ae 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -639,11 +639,12 @@ void intel_audio_codec_enable(struct intel_encoder *encoder, dev_priv->av_enc_map[pipe] = encoder; mutex_unlock(_priv->av_mutex); - if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) { + if (acomp && acomp->base.audio_ops && + acomp->base.audio_ops->pin_eld_notify) { /* audio drivers expect pipe = -1 to indicate Non-MST cases */ if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) pipe = -1; - acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, + acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr, (int) port, (int) pipe); } @@ -681,11 +682,12 @@ void intel_audio_codec_disable(struct intel_encoder *encoder, dev_priv->av_enc_map[pipe] = NULL; mutex_unlock(_priv->av_mutex); - if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) { + if (acomp && acomp->base.audio_ops && + acomp->base.audio_ops->pin_eld_notify) { /* audio drivers expect pipe = -1 to indicate Non-MST cases */ if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST)) pipe = -1; - acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, + acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr, (int) port, (int) pipe); } @@ -880,7 +882,7 @@ static int i915_audio_component_get_eld(struct device *kdev, int port, return ret; } -static const struct i915_audio_component_ops i915_audio_component_ops = { +static const struct drm_audio_component_ops i915_audio_component_ops = { .owner = THIS_MODULE, .get_power = i915_audio_component_get_power, .put_power = i915_audio_component_put_power, @@ -897,12 +899,12 @@ static int i915_audio_component_bind(struct device *i915_kdev, struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev); int i; - if (WARN_ON(acomp->ops || acomp->dev)) + if (WARN_ON(acomp->base.ops || acomp->base.dev)) return -EEXIST; drm_modeset_lock_all(_priv->drm); - acomp->ops = _audio_component_ops; - acomp->dev = i915_kdev; + acomp->base.ops = _audio_component_ops; + acomp->base.dev = i915_kdev; BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS); for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++) acomp->aud_sample_rate[i] = 0; @@ -919,8 +921,8 @@ static void i915_audio_component_unbind(struct device *i915_kdev, struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev); drm_modeset_lock_all(_priv->drm); - acomp->ops = NULL; - acomp->dev = NULL; + acomp->base.ops = NULL; + acomp->base.dev = NULL; dev_priv->audio_component = NULL; drm_modeset_unlock_all(_priv->drm); } diff --git a/include/drm/drm_audio_component.h b/include/drm/drm_audio_component.h new file mode 100644 index ..e85689f212c2 --- /dev/null +++ b/include/drm/drm_audio_component.h @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: MIT +// Copyright © 2014 Intel Corporation + +#ifndef _DRM_AUDIO_COMPONENT_H_ +#define _DRM_AUDIO_COMPONENT_H_ + +/** + * struct drm_audio_component_ops - Ops implemented by DRM driver, called by hda driver + */ +struct drm_audio_component_ops { + /** +* @owner:
[PATCH] drm/amdgpu: fix spelling mistake "successed" -> "succeeded"
From: Colin Ian King Trivial fix to spelling mistake in dev_err error message. Signed-off-by: Colin Ian King --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 9883fa9bb41b..e9feb3c58389 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2004,7 +2004,7 @@ static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev) continue; r = block->version->funcs->hw_init(adev); - DRM_INFO("RE-INIT: %s %s\n", block->version->funcs->name, r?"failed":"successed"); + DRM_INFO("RE-INIT: %s %s\n", block->version->funcs->name, r?"failed":"succeeded"); if (r) return r; } @@ -2039,7 +2039,7 @@ static int amdgpu_device_ip_reinit_late_sriov(struct amdgpu_device *adev) continue; r = block->version->funcs->hw_init(adev); - DRM_INFO("RE-INIT: %s %s\n", block->version->funcs->name, r?"failed":"successed"); + DRM_INFO("RE-INIT: %s %s\n", block->version->funcs->name, r?"failed":"succeeded"); if (r) return r; } @@ -3092,7 +3092,7 @@ static int amdgpu_device_handle_vram_lost(struct amdgpu_device *adev) * @adev: amdgpu device pointer * * attempt to do soft-reset or full-reset and reinitialize Asic - * return 0 means successed otherwise failed + * return 0 means succeeded otherwise failed */ static int amdgpu_device_reset(struct amdgpu_device *adev) { @@ -3170,7 +3170,7 @@ static int amdgpu_device_reset(struct amdgpu_device *adev) * @from_hypervisor: request from hypervisor * * do VF FLR and reinitialize Asic - * return 0 means successed otherwise failed + * return 0 means succeeded otherwise failed */ static int amdgpu_device_reset_sriov(struct amdgpu_device *adev, bool from_hypervisor) @@ -3295,7 +3295,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev, dev_info(adev->dev, "GPU reset(%d) failed\n", atomic_read(>gpu_reset_counter)); amdgpu_vf_error_put(adev, AMDGIM_ERROR_VF_GPU_RESET_FAIL, 0, r); } else { - dev_info(adev->dev, "GPU reset(%d) successed!\n",atomic_read(>gpu_reset_counter)); + dev_info(adev->dev, "GPU reset(%d) succeeded!\n",atomic_read(>gpu_reset_counter)); } amdgpu_vf_error_trans_all(adev); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/tve200: Replace drm_dev_unref with drm_dev_put
On Tue, Jul 17, 2018 at 10:53 AM Thomas Zimmermann wrote: > This patch unifies the naming of DRM functions for reference counting > of struct drm_device. The resulting code is more aligned with the rest > of the Linux kernel interfaces. > > Signed-off-by: Thomas Zimmermann Reviewed-by: Linus Walleij Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 21682] White screen with compiz/AIGLX on X.org server 1.5.2 when running on 16bpp (intel 945GM)
https://bugs.freedesktop.org/show_bug.cgi?id=21682 Stefan Dirsch changed: What|Removed |Added Resolution|--- |WONTFIX Status|NEEDINFO|RESOLVED --- Comment #25 from Stefan Dirsch --- compiz is no longer been shipped by any still supported Linux distribution I guess. Also 945GM were still 32 bit machines. Probably no longer been used by anyone. Let's better close this one as WONTFIX. -- 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: [Nouveau] [PATCH 2/5] drm/nouveau: Grab RPM ref while probing outputs
Reviewed-by: Karol Herbst On Tue, Jul 17, 2018 at 9:21 AM, Lukas Wunner wrote: > On Mon, Jul 16, 2018 at 07:59:26PM -0400, Lyude Paul wrote: >> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c >> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c >> @@ -2012,10 +2012,18 @@ nv50_disp_atomic_state_alloc(struct drm_device *dev) >> return >state; >> } >> >> +static void >> +nouveau_output_poll_changed(struct drm_device *dev) >> +{ >> + pm_runtime_get_sync(dev->dev); >> + drm_fb_helper_hotplug_event(dev->fb_helper); >> + pm_runtime_put_autosuspend(dev->dev); >> +} >> + >> static const struct drm_mode_config_funcs >> nv50_disp_func = { >> .fb_create = nouveau_user_framebuffer_create, >> - .output_poll_changed = drm_fb_helper_output_poll_changed, >> + .output_poll_changed = nouveau_output_poll_changed, > > It might make sense to provide a generic DRM helper for this. > Same for patch 3 in this series. > > Thanks, > > Lukas > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 3/5] drm/nouveau: Add missing RPM get/put() when probing connectors
Reviewed-by: Karol Herbst On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul wrote: > While the GPU is guaranteed to be on when a hotplug has been received, > the same assertion does not hold true if a connector probe has been > started by userspace without having had received a sysfs event. So > ensure that any connector probing keeps the GPU alive for the duration > of the probe. > > Signed-off-by: Lyude Paul > Cc: Karol Herbst > Cc: sta...@vger.kernel.org > --- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_connector.c | 21 +++-- > drivers/gpu/drm/nouveau/nouveau_connector.h | 3 +++ > 3 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c > b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index ea2a886854fe..0f283ca75189 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -858,7 +858,7 @@ static const struct drm_connector_funcs > nv50_mstc = { > .reset = nouveau_conn_reset, > .detect = nv50_mstc_detect, > - .fill_modes = drm_helper_probe_single_connector_modes, > + .fill_modes = nouveau_connector_probe_single_connector_modes, > .destroy = nv50_mstc_destroy, > .atomic_duplicate_state = nouveau_conn_atomic_duplicate_state, > .atomic_destroy_state = nouveau_conn_atomic_destroy_state, > diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c > b/drivers/gpu/drm/nouveau/nouveau_connector.c > index 2a45b4c2ceb0..feb142fb7a8a 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_connector.c > +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c > @@ -770,6 +770,23 @@ nouveau_connector_force(struct drm_connector *connector) > nouveau_connector_set_encoder(connector, nv_encoder); > } > > +int > +nouveau_connector_probe_single_connector_modes(struct drm_connector > *connector, > + uint32_t maxX, uint32_t maxY) > +{ > + struct device *dev = connector->dev->dev; > + int ret; > + > + ret = pm_runtime_get_sync(dev); > + if (WARN_ON(ret < 0 && ret != -EACCES)) > + return 0; > + > + ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY); > + > + pm_runtime_put_autosuspend(dev); > + return ret; > +} > + > static int > nouveau_connector_set_property(struct drm_connector *connector, >struct drm_property *property, uint64_t value) > @@ -1088,7 +1105,7 @@ nouveau_connector_funcs = { > .reset = nouveau_conn_reset, > .detect = nouveau_connector_detect, > .force = nouveau_connector_force, > - .fill_modes = drm_helper_probe_single_connector_modes, > + .fill_modes = nouveau_connector_probe_single_connector_modes, > .set_property = nouveau_connector_set_property, > .destroy = nouveau_connector_destroy, > .atomic_duplicate_state = nouveau_conn_atomic_duplicate_state, > @@ -1103,7 +1120,7 @@ nouveau_connector_funcs_lvds = { > .reset = nouveau_conn_reset, > .detect = nouveau_connector_detect_lvds, > .force = nouveau_connector_force, > - .fill_modes = drm_helper_probe_single_connector_modes, > + .fill_modes = nouveau_connector_probe_single_connector_modes, > .set_property = nouveau_connector_set_property, > .destroy = nouveau_connector_destroy, > .atomic_duplicate_state = nouveau_conn_atomic_duplicate_state, > diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h > b/drivers/gpu/drm/nouveau/nouveau_connector.h > index 2d9d35a146a4..e9ffc6eebda2 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_connector.h > +++ b/drivers/gpu/drm/nouveau/nouveau_connector.h > @@ -106,6 +106,9 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc) > > struct drm_connector * > nouveau_connector_create(struct drm_device *, const struct dcb_output *); > +int > +nouveau_connector_probe_single_connector_modes(struct drm_connector *, > + uint32_t, uint32_t); > > extern int nouveau_tv_disable; > extern int nouveau_ignorelid; > -- > 2.17.1 > > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 4/5] drm/nouveau: Grab RPM ref when i2c bus is in use
mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd the Linux glue code to the i2c stuff instead, but this is all done from inside of nvkm. I think we should move it out into drm/nouveau/nouveau_i2c.c and do the handling there. On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul wrote: > The i2c bus can be both accessed by DRM itself, along with any of it's > devnodes (/sys/class/i2c). So, we need to make sure that all codepaths > using the i2c bus keep the GPU resumed. > > Signed-off-by: Lyude Paul > Cc: Karol Herbst > Cc: sta...@vger.kernel.org > --- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > index 807a2b67bd64..1de48c990b80 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus) > BUS_TRACE(bus, "release"); > nvkm_i2c_pad_release(pad); > mutex_unlock(>mutex); > + pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev); > } > > int > nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus) > { > struct nvkm_i2c_pad *pad = bus->pad; > + struct device *dev = pad->i2c->subdev.device->dev; > int ret; > + > BUS_TRACE(bus, "acquire"); > + > + ret = pm_runtime_get_sync(dev); > + if (ret < 0 && ret != -EACCES) > + return ret; > + > mutex_lock(>mutex); > ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C); > - if (ret) > + if (ret) { > mutex_unlock(>mutex); > + pm_runtime_put_autosuspend(dev); > + } > return ret; > } > > -- > 2.17.1 > > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel